Skip to content

fix: Fix race condition in send flow validation for token balance check #33172

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented May 26, 2025

Description

This PR addresses a race condition in the send flow where users could bypass the token balance validation by quickly inputting an amount and proceeding to the confirmation screen.

Key changes include:

  • Improved validation logic for the submitDisabled condition to ensure accurate checks for insufficient token balance, invalid form states, and other edge cases.
  • Enhanced unit test coverage for the submitDisabled logic, including scenarios for smart transactions, recipient warnings, invalid hex data, and insufficient funds.

Open in GitHub Codespaces

Related issues

Fixes: #33062

Manual testing steps

  1. Go to send flow
  2. Choose a token to send
  3. Try to send more more tokens than what you own - you might need to try to send less and then go back and edit the transaction to send more than you own

Screenshots/Recordings

Screencast.from.2025-05-27.08-20-01.webm

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@vinistevam vinistevam added the team-confirmations Push issues to confirmations team label May 26, 2025
Copy link
Contributor

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.

@metamaskbot
Copy link
Collaborator

metamaskbot commented May 26, 2025

✨ Files requiring CODEOWNER review ✨

@MetaMask/confirmations (2 files, +185 -9)
  • 📁 ui/
    • 📁 components/
      • 📁 multichain/
        • 📁 pages/
          • 📁 send/
            • 📄 send.js +10 -8
            • 📄 send.test.js +175 -1

🖥️ @MetaMask/wallet-ux (3 files, +187 -11)
  • 📁 ui/
    • 📁 components/
      • 📁 multichain/
        • 📁 asset-picker-amount/
          • 📄 asset-picker-amount.tsx +2 -2
        • 📁 pages/
          • 📁 send/
            • 📄 send.js +10 -8
            • 📄 send.test.js +175 -1

@metamaskbot
Copy link
Collaborator

Builds ready [5728779]
UI Startup Metrics (1215 ± 63 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1215109814266312481319
load105093612115810781155
domContentLoaded104293212065910721144
domInteractive16133241630
firstPaint77082121340110711147
backgroundConnect85325825
firstReactRender19143732025
getState1463471930
initialActions001001
loadScripts80169793856829903
setupStore85152912
WebpackHomeuiStartup20671651258022422132416
load16051288196416017151837
domContentLoaded15991284195115917111829
domInteractive151172111242
firstPaint1646659368199276
backgroundConnect249353342343
firstReactRender13342362105158333
getState194313501229
initialActions215134
loadScripts15961283194015717081819
setupStore3863117917296
FirefoxBrowserifyHomeuiStartup13031135181213613101610
load1156992167612511771429
domContentLoaded1155992167512511771429
domInteractive943319429104165
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect20125882144
firstReactRender23205642327
getState10420021835
initialActions001001
loadScripts1138966164612311651412
setupStore64354610
WebpackHomeuiStartup15551340203215616341976
load13421167182013814331567
domContentLoaded13421167182013814331566
domInteractive77381491582110
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2315254242338
firstReactRender39285144245
getState94315920
initialActions102111
loadScripts13211152179913514131546
setupStore12529731817
Benchmark value 26 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 38 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 296 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 35 exceeds gate value 24 for firefox browserify home p95 getState
Benchmark value 39 exceeds gate value 38 for firefox webpack home mean firstReactRender
Benchmark value 1976 exceeds gate value 1935 for firefox webpack home p95 uiStartup
Sum of mean exceeds: 7ms | Sum of p95 exceeds: 291ms
Sum of all benchmark exceeds: 298ms

Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 8 Bytes (0%)
  • common: 0 Bytes (0%)

@metamaskbot
Copy link
Collaborator

Builds ready [57fad2e]
UI Startup Metrics (1210 ± 62 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1210110314836212481328
load104392312706010751172
domContentLoaded103692012626010691162
domInteractive16133841529
firstPaint69677118542310671134
backgroundConnect84254822
firstReactRender20154542027
getState1463271827
initialActions001001
loadScripts796688100859829922
setupStore85203814
WebpackHomeuiStartup20921682260320822442458
load16161306199315317251873
domContentLoaded16101302198215117201866
domInteractive1611108151342
firstPaint1746540971220327
backgroundConnect21135682440
firstReactRender15143367113295341
getState144303301226
initialActions318135
loadScripts16071301197215017181855
setupStore3963097819296
FirefoxBrowserifyHomeuiStartup13721154194112914261643
load12141028162712412721490
domContentLoaded12141028162712412711490
domInteractive1103542353116216
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect221273112249
firstReactRender24216452429
getState9417717912
initialActions002001
loadScripts11951011160712412561476
setupStore84578630
WebpackHomeuiStartup15551360220014716041885
load13331175197413713811650
domContentLoaded13321174197413713811650
domInteractive80331572186138
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect251678102547
firstReactRender40275054348
getState135248241031
initialActions001011
loadScripts13121158195413613641629
setupStore86344813
Benchmark value 22 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 39 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 2459 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 296 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 216 exceeds gate value 195 for firefox browserify home p95 domInteractive
Benchmark value 1476 exceeds gate value 1475 for firefox browserify home p95 loadScripts
Benchmark value 30 exceeds gate value 27 for firefox browserify home p95 setupStore
Benchmark value 41 exceeds gate value 38 for firefox webpack home mean firstReactRender
Sum of mean exceeds: 10ms | Sum of p95 exceeds: 265ms
Sum of all benchmark exceeds: 275ms

Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 8 Bytes (0%)
  • common: 0 Bytes (0%)

@vinistevam vinistevam force-pushed the fix/btn-submit-send branch from 57fad2e to 41462e0 Compare May 26, 2025 14:43
@metamaskbot
Copy link
Collaborator

Builds ready [11f0918]
UI Startup Metrics (1216 ± 60 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1216109013536012571324
load104894311825610811161
domContentLoaded104092511775710751154
domInteractive16134551628
firstPaint71893118142310671137
backgroundConnect84295823
firstReactRender20144552034
getState1363071827
initialActions001001
loadScripts80069794156835912
setupStore85182812
WebpackHomeuiStartup22041778273522823692640
load17121387214416318211966
domContentLoaded17061383213416218151956
domInteractive171273121345
firstPaint1696435260198295
backgroundConnect23105492846
firstReactRender15744358118304348
getState204338461538
initialActions317145
loadScripts17021381213116018111945
setupStore4373388522311
FirefoxBrowserifyHomeuiStartup13261140175512513811573
load1177995160812012461415
domContentLoaded1177994160712012451415
domInteractive963527334107156
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2112208212029
firstReactRender23206552328
getState94417837
initialActions001001
loadScripts1158981159311612251388
setupStore847211622
WebpackHomeuiStartup15141321187813616231810
load13031142166112313981540
domContentLoaded13031141166112313981539
domInteractive77332572482117
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect21146372241
firstReactRender38284744145
getState11422322928
initialActions102111
loadScripts12851128164512313791517
setupStore10520920810
Benchmark value 23 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 2204 exceeds gate value 2192 for chrome webpack home mean uiStartup
Benchmark value 1713 exceeds gate value 1711 for chrome webpack home mean load
Benchmark value 1706 exceeds gate value 1704 for chrome webpack home mean domContentLoaded
Benchmark value 1703 exceeds gate value 1699 for chrome webpack home mean loadScripts
Benchmark value 43 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 2641 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 311 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 37 exceeds gate value 24 for firefox browserify home p95 getState
Benchmark value 39 exceeds gate value 38 for firefox webpack home mean firstReactRender
Sum of mean exceeds: 32ms | Sum of p95 exceeds: 451ms
Sum of all benchmark exceeds: 483ms

Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 16 Bytes (0%)
  • common: 0 Bytes (0%)

@vinistevam vinistevam marked this pull request as ready for review May 27, 2025 05:13
@vinistevam vinistevam requested review from a team as code owners May 27, 2025 05:13
@metamaskbot
Copy link
Collaborator

Builds ready [f494991]
UI Startup Metrics (1210 ± 79 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1210109516937912501314
load104494015507510791130
domContentLoaded103691615427610731123
domInteractive17134661631
firstPaint69497155042410601127
backgroundConnect84285926
firstReactRender20155852030
getState1464982028
initialActions003001
loadScripts796691130375835888
setupStore75152811
WebpackHomeuiStartup21131702254522822852510
load16341321207617217601924
domContentLoaded16281318207117117521913
domInteractive161156101343
firstPaint1646264775189288
backgroundConnect2195472534
firstReactRender14244351102251327
getState144302301218
initialActions317134
loadScripts16221316192816417501891
setupStore3763127919308
FirefoxBrowserifyHomeuiStartup13401139199213813861634
load11811011153511812461459
domContentLoaded11811010153511812451458
domInteractive963846345106147
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect21135972329
firstReactRender25206182453
getState10421722812
initialActions001001
loadScripts1163998150111712311442
setupStore9422823613
WebpackHomeuiStartup15091324194012515691780
load13061139175711913571594
domContentLoaded13061139175611913571594
domInteractive83363144083138
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2515265322239
firstReactRender37275244045
getState10412213931
initialActions001011
loadScripts12841125173711413351523
setupStore85375822
Benchmark value 26 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 38 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 2511 exceeds gate value 2454 for chrome webpack home p95 uiStartup
Benchmark value 308 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 26 exceeds gate value 25 for firefox browserify home mean firstReactRender
Sum of mean exceeds: 7ms | Sum of p95 exceeds: 308ms
Sum of all benchmark exceeds: 315ms

Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0%)
  • ui: 79 Bytes (0%)
  • common: 4.05 KiB (0.05%)

@metamaskbot
Copy link
Collaborator

Builds ready [eaf77f2]
UI Startup Metrics (1210 ± 75 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1210107914907512701337
load104793613246810911147
domContentLoaded103993013176910801142
domInteractive16133241629
firstPaint75377115840410581138
backgroundConnect94285823
firstReactRender21165662135
getState1553882131
initialActions006101
loadScripts801690106968844908
setupStore85283813
WebpackHomeuiStartup21171674260922422762439
load16311302203115917281868
domContentLoaded16251298202115817211861
domInteractive151165101340
firstPaint1716268687180291
backgroundConnect21104972333
firstReactRender15743360115309349
getState174348471224
initialActions317134
loadScripts16211297201115617181855
setupStore3463097316297
FirefoxBrowserifyHomeuiStartup13321144186414013801660
load1177997170213112331444
domContentLoaded1177996170213112331443
domInteractive1004119631116166
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2312230242039
firstReactRender23205552327
getState11421729812
initialActions002001
loadScripts1157982161012612191427
setupStore73587620
WebpackHomeuiStartup15131350219514615901743
load12971166172712113361525
domContentLoaded12971165172612113361525
domInteractive79572752683109
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2414215222240
firstReactRender38275044144
getState12529929927
initialActions102111
loadScripts12771148170611613211500
setupStore11527427819
Benchmark value 24 exceeds gate value 18 for chrome browserify home p95 backgroundConnect
Benchmark value 34 exceeds gate value 32 for chrome webpack home mean setupStore
Benchmark value 297 exceeds gate value 65 for chrome webpack home p95 setupStore
Benchmark value 12 exceeds gate value 11 for firefox browserify home mean getState
Benchmark value 39 exceeds gate value 38 for firefox webpack home mean firstReactRender
Sum of mean exceeds: 4ms | Sum of p95 exceeds: 238ms
Sum of all benchmark exceeds: 242ms

Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 16 Bytes (0%)
  • common: 0 Bytes (0%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-confirmations Push issues to confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: User shouldn't get to token transfer confirmation if they don't have enough token balance
3 participants