-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: legacy gas miss match and not preserved after change #7305
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. |
0c405b6
to
602e8aa
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7305 +/- ##
==========================================
- Coverage 34.60% 34.59% -0.02%
==========================================
Files 1016 1017 +1
Lines 27107 27165 +58
Branches 2198 2215 +17
==========================================
+ Hits 9381 9397 +16
- Misses 17237 17278 +41
- Partials 489 490 +1
☔ View full report in Codecov by Sentry. |
app/components/UI/EditGasFeeLegacyUpdate/EditGasFeeLegacyUpdate.test.tsx
Show resolved
Hide resolved
@blackdevelopa : code changes look good, I left some small queries can you plz check. |
…present particularly for 1559 txn
For some reason, Sepolia is still showing the gas missmatch. It's interesting because it's indeed fixed in Goerli, so we should wonder why both networks behave differently, since in theory , they should be the same 🤔 sepolia-legacy-missmatch.mp4 |
Hey @seaona thanks for the QA feedback. Here's an update Screen.Recording.2023-10-03.at.17.14.46.mov |
thank you @blackdevelopa I also see the issue fixed sepolia-gas.mp4 |
Kudos, SonarCloud Quality Gate passed! |
Description
Whenever I trigger a legacy transaction from a dapp, I can see how the Estimated gas fees does not match the gas details (if I open the Edit gas). Furthermore if I change the suggested gas, I can see how the value is not changed and tx is submitted with the suggested gas value instead of the one I set. See here
Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions:
1. What is the reason for the change?
2. What is the improvement/solution?
Manual testing steps
Test for both 1559 supported network and legacy network. For legacy network, use ganache and check that
--hardfork berlin
or use Palm. For 1559, a testnet like Georli should sufficeSetup legacy network using
1. Step1: Setup wallet and import the right key and account
3. Step3: Check that transactions are submitted as expected.
Screenshots/Recordings
Before
gas_mis_match.mp4
legacy.mp4
After
Send token: https://recordit.co/KCujgfnwOO
Send legacy: http://recordit.co/QOSHpcmrMS
Create a token: http://recordit.co/ygpx2T3Wsw
Token approve: http://recordit.co/C4ItxYxWCB
Send legacy txn from Georli: http://recordit.co/TUmaOp9vHl
Related issues
_Fixes #7290
Bitrise build: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/35e839a1-6d20-491d-bfea-6956c207776a
Pre-merge author checklist
Pre-merge reviewer checklist