Skip to content

Commit fc02b6b

Browse files
alextse-bgalextse-bg
committed
feat(sdk-core): added OFC BitGo signing on trading accounts object
make wallet passphrase optional when signing trading account TXs allow the use of prv when signing trading account TXs Ticket: WCN-217-1
1 parent 88d2979 commit fc02b6b

5 files changed

Lines changed: 358 additions & 8 deletions

File tree

‎modules/sdk-core/src/bitgo/trading/iTradingAccount.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,19 @@
11
import { ITradingNetwork } from './network';
22

3+
/**
4+
* Parameters for the signing a payload from the trading account.
5+
* If both walletPassphrase and prv are not provided, the BitGo key will be used.
6+
*
7+
* @note If wallet has userKeySigningRequired set to true, then attempting to sign with BitGo key will throw.
8+
*
9+
* @param payload - The payload to sign
10+
* @param walletPassphrase - The passphrase of the wallet that will be used to decrypt the user key and sign the payload.
11+
* @param prv - The decrypted user key prv used to sign the payload
12+
*/
313
export interface SignPayloadParameters {
414
payload: string | Record<string, unknown>;
5-
walletPassphrase: string;
15+
walletPassphrase?: string;
16+
prv?: string;
617
}
718

819
export interface ITradingAccount {

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

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,74 @@ export class TradingAccount implements ITradingAccount {
2323
}
2424

2525
/**
26-
* Signs an arbitrary payload with the user key on this trading account
26+
* Signs an arbitrary payload. Use the user key if passphrase/prv is provided, or the BitGo key if not.
2727
* @param params
2828
* @param params.payload arbitrary payload object (string | Record<string, unknown>)
2929
* @param params.walletPassphrase passphrase on this trading account, used to unlock the account user key
30+
* @param params.prv user private key, used to sign the payload locally
3031
* @returns hex-encoded signature of the payload
3132
*/
3233
async signPayload(params: SignPayloadParameters): Promise<string> {
33-
const key = (await this.wallet.baseCoin.keychains().get({ id: this.wallet.keyIds()[0] })) as any;
34-
const prv = this.wallet.bitgo.decrypt({
35-
input: key.encryptedPrv,
36-
password: params.walletPassphrase,
37-
});
34+
// if no passphrase is provided, attempt to sign using the wallet's bitgo key remotely
35+
if (!params.walletPassphrase && !params.prv) {
36+
return this.signPayloadByBitGoKey(params);
37+
}
38+
// if a passphrase is provided, we must be trying to sign using the user private key - decrypt and sign locally
39+
return this.signPayloadByUserKey(params);
40+
}
41+
42+
/**
43+
* Signs the payload of a trading account via the trading account BitGo key
44+
* @param params
45+
* @private
46+
*/
47+
private async signPayloadByBitGoKey(
48+
params: Omit<SignPayloadParameters, 'walletPassphrase' | 'prv'>
49+
): Promise<string> {
50+
const walletData = this.wallet.toJSON();
51+
if (walletData.userKeySigningRequired) {
52+
throw new Error(
53+
'Wallet must use user key to sign ofc transaction, please provide the wallet passphrase or visit your wallet settings page to configure one.'
54+
);
55+
}
56+
if (walletData.keys.length < 2) {
57+
throw new Error(
58+
'Wallet does not support BitGo signing. Please reach out to support@bitgo.com to resolve this issue.'
59+
);
60+
}
61+
62+
// we do not parse the payload here, we instead sends the payload as a stringified JSON to be signed, just like how we process it locally
63+
const url = this.wallet.url('/tx/sign');
64+
const payload = typeof params.payload !== 'string' ? JSON.stringify(params.payload) : params.payload;
65+
const { signature } = await this.wallet.bitgo.post(url).send({ payload }).result();
66+
67+
return signature;
68+
}
69+
70+
/**
71+
* Signs the payload of a trading account locally by fetching the user's encrypted private key and decrypt using passphrase
72+
* @param params
73+
* @private
74+
*/
75+
private async signPayloadByUserKey(params: SignPayloadParameters): Promise<string> {
76+
if (!params.prv && !params.walletPassphrase) {
77+
throw new Error(
78+
'Must provide either prv or walletPassphrase to sign payload using user key. Please provide the wallet passphrase or visit your wallet settings page to configure one.'
79+
);
80+
}
81+
82+
let prv: string;
83+
if (params.prv) {
84+
prv = params.prv;
85+
} else {
86+
const key = await this.wallet.baseCoin.keychains().get({ id: this.wallet.keyIds()[0] });
87+
prv = this.wallet.bitgo.decrypt({
88+
input: key.encryptedPrv as string,
89+
password: params.walletPassphrase,
90+
});
91+
}
3892
const payload = typeof params.payload === 'string' ? params.payload : JSON.stringify(params.payload);
39-
return ((await this.wallet.baseCoin.signMessage({ prv }, payload)) as any).toString('hex');
93+
return (await this.wallet.baseCoin.signMessage({ prv }, payload)).toString('hex');
4094
}
4195

4296
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,7 @@ export interface WalletData {
909909
evmKeyRingReferenceWalletId?: string;
910910
isParent?: boolean;
911911
enabledChildChains?: string[];
912+
userKeySigningRequired?: boolean;
912913
}
913914

914915
export interface RecoverTokenOptions {

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

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/**
2+
* @prettier
3+
*/
4+
import sinon from 'sinon';
5+
import 'should';
6+
import { TradingAccount } from '../../../../src/bitgo/trading/tradingAccount';
7+
8+
describe('TradingAccount', function () {
9+
let tradingAccount: TradingAccount;
10+
let mockBitGo: any;
11+
let mockWallet: any;
12+
let mockBaseCoin: any;
13+
let sendStub: sinon.SinonStub;
14+
15+
const enterpriseId = 'test-enterprise-id';
16+
const walletPassphrase = 'test-passphrase';
17+
const encryptedPrv = 'encrypted-prv';
18+
const decryptedPrv = 'decrypted-prv';
19+
const signature = 'aabbccdd';
20+
const payload = { data: 'test-payload' };
21+
const payloadString = JSON.stringify(payload);
22+
23+
beforeEach(function () {
24+
sendStub = sinon.stub();
25+
sendStub.withArgs({ payload: payloadString }).returns({ result: sinon.stub().resolves({ signature }) });
26+
27+
mockBitGo = {
28+
post: sinon.stub().returns({ send: sendStub }),
29+
decrypt: sinon
30+
.stub()
31+
.callsFake(({ input, password }) =>
32+
input === encryptedPrv && password === walletPassphrase ? decryptedPrv : undefined
33+
),
34+
};
35+
36+
mockBaseCoin = {
37+
keychains: sinon.stub().returns({
38+
get: sinon.stub().resolves({ encryptedPrv }),
39+
}),
40+
signMessage: sinon.stub().callsFake(async (key: { prv: string }) => {
41+
if (key.prv === decryptedPrv) {
42+
return Buffer.from(signature, 'hex');
43+
}
44+
throw new Error(`signMessage called with unexpected prv: ${key.prv}`);
45+
}),
46+
};
47+
48+
mockWallet = {
49+
id: sinon.stub().returns('test-wallet-id'),
50+
keyIds: sinon.stub().returns(['user-key-id', 'bitgo-key-id']),
51+
url: sinon.stub().returns('https://example.com/wallet/test-wallet-id/tx/sign'),
52+
toJSON: sinon.stub().returns({
53+
id: 'test-wallet-id',
54+
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
55+
userKeySigningRequired: undefined, // default is undefined
56+
}),
57+
baseCoin: mockBaseCoin,
58+
bitgo: mockBitGo,
59+
};
60+
61+
tradingAccount = new TradingAccount(enterpriseId, mockWallet, mockBitGo);
62+
});
63+
64+
afterEach(function () {
65+
sinon.restore();
66+
});
67+
68+
describe('id', function () {
69+
it('should return the wallet id', function () {
70+
tradingAccount.id.should.equal('test-wallet-id');
71+
mockWallet.id.calledOnce.should.be.true();
72+
});
73+
});
74+
75+
describe('signPayload', function () {
76+
describe('without walletPassphrase or prv (BitGo remote signing)', function () {
77+
it('should sign using the BitGo key remotely when no passphrase is provided', async function () {
78+
const result = await tradingAccount.signPayload({ payload });
79+
80+
mockWallet.toJSON.calledOnce.should.be.true();
81+
mockWallet.url.calledWith('/tx/sign').should.be.true();
82+
mockBitGo.post.calledOnce.should.be.true();
83+
sendStub.calledWith({ payload: JSON.stringify(payload) }).should.be.true();
84+
result.should.equal(signature);
85+
});
86+
87+
it('should sign a string payload remotely when no passphrase is provided', async function () {
88+
const result = await tradingAccount.signPayload({ payload: payloadString });
89+
sendStub.calledWith({ payload: payloadString }).should.be.true();
90+
result.should.equal(signature);
91+
});
92+
93+
it('should throw if userKeySigningRequired is set and no passphrase and prv are provided', async function () {
94+
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
95+
id: 'test-wallet-id',
96+
keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'],
97+
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 throw if wallet has fewer than 2 keys and no passphrase and prv are provided', async function () {
108+
mockWallet.toJSON.onCall(mockWallet.toJSON.callCount).returns({
109+
id: 'test-wallet-id',
110+
keys: ['user-key-id'],
111+
userKeySigningRequired: undefined,
112+
});
113+
114+
await tradingAccount
115+
.signPayload({ payload })
116+
.should.be.rejectedWith(
117+
'Wallet does not support BitGo signing. Please reach out to support@bitgo.com to resolve this issue.'
118+
);
119+
});
120+
});
121+
122+
describe('with walletPassphrase (local user key signing)', function () {
123+
it('should decrypt the user key and sign the payload locally', async function () {
124+
const result = await tradingAccount.signPayload({ payload, walletPassphrase });
125+
126+
mockBaseCoin.keychains().get.calledWith({ id: 'user-key-id' }).should.be.true();
127+
mockBitGo.decrypt.calledWith({ input: encryptedPrv, password: walletPassphrase }).should.be.true();
128+
mockBaseCoin.signMessage.calledOnce.should.be.true();
129+
result.should.equal(Buffer.from(signature, 'hex').toString('hex'));
130+
});
131+
132+
it('should stringify a Record payload before signing locally', async function () {
133+
await tradingAccount.signPayload({ payload, walletPassphrase });
134+
135+
const signMessageCall = mockBaseCoin.signMessage.getCall(0);
136+
signMessageCall.args[1].should.equal(JSON.stringify(payload));
137+
});
138+
139+
it('should pass a string payload directly to signMessage', async function () {
140+
await tradingAccount.signPayload({ payload: payloadString, walletPassphrase });
141+
142+
const signMessageCall = mockBaseCoin.signMessage.getCall(0);
143+
signMessageCall.args[1].should.equal(payloadString);
144+
});
145+
});
146+
147+
describe('with both walletPassphrase and prv', function () {
148+
it('should use prv directly and not call decrypt when both are provided', async function () {
149+
await tradingAccount.signPayload({ payload, walletPassphrase, prv: decryptedPrv });
150+
151+
mockBitGo.decrypt.called.should.be.false();
152+
mockBaseCoin.keychains.called.should.be.false();
153+
const signMessageCall = mockBaseCoin.signMessage.getCall(0);
154+
signMessageCall.args[0].should.deepEqual({ prv: decryptedPrv });
155+
});
156+
});
157+
158+
describe('with prv (local user key signing without decryption)', function () {
159+
it('should sign using the provided prv without calling decrypt', async function () {
160+
const result = await tradingAccount.signPayload({ payload, prv: decryptedPrv });
161+
162+
mockBitGo.decrypt.called.should.be.false();
163+
mockBaseCoin.keychains.called.should.be.false();
164+
mockBaseCoin.signMessage.calledOnce.should.be.true();
165+
result.should.equal(Buffer.from(signature, 'hex').toString('hex'));
166+
});
167+
168+
it('should not use BitGo remote signing when prv is provided', async function () {
169+
await tradingAccount.signPayload({ payload, prv: decryptedPrv });
170+
171+
mockBitGo.post.called.should.be.false();
172+
});
173+
174+
it('should pass the prv directly to signMessage', async function () {
175+
await tradingAccount.signPayload({ payload, prv: decryptedPrv });
176+
177+
const signMessageCall = mockBaseCoin.signMessage.getCall(0);
178+
signMessageCall.args[0].should.deepEqual({ prv: decryptedPrv });
179+
});
180+
181+
it('should stringify a Record payload before signing with prv', async function () {
182+
await tradingAccount.signPayload({ payload, prv: decryptedPrv });
183+
184+
const signMessageCall = mockBaseCoin.signMessage.getCall(0);
185+
signMessageCall.args[1].should.equal(JSON.stringify(payload));
186+
});
187+
188+
it('should pass a string payload directly to signMessage when signing with prv', async function () {
189+
await tradingAccount.signPayload({ payload: payloadString, prv: decryptedPrv });
190+
191+
const signMessageCall = mockBaseCoin.signMessage.getCall(0);
192+
signMessageCall.args[1].should.equal(payloadString);
193+
});
194+
});
195+
});
196+
});

‎modules/sdk-core/test/unit/bitgo/wallet/ofcWalletSignTransaction.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/**
2+
* @prettier
3+
*/
4+
import sinon from 'sinon';
5+
import 'should';
6+
import { Wallet } from '../../../../src';
7+
8+
describe('Wallet - OFC signTransaction', function () {
9+
let wallet: Wallet;
10+
let mockBitGo: any;
11+
let mockBaseCoin: any;
12+
let mockWalletData: any;
13+
14+
beforeEach(function () {
15+
mockBitGo = {
16+
url: sinon.stub().returns('https://test.bitgo.com'),
17+
post: sinon.stub(),
18+
get: sinon.stub(),
19+
setRequestTracer: sinon.stub(),
20+
};
21+
22+
mockBaseCoin = {
23+
getFamily: sinon.stub().returns('ofc'),
24+
url: sinon.stub().returns('https://test.bitgo.com/wallet'),
25+
keychains: sinon.stub(),
26+
supportsTss: sinon.stub().returns(false),
27+
getMPCAlgorithm: sinon.stub(),
28+
presignTransaction: sinon.stub().resolvesArg(0),
29+
keyIdsForSigning: sinon.stub().returns([0]),
30+
signTransaction: sinon.stub().resolves({ halfSigned: { payload: 'test', signature: 'aabbcc' } }),
31+
};
32+
33+
mockWalletData = {
34+
id: 'test-wallet-id',
35+
coin: 'ofcusdt',
36+
keys: ['user-key', 'backup-key', 'bitgo-key'],
37+
multisigType: 'onchain',
38+
enterprise: 'ent-id',
39+
};
40+
41+
wallet = new Wallet(mockBitGo, mockBaseCoin, mockWalletData);
42+
});
43+
44+
afterEach(function () {
45+
sinon.restore();
46+
});
47+
48+
it('should pass wallet instance to baseCoin.signTransaction', async function () {
49+
const txPrebuild = { txInfo: { payload: '{"amount":"100"}' } } as any;
50+
const prv = 'test-prv';
51+
52+
await wallet.signTransaction({ txPrebuild, prv });
53+
54+
mockBaseCoin.signTransaction.calledOnce.should.be.true();
55+
const callArgs = mockBaseCoin.signTransaction.getCall(0).args[0];
56+
callArgs.wallet.should.equal(wallet);
57+
});
58+
59+
it('should pass prv to baseCoin.signTransaction when provided directly', async function () {
60+
const txPrebuild = { txInfo: { payload: '{"amount":"100"}' } } as any;
61+
const prv = 'test-prv';
62+
63+
await wallet.signTransaction({ txPrebuild, prv });
64+
65+
const callArgs = mockBaseCoin.signTransaction.getCall(0).args[0];
66+
callArgs.prv.should.equal(prv);
67+
});
68+
69+
it('should pass wallet instance to baseCoin.signTransaction even when no prv is available', async function () {
70+
sinon.stub(wallet, 'getUserPrv').returns(undefined as any);
71+
const txPrebuild = { txInfo: { payload: '{"amount":"100"}' } } as any;
72+
73+
await wallet.signTransaction({ txPrebuild });
74+
75+
mockBaseCoin.signTransaction.calledOnce.should.be.true();
76+
const callArgs = mockBaseCoin.signTransaction.getCall(0).args[0];
77+
callArgs.wallet.should.equal(wallet);
78+
});
79+
80+
it('should return the result from baseCoin.signTransaction', async function () {
81+
const txPrebuild = { txInfo: { payload: '{"amount":"100"}' } } as any;
82+
const prv = 'test-prv';
83+
84+
const result = await wallet.signTransaction({ txPrebuild, prv });
85+
86+
result.should.deepEqual({ halfSigned: { payload: 'test', signature: 'aabbcc' } });
87+
});
88+
});

0 commit comments

Comments
 (0)