Fix send max issue
#22694
Fix send max issue
#22694
Conversation
OGPoyraz
added
the
team-confirmations-system
Builds ready [433c3a9]
Page Load Metrics (727 ± 18 ms)
Bundle size diffs
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #22694 +/- ##
========================================
Coverage 68.34% 68.34%
========================================
Files 1088 1088
Lines 42813 42834 +21
Branches 11396 11405 +9
========================================
+ Hits 29258 29273 +15
- Misses 13555 13561 +6 ☔ View full report in Codecov by Sentry. |
|
The maximum send amount is accurately adjusted in response to fluctuations in gas fees. chrome.movFirefox 122.0 firefox.mov |
| const gasEstimationObject = getGasEstimationObject( | ||
| state, | ||
| { gasFeeEstimates, gasEstimateType }, | ||
| state.confirmTransaction.txData, |
There was a problem hiding this comment.
state.confirmTransaction.txData is repeated to compute txId and also below to compute transactionMeta. It might be more readable to store it in an intermediate variable at the top of the function.
const txData = state.confirmTransaction?.txData
| state, | ||
| { gasFeeEstimates, gasEstimateType }, | ||
| ) => { | ||
| const txId = state.confirmTransaction?.txData?.id; |
There was a problem hiding this comment.
I wonder about all the digging deep into state in this function: are there existing selectors to get this data? Should there be?
OGPoyraz
force-pushed
the
send-max-eth-issue
branch
from
7136e96
to
f362be6
Compare
Builds ready [db38892]
Page Load Metrics (743 ± 17 ms)
Bundle size diffs
|
ui/ducks/send/send.js
Outdated
| addTransactionAndRouteToConfirmationPage(txParams, { | ||
| sendFlowHistory: draftTransaction.history, | ||
| type: transactionType, | ||
| }), | ||
| ); | ||
|
|
||
| dispatch( |
There was a problem hiding this comment.
I noticed we're not using await with dispatch here, but we do throughout the code above. Looking at the return type of signTransaction, dispatch isn't even async. Could this be a good chance to make things more consistent and less confusing in this code?
There was a problem hiding this comment.
Sure, added await to dispatch(setMaxValueMode(/* ... */)))
There was a problem hiding this comment.
Is this not just a side effect of the action you are dispatching?
Some actions will simply update state entirely synchronously and so don't need an await where as many will submit a request to the background and therefore return a promise that can be awaited if needed.
There was a problem hiding this comment.
Maybe I misunderstood Derek here and for sure await will not have any effect at all since it is synchronous call.
Builds ready [bd0e5ef]
Page Load Metrics (809 ± 22 ms)
Bundle size diffs
|
metamaskbot
added
the
release-11.11.0
Description
Currently "send max" eth experience is broken. It doesn't recalculate the send amount due to gas changes while confirming transaction.
Related issues
Fixes:
Manual testing steps
See before/after recordings
Screenshots/Recordings
Before
not.updating.mov
After
working.mov
Pre-merge author checklist
Pre-merge reviewer checklist