Skip to content

core, eth, tests: v1.17.3 upstream merge, develop gap & remaining CI/UT fixes#4

Draft
zlacfzy wants to merge 76 commits into
developfrom
merge-v1.17.3_ci_fix
Draft

core, eth, tests: v1.17.3 upstream merge, develop gap & remaining CI/UT fixes#4
zlacfzy wants to merge 76 commits into
developfrom
merge-v1.17.3_ci_fix

Conversation

@zlacfzy

@zlacfzy zlacfzy commented Jul 2, 2026

Copy link
Copy Markdown
Owner

⚠️ DO NOT MERGE — tracking only. Tracks the gap between merge-v1.17.3_ci_fix and develop; real fixes land as the sub-PRs below.

Description

Tracking PR for the go-ethereum v1.17.3 upstream merge. 422b270876 unblocked the binary build (make all); this PR tracks the remaining test/CI breakage and the sub-PRs resolving it.

Status

On merge-v1.17.3_ci_fix the code is healthy: the unit-test, evm-test, golang-lint and truffle-test jobs are green (verified locally + in sub-PR CI). The two remaining red checks on this tracking PR are non-code:

  • build — the fork has no OSSINDEX_USERNAME / OSSINDEX_TOKEN secrets, so the Nancy Security Scan step fails (affects every PR on this fork, not the merge).
  • commitlint — a few historical commit headers exceed the 80-char limit.

Fixed (sub-PRs)

Note

The temporary CI merge-v1.17.3* branch triggers have been removed from the workflows (766e2e4bd9); these branches no longer auto-run CI.

zlacfzy and others added 4 commits July 2, 2026 10:09
…test after v1.17.3 merge

core/state/database_ubt.go: NoTries() now returns false instead of
panicking. It is called on the StateDB commit hot path
(updateStateObject/IntermediateRoot/commit); a unified binary trie always
maintains tries, so panicking there broke TestBinaryGenesisCommit and any
UBT genesis commit.

core/genesis_test.go:
- resolve leftover v1.17.3 merge conflict markers (keep BSC Chapel case,
  drop upstream Sepolia/Hoodi);
- rename TestGenesis_Commit -> TestGenesisCommit to match upstream;
- keep the total-difficulty assertion: unlike upstream (which removed TD
  entirely), BSC still writes TD at genesis (genesis.go) and reads it in
  production (headerchain.go), so the assertion still guards live behavior;
- remove TestConfigOrDefault: the inline config-defaulting it covered was
  reworked into params.GetBuiltInChainConfig (upstream #30907), leaving the
  test a tautological switch check.

core/state_prefetcher_test.go: remove TestPrefetchLeaking. The channel-send
goroutine leak it guarded (added in bnb-chain#1079) is structurally impossible in the
current Prefetch, which was rewritten to errgroup.Group + Wait(); the test
does not cover PrefetchMining (the method still using the manual
goroutine/channel pattern).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ructions tests

Both BenchmarkInterpreter (interpreter_test.go) and TestOpTstore
(instructions_test.go) still had `<<<<<<<`/`>>>>>>>` conflict markers, so the
entire core/vm test package failed to compile (`[setup failed]`).

Resolve contract construction to the current API: keep BSC's pooled
`GetContract` (used throughout evm.go) and adopt upstream's gas type change
(`GasBudget` via `NewGasBudget(...)`). The HEAD side still passed a bare
`uint64`, which no longer matches the `gas GasBudget` parameter.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…merge conflicts

Resolve the remaining `<<<<<<<`/`>>>>>>>` conflict markers in three test
packages left by the v1.17.3 merge:

* core/rawdb/accessors_chain_test.go: drop TestCanonicalHashIteration —
  ReadAllCanonicalHashes was removed upstream and no longer exists. Also
  restore the two ReadAllHashes edge-case assertions in TestHashesInRange that
  the resolution had dropped; upstream keeps them and ReadAllHashes still
  exists, so they still guard live behavior.
* core/state/statedb_test.go: TestDeleteStorage — combine both sides: keep
  BSC's 6-arg snapshot.New(..., cap, withoutTrie) and adopt upstream's
  NewMPTDatabase(tdb, nil).WithSnapshot(snaps) construction.
* triedb/pathdb/layertree_test.go: TestLookupZeroBaseRootFallback — both sides
  added this regression test for the zero-base-root sentinel fix; keep the more
  thorough version (fall-through account/storage, happy path, and stale-state
  cases) and drop the duplicate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
core/state, core: fix UBTDatabase.NoTries panic, clean up genesis_test
@zlacfzy zlacfzy changed the title Merge v1.17.3 ci fix v1.17.3 merge: develop gap & remaining CI/UT fixes (tracking, do not merge) Jul 2, 2026
zlacfzy and others added 16 commits July 2, 2026 16:31
core, triedb: resolve leftover v1.17.3 unit-test merge conflicts
Process() called postExecution() — which parses EIP-6110 deposit requests
from the block logs — while allLogs was still empty; allLogs was only
populated from the receipts *after* postExecution and Finalize. So during
block verification the deposit request was always dropped, yielding a wrong
requests hash (TestPragueRequests failed with local != remote, remote
carrying deposit+withdrawal+consolidation, local only withdrawal+consolidation).

Populate allLogs from the user-transaction receipts before postExecution
(matching upstream ordering), then append the system-transaction logs added
by Finalize so ProcessResult.Logs still carries the full set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* ethclient: resolve some ut conflicts

* worker: add is-not-in-bsc check to make ut succeed
… fork context

BSC's ValidateBlobTx is invoked with nil head/opts from the concurrent MEV
blob-validation paths (miner_mev, bid_simulator, bid_block) and from
TestValidateBlobTx_InvalidProof. The v1.17.3 merge grafted an
`opts.Config.IsOsaka(head.Number, head.Time)` sidecar-version check onto the top
of the function, which nil-dereferences for those callers and panics.

Gate that fork-version enforcement behind an `opts != nil && head != nil` check.
Without fork context the sidecar is still validated against its own declared
version by the fork-specific checks further down (restoring pre-merge behavior);
the real txpool path (validation.go:151, non-nil head/opts) keeps the check.

Additionally pin the expected sidecar version to V0 on BSC chains via
IsNotInBSC(): Osaka is already live on BSC mainnet/Chapel, and without the pin
the pool would demand version-1 (cell proof) sidecars — which BSC does not
support — rejecting every valid V0 blob transaction at admission.
TestValidateBlobTx_BSCOnlyV0PostOsaka pins this policy for both networks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r v1.17.3 merge

The v1.17.3 merge replaced the TypeMux downloader events with the downloader's
SyncEvent feed and rewired miner.update/VoteManager.loop through an inline
`eth.(interface{ Downloader() *downloader.Downloader })` assertion. That broke
the miner and vote test suites in three ways:

1. The test backends never implemented the asserted method, so every test
   panicked with an interface-conversion error. Narrow the dependency instead:
   miner.Backend and vote.Backend now declare
   `SubscribeSyncEvents(chan<- downloader.SyncEvent) event.Subscription`,
   implemented by *Ethereum (forwarding to the downloader) and by the test
   backends (event.Feed), which also makes sync events injectable from tests
   again (they previously posted to a TypeMux nothing listens to anymore).

2. worker.commit hard-cast `w.engine.(*parlia.Parlia).FinalizeAndAssemble`,
   panicking for the clique/ethash-based worker tests. Use core.AssembleBlock
   like generateWork already does: engines implementing FinalizeAndAssemble
   (parlia) keep the BSC path, others fall back to Finalize + NewBlock.

3. The merge dropped the one-shot semantics of miner.update: after the first
   successful sync the miner must stop reacting to downloader events (the
   documented DOS protection). Unsubscribe from the sync feed on
   SyncCompleted, restoring the pre-merge `events.Unsubscribe()` behavior.

With this, the miner package (excluding the two blob-validation cases fixed by
the ValidateBlobTx PR) and the core/vote package pass again.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rlp/ethapi: fix failed/conflict rlp/ethapi UTs
core: parse EIP-6110 deposit requests before Finalize in state processor
… context

Instead of skipping the sidecar-version check when head/opts are nil, default
the expectation to version 0 and only raise it to version 1 on non-BSC chains
past Osaka. The MEV blob-validation paths (miner_mev, bid_simulator,
bid_block) therefore now reject cell-proof (v1) sidecars outright, matching
the simulator's explicit "cell proof is not supported yet" stance and closing
the blind-signing gap where validateBidBlockBlobTxs would otherwise accept a
builder-supplied v1 sidecar into a block that the network's V0-only data
availability check then rejects (costing the proposer its slot).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
core/txpool: fix ValidateBlobTx nil-deref when called without fork context
testVotePool still posted downloader.SyncCompleted to a local TypeMux that
nothing subscribes to anymore, silently dropping coverage of the sync-event
path through VoteManager.loop (the test only kept passing because startVote
defaults to true). Give testBackend a retained sync feed and fire the event
through it, and drop the now-dead local mux and testBackend.EventMux.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
They restate the switch cases verbatim and still use the retired TypeMux
event terminology.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…PoSA engines (#10)

BSC overrides CurrentFinalBlock() to derive finality from the Parlia
fast-finality attestations, returning nil for any other engine. That made the
SetFinalized marker write-only outside PoSA — the engine API
(catalyst forkchoiceUpdated) and the syncer do call SetFinalized, but nothing
could read the result — and it broke the upstream regression test
TestSetHeadBeyondRootFinalizedBug (ethereum/go-ethereum#33486), whose setup
nil-panicked under its ethash engine.

Fall back to the marker (upstream semantics) when the engine is not PoSA.
Parlia chains are unaffected: the PoSA branch is taken unchanged. Non-PoSA
configurations regain upstream behavior, and the upstream test now passes
verbatim, keeping coverage for the setHeadBeyondRoot finalized-cleanup fix.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@will-2012
will-2012 marked this pull request as draft July 6, 2026 02:13
zlacfzy and others added 16 commits July 9, 2026 17:50
opDifficulty now returns PREVRANDAO when Context.Random is set (post-merge)
and DIFFICULTY otherwise, mirroring bsc-erigon's unified opDifficulty. Parlia
chains leave Context.Random nil (their block difficulty is non-zero, see
NewEVMBlockContext), so BSC keeps DIFFICULTY semantics byte-for-byte, while the
standard state tests set Random and observe PREVRANDAO.

This lets the Shanghai instruction set keep its London base on BSC without a
test patch swapping it to the Merge base. Combined with the earlier in-tree
precompile split (rules.IsInBSC), run-evm-tests.sh no longer applies ANY patch:
0001 is deleted (0002 was removed earlier).

Verified with zero patches applied: bcRandomBlockhashTest, stSStoreTest,
stRandom and the core/vm suite all pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Trim the opDifficulty comment and drop the now-stale "why no patch" notes
in run-evm-tests.sh (the test patches are gone).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Re-enable ci.go's downloadSpecTestFixtures (fixtures pinned to v5.1.0 in
build/checksums.txt, sha256-verified) and run the spec tests through
`ci.go test ./tests/` directly, matching upstream go-ethereum's CI. The
manual-wget-plus-source-patch wrapper is no longer needed now that BSC
precompiles are split in-tree (IsInBSC) and fixtures are back on v5.1.0.

- build/ci.go: uncomment the cachedir flag and the downloadSpecTestFixtures
  call that BSC had disabled while it relied on the wrapper.
- .github/workflows/evm-tests.yml: non-recursive submodule checkout so
  TestLegacyState self-skips (its Constantinople collision fixtures don't
  apply to BSC); run `ci.go test -timeout 1h ./tests/` (non-short, needed
  so fixtures download; 10m default timeout is too short for statetests).
- Remove tests/run-evm-tests.sh.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The four …ForBSC precompile maps kept upstream's pre-rename wording ("the
default set … used in the X release"), which no longer explained the ForBSC
suffix. State the convention once on the Istanbul map and reference it from
the others. Also tighten the two evm-tests.yml comments.

No behavior change (comments only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three independent unit-test failures surfaced by the go-ethereum v1.17.3
merge, none of them production bugs:

- cmd/devp2p ethtest: statusExchange replied only after breaking out of the
  read loop, so BSC's two-phase handshake68 (Status then UpgradeStatus)
  stalled on the node side and every TestEthSuite subtest hit EOF. Reply with
  our Status and only break after answering UpgradeStatusMsg.
- rlp/rlpgen pkgclash testdata referenced GetReceiptsPacket69, which does not
  exist (BSC ships only eth/68 and eth/70); point both the input and the
  expected output at GetReceiptsPacket68.
- cmd/geth db command registered dbInspectTrieCmd twice, so urfave/cli
  rejected the duplicate subcommand at startup and geth failed to launch
  (TestAttach). Drop the stray upstream-block entry, keep the BSC one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tests, core/vm: stabilize spec-tests after the v1.17.3 merge
…SC split

The precompile split repurposed PrecompiledContractsOsaka (and siblings)
to mean the standard, non-BSC set, moving the BSC set to
PrecompiledContractsOsakaForBSC. bnb-chain#3726's tests still assumed the old
meaning where those names were the BSC set:

- precompile_pasteur_test.go asserted 0x64/0x65/0x67 live in
  PrecompiledContractsOsaka; point it at PrecompiledContractsOsakaForBSC.
- contracts_lightclient_test.go built params.Rules{IsOsaka:true} and
  expected the BSC 0x67; add IsInBSC:true so activePrecompiledContracts
  selects the BSC set (otherwise 0x67 is absent -> nil-deref panic).

Production is unaffected: chainConfig.Rules() sets IsInBSC, so live
precompile selection was always correct; only these hand-built test
rules needed the flag.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…plit

The precompile split makes non-BSC chains (Parlia == nil — e.g. this
test's Ethash Hoodi config) expose the standard precompile set rather
than the BSC set. eth_config (EIP-7910) reports the active precompiles,
so the golden files had been listing the BSC-specific 0x64-0x69 and the
early 0x100 p256 on a non-BSC chain — which was exactly the pre-split
bug. Regenerate the golden to the correct standard set (matches upstream
Ethereum: no tendermint precompiles, p256 only from Osaka).

WRITE_TEST_FILES=1 go test ./internal/ethapi/ -run TestEIP7910Config

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tests, miner, cmd: fix failed ci actions
…mpile_tests

core/vm, internal/ethapi: fix tests broken by the ForBSC precompile split
The v1.17.3 merge branches no longer auto-run CI; workflows trigger only
on master/develop again. Removes the temporary '- merge-v1.17.3*' entry
from commit-lint, lint, integration-test, evm-tests, unit-test and
build-test workflows.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zlacfzy zlacfzy changed the title v1.17.3 merge: develop gap & remaining CI/UT fixes (tracking, do not merge) core, eth, tests: v1.17.3 upstream merge, develop gap & remaining CI/UT fixes Jul 10, 2026
Comment thread eth/downloader/downloader_test.go Outdated
defer tester.terminate()

chain := testChainBase.shorten(blockCacheMaxItems - 15)
peer := tester.newPeer("corrupt", eth.ETH69, chain.blocks[1:])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ETH69-->Eth68?

@allformless

Copy link
Copy Markdown

enable ETH70
and add TD field into struct StatusPacket, and stop supporting UpgradeStatusMsg like ETH68

The field lacked a toml omitempty tag, so dumpconfig/init-network always
wrote "MaxBlobsPerBlock = 0" even when unset (0 = protocol default), and
geth's strict TOML decoding makes binaries predating the field fatal on
such configs. Tag it omitempty like the other optional fields.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zlacfzy
zlacfzy force-pushed the merge-v1.17.3_ci_fix branch from 590f9ef to b48d947 Compare July 13, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants