-
Notifications
You must be signed in to change notification settings - Fork 307
refactor(abstract-substrate): extract MPCv2 helpers into SubstrateCoin #9408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
| } | ||
|
vibhavgo marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * 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( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method has no unit tests. It's the critical signing path — it branches on MPCv1 vs MPCv2, prepends the The existing tests only cover
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this was a refactor to create common utility for children of the substrate class |
||
| 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; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.