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(plugins/consume): Add HIVE_TERMINAL_TOTAL_DIFFICULTY to all pre-merge forks #1135

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

marioevz
Copy link
Member

@marioevz marioevz commented Jan 27, 2025

πŸ—’οΈ Description

Adds HIVE_TERMINAL_TOTAL_DIFFICULTY equal to 2**32 to all pre-merge fork rulesets.

Fixes most hive tests for go-ethereum when running using consume rlp.

cc @lightclient

πŸ”— Related Issues

None

βœ… Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md. Skipped
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

@marioevz marioevz added type:bug Something isn't working scope:consume Scope: Consume command suite labels Jan 27, 2025
@marioevz
Copy link
Member Author

Besu and Erigon having issues with the value used.

@marioevz
Copy link
Member Author

Reduced the value to 2**32 and this seems to have fixed Reth issues.

Besu behaves the same way with either value, and even with current main branch, so this is a separate issue which we can address in a different PR.

@marioevz
Copy link
Member Author

Now ethereumjs is failing with this error:

Running ethereumjs with flags --gethGenesis ./genesis.json --rpc --rpcEngine --saveReceipts --rpcAddr 0.0.0.0 --rpcEngineAddr 0.0.0.0 --rpcEnginePort 8551 --ws --logLevel debug --rpcDebug all --rpcDebugVerbose all --isSingleNode --jwtSecret ./jwtsecret --loadBlocksFromRlp=blocks/0001.rlp
Error: Error parsing parameters file: nonzero terminal total difficulty is not supported
    at parseGethGenesis (file:///ethereumjs-monorepo/packages/common/dist/esm/utils.js:175:15)
    at createCommonFromGethGenesis (file:///ethereumjs-monorepo/packages/common/dist/esm/constructors.js:34:27)
    at generateClientConfig (file:///ethereumjs-monorepo/packages/client/dist/esm/bin/utils.js:650:18)
    at async run (file:///ethereumjs-monorepo/packages/client/dist/esm/bin/cli.js:279:83)
Error: Error parsing parameters file: nonzero terminal total difficulty is not supported
    at parseGethGenesis (file:///ethereumjs-monorepo/packages/common/dist/esm/utils.js:175:15)
    at createCommonFromGethGenesis (file:///ethereumjs-monorepo/packages/common/dist/esm/constructors.js:34:27)
    at generateClientConfig (file:///ethereumjs-monorepo/packages/client/dist/esm/bin/utils.js:650:18)
    at async run (file:///ethereumjs-monorepo/packages/client/dist/esm/bin/cli.js:279:83)

Using this config:

"config": {
    "ethash": {},
    "chainId": 1,
    "homesteadBlock": 0,
    "daoForkSupport": true,
    "eip150Block": 0,
    "eip155Block": 0,
    "eip158Block": 0,
    "byzantiumBlock": 0,
    "constantinopleBlock": 0,
    "petersburgBlock": 0,
    "istanbulBlock": 0,
    "berlinBlock": 2000,
    "londonBlock": 2000,
    "terminalTotalDifficulty": 4294967296
  }

@jochem-brouwer it was previously passing with the same values in the Berlin and London blocks and zero TTD, but this configuration makes more sense IMO because previously it meant that we've reached the merge without those two pre-merge forks being active.

@jochem-brouwer
Copy link
Member

jochem-brouwer commented Jan 28, 2025

I'd like to test our fix (once it's made πŸ˜… ), how could I test this? Which test could I run here?

@danceratopz
Copy link
Member

I'd like to test our fix (once it's made πŸ˜… ), how could I test this? Which test could I run here?

@jochem-brouwer Let's DM, I'd like to speed-run some docs and run them by you πŸ˜†

@marioevz marioevz marked this pull request as draft January 28, 2025 15:16
@marioevz
Copy link
Member Author

I'm changing this to draft to try to evaluate a bit better what we are trying to accomplish with it, since Geth already fixed their issue by using the mapper here: ethereum/hive#1228

@winsvega
Copy link
Contributor

winsvega commented Feb 3, 2025

how its a bug if hive have default value?

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

Successfully merging this pull request may close these issues.

4 participants