Skip to content

Commit 9370461

Browse files
Merge pull request #8712 from BitGo/zahinmohammad/wcn-471-refactorsdk-consume-userkeysigningrequired-from
feat(sdk-core, express): consume userKeySigningRequired from coinSpecific
2 parents 0773632 + 0d223d5 commit 9370461

5 files changed

Lines changed: 69 additions & 5 deletions

File tree

modules/express/src/typedRoutes/schemas/wallet.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ export const WalletResponse = t.partial({
189189
/** Multisig type version (e.g., 'MPCv2') */
190190
multisigTypeVersion: t.string,
191191
/** Coin-specific wallet data */
192-
coinSpecific: t.UnknownRecord,
192+
coinSpecific: t.intersection([t.partial({ userKeySigningRequired: t.boolean }), t.UnknownRecord]),
193193
/** Admin settings including policy */
194194
admin: WalletAdmin,
195195
/** Users with access to this wallet */

modules/express/test/unit/typedRoutes/decode.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ExpressWalletUpdateParams,
2121
} from '../../../src/typedRoutes/api/v2/expressWalletUpdate';
2222
import { SignerMacaroonBody, SignerMacaroonParams } from '../../../src/typedRoutes/api/v2/signerMacaroon';
23+
import { WalletResponse } from '../../../src/typedRoutes/schemas/wallet';
2324

2425
export function assertDecode<T>(codec: t.Type<T, unknown>, input: unknown): T {
2526
const result = codec.decode(input);
@@ -306,4 +307,33 @@ describe('io-ts decode tests', function () {
306307
it('express.lightning.signerMacaroon params invalid', function () {
307308
assert.throws(() => assertDecode(t.type(SignerMacaroonParams), { coin: 'lnbtc' }));
308309
});
310+
describe('WalletResponse coinSpecific', function () {
311+
it('decodes wallets with arbitrary coinSpecific fields and no userKeySigningRequired', function () {
312+
// OFC subdocument toJSON in WP flattens fields directly into coinSpecific; non-OFC
313+
// wallets carry unrelated keys. Both shapes must decode through the permissive intersection.
314+
const decoded = assertDecode(WalletResponse, {
315+
id: 'wallet123',
316+
coinSpecific: { baseAddress: '0xabc', someEthField: 1 },
317+
});
318+
assert.deepStrictEqual(decoded.coinSpecific, { baseAddress: '0xabc', someEthField: 1 });
319+
});
320+
it('decodes wallets with coinSpecific.userKeySigningRequired', function () {
321+
const decoded = assertDecode(WalletResponse, {
322+
id: 'wallet123',
323+
coinSpecific: { userKeySigningRequired: true, needsKeyReshareAfterPasswordReset: false },
324+
});
325+
assert.strictEqual(decoded.coinSpecific?.userKeySigningRequired, true);
326+
});
327+
it('decodes wallets with empty coinSpecific', function () {
328+
assertDecode(WalletResponse, { id: 'wallet123', coinSpecific: {} });
329+
});
330+
it('rejects coinSpecific.userKeySigningRequired of wrong type', function () {
331+
assert.throws(() =>
332+
assertDecode(WalletResponse, {
333+
id: 'wallet123',
334+
coinSpecific: { userKeySigningRequired: 'yes' },
335+
})
336+
);
337+
});
338+
});
309339
});

modules/sdk-core/src/bitgo/trading/tradingAccount.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ export class TradingAccount implements ITradingAccount {
4848
params: Omit<SignPayloadParameters, 'walletPassphrase' | 'prv'>
4949
): Promise<string> {
5050
const walletData = this.wallet.toJSON();
51-
if (walletData.userKeySigningRequired) {
51+
const userKeySigningRequired = walletData.coinSpecific?.userKeySigningRequired ?? walletData.userKeySigningRequired;
52+
if (userKeySigningRequired) {
5253
throw new Error(
5354
'Wallet must use user key to sign ofc transaction, please provide the wallet passphrase or visit your wallet settings page to configure one.'
5455
);

modules/sdk-core/src/bitgo/wallet/iWallet.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ export interface WalletCoinSpecific {
379379
pendingEcdsaTssInitialization?: boolean;
380380
features?: string[];
381381
freezeDepositsFromShielded?: boolean;
382+
userKeySigningRequired?: boolean;
382383
/**
383384
* Lightning coin specific data starts
384385
*/
@@ -946,6 +947,11 @@ export interface WalletData {
946947
evmKeyRingReferenceWalletId?: string;
947948
isParent?: boolean;
948949
enabledChildChains?: string[];
950+
/**
951+
* @deprecated Read from `coinSpecific.userKeySigningRequired` instead. Retained
952+
* temporarily as a fallback while the field migrates from the top level to the OFC
953+
* coinSpecific subdocument; will be removed in a follow-up major release.
954+
*/
949955
userKeySigningRequired?: boolean;
950956
}
951957

modules/sdk-core/test/unit/bitgo/trading/tradingAccount.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe('TradingAccount', function () {
5252
toJSON: sinon.stub().returns({
5353
id: 'test-wallet-id',
5454
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
55-
userKeySigningRequired: undefined, // default is undefined
55+
coinSpecific: {},
5656
}),
5757
baseCoin: mockBaseCoin,
5858
bitgo: mockBitGo,
@@ -90,10 +90,25 @@ describe('TradingAccount', function () {
9090
result.should.equal(signature);
9191
});
9292

93-
it('should throw if userKeySigningRequired is set and no passphrase and prv are provided', async function () {
93+
it('should throw if coinSpecific.userKeySigningRequired is true and no passphrase and prv are provided', async function () {
9494
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
9595
id: 'test-wallet-id',
9696
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
97+
coinSpecific: { userKeySigningRequired: true },
98+
});
99+
100+
await tradingAccount
101+
.signPayload({ payload })
102+
.should.be.rejectedWith(
103+
'Wallet must use user key to sign ofc transaction, please provide the wallet passphrase or visit your wallet settings page to configure one.'
104+
);
105+
});
106+
107+
it('should fall back to top-level userKeySigningRequired when coinSpecific does not carry it', async function () {
108+
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
109+
id: 'test-wallet-id',
110+
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
111+
coinSpecific: {},
97112
userKeySigningRequired: true,
98113
});
99114

@@ -104,11 +119,23 @@ describe('TradingAccount', function () {
104119
);
105120
});
106121

122+
it('should prefer coinSpecific.userKeySigningRequired over top-level when both are present', async function () {
123+
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
124+
id: 'test-wallet-id',
125+
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
126+
coinSpecific: { userKeySigningRequired: false },
127+
userKeySigningRequired: true,
128+
});
129+
130+
const result = await tradingAccount.signPayload({ payload });
131+
result.should.equal(signature);
132+
});
133+
107134
it('should throw if wallet has fewer than 2 keys and no passphrase and prv are provided', async function () {
108135
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
109136
id: 'test-wallet-id',
110137
keys: ['user-key-id'],
111-
userKeySigningRequired: undefined,
138+
coinSpecific: {},
112139
});
113140

114141
await tradingAccount

0 commit comments

Comments
 (0)