feat: add ledger methods for faucet spending #6409

hansieodendaal · 2024-07-17T08:54:40Z

Description

Added ledger methods to support faucet spending.
Consolidated h/w ledger support into the minotari_ledger_wallet_comms crate.
Added test methods for all h/w ledger functions in the form of an example.
Addedd a seperate crate for common types shared between the Ledger application and the rest of the code base.

Note: I do not think the ledger feature (#[cfg(feature = "ledger")]) is nesseccary anymore. Any opinions?

Motivation and Context

Faucet spending was not enabled on ledger devices.

How Has This Been Tested?

Compiled the ledger project minotari_ledger_wallet, uploaded the binaries to a ledger nano s device, then ran cargo run --release --example ledger_demo.

Transport created in 103.1686ms
Transport created in 5.8315ms
Transport created in 6.0115ms
Transport created in 5.9472ms
Transport created in 5.9368ms
Transport created in 5.8717ms
Transport created in 5.9897ms
Transport created in 6.0287ms
Transport created in 5.9255ms
Transport created in 5.9359ms

Application verified in 1.293162s
Application verified in 200ns
Application verified in 200ns
Application verified in 100ns
Application verified in 200ns
Application verified in 400ns
Application verified in 10.2µs
Application verified in 200ns
Application verified in 100ns
Application verified in 100ns


test: GetAppName
app name:       minotari_ledger_wallet

test: GetVersion
version:        1.0.0-pre.16

test: GetPublicAlpha
public_alpha:   ea10ad3d125beb2c1ebfa2d295ee8e42fd4a85ad831886f8b0b408f8390a9e3c

test: GetPublicKey
public_key:     4a01ab9c27a256827c6ee85373412f80cc88040425afe6301c9b1c7773356e2a

test: GetScriptSignature
script_sig:     (3058744ade0c70a5fe914ed64695c82bc4c71477f8df46578fb3780c06c8110a,5e3fb09be68a31f397330037b6fc5b09227f1a89f74c66fe72fa34c507b90e2b,ed1a3a74f17ae19d70252a2d0ca92a0295f1f0c43565653b4f1fea9ffac32c0d,4f73a5c7cb723b12eca2c3ff57b8156ccacf9ed2790ac2b2e6c033147536e30d,7e8abeb6774fa9e63b181b54e47f09198116d02ec2c1f536c977cf8254d69a09)

test: GetScriptOffset
script_offset:  3a46a979ec9c2d94fbb55e09221fd0deb9e869dc849975e5ee163fb36bbe0709

test: GetViewKey
view_key:       158e9d986d15c40ac3f6178f5947fde0cbd4f71f7de01c00c88cfa570e648e09

test: GetDHSharedSecret
shared_secret:  e85e660bd4d55f4d57784c3592e1ed063c77b8b1aa37431c078e8a7e18c8d23a

test: GetRawSchnorrSignature
signature:      (2971d25b5920bc7a102a9d22c652fb1cf8da85ef08b8464dca6a363612040903,0a7f70764654239c51590d0f32b3b23368e355bb269d99cc151e1c261db56156)

test: GetScriptSchnorrSignature
signature:      (c1fce9daf50cc65e69440f3f4383db1f64b1fd190bdcf7824c9dce0dcd68c208,ace0f952697b2496e60deadee0d40a23c263fa0d73c66a2742767fadad61fe33)

test: Ledger app not running
✔ Exit the 'MinoTari Wallet' Ledger app and press Enter to continue.. · Ok

test: Ledger disconnected
✔ Disconnect the Ledger device and press Enter to continue.. · Ok

test: Ledger reconnected
✔ Reconnect the Ledger device (with password) and press Enter to continue.. · Ok

test: Ledger app restart
✔ Start the 'MinoTari Wallet' Ledger app and press Enter to continue.. · Ok
view_key:       158e9d986d15c40ac3f6178f5947fde0cbd4f71f7de01c00c88cfa570e648e09

Test completed successfully

What process can a PR reviewer use to test or verify this change?

Code review.
Compile the ledger project minotari_ledger_wallet, upload the binaries to a ledger nano s device, then run cargo run --release --example ledger_demo.

Breaking Changes

None
Requires data directory on base node to be deleted
Requires hard fork
Other - Please specify

github-actions · 2024-07-17T09:05:51Z

Test Results (CI)

3 files 123 suites 44m 26s ⏱️
1 298 tests 1 298 ✅ 0 💤 0 ❌
3 886 runs 3 886 ✅ 0 💤 0 ❌

Results for commit 30dcdee.

♻️ This comment has been updated with latest results.

github-actions · 2024-07-17T09:29:47Z

Test Results (Integration tests)

2 files + 2 11 suites +11 24m 24s ⏱️ + 24m 24s
35 tests +35 33 ✅ +33 0 💤 ±0 2 ❌ +2
38 runs +38 34 ✅ +34 0 💤 ±0 4 ❌ +4

For more details on these failures, see this check.

Results for commit 422c99a. ± Comparison against base commit 1bd2fde.

♻️ This comment has been updated with latest results.

SWvheerden

This looks good
A few missed name changes but the key concept here is correct

applications/minotari_console_wallet/src/automation/commands.rs

applications/minotari_ledger_wallet/comms/src/ledger_wallet.rs

applications/minotari_ledger_wallet/wallet/src/handlers/get_schnorr_signature.rs

SWvheerden · 2024-07-23T18:23:08Z

applications/minotari_ledger_wallet/comms/src/ledger_wallet.rs

 }

+#[repr(u16)]
+#[derive(FromPrimitive, Debug, Copy, Clone, PartialEq)]
+pub enum AppSW {


this is delcared twice?

Yes, not ideal. To solve this I created a seperate crate for common types shared between the Ledger application and the rest of the code base.

base_layer/core/src/transactions/key_manager/inner.rs

- Added ledger methods to support faucet spending. - Consolidated h/w ledger support into the `minotari_ledger_wallet_comms` crate. - Added test methods for all h/w ledger functions in the form of an example.

hansieodendaal requested a review from a team as a code owner July 17, 2024 08:54

hansieodendaal marked this pull request as draft July 17, 2024 08:54

ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Jul 17, 2024

hansieodendaal force-pushed the ho_add_ledger_methods branch from 58dd9e9 to 63b5981 Compare July 19, 2024 13:33

ghpbot-tari-project added the CR-too_long Changes Requested - Your PR is too long label Jul 19, 2024

hansieodendaal force-pushed the ho_add_ledger_methods branch 9 times, most recently from d30ef8f to 86a80d3 Compare July 22, 2024 16:14

hansieodendaal changed the title ~~[wip] feat:add ledger methods~~ feat:add ledger methods Jul 22, 2024

hansieodendaal marked this pull request as ready for review July 22, 2024 16:15

hansieodendaal changed the title ~~feat:add ledger methods~~ feat: add ledger methods for faucet spending Jul 23, 2024

hansieodendaal force-pushed the ho_add_ledger_methods branch 3 times, most recently from 48671e5 to 6ad33e0 Compare July 23, 2024 03:15

SWvheerden reviewed Jul 23, 2024

View reviewed changes

hansieodendaal added 3 commits July 24, 2024 16:29

Add ledger methods

dedcca6

- Added ledger methods to support faucet spending. - Consolidated h/w ledger support into the `minotari_ledger_wallet_comms` crate. - Added test methods for all h/w ledger functions in the form of an example.

ledger include disconnect/reconnect

9378393

review comments

200b7dd

hansieodendaal force-pushed the ho_add_ledger_methods branch from 2bf3c28 to 200b7dd Compare July 24, 2024 14:35

wip

30dcdee

SWvheerden approved these changes Jul 24, 2024

View reviewed changes

ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Jul 24, 2024

SWvheerden merged commit 80acbd2 into tari-project:development Jul 24, 2024
14 of 15 checks passed

hansieodendaal deleted the ho_add_ledger_methods branch July 24, 2024 15:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add ledger methods for faucet spending #6409

feat: add ledger methods for faucet spending #6409

hansieodendaal commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

SWvheerden left a comment

SWvheerden Jul 23, 2024

hansieodendaal Jul 24, 2024

feat: add ledger methods for faucet spending #6409

feat: add ledger methods for faucet spending #6409

Conversation

hansieodendaal commented Jul 17, 2024 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

What process can a PR reviewer use to test or verify this change?

Breaking Changes

github-actions bot commented Jul 17, 2024 • edited Loading

Test Results (CI)

github-actions bot commented Jul 17, 2024 • edited Loading

Test Results (Integration tests)

SWvheerden left a comment

Choose a reason for hiding this comment

SWvheerden Jul 23, 2024

Choose a reason for hiding this comment

hansieodendaal Jul 24, 2024

Choose a reason for hiding this comment

hansieodendaal commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading