Fetch eth_gasprice in send flow on non-Mainnet networks #10444
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. |
There was a problem hiding this comment.
There is a fair bit of overlap between this PR and #10384
In #10384 the fetchBasicGasEstimates function is updated to remove the dependence upon the storage helpers (getStorageItem and setStorageItem), as it relies upon browser fetch caching instead. We'll still want caching for the eth.gasPrice result though as that won't be handled by fetch browser caching (since it's an internal call that we can't easily augment, and it uses POST).
So to that end, perhaps it would be better to put all of the new logic changes made in fetchBasicGasEstimates in its own if block or function. That would help reduce conflicts.
Builds ready [7f56cda]
Page Load Metrics (537 ± 26 ms)
|
ui/app/ducks/gas/gas.duck.js
Outdated
|
|
||
| async function fetchEthGasPriceEstimates(state) { | ||
| const chainId = getCurrentChainId(state); | ||
| const timeLastRetrieved = |
There was a problem hiding this comment.
Nit: Not super important, but, it would be slightly faster if we used Promise.all to ask for the last retrieved time and the basic price estimates in parallel.
|
Thanks for addressing my last review! The new changes look good, aside from additional one problem that I found. |
Builds ready [1cd3cfd]
Page Load Metrics (613 ± 35 ms)
|
|
Something got messed up with that last update 🤔 You pulled in a bunch of unrelated commits somehow. |
Builds ready [1f8b8dd]
Page Load Metrics (571 ± 24 ms)
|
…network in between transactions.
…are now minimal and moved corresponding code to fetchEthGasPriceEstimates
NiranjanaBinoy
force-pushed
the
non-mainnet-gas-price-10003
branch
from
1f8b8dd to
9ecf4ab
Compare
Builds ready [53306eb]
Page Load Metrics (679 ± 76 ms)
|
Co-authored-by: Mark Stacey <[email protected]>
Builds ready [4cf5821]
Page Load Metrics (573 ± 36 ms)
|
NiranjanaBinoy
changed the title
Fixes: #10003
Explanation:
The issue was that the
gasPricefetched for the test networks while performingsendis the same as the Mainnet.The fix will check for the network type and, if it is a test network then the gasprice will be fetched using
gasPrice()fromethjswhich in turn usesethjs-query.If user switches the network from
Send Ethpage, the gas values will be updated for the new network.Manual testing steps:
gasprice.mov
gasprice_refresh.mov