Merge pull request #9557 from BitGo/fix/sdk-abstract-substrate/mpcv2-recovery

fix(abstract-substrate): remove double Ed25519 prefix in MPCv2 recovery

Author: vibhavgo

modules/abstract-substrate/src/abstractSubstrateCoin.ts

@@ -517,8 +517,14 @@ export class SubstrateCoin extends BaseCoin {
 
   /**
    * Adds an MPCv1 or MPCv2 signature to a Substrate transaction builder.
-   * MPCv2 signatures are prefixed with ED25519_MULTI_SIGNATURE_PREFIX (Ed25519 discriminant
-   * in the Substrate MultiSignature enum).
+   *
+   * Both branches hand off the raw 64-byte Ed25519 signature untouched.
+   * Transaction#constructSignedPayload already prepends the 0x00 type-tag
+   * (the Substrate MultiSignature enum discriminant for Ed25519) to whatever
+   * signature buffer is passed to addSignature, so wrapping the signature
+   * here would produce a double discriminant, corrupting the on-wire sig
+   * bytes and causing the chain to reject the extrinsic with `1010: Bad
+   * signature`.
    */
   protected async addSubstrateRecoverySignature(
     txBuilder: NativeTransferBuilder,
@@ -530,7 +536,6 @@ export class SubstrateCoin extends BaseCoin {
     bitgoKey: string,
     accountId: string
   ): Promise<void> {
-    const ED25519_MULTI_SIGNATURE_PREFIX = 0x00;
     const substrateKeyPair = new SubstrateKeyPair({ pub: accountId });
 
     if (signingMaterial.version === 'v2') {
@@ -543,8 +548,7 @@ export class SubstrateCoin extends BaseCoin {
         derivationPath: currPath,
         bitgo: this.bitgo,
       });
-      const substrateSig = Buffer.concat([Buffer.from([ED25519_MULTI_SIGNATURE_PREFIX]), rawSig]);
-      txBuilder.addSignature({ pub: substrateKeyPair.getKeys().pub }, substrateSig);
+      txBuilder.addSignature({ pub: substrateKeyPair.getKeys().pub }, rawSig);
     } else {
       const userSigningMaterial = JSON.parse(signingMaterial.userPrv) as EDDSAMethodTypes.UserSigningMaterial;
       const backupPrv = await decryptKeychainPrivateKey(this.bitgo, { encryptedPrv: backupKey }, walletPassphrase);

modules/abstract-substrate/test/unit/abstractSubstrateCoin.ts

@@ -86,7 +86,12 @@ describe('SubstrateCoin MPCv2 recovery helpers:', function () {
       (coin as unknown as { bitgo: unknown }).bitgo = { decrypt: instanceDecryptStub };
     });
 
-    it('should prepend ED25519 0x00 discriminant on MPCv2 path', async function () {
+    it('should pass the raw 64-byte signature to addSignature on MPCv2 path', async function () {
+      // Regression: previously wrapped rawSig with a manual Ed25519 discriminant
+      // (0x00) before addSignature. constructSignedPayload already prepends that
+      // discriminant, so wrapping here caused a double prefix that shifted the
+      // on-wire signature by one byte, dropping the last byte of `sigma` and
+      // producing `1010: Bad signature` on-chain.
       const rawSig = Buffer.alloc(64, 0xab);
       sinon.stub(coin as unknown, 'signSubstrateMpcV2Recovery').resolves(rawSig);
 
@@ -103,8 +108,8 @@ describe('SubstrateCoin MPCv2 recovery helpers:', function () {
 
       addSignatureStub.calledOnce.should.be.true();
       const sig: Buffer = addSignatureStub.firstCall.args[1];
-      sig[0].should.equal(0x00);
-      sig.slice(1).should.deepEqual(rawSig);
+      sig.length.should.equal(64);
+      sig.should.deepEqual(rawSig);
     });
 
     it('should call getTSSSignature and pass result to addSignature on MPCv1 path', async function () {