eth/consensus : implement eccpow consensus engine - #10
Open
mmingyeomm wants to merge 3694 commits into
Open
Conversation
We recently changed the JSON encoding logic to use an internal `[]byte` buffer. This means we can now always set `Content-Length` on the response.
#35016 + cmd/keeper go mod tidy
Closes #20554 It makes it easier to reason about the lifecycle.
…freezer (#34977) This PR implements flat-file storage for finalized block access lists, specifically: * The freezer is extended with the notion of tail groups, allowing different groups within a single freezer instance to maintain independent tails, while all tables within the same group remain tail-aligned. * The freezer can now dynamically attach new tables to an existing freezer instance, with both the table head and tail initialized to the freezer's common head. * A new freezer table, **bals**, has been added to the chain freezer with its own dedicated tail group, preserving the flexibility to deploy a tail-pruning policy different from the main chain data group. Additionally, the BALs in the key-value store will be migrated to the freezer instance once they are finalized or there are at least 90K block confirmations on top acting as a "soft finalization". This freezing policy is same with all chain segment data.
Supersedes #35060 ``` go test -race ./core/txpool/locals/ ok github.com/ethereum/go-ethereum/core/txpool/locals 1.782s ```
This PR is a prerequisite for landing snap v2, the BAL-healing snap sync algorithm. It duplicates much of the snap v1 skeleton, which is expected to be deprecated once v2 is enabled. The code duplication is acceptable as a short-term tradeoff, simplifying development and reducing integration complexity.
The per-call SERVER span ended inside `handleCall()`, so the JSON-RPC response write happened after the span closed. For large responses like `engine_getBlobsV*`, that write time was missing from traces. - Extend the SERVER span past `writeJSON`. - For batches, add a top-level `jsonrpc.batch` SERVER span (with `rpc.batch.size`) covering the whole batch including `callBuffer.write`. - Add `rpc.writeJSON` span around the non-batch response write. - Add `rpc.writeJSONBatch` span around the batch response write. - Add `rpc.httpWrite` span around the actual HTTP write, separating JSON encoding from network write. - Add additional telemetry helpers. --------- Co-authored-by: Felix Lange <fjl@twurst.com>
stun-list.txt includes 10 bracketd IPv6 server, but the dial network is fixed to "udp4"
The response can reach the client before the deferred spanEnd fires, so call `httpsrv.Close()` before GetSpans is called.
…port (#34807) Rewrites triedb.GenerateTrie as a single partitioned pass that reconciles stale account.Root fields and rebuilds the trie at the same time, with 16-way parallelism and crash resume baked in. --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Update the EraE (ere) reader and builder to the latest e2store ere spec (eth-clients/e2store-format-specs#16). The reader now derives the component layout from the on-disk e2store type tags via the dynamic block index, rather than assuming fixed slot positions. This makes the optional components (receipts, td, proof) resolvable in any supported subset. --------- Co-authored-by: lightclient <lightclient@protonmail.com>
…35100) Make the `Block` parameter optional on the six state-reading methods, defaulting to `latest` when omitted: - `eth_getBalance` - `eth_getCode` - `eth_getStorageAt` - `eth_getTransactionCount` - `eth_getProof` - `eth_getStorageValues` This implements the behavior proposed in ethereum/execution-apis#812. --------- Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>
With Osaka being a while ago I believe we can drop this transition and drop the tx instead.
`clef` is a great tool, however: * It is no longer maintained * No one else in the team can pronounce it properly We are however receiving some slop PRs for it, so I think it's time - with infinite sadness - to say goodbye.
The checksum count during EraE import is off by one when `checksums.txt`
ends its last line on a newline, as the pandaops file does. The current
code would result in one empty string after the final `\n`, something
like
```
[]string{
"line1",
"line2",
"line3",
"",
}
```
Trim off the final `\n`, if it exists: `return
strings.Split(strings.TrimRight(string(b), "\n"), "\n"), nil`
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
check the len of BlobVersionedHashed in blob tx.
### Summary `TestServerWebsocketReadLimit/limit_with_large_request_-_should_fail` is flaky on `windows/amd64` (see [run 25364841576](https://github.com/ethereum/go-ethereum/actions/runs/25364841576/job/74378334589) referenced in #34877): ``` --- FAIL: TestServerWebsocketReadLimit/limit_with_large_request_-_should_fail (0.02s) server_test.go:279: unexpected error for read limit violation: read tcp 127.0.0.1:56703->127.0.0.1:56700: wsarecv: An existing connection was forcibly closed by the remote host. ``` When the server enforces the read limit and tears the connection down, the client's read can race the close frame. On Windows the OS surfaces that race as `wsarecv: An existing connection was forcibly closed by the remote host` instead of the gorilla `CloseError(1009)`, `websocket.ErrReadLimit`, or the POSIX `connection reset by peer` the test already tolerates. This change adds `"forcibly closed"` to the set of acceptable error substrings for the failure case, so the Windows reset is recognized as a valid signal that the server enforced the limit. ### Fixes #34877 ### Test plan - [x] `go test -count=5 -run TestServerWebsocketReadLimit ./rpc/` (darwin/arm64) — pass - [x] `go test ./rpc/...` — pass - [x] `go vet ./rpc/...` / `gofmt -l rpc/server_test.go` — clean - [ ] CI on `windows/amd64` confirms the flake no longer trips --------- Co-authored-by: lightclient <lightclient@protonmail.com>
Previously was iterating Blobs, but that could cause panic if the sidecar is malformed.
) ## Overview This PR fixes a race condition during blockchain shutdown where snapshot generation could continue accessing the trie database after it has been closed, leading to iterator errors. We noticed this in one of our nodes on https://github.com/ava-labs/avalanchego, which relies on an older version of geth with the same issue (so this behavior does happen!). During node shutdown, the following sequence occurs: 1. `BlockChain.Stop()` calls `snaps.Release()` to clean up snapshot resources 2. `Release()` only resets the cache but doesn't stop the generator goroutine 3. The trie database is then closed via `triedb.Close()` 4. The still-running generator attempts to iterate storage tries 5. Iterator fails because the database is closed (`"Generator failed to iterate storage trie"`) ## Problem There are three related bugs: 1. `Release()` doesn't stop generation: The `diskLayer.Release()` method only resets the cache without stopping ongoing snapshot generation, leaving the generator goroutine running after database closure. 2. `stopGeneration()` has an incorrect completion check: The `stopGeneration()` method checks `genMarker != nil` to determine if generation is running. However, `genMarker` is set to nil when generation completes successfully, even though the generator goroutine is still waiting for the abort signal at the end of `generate()`. See line 705 in `generate.go`: https://github.com/ethereum/go-ethereum/blob/eaaa5b716dcf97e94eb17a1469a7385a7101ffab/core/state/snapshot/generate.go#L699-L707 This means `stopGeneration()` returns early without sending the abort signal. 3. Node shutdown doesn't stop generation: During shutdown, no code path calls `stopGeneration()` or sends the abort signal to the generator, causing the generator to access a closed database and error. ## Fix - Modified `diskLayer.Release()` to call `stopGeneration()` before releasing resources - Added cancelation architecture, removing reliance on someone having to wait - Fixed `stopGeneration()` to properly and safely stop snapshot generation - Added `TestGenerateGoroutineLeak` to verify the fix and prevent regression. The test fails without the fix and passes with it. - The test creates a snapshot with active generation, waits for completion, then calls `Release()`, and uses `go.uber.org/goleak` to assert no generator goroutine survives. - Without the fix, the test fails: `Release()` returns without stopping the generator, which stays parked at `generate.go:705` waiting for an abort signal that never comes: ``` --- FAIL: TestGenerateGoroutineLeak (0.88s) generate_test.go: found unexpected goroutines: [Goroutine 6 in state chan receive, with core/state/snapshot.(*diskLayer).generate on top of the stack: core/state/snapshot.(*diskLayer).generate(...) core/state/snapshot/generate.go:705 created by core/state/snapshot.generateSnapshot core/state/snapshot/generate.go:79 ] ``` - With the fix, the test passes: `Release()` -> `stopGeneration()` blocks until the generator goroutine has fully exited, so nothing leaks Note that this fix follows the same pattern used in `Tree.Disable()` in #30040, which introduced `stopGeneration()` for use in `Disable()` and `Rebuild()` but didn't address the shutdown path. The test follows the same pattern used in `TestCheckSimBackendGoroutineLeak`
…rrect variable usages in error strings (#35121) This PR is trying to stem further slop PRs by going over all incorrect strings and fixing them. --------- Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: jwasinger <j-wasinger@hotmail.com>
Fixes a flaky benchmark-output test. Here is an example of it failing: https://github.com/ethereum/go-ethereum/actions/runs/32334412685/job/96322388380?pr=35440 The expected-output regex previously required a fractional component, causing the test to fail intermittently when the duration was a whole number. This change makes that component optional, accepting both `73µs` and `73.001µs`.
This PR moves the state response persistence and trie generation from the single-threaded loop to background, maximizing concurrency for a shorter sync time. One behavioral change: previously, if a storage retrieval was flagged as chunked but the peer delivered all remaining slots within a single range, the healing of that account was skipped. In this PR this behavior would race the account task forwarding, so such accounts are now healed regardless. This ends up with ~9k additionally healed accounts, still a small portion.
…eth_config (#35553) Fixes `eth_config` (EIP-7910) reporting `next: null` and `last: null` even when a future fork is configured. `Config()` computed the next fork as `c.Timestamp(c.LatestFork(t)+1)`, i.e. the timestamp of the next fork *enum value*. Since the BPO forks are optional, a chain whose head is past its last configured BPO fork (e.g. current fork BPO2, BPO3-BPO5 unconfigured, Amsterdam scheduled) got `nil` for `Timestamp(BPO3)`, so `next` came back null — and the existing guard then nulled out `last` as well. This is visible on the glamsterdam devnets today: only `current` is exposed once the head passes the last BPO fork. Per [EIP-7910](https://eips.ethereum.org/EIPS/eip-7910), `next`/`last` may only be null "if the client is not configured to support a future fork". Other clients get this right by iterating over the *configured* forks only (e.g. reth builds its fork list with `forks_iter().filter_map(|(_, cond)| cond.as_timestamp())`, so unscheduled forks never appear). This change scans forward from the current fork to the last scheduled fork and picks the first one with a configured activation time. `last` was already computed correctly (it uses the `Is*` predicates, which skip unconfigured forks). Adds a regression test scenario (Osaka + BPO1 + BPO2 + Amsterdam scheduled, BPO3-BPO5 unset, head at BPO2) asserting `current` = BPO2 and `next` = `last` = Amsterdam; it fails on master.
`newPayload` was spending about 16 ms per block, roughly 20% of its total time, before block processing even started. Profiling that gap showed it was almost entirely JSON framing. The request bytes were being walked like 8 times and they cost more than the decode itself. The fix is two changes. 1. **Stop re-reading the same bytes.** The layers above the decoder do not parse anything, they only need to know where a value starts and ends, but they were each re-parsing and copying the whole request to find out. They now find those bounds and hand out slices of the original bytes. `encoding/json` still performs every value conversion, and the scanning helpers in `jsonscan.go` only ever run on input `encoding/json` has already accepted. 2. **Read a message once instead of twice.** Decoding a message into a `json.RawMessage` makes the decoder scan it to find where it ends, then the `RawMessage` scans it again before copying it out. Where the transport already knows where a message ends, an HTTP body or a WebSocket frame, the message is now read whole and checked once. Streams carry no framing of their own, so IPC, stdio and in-process connections keep the decoder. > **Important Note:** A body with two JSON values in it is now a parse error. The old code answered the first one and quietly ignored the rest. ### Benchmark `BenchmarkNewPayloadDecode`, added here, so it needs no external data. It sends an `engine_newPayloadV4` request over HTTP with the chain stubbed out, and measures decoding the arguments plus assembling and hash checking the block. | payload | master | after change 1 | after both | allocated, master to both | | --- | --- | --- | --- | --- | | 95 KB, 64 txs | 1.37 ms | 0.74 ms | 0.58 ms | 0.91 MB to 0.43 MB | | 293 KB, 192 txs | 4.15 ms | 2.17 ms | 1.61 ms | 3.19 MB to 1.26 MB | | 584 KB, 384 txs | 8.11 ms | 4.29 ms | 3.20 ms | 6.33 MB to 2.47 MB | Small requests get faster too: a one line `eth_blockNumber` goes from 1005 ns to 552 ns.
Tail groups are pruned independently. Truncating the head below the tail of group can happen just in case the tails across the groups are not aligned. Reset such table to empty at the new head instead, and refuse only truncations that fall below every group's tail. Superseded #35536 --------- Co-authored-by: Bosul Mun <bsbs8645@snu.ac.kr>
The `begin > 0 && end > 0` guard skips validation when toBlock == 0 (genesis); switch both bounds to `>= 0` so the API rejects ranges like fromBlock=5, toBlock=0 directly instead of relying on the deeper rangeLogs check.
Add three discv5 integration tests covering negative/adversarial request paths that were previously missing from the v5 suite: - `FindnodeWrongIP`: verifies FINDNODE is challenged with WHOAREYOU when a session is reused from a different UDP endpoint - `FindnodeHandshake`: verifies FINDNODE does not bypass the WHOAREYOU handshake and only returns NODES after handshake completion - `UnsolicitedNodes`: verifies unsolicited authenticated NODES do not cause node injection into later FINDNODE results These tests extend discv5 coverage in areas already exercised more explicitly by the discv4 suite, but adapted to discv5 session and handshake semantics. Assisted-by: Claude:claude-3-opus
This is a drop-in replacement to add metrics to an LRU cache. There are no uses of this as of yet, it is being added to aid temporary debugging code where you want to see the utilization of a cache in Grafana. --------- Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com>
A JSON null passed for a required argument was silently decoded as the zero value instead of being rejected. The check meant to catch it never actually ran. This PR fixes it. Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
…get (#35578) #35526 ensured that the precompile cache size was bounded by counting the key plus the value, but it didn't account for what an entry costs to exist (around 150 bytes see TestPerEntryOverhead). Filling a 1 MB cache with empty values costs 3 MB of memory at 128 byte keys, 9 MB at 32 bytes, and 68.8 MB at 3 bytes, growing as the entries get smaller. This PR ensures the cost is about 2 MB regardless of the key size by charging a 150 byte fixed cost per entry. It also adds back a gauge for the bytes held, which needs a Size accessor on SizeConstrainedCache.
Add a few defensive operations to the blob fetcher, rejecting the deliveries with overlapping custody index. --------- Co-authored-by: Bosul Mun <bsbs8645@snu.ac.kr>
…35537) When an eth/70 receipts response arrives with LastBlockIncomplete set, requestPartialReceipts re-requested the remainder while holding receiptBufferLock across the dispatch. That dispatch blocks until the dispatcher loop accepts the request. If the loop is concurrently cancelling a request for that peer, its cleanup takes the same receiptBufferLock. The dispatcher then waits for the lock the re-request holds while the re-request waits on a channel only the dispatcher reads. The fix releases the lock before sending and routes the follow-up through the dispatcher as a new resend request. --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
#35512) `ExecuteConfig.EnableTracer` gates only the `OnBlockStart`/`OnBlockEnd` envelope. `ProcessBlock` passes `bc.cfg.VmConfig` — tracer included — to `bc.processor.Process` and to `ExecuteStateless`, so the node-wide live tracer still receives the transaction-level hooks when a caller opts out. `debug_executionWitness` is the caller that opts out (`eth/api_debug.go`, `EnableTracer: false`), and it runs `ProcessBlock` directly on an RPC goroutine. On a node started with `--vmtrace`, a single call therefore replays a whole block into the live tracer concurrently with chain insertion. ### Evidence Processing a one-transaction block with `EnableTracer: false` on master: ``` live tracer fired 9 hook(s) with EnableTracer=false: map[OnTxStart:1 OnEnter:1 OnExit:1 OnTxEnd:1 OnBalanceChange:5] ``` `OnBlockStart`/`OnBlockEnd` correctly stay silent, which makes the current state worse rather than better: the live tracer is a stateful singleton whose hooks are documented as being invoked serially during import, and here it observes transactions with no enclosing block. Interleaving two hook streams also corrupts a journal wrapped around the tracer via `tracing.WrapWithJournal`, whose revision stack underflows in `popRevision`. ### Change Resolve the `vm.Config` for an execution from `ExecuteConfig` in one place (`bc.vmConfig`) and use it at both remaining call sites, so `EnableTracer` is the single switch governing the tracer at every layer. The speculative prefetcher, which hand-rolled the same `Tracer = nil`, now goes through the same helper. `useBALExecution` deliberately keeps keying off the node-wide `bc.cfg.VmConfig.Tracer`, so no path flips between sequential and BAL-parallel execution. It is moot for `debug_executionWitness` anyway, where `supportsParallelExecution` already returns false because a witness is wanted. ### Tests `TestProcessBlockTracerOptIn` installs a hook-counting live tracer as the chain's `VmConfig.Tracer` and asserts that the `debug_executionWitness` config fires zero hooks, while the import config still fires the full set. It fails on master with the census above. --------- Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
This PR deprecates the support for golang 1.24 and adds the support for 1.27. Also, the dockerFile and all releases will be built with 1.27 by defaults.
In `Get`, hold the pool read lock around `storeidOfTx` on the RLP decode error path so the lookup is not accessed without synchronization when logging corrupted blob data.
In parallel block execution, each transaction gets a gas pool based on its own gas limit. This makes the EIP-8037 `preCheck` compare the transaction gas limit against itself, so it always passes. The actual block gas limit is only enforced after all transactions have already executed. Reject transactions that could never fit into the block, and stop scheduling new transactions once the completed ones already exceed either gas limit. Two fixes: * Size each transaction’s local gas pool from the block gas limit. This rejects a transaction before execution if its gas limit alone is larger than the block limit. * Keep track of execution gas and state gas used by completed transactions, and stop scheduling more once either exceeds the block limit. This is safe because, in a valid block, no subset of transactions can already exceed the limit. --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
…nses (#35593) eth/70 can complete a receipts request over multiple round trips. Current implementation has two issues in requester side. 1. A peer could keep a request loop alive with incomplete responses that deliver no receipt. An incomplete response must now add at least one receipt. A trailing empty list after other blocks is still accepted. We already have a grace period and peer-dropping logic covering this, but an additional guard is nice to have. 2. Response.Time is recorded across all rounds, making the peer's roundtrip estimate incorrect. The dispatcher now counts the rounds made under a request id, and the downloader divides the delivered items and elapsed time to one round trip.
This PR adds cap to the total blocked transaction size. Problem: Since we include transactions from the same account in nonce order, if there is a partial transaction, subsequent transactions with higher nonces from the same from account cannot be included by us. I refer to these as blocked transactions (feel free to recommend a better name) Because a partial transaction can be injected by running multiple nodes above the threshold around the victim, we need a restriction for such transactions. This PR caps the total size of blocked transactions. Under this policy, we drop transactions whenever size(blockedTxs) > blockedCap || size(txs) > cap (Here blockedCap can be like cap * 0.5). During the eviction, blocked transactions are always considered as cheaper than normal transactions, regardless of the fee they pay. --------- Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: Csaba Kiraly <csaba.kiraly@gmail.com>
`debug_traceCall` requires the block number or hash parameter today. `TraceCall` takes `rpc.BlockNumberOrHash` by value, so an omitted parameter returns "missing value for required argument 1". The pending callTracer spec (ethereum/execution-apis#855) marks the parameter optional with a default of `latest`, consistent with the state-method defaults from #35100 (execution-apis #812/#814 lineage). This changes the argument to `*rpc.BlockNumberOrHash` and defaults to latest when nil. A regression test calls `debug_traceCall` through the RPC server with the parameter omitted and compares against an explicit `latest`. No behavior change when the parameter is supplied.
The plain-transfer shortcut in the gas estimator executes the call with a 21,000 gas limit and returns 21,000 on success. After Amsterdam, EIP-2780 prices these calls below 21,000: a zero-value call to an existing account costs 15,000 and a self transfer costs 12,000. The shortcut hides that and `eth_estimateGas` over-reports by up to 75%. This change returns the used gas from the trial execution instead of the constant. A plain transfer runs no code and gets no refunds, so its used gas is the minimum gas limit that succeeds. Before Amsterdam the used gas is exactly 21,000, so behavior there does not change. Cross-client context, measured with hive rpc-compat on the Amsterdam fixtures from ethereum/execution-apis#867: nethermind and erigon return the exact EIP-2780 minimum (15,000), besu returns 15,159, geth and reth return the 21,000 floor. Includes a regression test at the RPC layer: 21,000 / 15,000 / 12,000 for the three transfer shapes under an Amsterdam config. The test fails without the estimator change.
Adds eth/71 wire conformance tests analogous to [this PR](jrhea#4). Tested through hive via `hive --sim devp2p --sim.limit eth/TestEth71GetBlockAccessLists` against Besu (couple of months ago, opening PR only now because back then eth/71 was not yet on master). Response validation is reused between snap/2 and eth/71. --------- Signed-off-by: Miroslav Kovář <miroslavkovar@protonmail.com> Co-authored-by: healthykim <bsbs8645@snu.ac.kr>
…35589) Sparse blobpool samplers may hold fewer than the 64 cells required to reconstruct a full blob, but they currently announce these transactions to pre-eth/72 peers. Those peers request the legacy full-blob `GetPooledTransactions` encoding, which the sampler cannot serve. This causes unnecessary requests and repeated blob reconstruction failures. This change suppresses announcements to legacy peers unless the transaction is locally reconstructable. Announcements to eth/72 peers are unchanged. Live verification on Platåberget: - 9 eth/71 peers - 7 eth/72 peers - 8 custody columns - repeated reconstruction failures before the change - no reconstruction failures during the post-change observation window Tests: - `go test ./eth/protocols/eth -count=1` - `go test ./eth/... -count=1` - `make all` - `go run ./build/ci.go test` - `go run ./build/ci.go lint` - `go run ./build/ci.go check_generate` - `go run ./build/ci.go check_baddeps`
This file contains hidden or 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
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.
implements eccpow consensus engine for Worldland Network