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

Use cached header for latest eth_getBlockByNumber #5292

Merged

Conversation

siladu
Copy link
Contributor

@siladu siladu commented Apr 2, 2023

While syncing, it is possible that we access the cached header's number before the block itself has had chance to be imported. In other words there is a mismatch between cached blockNumber and non-cached blockHeader that is being retrieved for the RPC.

Discovered after adding debug in #5288

Fixed Issue(s)

#5281

Testing

Tested X_SNAP with each CL on sepolia as well as nimbus on goerli.
Without the fix: prysm, lighthouse and nimbus received the error (see #5281 (comment)).
With the fix: no errors

While syncing, we can obtain the cached header's number before the block has had chance to be imported yet.
In other words there is a mismatch between cached blockNumber and non-cached blockHeader that is being retrieved for the RPC.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@github-actions
Copy link

github-actions bot commented Apr 2, 2023

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.
  • I thought about the changelog and included a changelog update if required.

@siladu siladu added the mainnet label Apr 2, 2023
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu marked this pull request as ready for review April 2, 2023 21:50
@siladu siladu enabled auto-merge (squash) April 2, 2023 21:54
@siladu siladu added the TeamGroot GH issues worked on by Groot Team label Apr 2, 2023
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM

@siladu siladu merged commit 27fd97f into hyperledger:main Apr 3, 2023
@siladu siladu deleted the use-cached-headHeader-getBlockByNumber-latest branch April 3, 2023 06:34
jflo pushed a commit to jflo/besu that referenced this pull request Apr 5, 2023
While syncing, it is possible that we access the cached header's number before the block itself has had chance to be imported. In other words there is a mismatch between cached blockNumber and non-cached blockHeader that is being retrieved for the RPC.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
While syncing, it is possible that we access the cached header's number before the block itself has had chance to be imported. In other words there is a mismatch between cached blockNumber and non-cached blockHeader that is being retrieved for the RPC.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
While syncing, it is possible that we access the cached header's number before the block itself has had chance to be imported. In other words there is a mismatch between cached blockNumber and non-cached blockHeader that is being retrieved for the RPC.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mainnet TeamGroot GH issues worked on by Groot Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants