refactor(abstract-substrate): extract MPCv2 helpers into SubstrateCoin

vibhavgo · vibhavgo · commit 3e562a44b59f · 2026-08-06T15:54:34.000+05:30
Add protected isMpcv2SigningMaterial() and addSubstrateRecoverySignature() to SubstrateCoin so DOT and POLYX can call shared methods rather than duplicating ~40 lines of identical MPCv2 signing logic. Refactors recover() to delegate to these helpers; MPCv1 path unchanged. Ticket: WCI-1239
diff --git a/modules/abstract-substrate/src/abstractSubstrateCoin.ts b/modules/abstract-substrate/src/abstractSubstrateCoin.ts
@@ -25,6 +25,8 @@ import {
   UnexpectedAddressError,
   verifyEddsaTssWalletAddress,
   VerifyTransactionOptions,
+  EDDSAUtils,
+  decryptKeychainPrivateKey,
 } from '@bitgo/sdk-core';
 import { CoinFamily, BaseCoin as StaticsBaseCoin } from '@bitgo/statics';
 import { KeyPair as SubstrateKeyPair, Transaction } from './lib';
@@ -38,6 +40,12 @@ import { ApiPromise } from '@polkadot/api';
 
 export const DEFAULT_SCAN_FACTOR = 20;
 
+/**
+ * Discriminated union carrying keycard version and decrypted V1 user key (to avoid re-decryption).
+ * V1 keycards are JSON; V2 keycards are CBOR-encoded reduced key shares.
+ */
+type SubstrateSigningMaterial = { version: 'v1'; userPrv: string } | { version: 'v2'; encryptedUserKey: string };
+
 export class SubstrateCoin extends BaseCoin {
   protected readonly _staticsCoin: Readonly<StaticsBaseCoin>;
   readonly MAX_VALIDITY_DURATION = 2400;
@@ -356,42 +364,17 @@ export class SubstrateCoin extends BaseCoin {
         throw new Error('missing wallet passphrase');
       }
 
-      const userKey = params.userKey.replace(/\s/g, '');
-      const backupKey = params.backupKey.replace(/\s/g, '');
-
-      // Decrypt private keys from KeyCard values
-      let userPrv;
-      try {
-        userPrv = await this.bitgo.decrypt({
-          input: userKey,
-          password: params.walletPassphrase,
-        });
-      } catch (e) {
-        throw new Error(`Error decrypting user keychain: ${e.message}`);
-      }
-      const userSigningMaterial = JSON.parse(userPrv) as EDDSAMethodTypes.UserSigningMaterial;
-
-      let backupPrv;
-      try {
-        backupPrv = await this.bitgo.decrypt({
-          input: backupKey,
-          password: params.walletPassphrase,
-        });
-      } catch (e) {
-        throw new Error(`Error decrypting backup keychain: ${e.message}`);
-      }
-      const backupSigningMaterial = JSON.parse(backupPrv) as EDDSAMethodTypes.BackupSigningMaterial;
-
-      // add signature
-      const signatureHex = await EDDSAMethods.getTSSSignature(
-        userSigningMaterial,
-        backupSigningMaterial,
+      const signingMaterial = await this.isMpcV2Keycard(params.userKey!, params.walletPassphrase!);
+      await this.addSubstrateRecoverySignature(
+        txBuilder,
+        signingMaterial,
+        params.backupKey!.replace(/\s/g, ''),
+        params.walletPassphrase!,
+        unsignedTransaction,
         currPath,
-        unsignedTransaction
+        bitgoKey,
+        accountId
       );
-
-      const substrateKeyPair = new SubstrateKeyPair({ pub: accountId });
-      txBuilder.addSignature({ pub: substrateKeyPair.getKeys().pub }, signatureHex);
       const signedTransaction = await txBuilder.build();
       serializedTx = signedTransaction.toBroadcastFormat();
     } else {
@@ -526,6 +509,110 @@ export class SubstrateCoin extends BaseCoin {
     return { transactions: consolidationTransactions, lastScanIndex };
   }
 
+  /**
+   * Decrypts an encrypted keychain value, wrapping errors with a descriptive message.
+   */
+  private async decryptKeychain(encryptedKey: string, passphrase: string, label: string): Promise<string> {
+    const prv = await decryptKeychainPrivateKey(this.bitgo, { encryptedPrv: encryptedKey }, passphrase);
+    if (!prv) {
+      throw new Error(`Error decrypting ${label} keychain: invalid password or corrupted key`);
+    }
+    return prv;
+  }
+
+  /**
+   * Probes the key format and returns a discriminated union so callers avoid a second decrypt.
+   * V1 keycards are JSON; V2 keycards are CBOR-encoded reduced key shares.
+   */
+  protected async isMpcV2Keycard(userKey: string, walletPassphrase: string): Promise<SubstrateSigningMaterial> {
+    const normalized = userKey.replace(/\s/g, '');
+    let isV1: boolean;
+    try {
+      isV1 = await EDDSAUtils.isEddsaMpcV1SigningMaterial(normalized, walletPassphrase, this.bitgo);
+    } catch (e) {
+      throw new Error(`Error decrypting user keychain: ${e instanceof Error ? e.message : String(e)}`);
+    }
+    if (isV1) {
+      const userPrv = await this.decryptKeychain(normalized, walletPassphrase, 'user');
+      return { version: 'v1', userPrv };
+    }
+    return { version: 'v2', encryptedUserKey: normalized };
+  }
+
+  // Protected so tests can stub via instance overrides without adding new test dependencies.
+  protected async getEddsaMpcV2RecoveryKeyShares(
+    encryptedUserKey: string,
+    encryptedBackupKey: string,
+    walletPassphrase: string
+  ): ReturnType<typeof EDDSAUtils.getEddsaMpcV2RecoveryKeySharesFromReducedKey> {
+    return EDDSAUtils.getEddsaMpcV2RecoveryKeySharesFromReducedKey(
+      encryptedUserKey,
+      encryptedBackupKey,
+      walletPassphrase,
+      this.bitgo
+    );
+  }
+
+  // Protected so tests can stub via instance overrides without adding new test dependencies.
+  protected async signEddsaMpcV2Recovery(
+    signablePayload: Buffer,
+    currPath: string,
+    ...args: Parameters<typeof EDDSAUtils.signRecoveryEddsaMPCv2> extends [Buffer, string, ...infer R] ? R : never
+  ): Promise<Buffer> {
+    return EDDSAUtils.signRecoveryEddsaMPCv2(signablePayload, currPath, ...args);
+  }
+
+  /**
+   * 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).
+   */
+  protected async addSubstrateRecoverySignature(
+    txBuilder: NativeTransferBuilder,
+    signingMaterial: SubstrateSigningMaterial,
+    backupKey: string,
+    walletPassphrase: string,
+    unsignedTransaction: Transaction,
+    currPath: string,
+    bitgoKey: string,
+    accountId: string
+  ): Promise<void> {
+    const ED25519_MULTI_SIGNATURE_PREFIX = 0x00;
+    const substrateKeyPair = new SubstrateKeyPair({ pub: accountId });
+
+    if (signingMaterial.version === 'v2') {
+      const { userKeyShare, backupKeyShare, commonKeyChain } = await this.getEddsaMpcV2RecoveryKeyShares(
+        signingMaterial.encryptedUserKey,
+        backupKey,
+        walletPassphrase
+      );
+      if (commonKeyChain.toLowerCase() !== bitgoKey.toLowerCase()) {
+        throw new Error('EdDSA MPCv2 recovery: commonKeyChain from keycard does not match bitgoKey');
+      }
+      const rawSig = await this.signEddsaMpcV2Recovery(
+        unsignedTransaction.signablePayload,
+        currPath,
+        userKeyShare,
+        backupKeyShare,
+        commonKeyChain
+      );
+      const substrateSig = Buffer.concat([Buffer.from([ED25519_MULTI_SIGNATURE_PREFIX]), rawSig]);
+      txBuilder.addSignature({ pub: substrateKeyPair.getKeys().pub }, substrateSig);
+    } else {
+      const userSigningMaterial = JSON.parse(signingMaterial.userPrv) as EDDSAMethodTypes.UserSigningMaterial;
+      const backupPrv = await this.decryptKeychain(backupKey, walletPassphrase, 'backup');
+      const backupSigningMaterial = JSON.parse(backupPrv) as EDDSAMethodTypes.BackupSigningMaterial;
+
+      const signatureHex = await EDDSAMethods.getTSSSignature(
+        userSigningMaterial,
+        backupSigningMaterial,
+        currPath,
+        unsignedTransaction
+      );
+      txBuilder.addSignature({ pub: substrateKeyPair.getKeys().pub }, signatureHex);
+    }
+  }
+
   /** inherited doc */
   async createBroadcastableSweepTransaction(params: MPCSweepRecoveryOptions): Promise<MPCTxs> {
     const req = params.signatureShares;
diff --git a/modules/abstract-substrate/test/unit/abstractSubstrateCoin.ts b/modules/abstract-substrate/test/unit/abstractSubstrateCoin.ts
@@ -0,0 +1,167 @@
+import * as sinon from 'sinon';
+import * as sdkCore from '@bitgo/sdk-core';
+// Cross-module relative import: tsx resolves TypeScript source directly in the monorepo,
+// avoiding a circular devDependency (sdk-coin-tao depends on abstract-substrate at runtime).
+import { Ttao } from '../../../sdk-coin-tao/src';
+
+interface SubstrateCoinTestAccessor {
+  isMpcV2Keycard(userKey: string, walletPassphrase: string): Promise<{ version: 'v1' | 'v2' }>;
+  addSubstrateRecoverySignature(
+    txBuilder: unknown,
+    signingMaterial: unknown,
+    backupKey: string,
+    walletPassphrase: string,
+    unsignedTransaction: unknown,
+    currPath: string,
+    bitgoKey: string,
+    accountId: string
+  ): Promise<void>;
+}
+
+// 32-byte all-zeros hex: minimal valid Ed25519 public key for SubstrateKeyPair construction.
+const MOCK_ACCOUNT_ID = '0'.repeat(64);
+const MOCK_BITGO_KEY = 'aa'.repeat(32);
+const MOCK_UNSIGNED_TX = { signablePayload: Buffer.from('deadbeef', 'hex') };
+
+describe('SubstrateCoin MPCv2 recovery helpers:', function () {
+  const sandBox = sinon.createSandbox();
+  // Bypass the constructor (which requires a full BitGoBase) — we only need the
+  // prototype methods.
+  const basecoin = Object.create(Ttao.prototype) as Ttao & SubstrateCoinTestAccessor;
+  // isEddsaMpcV1SigningMaterial is gated behind non-configurable namespace getters that
+  // sinon cannot replace. Provide bitgo.decrypt so it uses that path instead of sjcl,
+  // then control V1 vs V2 detection by returning JSON (V1) or non-JSON (V2).
+  let decryptStub: sinon.SinonStub;
+
+  beforeEach(function () {
+    decryptStub = sinon.stub();
+    (basecoin as unknown as { bitgo: unknown }).bitgo = { decrypt: decryptStub };
+  });
+
+  afterEach(function () {
+    sandBox.restore();
+  });
+
+  describe('isMpcV2Keycard()', function () {
+    it('should return version v2 for a CBOR (MPCv2) keycard', async function () {
+      // Non-JSON decrypted value → V2 CBOR keycard
+      decryptStub.resolves('not-json-cbor-bytes');
+      const result = await basecoin.isMpcV2Keycard('encryptedKey', 'passphrase');
+      result.version.should.equal('v2');
+    });
+
+    it('should return version v1 for a JSON (MPCv1) keycard', async function () {
+      // isMpcV2Keycard checks uShare.seed + bitgoYShare.u + backupYShare.u
+      decryptStub.resolves(
+        JSON.stringify({
+          uShare: { seed: 'deadbeef' },
+          bitgoYShare: { u: 'aabbcc' },
+          backupYShare: { u: 'ddeeff' },
+        })
+      );
+      const result = await basecoin.isMpcV2Keycard('encryptedKey', 'passphrase');
+      result.version.should.equal('v1');
+    });
+
+    it('should throw with a descriptive message when decryption fails', async function () {
+      decryptStub.rejects(new Error('bad password'));
+      await basecoin
+        .isMpcV2Keycard('encryptedKey', 'wrong-passphrase')
+        .should.be.rejectedWith(/Error decrypting user keychain/);
+    });
+  });
+
+  describe('addSubstrateRecoverySignature()', function () {
+    // EDDSAUtils.* are exported via `export * as Namespace`, compiling to non-configurable
+    // property getters — sinon cannot replace them. Instead, SubstrateCoin exposes
+    // getEddsaMpcV2RecoveryKeyShares() and signEddsaMpcV2Recovery() as protected methods
+    // so they can be stubbed on the instance (own property shadows the prototype).
+    // EDDSAMethods.getTSSSignature is a regular writable property — sinon can stub it directly.
+    let addSignatureStub: sinon.SinonStub;
+    let coin: SubstrateCoinTestAccessor;
+
+    beforeEach(function () {
+      addSignatureStub = sinon.stub();
+      const instanceDecryptStub = sinon.stub();
+      coin = Object.create(Ttao.prototype) as Ttao & SubstrateCoinTestAccessor;
+      (coin as unknown as { bitgo: unknown }).bitgo = { decrypt: instanceDecryptStub };
+    });
+
+    it('should prepend ED25519 0x00 discriminant on MPCv2 path', async function () {
+      const rawSig = Buffer.alloc(64, 0xab);
+      sinon.stub(coin as unknown, 'getEddsaMpcV2RecoveryKeyShares').resolves({
+        userKeyShare: 'ks1',
+        backupKeyShare: 'ks2',
+        commonKeyChain: MOCK_BITGO_KEY,
+      });
+      sinon.stub(coin as unknown, 'signEddsaMpcV2Recovery').resolves(rawSig);
+
+      await coin.addSubstrateRecoverySignature(
+        { addSignature: addSignatureStub },
+        { version: 'v2', encryptedUserKey: 'encKey' },
+        'encBackupKey',
+        'passphrase',
+        MOCK_UNSIGNED_TX,
+        'm/0',
+        MOCK_BITGO_KEY,
+        MOCK_ACCOUNT_ID
+      );
+
+      addSignatureStub.calledOnce.should.be.true();
+      const sig: Buffer = addSignatureStub.firstCall.args[1];
+      sig[0].should.equal(0x00);
+      sig.slice(1).should.deepEqual(rawSig);
+    });
+
+    it('should throw when commonKeyChain does not match bitgoKey on MPCv2 path', async function () {
+      sinon.stub(coin as unknown, 'getEddsaMpcV2RecoveryKeyShares').resolves({
+        userKeyShare: 'ks1',
+        backupKeyShare: 'ks2',
+        commonKeyChain: 'mismatch',
+      });
+
+      await coin
+        .addSubstrateRecoverySignature(
+          { addSignature: addSignatureStub },
+          { version: 'v2', encryptedUserKey: 'encKey' },
+          'encBackupKey',
+          'passphrase',
+          MOCK_UNSIGNED_TX,
+          'm/0',
+          MOCK_BITGO_KEY,
+          MOCK_ACCOUNT_ID
+        )
+        .should.be.rejectedWith(/commonKeyChain from keycard does not match bitgoKey/);
+    });
+
+    it('should call getTSSSignature and pass result to addSignature on MPCv1 path', async function () {
+      const mockSig = 'ff'.repeat(64);
+      sandBox.stub(sdkCore.EDDSAMethods, 'getTSSSignature').resolves(mockSig);
+      // decryptKeychain calls decryptKeychainPrivateKey → bitgo.decrypt for the backup key
+      (coin as unknown as { bitgo: { decrypt: sinon.SinonStub } }).bitgo.decrypt.resolves(
+        JSON.stringify({ yShares: {} })
+      );
+
+      const userPrv = JSON.stringify({
+        uShare: { seed: 'deadbeef' },
+        bitgoYShare: { u: 'aabbcc' },
+        backupYShare: { u: 'ddeeff' },
+      });
+
+      await coin.addSubstrateRecoverySignature(
+        { addSignature: addSignatureStub },
+        { version: 'v1', userPrv },
+        'encBackupKey',
+        'passphrase',
+        MOCK_UNSIGNED_TX,
+        'm/0',
+        MOCK_BITGO_KEY,
+        MOCK_ACCOUNT_ID
+      );
+
+      (sdkCore.EDDSAMethods.getTSSSignature as sinon.SinonStub).calledOnce.should.be.true();
+      addSignatureStub.calledOnce.should.be.true();
+      addSignatureStub.firstCall.args[1].should.equal(mockSig);
+    });
+  });
+});
