Merge go-ethereum v1.15.0 #507
Draft
+4,205
−2,700
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The total difficulty is the sum of all block difficulties from genesis to a certain block. This value was used in PoW for deciding which chain is heavier, and thus which chain to select. Since PoS has a different fork selection algorithm, all blocks since the merge have a difficulty of 0, and all total difficulties are the same for the past 2 years. Whilst the TDs are mostly useless nowadays, there was never really a reason to mess around removing them since they are so tiny. This reasoning changes when we go down the path of pruned chain history. In order to reconstruct any TD, we **must** retrieve all the headers from chain head to genesis and then iterate all the difficulties to compute the TD. In a world where we completely prune past chain segments (bodies, receipts, headers), it is not possible to reconstruct the TD at all. In a world where we still keep chain headers and prune only the rest, reconstructing it possible as long as we process (or download) the chain forward from genesis, but trying to snap sync the head first and backfill later hits the same issue, the TD becomes impossible to calculate until genesis is backfilled. All in all, the TD is a messy out-of-state, out-of-consensus computed field that is overall useless nowadays, but code relying on it forces the client into certain modes of operation and prevents other modes or other optimizations. This PR completely nukes out the TD from the node. It doesn't compute it, it doesn't operate on it, it's as if it didn't even exist. Caveats: - Whenever we have APIs that return TD (devp2p handshake, tracer, etc.) we return a TD of 0. - For era files, we recompute the TD during export time (fairly quick) to retain the format content. - It is not possible to "verify" the merge point (i.e. with TD gone, TTD is useless). Since we're not verifying PoW any more, just blindly trust it, not verifying but blindly trusting the many year old merge point seems just the same trust model. - Our tests still need to be able to generate pre and post merge blocks, so they need a new way to split the merge without TTD. The PR introduces a settable ttdBlock field on the consensus object which is used by tests as the block where originally the TTD happened. This is not needed for live nodes, we never want to generate old blocks. - One merge transition consensus test was disabled. With a non-operational TD, testing how the client reacts to TTD is useless, it cannot react. Questions: - Should we also drop total terminal difficulty from the genesis json? It's a number we cannot react on any more, so maybe it would be cleaner to get rid of even more concepts. --------- Co-authored-by: Gary Rong <[email protected]>
…all flag (#31036) Same as #31015 but requires the contract to exist. Not compatible with any verkle testnet up to now. This adds a `isSytemCall` flag so that it is possible to detect when a system call is executed, so that the code execution and other locations are not added to the witness. --------- Signed-off-by: Guillaume Ballet <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> Co-authored-by: Felix Lange <[email protected]>
Travis often fails because the test times out.
This is an attempt to work around a gcc issue in the Docker build.
This reverts commit 7b96ec4.
This changes the `-upload` flag to just toggle the upload. The remote image name is now configured using the `-hub` flag.
This PR builds on #29040 and updates it to the new version of the spec. I filled the EEST tests and they pass. Link to spec: https://eips.ethereum.org/EIPS/eip-7623 --------- Co-authored-by: Marius van der Wijden <[email protected]> Co-authored-by: lightclient <[email protected]> Co-authored-by: lightclient <[email protected]>
Finalize Prague system contract addresses. Reference: * ethereum/EIPs#9287 * ethereum/EIPs#9288 * ethereum/EIPs#9289
I caught this error on Hive. It was introduced by ethereum/go-ethereum#31071 because after adding the equality check the request type 0 will be rejected.
Removes duplicate code in the interpreter loop.
Fixes a typo in the error message within the `fuzzCrossG2Add` function. The panic message incorrectly references "G1 point addition mismatch" when it should be "G2 point addition mismatch," as the function deals with G2 points. This doesn't affect functionality but could cause confusion during debugging. I've updated the message to reflect the correct curve.
This is a follow-up PR to #29792 to get rid of the data file sync. **This is a non-backward compatible change, which increments the database version from 8 to 9**. We introduce a flushOffset for each freezer table, which tracks the position of the most recently fsync’d item in the index file. When this offset moves forward, it indicates that all index entries below it, along with their corresponding data items, have been properly persisted to disk. The offset can also be moved backward when truncating from either the head or tail of the file. Previously, the data file required an explicit fsync after every mutation, which was highly inefficient. With the introduction of the flush offset, the synchronization strategy becomes more flexible, allowing the freezer to sync every 30 seconds instead. The data items above the flush offset are regarded volatile and callers must ensure they are recoverable after the unclean shutdown, or explicitly sync the freezer before any proceeding operations. --------- Co-authored-by: Felix Lange <[email protected]>
This PR fixes a data race in SetupGenesisWithOverride.
This PR defines the Osaka fork. An easy first step to start our work on the next hardfork (This is needed for EOF testing as well) --------- Co-authored-by: lightclient <[email protected]>
Implements [EIP-7840](ethereum/EIPs#9129) and [EIP-7691](https://github.com/ethereum/EIPs/blob/d96625a4dcbbe2572fa006f062bd02b4582eefd5/EIPS/eip-7691.md). --------- Co-authored-by: Marius van der Wijden <[email protected]> Co-authored-by: Felix Lange <[email protected]>
…067) I hit this case while trying something with the simulated backend. The EVM only enables instruction set forks after the merge when 'Random' is set. In the simulated backend, the random value will be set via the engine API for all blocks after genesis. But for the genesis block itself, the random value will not be assigned in the vm.BlockContext because the genesis has a non-zero difficulty. For my case, this meant that estimateGas did not work for the first transaction sent on the simulated chain, since the contract contained a PUSH0 instruction. This could also be fixed by explicitly configuring a zero difficulty in the simulated backend. However, I think that zero difficulty is a better default these days. --------- Co-authored-by: lightclient <[email protected]>
Replaces #29297, descendant from #27535 --------- This PR removes `locals` as a concept from transaction pools. Therefore, the pool now acts as very a good simulation/approximation of how our peers' pools behave. What this PR does instead, is implement a locals-tracker, which basically is a little thing which, from time to time, asks the pool "did you forget this transaction?". If it did, the tracker resubmits it. If the txpool _had_ forgotten it, chances are that the peers had also forgotten it. It will be propagated again. Doing this change means that we can simplify the pool internals, quite a lot. ### The semantics of `local` Historically, there has been two features, or usecases, that has been combined into the concept of `locals`. 1. "I want my local node to remember this transaction indefinitely, and resubmit to the network occasionally" 2. "I want this (valid) transaction included to be top-prio for my miner" This PR splits these features up, let's call it `1: local` and `2: prio`. The `prio` is not actually individual transaction, but rather a set of `address`es to prioritize. The attribute `local` means it will be tracked, and `prio` means it will be prioritized by miner. For `local`: anything transaction received via the RPC is marked as `local`, and tracked by the tracker. For `prio`: any transactions from this sender is included first, when building a block. The existing commandline-flag `--txpool.locals` sets the set of `prio` addresses. --------- Co-authored-by: Gary Rong <[email protected]>
Fixes the linter on master which was broken by ethereum/go-ethereum#30559
A clarification was made to EIP-7691 stating that at the fork boundary it is required to use the target blob count associated with the head block, rather than the parent as implemented here. See for more: ethereum/EIPs#9249
This PR changes the signature of `CalcExcessBlobGas` to take in just the header timestamp instead of the whole object. It also adds a sanity check for the parent->child block order to `VerifyEIP4844Header`.
Here we add some more changes for live tracing API v1.1: - Hook `OnSystemCallStartV2` was introduced with `VMContext` as parameter. - Hook `OnBlockHashRead` was introduced. - `GetCodeHash` was added to the state interface - The new `WrapWithJournal` construction helps with tracking EVM reverts in the tracer. --------- Co-authored-by: Felix Lange <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
Co-authored-by: lightclient <[email protected]>
sebastianst
force-pushed
the
seb/geth-v1.15.0
branch
from
22baa49 to
60132b9
Compare
sebastianst
commented
Feb 14, 2025
Comment on lines
+302
to
+308
| if !config.TxPool.NoLocals { | ||
| eth.localTxTracker = locals.New(config.TxPool.Journal, rejournal, eth.blockchain.Config(), eth.txPool) | ||
| stack.RegisterLifecycle(eth.localTxTracker) | ||
| } else if config.TxPool.JournalRemote { | ||
| pj := locals.NewPoolJournaler(config.TxPool.Journal, rejournal, eth.txPool) | ||
| stack.RegisterLifecycle(pj) | ||
| } |
There was a problem hiding this comment.
Note for reviewers, I added the second else if config.TxPool.JournalRemote branch to start the alternative PoolJournaler service if journalremotes is enabled. However, it is only started if nolocals is also enabled. This wasn't required before and as such is a breaking change in behavior. But to my understanding, this is usually the case and we only really need full pool journaling if using nolocals.
We may also consider renaming the flag to txpool.fulljournal. This is clearer to me than journalremotes. After the upstream refactor, the notion of "remote" transactions lost some meaning. There are now only "local" and "prio" transactions. We can keep the old flag as an alias for backwards compatibility.
sebastianst
commented
Feb 14, 2025
There was a problem hiding this comment.
I added this file. I wasn't sure what copyright header we want to put into new files since op-geth is a fork of go-ethereum, and in existing files we just retain the old copyright header. I took the upstream fiel header and replaced "go-ethereum" with "op-geth".
sebastianst
commented
Feb 14, 2025
Comment on lines
+28
to
+33
| // A PoolJournaler periodically journales a transaction pool to disk. | ||
| // It is meant to be used instead of the TxTracker when the noLocals and | ||
| // journalRemotes settings are enabled. | ||
| // | ||
| // OP-Stack addition. | ||
| type PoolJournaler struct { |
There was a problem hiding this comment.
I chose to add an alternative service to the new TxTracker instead of hacking the journalremotes feature into it, because that seemed more messy and less modular and maintainable to me. This way, our journalremotes feature is nicely encapsulated in a separate service.
protolambda
reviewed
Feb 19, 2025
Comment on lines
+126
to
+129
| // Check >0 TDs with pre-merge, --0 TDs with post-merge rules | ||
| if header.Difficulty.Sign() > 0 || | ||
| // OP-Stack: transitioned networks must use legacy consensus pre-Bedrock | ||
| cfg.IsOptimism() && !cfg.IsBedrock(header.Number) { |
There was a problem hiding this comment.
Suggested change
| // Check >0 TDs with pre-merge, --0 TDs with post-merge rules | |
| if header.Difficulty.Sign() > 0 || | |
| // OP-Stack: transitioned networks must use legacy consensus pre-Bedrock | |
| cfg.IsOptimism() && !cfg.IsBedrock(header.Number) { | |
| // Check >0 TDs with pre-merge, --0 TDs with post-merge rules | |
| if header.Difficulty.Sign() > 0 || | |
| // OP-Stack: transitioned networks must use legacy consensus pre-Bedrock | |
| (cfg.IsOptimism() && !cfg.IsBedrock(header.Number)) { |
Comment on lines
+89
to
+90
| case <-pj.shutdownCh: | ||
| return |
There was a problem hiding this comment.
Can you add a log here, so we know when it successfully journals the transactions on shutdown?
There was a problem hiding this comment.
Also, should we try to journal.rotate(pj.pool.ToJournal()) one last time upon shutdown, so we ensure we haven't lost any txs?
Comment on lines
+85
to
+86
| timer := time.NewTimer(pj.rejournal) | ||
|
|
There was a problem hiding this comment.
timer is only going to fire once right? I don't see a timer.Reset call. Maybe use a ticker instead?
Also, add a defer ticker.Stop() to stop it. I think in later Go versions it can be GC'ed without stopping, but nice to clean up still.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Merges in all changes from the upstream geth v1.15.0 release.
A few remarks for reviewers and open tasks on which I'm working:
IsTTDReachedinconsensus/beacon/consensus.goto deal with Bedrock-transitioned networksIsTTDReachedgot deleted — I just removed our diff as welltype Beaconhas a new fieldtddlock *uint64for testing — do we need to use it?VerifyHeaderwith 8815245TestEthClientHistoricalBackendis probably failing because of this.ethclient/ethclient_test.go,generateTestChainswitched to usingbeacon.New(ethash.NewFaker())as consensus engine, which is what we chose in our historical genesis test before. So now they’re the same and I removed the diff to make the consensus engine configurable.localsor ajournalany morejournalRemotes, seecore/txpool/legacypool/legacypool.go. We need to revisit the implementation of this feature.PoolJournalerthat periodically journals the pool. It's an alternative to theTxTracker.Metadata
Closes #500