-
Notifications
You must be signed in to change notification settings - Fork 66
fix: check eth estimate in offramp #758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds BTC and ETH preflight balance checks and gas budgeting to GatewayApiClient.executeQuote: verifies onramp BTC via Esplora, checks ETH balance and estimates gas/fees for ERC20 approve and createOrder on the offramp path, computes total gas cost, conditionally submits approve, and enforces sufficiency before proceeding. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant GAC as GatewayApiClient
participant Esplora as Esplora
participant RPC as EVM Node
participant ERC20 as ERC20 Token
participant Offramp as Offramp Contract
Note over GAC: Preflight (BTC/ETH balance + gas budgeting)
App->>GAC: executeQuote(quote)
alt Onramp (BTC)
GAC->>Esplora: getBalance(fromUserAddress)
Esplora-->>GAC: btcBalance
alt Insufficient BTC
GAC-->>App: throw Error("insufficient BTC ...")
end
else Offramp (ETH/ERC20)
GAC->>RPC: getBalance(accountAddress)
GAC->>RPC: estimateFeesPerGas()/getGasPrice()
GAC->>ERC20: multicall(allowance, decimals)
GAC->>RPC: estimateContractGas.approve(...)
GAC->>RPC: estimateContractGas.createOrder(...)
GAC->>GAC: compute requiredAmount, needsApproval, totalGasCost
alt Insufficient ETH
GAC-->>App: throw Error("insufficient ETH for gas ...")
else
opt needsApproval
GAC->>ERC20: sendTransaction approve(...)
ERC20-->>GAC: txReceipt
end
GAC->>Offramp: simulate createOrder(...)
Offramp-->>GAC: simulationResult
GAC->>Offramp: sendTransaction createOrder(...)
Offramp-->>GAC: txReceipt
GAC-->>App: result (receipt)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @gregdhill, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a crucial improvement to the offramp transaction process by proactively checking for sufficient ETH balance to cover gas fees. It ensures that users have enough funds before attempting transactions, thereby reducing failed transactions and improving the overall user experience.
Highlights
- Gas Fee Pre-check for Offramp Transactions: Implemented a pre-check mechanism within the offramp transaction flow to estimate and verify sufficient ETH balance for gas fees, preventing transaction failures due to low funds.
- Dynamic Gas Cost Calculation: Introduced logic to dynamically calculate the total estimated gas cost, accounting for both ERC20 approve and offramp createOrder transactions, based on whether an approval is required.
- Insufficient Balance Error Handling: Added an explicit error throw if the user's ETH balance is less than the calculated total gas cost, providing clear feedback on insufficient funds.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a helpful pre-flight check in the offramp process to ensure the user has sufficient ETH for gas fees. It estimates the gas for both the createOrder and a potential approve transaction and throws an error if the user's balance is too low. This is a great improvement for user experience, preventing failed transactions. My review includes a suggestion to improve the accuracy of the gas fee estimation by using modern EIP-1559 methods, and another to optimize the logic to avoid an unnecessary network call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk/src/gateway/client.ts (2)
855-861: Propagate safe account to simulate approveIf
walletClient.accountis not set,simulateContractshould still carry anaccount. Use the same fallback as above.- const { request } = await publicClient.simulateContract({ - account: walletClient.account, + const { request } = await publicClient.simulateContract({ + account: walletClient.account ?? accountAddress, address: tokenAddress, abi: erc20Abi, functionName: 'approve', args: [offrampRegistryAddress, maxUint256], });
868-874: Propagate safe account to simulate createOrderSame reasoning as above; keep the execution path consistent when no default account is configured.
- const { request } = await publicClient.simulateContract({ - account: walletClient.account, + const { request } = await publicClient.simulateContract({ + account: walletClient.account ?? accountAddress, address: offrampRegistryAddress, abi: offrampOrder.offrampABI, functionName: offrampOrder.offrampFunctionName, args: offrampOrder.offrampArgs, });
🧹 Nitpick comments (3)
sdk/src/gateway/client.ts (3)
804-819: Estimate approval gas only if needed; trim RPC calls and reduce failure surfaceYou eagerly estimate gas for
approveeven when allowance is already sufficient. ComputeneedsApprovalfirst (multicall), then:
- always estimate
createOrdergas,- conditionally estimate
approvegas only ifneedsApprovalis true,- sum the costs.
This cuts one RPC on the common “already approved” path and avoids estimating an approve some tokens might reject.
Also applies to: 845-847
848-852: Polish error message: show ETH values instead of raw weiUser-facing error in wei is hard to read. Prefer formatting both values in ETH.
Example change (requires
formatEtherfromviem):- throw new Error( - `Insufficient ETH balance for gas fees. Required: ${totalGasCost} wei, Available: ${ethBalance} wei` - ); + throw new Error( + `Insufficient ETH for gas. Required: ${formatEther(totalGasCost)} ETH, available: ${formatEther(ethBalance)} ETH.` + );Outside this hunk, add:
-import { +import { Account, Address, erc20Abi, isAddress, PublicClient, Transport, Chain as ViemChain, WalletClient, zeroAddress, maxUint256, Hash, Hex, + formatEther, } from 'viem';
854-866: Handle non-standard ERC-20s that forbid “non-zero → non-zero” approvalsSome tokens require resetting allowance to 0 before setting a new value. Consider a guarded fallback to
approve(0)thenapprove(max)when the first approve reverts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/client.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Check: Tests
sdk/src/gateway/client.ts
[failure] 800-800: test/gateway.test.ts > Gateway Tests > should return evm txid for offramp
TypeError: Cannot read properties of undefined (reading 'address')
❯ GatewayApiClient.executeQuote src/gateway/client.ts:800:47
❯ test/gateway.test.ts:742:25
🪛 GitHub Actions: SDK
sdk/src/gateway/client.ts
[error] 800-800: Cannot read properties of undefined (reading 'address') in GatewayApiClient.executeQuote when accessing walletClient.account.address. (src/gateway/client.ts:800)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
f1ca94a to
a9d9b8e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
sdk/src/gateway/client.ts (3)
816-831: Optimize gas estimation to avoid unnecessary RPC callsYou're always estimating gas for the approval transaction even when it might not be needed. Check the allowance first, then only estimate approval gas if required.
Apply this optimization:
-// Estimate gas for both potential transactions -const [approvalGasEstimate, createOrderGasEstimate] = await Promise.all([ - publicClient.estimateContractGas({ - address: tokenAddress, - abi: erc20Abi, - functionName: 'approve', - args: [offrampRegistryAddress, maxUint256], - account: walletClient.account, - }), - publicClient.estimateContractGas({ - address: offrampRegistryAddress, - abi: offrampOrder.offrampABI, - functionName: offrampOrder.offrampFunctionName, - args: offrampOrder.offrampArgs, - account: walletClient.account, - }), -]); +// First check if approval is needed +const [allowance, decimals] = await publicClient.multicall({ + allowFailure: false, + contracts: [ + { + address: tokenAddress, + abi: erc20Abi, + functionName: 'allowance', + args: [accountAddress, offrampRegistryAddress], + }, + { + address: tokenAddress, + abi: erc20Abi, + functionName: 'decimals', + }, + ], +}); + +if (decimals < 8) { + throw new Error('Tokens with less than 8 decimals are not supported'); +} +const multiplier = 10n ** BigInt(decimals - 8); +const requiredAmount = BigInt(quote.amountLockInSat) * multiplier; +const needsApproval = requiredAmount > allowance; + +// Only estimate approval gas if needed +const approvalGasEstimate = needsApproval + ? await publicClient.estimateContractGas({ + address: tokenAddress, + abi: erc20Abi, + functionName: 'approve', + args: [offrampRegistryAddress, maxUint256], + account: walletClient.account, + }) + : 0n; + +const createOrderGasEstimate = await publicClient.estimateContractGas({ + address: offrampRegistryAddress, + abi: offrampOrder.offrampABI, + functionName: offrampOrder.offrampFunctionName, + args: offrampOrder.offrampArgs, + account: walletClient.account, +});Then move the
multicalland decimals check logic before gas estimation, and remove the duplicate code from lines 839-862.
833-837: Consider EIP-1559 priority for better UXGood use of
estimateFeesPerGas()with fallback togetGasPrice()for accurate fee estimation on EIP-1559 compatible chains.
856-861: LGTM! Proper BigInt arithmetic for decimal conversionThe implementation correctly validates decimals >= 8 and uses BigInt-only arithmetic to avoid precision loss.
🧹 Nitpick comments (1)
sdk/src/gateway/client.ts (1)
866-870: Consider more user-friendly units for error messageThe gas cost error message displays values in wei, which can be hard to interpret. Consider showing the amounts in ETH for better user experience.
Apply this enhancement:
if (ethBalance < totalGasCost) { + const formatEth = (wei: bigint) => { + const eth = Number(wei) / 1e18; + return eth.toFixed(6); + }; throw new Error( - `Insufficient ETH balance for gas fees. Required: ${totalGasCost} wei, Available: ${ethBalance} wei` + `Insufficient ETH balance for gas fees. Required: ${formatEth(totalGasCost)} ETH, Available: ${formatEth(ethBalance)} ETH` ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/src/gateway/client.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T11:18:53.195Z
Learnt from: slavastartsev
PR: bob-collective/bob#634
File: sdk/src/gateway/client.ts:136-140
Timestamp: 2025-06-17T11:18:53.195Z
Learning: The codebase has a requirement not to introduce discriminated unions in the Gateway SDK client methods, including the getQuote method return types in sdk/src/gateway/client.ts.
Applied to files:
sdk/src/gateway/client.ts
🧬 Code graph analysis (1)
sdk/src/gateway/client.ts (2)
sdk/src/esplora.ts (1)
EsploraClient(154-517)sdk/src/gateway/utils.ts (1)
formatBtc(108-110)
🪛 GitHub Check: Tests
sdk/src/gateway/client.ts
[failure] 763-763: test/gateway.test.ts > Gateway Tests > should return btc txid for onramp
Error: Insufficient BTC balance in address bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d. Required: 0.00001
❯ GatewayApiClient.executeQuote src/gateway/client.ts:763:23
❯ test/gateway.test.ts:680:25
[failure] 811-811: test/gateway.test.ts > Gateway Tests > should return evm txid for offramp
TypeError: publicClient.getBalance is not a function
❯ GatewayApiClient.executeQuote src/gateway/client.ts:811:51
❯ test/gateway.test.ts:742:25
🪛 GitHub Actions: SDK
sdk/src/gateway/client.ts
[error] 763-763: Insufficient BTC balance in address bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d. Required: 0.00001
[error] 811-811: TypeError: publicClient.getBalance is not a function
🔇 Additional comments (2)
sdk/src/gateway/client.ts (2)
808-813: LGTM! Good fix for handling undefined wallet accountThe implementation correctly handles the case where
walletClient.accountmay be undefined by falling back toparams.fromUserAddress. This prevents the crash reported in tests.
810-870: Mock publicClient.getBalance (and related RPC calls) in testsCI failures show publicClient.getBalance is not a function in the test environment. Repository search returned no test files (test/gateway.test.ts and test/ directory not found), so I couldn't confirm mock locations. Add/extend your test setup mocks to provide the chain RPC methods used in this snippet:
- getBalance
- estimateContractGas
- estimateFeesPerGas (or provide maxFeePerGas in the mocked response)
- getGasPrice
- multicall (returning allowance and decimals)
After adding those mocks, rerun CI or provide the test path for re-check.
| const esploraClient = new EsploraClient(this.chain.id === bob.id ? Network.mainnet : Network.signet); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect network parameter in EsploraClient constructor
The EsploraClient constructor expects a string literal ('mainnet' | 'signet' | 'testnet' | 'regtest'), but you're passing Network.mainnet and Network.signet which are enums from bitcoin-address-validation. This type mismatch will cause runtime errors.
Apply this fix:
-const esploraClient = new EsploraClient(this.chain.id === bob.id ? Network.mainnet : Network.signet);
+const esploraClient = new EsploraClient(this.chain.id === bob.id ? 'mainnet' : 'signet');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const esploraClient = new EsploraClient(this.chain.id === bob.id ? Network.mainnet : Network.signet); | |
| const esploraClient = new EsploraClient(this.chain.id === bob.id ? 'mainnet' : 'signet'); |
🤖 Prompt for AI Agents
In sdk/src/gateway/client.ts around lines 758-759, the EsploraClient is being
constructed with Network.mainnet/Network.signet (enum values) but the
constructor expects string literals ('mainnet'|'signet'|'testnet'|'regtest');
replace the enum usage with the corresponding string literals (e.g. use
this.chain.id === bob.id ? 'mainnet' : 'signet') so the correct string is passed
to EsploraClient, and update any type annotations if necessary to reflect the
string union.
sdk/src/gateway/client.ts
Outdated
| const availableBtcBalance = await esploraClient.getBalance(quote.bitcoinAddress); | ||
| if (availableBtcBalance.total < BigInt(quote.satoshis)) { | ||
| throw new Error( | ||
| `Insufficient BTC balance in address ${quote.bitcoinAddress}. Required: ${formatBtc(BigInt(quote.satoshis))}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use confirmed balance for preflight check instead of total
The BTC balance check uses availableBtcBalance.total which includes unconfirmed mempool transactions. This could lead to issues if those transactions are replaced, dropped, or double-spent. Consider using only the confirmed balance for more reliable validation.
Apply this fix:
-if (availableBtcBalance.total < BigInt(quote.satoshis)) {
+if (availableBtcBalance.confirmed < quote.satoshis) {
throw new Error(
- `Insufficient BTC balance in address ${quote.bitcoinAddress}. Required: ${formatBtc(BigInt(quote.satoshis))}`
+ `Insufficient confirmed BTC balance in address ${quote.bitcoinAddress}. Required: ${formatBtc(BigInt(quote.satoshis))} BTC, Available: ${formatBtc(BigInt(availableBtcBalance.confirmed))} BTC`
);
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Tests
[failure] 763-763: test/gateway.test.ts > Gateway Tests > should return btc txid for onramp
Error: Insufficient BTC balance in address bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d. Required: 0.00001
❯ GatewayApiClient.executeQuote src/gateway/client.ts:763:23
❯ test/gateway.test.ts:680:25
🪛 GitHub Actions: SDK
[error] 763-763: Insufficient BTC balance in address bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d. Required: 0.00001
🤖 Prompt for AI Agents
In sdk/src/gateway/client.ts around lines 761 to 766, the preflight BTC balance
check uses availableBtcBalance.total (which includes unconfirmed mempool funds);
change it to use only availableBtcBalance.confirmed for the comparison so
unconfirmed inputs aren't relied upon. Replace the comparison to check if
availableBtcBalance.confirmed < BigInt(quote.satoshis) and keep the same error
message/formatBtc usage, ensuring types remain consistent (BigInt) when
comparing.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sdk/src/gateway/client.ts (3)
840-855: Allowance owner must match signer; and compute needsApproval before estimating approve gasOwner should be
accountAddress(notparams.fromUserAddress) to avoid false allowance decisions when the wallet’s default account differs from the passed param. Then gate approval gas estimation byneedsApproval.Apply:
- const [allowance, decimals] = await publicClient.multicall({ + const [allowance, decimals] = await publicClient.multicall({ allowFailure: false, contracts: [ { address: tokenAddress, abi: erc20Abi, functionName: 'allowance', - args: [params.fromUserAddress as Address, offrampRegistryAddress], + args: [accountAddress, offrampRegistryAddress], }, { address: tokenAddress, abi: erc20Abi, functionName: 'decimals', }, ], }); @@ - const multiplier = 10n ** BigInt(decimals - 8); - const requiredAmount = BigInt(quote.amountLockInSat) * multiplier; - const needsApproval = requiredAmount > allowance; + const multiplier = 10n ** BigInt(decimals - 8); + const requiredAmount = BigInt(quote.amountLockInSat) * multiplier; + const needsApproval = requiredAmount > allowance; @@ - // Calculate total gas cost needed - const totalGasCost = needsApproval ? approvalGasCost + createOrderGasCost : createOrderGasCost; + // Calculate total gas cost (estimate approve only if needed) + let approvalGasCost = 0n; + if (needsApproval) { + const approvalGasEstimate = await publicClient.estimateContractGas({ + address: tokenAddress, + abi: erc20Abi, + functionName: 'approve', + args: [offrampRegistryAddress, maxUint256], + account: accountAddress, + }); + approvalGasCost = approvalGasEstimate * fee; + } + const totalGasCost = createOrderGasCost + approvalGasCost;
873-885: UseaccountAddressfor approve simulationAvoid passing a possibly undefined
walletClient.account.Apply:
- const { request } = await publicClient.simulateContract({ - account: walletClient.account, + const { request } = await publicClient.simulateContract({ + account: accountAddress, address: tokenAddress, abi: erc20Abi, functionName: 'approve', args: [offrampRegistryAddress, maxUint256], });
887-893: UseaccountAddressfor createOrder simulationSame rationale as above.
Apply:
- const { request } = await publicClient.simulateContract({ - account: walletClient.account, + const { request } = await publicClient.simulateContract({ + account: accountAddress, address: offrampRegistryAddress, abi: offrampOrder.offrampABI, functionName: offrampOrder.offrampFunctionName, args: offrampOrder.offrampArgs, });
♻️ Duplicate comments (2)
sdk/src/gateway/client.ts (2)
816-833: UseaccountAddressin estimates and avoid estimating approve gas unless needed
- Pass
account: accountAddress(current code may beundefined).- Small perf: estimate approve gas only if allowance is insufficient.
Apply:
- // Estimate gas for both potential transactions - const [approvalGasEstimate, createOrderGasEstimate] = await Promise.all([ - publicClient.estimateContractGas({ - address: tokenAddress, - abi: erc20Abi, - functionName: 'approve', - args: [offrampRegistryAddress, maxUint256], - account: walletClient.account, - }), - publicClient.estimateContractGas({ - address: offrampRegistryAddress, - abi: offrampOrder.offrampABI, - functionName: offrampOrder.offrampFunctionName, - args: offrampOrder.offrampArgs, - account: walletClient.account, - }), - ]); + // Estimate createOrder gas first (always needed) + const createOrderGasEstimate = await publicClient.estimateContractGas({ + address: offrampRegistryAddress, + abi: offrampOrder.offrampABI, + functionName: offrampOrder.offrampFunctionName, + args: offrampOrder.offrampArgs, + account: accountAddress, + });Additional changes below compute
needsApprovalfirst, then conditionally estimate approval gas. See follow‑up diffs in lines 840‑866 and 873‑893.
758-767: Fix Esplora network arg, use confirmed balance, correct address in error, and drop console.log
- Pass string literals to EsploraClient (not the
Networkenum).- Use confirmed BTC only for preflight.
- Error message should reference the user’s address, not the gateway deposit address.
- Remove noisy console.log in SDK code.
Apply:
- const esploraClient = new EsploraClient(this.chain.id === bob.id ? Network.mainnet : Network.signet); + const esploraClient = new EsploraClient(this.chain.id === bob.id ? 'mainnet' : 'signet'); @@ - const availableBtcBalance = await esploraClient.getBalance(params.fromUserAddress!); - console.log(availableBtcBalance); - if (availableBtcBalance.total < BigInt(quote.satoshis)) { - throw new Error( - `Insufficient BTC balance in address ${quote.bitcoinAddress}. Required: ${formatBtc(BigInt(quote.satoshis))}, Got: ${formatBtc(BigInt(availableBtcBalance.total))}` - ); - } + const availableBtcBalance = await esploraClient.getBalance(params.fromUserAddress!); + const confirmed = BigInt(availableBtcBalance.confirmed); + if (confirmed < BigInt(quote.satoshis)) { + throw new Error( + `Insufficient confirmed BTC balance in address ${params.fromUserAddress}. Required: ${formatBtc(BigInt(quote.satoshis))} BTC, Available: ${formatBtc(confirmed)} BTC` + ); + }Note: apply the same EsploraClient arg fix in finalizeOnrampOrder (Line 963).
🧹 Nitpick comments (1)
sdk/test/gateway.test.ts (1)
713-721: Mocks align with EIP‑1559 fee path; good coverageAdding getBalance, estimateContractGas, estimateFeesPerGas, and getGasPrice stubs unblocks the new gas budgeting flow.
To de‑flake the onramp test (preflight BTC balance), consider stubbing Esplora instead of hitting the network:
vi.spyOn((await import('../src/esplora')).EsploraClient.prototype, 'getBalance') .mockResolvedValue({ confirmed: 2_000_000, unconfirmed: 0, total: 2_000_000 });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/src/gateway/client.ts(3 hunks)sdk/test/gateway.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/gateway/client.ts (2)
sdk/src/esplora.ts (1)
EsploraClient(154-517)sdk/src/gateway/utils.ts (1)
formatBtc(108-110)
🔇 Additional comments (1)
sdk/src/gateway/client.ts (1)
834-839: LGTM on EIP‑1559 fee selectionUsing
estimateFeesPerGas().maxFeePerGas ?? getGasPrice()is the right fallback chain.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Summary by CodeRabbit
New Features
Tests
Chores