Skip to content
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

Bug/skaled 1714 testeth in historic #1778

Merged
merged 14 commits into from
Jan 15, 2024

Conversation

dimalit
Copy link
Contributor

@dimalit dimalit commented Jan 5, 2024

  1. Run unit tests for historic build as part of CI
  2. Fix wrong responses to some RPC requests (see internal-support/issues/900)

Resolves #1714
Resolves https://github.com/skalenetwork/internal-support/issues/900

Tested with JsonRpcSuite/skip_invalid_transactions unit test

Architectural description:
When eth_getBlockTransactionCountByNumber or eth_getTransactionByBlockNumberAndIndex are called with argument ("latest") on a historic node, the node needs to remove "invalid" transactions from it's answer. For this, it checks transactions' consumed gas (for invalid tx it is 0), and also checks the following:

client.numberFromHash( loc.first ) != _bn || loc.second != realIndex

This condition checks that transaction is reported to be contained in the same block, that is currently requested. But unfortunately numberFromHash() function returned actual block number, but _bn was equal to 0xFFFFFFFF (that means "latest"). This led to false conclusion that transaction is reported to be in different block. So, after fix, we find actual block number:

    if ( _bn == LatestBlock )
        realBn = client.number();
    else if ( _bn == PendingBlock )
        realBn = client.number() + 1;

Now transactions are not being deleted from the answer.

@dimalit dimalit linked an issue Jan 5, 2024 that may be closed by this pull request
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d026bcb) 45.60% compared to head (0921108) 37.83%.
Report is 1 commits behind head on v3.18.0.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           v3.18.0    #1778      +/-   ##
===========================================
- Coverage    45.60%   37.83%   -7.78%     
===========================================
  Files          358      358              
  Lines        51820    51820              
===========================================
- Hits         23635    19604    -4031     
- Misses       28185    32216    +4031     

@dimalit dimalit marked this pull request as ready for review January 9, 2024 15:27
@kladkogex kladkogex merged commit 15cb6a8 into v3.18.0 Jan 15, 2024
7 of 8 checks passed
@kladkogex kladkogex deleted the bug/SKALED-1714-testeth-in-historic branch January 15, 2024 12:26
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run testeth with HISTORIC_STATE
4 participants