Skip to content

Conversation

stoyanov-st
Copy link
Contributor

Description

Running the Ethereum Execution Spec Tests against Solo environment uncovered some inconsistencies in the response of etc_getTransactionByHash.

  1. gas field was assigned from gas_used instead of gas_limit -> fixed and adjusted the related tests.
  2. value field was providing wrong response due to overflow of the numeric type -> cast to BigInt and adjusted related tests.

Related issue(s)

Fixes #4327
Fixes #4318

Testing Guide

  1. npm run test - to run unit tests
  2. npm run acceptancetest - for acceptance tests
    For manual testing
  3. Submit transaction with gas limit set to 0xf4240 and value 0x36183a660c8eff1c00
  4. Query eth_getTransactionByHash with the transaction hash
  5. Expect the returned gas field to be the exact 1M and value to be 997869999990000000000 initially provided

Changes from original design (optional)

N/A

Additional work needed (optional)

N/A

Checklist

  • I've assigned an assignee to this PR and related issue(s) (if applicable)
  • I've assigned a label to this PR and related issue(s) (if applicable)
  • I've assigned a milestone to this PR and related issue(s) (if applicable)
  • I've updated documentation (code comments, README, etc. if applicable)
  • I've done sufficient testing (unit, integration, etc.)

@stoyanov-st stoyanov-st requested review from a team as code owners August 28, 2025 11:14
@simzzz simzzz added the bug Something isn't working label Aug 28, 2025
@simzzz simzzz added this to the 0.72.0 milestone Aug 28, 2025
Signed-off-by: Stanimir Stoyanov <[email protected]>
simzzz
simzzz previously approved these changes Aug 29, 2025
@konstantinabl konstantinabl requested review from a team and andrewb1269hg September 1, 2025 12:41
konstantinabl
konstantinabl previously approved these changes Sep 1, 2025
natanasow
natanasow previously approved these changes Sep 1, 2025
Copy link
Contributor

@natanasow natanasow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, 2 nits

Signed-off-by: Stanimir Stoyanov <[email protected]>
@natanasow natanasow merged commit 3efdcad into hiero-ledger:main Sep 4, 2025
92 of 99 checks passed
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/relay/src/formatters.ts 0.00% 1 Missing ⚠️
...ages/relay/src/lib/factories/transactionFactory.ts 0.00% 1 Missing ⚠️

❌ 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.
❌ Your project status has failed because the head coverage (68.90%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (4361c54) and HEAD (b93948e). Click for more details.

HEAD has 22 uploads less than BASE
Flag BASE (4361c54) HEAD (b93948e)
config-service 1 0
relay 1 0
server 1 0
ws-server 1 0
19 1
@@             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     
Flag Coverage Δ
config-service ?
relay ?
server ?
ws-server ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/relay/src/formatters.ts 56.55% <0.00%> (-34.14%) ⬇️
...ages/relay/src/lib/factories/transactionFactory.ts 29.13% <0.00%> (-70.87%) ⬇️

... and 67 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_getTransactionByHash - value field seems to be off GasUsed returned instead of GasLimit
4 participants