✨ (signer-btc) [DSDK-471]: SignPsbt task #581

jdabbech-ledger · 2024-12-27T17:42:42Z

📝 Description

Create CommandErrorHelper to handle command error codes in parse response
Refactor SignMessage with ContinueTask
Create SignPsbt task & DA
Update sample app with Sign Psbt

❓ Context

JIRA or GitHub link: DSDK-471 DSDK-469 DSDK-478 DSDK-483

Feature:

✅ Checklist

Pull Requests must pass CI checks and undergo code review. Set the PR as Draft if it is not yet ready for review.

Covered by automatic tests
Changeset is provided
Impact of the changes:
- list of the changes

🧐 Checklist for the PR Reviewers

The code aligns with the requirements described in the linked JIRA or GitHub issue.
The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
There are no undocumented trade-offs, technical debt, or maintainability issues.
The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
Any new dependencies have been justified and documented.

vercel · 2024-12-27T17:42:47Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
device-sdk-ts-sample	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 10, 2025 7:16pm

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
doc-device-management-kit	⬜️ Ignored (Inspect)	Visit Preview		Jan 10, 2025 7:16pm

github-actions · 2024-12-27T17:43:46Z

	Messages
✅	Danger: All checks passed successfully! 🎉

Generated by 🚫 dangerJS against 242b0c1

aussedatlo · 2025-01-08T15:51:16Z

apps/sample/src/components/SignerBtcView/index.tsx

+        },
+        initialValues: {
+          derivationPath: DEFAULT_DERIVATION_PATH,
+          psbt: "70736274ff0104010101fb0402000000010204020000000105010100011004000000000103040100000001007102000000013daeeb9a92e7b5af90c787d53f0e60d2cf4cfd47bca9a0d8bc77a7464b024c0b00000000000000000002ff0300000000000016001402fe597c6ec0e2982712929bcf079a4e11d37e8d950b0000000000001600144dc432cb6a26c52a1e6ddd2bcf0ee49199fae0cc000000002206031869567d5e88d988ff7baf6827983f89530ddd79dbaeadaa6ec538a8f03dea8b18f5acc2fd540000800000008000000080000000000000000001011fff0300000000000016001402fe597c6ec0e2982712929bcf079a4e11d37e8d010e200cf08d04fa11ff024d5a50165ba65e495409b50ba6657788dfa15274adb682df010f0400000000000103086b01000000000000010416001429159115f12bb6a7e977439c83d3f8d555d72d5f00",


[COULD] Remove the default psbt here

ok, it's a heartache after so much difficulties to create it 😿

aussedatlo · 2025-01-08T15:52:31Z

packages/signer/signer-btc/src/api/app-binder/SignPsbtDeviceActionTypes.ts

+
+export type SignPsbtDAInternalState = {
+  readonly error: SignPsbtDAError | null;
+  // [SHOULD] be psbt instead of signature


[ASK] What to do with this comment ?

The next task I'm working on, UpdatePsbtTask, will modify this device action to return a psbt instead of a Signature

actually as discussed, we won't return a PSBT but the list of signature, with according index/tag/pubkey_augm (typing to be refined)

aussedatlo · 2025-01-08T15:55:26Z

packages/signer/signer-btc/src/internal/app-binder/command/GetWalletAddressCommand.ts

 {
-  constructor(private readonly args: GetWalletAddressCommandArgs) {}
+  constructor(
+    private readonly args: GetWalletAddressCommandArgs,


[COULD] rename args to _args

aussedatlo · 2025-01-08T15:56:55Z

packages/signer/signer-btc/src/internal/app-binder/command/SignMessageCommand.ts

@@ -41,19 +37,26 @@ export type SignMessageCommandArgs = {
  readonly messageMerkleRoot: Uint8Array;
 };

-export type SignMessageCommandResponse = Signature | ApduResponse;
+export type SignMessageCommandResponse = ApduResponse;


[ASK] Why returning an ApduResponse instead of a Signature ?

To handle the signature only in the task, with a client interpreter flow as the it should respond with those client commands.

fAnselmi-Ledger · 2025-01-09T11:15:23Z

packages/signer/signer-btc/src/internal/use-cases/sign-psbt/SignPsbtUseCase.ts

+      psbt,
+    });
+  }
+}


[SHOULD] test file missing for this!

paoun-ledger · 2025-01-10T14:32:34Z

apps/sample/src/components/SignerBtcView/index.tsx

+          return signer.signPsbt(
+            new DefaultWallet(
+              derivationPath,
+              DefaultDescriptorTemplate.NATIVE_SEGWIT,


ideally we should be able to select the template we want to test. BOr another possibility would be to deduce it from the derivation path since we only support standard wallets for now, and derivation types also follow the standard as described here: https://learnmeabitcoin.com/technical/keys/hd-wallets/derivation-paths/

m/44'/... -> legacy

m/49'/... -> nested segwit

m/84'/... -> native segwit

m/86'/... -> taproot

(also note that if derivation path and descriptor template are not consistent, it will be rejected by the device anyway)

ok I'll update the device action command form with a render prop to allow custom inputs

paoun-ledger · 2025-01-10T14:53:08Z

packages/signer/signer-btc/src/internal/app-binder/task/SignPsbtTask.ts

+  ) {
+    const hasher = new Sha256HasherService();
+    this._walletSerializer =
+      walletSerializer || new DefaultWalletSerializer(hasher);


[COULD] we usually don't instantiate manually services like that. In ethereum signer, we use inversify to inject it in the app binder, and then give it as an input of the device action:
https://github.com/LedgerHQ/device-sdk-ts/blob/develop/packages/signer/signer-eth/src/internal/app-binder/EthAppBinder.ts#L35
That would make it easier to change the implementation later on if needed, and keep consistency to inject with inversify in di folder.
WDYT?

Yep it's definitely not the best way to inject the dependancy here but wasn't sure how to plug the task with inversify.
Will implement the same way as it's done in signer-eth then 👍

jdabbech-ledger requested a review from a team as a code owner December 27, 2024 17:42

github-actions bot assigned jdabbech-ledger Dec 27, 2024

vercel bot deployed to Preview – device-sdk-ts-sample December 27, 2024 17:43 View deployment

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from 9cce5af to 47b61b5 Compare January 2, 2025 10:46

vercel bot deployed to Preview – device-sdk-ts-sample January 2, 2025 10:47 View deployment

vercel bot deployed to Preview – device-sdk-ts-sample January 2, 2025 11:12 View deployment

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from 0034834 to 7182bae Compare January 2, 2025 11:48

vercel bot deployed to Preview – device-sdk-ts-sample January 2, 2025 11:49 View deployment

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from 7182bae to 76bdcfd Compare January 2, 2025 17:25

vercel bot deployed to Preview – device-sdk-ts-sample January 2, 2025 17:27 View deployment

vercel bot had a problem deploying to Preview – device-sdk-ts-sample January 6, 2025 10:12 Failure

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from f08eb42 to c922a05 Compare January 6, 2025 10:24

vercel bot had a problem deploying to Preview – device-sdk-ts-sample January 6, 2025 10:26 Failure

jdabbech-ledger added 9 commits January 8, 2025 16:26

🎨 (dmk): Export CommandSuccessResult

a342407

✨ (signer-utils): Create CommandErrorHelper

4ab77ac

✨ (signer-btc): Use of CommandErrorHelper

758c7f9

♻️ (signer-btc): Create ContinueTask & use it for SignMessage

4d156b7

✨ (signer-btc): Create SignPsbt DA & task

562e05f

💄 (smpl): SignPsbt view

42114ec

🐛 (dmk): Fix CommandUtils static calls

5ee3e96

🐛 (signer-btc): Fix wallet & pub key

cee7864

✨ (signer-btc): Get PSBT signature

0663f38

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from c922a05 to 0663f38 Compare January 8, 2025 15:26

vercel bot deployed to Preview – device-sdk-ts-sample January 8, 2025 15:30 View deployment

aussedatlo reviewed Jan 8, 2025

View reviewed changes

fAnselmi-Ledger reviewed Jan 9, 2025

View reviewed changes

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from 02dd40e to f9c201f Compare January 9, 2025 15:21

vercel bot deployed to Preview – device-sdk-ts-sample January 9, 2025 15:24 View deployment

🎨 (signer-btc): Reviews

4f01dfe

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from f9c201f to 4f01dfe Compare January 10, 2025 12:23

vercel bot deployed to Preview – device-sdk-ts-sample January 10, 2025 12:25 View deployment

paoun-ledger reviewed Jan 10, 2025

View reviewed changes

vercel bot deployed to Preview – device-sdk-ts-sample January 10, 2025 16:12 View deployment

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from 9af8aea to cfdf8dd Compare January 10, 2025 17:13

vercel bot deployed to Preview – device-sdk-ts-sample January 10, 2025 17:15 View deployment

💄 (smpl): Custom device action input form for signpsbt

242b0c1

jdabbech-ledger force-pushed the feat/dsdk-471-sign-psbt-task branch from cfdf8dd to 242b0c1 Compare January 10, 2025 19:15

vercel bot deployed to Preview – device-sdk-ts-sample January 10, 2025 19:16 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ (signer-btc) [DSDK-471]: SignPsbt task #581

✨ (signer-btc) [DSDK-471]: SignPsbt task #581

jdabbech-ledger commented Dec 27, 2024 •

edited by jira bot

Loading

vercel bot commented Dec 27, 2024 •

edited

Loading

github-actions bot commented Dec 27, 2024 •

edited

Loading

aussedatlo Jan 8, 2025

jdabbech-ledger Jan 9, 2025

aussedatlo Jan 8, 2025

jdabbech-ledger Jan 9, 2025

paoun-ledger Jan 10, 2025

aussedatlo Jan 8, 2025

aussedatlo Jan 8, 2025

jdabbech-ledger Jan 9, 2025

fAnselmi-Ledger Jan 9, 2025

paoun-ledger Jan 10, 2025

jdabbech-ledger Jan 10, 2025 •

edited

Loading

paoun-ledger Jan 10, 2025

jdabbech-ledger Jan 10, 2025

✨ (signer-btc) [DSDK-471]: SignPsbt task #581

Are you sure you want to change the base?

✨ (signer-btc) [DSDK-471]: SignPsbt task #581

Conversation

jdabbech-ledger commented Dec 27, 2024 • edited by jira bot Loading

📝 Description

❓ Context

✅ Checklist

🧐 Checklist for the PR Reviewers

vercel bot commented Dec 27, 2024 • edited Loading

github-actions bot commented Dec 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdabbech-ledger Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdabbech-ledger commented Dec 27, 2024 •

edited by jira bot

Loading

vercel bot commented Dec 27, 2024 •

edited

Loading

github-actions bot commented Dec 27, 2024 •

edited

Loading

jdabbech-ledger Jan 10, 2025 •

edited

Loading