-
Notifications
You must be signed in to change notification settings - Fork 85
fix: eth_getTransactionByHash
correct response parsing
#4331
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: eth_getTransactionByHash
correct response parsing
#4331
Conversation
Signed-off-by: Stanimir Stoyanov <[email protected]>
Signed-off-by: Stanimir Stoyanov <[email protected]>
Signed-off-by: Stanimir Stoyanov <[email protected]>
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.
LGTM, 2 nits
...s/acceptance/data/conformity/overwrites/eth_getTransactionByBlockHashAndIndex/get-block-n.io
Outdated
Show resolved
Hide resolved
...acceptance/data/conformity/overwrites/eth_getTransactionByBlockNumberAndIndex/get-block-n.io
Outdated
Show resolved
Hide resolved
Signed-off-by: Stanimir Stoyanov <[email protected]>
b93948e
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.
@@ Coverage Diff @@
## main #4331 +/- ##
===========================================
- Coverage 95.60% 68.90% -26.71%
===========================================
Files 121 121
Lines 19936 19936
Branches 1732 511 -1221
===========================================
- Hits 19060 13737 -5323
- Misses 850 6185 +5335
+ Partials 26 14 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 67 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Description
Running the Ethereum Execution Spec Tests against Solo environment uncovered some inconsistencies in the response of
etc_getTransactionByHash
.gas
field was assigned fromgas_used
instead ofgas_limit
-> fixed and adjusted the related tests.value
field was providing wrong response due to overflow of the numeric type -> cast toBigInt
and adjusted related tests.Related issue(s)
Fixes #4327
Fixes #4318
Testing Guide
npm run test
- to run unit testsnpm run acceptancetest
- for acceptance testsFor manual testing
gas
limit set to0xf4240
andvalue
0x36183a660c8eff1c00
eth_getTransactionByHash
with the transaction hashgas
field to be the exact 1M and value to be 997869999990000000000 initially providedChanges from original design (optional)
N/A
Additional work needed (optional)
N/A
Checklist