feat(sdk-core): pre-flight balance check for depositToVault#9293
Open
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
Open
feat(sdk-core): pre-flight balance check for depositToVault#9293bitgo-ai-agent-dev[bot] wants to merge 1 commit into
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
Conversation
Add InsufficientBalanceError and assertSufficientAssetBalance to the DefiVault class. Before creating any txRequest, depositToVault now fetches a fresh wallet snapshot (allTokens=true) and throws InsufficientBalanceError if spendableBalance < amount. For native-coin vaults (Concrete BTC: assetToken === coin), the top- level spendableBalanceString is used. For token vaults (Morpho USDC/ USDT/ETH: assetToken !== coin), the tokens array is searched by tokenName. Balance comparison uses BigNumber to avoid float precision issues. Exports InsufficientBalanceError from the defi module index. Ticket: DEFI-371 Session-Id: 39a66f44-5586-4260-9a85-c6fd946c9064 Task-Id: 48766e58-a805-49ca-b015-534a996101ac
Contributor
Author
|
@claude please review this PR |
bitgo-ai-agent-dev
Bot
force-pushed
the
DEFI-371-pre-flight-balance-check-deposit
branch
from
July 20, 2026 12:15
8c61b6e to
8454f57
Compare
davidkaplanbitgo
requested changes
Jul 20, 2026
davidkaplanbitgo
left a comment
davidkaplanbitgo
left a comment
Contributor
There was a problem hiding this comment.
Change the code owners so that this does not require a BTC team review
| * propagates network errors as-is. | ||
| */ | ||
| private async assertSufficientAssetBalance(config: GetVaultResponse, amount: string): Promise<void> { | ||
| const walletData: Record<string, any> = await this.bitgo.get(this.wallet.url()).query({ allTokens: true }).result(); |
Contributor
There was a problem hiding this comment.
is there no better typing for the value we can infer here, as opposed to any?
| * Error thrown when the wallet does not hold sufficient balance of the underlying asset | ||
| * to cover the requested deposit amount. | ||
| */ | ||
| export class InsufficientBalanceError extends Error { |
Contributor
There was a problem hiding this comment.
nit - consider moving this to an error.ts file
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
3 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.
Summary
InsufficientBalanceError(structured error withassetToken,available,requestedfields) todefiVault.tsand exports it from the defi module index.assertSufficientAssetBalanceprivate method that fetches a fresh wallet snapshot (?allTokens=true) and compares the spendable balance of the vault'sassetTokenagainstparams.amountusingBigNumber.depositToVaultcalls this check after fetching vault config, before dispatching todepositToConcreteVaultordepositToMorphoVault.Decision log
1. Where to place the balance check
defiVault.ts:102—depositToVaultis the single entrypoint for all deposit paths.depositToVaultbefore dispatch — covers both Concrete and Morpho uniformly; (B) Duplicated in each private path; (C) Caller's responsibility.2. Balance source: stale
_walletvs. fresh API call_wallet.spendableBalanceStringmay be stale (wallet constructed before deposit call).GET /wallet/{id}?allTokens=true; (B) Use cached_walletdata.?allTokens=trueensures ERC-20 token balances are included.3. Token lookup:
assetToken === walletCoinvs. tokens arrayassetTokenis'btc'which matches vaultcoin'btc'— use top-levelspendableBalanceString.assetTokenis e.g.'usdc', vaultcoinis'eth'— search thetokensarray bytokenName.'0'→ throwsInsufficientBalanceError.4. BigNumber for comparison
BigNumber(already a dep ofsdk-core) to avoid float precision issues on base-unit integer strings. Never floats for monetary amounts.5. Test structure
depositToVaultnow makes two GET requests: vault config + wallet balance.onFirstCall()/onSecondCall()onmockBitGo.get.mockBaseCoin.urlstub to return a real URL sowallet.url()works.Test evidence
New tests added (5 total):
concrete_btccx: throwsInsufficientBalanceErrorwhen native coin balance is too lowconcrete_btccx: does not throw when balance exactly equals requested amountmorpho: throwsInsufficientBalanceErrorwhen token balance is too lowmorpho: throwsInsufficientBalanceErrorwhen token is absent from wallet (balance treated as 0)morpho: does not throw when balance exactly equals requested amountLint: ✅ TypeCheck: ✅ Full suite: ✅ (478 passing)
Open questions
None.
Out of scope
resumeDeposit— no balance check needed; the approve txRequest is already created at this point.withdrawFromVaultbalance check (receipt/share token) is tracked in sibling ticket DEFI-372.🤖 Generated with Ralph