Skip to content

Commit 18b0093

Browse files
committed
fix(sdk-core): derive EdDSA retrofit data via real MPCv1 keyCombine
Replaces the hand-rolled scalar derivation (manual SHA-512 + clamp with a zeroize try/finally, and reading a non-existent `pShare` field off the raw decrypted keycard JSON) with a call to `MPC.keyCombine(uShare, yShares)` — the same combine step every other MPCv1 EdDSA code path already uses. The decrypted MPCv1 key share (`SigningMaterial`) only ever contains `uShare` + `bitgoYShare` + `backupYShare`/`userYShare`; it has no `pShare`, and `uShare.chaincode` is only one party's additive contribution to the real BIP32 chain code, not the combined value. Deriving `s_i_0`/`expectedPk`/`chainCode` from the real `pShare.u`/`pShare.y`/`pShare.chaincode` output of keyCombine fixes both issues at once and asserts user/backup agree on the aggregate public key and chain code before returning. Moves `EddsaRetrofitData` into `@bitgo/sdk-lib-mpc`'s `tss/eddsa-mps/types.ts` (as `MPSTypes.EddsaRetrofitData`), mirroring where DKLS keeps its `RetrofitData` type, instead of defining and re-exporting it from sdk-core. Rewrites tests to exercise real 3-party MPCv1 key shares generated via `Eddsa.keyShare`/`keyCombine` instead of hand-built JSON fixtures with a `pShare` field that never occurs in production data. Ticket: WCI-1263
1 parent 4af287d commit 18b0093

4 files changed

Lines changed: 108 additions & 143 deletions

File tree

modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import assert from 'assert';
2-
import crypto from 'crypto';
32
import * as pgp from 'openpgp';
43
import * as sjcl from '@bitgo/sjcl';
54
import { NonEmptyString } from 'io-ts-types';
@@ -51,12 +50,8 @@ import { BaseEddsaUtils } from './base';
5150
import { resolveEffectiveTxParams } from '../recipientUtils';
5251
import { EddsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './eddsaMPCv2KeyGenSender';
5352
import { EddsaMPCv2RecoveryKeyShares } from './types';
54-
55-
export type EddsaRetrofitData = {
56-
s_i_0: string;
57-
expectedPk: string;
58-
chainCode: string;
59-
};
53+
import { SigningMaterial } from '../../../tss';
54+
import { UShare, YShare } from '../../../../account-lib/mpc/tss';
6055

6156
export class EddsaMPCv2Utils extends BaseEddsaUtils {
6257
private static readonly MPS_DSG_SIGNING_USER_GPG_KEY = 'MPS_DSG_SIGNING_USER_GPG_KEY';
@@ -1076,42 +1071,47 @@ export class EddsaMPCv2Utils extends BaseEddsaUtils {
10761071

10771072
// #region retrofit
10781073

1079-
getMpcV2RetrofitDataFromMpcV1Keys(params: {
1080-
mpcv1UserKeyShare: string;
1081-
mpcv1BackupKeyShare: string;
1082-
}): {
1083-
userRetrofitData: EddsaRetrofitData;
1084-
backupRetrofitData: EddsaRetrofitData;
1085-
} {
1086-
const userKey = JSON.parse(params.mpcv1UserKeyShare);
1087-
assert(typeof userKey?.pShare?.y === 'string', 'MPCv1 user key missing pShare.y (aggregate public key)');
1088-
const expectedPk: string = userKey.pShare.y;
1074+
async getMpcV2RetrofitDataFromMpcV1Keys(params: { mpcv1UserKeyShare: string; mpcv1BackupKeyShare: string }): Promise<{
1075+
userRetrofitData: MPSTypes.EddsaRetrofitData;
1076+
backupRetrofitData: MPSTypes.EddsaRetrofitData;
1077+
}> {
1078+
const userSigningMaterial: SigningMaterial = JSON.parse(params.mpcv1UserKeyShare);
1079+
const backupSigningMaterial: SigningMaterial = JSON.parse(params.mpcv1BackupKeyShare);
1080+
assert(userSigningMaterial.backupYShare, 'MPCv1 user key missing backupYShare');
1081+
assert(backupSigningMaterial.userYShare, 'MPCv1 backup key missing userYShare');
1082+
1083+
const MPC = await getInitializedMpcInstance();
1084+
const userRetrofitData = EddsaMPCv2Utils.getMpcV2RetrofitDataFromMpcV1Key(MPC, userSigningMaterial.uShare, [
1085+
userSigningMaterial.bitgoYShare,
1086+
userSigningMaterial.backupYShare,
1087+
]);
1088+
const backupRetrofitData = EddsaMPCv2Utils.getMpcV2RetrofitDataFromMpcV1Key(MPC, backupSigningMaterial.uShare, [
1089+
backupSigningMaterial.bitgoYShare,
1090+
backupSigningMaterial.userYShare,
1091+
]);
10891092

1090-
return {
1091-
userRetrofitData: EddsaMPCv2Utils.getMpcV2RetrofitDataFromMpcV1Key(params.mpcv1UserKeyShare, expectedPk),
1092-
backupRetrofitData: EddsaMPCv2Utils.getMpcV2RetrofitDataFromMpcV1Key(params.mpcv1BackupKeyShare, expectedPk),
1093-
};
1093+
assert(
1094+
userRetrofitData.expectedPk === backupRetrofitData.expectedPk,
1095+
'MPCv1 user and backup keys combine to different aggregate public keys'
1096+
);
1097+
assert(
1098+
userRetrofitData.chainCode === backupRetrofitData.chainCode,
1099+
'MPCv1 user and backup keys combine to different chain codes'
1100+
);
1101+
1102+
return { userRetrofitData, backupRetrofitData };
10941103
}
10951104

10961105
private static getMpcV2RetrofitDataFromMpcV1Key(
1097-
decryptedKeyShare: string,
1098-
expectedPk: string
1099-
): EddsaRetrofitData {
1100-
const key = JSON.parse(decryptedKeyShare);
1101-
assert(typeof key?.uShare?.seed === 'string', 'MPCv1 key missing uShare.seed');
1102-
assert(typeof key?.uShare?.chaincode === 'string', 'MPCv1 key missing uShare.chaincode');
1103-
1104-
const seedBytes = Buffer.from(key.uShare.seed, 'hex');
1105-
const hash = crypto.createHash('sha512').update(seedBytes).digest();
1106-
const scalar = Buffer.from(hash.subarray(0, 32));
1107-
scalar[0] &= 248;
1108-
scalar[31] &= 127;
1109-
scalar[31] |= 64;
1110-
1106+
mpc: Awaited<ReturnType<typeof getInitializedMpcInstance>>,
1107+
uShare: UShare,
1108+
yShares: YShare[]
1109+
): MPSTypes.EddsaRetrofitData {
1110+
const { pShare } = mpc.keyCombine(uShare, yShares);
11111111
return {
1112-
s_i_0: scalar.toString('hex'),
1113-
expectedPk,
1114-
chainCode: key.uShare.chaincode,
1112+
s_i_0: pShare.u,
1113+
expectedPk: pShare.y,
1114+
chainCode: pShare.chaincode,
11151115
};
11161116
}
11171117

modules/sdk-core/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ import { EcdsaUtils } from './bitgo/utils/tss/ecdsa/ecdsa';
2929
export { EcdsaUtils };
3030
import { EcdsaMPCv2Utils } from './bitgo/utils/tss/ecdsa/ecdsaMPCv2';
3131
export { EcdsaMPCv2Utils };
32-
import { EddsaMPCv2Utils, EddsaRetrofitData } from './bitgo/utils/tss/eddsa/eddsaMPCv2';
33-
export { EddsaMPCv2Utils, EddsaRetrofitData };
32+
import { EddsaMPCv2Utils } from './bitgo/utils/tss/eddsa/eddsaMPCv2';
33+
export { EddsaMPCv2Utils };
3434
export { verifyEddsaTssWalletAddress, verifyMPCWalletAddress } from './bitgo/utils/tss/addressVerification';
3535
export { GShare, SignShare, YShare } from './account-lib/mpc/tss/eddsa/types';
3636
export { TssEcdsaStep1ReturnMessage, TssEcdsaStep2ReturnMessage } from './bitgo/tss/types';

modules/sdk-core/test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts

Lines changed: 54 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
EDDSAUtils,
2323
EddsaMPCv2KeyGenCallbacks,
2424
EddsaMPCv2Utils,
25-
EddsaRetrofitData,
2625
IBaseCoin,
2726
IWallet,
2827
RequestTracer,
@@ -2181,135 +2180,87 @@ describe('EddsaMPCv2Utils.createKeychainsWithExternalSigner', function () {
21812180
});
21822181

21832182
describe('EddsaMPCv2Utils.getMpcV2RetrofitDataFromMpcV1Keys', () => {
2184-
// 32-byte seed and chaincode values used across all tests
2185-
const userSeed = randomBytes(32).toString('hex');
2186-
const backupSeed = randomBytes(32).toString('hex');
2187-
const userChaincode = randomBytes(32).toString('hex');
2188-
const backupChaincode = randomBytes(32).toString('hex');
2189-
const aggregatePk = randomBytes(32).toString('hex');
2190-
2191-
const userMpcV1Key = JSON.stringify({
2192-
uShare: { i: 1, seed: userSeed, chaincode: userChaincode, y: randomBytes(32).toString('hex') },
2193-
pShare: { y: aggregatePk },
2194-
bitgoYShare: { u: randomBytes(32).toString('hex') },
2195-
backupYShare: { u: randomBytes(32).toString('hex') },
2196-
});
2183+
let utils: EddsaMPCv2Utils;
2184+
// Real 3-party MPCv1 EdDSA key shares: 1 = user, 2 = backup, 3 = bitgo.
2185+
let userSigningMaterial: Record<string, unknown>;
2186+
let backupSigningMaterial: Record<string, unknown>;
2187+
let expectedUserPShare: { y: string; u: string; chaincode: string };
2188+
let expectedBackupPShare: { y: string; u: string; chaincode: string };
21972189

2198-
const backupMpcV1Key = JSON.stringify({
2199-
uShare: { i: 2, seed: backupSeed, chaincode: backupChaincode, y: randomBytes(32).toString('hex') },
2200-
bitgoYShare: { u: randomBytes(32).toString('hex') },
2201-
userYShare: { u: randomBytes(32).toString('hex') },
2190+
before(async () => {
2191+
const MPC = await getInitializedMpcInstance();
2192+
const user = MPC.keyShare(1, 2, 3);
2193+
const backup = MPC.keyShare(2, 2, 3);
2194+
const bitgo = MPC.keyShare(3, 2, 3);
2195+
2196+
expectedUserPShare = MPC.keyCombine(user.uShare, [backup.yShares[1], bitgo.yShares[1]]).pShare;
2197+
expectedBackupPShare = MPC.keyCombine(backup.uShare, [user.yShares[2], bitgo.yShares[2]]).pShare;
2198+
2199+
userSigningMaterial = {
2200+
uShare: user.uShare,
2201+
bitgoYShare: bitgo.yShares[1],
2202+
backupYShare: backup.yShares[1],
2203+
};
2204+
backupSigningMaterial = {
2205+
uShare: backup.uShare,
2206+
bitgoYShare: bitgo.yShares[2],
2207+
userYShare: user.yShares[2],
2208+
};
22022209
});
22032210

2204-
let utils: EddsaMPCv2Utils;
2205-
22062211
beforeEach(() => {
22072212
const mockBitGo = {} as unknown as BitGoBase;
22082213
const mockCoin = {} as unknown as IBaseCoin;
22092214
utils = new EddsaMPCv2Utils(mockBitGo, mockCoin);
22102215
});
22112216

2212-
function deriveScalar(seedHex: string): string {
2213-
const { createHash } = require('crypto');
2214-
const seedBytes = Buffer.from(seedHex, 'hex');
2215-
const hash = createHash('sha512').update(seedBytes).digest();
2216-
const scalar = Buffer.from(hash.subarray(0, 32));
2217-
scalar[0] &= 248;
2218-
scalar[31] &= 127;
2219-
scalar[31] |= 64;
2220-
return scalar.toString('hex');
2221-
}
2222-
2223-
it('returns EddsaRetrofitData for user and backup with matching expectedPk', () => {
2224-
const result = utils.getMpcV2RetrofitDataFromMpcV1Keys({
2225-
mpcv1UserKeyShare: userMpcV1Key,
2226-
mpcv1BackupKeyShare: backupMpcV1Key,
2217+
it('derives matching expectedPk and chainCode for user and backup from real MPCv1 key combine', async () => {
2218+
const { userRetrofitData, backupRetrofitData } = await utils.getMpcV2RetrofitDataFromMpcV1Keys({
2219+
mpcv1UserKeyShare: JSON.stringify(userSigningMaterial),
2220+
mpcv1BackupKeyShare: JSON.stringify(backupSigningMaterial),
22272221
});
22282222

2229-
const { userRetrofitData, backupRetrofitData } = result as {
2230-
userRetrofitData: EddsaRetrofitData;
2231-
backupRetrofitData: EddsaRetrofitData;
2232-
};
2233-
2234-
assert.strictEqual(userRetrofitData.expectedPk, aggregatePk);
2235-
assert.strictEqual(backupRetrofitData.expectedPk, aggregatePk);
2236-
});
2237-
2238-
it('returns correct chainCode per party', () => {
2239-
const { userRetrofitData, backupRetrofitData } = utils.getMpcV2RetrofitDataFromMpcV1Keys({
2240-
mpcv1UserKeyShare: userMpcV1Key,
2241-
mpcv1BackupKeyShare: backupMpcV1Key,
2242-
});
2223+
assert.strictEqual(userRetrofitData.expectedPk, expectedUserPShare.y);
2224+
assert.strictEqual(backupRetrofitData.expectedPk, expectedBackupPShare.y);
2225+
assert.strictEqual(userRetrofitData.expectedPk, backupRetrofitData.expectedPk);
22432226

2244-
assert.strictEqual(userRetrofitData.chainCode, userChaincode);
2245-
assert.strictEqual(backupRetrofitData.chainCode, backupChaincode);
2227+
assert.strictEqual(userRetrofitData.chainCode, expectedUserPShare.chaincode);
2228+
assert.strictEqual(backupRetrofitData.chainCode, expectedBackupPShare.chaincode);
2229+
assert.strictEqual(userRetrofitData.chainCode, backupRetrofitData.chainCode);
22462230
});
22472231

2248-
it('returns correctly clamped s_i_0 scalars', () => {
2249-
const { userRetrofitData, backupRetrofitData } = utils.getMpcV2RetrofitDataFromMpcV1Keys({
2250-
mpcv1UserKeyShare: userMpcV1Key,
2251-
mpcv1BackupKeyShare: backupMpcV1Key,
2232+
it('derives s_i_0 as the combined pShare.u (clamped scalar) for each party', async () => {
2233+
const { userRetrofitData, backupRetrofitData } = await utils.getMpcV2RetrofitDataFromMpcV1Keys({
2234+
mpcv1UserKeyShare: JSON.stringify(userSigningMaterial),
2235+
mpcv1BackupKeyShare: JSON.stringify(backupSigningMaterial),
22522236
});
22532237

2254-
assert.strictEqual(userRetrofitData.s_i_0, deriveScalar(userSeed));
2255-
assert.strictEqual(backupRetrofitData.s_i_0, deriveScalar(backupSeed));
2238+
assert.strictEqual(userRetrofitData.s_i_0, expectedUserPShare.u);
2239+
assert.strictEqual(backupRetrofitData.s_i_0, expectedBackupPShare.u);
2240+
assert.notStrictEqual(userRetrofitData.s_i_0, backupRetrofitData.s_i_0);
22562241
});
22572242

2258-
it('scalar byte[0] has low 3 bits cleared, byte[31] has bit7 cleared and bit6 set', () => {
2259-
const { userRetrofitData } = utils.getMpcV2RetrofitDataFromMpcV1Keys({
2260-
mpcv1UserKeyShare: userMpcV1Key,
2261-
mpcv1BackupKeyShare: backupMpcV1Key,
2262-
});
2263-
2264-
const scalarBytes = Buffer.from(userRetrofitData.s_i_0, 'hex');
2265-
assert.strictEqual(scalarBytes[0] & 0b111, 0, 'byte[0] low 3 bits should be cleared');
2266-
assert.strictEqual(scalarBytes[31] & 0b10000000, 0, 'byte[31] bit7 should be cleared');
2267-
assert.strictEqual(scalarBytes[31] & 0b01000000, 0b01000000, 'byte[31] bit6 should be set');
2268-
});
2269-
2270-
it('throws if user key is missing pShare.y', () => {
2271-
const keyNoPShare = JSON.stringify({
2272-
uShare: { i: 1, seed: userSeed, chaincode: userChaincode },
2273-
bitgoYShare: { u: 'x' },
2274-
});
2275-
assert.throws(
2276-
() =>
2277-
utils.getMpcV2RetrofitDataFromMpcV1Keys({
2278-
mpcv1UserKeyShare: keyNoPShare,
2279-
mpcv1BackupKeyShare: backupMpcV1Key,
2280-
}),
2281-
/MPCv1 user key missing pShare\.y/
2282-
);
2283-
});
2284-
2285-
it('throws if user key is missing uShare.seed', () => {
2286-
const keyNoSeed = JSON.stringify({
2287-
uShare: { i: 1, chaincode: userChaincode },
2288-
pShare: { y: aggregatePk },
2289-
bitgoYShare: { u: 'x' },
2290-
});
2291-
assert.throws(
2243+
it('throws if user key is missing backupYShare', async () => {
2244+
const keyNoBackupYShare = JSON.stringify({ uShare: userSigningMaterial.uShare });
2245+
await assert.rejects(
22922246
() =>
22932247
utils.getMpcV2RetrofitDataFromMpcV1Keys({
2294-
mpcv1UserKeyShare: keyNoSeed,
2295-
mpcv1BackupKeyShare: backupMpcV1Key,
2248+
mpcv1UserKeyShare: keyNoBackupYShare,
2249+
mpcv1BackupKeyShare: JSON.stringify(backupSigningMaterial),
22962250
}),
2297-
/MPCv1 key missing uShare\.seed/
2251+
/MPCv1 user key missing backupYShare/
22982252
);
22992253
});
23002254

2301-
it('throws if backup key is missing uShare.seed', () => {
2302-
const backupNoSeed = JSON.stringify({
2303-
uShare: { i: 2, chaincode: backupChaincode },
2304-
bitgoYShare: { u: 'x' },
2305-
});
2306-
assert.throws(
2255+
it('throws if backup key is missing userYShare', async () => {
2256+
const keyNoUserYShare = JSON.stringify({ uShare: backupSigningMaterial.uShare });
2257+
await assert.rejects(
23072258
() =>
23082259
utils.getMpcV2RetrofitDataFromMpcV1Keys({
2309-
mpcv1UserKeyShare: userMpcV1Key,
2310-
mpcv1BackupKeyShare: backupNoSeed,
2260+
mpcv1UserKeyShare: JSON.stringify(userSigningMaterial),
2261+
mpcv1BackupKeyShare: keyNoUserYShare,
23112262
}),
2312-
/MPCv1 key missing uShare\.seed/
2263+
/MPCv1 backup key missing userYShare/
23132264
);
23142265
});
23152266
});

modules/sdk-lib-mpc/src/tss/eddsa-mps/types.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,20 @@ import { decode } from 'cbor-x';
22
import { isLeft } from 'fp-ts/Either';
33
import * as t from 'io-ts';
44

5+
/**
6+
* Retrofit data derived from an existing MPCv1 EdDSA key share, used to seed
7+
* an MPCv2 (MPS) DKG retrofit ceremony (`ed25519_dkg_round0_import`).
8+
*
9+
* @property s_i_0 - Party's clamped additive scalar (pShare.u), 32 bytes LE hex.
10+
* @property expectedPk - Aggregate Ed25519 public key (pShare.y), 32 bytes hex.
11+
* @property chainCode - Combined 32-byte BIP32 chain code (pShare.chaincode), hex.
12+
*/
13+
export type EddsaRetrofitData = {
14+
s_i_0: string;
15+
expectedPk: string;
16+
chainCode: string;
17+
};
18+
519
export const ReducedKeyShareType = t.type({
620
keyShare: t.array(t.number),
721
pub: t.array(t.number),

0 commit comments

Comments
 (0)