feat(sdk-core): add wrap() and unwrap() to DefiVault - #9569
Draft
kamleshmugdiya wants to merge 1 commit into
Draft
feat(sdk-core): add wrap() and unwrap() to DefiVault#9569kamleshmugdiya wants to merge 1 commit into
kamleshmugdiya wants to merge 1 commit into
Conversation
Expose ETH -> WETH wrap and WETH -> ETH unwrap on wallet.defi so the UI can call them directly. Both are thin orchestrators over a single wallet.sendMany, modelled on the existing withdrawFromVault; WP builds the WETH9 calldata and resolves the contract address from the vault binding. - iDefiVault: WrapOptions / WrapResult, plus wrap/unwrap on IDefiVault. operationId is declared optional now even though nothing populates it, so that adding operation tracking in M5 is not a breaking change to a published SDK type. - defiVault: wrap/unwrap share sendWrapIntent, which deliberately does not call extractOperationId - no operation is minted in v1. - wallet: wrapNative / unwrapNative cases map to the wrap-native / unwrap-native intents, decoding defiParams with decodeWithCodec rather than a bare cast so an 18dp amount stays a string. - mpcUtils: both sites - the recipients-required exemption list and the EVM intent-shape switch. Omitting the first makes the recipients assertion throw before the switch is reached. - recipientUtils: register both the camelCase and kebab-case spellings. NO_RECIPIENT_TX_TYPES is matched against txParams.type (camelCase, from buildParams) and against intent.intentType (kebab-case, as WP persists it); signing paths that carry no txParams only ever see the latter. @bitgo/public-types is intentionally left at 6.58.0: sdk-core imports no intent codec from it, so the 6.60.0 publish carrying WrapNativeIntent / UnwrapNativeIntent is only needed by wallet-platform. DEFI-661
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements DEFI-661 —
wallet.defi.wrap()andwallet.defi.unwrap().Both are thin orchestrators over a single
wallet.sendMany, modelled on the existingwithdrawFromVault. WP builds the WETH9 calldata and resolves the contract address from the vault binding; the SDK only forwardsvaultIdandamount.Changes
defi/iDefiVault.tsWrapOptions/WrapResult;wrap/unwraponIDefiVaultdefi/defiVault.tswrap/unwrapsharing a privatesendWrapIntentwallet/wallet.tswrapNative/unwrapNative→wrap-native/unwrap-nativeintentsutils/mpcUtils.tsutils/tss/recipientUtils.tsNO_RECIPIENT_TX_TYPES, in both spellingsexamples/ts/defi-vault-wrap.tsTwo deliberate choices worth flagging in review:
WrapResult.operationIdis declared optional even though nothing populates it. No operation is minted for wrap/unwrap in v1, andsendWrapIntentdeliberately does not callextractOperationId— it would only ever returnundefined. Declaring the field now keeps M5's operation tracking from being a breaking change to a published SDK type.vaultIdis required. Binding the wrap to a vault is what supplies the per-enterprise authorization gate and the address-whitelist path server-side (TDD §3.6). M7 relaxes it to optional, which is backward-compatible.defiParamsis decoded withdecodeWithCodec(thedefiWithdrawpattern) rather than the bareascastsdefiApprove/defiDeposituse, so an 18dp amount is validated as a numeric string and stays a string end-to-end.Both spellings in
NO_RECIPIENT_TX_TYPESNO_RECIPIENT_TX_TYPESis matched against two different sources:txParams.type— which isbuildParams.type, the camelCase value passed towallet.sendManytxRequest.intent.intentType— kebab-case, as WP persists itSigning paths that carry no
txParams—pendingApproval.approve()→recreateTxRequest()→signTxRequest()— only ever see the kebab-case spelling. Registering only one spelling leaves the other path throwingInvalidTransactionErrorbefore signing, so all four strings are registered and there is a regression test per path.Pre-existing bug found, filed separately
That asymmetry is already live for the three existing DeFi types:
defi-approve/defi-deposit/defi-withdraware registered in camelCase only, so approving a pending approval for any of them throws today. Filed as DEFI-688 rather than folded in here — the fix also requires changing an existing assertion (recipientUtils.ts:67asserts'defi-deposit'must not be in the set) that deserves its own review. The new wrap/unwrap types are not affected.public-types is intentionally not bumped
DEFI-661 asks to bump
@bitgo/public-typesto the version from DEFI-657 (6.60.0). Not done here, on purpose:GetVaultResponse/VaultProtocol/ MPC types. Intents are plain objects built bympcUtils.populateIntent, soWrapNativeIntent/UnwrapNativeIntentare needed by wallet-platform, not by the SDK.867580cf71).build:commit.wallet-platform still needs
@bitgo/public-types@6.60.0for DEFI-659.Security note
Registering
wrapNativeinNO_RECIPIENT_TX_TYPESsuppresses the SDK's missing-recipients guard, andverifyTssTransaction's client-side amount/destination comparison only runs whentxParams.type === 'transfer'(abstractEthLikeNewCoins.ts:3218), so it never fires for DeFi types. After this change WP's server-side assertion is the only control on how much ETH a wrap moves. That is the accepted design (TDD §5.2); nothing here weakens it further.Testing
modules/sdk-core— 642 passing, 1 pending (pre-existing). Adds 22 wrap/unwrap cases: sendMany shape (type+defiParamsasserted exactly),txRequestIdfrom bothfullandliteresponse shapes,operationIdstaysundefined, passphrase forwarded/omitted, missingvaultId/amountthrowing with no network call, anddefiParamscodec validation.modules/sdk-core/test/.../recipientUtils.ts— regression tests for camelCase, kebab-case, andtxParams: undefined.modules/bitgo/test/v2/unit/wallet.ts—populateIntentfor both intents, proving the recipients assertion does not throw and the intent carries a plainamountrather thanshareTokenAmount. Full file passes.tsc --noEmitclean across sdk-core (src + test); eslint and prettier clean.End-to-end only works once DEFI-659 is deployed; this is unit-tested against mocks.
Out of scope
No
autoWrap/ chaining intodepositToVault(the UI drives that, M4), no confirmation polling, no operation tracking (M5).TICKET: DEFI-661
🤖 Generated with Claude Code