feat(sdk-core): added OFC BitGo signing on trading accounts object#8666
feat(sdk-core): added OFC BitGo signing on trading accounts object#8666alextse-bg wants to merge 3 commits intomasterfrom
Conversation
alextse-bg
force-pushed
the
WCN-217
branch
5 times, most recently
from
4f679c7 to
467a1cc
Compare
make wallet passphrase optional when signing trading account TXs allow the use of prv when signing trading account TXs Ticket: WCN-217-1
When no walletPassphrase is present in the request body or environment, pass undefined to tradingAccount.signPayload() instead of throwing. The SDK routes passphrase-less signing through KMS internally. Ticket: WCN-215-1
|
reimplements the reverted PR: #8624 Taken out the changes to ofcToken/ofc.ts. Changes to these files has no impact on current code path (since we are signing with the user key right now, |
alextse-bg
changed the title
| const prv = await this.wallet.getPrv({ walletPassphrase }); | ||
| const signedBuffer: Buffer = await this.wallet.baseCoin.signMessage({ prv }, payload); | ||
| const signature = signedBuffer.toString('hex'); | ||
| const signature = await this.wallet.toTradingAccount().signPayload({ payload, walletPassphrase }); |
There was a problem hiding this comment.
Should prepareAllocation fail fast when walletPassphrase / prv is missing but the wallet requires user signing (if that flag is knowable without extra round trips), instead of building the payload and failing at sign time?
zahin-mohammad
left a comment
zahin-mohammad
left a comment
There was a problem hiding this comment.
In general lgtm, but please address the as any
|
|
||
| /** | ||
| * Parameters for the signing a payload from the trading account. | ||
| * If both walletPassphrase and prv is not provided the BitGo key will be used |
There was a problem hiding this comment.
We should note that this is only valid when they have the wallet setting enabled.
| payload: string | Record<string, unknown>; | ||
| walletPassphrase: string; | ||
| walletPassphrase?: string; | ||
| prv?: string; |
There was a problem hiding this comment.
that was what caused the original bug, with the wallets and ofcToken changes, but now I removed it this is no longer need. removed.
| if (params.prv) { | ||
| prv = params.prv; | ||
| } else { | ||
| const key = (await this.wallet.baseCoin.keychains().get({ id: this.wallet.keyIds()[0] })) as any; |
There was a problem hiding this comment.
why are we casting as any here?
3 participants
TICKET: WCN-217