Merge pull request #8821 from BitGo/CECHO-1140

mohd-kashif · web-flow · commit ab2220975044 · 2026-05-21T11:28:41.000+05:30
feat(sdk-coin-flrp): enhance txn verification for atomic transactions
diff --git a/modules/sdk-coin-flrp/test/unit/flrp.ts b/modules/sdk-coin-flrp/test/unit/flrp.ts
@@ -959,5 +959,94 @@ describe('Flrp test cases', function () {
         isVerified.should.equal(true);
       });
     });
+
+    describe('verifyTransaction with TSS wallet (Avalanche atomic)', () => {
+      it('should verify TSS ExportInP when passed serializedTxHex (not signableHex)', async () => {
+        // The SDK's signRequestBase now passes serializedTxHex (the full
+        // PVM transaction bytes) to verifyTransaction for Avalanche atomic txs,
+        // instead of signableHex (a 32-byte SHA-256 hash that can't be parsed).
+        // This test confirms verifyTransaction succeeds with the actual tx bytes.
+        const txHex = await buildUnsignedExportInP();
+        const txPrebuild = { txHex, txInfo: {} };
+        const txParams = {
+          recipients: [{ address: ON_CHAIN_TEST_WALLET.user.pChainAddress, amount: '30000000' }],
+          type: 'Export',
+          locktime: 0,
+        };
+
+        const isVerified = await basecoin.verifyTransaction({
+          txParams,
+          txPrebuild,
+          walletType: 'tss',
+        });
+        isVerified.should.equal(true);
+      });
+
+      it('should verify TSS ImportInP when passed serializedTxHex', async () => {
+        const txHex = await buildUnsignedImportInP();
+        const txPrebuild = { txHex, txInfo: {} };
+        const txParams = {
+          recipients: [{ address: ON_CHAIN_TEST_WALLET.user.pChainAddress, amount: '1' }],
+          type: 'Import',
+          locktime: 0,
+        };
+
+        const isVerified = await basecoin.verifyTransaction({
+          txParams,
+          txPrebuild,
+          walletType: 'tss',
+        });
+        isVerified.should.equal(true);
+      });
+
+      it('should verify TSS ImportInC when passed serializedTxHex', async () => {
+        const txHex = await buildUnsignedImportInC();
+        const txPrebuild = { txHex, txInfo: {} };
+        const txParams = {
+          recipients: [{ address: '0x96993BAEb6AaE2e06BF95F144e2775D4f8efbD35', amount: '1' }],
+          type: 'ImportToC',
+          locktime: 0,
+        };
+
+        const isVerified = await basecoin.verifyTransaction({
+          txParams,
+          txPrebuild,
+          walletType: 'tss',
+        });
+        isVerified.should.equal(true);
+      });
+
+      it('should verify signablePayload is SHA-256 of serialized tx (sandbox-verified)', async () => {
+        // unsignedTx.toBytes() → sha256 → MPC.sign()
+        // The signablePayload must be exactly 32 bytes (SHA-256 digest).
+        // This is what the WP stores as signableHex and what ecdsaMPCv2.ts
+        // must use directly (no keccak256 re-hashing).
+        const txHex = await buildUnsignedExportInP();
+        const payload = await basecoin.getSignablePayload(txHex);
+
+        // signablePayload is SHA-256(txBody) — always 32 bytes
+        assert.strictEqual(payload.length, 32, 'signablePayload must be 32 bytes (SHA-256)');
+
+        // The serializedTxHex (after stripping 0x) starts with Avalanche codec '0000'
+        // This is how ecdsaMPCv2.ts detects atomic transactions
+        const rawHex = txHex.startsWith('0x') ? txHex.substring(2) : txHex;
+        assert.ok(rawHex.startsWith('0000'), 'Avalanche atomic tx must start with codec prefix 0000');
+      });
+
+      it('should reject a SHA-256 hash as txHex (proves the fix is needed)', async () => {
+        const sha256Hash = 'a'.repeat(64);
+        const txPrebuild = { txHex: sha256Hash, txInfo: {} };
+        const txParams = { recipients: [], type: 'Export', locktime: 0 };
+
+        let threw = false;
+        try {
+          await basecoin.verifyTransaction({ txParams, txPrebuild, walletType: 'tss' });
+        } catch (e) {
+          threw = true;
+          assert(e.message.includes('Invalid transaction'), `Expected 'Invalid transaction', got: ${e.message}`);
+        }
+        assert(threw, 'Expected verifyTransaction to throw for SHA-256 hash as txHex');
+      });
+    });
   });
 });
diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts
@@ -790,6 +790,7 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
     let txOrMessageToSign;
     let derivationPath;
     let bufferContent;
+    let serializedTxHex: string | undefined;
     const userGpgKey = await generateGPGKeyPair('secp256k1');
     const bitgoGpgPubKey = await this.pickBitgoPubGpgKeyForSigning(true, params.reqId, txRequest.enterpriseId);
 
@@ -812,11 +813,16 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
         );
       }
 
-      // For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately.
-      // Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex
-      // to regenerate the signableHex and compare it against the provided value for verification.
-      // In contrast, for other coin families, verification is typically done using just the signableHex.
-      if (this.baseCoin.getConfig().family === 'icp') {
+      // For ICP and Avalanche atomic transactions, signableHex is a digest (not
+      // a parseable transaction).  Pass serializedTxHex so verifyTransaction can
+      // parse the full transaction bytes.
+      // - ICP: signableHex is a hash; serializedTxHex is the CBOR-encoded tx.
+      // - Avalanche atomic (FLRP/FLR cross-chain): signableHex is SHA-256(txBody);
+      //   serializedTxHex is the PVM/EVM atomic tx (codec prefix 0x0000).
+      // For all other coins, signableHex IS the unsigned transaction (e.g. RLP bytes).
+      const isIcp = this.baseCoin.getConfig().family === 'icp';
+      const isAvalancheAtomic = unsignedTx.serializedTxHex && unsignedTx.serializedTxHex.startsWith('0000');
+      if (isIcp || isAvalancheAtomic) {
         await this.baseCoin.verifyTransaction({
           txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
           txParams: params.txParams || { recipients: [] },
@@ -833,6 +839,7 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
       }
       txOrMessageToSign = unsignedTx.signableHex;
       derivationPath = unsignedTx.derivationPath;
+      serializedTxHex = unsignedTx.serializedTxHex;
       bufferContent = Buffer.from(txOrMessageToSign, 'hex');
     } else if (requestType === RequestType.message) {
       txOrMessageToSign = txRequest.messages![0].messageEncoded;
@@ -842,14 +849,24 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
       throw new Error('Invalid request type');
     }
 
-    let hash: Hash;
-    try {
-      hash = this.baseCoin.getHashFunction();
-    } catch (err) {
-      hash = createKeccakHash('keccak256') as Hash;
+    // For Avalanche atomic transactions (cross-chain export/import between
+    // C-chain and P-chain), signableHex is already SHA-256(txBody) — a 32-byte
+    // pre-hashed digest.  Use it directly as the DKLS message hash instead of
+    // applying the coin's hash function (keccak256 for EVM coins).
+    // Same logic as getHashStringAndDerivationPath (external signer path).
+    let hashBuffer: Buffer;
+    if (serializedTxHex && serializedTxHex.startsWith('0000')) {
+      hashBuffer = bufferContent;
+      assert(hashBuffer.length === 32, `Avalanche pre-hashed signableHex must be 32 bytes, got ${hashBuffer.length}`);
+    } else {
+      let hash: Hash;
+      try {
+        hash = this.baseCoin.getHashFunction();
+      } catch (err) {
+        hash = createKeccakHash('keccak256') as Hash;
+      }
+      hashBuffer = hash.update(bufferContent).digest();
     }
-    // check what the encoding is supposed to be for message
-    const hashBuffer = hash.update(bufferContent).digest();
     const otherSigner = new DklsDsg.Dsg(
       userKeyShare,
       params.mpcv2PartyId !== undefined ? params.mpcv2PartyId : 0,
diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts
@@ -411,10 +411,21 @@ describe('ECDSA MPC v2', async () => {
     // serializedTxHex = full unsigned Avalanche atomic tx (codec type ID 0x0000)
     // signableHex     = SHA-256(txBody) — 32 bytes, already the final signing hash
     //
-    // Reference sandbox output (c2p-tss):
+    // Sandbox reference (coins-sandbox/flareCP/flrC_MPC_to_flrP_MPC):
+    //
+    // C→P direction (c2pMpcToMpcTss.ts — export from C-chain):
     //   Message hash (SHA-256): 9b3e1c8fc9322b667ec61619487b3993e91dcfc5...
     //   Signature r: d5bc2e2cad314023...  s: 47af9d7109135f7a...  Recovery: 1
     //   Export TX ID: 2Z5ELShnmmMgvTeupzLQzEKtAgbvZkDvq6KRYqbzVgcyBGVGpb
+    //
+    // P→C direction (p2cMpcToMpcTss.ts — export from P-chain):
+    //   Threshold: 1 (MPC single-sig on-chain, NO hop transaction)
+    //   Message hash (SHA-256): f1afd7bb3df2019ee61b41334abf95172d469d18...
+    //   Signature r: fae44ca89e7a0d3effd0912c16d69735aabbc73ad2d140ffa2c3b46af48d159c
+    //   Signature s: 1dec05d0d477a5b245a0a2e5f3a67e75489ff9b98b29780fc757b12d9f687db3
+    //   Recovery: 0
+    //   Export TX ID: 2tDQmQUtDMyVWe8Bo36yHXykV2RMvh8rft3to5QsgoNhATMDXz
+    //   Network: Coston2 Testnet (ID: 114)
     const serializedTxHex =
       '0000000000010000007278db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da5552479' +
       '00000000000000000000000000000000000000000000000000000000000000000000000128a05933dc76' +
@@ -572,14 +583,155 @@ describe('ECDSA MPC v2', async () => {
       false // shouldHash=false: message is already SHA-256(txBody)
     );
     assert.ok(convertedSignature, 'Pre-hashed Avalanche atomic signature is not valid');
-    // Format: recid:R_hex:S_hex:publicKey_hex (same as sandbox 65-byte r+s+recovery)
+    // Format: recid:R_hex:S_hex:publicKey_hex
+    // Sandbox produces the same structure — e.g. P→C export:
+    //   r: fae44ca89e7a0d3effd0912c16d69735aabbc73ad2d140ffa2c3b46af48d159c (32 bytes)
+    //   s: 1dec05d0d477a5b245a0a2e5f3a67e75489ff9b98b29780fc757b12d9f687db3 (32 bytes)
+    //   Recovery: 0
     const sigParts = convertedSignature.split(':');
     assert.strictEqual(sigParts.length, 4, 'Signature must be recid:R:S:pubkey format');
     assert.ok(['0', '1'].includes(sigParts[0]), 'Recovery ID must be 0 or 1');
     assert.strictEqual(sigParts[1].length, 64, 'Signature R must be 32 bytes hex');
     assert.strictEqual(sigParts[2].length, 64, 'Signature S must be 32 bytes hex');
   });
 
+  it('signRequestBase (hot wallet path) should skip keccak256 for Avalanche atomic tx', async () => {
+    const serializedTxHex =
+      '0000000000010000007278db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da5552479' +
+      '00000000000000000000000000000000000000000000000000000000000000000000000128a05933dc76' +
+      'e4e6c25f35d5c9b2a58769700e760000000002ff3d1658734f94af871c3d131b56131b6fb7a0291eac' +
+      'add261e69dfb42a9cdf6f7fddd00000000000000090000000158734f94af871c3d131b56131b6fb7a029' +
+      '1eacadd261e69dfb42a9cdf6f7fddd000000070000000002faf08000000000000000000000000200000003' +
+      '12cb32eaf92553064db98d271b56cba079ec78f5a6e0c1abd0132f70efb77e2274637ff336a29a57c386' +
+      'd58d09a9ae77cf1cf07bf1c9de44ebb0c9f3';
+    const signableHex = createHash('sha256').update(Buffer.from(serializedTxHex, 'hex')).digest('hex');
+    const derivationPath = 'm/0';
+
+    const mockBgWithPost = {} as BitGoBase;
+    mockBgWithPost.getEnv = sinon.stub().returns('test');
+    mockBgWithPost.setRequestTracer = sinon.stub();
+    mockBgWithPost.encrypt = sinon.stub().returns('encrypted');
+    mockBgWithPost.decrypt = sinon.stub().returns('decrypted');
+    mockBgWithPost.post = sinon.stub().returns({
+      send: sinon.stub().returnsThis(),
+      set: sinon.stub().returnsThis(),
+      result: sinon.stub().rejects(new Error('mock: HTTP not available')),
+    });
+
+    const hashFunctionSpy = sinon.stub().callsFake(() => createKeccakHash('keccak256') as Hash);
+    const mockCoinForHotWallet = {
+      getHashFunction: hashFunctionSpy,
+      verifyTransaction: sinon.stub().resolves(true),
+      getMPCAlgorithm: sinon.stub().returns('ecdsa'),
+      getConfig: sinon.stub().returns({ family: 'flrp' }),
+    } as unknown as IBaseCoin;
+
+    const mockWallet = {
+      id: sinon.stub().returns(walletID),
+      multisigType: sinon.stub().returns('tss'),
+      multisigTypeVersion: sinon.stub().returns('MPCv2'),
+    };
+
+    const hotWalletUtils = new EcdsaMPCv2Utils(mockBgWithPost, mockCoinForHotWallet, mockWallet as any);
+    sinon.stub(hotWalletUtils as any, 'pickBitgoPubGpgKeyForSigning').resolves(bitgoGpgKey.public);
+
+    const txRequest = {
+      txRequestId: 'flrp-export-test',
+      apiVersion: 'full',
+      walletId: walletID,
+      transactions: [
+        {
+          unsignedTx: {
+            derivationPath,
+            signableHex,
+            serializedTxHex,
+          },
+          signatureShares: [],
+        },
+      ],
+    } as unknown as TxRequest;
+
+    try {
+      await hotWalletUtils.signTxRequest({
+        txRequest,
+        prv: userShare.toString('base64'),
+        reqId: { inc: sinon.stub(), toString: sinon.stub().returns('test-req') } as any,
+      });
+    } catch (e) {}
+
+    assert.strictEqual(
+      hashFunctionSpy.callCount,
+      0,
+      'getHashFunction must NOT be called for Avalanche atomic tx (serializedTxHex starts with 0000)'
+    );
+  });
+
+  it('signRequestBase (hot wallet path) should apply keccak256 for regular EVM tx', async () => {
+    const serializedTxHex = 'f86c808504a817c80082520894' + '00'.repeat(20) + '80808080';
+    const signableHex = serializedTxHex;
+    const derivationPath = 'm/0';
+
+    assert.ok(!serializedTxHex.startsWith('0000'), 'EVM tx must not start with Avalanche prefix');
+
+    const mockBgWithPost = {} as BitGoBase;
+    mockBgWithPost.getEnv = sinon.stub().returns('test');
+    mockBgWithPost.setRequestTracer = sinon.stub();
+    mockBgWithPost.encrypt = sinon.stub().returns('encrypted');
+    mockBgWithPost.decrypt = sinon.stub().returns('decrypted');
+    mockBgWithPost.post = sinon.stub().returns({
+      send: sinon.stub().returnsThis(),
+      set: sinon.stub().returnsThis(),
+      result: sinon.stub().rejects(new Error('mock: HTTP not available')),
+    });
+
+    const hashFunctionSpy = sinon.stub().callsFake(() => createKeccakHash('keccak256') as Hash);
+    const mockCoinForEvmWallet = {
+      getHashFunction: hashFunctionSpy,
+      verifyTransaction: sinon.stub().resolves(true),
+      getMPCAlgorithm: sinon.stub().returns('ecdsa'),
+      getConfig: sinon.stub().returns({ family: 'flr' }),
+    } as unknown as IBaseCoin;
+
+    const mockWallet = {
+      id: sinon.stub().returns(walletID),
+      multisigType: sinon.stub().returns('tss'),
+      multisigTypeVersion: sinon.stub().returns('MPCv2'),
+    };
+
+    const evmUtils = new EcdsaMPCv2Utils(mockBgWithPost, mockCoinForEvmWallet, mockWallet as any);
+    sinon.stub(evmUtils as any, 'pickBitgoPubGpgKeyForSigning').resolves(bitgoGpgKey.public);
+
+    const txRequest = {
+      txRequestId: 'flr-evm-test',
+      apiVersion: 'full',
+      walletId: walletID,
+      transactions: [
+        {
+          unsignedTx: {
+            derivationPath,
+            signableHex,
+            serializedTxHex,
+          },
+          signatureShares: [],
+        },
+      ],
+    } as unknown as TxRequest;
+
+    try {
+      await evmUtils.signTxRequest({
+        txRequest,
+        prv: userShare.toString('base64'),
+        reqId: { inc: sinon.stub(), toString: sinon.stub().returns('test-req') } as any,
+      });
+    } catch (e) {}
+
+    assert.strictEqual(
+      hashFunctionSpy.callCount,
+      1,
+      'getHashFunction must be called for regular EVM tx (serializedTxHex does not start with 0000)'
+    );
+  });
+
   it('should still apply keccak256 for regular FLR EVM transactions', async () => {
     // Regular EVM transaction on FLR (e.g. token transfer, not cross-chain).
     // serializedTxHex starts with 'f8' (RLP prefix), NOT '0000'.
