celo-org / celo-blockchain

Official repository for the golang Celo Blockchain
https://celo.org
GNU Lesser General Public License v3.0
554 stars 199 forks source link

Hardcode pre gingerbread block gas limits for existing chains #2216

Closed piersy closed 7 months ago

piersy commented 7 months ago

Note this wasn't actually merged because we force pushed to master to clean up an un-squashed rebase.

Description

Ensures that for mainnet, alfajores and baklava, nodes will return the correct block gas limit up to the gingerbread fork even if they are not archive nodes when running in eth compatibility mode (I.E. --disablerpcethcompatibility is not set).

Other changes

A small re-factor of GetBlockByNumber and GetBlockByHash to reduce the depth of nesting.

Tested

An explanation of how the changes were tested or an explanation as to why they don't need to be.

Related issues

Backwards compatibility

This change is not backwards compatible with the previous implementations on mainnet, alfajores or baklava which would have returned a zero gas limit for a block if there was no state for that block.

github-actions[bot] commented 7 months ago

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

coverage: 49.8% of statements across all listed packages
coverage:  63.4% of statements in consensus/istanbul
coverage:  42.7% of statements in consensus/istanbul/announce
coverage:  55.7% 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:  61.6% 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
github-actions[bot] commented 7 months ago
5888 passed, 45 skipped
codecov[bot] commented 7 months ago

Codecov Report

Attention: 55 lines in your changes are missing coverage. Please review.

Comparison is base (346c5b3) 55.08% compared to head (768db5d) 55.08%. Report is 18 commits behind head on master.

Files Patch % Lines
internal/ethapi/api.go 0.00% 44 Missing :warning:
params/gas_limts.go 0.00% 9 Missing :warning:
contracts/currency/currency.go 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2216 +/- ## ========================================== - Coverage 55.08% 55.08% -0.01% ========================================== Files 683 684 +1 Lines 114533 114578 +45 ========================================== + Hits 63095 63116 +21 - Misses 47555 47573 +18 - Partials 3883 3889 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.