Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/express/src/typedRoutes/schemas/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export const WalletResponse = t.partial({
/** Multisig type version (e.g., 'MPCv2') */
multisigTypeVersion: t.string,
/** Coin-specific wallet data */
coinSpecific: t.UnknownRecord,
coinSpecific: t.intersection([t.partial({ userKeySigningRequired: t.boolean }), t.UnknownRecord]),
Comment thread
zahin-mohammad marked this conversation as resolved.
/** Admin settings including policy */
admin: WalletAdmin,
/** Users with access to this wallet */
Expand Down
30 changes: 30 additions & 0 deletions modules/express/test/unit/typedRoutes/decode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
ExpressWalletUpdateParams,
} from '../../../src/typedRoutes/api/v2/expressWalletUpdate';
import { SignerMacaroonBody, SignerMacaroonParams } from '../../../src/typedRoutes/api/v2/signerMacaroon';
import { WalletResponse } from '../../../src/typedRoutes/schemas/wallet';

export function assertDecode<T>(codec: t.Type<T, unknown>, input: unknown): T {
const result = codec.decode(input);
Expand Down Expand Up @@ -306,4 +307,33 @@ describe('io-ts decode tests', function () {
it('express.lightning.signerMacaroon params invalid', function () {
assert.throws(() => assertDecode(t.type(SignerMacaroonParams), { coin: 'lnbtc' }));
});
describe('WalletResponse coinSpecific', function () {
it('decodes wallets with arbitrary coinSpecific fields and no userKeySigningRequired', function () {
// OFC subdocument toJSON in WP flattens fields directly into coinSpecific; non-OFC
// wallets carry unrelated keys. Both shapes must decode through the permissive intersection.
const decoded = assertDecode(WalletResponse, {
id: 'wallet123',
coinSpecific: { baseAddress: '0xabc', someEthField: 1 },
});
assert.deepStrictEqual(decoded.coinSpecific, { baseAddress: '0xabc', someEthField: 1 });
});
it('decodes wallets with coinSpecific.userKeySigningRequired', function () {
const decoded = assertDecode(WalletResponse, {
id: 'wallet123',
coinSpecific: { userKeySigningRequired: true, needsKeyReshareAfterPasswordReset: false },
});
assert.strictEqual(decoded.coinSpecific?.userKeySigningRequired, true);
});
it('decodes wallets with empty coinSpecific', function () {
assertDecode(WalletResponse, { id: 'wallet123', coinSpecific: {} });
});
it('rejects coinSpecific.userKeySigningRequired of wrong type', function () {
assert.throws(() =>
assertDecode(WalletResponse, {
id: 'wallet123',
coinSpecific: { userKeySigningRequired: 'yes' },
})
);
});
});
});
3 changes: 2 additions & 1 deletion modules/sdk-core/src/bitgo/trading/tradingAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export class TradingAccount implements ITradingAccount {
params: Omit<SignPayloadParameters, 'walletPassphrase' | 'prv'>
): Promise<string> {
const walletData = this.wallet.toJSON();
if (walletData.userKeySigningRequired) {
const userKeySigningRequired = walletData.coinSpecific?.userKeySigningRequired ?? walletData.userKeySigningRequired;
if (userKeySigningRequired) {
throw new Error(
'Wallet must use user key to sign ofc transaction, please provide the wallet passphrase or visit your wallet settings page to configure one.'
);
Expand Down
6 changes: 6 additions & 0 deletions modules/sdk-core/src/bitgo/wallet/iWallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ export interface WalletCoinSpecific {
pendingEcdsaTssInitialization?: boolean;
features?: string[];
freezeDepositsFromShielded?: boolean;
userKeySigningRequired?: boolean;
/**
* Lightning coin specific data starts
*/
Expand Down Expand Up @@ -946,6 +947,11 @@ export interface WalletData {
evmKeyRingReferenceWalletId?: string;
isParent?: boolean;
enabledChildChains?: string[];
/**
* @deprecated Read from `coinSpecific.userKeySigningRequired` instead. Retained
* temporarily as a fallback while the field migrates from the top level to the OFC
* coinSpecific subdocument; will be removed in a follow-up major release.
*/
userKeySigningRequired?: boolean;
}

Expand Down
33 changes: 30 additions & 3 deletions modules/sdk-core/test/unit/bitgo/trading/tradingAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('TradingAccount', function () {
toJSON: sinon.stub().returns({
id: 'test-wallet-id',
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
userKeySigningRequired: undefined, // default is undefined
coinSpecific: {},
}),
baseCoin: mockBaseCoin,
bitgo: mockBitGo,
Expand Down Expand Up @@ -90,10 +90,25 @@ describe('TradingAccount', function () {
result.should.equal(signature);
});

it('should throw if userKeySigningRequired is set and no passphrase and prv are provided', async function () {
it('should throw if coinSpecific.userKeySigningRequired is true and no passphrase and prv are provided', async function () {
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
id: 'test-wallet-id',
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
coinSpecific: { userKeySigningRequired: true },
});

await tradingAccount
.signPayload({ payload })
.should.be.rejectedWith(
'Wallet must use user key to sign ofc transaction, please provide the wallet passphrase or visit your wallet settings page to configure one.'
);
});

it('should fall back to top-level userKeySigningRequired when coinSpecific does not carry it', async function () {
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
id: 'test-wallet-id',
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
coinSpecific: {},
userKeySigningRequired: true,
});

Expand All @@ -104,11 +119,23 @@ describe('TradingAccount', function () {
);
});

it('should prefer coinSpecific.userKeySigningRequired over top-level when both are present', async function () {
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
id: 'test-wallet-id',
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
coinSpecific: { userKeySigningRequired: false },
userKeySigningRequired: true,
});

const result = await tradingAccount.signPayload({ payload });
result.should.equal(signature);
});

it('should throw if wallet has fewer than 2 keys and no passphrase and prv are provided', async function () {
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
id: 'test-wallet-id',
keys: ['user-key-id'],
userKeySigningRequired: undefined,
coinSpecific: {},
});

await tradingAccount
Expand Down
Loading