-
Notifications
You must be signed in to change notification settings - Fork 302
feat(sdk-core): added OFC BitGo signing on trading accounts object #8666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
46cfaf8
871c837
d75bd7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,16 @@ | ||
| import { ITradingNetwork } from './network'; | ||
|
|
||
| /** | ||
| * Parameters for the signing a payload from the trading account. | ||
| * If both walletPassphrase and prv is not provided the BitGo key will be used | ||
| * @param payload - The payload to sign | ||
| * @param walletPassphrase - The passphrase of the wallet that will be used to decrypt the user key and sign the payload. | ||
| * @param prv - The decrypted user key prv used to sign the payload | ||
| */ | ||
| export interface SignPayloadParameters { | ||
| payload: string | Record<string, unknown>; | ||
| walletPassphrase: string; | ||
| walletPassphrase?: string; | ||
| prv?: string; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this needed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that was what caused the original bug, with the wallets and ofcToken changes, but now I removed it this is no longer need. removed. |
||
| } | ||
|
|
||
| export interface ITradingAccount { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,7 +109,7 @@ export class TradingNetwork implements ITradingNetwork { | |
|
|
||
| /** | ||
| * Prepare an allocation for submission | ||
| * @param {string} walletPassphrase ofc wallet passphrase | ||
| * @param {string} walletPassphrase ofc wallet passphrase - required only when signing via user key | ||
| * @param {string} connectionId connection to whom to make the allocation or deallocation | ||
| * @param {string=} clientExternalId one time generated uuid v4 | ||
| * @param {string} currency currency for which the allocation should be made. e.g. btc / tbtc | ||
|
|
@@ -130,10 +130,7 @@ export class TradingNetwork implements ITradingNetwork { | |
| } | ||
|
|
||
| const payload = JSON.stringify(body); | ||
|
|
||
| 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 }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should |
||
|
|
||
| return { | ||
| ...body, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,18 +23,72 @@ export class TradingAccount implements ITradingAccount { | |
| } | ||
|
|
||
| /** | ||
| * Signs an arbitrary payload with the user key on this trading account | ||
| * Signs an arbitrary payload. Use the user key if passphrase/prv is provided, or the BitGo key if not. | ||
| * @param params | ||
| * @param params.payload arbitrary payload object (string | Record<string, unknown>) | ||
| * @param params.walletPassphrase passphrase on this trading account, used to unlock the account user key | ||
| * @param params.prv user private key, used to sign the payload locally | ||
| * @returns hex-encoded signature of the payload | ||
| */ | ||
| async signPayload(params: SignPayloadParameters): Promise<string> { | ||
| const key = (await this.wallet.baseCoin.keychains().get({ id: this.wallet.keyIds()[0] })) as any; | ||
| const prv = this.wallet.bitgo.decrypt({ | ||
| input: key.encryptedPrv, | ||
| password: params.walletPassphrase, | ||
| }); | ||
| // if no passphrase is provided, attempt to sign using the wallet's bitgo key remotely | ||
| if (!params.walletPassphrase && !params.prv) { | ||
| return this.signPayloadByBitGoKey(params); | ||
| } | ||
| // if a passphrase is provided, we must be trying to sign using the user private key - decrypt and sign locally | ||
| return this.signPayloadByUserKey(params); | ||
| } | ||
|
|
||
| /** | ||
| * Signs the payload of a trading account via the trading account BitGo key | ||
| * @param params | ||
| * @private | ||
| */ | ||
| private async signPayloadByBitGoKey( | ||
| params: Omit<SignPayloadParameters, 'walletPassphrase' | 'prv'> | ||
| ): Promise<string> { | ||
| const walletData = this.wallet.toJSON(); | ||
| if (walletData.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.' | ||
| ); | ||
| } | ||
| if (walletData.keys.length < 2) { | ||
| throw new Error( | ||
| 'Wallet does not support BitGo signing. Please reach out to support@bitgo.com to resolve this issue.' | ||
| ); | ||
| } | ||
|
|
||
| // 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 | ||
| const url = this.wallet.url('/tx/sign'); | ||
| const payload = typeof params.payload !== 'string' ? JSON.stringify(params.payload) : params.payload; | ||
| const { signature } = await this.wallet.bitgo.post(url).send({ payload }).result(); | ||
|
|
||
| return signature; | ||
| } | ||
|
|
||
| /** | ||
| * Signs the payload of a trading account locally by fetching the user's encrypted private key and decrypt using passphrase | ||
| * @param params | ||
| * @private | ||
| */ | ||
| private async signPayloadByUserKey(params: SignPayloadParameters): Promise<string> { | ||
| if (!params.prv && !params.walletPassphrase) { | ||
| throw new Error( | ||
| '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.' | ||
| ); | ||
| } | ||
|
|
||
| let prv: string; | ||
| if (params.prv) { | ||
| prv = params.prv; | ||
| } else { | ||
| const key = (await this.wallet.baseCoin.keychains().get({ id: this.wallet.keyIds()[0] })) as any; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we casting as any here? |
||
| prv = this.wallet.bitgo.decrypt({ | ||
| input: key.encryptedPrv, | ||
| password: params.walletPassphrase, | ||
| }); | ||
| } | ||
| const payload = typeof params.payload === 'string' ? params.payload : JSON.stringify(params.payload); | ||
| return ((await this.wallet.baseCoin.signMessage({ prv }, payload)) as any).toString('hex'); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should note that this is only valid when they have the wallet setting enabled.