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

Fix eth_getBlockReceipts #2295

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Fix eth_getBlockReceipts #2295

merged 3 commits into from
Apr 11, 2024

Conversation

piersy
Copy link
Contributor

@piersy piersy commented Apr 9, 2024

Updated the api method to take into account the celo block receipt.

There was previous confusion around whether this was working or not, as it had worked for some people but not others.

This confusion was caused because block receipts are only added when logs have been generated by system calls. Before gingerbread, we would update the gas price minimum on every block and hence create a log for the update, but post gingerbread the only updates that are happening are for epoch blocks.

Tested

Manually tested using cast

cast rpc eth_getBlockReceipts `cast to-hex 53906`

Related issues

Updated the api method to take into account the celo block receipt.
Copy link

github-actions bot commented Apr 9, 2024

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit d832d98

coverage: 50.6% of statements across all listed packages
coverage:  63.2% of statements in consensus/istanbul
coverage:  41.8% of statements in consensus/istanbul/announce
coverage:  55.8% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  64.9% of statements in consensus/istanbul/core
coverage:  50.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.2% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random

Copy link

github-actions bot commented Apr 9, 2024

5882 passed, 45 skipped

@piersy piersy requested a review from palango April 9, 2024 13:24
Copy link
Contributor

@palango palango left a comment

Choose a reason for hiding this comment

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

Logic makes sense, and I tested it locally on both epoch and non-epoch blocks.

@piersy piersy merged commit d832d98 into master Apr 11, 2024
26 checks passed
@piersy piersy deleted the piersy/fix-get-block-receipts branch April 11, 2024 09:34
ezdac pushed a commit that referenced this pull request Apr 18, 2024
Updated the api method to take into account the celo block receipt.

There was previous confusion around whether eth_getBlockReceipts was
working or not, as it had worked for some people but not others.

This confusion was caused because block receipts are only added when logs
have been generated by system calls. Before gingerbread, we would update
the gas price minimum on every block and hence create a log for the update,
but post gingerbread the only updates that are happening are for epoch blocks.

Co-authored-by: Paul Lange <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_getBlockReceipts is broken, because it does not take into account the celo block receipt
2 participants