feat: add ETH support for multisign in external mode#208
Conversation
| "zod": "^3.25.48" | ||
| }, | ||
| "overrides": { | ||
| "@bitgo-beta/sdk-core": "8.2.1-beta.1775", |
There was a problem hiding this comment.
add to add these due to conflicts with the beta release of BitGoJs - once stable versions are out, maybe we won't need to resolve it like so
| function isEthTransactionPrebuild(txPrebuild: TransactionPrebuild): txPrebuild is EthTxPrebuild { | ||
| return ( | ||
| Array.isArray((txPrebuild as any).recipients) && | ||
| (txPrebuild as any).nextContractSequenceId != null |
There was a problem hiding this comment.
very nitty, don't love the to any casting. in fact, there's probably a similar method that exists within BGMS where we do a similar check for if something is an ethPrebuildTxn, maybe do a quick claude scan?
There was a problem hiding this comment.
No similar type guard found in bitgo-microservices. The reviewer was hoping there was a shared utility, but there isn't one.
There was a problem hiding this comment.
I'll think of an alternative to skip the any that irked you 😄
| if (!txPrebuild.txHex) { | ||
| throw new Error(`txPrebuild must include txHex for external signing`); | ||
| if (isEthLikeCoin(coin)) { | ||
| if (!isEthTransactionPrebuild(txPrebuild)) { |
There was a problem hiding this comment.
I'd argue that we should have this prebuild method actually check for those fields? add more details to those
There was a problem hiding this comment.
this should be handled by isEthTransactionPrebuild - did you mean smth else?
| '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], | ||
| '@typescript-eslint/no-unused-vars': [ | ||
| 'error', | ||
| { |
There was a problem hiding this comment.
not have the compiler complain when variables are named with _
This commit adds support for signing multisig transactions for ETH in external mode. It includes validation to ensure that the transaction prebuild contains the necessary fields for ETH transactions, such as recipients and nextContractSequenceId. The tests have been updated to reflect these changes and to ensure that the new functionality works as expected - tested e2e. Re Recovery Flow: it requires a few methods that are not exposed by COINS team, so that will be a follow-up work. Ticket: WCN-544
|
@margueriteblair -- updated! |
| */ | ||
| interface EthExternalSigningResponse extends Omit<HalfSignedTransaction, 'halfSigned'> { | ||
| halfSigned: HalfSignedTransaction['halfSigned'] & { | ||
| operationHash: string; |
There was a problem hiding this comment.
without these, we get errors like:
"error": "BitGoApiResponseError",
"details": {
"error": "missing signature",
"name": "Invalid",
"requestId": "d3c7b37cf8435227083b-0002",
"context": {}
}
}
so extended SDK type
2 participants
What
This PR adds support for signing multisig transactions for ETH in external mode. It includes validation to ensure that the transaction prebuild contains the necessary fields for ETH transactions,
such as recipients and nextContractSequenceId.
The tests have been updated to reflect these changes and to ensure that
the new functionality works as expected - tested e2e.
Re Recovery Flow: it requires a few methods that are not exposed by COINS team, so that will be a follow-up work.
Ticket: WCN-544
Testing