diff --git a/modules/abstract-substrate/src/abstractSubstrateCoin.ts b/modules/abstract-substrate/src/abstractSubstrateCoin.ts index 0491e00665..d2f2fcba8d 100644 --- a/modules/abstract-substrate/src/abstractSubstrateCoin.ts +++ b/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 { - 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); diff --git a/modules/abstract-substrate/test/unit/abstractSubstrateCoin.ts b/modules/abstract-substrate/test/unit/abstractSubstrateCoin.ts index 1533b1a950..e5476a7912 100644 --- a/modules/abstract-substrate/test/unit/abstractSubstrateCoin.ts +++ b/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 () {