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

Geth Import should support importing PoS blocks #24601

Closed
marioevz opened this issue Mar 28, 2022 · 11 comments · Fixed by #24620
Closed

Geth Import should support importing PoS blocks #24601

marioevz opened this issue Mar 28, 2022 · 11 comments · Fixed by #24620

Comments

@marioevz
Copy link
Member

Rationale

At the moment, using geth import for PoS blocks (blocks that happen after the Terminal Total Difficulty is reached) fails because the feature requires all imported blocks to have a non-zero difficulty value, which is not the case for PoS blocks, which all require 0 in the difficulty field.

Why should this feature exist?
This feature would aid testing in the consensus hive simulator, which currently attempts to import blocks in the BlockchainTests category of the tests repository.

What are the use-cases?
Importing blocks with non-zero difficulty after the TTD has been reached should still result in error.
PoS blocks with ommers should also fail, or non-zero Nonce.
These are test cases that cannot be tested through the Engine API since the payload does not contain these fields.

This feature was also mentioned in #24003

Implementation

Do you have ideas regarding the implementation of this feature?
The import feature should be aware of the TerminalTotalDifficulty setting in the genesis, and act accordingly when doing the block imports.

Are you willing to implement this feature?

@rjl493456442
Copy link
Member

I think it's a good feature request.

@holiman
Copy link
Contributor

holiman commented Mar 31, 2022

Some background: up until now, we've kind of treated import of rlp blocks as if the blocks were delivered from the network -- we do a full validation of the blocks.

So now, we either need to change the model, and treat the import as if the imported blocks comes from the trusted engine api. And that makes it easy/possible to test post-merge import -- but it makes it difficult to test certain scenarios, like, if we want the import to reject post-merge blocks which are not coming from engine.

Alternatives:

  • Import blocks as if they were from engine
  • Provide import (normal) and add switch: import --trusted(engine)

I'm leaning towards the second option. Regular import would mean: "try to import this block as if it were delivered from some peer on the network", And --trusted would mean "try to import this block as if it came from the CL"

@holiman
Copy link
Contributor

holiman commented Mar 31, 2022

At the moment, using geth import for PoS blocks (blocks that happen after the Terminal Total Difficulty is reached) fails because the feature requires all imported blocks to have a non-zero difficulty value, which is not the case for PoS blocks, which all require 0 in the difficulty field.

Do you have an example rlp dump of such blocks, which fails the import?

@karalabe
Copy link
Member

@holiman Isn't this case simply exporting and reimporting the blocks from a network post-merge? E.g. Kiln?

In that case once TTD is reached, we should just start accepting blocks without PoW IMHO. Nothing else we can do really.

@winsvega
Copy link
Contributor

winsvega commented Mar 31, 2022

Does geth import rely on genesis.config?

Thus the client can be initialized with TTD defined in genesis config. And geth import continue to work as it was given the merge rules. (Stop accepting PoW blocks and start PoS once TTD is reached)

Btw is it ignoring the nonce and mixhash for testing?

Basically this is test_importRawBlock kind of method, I was asking to expose to simulate a block coming from the network for the client

@marioevz
Copy link
Member Author

Do you have an example rlp dump of such blocks, which fails the import?

@holiman this is the blockchain test used to find the issue, it contains the rlp of the block:
https://github.com/ethereum/tests/blob/merge/BlockchainTests/GeneralStateTests/stExample/mergeTest.json

However the genesis does not contain the TerminalTotalDifficulty, this is added in the hive consensus simulator by setting HIVE_TERMINAL_TOTAL_DIFFICULTY to zero, which was triggered by this line in the mergeTest.json:
"network" : "Merge",

@holiman
Copy link
Contributor

holiman commented Mar 31, 2022

@marioevz you don't happen to have the output from

echo "Initializing database with genesis state..."
$geth $FLAGS init /genesis.json

i.e, the genesis that gets created... do you?

EDIT: my bad, that doesn't spit out the genesis, nevermind. I'll dig it out

@holiman
Copy link
Contributor

holiman commented Mar 31, 2022

HIVE_TERMINAL_TOTAL_DIFFICULTY isn't defined in the go-ethereum client on hive, afaict.

@marioevz
Copy link
Member Author

Here it is:

{
  "config": {
    "ethash": {},
    "chainId": 1,
    "homesteadBlock": 0,
    "daoForkSupport": true,
    "eip150Block": 0,
    "eip155Block": 0,
    "eip158Block": 0,
    "byzantiumBlock": 0,
    "constantinopleBlock": 0,
    "petersburgBlock": 0,
    "istanbulBlock": 0,
    "berlinBlock": 0,
    "yolov2Block": 0,
    "yolov3Block": 0,
    "londonBlock": 0,
    "terminalTotalDifficulty": 0
  },
  "nonce": "0x0",
  "timestamp": "0x0",
  "extraData": "0x00",
  "gasLimit": "0x1000000",
  "difficulty": "0x0",
  "mixHash": "0x1500000000000000000000000000000000000000000000000000000000000000",
  "coinbase": "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
  "alloc": {
    "a94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
      "balance": "0xde0b6b3a7640000",
      "nonce": "0x1"
    },
    "cccccccccccccccccccccccccccccccccccccccc": {
      "code": "0x3a6000554860015500",
      "balance": "0xde0b6b3a7640000",
      "nonce": "0x1"
    }
  },
  "number": "0x0",
  "gasUsed": "0x0",
  "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
  "baseFeePerGas": "0x476"
}

@holiman
Copy link
Contributor

holiman commented Mar 31, 2022

Awesome, now I can repro it

########## BAD BLOCK #########
Chain config: {ChainID: 1 Homestead: 0 DAO: <nil> DAOSupport: true EIP150: 0 EIP155: 0 EIP158: 0 Byzantium: 0 Constantinople: 0 Petersburg: 0 Istanbul: 0, Muir Glacier: <nil>, Berlin: 0, London: 0, Arrow Glacier: <nil>, MergeFork: <nil>, Terminal TD: 0, Engine: ethash}

Number: 1
Hash: 0x8ce374ca14f5c8f25f1a5731cf10e44f8f05ab6968a764fa9d13752e04796c94


Error: invalid difficulty: have 0, want 131072
##############################
 

@holiman
Copy link
Contributor

holiman commented Apr 1, 2022

@marioevz PTAL at #24620. I think the blocks are erroneously generated, not correctly calculating the basefee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@karalabe @holiman @winsvega @rjl493456442 @marioevz and others