feat: Add support for EVM keyring wallets in generateWallet API#191
feat: Add support for EVM keyring wallets in generateWallet API#191pranavjain97 merged 1 commit intomasterfrom
Conversation
| if (evmKeyRingReferenceWalletId) { | ||
| return handleGenerateEvmKeyRingWallet(req); | ||
| } |
There was a problem hiding this comment.
this should be run after coin validation. if someone passes it with a non-EVM coin, the handler calls baseCoin.wallets().generateWallet() with an unsupported param and relies on the downstream API to reject it. should validate the coin is EVM-compatible and return 400 early imo
There was a problem hiding this comment.
Good point, added an early validation!
There was a problem hiding this comment.
you could directly do smthn like
if(baseCoin.isEVM() && evmKeyRingReferenceWalletId) {
result = await baseCoin.wallets().generateWallet(req.decoded);
}
There was a problem hiding this comment.
you could directly do smthn like
if(baseCoin.isEVM() && evmKeyRingReferenceWalletId) {
result = await baseCoin.wallets().generateWallet(req.decoded);
}
But this way, if they pass a non-EVM compatible coin, we won't error with a 400; we'd need to maintain another layer for if valid, if !valid logic. So, i feel like the helper function is useful here
| const { label, enterprise, evmKeyRingReferenceWalletId } = req.decoded; | ||
|
|
||
| const result = await baseCoin.wallets().generateWallet({ | ||
| label, | ||
| enterprise, | ||
| evmKeyRingReferenceWalletId, | ||
| }); |
There was a problem hiding this comment.
why are we only selecting specific params? should send multisigType etc along and maybe there are other params it uses
There was a problem hiding this comment.
Ah, right. The version of @bitgo-beta/sdk-core we're using has a mismatch against this package's schema.
The current version being used defines disableTransactionNotifications as a string, but here we have it as boolean.
If we get this bump PR in - #192 first, that should correct the typing in @bitgo-beta/sdk-core as well
pranishnepal
force-pushed
the
WAL-479
branch
5 times, most recently
from
03b6277 to
e340141
Compare
This commit introduces support for generating EVM keyring wallets in the `generateWallet` API. Ticket: WAL-479
| /** | ||
| * This function generates an EVM keyring wallet by reusing keys from a reference wallet. | ||
| */ | ||
| async function handleGenerateEvmKeyRingWallet( | ||
| req: MasterApiSpecRouteRequest<'v1.wallet.generate', 'post'>, | ||
| ) { |
There was a problem hiding this comment.
this just wraps the generateWallet method and is repetetive of the main handler, we can remove this
| if (evmKeyRingReferenceWalletId) { | ||
| return handleGenerateEvmKeyRingWallet(req); | ||
| } |
There was a problem hiding this comment.
you could directly do smthn like
if(baseCoin.isEVM() && evmKeyRingReferenceWalletId) {
result = await baseCoin.wallets().generateWallet(req.decoded);
}
2 participants
What
This PR introduces support for generating EVM keyring wallets in the
generateWalletAPI.Ticket: WAL-479
Testing