🎨 (signer-btc): Reviews

LedgerHQ · Jan 17, 2025 · b78c6e2 · b78c6e2
1 parent 06753e0
commit b78c6e2
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 34 deletions.
diff --git a/packages/signer/signer-btc/src/internal/app-binder/task/ExtractTransactionTask.test.ts b/packages/signer/signer-btc/src/internal/app-binder/task/ExtractTransactionTask.test.ts
@@ -77,7 +77,7 @@ describe("ExtractTransactionTask", () => {
     // then
     expect(tx).toStrictEqual(
       CommandResultFactory({
-        data: "0x0000000000010108000000000293980000000000203009080706",
+        data: "0x000000000001010800000000029398000000000100000000000000000109203009080706",
       }),
     );
   });

diff --git a/packages/signer/signer-btc/src/internal/app-binder/task/ExtractTransactionTask.ts b/packages/signer/signer-btc/src/internal/app-binder/task/ExtractTransactionTask.ts
@@ -65,7 +65,8 @@ export class ExtractTransactionTask {
       const scriptSig = psbt
         .getInputValue(i, PsbtIn.FINAL_SCRIPTSIG)
         .mapOrDefault((v) => v.data, Uint8Array.from([]));
-      transaction.encodeInLVFromBuffer(scriptSig);
+      transaction.addBufferToData(encodeVarint(scriptSig.length).extract()!);
+      transaction.addBufferToData(scriptSig);
       const sequence = psbt
         .getInputValue(i, PsbtIn.SEQUENCE)
         .chain((value) => this._valueParser.getUInt32LE(value.data))
@@ -78,21 +79,22 @@ export class ExtractTransactionTask {
         witnessBuffer.addBufferToData(witness);
       }
     }
-    const ouputCount = psbt
+    const outputCount = psbt
       .getGlobalValue(PsbtGlobal.OUTPUT_COUNT)
-      .chain((value) => this._valueParser.getUInt32LE(value.data))
+      .chain((value) => this._valueParser.getVarint(value.data))
       .orDefault(0);
-    transaction.addBufferToData(encodeVarint(ouputCount).extract()!);
-    for (let o = 0; o < ouputCount; o++) {
+    transaction.addBufferToData(encodeVarint(outputCount).extract()!);
+    for (let o = 0; o < outputCount; o++) {
       const amount = psbt
         .getOutputValue(o, PsbtOut.AMOUNT)
-        .chain((value) => this._valueParser.getVarint(value.data))
-        .orDefault(0);
+        .chain((value) => this._valueParser.getInt64LE(value.data))
+        .orDefault(0n);
       const script = psbt
         .getOutputValue(o, PsbtOut.SCRIPT)
         .mapOrDefault((v) => v.data, Buffer.from([]));
-      transaction.addBufferToData(encodeVarint(amount).extract()!);
-      transaction.encodeInLVFromBuffer(script);
+      transaction.add64BitIntToData(amount, false);
+      transaction.addBufferToData(encodeVarint(script.length).extract()!);
+      transaction.addBufferToData(script);
     }
     transaction.addBufferToData(witnessBuffer.build());
     const locktime = psbt

diff --git a/packages/signer/signer-btc/src/internal/app-binder/task/UpdatePsbtTask.test.ts b/packages/signer/signer-btc/src/internal/app-binder/task/UpdatePsbtTask.test.ts
@@ -162,7 +162,7 @@ describe("UpdatePsbtTask", () => {
               ],
               [
                 new Key(PsbtIn.FINAL_SCRIPTSIG).toHexaString(),
-                new Value(Uint8Array.from([0x01, 0x21, 0x01, 0x42])),
+                new Value(Uint8Array.from([0x01, 0x42, 0x01, 0x21])),
               ],
             ]),
           ],
@@ -242,7 +242,7 @@ describe("UpdatePsbtTask", () => {
               ],
               [
                 new Key(PsbtIn.FINAL_SCRIPTWITNESS).toHexaString(),
-                new Value(Uint8Array.from([0x02, 0x01, 0x21, 0x01, 0x42])),
+                new Value(Uint8Array.from([0x02, 0x01, 0x42, 0x01, 0x21])),
               ],
               [
                 new Key(PsbtIn.FINAL_SCRIPTSIG).toHexaString(),

diff --git a/packages/signer/signer-btc/src/internal/app-binder/task/UpdatePsbtTask.ts b/packages/signer/signer-btc/src/internal/app-binder/task/UpdatePsbtTask.ts
@@ -29,6 +29,9 @@ type UpdatePsbtTaskArgs = {
   signatures: PsbtSignature[];
 };
 
+const SCHORR_SIG_LENGTH = 64;
+const SCHORR_SIG_LENGTH_WITH_EXTRA_BYTE = 65;
+
 export class UpdatePsbtTask {
   constructor(
     private readonly _args: UpdatePsbtTaskArgs,
@@ -118,18 +121,24 @@ export class UpdatePsbtTask {
                   `Missing pubkey derivation for input ${psbtSignature.inputIndex}`,
                 );
               }
+              // @todo handle PSBT_IN_TAP_SCRIPT_SIG as described here https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki#specification
+              if (psbtSignature.tapleafHash !== undefined) {
+                throw new Error(
+                  "Unhandled psbt key type PSBT_IN_TAP_SCRIPT_SIG",
+                );
+              }
               psbt.setInputValue(
                 psbtSignature.inputIndex,
                 PsbtIn.TAP_KEY_SIG,
                 new Value(psbtSignature.signature),
               );
             });
           } else {
-            psbt.setKeyDataInputValue(
+            psbt.setInputValue(
               psbtSignature.inputIndex,
               PsbtIn.PARTIAL_SIG,
-              psbtSignature.signature,
-              new Value(psbtSignature.pubkey),
+              new Value(psbtSignature.signature),
+              psbtSignature.pubkey,
             );
           }
         });
@@ -325,7 +334,6 @@ export class UpdatePsbtTask {
     inputIndex: number,
   ): InternalPsbt {
     const psbt = fromPsbt;
-    // Taproot input
     const maybeSignature = psbt.getInputValue(inputIndex, PsbtIn.TAP_KEY_SIG);
     if (maybeSignature.isNothing()) {
       throw Error("No signature for taproot input " + inputIndex);
@@ -335,13 +343,15 @@ export class UpdatePsbtTask {
       Uint8Array.from([]),
     );
 
-    if (signature.length != 64 && signature.length != 65) {
+    if (
+      ![SCHORR_SIG_LENGTH, SCHORR_SIG_LENGTH_WITH_EXTRA_BYTE].includes(
+        signature.length,
+      )
+    ) {
       throw Error("Unexpected length of schnorr signature.");
     }
     const witnessBufferBuilder = new ByteArrayBuilder();
-    witnessBufferBuilder.addBufferToData(
-      encodeVarint(1).mapOrDefault((v) => v, Uint8Array.from([1])),
-    );
+    witnessBufferBuilder.add8BitUIntToData(0x01);
     witnessBufferBuilder.encodeInLVFromBuffer(signature);
     psbt.setInputValue(
       inputIndex,

diff --git a/packages/signer/signer-btc/src/internal/psbt/model/Psbt.test.ts b/packages/signer/signer-btc/src/internal/psbt/model/Psbt.test.ts
@@ -186,12 +186,7 @@ describe("Psbt", () => {
     const psbt = new Psbt(new Map(), [new Map()]);
     const value = new Value(Uint8Array.of(0x03));
     // when
-    psbt.setKeyDataInputValue(
-      0,
-      PsbtIn.PARTIAL_SIG,
-      Uint8Array.from([0x42]),
-      value,
-    );
+    psbt.setInputValue(0, PsbtIn.PARTIAL_SIG, value, Uint8Array.from([0x42]));
     // then
     expect(psbt).toStrictEqual(
       new Psbt(

diff --git a/packages/signer/signer-btc/src/internal/psbt/model/Psbt.ts b/packages/signer/signer-btc/src/internal/psbt/model/Psbt.ts
@@ -114,18 +114,14 @@ export class Psbt {
     this.globalMap.set(new Key(key).toHexaString(), value);
   }
 
-  setInputValue(inputIndex: number, key: PsbtIn, value: Value) {
-    this.inputMaps[inputIndex]?.set(new Key(key).toHexaString(), value);
-  }
-
-  setKeyDataInputValue(
+  setInputValue(
     inputIndex: number,
-    keyIn: PsbtIn,
-    keyData: Uint8Array,
+    key: PsbtIn,
     value: Value,
+    keyData?: Uint8Array,
   ) {
     this.inputMaps[inputIndex]?.set(
-      new Key(keyIn, keyData).toHexaString(),
+      new Key(key, keyData).toHexaString(),
       value,
     );
   }

diff --git a/packages/signer/signer-btc/src/internal/psbt/service/value/DefaultValueParser.test.ts b/packages/signer/signer-btc/src/internal/psbt/service/value/DefaultValueParser.test.ts
@@ -44,6 +44,68 @@ describe("DefaultValueParser", () => {
     });
   });
 
+  describe("getUInt32LE", () => {
+    it("Get an unsigned 32-bit positive integer", () => {
+      // GIVEN
+      const data = Uint8Array.from([214, 255, 255, 255]);
+
+      // WHEN
+      const result = service.getUInt32LE(data);
+
+      // THEN
+      expect(result.isJust()).toStrictEqual(true);
+      expect(result.unsafeCoerce()).toStrictEqual(4294967254);
+    });
+
+    it("Invalid data", () => {
+      // GIVEN
+      const data = Uint8Array.from([0, 0, 0]);
+
+      // WHEN
+      const result = service.getUInt32LE(data);
+
+      // THEN
+      expect(result.isJust()).toStrictEqual(false);
+    });
+  });
+
+  describe("getInt64LE", () => {
+    it("Get a signed 64-bit positive integer", () => {
+      // GIVEN
+      const data = Uint8Array.from([42, 0, 0, 0, 0, 0, 0, 0]);
+
+      // WHEN
+      const result = service.getInt64LE(data);
+
+      // THEN
+      expect(result.isJust()).toStrictEqual(true);
+      expect(result.unsafeCoerce()).toStrictEqual(42n);
+    });
+
+    it("Get a signed 64-bit negative integer", () => {
+      // GIVEN
+      const data = Uint8Array.from([214, 255, 255, 255, 255, 255, 255, 255]);
+
+      // WHEN
+      const result = service.getInt64LE(data);
+
+      // THEN
+      expect(result.isJust()).toStrictEqual(true);
+      expect(result.unsafeCoerce()).toStrictEqual(-42n);
+    });
+
+    it("Invalid data", () => {
+      // GIVEN
+      const data = Uint8Array.from([0, 0, 0, 0, 0]);
+
+      // WHEN
+      const result = service.getInt64LE(data);
+
+      // THEN
+      expect(result.isJust()).toStrictEqual(false);
+    });
+  });
+
   describe("getVarint", () => {
     it("Get a varint", () => {
       // GIVEN

diff --git a/packages/signer/signer-btc/src/internal/psbt/service/value/DefaultValueParser.ts b/packages/signer/signer-btc/src/internal/psbt/service/value/DefaultValueParser.ts
@@ -15,6 +15,10 @@ export class DefaultValueParser implements ValueParser {
     );
   }
 
+  getInt64LE(data: Uint8Array): Maybe<bigint> {
+    return Maybe.fromNullable(new ByteArrayParser(data).extract64BitInt(false));
+  }
+
   getVarint(data: Uint8Array): Maybe<number> {
     return extractVarint(new ByteArrayParser(data)).map(
       (varint) => varint.value,

diff --git a/packages/signer/signer-btc/src/internal/psbt/service/value/ValueParser.ts b/packages/signer/signer-btc/src/internal/psbt/service/value/ValueParser.ts
@@ -3,5 +3,6 @@ import { type Maybe } from "purify-ts";
 export interface ValueParser {
   getInt32LE(data: Uint8Array): Maybe<number>;
   getUInt32LE(data: Uint8Array): Maybe<number>;
+  getInt64LE(data: Uint8Array): Maybe<bigint>;
   getVarint(data: Uint8Array): Maybe<number>;
 }