core, core/state, core/vm: merge geth v1.17.3 part 3 (v1.17.4 sync, milestone 5/6) - #2345
Draft
pratikspatil024 wants to merge 89 commits into
Draft
core, core/state, core/vm: merge geth v1.17.3 part 3 (v1.17.4 sync, milestone 5/6)#2345pratikspatil024 wants to merge 89 commits into
pratikspatil024 wants to merge 89 commits into
Conversation
This PR introduces a gasBudget struct to track the available gas for EVM execution. With the upcoming EIP-8037, multi-dimensional gas accounting will be introduced, requiring multiple gas budget counters to be tracked simultaneously. To support this, the counters are grouped into a gasBudget structure. This change is a prerequisite for internal refactoring in preparation for EIP-8037. --------- Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
… (#34055) ## Summary Replace the `BinaryNode` interface with `NodeRef uint32` indices into typed arena pools, eliminating GC-scanned pointers from binary trie nodes. Inspired by [fjl's observation](ethereum/go-ethereum#34034 (comment)): > *"if the binary trie produces such a large graph, it should probably be changed so that the trie node type does not contain pointers. The runtime does not scan objects that do not contain pointers, so it can really help with the performance to build it this way."* ### The problem CPU profiling of the binary trie (EIP-7864) showed **44% of CPU time in garbage collection**. Each `InternalNode` held two `BinaryNode` interface values (2 pointer-words each), and the GC scanned every one. With ~25K `InternalNode`s in memory during block processing, this created enormous GC pressure. ### The solution `NodeRef` is a compact `uint32` (2-bit kind tag + 30-bit pool index). `NodeStore` manages chunked typed pools per node kind: - **InternalNode pool**: ZERO Go pointers (children are `NodeRef`, hash is `[32]byte`) → noscan spans - **HashedNode pool**: ZERO Go pointers → noscan spans - **StemNode pool**: retains `Values [][]byte` (matching existing format) The serialization format is unchanged — flat InternalNode `[type][leftHash][rightHash]` = 65 bytes. ## Benchmark: Apple M4 Pro (`--benchtime=10s --count=3`, on top of #34021) | Metric | Baseline | Arena | Delta | |--------|----------|-------|-------| | Approve (Mgas/s) | 374 | 382 | **+2.1%** | | BalanceOf (Mgas/s) | 885 | 901 | **+1.8%** | | Approve allocs/op | 775K | **607K** | **-21.7%** | | BalanceOf allocs/op | 265K | **228K** | **-14.0%** | ## Benchmark: AMD EPYC 48-core (50GB state, execution-specs ERC-20, on top of #34021 + #34032) | Benchmark | Baseline | Arena | Delta | |-----------|----------|-------|-------| | erc20_approve (write) | 22.4 Mgas/s | **27.0 Mgas/s** | **+20.5%** | | mixed_sload_sstore | 62.9 Mgas/s | **97.3 Mgas/s** | **+54.7%** | | erc20_balanceof (read) | 180.8 Mgas/s | 167.6 Mgas/s | -7.3% (cold cache variance) | The arena benefit scales with heap size — the EPYC (larger heap, more GC pressure) shows much larger gains than the M4 Pro (efficient unified memory). The mixed workload baseline was unstable (62.9 vs 16.3 Mgas/s between runs due to GC-induced throughput collapse); the arena eliminates this entirely (95-97 Mgas/s, stable). ## Dependencies Benchmarked with #34021 (H01 N+1 fix) + #34032 (R14 parallel hashing). No code dependency — applies independently to master. All test suites pass (`trie/bintrie` with `-race`, `core/state`, `triedb/pathdb`, `cmd/geth`). --------- Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
This PR separates the trie reader to mptTrieReader and ubtTrieReader for improved readability and extensibility. --------- Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
In the recent refactoring, the state commit logic has been abstracted, making it more flexible to design state databases for various use cases. For example, execution-only modes where state mutation is disabled. As part of this change, the database interface was extended with a Commit function. However, it currently accepts an unexported struct `stateUpdate`, which prevents downstream projects from customizing the state commit behavior. To address this limitation, the stateUpdate type is now exported.
clarify that `ReadLastPivotNumber` returns `nil` only when snap sync has never been attempted, since the marker is written during snap sync and never cleared.
Increases calldata floor cost from 10/40 to 64/64
## Summary - Add `grpc://` and `grpcs://` URL scheme support for OTLP trace export alongside existing `http://`/`https://` - The OTLP spec defines two transports: HTTP (port 4318) and gRPC (port 4317). Many observability backends (Jaeger, Tempo, Datadog) prefer gRPC for lower overhead - Both `otlptracehttp` and `otlptracegrpc` return `*otlptrace.Exporter`, so only exporter construction changes — everything downstream (batch processor, tracer provider, lifecycle) is untouched - Update flag usage strings to be transport-agnostic ## Example usage ``` geth --rpc.telemetry --rpc.telemetry.endpoint grpc://localhost:4317 geth --rpc.telemetry --rpc.telemetry.endpoint grpcs://tempo-grpc.example.com:443 ``` --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The nodes were named using the byte representation of the path, instead of the binary representation. This was confusing to other client devs trying to achieve interop.
This PR removes `FinalizeAndAssemble` from the consensus engine interface and relocates block assembly logic outside of the consensus engine. Block assembly is consensus-agnostic. Most validations can be performed by the caller. For example: - Withdrawals must be nil prior to Shanghai - After Shanghai upgrade, withdrawals must be non-nil, even if empty. The only notable consensus-specific validation is related to uncles. In clique, the concept of uncles does not exist, and any block containing uncles should be considered invalid. Within the block production package, the policy is to produce blocks according to the latest chain specification. As a result, Clique-specific block production is no longer supported. This tradeoff is considered acceptable.
This is a pre-requisite PR for landing the BAL construction
Difference to Appveyor: - Missing 386 build. Hit some issue because user-space memory there is around 2Gbs. Also seems generally extremely niche. - Not doing the archive step and NSIS installer and uploads (those are done on the builder).
…ld (#34784) This PR reverts the last change to the freebsd build, and it fixes the _direct_ FreeBSD build. Here, we change the upstream of github.com/karalabe/hid to its new home, github.com/ethereum/hid. The new dependency includes a dummy.go file that makes `go mod vendor` work. ##### Origin of the problem Enrique is maintaining the FreeBSD ports, and FreeBSD ports only support vendored go modules. It turns out that `go mod vendor` will not include C files if there is no `.go` file in the directory. Since the C files were missing for `karalabe/hid`, the ports maintainer tried to use the version of `hidapi` that is provided by the ports. To do so, he had to modify the way things are included. This broke the _out of ports_ FreeBSD build.
Adds the installer + archive steps that were done on appveyor to gitea builder.
The rlpx ping command mishandled disconnect responses on two counts:
the error return from rlp.DecodeBytes was ignored, so decode failures
silently produced an "invalid disconnect message" error with no context;
and the decoder assumed the spec-compliant list form exclusively, while
older geth and some other implementations send the reason as a bare
byte.
Accept both wire forms (matching the legacy-tolerant behavior already
in p2p.decodeDisconnectMessage), and on decode failure include the raw
payload so operators can see exactly what the peer sent. Add a unit
test for the decoder covering both forms plus the empty-payload error
path.
scheduleFetches.func1 is the single biggest allocator in the Pyroscope
profile of a busy node (~13.5 GB/hr, 8% of total alloc_space). Each
peer-iteration pre-allocated 'make([]common.Hash, 0, maxTxRetrievals)'
= 8 KB, even for peers that end up collecting no new hashes (all their
announces were already being fetched by someone else).
Defer the slice allocation to the first append. Peers that collect zero
hashes now pay zero allocation, which is the common case on the
timeoutTrigger path where all peers with any announces are iterated.
New benchmarks BenchmarkScheduleFetches_{100peers_10new,
100peers_allFetching, 500peers_3new} (benchstat, 6 samples):
scenario ns/op B/op allocs/op
100p/10new unchanged unchanged unchanged (fast path)
100p/allFetching -62% -92% -20%
500p/3new -22% -44% -7%
geomean -33% -65% -9%
This PR adds three cell-level kzg functions required for the sparse blobpool (eth/72). - VerifyCells: Verifies cells corresponding to proofs. This is used to verify cells received from eth/72 peers. - ComputeCells: Computes cells from blobs. This is needed because user submissions and eth/71 transaction deliveries contain blobs, while eth/72 peers expect cells. - RecoverBlobs: Recovers blobs from partial cells. This is needed to support both eth/71 and eth/72 --------- Co-authored-by: Felix Lange <fjl@twurst.com>
When `rpc.Client.Close()` is called, the TCP connection is torn down without sending a WebSocket Close frame. The server sees `websocket: close 1006 (abnormal closure): unexpected EOF` instead of a clean 1000 (normal closure). ### Root cause `websocketCodec.close()` delegates to `jsonCodec.close()` which calls `c.conn.Close()` — gorilla/websocket's `Conn.Close` explicitly "[closes the underlying network connection without sending or waiting for a close message](https://pkg.go.dev/github.com/gorilla/websocket#Conn.Close)" (per RFC 6455). ### Fix Send a WebSocket Close control frame (opcode 0x8, status 1000) before closing the underlying connection. Uses `WriteControl` with the same `encMu` mutex pattern already used by `pingLoop` for write serialization, and reuses the existing `wsPingWriteTimeout` (5s) constant. `WriteControl` errors are safe to ignore — the connection may already be broken by the time we attempt the close frame. Fixes #30482
Co-authored-by: jwasinger <j-wasinger@hotmail.com>
scheduleFetches.func1 is the biggest allocator in the long-duration profile of node (11% of total alloc_space). Each peer-iteration pre-allocated make([]common.Hash, 0, maxTxRetrievals), even for peers that end up collecting no new hashes (all their announces were already being fetched by someone else). Defer the slice allocation to the first append. Peers that collect zero hashes now pay zero allocation, which is the common case on the timeoutTrigger path where all peers with any announces are iterated.
The testPeer request counters (nAccountRequests, nStorageRequests, nBytecodeRequests, nTrienodeRequests) were plain int fields incremented with ++. These increments happen in Request* methods that are invoked concurrently by the Syncer from multiple goroutines (assignBytecodeTasks, assignStorageTasks, etc.), causing a data race reliably detected by go test -race. Change the counters to atomic.Int64 so increments and reads are synchronized without introducing a mutex. Fixes races detected in TestMultiSyncManyUseless, TestMultiSyncManyUselessWithLowTimeout, TestMultiSyncManyUnresponsive, TestSyncWithStorageAndOneCappedPeer, TestSyncWithStorageAndCorruptPeer, and TestSyncWithStorageAndNonProvingPeer.
The stateReadList field introduced by #34776 to track the state access footprint for EIP-7928 was not propagated by StateDB.Copy. Every other per-transaction field that lives alongside it (accessList, transientStorage, journal, witness, accessEvents) is copied explicitly, so this field was simply missed. After Copy the copy's stateReadList is nil while the original keeps its entries, so the nil-safe guards on StateAccessList.AddAccount / AddState silently drop every access recorded on the copy. For any post-Amsterdam code path that copies a prepared state and keeps reading from the copy, the BAL footprint becomes incomplete. Add a Copy method on bal.StateAccessList and invoke it from StateDB.Copy, matching the pattern used for accessList and accessEvents. --------- Co-authored-by: jwasinger <j-wasinger@hotmail.com>
This PR updates the BAL structure definition to the latest the spec, - Balance has been changed from [16]byte to uint256 - Storage key and value has been changed from [32]byte to uint256 - BlockAccessList has been changed from a struct to a slice of AccountChanges - TxIndex has been changed from uint16 to uint32
`StateSetWithOrigin.decode()` was missing size computation after deserializing origin data, causing `size` to remain zero after journal reload. Added the same calculation logic used in `NewStateSetWithOrigin()`.
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Removes the appveyor.yml since we moved to github runners. --------- Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com>
Passing `--dev=false` currently still enters the dev-mode startup path because a couple of branches check whether the flag was set, not its boolean value. This switches those branches to use `ctx.Bool`, so explicit false does not start dev mode or emit a dev genesis, while `--dev` keeps its existing behavior.
Changes core.Message to use Uint256 which is faster --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
20 upstream first-parent commits, 37 conflicts across 12 upstream PRs — the largest conflict count of this sync, and the only batch carrying three separate refactors: the gas budget, the StateUpdate export, and the FinalizeAndAssemble removal. Witness generation, propagation and import are untouched, per the standing constraint now recorded in the plan's risk flags. That needed checking rather than asserting, because two commits land in files holding witness code: #34724's footprint contains no core/stateless or eth/protocols/wit file and nothing in Bor's witness path consumes stateUpdate, and core/state/reader.go holds CollectStateWitness inside the very type #34763 renames. After resolution that method body is byte-identical to b0ea85c apart from its receiver name, and Bor's concurrent-reader machinery is unchanged by occurrence count. #34726 removes FinalizeAndAssemble from the consensus interface, on the premise that block assembly is consensus-agnostic. Declined, because the premise does not hold for Bor: finalizeAndAssemble calls commitSprintWork at sprint start, which performs the state-sync commits and therefore produces receipts during finalization, and it returns those receipts plus a duration. Upstream's replacement helper takes receipts as an input and returns only a block, so it cannot express this; Bor also carries a separate simulation entry point that skips the sprint span. Bor's method and every implementation are kept. Both failure modes of a partial decline showed up and were fixed rather than shipped. core.AssembleBlock auto-merged into state_processor.go outside any conflict, leaving a helper with no caller and an import Bor does not take. chain_makers.go gained a second assembly block after Bor's retained call, using an IsShanghai signature Bor does not have. #34724 is titled an export of StateUpdate but also changes its representation: accounts move from slim-RLP bytes to decoded structs, storage to hashes, and stateSet is replaced by EncodeMPTState and EncodeUBTState so encoding happens at the consumer. Adopted, because it is byte-equivalent for MPT — EncodeMPTState emits exactly the SlimAccountRLP and trimmed-zero slot RLP Bor produced at construction — so the same bytes reach the snapshot tree and the pathdb state set and the state root is unaffected. The port is commitAndFlush, which in Bor calls triedb.Update directly: it now encodes once and feeds both consumers. The two-origin-map design turned out to be upstream's own, not a Bor divergence. Bor's declined code-origin and tracing feature stays declined, so ContractCode keeps only hash and blob, deriveCodeFields and ToTracingUpdate are absent, and state_sizer keeps Bor's own loop rather than half-adopting a dedup that depends on fields Bor does not populate. #34763 splits the trie reader in two, and the CachingDB adoption in b0ea85c paid for itself here: the new constructors auto-merged cleanly into database_mpt.go and database_ubt.go instead of conflicting inside a single type. Bor's reader is diverged for parallel reads, so all four conflicts kept Bor's side and upstream's ubtTrieReader was inserted verbatim from the boundary. EIP-7976 raises the calldata floor to 64/64 and needs no new wiring: it branches inside FloorDataGas on the existing Amsterdam gate, which is dormant on every preset, so params/config.go is untouched and both fork meta-guards are unaffected. Bor's narrower txpool gate is kept, which means the floor value becomes Amsterdam-dependent while the decision to check it stays Prague-and-Bor; both need reviewing together before Amsterdam is scheduled. Four Bor-only dependents broke without raising a conflict, three of them on the gas budget: ParallelStateDB stopped satisfying vm.StateDB and was caught by Bor's own compile-time assertion, the state-sync system-contract call path still used uint64 gas, the Parity tracer consumed an intrinsic-gas return that became a struct, and ReaderTrieOnly still called the pre-split constructor. Two pre-existing failure sets were re-baselined rather than assumed. The core/vm interrupt and abort tests failed with a gas mismatch that did not match their recorded description, so they were treated as a regression until repeated runs on both revisions showed the same one-in-five failure at b0ea85c with identical text; the tests race a 5ms abort against gas exhaustion of a million-iteration loop, which makes them racy rather than load-dependent. The t8n failures were diffed in full rather than matched by name, since t8n golden output carries gas fields, and differ only in temp paths, timings and allocation counts. No fork gate was flipped. Amsterdam, Verkle/UBT and the binary trie remain dormant.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (63.80%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## ppatil-upstream-mptubt #2345 +/- ##
==========================================================
+ Coverage 54.33% 54.40% +0.07%
==========================================================
Files 927 932 +5
Lines 166781 167729 +948
==========================================================
+ Hits 90616 91256 +640
- Misses 70440 70696 +256
- Partials 5725 5777 +52
... and 22 files with indirect coverage changes
🚀 New features to boost your workflow:
|
20 upstream first-parent commits, 29 conflicts across 9 upstream PRs. Witness generation, propagation and import are untouched, per the standing constraint recorded in the plan's risk flags. That needed checking rather than asserting, because two commits land in the downloader: #34745 rewrites the delivery path in concurrentFetch and queue.deliver, the generic machinery Bor's witness fetcher also flows through, and its new peer-drop check reports errInvalidBody and errInvalidReceipt back through res.Done. Both sentinels are produced only by body and receipt validation, and the witness queue's deliver returns neither, so the witness path still sends nil and behaves exactly as before. The stack arena is the batch's defining decision. Upstream rewrote the EVM stack as a shared arena claimed per call frame, but Bor already carries its own optimisation of the same hot path: a fixed array with a top index, ported from GEVM, with top ordered first so it shares a cache line with the first element. The two designs trade differently, and no measurement exists for Bor's workload. Since no call site depends on arena internals, Bor keeps its storage layout and adopts only the call-site API: a get method, an off-pool constructor for tests, and the Back to back rename. The instruction, gas, memory and eips files take upstream verbatim, because those are the files future core/vm batches keep touching. Declined with the arena: the EVM arena field, Release, and its seventeen call sites, all recorded with the recurring cost that future upstream commits adding a deferred Release will auto-merge and break the build. EIP-7825's per-transaction gas cap is lifted at Amsterdam, because under EIP-8037 the transaction gas limit also covers the state gas reservoir. Bor's gate is wider than upstream's, firing on Osaka or Madhugiri since Bor shipped EIP-7825 at Madhugiri, so every site combines both conditions with the new Amsterdam exclusion. Five sites in total; the miner's pending-filter construction is Bor-only and raised no conflict. The legacy pool kept Bor's version, since upstream's change there is only a line wrap despite what its description claims. Amsterdam is dormant, so all five are inert, and params/config.go, the chain presets, the packaged genesis files and forkid are untouched by the whole batch. The downloader delivery fix is substantive and was adopted with Bor's tracing re-seated on top: accepted now counts only reconstructed items, the task pool is keyed by header hash rather than a parallel slice, failed fetches requeue from the validated index, and errors return unwrapped so the peer-drop check can match them. The slice that fix removes had already been deleted by an auto-merged declaration block while its uses sat inside conflicts, so keeping Bor's side would not have compiled. The discovery timeout fix is adopted verbatim including upstream's own latent nil dereference, because upstream repairs it later inside this sync's range; converging now and taking the repair with its own commit beats diverging. t8n alloc streaming is adopted while keeping Bor's block-based Verkle predicate, and the binary-trie config knob was added to both generated config structs by hand, exactly as gencodec would emit, because the marshal bodies had already auto-merged the field references. Four Bor-only dependents broke without raising a conflict: the PIP-88 SSTORE gas twin and Bor's own stack benchmarks still called the renamed accessor, two files kept an import that upstream dropped, and the miner's pending filter needed the Amsterdam exclusion. The generated switch dispatch regenerated byte-identical. No fork gate was flipped. Amsterdam, Verkle/UBT and the binary trie remain dormant.
20 upstream first-parent commits, 17 conflicts across 10 upstream PRs. Witness generation, propagation and import are untouched. That needed checking because core/state/statedb.go conflicted, which is on the no-go list: the conflict is a one-line fix in the commit path, and the resolved diff for that file contains no witness lines at all. The batch's largest PR is declined. Upstream replaced the deprecated event mux with a feed across eth, node, the downloader, the syncer and the geth command, turning the downloader's start, done and failed events into a single published sync event. That event type carries the sync mode from ethconfig, which is precisely the type relocation this sync already declined earlier because Bor keeps sync mode in the downloader package alongside its own extra modes. The same commit also removes the node's event mux accessor while Bor's geth command still subscribes through it for exit-when-synced and Bor's forked downloader posts to the mux in five places, with upstream's matching changes landing in a downloader file Bor does not have. Adopting it would mean rewriting Bor's downloader event plumbing on top of a still-declined type relocation, so the whole footprint is reverted, after verifying that no other commit in range touches any file in it. Two commits are direct follow-ups to the previous batch, and both arrived as predicted. The discovery timeout fix restores the assignment that the previous batch knowingly adopted with upstream's latent nil dereference rather than diverging over it, which vindicates waiting one batch instead of patching. The downloader delivery fix switches reconstruct to the batch index, which matters because of the previous batch: once accepted counts only successfully reconstructed items, it diverges from the request index as soon as a stale slot appears, and reconstruct would write to the wrong slot. EIP-7981 charges access list entries additional calldata tokens under Amsterdam, adding an Amsterdam parameter to intrinsic gas and an access list parameter to the floor calculation, with overflow guards throughout. It rides the existing dormant Amsterdam gate, so params/config.go, the chain presets, the packaged genesis files and forkid are untouched and both meta-guards are unaffected. The txpool combine keeps Bor's narrower Prague-and-Bor gate with the new signature. The serving loops now stop rather than skip when a body or receipt is missing, so a response is always a contiguous prefix of the request. Adopted into Bor's diverged handlers rather than deferred, because Bor's requester validates by position and the delivery fix in this same batch keys reconstruction on the batch index; serving holes to a position-validating requester is exactly the misalignment both fixes exist to prevent. Bor's own empty-receipt branch and its state-sync receipt path were kept, with only the control flow converted. A reader error after commit now propagates instead of being discarded. The legacy pool's pending method takes a read lock, which was checked rather than assumed: it flattens transaction lists, and that cache write is protected by the sorted map's own mutex in Bor as well. The pathdb synced-state adoption helper is reverted as an orphan on an already declined feature, since it is the completion path for the snap protocol version Bor deferred while block access lists cannot be produced. It surfaced at build time because it calls Bor's diverged buffer constructor. One Bor-only dependent broke without a conflict: the Parity tracer needed the new Amsterdam flag threaded through its intrinsic gas call. That is the third time this file has broken in this sync, and it is now recorded as a standing twin-scan check. No fork gate was flipped. Amsterdam, Verkle/UBT and the binary trie remain dormant.
16 upstream first-parent commits ending at the go-ethereum v1.17.3 release tag, 7 conflicts. Small in conflicts, large in blast radius: the message struct's fee and value fields move to uint256, which auto-merged almost everywhere and surfaced as build breaks in Bor-only callers instead of as conflicts. Witness generation, propagation and import are untouched; no file under core/stateless or the witness protocol is in the footprint. The batch nearly shipped a real regression, caught by the simulate test in internal/ethapi. That package passed in the previous two batches, which settled it as this batch's regression without needing a baseline worktree. The cause is a silent signedness change on a Bor-only path: Bor computed the effective tip as a signed subtraction of base fee from gas price, so it legitimately goes negative when the price is below the base fee, and Bor's fee transfer log consumed that negative value coherently. Upstream rewrote the same lines in uint256, where the subtraction wraps instead. Upstream is unaffected because it skips fee payment entirely when no-base-fee simulation is combined with zero fee caps, which is exactly the guard Bor has commented out, so Bor always evaluates the subtraction. The effective tip is therefore pinned to signed big.Int with a comment explaining why it cannot follow upstream, while the rest of the message struct moves to uint256. The amount, burn amount, execution-result fee fields and the fee transfer log all stay on big.Int, since converting them would ripple through Bor's public execution-result surface for no benefit. The general lesson is recorded: an upstream conversion from big.Int to uint256 is not type-neutral on any path where Bor removed upstream's guard, because the guard is what keeps the value non-negative. The version file is deliberately not bumped. This batch ends at upstream's release commit, which would set the patch level and flip the metadata to stable, but the version recorded here is a Bor release decision and belongs in the milestone chores commit alongside the ledger and plan updates, not inside a merge commit. Elsewhere: the gas estimator keeps Bor's block-based Cancun predicate while adopting upstream's uint256 blob arithmetic; the transaction-to-message conversion takes upstream's form, where the bit-length guards Bor carried are replaced by overflow checks inside the conversions themselves; the state processor takes only the new import, leaving the trie import dropped as it has been since the assembly decline; and the signer no longer mutates the caller's signature buffer. Five Bor-only dependents broke at build with no conflict, all from the message conversion: the access-list creation path, the Parity trace_call price clamp, the simulated backend upstream deleted long ago, and the state-sync system-contract test helper. No fork gate was flipped. Amsterdam, Verkle/UBT and the binary trie remain dormant.
Documentation only; no code changes. Covers batches 20 through 26 plus the two hand-written adoption commits, closing the v1.17.3 milestone. The ledger gains a section per batch, each recording what upstream changed, how it interacts with Bor's divergences, and why every non-trivial conflict was resolved the way it was. Three decisions carry most of the weight. The consensus interface keeps finalize-and-assemble. Upstream removed it on the premise that block assembly is consensus-agnostic, which does not hold for Bor: finalization performs the state-sync commits and therefore produces receipts, while upstream's replacement takes receipts as an input. The EVM stack keeps Bor's storage layout while adopting upstream's call-site API. Upstream's arena and Bor's fixed-array port pursue the same goal with opposite trade-offs, and no measurement exists for Bor's workload, so the hot path was left alone and only the accessor surface converged, which is where future conflicts actually come from. The event mux migration is deferred, because it is entangled with an already deferred sync-mode relocation and with Bor's forked downloader at the same time. The fork register gains three Amsterdam-gated pricing changes: the calldata floor increase, the lifting of the per-transaction gas cap, and the access list cost increase. All three are inert while Amsterdam is dormant, and all three need reviewing together before it is scheduled, since Bor's gates are wider than upstream's in two of the three cases and the gas-cap lift only makes sense alongside a state-gas dimension Bor has not adopted. The needs-wiring backlog gains twelve rows across the milestone, including two that are explicitly not for the merge to decide: the binary-trie reader is invisible to Bor's reader dispatch, which would silently collect no witness data if that trie type were ever enabled, and the effective tip must stay signed because Bor removed the guard that keeps it non-negative. The version file is intentionally unchanged. Checking the earlier milestone tips before deciding showed all of them still on the same version constants, and the only commits in this sync that ever touched the file were auto-merges of upstream's release-cycle commits. Bor does not track upstream's release version here, so the batch that ended on upstream's release tag kept Bor's values, and this commit does not bump them either. What that file should say for a tree based on the final sync target is a Bor release decision, and it is recorded as an open question rather than answered here.
Documentation only; no code changes. Adds the per-milestone verification tier the skill requires alongside the per-batch gates already recorded above it: two devnets, govulncheck and diffguard. The first devnet covers chain health on a build made from the milestone tip and verified by GitCommit from inside the container. It also pins two of the three Amsterdam-gated pricing EIPs added by this milestone as inert by exact measurement rather than by reading the gates, using the figures Bor reports in its own error messages, taken after Madhugiri activated so the PIP-88 rules were live. The access-list intrinsic cost and the calldata floor both come back at their pre-Amsterdam values. The third, the per-transaction gas cap lift, is not reachable from a call because the check sits behind the skip-transaction-checks flag that calls set; an initial probe was accepted for that reason, which is an instrument artifact and is recorded as such so it is not misread later. That one is argued by inspection instead, soundly, since with Amsterdam dormant the merged condition reduces identically to the pre-merge form. The same devnet validates the effective-tip pin from the last batch end to end. Nearly six thousand fee-transfer logs were scanned with no wrapped values and arithmetic coherent to the wei, which is exactly the corruption the uint256 conversion would have shipped. A second devnet exists because the first one covered none of the witness path, which is the divergence this sync guards most carefully and the one three changes in this milestone touch. It runs a producer, a control node with the witness protocol disabled, and a node syncing statelessly from witnesses. All three agree on block hash and state root, at the finalized block and at a fixed height, with the stateless node at zero lag. Since a stateless node reconstructs state from witnesses, that converts three code-reading arguments into live evidence: the renamed reader type still resolves through the runtime dispatch, the rewritten delivery path still returns nil on the witness queue, and the batch-index reconstruction is correct in the queue that also carries witness tasks. The witness store counts confirm the path was loaded rather than merely enabled, since every witness the producer generated was transferred and retained. Both devnets' error output was checked rather than assumed. The only errors are a bounded startup cluster on freshly-joining RPC nodes, identical on the control node that has the witness protocol disabled, so they are unrelated to witnesses. govulncheck reports one called vulnerability, and the earlier recorded set of three is gone. That was verified rather than reported as an improvement: the relevant dependency versions are byte-identical between the previous milestone tip and this one, so nothing here moved them. The earlier entries cleared because the local toolchain advanced past one of them and the other two were already beyond their affected ranges. The remaining entry is a newly published advisory against an unmoved dependency, and bumping it mid-sync would diverge from upstream's module file, so it is a team follow-up rather than a merge decision. diffguard ran structurally and was then re-run at the pull request base in a worktree, so every finding is baselined instead of assumed. Its single failure, a self-referential package cycle, is present identically at the base. Its dead-code hit is a function that arrived with the block-access-list prefetch reader and is byte-identical to upstream, which has no production caller either, so it is folded into the existing row about access lists not being built under parallel execution. That hit is worth noting as a method point, since an unused private function compiles and passes vet, so only this gate could have surfaced it. Mutation testing remains a CI and operator gate, matching the decision recorded at the first milestone. Also records what the verification does not cover, so the gaps are explicit: the binary-trie reader gap cannot be exercised while that trie type is dormant, parallel stateless import was deliberately left off so a failure would isolate, and long-range stateless sync was not exercised because the node joined near genesis.
The previous batch moved the message struct's value field to uint256, which correctly reduced a conversion at the only call site in this file from wrapping the value to passing it through. That was the file's single reference to the package, so the import was left behind and the package no longer compiles under the integration build tag. The change is confined to the import block; the call site is already correct. Worth recording why the per-batch gates missed it: this file is behind a build tag, so it is compiled by neither the ordinary build of all packages nor the untagged vet, and the integration suite that does compile it splits per package, so the Bor-specific integration tests passed while the package next to them failed to build. A vet run with the tag enabled reproduces it in seconds, since vet type-checks test files, and that is the gate this belongs in from now on.
Develop-drift cascade: carries develop's #2347 (Kurtosis e2e and stateless-e2e on every PR base) and #2333 (complete witnesses under BlockSTM v2) one hop further up the stack. No conflict, but this hop splits the trie reader into mptTrieReader and ubtTrieReader, and develop's witness code is written entirely against the old name. The production build broke in four places: the two reader_witness.go helpers that take the reader, and findTrieReader's signature and its type-switch case. Two test constructors moved with it. Renamed all of them; the doc comments naming the type were updated too so they don't refer to something that no longer exists. Mechanical, but this was the resolution worth verifying rather than assuming. findTrieReader is a type switch, so the failure mode here is not a compile error but a switch that still builds while no longer matching what the reader chain actually bottoms out at — witness collection would then no-op with green CI. mptTrieReader carries the Account and Storage methods walkWitnessItems needs, and the tests below exercise the switch end to end. Verified: build clean; vet clean apart from the pre-existing parallel_state_processor.go:341 lock-copy finding; gofmt clean. All seven prewalk and read-set tests pass, including TestWitnessReadSetPrewalkWalksCachedKeys and TestResolveCachedKeysIntoTrieIdempotent, which drive findTrieReader and resolveCachedKeysIntoTrie directly, and TestCollectStateWitnessIncludesFlatServedReads, which drives the whole path. Both regeneration tests pass with 241/241 real mainnet blocks round-tripped and no skips. Noted for the UBT enablement decision, pre-existing on both sides and not introduced here: ubtTrieReader has Account and Storage but no CollectStateWitness, and it appears in neither this type switch nor collectStateWitnessFromReader's. A UBT-backed reader chain therefore collects no witness. Dormant while UBT is off, but it should be settled before the fork rather than rediscovered at it.
Ancestry only. The newTrieReader point-cache fix this carries was already present here, so the merge records the relationship without changing a byte. That is the reason it exists. Without it this branch would not contain its predecessor, and a stacked pull request whose head does not contain its base misreports its own diff and turns an eventual merge into an argument. Deliberately not re-verified, because there is nothing new to verify: the merge result's tree is identical to this branch's previous tree, which is the tree that already passed build, full-tree vet, #2333's prewalk and read-set tests, and CI. A merge with no tree delta cannot break what that tree established.
This was referenced Aug 12, 2026
core, consensus/beacon, eth: merge geth v1.17.3 batch 1/7 (v1.17.4 sync, milestone 5/6 part 1)
#2340
Draft
…rt3 (develop-drift cascade)
…rt3 (develop-drift cascade)
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
20 participants
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.
Important
Reviewer guide — stacked PR 10 of 12. Part of the combined go-ethereum v1.17.4 + v1.17.5 upstream sync, which ships as one stable release. Every PR in the stack merges into the base branch
upstream-merge-v1.17.4; that base merges intodeveloponce, at the very end — not per-PR.Merge-commit only — never squash. Squashing rewrites a branch's SHAs and breaks every PR stacked above it.
Review bottom-up: #2308 → #2319 → #2325 → #2328 → #2337 → #2340 → #2341 → #2342 → #2343 → #2345 → #2346 → #2354. Start at #2308 / #2319 — every PR above inherits them, so reviewing top-down means re-reviewing.
Expected-red / flaky checks (not code blockers):
Quality metrics(diffguard — skipped by team decision; it also mis-scopes across a stacked diff, comparing against the bottom of the stack), andcodecov/project(repo-wide coverage threshold; per-PR patch coverage is green). Kurtosis e2e occasionally flakes (~1-in-5, devtools-owned) and is re-run by hand. Full per-batch conflict-resolution reasoning is indocs/upstream-merges/.Summary
Final PR of the go-ethereum v1.17.3 milestone in the ongoing v1.17.4 sync,
and the one that closes it. Carries batches 23–26 — the last four merge
commits of v1.17.3 — plus the milestone documentation commit.
170 files, +7008/−3687 across the PR. Batch 23 alone is the largest conflict
count of the entire sync.
7b7b9f35233c1bd59fStateUpdateexport,FinalizeAndAssembleremoval9c16c424441b856d4790cbef62b1abbae23996945e338117e067f0core.Message→ uint256, completes v1.17.3df1e813aaWitness surface: verified in every batch, not asserted
Bor's witness generation, propagation and import is a deliberate divergence and
was treated as off-limits throughout. Three batches landed in files that hold
witness code, so each was checked rather than assumed:
CollectStateWitness.After resolution the method body is byte-identical apart from its receiver
name, and Bor's concurrent-reader machinery is unchanged by occurrence count
(
accountCache11,storageCache16,concurrentEnabled4,subTrieConcurrent3,EnableConcurrentReads5,subTrieFor3,resolveSubRoot4 — identical before and after).fetcher also flows through. Its new peer-drop reports
errInvalidBody/errInvalidReceipt, both produced only by body/receipt validation;witnessQueue.deliverreturns neither, so the witness path still sendsnil—byte-for-byte today's behaviour.
core/state/statedb.goconflicted; the conflict is a one-linecommit-path fix and the resolved diff for that file contains zero witness
lines.
One consequence is recorded rather than fixed — see Open items.
Decisions
FinalizeAndAssemblestays on the consensus interface (#34726 declined)Upstream removed it, on the premise that block assembly is consensus-agnostic.
That premise does not hold for Bor:
(*Bor).finalizeAndAssemblecallscommitSprintWorkat sprint start — the state-sync commits — so finalizationproduces receipts, and returns
(*types.Block, []*types.Receipt, time.Duration, error).Upstream's replacement takes receipts as an input and returns only a block, so
it cannot express this. Bor also carries
FinalizeAndAssembleForSimulationforeth_simulateV1.Both failure modes of a partial decline materialised and were fixed rather than
shipped:
core.AssembleBlockauto-merged intostate_processor.gooutside anyconflict, and
chain_makers.gogained a duplicate assembly block calling anIsShanghai(num, time)signature Bor does not have.EVM stack: upstream's API adopted, Bor's storage layout kept (#33960)
Upstream rewrote the stack as a shared arena. Bor already carries its own
optimisation of the same hot path — a fixed
[1024]uint256.Intwith atopindex, ported from GEVM, with
topordered first to share a cache line withdata[0]. Same goal, opposite trade-offs: the arena amortises allocation acrosscall depth but adds a pointer hop to every push/pop/peek.
Since no call site depends on arena internals, the resolution keeps Bor's
storage and adopts only the call-site API (
get(),back(),newStackForTesting()), taking upstream verbatim in the eight files that futurecore/vmbatches will keep touching.evm.arena,EVM.Release()and its 17 callsites are declined and recorded, including the recurring cost that future
upstream commits adding
defer evm.Release()will auto-merge and break the build.TypeMux → Feed deferred (#32585)
Entangled with two Bor divergences at once: the new event type carries
ethconfig.SyncMode— precisely the relocation declined earlier in this sync(Bor keeps
SyncModeindownloaderalongside its extra modes) — andnode/node.golosesEventMux()while Bor'scmd/geth/main.gostill subscribesthrough it and
bor_downloader.goposts to the mux in five places. Full footprintreverted after verifying no other in-range commit touches any of those files.
StateUpdateexport adopted as byte-equivalent (#34724)Titled an export, it also changes the representation (accounts to decoded
structs, storage to hashes, encode-at-consumer). Adopted after proving
EncodeMPTStateemits exactly theSlimAccountRLPand trimmed-zero slot RLP Borproduced at construction — so identical bytes reach the snapshot tree and pathdb
state set, and the state root is unaffected.
A regression caught before it shipped (#34934)
core.Message's fee fields moved touint256. Bor computedeffectiveTip = GasPrice - BaseFeeon signedbig.Int, which legitimatelygoes negative when price < base fee;
uint256.Subwraps instead, corruptingBor's fee-transfer log. Upstream is unaffected because it skips fee payment when
NoBaseFeemeets zero fee caps — exactly the guard Bor has commented out, soBor always evaluates the subtraction.
Caught by
TestSimulateV1/basefee-non-validation, which passed in the twopreceding batches — that settled it as a regression rather than a pre-existing
failure without needing a baseline worktree.
effectiveTipis pinned to signedbig.Intwith a comment; the rest of the struct moves to uint256.The general lesson is recorded: an upstream
big.Int→uint256conversion isnot type-neutral on any path where Bor removed upstream's guard, because the
guard is what keeps the value non-negative.
Fork/EIP scan
No fork gate is flipped, and no activation height changes.
params/config.go,internal/cli/server/chains/,builder/files/andcore/forkid/are untouched by all four batches, so both fork meta-guards areunaffected and pass throughout. Three Amsterdam-gated pricing changes ride the
existing dormant gate and are recorded in
fork-register.md:IsPrague && Bor != nil, so the floor value becomes Amsterdam-dependent while the decision to check stays Prague+BorIsOsaka || IsMadhugiri), because Bor shipped EIP-7825 at MadhugiriAll three must be reviewed together before Amsterdam is scheduled, and the
gas-cap lift only makes sense alongside EIP-8037's state-gas dimension, which Bor
has not adopted (
vm.GasBudget.StateGasremains unused groundwork).Executed tests
Per-batch gates, run on every one of the four batches:
go build ./...,go vet ./...//nolintcopylocksgofmt -l,go mod tidymake lint(golangci-lint v2.11.4, matches CI)tests/bor(integration)Two pre-existing failure sets were re-baselined rather than assumed.
core/vm'sTestInterruptDuringExecution/TestAbortDuringJumpfailed with a gasmismatch that did not match their recorded description; repeated runs on both
revisions gave the same ~1-in-5 rate with identical text. They race a 5 ms abort
against gas exhaustion of a ~1M-iteration JUMP loop — racy tests, not
load-dependent ones, worth fixing independently of this sync.
cmd/evm's t8nfailures were diffed in full rather than matched by name, since t8n golden output
carries gas fields: only temp paths, timings and allocation counts differ.
Milestone suite
bor:v1173-sync(GitCommit-verifieddf1e813aa15…)Devnet: small preset, 13 services, zero Bor-side log errors. Both nodes
205→233 with leader-lag 0 and an identical finalized hash; Heimdall 488 on
both; milestones 232→621; checkpoints ack_count 4→17; state-sync clerk
records ids 1..113 accumulating from L1.
Two of the three Amsterdam-gated pricing EIPs are proven inert exactly, not
just by reading the gates — measured at head 392, i.e. after Madhugiri and
MadhugiriPro activated, so PIP-88 was live. The instrument is the exact figure Bor
reports in its own error messages:
want 25300want 31000The 7825 cap lift is not devnet-provable — the check sits inside
if !msg.SkipTransactionChecksandeth_callsets that flag, so an initialgas = 20,000,000probe was accepted, which is an instrument artifact and notevidence. It is provable by inspection: with
isAmsterdamfalse the mergedcondition reduces identically to Bor's pre-merge form.
Batch 26's
effectiveTippin validated end to end: 5907LogFeeTransferevents on
0x1010scanned from block 100 to head — zero words above 2^200,and the arithmetic exactly coherent (
input1 − amount = output1,input2 + amount = output2, to the wei). That corrupted log is exactly what theuint256 wrap would have shipped.
Full report:
runs/pos-spawn-devnet/2026-08-04T04-39-49Z-claude-pos-v1173-sync/summary.md.Witness path — stateless node, second devnet
The health devnet gave no coverage of the witness path, which is the divergence
this sync guards most carefully and the one three changes in this milestone touch.
A second devnet ran three L2 nodes: a validator producing and serving witnesses,
a control RPC on full sync with the wit protocol off, and an RPC on
syncmode = statelessimporting blocks from witnesses. Rendered config wasverified inside each container.
All three agree on block hash and state root — at the finalized block
(
0xcf, stateRoot0x9c56c6…9c3fae) and at block 202 — with the stateless node atlag 0. A stateless node reconstructs state from witnesses, so that agreement is
end-to-end evidence for the three witness-adjacent changes that were previously
supported only by code reading: batch 23's rename of the type carrying
CollectStateWitness(occurrence counting showed the body unchanged; this shows theruntime dispatch resolves), batch 24's rewrite of
queue.deliver/concurrentFetchthat Bor's witness fetcher flows through, and batch 24/25's switchof
reconstructto a batch index in the queue carryingwitnessTaskPool/SetWitnessDone.The path was loaded, not merely enabled: the stateless node's witness store grew
169 → 178 → 189 files as the head advanced, equal to the producer's 189.
Errors were checked, not waved past: validator 0; both RPCs had 22, all the same
error handling milestone ws event err="chain out of sync"line inside a 4-secondstartup window with none in the following 4 minutes — identical on the control node
with wit disabled, so startup noise on any freshly-joining RPC.
Not covered: the
ubtTrieReaderopen item is unaffected — it only manifestsunder UBT, which can't be enabled without scheduling the fork, so this validates the
MPT dispatch only. Parallel stateless import was left off to keep a failure
isolable, and long-range stateless sync was not exercised.
Report:
runs/pos-spawn-devnet/2026-08-04T04-39-49Z-claude-pos-v1173-witness/summary.md.Note on the image: Docker Desktop's resolver cannot forward to this host's IPv6
link-local-only nameserver, so the builder stage ran as an explicit
docker runwith
--dns/--add-host(flagsdocker builddoes not accept) using the samegolang:1.26.5-alpineimage andmake bor, then packaged with the repoDockerfile's runtime stage verbatim. Functionally equivalent and GitCommit-verified,
but not bit-identical to a CI build — not a release artifact.
Rollout notes
Consensus-affecting only in the sense that this is an upstream sync of execution
code; no fork gate is flipped and no activation height changes. Amsterdam,
Verkle/UBT and the binary trie all remain dormant. Not independently deployable —
this PR is one link in a stack that lands on
developas a unit.Open items for review
UBTDatabase.Readeryields a*ubtTrieReader, while Bor's three Bor-only typeswitches in
core/state/statedb.go(witness collection, storage-cache finder,concurrent-reads enabler) only case on
*mptTrieReader. Closing this meanswriting new witness logic, so it was deliberately surfaced rather than resolved.
Inert today — UBT is dormant and
UBTDatabaseis never constructed on any Bornetwork.
bal.StateAccessListisan unsynchronised map while Bor reads state objects concurrently; safe only
because the list is allocated exclusively under
rules.IsAmsterdam. Amsterdammust not be scheduled on a network running V2 until this is settled.
effectiveTipmust stay signed while Bor keeps upstream'sNoBaseFeeskipcommented out. The
TODO(raneet10)comment there reads as unfinished workrather than a decision, and should be resolved either way.
Twelve needs-wiring rows were added across this milestone; all deferrals, orphans
and coverage gaps are tracked in
docs/upstream-merges/v1.17.4/needs-wiring.md.