Minimized changes for the merge #23761
Conversation
| @@ -103,9 +173,100 @@ func (api *consensusAPI) makeEnv(parent *types.Block, header *types.Header) (*bl | |||
| return env, nil | |||
| } | |||
|
|
|||
| func (api *ConsensusAPI) PreparePayload(params AssembleBlockParams) (*PayloadResponse, error) { | |||
| data, err := api.assembleBlock(params) | |||
There was a problem hiding this comment.
TODO: clear unused payloads after a certain time
There was a problem hiding this comment.
Just want to say this. The generated data is basically block. Accumulate the blocks non-stop is very bad.
Do we have any guarantee that when can we clean some cached blocks from the consensus layer?
There was a problem hiding this comment.
Previously I enabled the "transition" in this catalyst package by default, so that all the block creation via catalyst API are using the new rules.
But now we pass the merger into the consensus engine, engine = beacon.New(b.InnerEngine(), eth.Merger()). So that it can happen:
for the first block creation, the TTDReached is still false, block is created with old rule.
MariusVanDerWijden
requested review from
fjl,
gballet,
holiman,
karalabe,
rjl493456442 and
zsfelfoldi
as code owners
MariusVanDerWijden
changed the title
| oldDone, oldResult = beacon.ethone.VerifyHeaders(chain, preHeaders, preSeals) | ||
| newDone, newResult = beacon.verifyHeaders(chain, postHeaders, preHeaders[len(preHeaders)-1]) |
There was a problem hiding this comment.
The results here can lie a bit. The last of the Pow-headers will be fed as the 'ancestor' to the first PoS block. So the PoS block can be deemed 'valid', even if the parent block is later found to be invalid.
So the results can be [valid, valid, valid.. invalid, pos:valid, pos:valid..].
Might not matter 🤷
There was a problem hiding this comment.
Hehe, it's interesting! Maybe we can invalidate all the children in case one parent header is invalid. Or we can choose to add this scenario to comment if we think in practice it's impossible?
There was a problem hiding this comment.
AFAIK the same can happen even now in ethash. I can make the middle header invalid, it will still leave the headers after it potentially valid since we're evaluating each header individually from the rest.
MariusVanDerWijden
force-pushed
the
test-branch
branch
2 times, most recently
from
b117b69
to
ce4f634
Compare
This commit updates the code to the new spec, moving payloadId into
it's own object. It also fixes an issue with finalizing an empty blockhash.
It also properly sets the basefee
MariusVanDerWijden
force-pushed
the
test-branch
branch
from
ce4f634
to
7727d0e
Compare
| // a results channel to retrieve the async verifications. | ||
| // VerifyHeaders expect the headers to be ordered and continuous. | ||
| func (beacon *Beacon) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) { | ||
| if !beacon.IsPoSHeader(headers[len(headers)-1]) { |
There was a problem hiding this comment.
Should we also use IsTTDReached to distinguish pow headers and pos headers?
There was a problem hiding this comment.
I tried to implement this change, but it fails in different places.
The biggest one is during VerifyHeaders, the TD of the last parenthash is not in the db, thus verification of it will fail.
During Author we don't have access to the chain reader.
Also the GenerateChain which is used throughout the tests is very opinionated about this as it uses a fakeChainReader which has no idea about the td. Hacking it in is not super easy
| } | ||
|
|
||
| // NewMerger creates a new Merger which stores its transition status in the provided db. | ||
| func NewMerger(db ethdb.KeyValueStore) *Merger { |
There was a problem hiding this comment.
nitpick, can ignore it right now. The Merger object I think can be moved to core package since it's irrelevant with consensus anymore.
It's totally fine to leave it here, we can do it in the following PR.
| inserted []numberHash // Ephemeral lookup of number/hash for the chain | ||
| firstInserted = -1 // Index of the first non-ignored header | ||
| newTD = new(big.Int).Set(ptd) // Total difficulty of inserted chain | ||
| inserted []rawdb.NumberHash // Ephemeral lookup of number/hash for the chain |
There was a problem hiding this comment.
Why is this a slice and not a simple number if we only care about how many header we've inserted?
* all: work for eth1/2 transtition
* consensus/beacon, eth: change beacon difficulty to 0
* eth: updates
* all: add terminalBlockDifficulty config, fix rebasing issues
* eth: implemented merge interop spec
* internal/ethapi: update to v1.0.0.alpha.2
This commit updates the code to the new spec, moving payloadId into
it's own object. It also fixes an issue with finalizing an empty blockhash.
It also properly sets the basefee
* all: sync polishes, other fixes + refactors
* core, eth: correct semantics for LeavePoW, EnterPoS
* core: fixed rebasing artifacts
* core: light: performance improvements
* core: use keyed field (f)
* core: eth: fix compilation issues + tests
* eth/catalyst: dbetter error codes
* all: move Merger to consensus/, remove reliance on it in bc
* all: renamed EnterPoS and LeavePoW to ReachTDD and FinalizePoS
* core: make mergelogs a function
* core: use InsertChain instead of InsertBlock
* les: drop merger from lightchain object
* consensus: add merger
* core: recoverAncestors in catalyst mode
* core: fix nitpick
* all: removed merger from beacon, use TTD, nitpicks
* consensus: eth: add docstring, removed unnecessary code duplication
* consensus/beacon: better comment
* all: easy to fix nitpicks by karalabe
* consensus/beacon: verify known headers to be sure
* core: comments
* core: eth: don't drop peers who advertise blocks, nitpicks
* core: never add beacon blocks to the future queue
* core: fixed nitpicks
* consensus/beacon: simplify IsTTDReached check
* consensus/beacon: correct IsTTDReached check
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
Co-authored-by: Péter Szilágyi <peterke@gmail.com>
This PR aims to provide the changes needed for running geth as an execution layer.
If you want to run a node on the current network, please use #23607
In this PR, the reverse header sync is disabled, so it can't be used to join the testnets