feat(money-account-utils): add money account transaction batch builders - #9680
Open
Jwhiles wants to merge 4 commits into
Open
feat(money-account-utils): add money account transaction batch builders#9680Jwhiles wants to merge 4 commits into
Jwhiles wants to merge 4 commits into
Conversation
Jwhiles
force-pushed
the
feat/money-account-utils-package
branch
3 times, most recently
from
July 30, 2026 12:51
bc4b92b to
d90583a
Compare
Ports the Money Account deposit and withdrawal batch builders from metamask-mobile (`app/components/UI/Money/utils/moneyAccountTransactions.ts`) so both clients encode the vault calls identically. The client-resident wrappers around these builders stay in mobile: they read vault config, provider and recipient from Redux/Engine singletons, which do not belong in a shared package. Everything below that — ABIs, calldata encoding, slippage and share arithmetic, and the two `previewDeposit`/`getRate` reads — moves here, keeping the existing signatures so call sites only change their import path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The description predated the rescope: activity parsing and classification were never extracted, and the vault transaction builders now are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Address review feedback on the ported vault builders: - Replace the `initialiseWithoutData` flag with a dedicated `buildMoneyAccountDepositPlaceholderBatch`. The flag path still awaited `previewDeposit` and discarded the result, and required five vault addresses it never used; the new builder is synchronous, takes only `chainId` and `tellerAddress`, and performs no vault reads. - Make `MoneyAccountTxParams.params.data` required, with the no-calldata case as `MoneyAccountPlaceholderTxParams`, so callers of the encoding builders no longer have to narrow an optional field. - Return `CaipAssetType | undefined` from `getMoneyAccountDepositAssetId` rather than silently defaulting to Monad, so an unsupported chain stays distinguishable. Clients apply their own default at the call site. - Type `MUSD_TOKEN_ASSET_ID_BY_CHAIN` values as `CaipAssetType` and the deposit options' addresses as `Hex`, removing the internal casts. - Fix withdraw comments describing the redeemed asset as USDC; the builders encode mUSD throughout. Also reconcile the changelog with the released 1.0.0 of the package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
ffmcgee725
reviewed
Jul 31, 2026
Comment on lines
+458
to
+480
| const shareAmount = | ||
| amount === 0n | ||
| ? 0n | ||
| : getSharesForWithdrawal( | ||
| amount, | ||
| await getVaultRate({ accountantAddress, provider }), | ||
| ); | ||
| // Allow 1-unit slippage on minimumAssets as defense-in-depth against | ||
| // rounding: the contract's mulDivDown can truncate assetsOut by up to | ||
| // 1 unit relative to the requested amount. This tolerance is safe | ||
| // because ceiling division in getSharesForWithdrawal already guarantees | ||
| // assetsOut >= amount; the 1-unit slack here is a second line of | ||
| // defense, not a standalone fix. The subsequent ERC-20 transfer uses | ||
| // the original `amount`, so the tolerance does not affect how much the | ||
| // user receives — it only prevents a spurious revert from the teller's | ||
| // MinimumAssetsNotMet check. | ||
| const minimumAssets = amount > 0n ? amount - 1n : 0n; | ||
| const withdrawData = buildWithdrawData( | ||
| musdAddress, | ||
| shareAmount, | ||
| minimumAssets, | ||
| moneyAccountAddress, | ||
| ); |
Member
There was a problem hiding this comment.
Zero-amount withdraw placeholders encode shareAmount = 0, which Veda Teller rejects with ZeroShares.
Unlike deposit placeholders (which use a dedicated builder that omits calldata), withdraw placeholders may fail during simulation or submission before re-encoding.
Would it be worth adding a buildMoneyAccountWithdrawPlaceholderBatch, mirroring the deposit placeholder builder, to solve this ?
Returning something like
withdrawTx: teller target/type, no data,
transferTx: mUSD target/type, no data,
No amount, provider, accountant, recipient, or money-account address required
Then make buildMoneyAccountWithdrawBatch reject amount === 0n, so it cannot produce known-reverting calldata.
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
2 participants
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.
Explanation
Stacked on #9397.
Ports the Money Account deposit and withdrawal transaction batch builders from metamask-mobile (
app/components/UI/Money/utils/moneyAccountTransactions.ts) into this package, so mobile and extension encode the vault calls identically rather than each keeping its own copy.What moved: the ABIs, the calldata encoders, the slippage and share arithmetic, the mUSD deposit-asset resolution, and the two contract reads (
previewDepositon the lens,getRateon the accountant).What deliberately did not move: the four client wrappers (
updateMoneyAccountDepositTokenAmount,updateMoneyAccountWithdrawTokenAmount,getMoneyAccountDepositTransactionsData,getMoneyAccountWithdrawTransactionsData). Those read vault config, provider and recipient from mobile's Redux/Engine singletons and return a Confirmations-owned type, so they stay in the client and delegate to these builders.Signatures are unchanged from mobile, including the injected
provider, so client call sites only change their import path.@ethersproject/{abi,abstract-provider,contracts}v5 is used directly rather than theethersumbrella, matching the convention already established bymoney-account-balance-service(which reads the same lens and accountant contracts).References
N/A
Checklist
🤖 Generated with Claude Code
Note
High Risk
Encodes on-chain vault deposit/withdraw flows that move mUSD; incorrect share/slippage math or calldata could cause reverts or wrong batch composition, though behavior is ported from mobile with targeted rounding fixes and broad unit tests.
Overview
Ports Money Account vault deposit and withdraw batch encoding from MetaMask Mobile into
@metamask/money-account-utils, so clients share one implementation for approve/deposit and withdraw/transfer calldata.Deposits:
buildMoneyAccountDepositBatchreturns keyedapproveTx+depositTx, readspreviewDepositon the lens (skipped for zero amount), and setsminimumMintvia 0.2% slippage (applySlippage).buildMoneyAccountDepositPlaceholderBatchresolves targets andTransactionTypes only—no provider or calldata—for Pay placeholder batches.Withdrawals:
buildMoneyAccountWithdrawBatchreturnswithdrawTx+transferTx, fetches the accountant rate (skipped at zero amount), converts assets to shares withgetSharesForWithdrawal(ceiling division) to avoidMinimumAssetsNotMetreverts, and setsminimumAssetstoamount - 1as rounding defense; the user-facing payout still uses the fullamounton the ERC-20 transfer.Also adds
getMoneyAccountDepositAssetId, exportsTELLER_ABIand related types, adds@ethersproject/*v5 dependencies, and typesMUSD_TOKEN_ASSET_ID_BY_CHAINasCaipAssetType.Reviewed by Cursor Bugbot for commit a42e8d0. Bugbot is set up for automated code reviews on this repo. Configure here.