all: sync with go-ethereum v1.17.2 (upstream merge 4/6) - #2328
Draft
pratikspatil024 wants to merge 85 commits into
Draft
all: sync with go-ethereum v1.17.2 (upstream merge 4/6)#2328pratikspatil024 wants to merge 85 commits into
pratikspatil024 wants to merge 85 commits into
Conversation
ethereum/go-ethereum#33916 + cmd/keeper go mod tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
In `buildPayload()`, the background goroutine uses a `select` to wait on the recommit timer, the stop channel, and the end timer. When both `timer.C` and `payload.stop` are ready simultaneously, Go's `select` picks a case non-deterministically. This means the loop can enter the `timer.C` case and perform an unnecessary `generateWork` call even after the payload has been resolved. Add a non-blocking check of `payload.stop` at the top of the `timer.C` case to exit immediately when the payload has already been delivered.
Return the Amsterdam instruction set from `LookupInstructionSet` when `IsAmsterdam` is true, so Amsterdam rules no longer fall through to the Osaka jump table. --------- Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
For bal-devnet-3 we need to update the EIP-8024 implementation to the latest spec changes: ethereum/EIPs#11306 > Note: I deleted tests not specified in the EIP bc maintaining them through EIP changes is too error prone.
Pebble maintains a batch pool to recycle the batch object. Unfortunately batch object must be explicitly returned via `batch.Close` function. This PR extends the batch interface by adding the close function and also invoke batch.Close in some critical code paths. Memory allocation must be measured before merging this change. What's more, it's an open question that whether we should apply batch.Close as much as possible in every invocation.
Implements https://eips.ethereum.org/EIPS/eip-7778 --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Mainnet was already overriding --cache to 4096. This PR just makes this the default.
…#33927) The BatchSpanProcessor queue size was incorrectly set to DefaultMaxExportBatchSize (512) instead of DefaultMaxQueueSize (2048). I noticed the issue on bloatnet when analyzing the block building traces. During a particular run, the miner was including 1000 transactions in a single block. When telemetry is enabled, the miner creates a span for each transaction added to the block. With the queue capped at 512, spans were silently dropped when production outpaced the span export, resulting in incomplete traces with orphaned spans. While this doesn't eliminate the possibility of drops under extreme load, using the correct default restores the 4x buffer between queue capacity and export batch size that the SDK was designed around.
…789) closes #32741
Fixes a regression in #33593 where a block gas limit > gasCap resulted in more execution than the gas cap.
Eth currently has a flaky test, related to the tx fetcher. The issue seems to happen when Unsubscribe is called while sub is nil. It seems that chain.Stop() may be invoked before the loop starts in some tests, but the exact cause is still under investigation through repeated runs. I think this change will at least prevent the error.
The computation of `MAIN_STORAGE_OFFSET` was incorrect, causing the last byte of the stem to be dropped. This means that there would be a collision in the hash computation (at the preimage level, not a hash collision of course) if two keys were only differing at byte 31.
…le tree (#33961) This is an optimization that existed for verkle and the MPT, but that got dropped during the rebase. Mark the nodes that were modified as needing recomputation, and skip the hash computation if this is not needed. Otherwise, the whole tree is hashed, which kills performance.
`GenerateChain` commits trie nodes asynchronously, and it can happen that some nodes aren't making it to the db in time for `GenerateChain` to open it and find the data it is looking for.
Add nil checks to prevent potential panics when keystore backend is unavailable in the Clef signer API.
Reduce allocations in calculation of tx cost. --------- Co-authored-by: weixie.cui <weixie.cui@okg.com> Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>
Updates go-eth-kzg to https://github.com/crate-crypto/go-eth-kzg/releases/tag/v1.5.0 Significantly reduces the allocations in VerifyCellProofBatch which is around ~5% of all allocations on my node --------- Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
I observed failing tests in Hive `engine-withdrawals`: - https://hive.ethpandaops.io/#/test/generic/1772351960-ad3e3e460605c670efe1b4f4178eb422?testnumber=146 - https://hive.ethpandaops.io/#/test/generic/1772351960-ad3e3e460605c670efe1b4f4178eb422?testnumber=147 ```shell DEBUG (Withdrawals Fork on Block 2): NextPayloadID before getPayloadV2: id=0x01487547e54e8abe version=1 >> engine_getPayloadV2("0x01487547e54e8abe") << error: {"code":-38005,"message":"Unsupported fork"} FAIL: Expected no error on EngineGetPayloadV2: error=Unsupported fork ``` The same failure pattern occurred for Block 3. Per Shanghai engine_getPayloadV2 spec, pre-Shanghai payloads should be accepted via V2 and returned as ExecutionPayloadV1: - executionPayload: ExecutionPayloadV1 | ExecutionPayloadV2 - ExecutionPayloadV1 MUST be returned if payload timestamp < Shanghai timestamp - ExecutionPayloadV2 MUST be returned if payload timestamp >= Shanghai timestamp Reference: - https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md#engine_getpayloadv2 Current implementation only allows GetPayloadV2 on the Shanghai fork window (`[]forks.Fork{forks.Shanghai}`), so pre-Shanghai payloads are rejected with Unsupported fork. If my interpretation of the spec is incorrect, please let me know and I can adjust accordingly. --------- Co-authored-by: muzry.li <muzry.li1@ambergroup.io>
This PR fixes a regression introduced in https://github.com/ethereum/go-ethereum/pull/33836/changes Before PR 33836, running mainnet would automatically bump the cache size to 4GB and trigger a cache re-calculation, specifically setting the key-value database cache to 2GB. After PR 33836, this logic was removed, and the cache value is no longer recomputed if no command line flags are specified. The default key-value database cache is 512MB. This PR bumps the default key-value database cache size alongside the default cache size for other components (such as snapshot) accordingly.
We got a report for a bug in the tracing journal which has the responsibility to emit events for all state that must be reverted. The edge case is as follows: on CREATE operations the nonce is incremented. When a create frame reverts, the nonce increment associated with it does **not** revert. This works fine on master. Now one step further: if the parent frame reverts tho, the nonce **should** revert and there is the bug.
This PR contains two changes: Firstly, the finalized header will be resolved from local chain if it's not recently announced via the `engine_newPayload`. What's more importantly is, in the downloader, originally there are two code paths to push forward the pivot point block, one in the beacon header fetcher (`fetchHeaders`), and another one is in the snap content processer (`processSnapSyncContent`). Usually if there are new blocks and local pivot block becomes stale, it will firstly be detected by the `fetchHeaders`. `processSnapSyncContent` is fully driven by the beacon headers and will only detect the stale pivot block after synchronizing the corresponding chain segment. I think the detection here is redundant and useless.
…3984) The `--remove.chain` flag incorrectly described itself as selecting "state data" for removal, which could mislead operators into removing the wrong data category. This corrects the description to accurately reflect that the flag targets chain data (block bodies and receipts).
This PR allows users to prune their nodes up to the Prague fork. It indirectly depends on #32157 and can't really be merged before eraE files are widely available for download. The `--history.chain` flag becomes mandatory for `prune-history` command. Here I've listed all the edge cases that can happen and how we behave: ## prune-history Behavior | From | To | Result | |-------------|--------------|--------------------------| | full | postmerge | ✅ prunes | | full | postprague | ✅ prunes | | postmerge | postprague | ✅ prunes further | | postprague | postmerge | ❌ can't unprune | | any | all | ❌ use import-history | ## Node Startup Behavior | DB State | Flag | Result | |-------------|--------------|----------------------------------------------------------------| | fresh | postprague | ✅ syncs from Prague | | full | postprague | ❌ "run prune-history first" | | postmerge | postprague | ❌ "run prune-history first" | | postprague | postmerge | ❌ "can't unprune, use import-history or fix flag" | | pruned | all | ✅ accepts known prune points |
This PR adds a cmd tool fetchpayload which connects to a node and gets all the information in order to create a serialized payload that can then be passed to the zkvm.
This PR relocates the witness statistics into the witness itself, making it more self-contained.
This PR enables the block validation of keeper in the womir/openvm zkvm. It also fixes some issues related to building the executables in CI. Namely, it activates the build which was actually disabled, and also resolves some resulting build conflicts by fixing the tags. Co-authored-by: Leo <leo@powdrlabs.com>
…4059) `pool.signer.Sender(tx)` bypasses the sender cache used by types.Sender, which can force an extra signature recovery for every promotable tx (promotion runs frequently). Use `types.Sender(pool.signer, tx)` here to keep sender derivation cached and consistent.
Later on we can consider making these limits configurable if the use-case arose.
We can consider making this limit configurable if ever the need arose.
…r, needs-wiring, plan) Batch 18/19 ledger sections, the EIP-7708 fork-register row, the milestone-wide upstream-PR triage (77 commits) and the full-gate results. Triage surfaced one item needing a team decision: upstream's default cache bump (#33836/#33975) merges clean but never reaches bor server, which computes its caches from internal/cli/server's own Cache: 1024 split. Recorded in needs-wiring.md rather than taken -- it would quadruple the default per-node memory footprint.
|
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 (54.84%) 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-v1.17.1 #2328 +/- ##
===========================================================
+ Coverage 54.27% 54.31% +0.03%
===========================================================
Files 915 919 +4
Lines 165091 165793 +702
===========================================================
+ Hits 89608 90043 +435
- Misses 69872 70084 +212
- Partials 5611 5666 +55
... and 13 files with indirect coverage changes
🚀 New features to boost your workflow:
|
Develop-drift cascade: carries develop's #2347 (Kurtosis e2e and stateless-e2e on every PR base) and #2333 (complete witnesses under BlockSTM v2) up the stack. This is the first hop that carries upstream #34106, so it is where develop's witness code needs adapting; the hops above inherit these resolutions. Witness surface — one conflict and two silent API breaks. StateDB's witness field block conflicted. The merge base carried both `witness` and `witnessStats`; upstream #34106 relocated witness statistics out of StateDB and into `stateless.Witness` (gated by `VmConfig.EnableWitnessStats`, reported via `Witness.ReportMetrics`), so this side had already dropped `witnessStats` and swept its ~25 call sites. develop kept the field and added `witnessPrewalkStop`. Resolved as the plain three-way answer: this side's removal plus develop's addition. Keeping `witnessStats` was considered and rejected. Post-#34106 `StartPrefetcher` no longer takes a stats argument, so the field would have had no writer and no reader — dead weight rather than preserved behaviour. The metric itself is unaffected: `AddState(nodes, owner)` receives exactly the owner develop passed to `witnessStats.Add`, so attribution is identical, and because the relocated collector counts inside `AddState` it also covers the prewalk path that develop's design left uninstrumented. Consequence worth knowing when comparing dashboards across branches: the witness-stats counters read higher here than on develop, because prewalk-collected nodes are now counted. Two further breaks were invisible to git and only the compiler found them, both in develop's new tests: `stateless.NewWitness` gained the `enableStats` parameter (passed false — this test regenerates and compares witness content, which stats do not affect), and `newTrieReader` lost its point-cache parameter (the `nil` argument dropped). Verified: build clean; `go vet ./core ./core/state` clean apart from the pre-existing `parallel_state_processor.go:341` lock-copy finding; gofmt clean. All seven of #2333's prewalk and read-set tests pass, including TestCollectStateWitnessIncludesFlatServedReads. Both witness regeneration tests pass, with the full sweep round-tripping 241/241 real mainnet blocks and no skips — so the fix is verified working here, not merely present.
Ancestry only. The newTrieReader point-cache fix this carries was already present here — it was made at this hop originally, and the two branches below have only now caught up — 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
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.
Important
Reviewer guide — stacked PR 4 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
Fourth milestone of the Bor ← go-ethereum sync to v1.17.4: takes upstream from v1.17.1 → v1.17.2 (77 first-parent commits, 4 batches of ~20). Stacked on #2325 (v1.17.1) — base is
ppatil-upstream-v1.17.1, so this PR's diff is exactly this milestone.Upstream release notes: https://github.com/ethereum/go-ethereum/releases/tag/v1.17.2
The headline is EIP-7708 (ETH transfers as logs), the first Amsterdam EIP whose content had to be reshaped rather than copied — Bor's transfer, selfdestruct and parallel-state paths all diverge from upstream's. Like every other upstream fork/EIP in this sync it is merged wired but dormant:
AmsterdamBlockis nil on every shipped preset, so no behavior changes on mainnet or Amoy.16783c167..be4dc0c4b(77 first-parent commits)Commits
00540f94609c78485177e7e5ad10a83ed542e23b0cbc21abb57b8fbe4dc0c4b682b4c3802bc495ff9Reviewing this PR
docs/upstream-merges/v1.17.4/ledger.mdis the review map — every non-trivial resolution is recorded there with its rationale. The sections added by this PR are the four## v1.17.2 batch N/4blocks, the milestone triage table, and the full-gate results.fork-register.mdcarries one row per fork surface;needs-wiring.mdcarries everything deliberately deferred.EIP-7708 — the three adaptations worth reviewer attention
Log ordering across Bor's two transfer paths. Upstream appends the log right after the balance change in
Transfer. Bor has two transfer functions and an early-return V2 BlockSTM fast path whose 0x1010LogTransferis generated later, at settlement. The emission was factored intoemitEthTransferLogand called at the same point relative to the balance change on all three paths (serial, V2 fast path,EthereumTransfer). Placing it afterAddTransferLogon the serial path — the natural reading of the diff — would order the pair one way under V1 and the other under V2, i.e. a receipt-root divergence the day Amsterdam is enabled.Selfdestruct without #32919. Upstream's burn/transfer branch reads
StateDB.IsNewContract, which arrives with the selfdestruct rework Bor declined. Rather than drop the branch, the signal is taken fromSelfDestruct6780's existing second return value (wasNewContract), which Bor's opcode handler was discarding. Verified across all four cases. Dropping it would not have been covered by the adoptedEmitLogsForBurnAccountssafety net — Bor zeroes the balance insideSelfDestruct6780, so the tx-boundary sweep skips the account and the burn log would vanish silently.ParallelStateDB.EmitLogsForBurnAccounts. Newvm.StateDBmethod; implemented against the parallel executor'sdestructedmap, address-sorted to match the serial executor's ordering.Enable-time consequence to note now: once Amsterdam activates, a plain value transfer on Bor emits both the EIP-7708 system log and Bor's existing 0x1010
LogTransfer. Bloom/receipt-size change, not a correctness problem — but a product decision that should be taken deliberately, not discovered at activation. Recorded infork-register.md.Executed tests
Full per-milestone gate at
682b4c380:go build ./...clean;go vet ./...clean bar the two pre-existing//nolintcopylocks.make lint(golangci-lint v2.11.4, repo config): 0 issues.gofmt -lclean;go mod tidyno-op.go test ./...: 148 packages pass. 4 fail, all pre-existing with no new test names —cmd/geth+cmd/devp2p/internal/ethtest(VEBLOP/non-Bor-genesis nil-deref),cmd/evm(t8n golden drift),core/vm(TestAbortDuringJumpinterrupt-timing flake). The latter two were re-baselined on a detached worktree at1abb57b8fand fail identically there.make test-integration:tests/borok, 628 s, 77.6% coverage. (Caveat recorded in the ledger: the./testsconsensus-fixture suite is a no-op in this repo —tests/testdatais absent and isn't a submodule, so those cases skip rather than run. Pre-existing.)smallpreset on an image built from this tip): 8/8 checks pass — ~1 s block production with validator/RPC in lockstep, 45–100 txs/block, 29StateSyncedlogs, span 3 active, checkpoint 6 submitted, zero errors in either client.EIP-7708 dormancy, proven on a live chain
The unit test proves the emission works with the gate forced on. The devnet proves it stays off. Over 50 blocks at 45–100 txs/block:
0xfffffffffffffffffffffffffffffffffffffffe(EIP-7708SystemAddress)0x0000000000000000000000000000000000001010(BorLogTransfer)The 2598 is what makes the 0 meaningful: thousands of value transfers ran through
core.Transferand none emitted a system log. A leaked gate, or a misplaced emission on any of the three transfer paths, would show a non-zero count.Milestone triage — one item needs a team decision
Beyond conflicts, all 77 upstream PRs were triaged for Bor wiring (a PR can merge perfectly clean and still leave Bor un-wired). Result: 1 needs wiring, 5 consensus-relevant (all dormant, in the register), 7 operator-visible but no wiring, 12 deferred, 52 inert.
The one: upstream's default cache bump 1024 → 4096 MB (#33836, #33975) merged clean but has no effect on
bor server.internal/cli/server/config.gocarries its ownCache: 1024with a 50/15/25/10 split whosecalcPercoverwrites all fourethconfigfields — and that split against 1024 MB reproduces geth's old defaults exactly (512/154/256/102), which is evidently how it was tuned. Socmd/gethand tests get upstream's new values while production Bor stays 4× below them.Parity is a one-line change (
Cache: 4096), deliberately not taken here — it quadruples the default per-node memory footprint, which is a PoS/devops call rather than a merge decision. pos-ops pinscacheper host in each BP'sbor/config.toml, so deployed BPs are unaffected either way; this only moves the default for operators who don't set it. Filed as a row inneeds-wiring.md.Deferred this milestone
Six whole-feature declines, each with rationale and a re-adoption path in
needs-wiring.md: #33931 (ExecuteConfig), #33816 (codedb/cachingDB), #33773 (miner OTel ctx-threading), #33648 (call-variant gas rework), #34036 (history-pruning policy), #33894 (history-import batching). Plus dependent declines that re-apply automatically when their parent lands: #34011, #34062, #34094, #33950, #33955, #33150.core/vm/{gas_table,operations_acl}.gonow carries four deferred upstream changes (#33281, #33637, #33450, #33648) plus blocked #32919, and batch 19 added a fifth entanglement — the EIP-7708 selfdestruct adaptation readsSelfDestruct6780's return value because #32919 is declined, so adopting #32919 later means revisiting that code rather than replacing it. Milestones 5 and 6 bring the gas-vector rework and gas budget into the same files. Worth a team discussion before v1.17.3.Rollout notes
IsAmsterdam, which is nil on every shipped preset. No fork block,params.Rulesfield, forkid input, chain preset or genesis file changed.eth_getProofkeys andeth_simulateV1blocks/calls, a newMaxUsedGasfield in theeth_simulateV1response,eth_getLogsnow erroring on an inverted block range instead of returning empty, andeth_createAccessListreturning[]rather thannullfor emptyStorageKeys.🤖 Generated with Claude Code