-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix: tracks insufficient balance correctly for nonevm #37988
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
fix: tracks insufficient balance correctly for nonevm #37988
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
✨ Files requiring CODEOWNER review ✨🔄 @MetaMask/swaps-engineers (3 files, +51 -15)
|
Builds ready [500d78e]
UI Startup Metrics (1225 ± 96 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [c4a5908]
UI Startup Metrics (1537 ± 143 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [1b120d9]
UI Startup Metrics (1223 ± 106 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| insufficientBal: providerConfig?.rpcUrl?.includes('localhost') | ||
| ? true | ||
| : undefined, | ||
| : insufficientBalOverride, |
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.
| : insufficientBalOverride, | |
| : isInsufficientBalance, |
Can you reuse the isInsufficientBalance value returned by the getValidationErrors selector instead? It's the same condition used for showing balance validation errors
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.
I think isInsufficientBalance is from the previous quote request, so it's always going to be behind a little, whereas these changes do it 'live' so the flag is always up to date
| providerConfig?.rpcUrl, | ||
| gasIncluded, | ||
| gasIncluded7702, | ||
| insufficientBalOverride, |
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.
| insufficientBalOverride, | |
| isInsufficientBalance, |
Description
Adds tracking for insufficientBalance correctly for nonEVM chains.
Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Computes insufficient balance for non-EVM chains using on-chain balance and sanitized input, and centralizes amount sanitization with safer amount calculations.
insufficientBalOverridefor non-EVM chains usingfromTokenBalanceandBigNumber, passed toupdateBridgeQuoteRequestParams.safeAmountForCalc(fromAmount)when derivingsrcTokenAmountto avoid invalid/partial inputs.sanitizeAmountInputandsafeAmountForCalcinui/pages/bridge/utils/quote.ts.bridge-input-group.tsxnow imports and uses sharedsanitizeAmountInput; removes local sanitizer.Written by Cursor Bugbot for commit 1b120d9. This will update automatically on new commits. Configure here.