-
Notifications
You must be signed in to change notification settings - Fork 1
recovery multisig for eve and mbe #23
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
Changes from all commits
8639449
ed414f7
efaab2b
3642a2a
f84a799
0ec107a
a5ff857
bbf2bf4
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 |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "halfSigned": { | ||
| "recipients": [ | ||
| { | ||
| "address": "0xe7d07af8e3e7472ea8391a3372ab98d04ac4df20", | ||
| "amount": "1000000000000000000" | ||
| } | ||
| ], | ||
| "expireTime": 1750182870, | ||
| "contractSequenceId": 1, | ||
| "operationHash": "0x92d3a28bd75dfa559c60e679b98fddfcb7dcaeb579c25cab3f9442b25fd270e2", | ||
| "signature": "0x62c594b62ce2fc9f1d2e82a105668ed53528eb02635b8ad73206fe75ed26b26923450ed54b87980296c362fe03c7bc8e156d1ab38bfe9a682ba585e7d92d88e31b", | ||
| "backupKeyNonce": 0 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "txHex": "f901c380808094223fe2adcc8f28d8a46f72f7f355117d2727554d80b9016439125215000000000000000000000000e7d07af8e3e7472ea8391a3372ab98d04ac4df200000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000006851b0bf000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000041917bcebd0b1f43a25b72b161dbd4db539c282af9f3856fc60f701471f0df22e22b5428593f5affc1a88944a5c5255c6c2d0df87a0668864d551a5409ec9f82ca1c000000000000000000000000000000000000000000000000000000000000001ca0c1e0750ac2c3c1cc98997dd1a14f96f1ba65929503bbdbd96ffa00fdeea2e51fa05cca504d869dcf2935c46dc5a733c0c3d8483ce2b452cae2bf89b7417d7f80b9" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| { | ||
| "tx": "f9012b808504a817c8008307a12094223fe2adcc8f28d8a46f72f7f355117d2727554d80b9010439125215000000000000000000000000e7d07af8e3e7472ea8391a3372ab98d04ac4df200000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000006851a693000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000808080", | ||
| "userKey": "xpub661MyMwAqRbcFigezGWEYSbCPVuaUmvnp1u7iEpH9YsKU6uYQtPANvudjgAo82QRHXsUieMqKeB1xEj89VUKU1ugtmyAZ3xzNEbHPexxgKK", | ||
| "backupKey": "xpub661MyMwAqRbcGbCirzmQsUJT2eidt9tFLw2m77w6FiKco6TKu49CP3GkHF88xGCpvqkP93SYMAarfyWAn8UWevQtNT6pDo8xH7xmf6GqK6e", | ||
| "coin": "hteth", | ||
| "gasPrice": "20000000000", | ||
| "gasLimit": "500000", | ||
| "recipients": [ | ||
| { | ||
| "address": "0xe7d07af8e3e7472ea8391a3372ab98d04ac4df20", | ||
| "amount": "1000000000000000000" | ||
| } | ||
| ], | ||
| "walletContractAddress": "0x223fe2adcc8f28d8a46f72f7f355117d2727554d", | ||
| "amount": "1000000000000000000", | ||
| "backupKeyNonce": 0, | ||
| "recipient": { | ||
| "address": "0xe7d07af8e3e7472ea8391a3372ab98d04ac4df20", | ||
| "amount": "1000000000000000000" | ||
| }, | ||
| "expireTime": 1750181523, | ||
| "contractSequenceId": 1, | ||
| "nextContractSequenceId": 1 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { SignFinalOptions } from '@bitgo/abstract-eth'; | ||
| import { MethodNotImplementedError } from 'bitgo'; | ||
| import { EnclavedApiSpecRouteRequest } from '../../enclavedBitgoExpress/routers/enclavedApiSpec'; | ||
| import logger from '../../logger'; | ||
| import { isEthLikeCoin } from '../../shared/coinUtils'; | ||
| import { retrieveKmsKey } from './utils'; | ||
|
|
||
| export async function recoveryMultisigTransaction( | ||
| req: EnclavedApiSpecRouteRequest<'v1.multisig.recovery', 'post'>, | ||
| ): Promise<any> { | ||
| const { userPub, backupPub, unsignedSweepPrebuildTx, walletContractAddress } = req.body; | ||
|
|
||
| //fetch prv and check that pub are valid | ||
| const userPrv = await retrieveKmsKey({ pub: userPub, source: 'user', cfg: req.config }); | ||
| const backupPrv = await retrieveKmsKey({ pub: backupPub, source: 'backup', cfg: req.config }); | ||
|
|
||
| if (!userPrv || !backupPrv) { | ||
| const errorMsg = `Error while recovery wallet, missing prv keys for user or backup on pub keys user=${userPub}, backup=${backupPub}`; | ||
| logger.error(errorMsg); | ||
| throw new Error(errorMsg); | ||
| } | ||
|
|
||
| const bitgo = req.bitgo; | ||
| const coin = bitgo.coin(req.decoded.coin); | ||
|
|
||
| // The signed transaction format depends on the coin type so we do this check as a guard | ||
| // If you check the type of coin before and after the "if", you may see "BaseCoin" vs "AbstractEthLikeCoin" | ||
| if (coin.isEVM()) { | ||
| // Every recovery method on every coin family varies one from another so we need to ensure with a guard. | ||
| if (isEthLikeCoin(coin)) { | ||
| try { | ||
| const halfSignedTx = await coin.signTransaction({ | ||
| isLastSignature: false, | ||
| prv: userPrv, | ||
| txPrebuild: { ...unsignedSweepPrebuildTx } as unknown as SignFinalOptions, | ||
| walletContractAddress, | ||
| }); | ||
|
|
||
| const { halfSigned } = halfSignedTx 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 do we need to typecast to
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. You need the typecast to any because something on the SDK types isn't finished unfortunately and halfSigned is not recognized as a prop of halfSignedTx (the output of coin.signTransaction) despite that it's there. In @mohammadalfaiyazbitgo initial example code, I found the same any, so I decided to try to remove it and found this ungrateful surprise : (.
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. I think that you also got confused with what we're casting @pranavjain97 , the cast is not on halfSigned, but in halfSignedTx, for halfSigned as you say we know that it have those properties you mentioned and there is no cast. |
||
| const fullSignedTx = await coin.signTransaction({ | ||
| isLastSignature: true, | ||
| prv: backupPrv, | ||
| txPrebuild: { | ||
| ...halfSignedTx, | ||
| txHex: halfSigned.signatures, | ||
| halfSigned, | ||
| recipients: halfSigned.recipients ?? [], | ||
| } as unknown as SignFinalOptions, | ||
| walletContractAddress, | ||
| signingKeyNonce: halfSigned.signingKeyNonce ?? 0, | ||
| backupKeyNonce: halfSigned.backupKeyNonce ?? 0, | ||
| recipients: halfSigned.recipients ?? [], | ||
| }); | ||
|
|
||
| return fullSignedTx; | ||
| } catch (error) { | ||
| logger.error('error while recovering wallet transaction:', error); | ||
| throw error; | ||
| } | ||
| } else { | ||
| const errorMsg = 'Unsupported coin type for recovery: ' + req.decoded.coin; | ||
| logger.error(errorMsg); | ||
| throw new Error(errorMsg); | ||
| } | ||
| } else { | ||
| throw new MethodNotImplementedError('Unsupported coin type for recovery: ' + coin); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // TODO: this function is duplicated in multisigTransactioSign.ts but as hardcoded. Replace that code later with this call (to avoid merge conflicts/duplication) | ||
| import { KmsClient } from '../../kms/kmsClient'; | ||
| import { EnclavedConfig } from '../../types'; | ||
| export async function retrieveKmsKey({ | ||
|
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. I'm sure this already exists. See
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. Hi! it's not the same code that's why I left a comment as an explanation to avoid confusions but i must admit that it's kinda short and in the rush we're a proper explanation could be better so let me proceed with that please. Instead of throwing the piece of code that does this directly in the main flow of the code, I moved it to a function to avoid clutter and let the one reading the function behavior focus on the important parts instead of the error responses. The comment is also a TODO for latter, the code on signMultisigTransaction.ts was worked by Alex at that time and since I didn't know how much could someone change it, I didn't wanna to start randomly moving things on code non related to my PR to avoid conflicts. Hope that clarifies a little! Thanks. |
||
| pub, | ||
| source, | ||
| cfg, | ||
| }: { | ||
| pub: string; | ||
| source: string; | ||
| cfg: EnclavedConfig; | ||
| }): Promise<string> { | ||
| const kms = new KmsClient(cfg); | ||
| // Retrieve the private key from KMS | ||
| let prv: string; | ||
| try { | ||
| const res = await kms.getKey({ pub, source }); | ||
| prv = res.prv; | ||
| return prv; | ||
| } catch (error: any) { | ||
| throw { | ||
| status: error.status || 500, | ||
| message: error.message || 'Failed to retrieve key from KMS', | ||
| }; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,24 @@ | ||
| import * as t from 'io-ts'; | ||
| import { | ||
| apiSpec, | ||
| httpRoute, | ||
| Method as HttpMethod, | ||
| httpRequest, | ||
| HttpResponse, | ||
| Method as HttpMethod, | ||
| httpRoute, | ||
| } from '@api-ts/io-ts-http'; | ||
| import { Response } from '@api-ts/response'; | ||
| import { | ||
| createRouter, | ||
| type WrappedRouter, | ||
| TypedRequestHandler, | ||
| type WrappedRouter, | ||
| } from '@api-ts/typed-express-router'; | ||
| import { Response } from '@api-ts/response'; | ||
| import express from 'express'; | ||
| import { BitGoRequest } from '../../types/request'; | ||
| import { EnclavedConfig } from '../../types'; | ||
| import * as t from 'io-ts'; | ||
| import { postIndependentKey } from '../../api/enclaved/postIndependentKey'; | ||
| import { recoveryMultisigTransaction } from '../../api/enclaved/recoveryMultisigTransaction'; | ||
| import { signMultisigTransaction } from '../../api/enclaved/signMultisigTransaction'; | ||
| import { prepareBitGo, responseHandler } from '../../shared/middleware'; | ||
| import { EnclavedConfig } from '../../types'; | ||
| import { BitGoRequest } from '../../types/request'; | ||
|
|
||
| // Request type for /key/independent endpoint | ||
| const IndependentKeyRequest = { | ||
|
|
@@ -39,7 +40,7 @@ const IndependentKeyResponse: HttpResponse = { | |
| const SignMultisigRequest = { | ||
| source: t.string, | ||
| pub: t.string, | ||
| txPrebuild: t.any, // TransactionPrebuild type from BitGo | ||
| txPrebuild: t.any, | ||
| }; | ||
|
|
||
| // Response type for /multisig/sign endpoint | ||
|
|
@@ -52,6 +53,32 @@ const SignMultisigResponse: HttpResponse = { | |
| }), | ||
| }; | ||
|
|
||
| // Request type for /multisig/recovery endpoint | ||
| const RecoveryMultisigRequest = { | ||
| userPub: t.string, | ||
| backupPub: t.string, | ||
| apiKey: t.string, | ||
| // TODO: best typing for this, they come from sdk TS types | ||
| unsignedSweepPrebuildTx: t.any, | ||
| coinSpecificParams: t.union([ | ||
| t.undefined, | ||
| t.partial({ | ||
| bitgoPub: t.union([t.undefined, t.string]), | ||
| ignoreAddressTypes: t.union([t.undefined, t.array(t.string)]), | ||
| }), | ||
| ]), | ||
| }; | ||
|
|
||
| // Response type for /multisig/recovery endpoint | ||
| const RecoveryMultisigResponse: HttpResponse = { | ||
| // TODO: Define proper response type for recovery multisig transaction | ||
| 200: t.any, // the full signed tx | ||
|
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. Please type this out |
||
| 500: t.type({ | ||
| error: t.string, | ||
| details: t.string, | ||
| }), | ||
| }; | ||
|
|
||
| // API Specification | ||
| export const EnclavedAPiSpec = apiSpec({ | ||
| 'v1.multisig.sign': { | ||
|
|
@@ -68,6 +95,20 @@ export const EnclavedAPiSpec = apiSpec({ | |
| description: 'Sign a multisig transaction', | ||
| }), | ||
| }, | ||
| 'v1.multisig.recovery': { | ||
| post: httpRoute({ | ||
| method: 'POST', | ||
| path: '/{coin}/multisig/recovery', | ||
| request: httpRequest({ | ||
| params: { | ||
| coin: t.string, | ||
| }, | ||
| body: RecoveryMultisigRequest, | ||
| }), | ||
| response: RecoveryMultisigResponse, | ||
| description: 'Recover a multisig transaction', | ||
| }), | ||
| }, | ||
| 'v1.key.independent': { | ||
| post: httpRoute({ | ||
| method: 'POST', | ||
|
|
@@ -121,5 +162,13 @@ export function createKeyGenRouter(config: EnclavedConfig): WrappedRouter<typeof | |
| }), | ||
| ]); | ||
|
|
||
| router.post('v1.multisig.recovery', [ | ||
| responseHandler<EnclavedConfig>(async (req) => { | ||
| const typedReq = req as EnclavedApiSpecRouteRequest<'v1.multisig.recovery', 'post'>; | ||
| const result = await recoveryMultisigTransaction(typedReq); | ||
| return Response.ok(result); | ||
| }), | ||
| ]); | ||
|
|
||
| return router; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { MethodNotImplementedError } from 'bitgo'; | ||
| import { isEthLikeCoin } from '../shared/coinUtils'; | ||
| import { MasterApiSpecRouteRequest } from './routers/masterApiSpec'; | ||
|
|
||
| export async function handleRecoveryWalletOnPrem( | ||
| req: MasterApiSpecRouteRequest<'v1.wallet.recovery', 'post'>, | ||
| ) { | ||
| const bitgo = req.bitgo; | ||
| const coin = req.decoded.coin; | ||
| const enclavedExpressClient = req.enclavedExpressClient; | ||
|
|
||
| const { | ||
| userPub, | ||
| backupPub, | ||
| walletContractAddress, | ||
| recoveryDestinationAddress, | ||
| coinSpecificParams, | ||
| apiKey, | ||
| } = req.decoded; | ||
|
|
||
| //construct a common payload for the recovery that it's repeated in any kind of recovery | ||
| const commonRecoveryParams = { | ||
| userKey: userPub, | ||
| backupKey: backupPub, | ||
| walletContractAddress, | ||
| recoveryDestination: recoveryDestinationAddress, | ||
| apiKey, | ||
| }; | ||
|
|
||
| const sdkCoin = bitgo.coin(coin); | ||
|
|
||
| if (isEthLikeCoin(sdkCoin)) { | ||
| try { | ||
| const unsignedSweepPrebuildTx = await sdkCoin.recover({ | ||
| ...commonRecoveryParams, | ||
| }); | ||
| const fullSignedRecoveryTx = await enclavedExpressClient.recoveryMultisig({ | ||
| userPub, | ||
| backupPub, | ||
| apiKey, | ||
| unsignedSweepPrebuildTx, | ||
| coinSpecificParams, | ||
| walletContractAddress, | ||
| }); | ||
|
|
||
| return fullSignedRecoveryTx; | ||
| } catch (err) { | ||
| throw err; | ||
|
mohammadalfaiyazbitgo marked this conversation as resolved.
|
||
| } | ||
| } else { | ||
| throw new MethodNotImplementedError('Recovery wallet is not supported for this coin: ' + coin); | ||
| } | ||
| } | ||


Uh oh!
There was an error while loading. Please reload this page.