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

Bad indexed data for block gas limit #2214

Open
piersy opened this issue Jan 18, 2024 · 1 comment
Open

Bad indexed data for block gas limit #2214

piersy opened this issue Jan 18, 2024 · 1 comment
Labels
type:bug Something isn't working

Comments

@piersy
Copy link
Contributor

piersy commented Jan 18, 2024

As of 8th Jan 2023 both explorer.celo.org and celoscan were displaying incorrect block gas limits for some of their blocks.

The block gas limit was only added as a field on the block by the gingerbread fork (effective from block 21616000), before that blocks did not contain the gas limit, instead it was stored in the state and the rpc api could be configured to return the block gas limit (see this pr). This was done to improve compatibility with existing eth tooling. This meant that depending on what version of the client was running and what options were set, different results for block gas limit could be seen for the same block. This likely is what caused the problems in the indexed data of the explorers.

Before the change to improve compatibility with existing tooling the gas limit was never part of blocks returned by the rpc API.

After the compatibility change the gas limit, the below matrix shows when the block gas limit would be returned.

----------------- Compatibility Enabled Compatibility Disabled
Has block state Yes No
No block state No No

Subsequent to this change which went live in the gingerbread hardfork) the behaviour changed slightly for blocks before the gingerbread hardfork.

----------------- Compatibility Enabled Compatibility Disabled
Has block state Yes No
No block state Yes (Zero value) No

In the case where the block was before the gingerbread hardfork block and node did not have the state, it would now return an empty field.

This doesn't seem to have caused problems for users, no tickets have been raised because of this change and I see no discussion about problems caused by this change on our internal comms platform.

So I suggest here, that for the sake of simplicity going forward we always return the gas limit as a field for RPC blocks when compatibility mode is enabled even if we cannot load a value for it. On mainnet, alfajores and baklava, for the blocks from genesis up to the gingerbread hardfork we can hardcode the value returned to match the value that was used by the vm for execution at that block. ( We can do this fairly easily since the gas limit was only modified a handful of times).

Once the change has been enacted we can get blockscout to re-index blocks using the new implementation in order to correct the indexed gasLimit values.

The following block ranges and gas limits were extracted using the script found [here], (https://github.com/piersy/celo_investigations/tree/master/gasLimit). Celoscan has already updated their returned gas limit values based on the table for mainnet below.

Mainnet

Gingerbread hardfork block was 21616000 so we want to hardcode gas limits up to 21615999.

Begin End Limit
0 3316 20M
3317 3251771 10M
3251772 6137284 13M
6137285 13562577 20M
13562578 14137510 50M
14137511 21355414 20M
21355415 21615999 32M

Alfajores

Gingerbread hardfork block was 19814000 so we want to hardcode gas limits up to 19813999.

Begin End Limit
0 911 20M
912 1392354 10M
1392355 1507904 130M
1507905 4581181 13M
4581182 11143972 20M
11143973 19813999 35M

Baklava

Gingerbread hardfork block was 18785000 so we want to hardcode gas limits up to 18784999.

Begin End Limit
0 1229 20M
1230 1713180 10M
1713181 1945002 130M
1945003 15158970 13M
15158971 18784999 20M
@piersy piersy added triage Issue needs triaging type:bug Something isn't working labels Jan 18, 2024
@palango palango removed the triage Issue needs triaging label Jan 22, 2024
piersy added a commit that referenced this issue Jan 23, 2024
Ensures that mainnet, alfajores and baklava will return the correct gas limit
on pre gingerbread blocks when running in eth compatibility mode.

See this ticket for more detail and the correct gas limit values:
#2214
piersy added a commit that referenced this issue Feb 2, 2024
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). More detail can be found here: #2214

Also includes a small re-factor of GetBlockByNumber and GetBlockByHash to reduce the depth of nesting.

This change is not backwards compatible since it is changing what is returned by the rpc for historical blocks.
piersy added a commit that referenced this issue Mar 4, 2024
This fixes a typo in the hardcoded historical gas limits

Relates to #2229 #2214
@piersy
Copy link
Contributor Author

piersy commented Mar 14, 2024

Mainnet nodes have been updated to report correct gas limits.

However blockscout is still not indexing correctly, blockscout seems to index gas limit changes as happening one block earlier as we can see from this query showing blocks where the gas limit changed. This will require more investigatio.

+----------+-----------+----------------+
| number   | gas_limit | prev_gas_limit |
|----------+-----------+----------------|
| 3316     | 10000000  | 20000000       |
| 3251771  | 13000000  | 10000000       |
| 6137284  | 20000000  | 13000000       |
| 13562577 | 50000000  | 20000000       |
| 14137510 | 20000000  | 50000000       |
| 21355414 | 32000000  | 20000000       |
| 21856834 | 50000000  | 32000000       |
+----------+-----------+----------------+

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

No branches or pull requests

2 participants