eth, eth/downloader: implement EIP-7975 (eth/70 partial receipt lists) - #2341
Draft
pratikspatil024 wants to merge 6 commits into
Draft
eth, eth/downloader: implement EIP-7975 (eth/70 partial receipt lists)#2341pratikspatil024 wants to merge 6 commits into
pratikspatil024 wants to merge 6 commits into
Conversation
Ports upstream #33153 (965bd6b) onto Bor's diverged receipt code. Batch 20 of the v1.17.4 sync reverted the feature's 31-file footprint wholesale, so this is a hand-written adoption rather than a merge, taken before batch 21 while the tree still matches the state upstream's commit expected. eth/70 lets a peer answer a receipt request with part of a block when the response would otherwise exceed the packet limit. The requester buffers the partial lists and resumes from the next index using the same request ID; nothing reaches the downloader until the block is whole again. The receipt-list encoding is unchanged from eth/69, so eth/70 reuses ReceiptList69 and only the packet envelope grows a truncation flag. Bor keeps eth/68, so ProtocolVersions becomes [ETH70, ETH69, ETH68] against upstream's [ETH70, ETH69]. Three adaptations to Bor: - The state-sync receipt is identified positionally, by absolute index, so the encoder keeps its loop counter absolute across a truncated response and advances the tx-type iterator in lockstep. A split therefore cannot displace it, which matters because the Madhugiri-gated receipt-root exclusion runs on the reassembled list in the downloader queue. - Upstream gates the reduced minimum transaction gas on AmsterdamTime. Bor has no Amsterdam timestamp, so this is rewired to the block-based IsAmsterdam and RequestReceipts carries block numbers where upstream carries timestamps. AmsterdamBlock is nil, so the pre-fork bound applies today. - A Bor block carries one receipt more than its gas can account for, because the state-sync transaction burns none, so the receipt-count bound applied to a truncated response gains a corresponding tolerance. Rather than clone blockReceiptsToNetwork69 for the bounded case, it gains a parameter whose zero value reproduces the previous whole-block behavior, which matches upstream's single parameterised function. The pre-Madhugiri state-sync merge is likewise extracted into gatherBlockReceipts and shared by both service functions instead of being duplicated. One deliberate behavior change follows from the first: a receipt with no corresponding body transaction is now an error instead of being encoded as type 0, reachable only from locally corrupt data, and the caller skips the block. The truncation path only fires on blocks whose receipts exceed the packet limit, which no devnet produces, so it is covered by unit tests over synthetic size limits rather than end to end. Upstream's cmd/devp2p ethtest suite for eth/70 is not ported; its regenerated testdata is specific to the geth test chain.
Brings the core/state goimports fix so lint passes on this branch too.
Handshake switches on the negotiated version to pick a status-packet format, and eth/70 was never added to it, so every eth/70 peer fell through to the default branch and was rejected with "unsupported protocol version". Since eth/70 is first in ProtocolVersions it is what two Bor nodes negotiate with each other, so in practice nothing peered at all: tests/bor panicked with "Node is not connected to any peers". EIP-7975 leaves the status message untouched, so eth/70 shares the eth/69 handshake. Upstream needs no branch here at all — it only has one because Bor still carries eth/68's older status packet, which is also why adding a version to ProtocolVersions does not fail to compile when this site is missed. The handshake tests enumerated 68 and 69 explicitly, which is why the unit suite stayed green; TestHandshake70 now covers the third.
params.TxGas is already a uint64, so the conversion added with the eth/70 partial receipt bounds was flagged by unconvert.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (37.03%) 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.3 #2341 +/- ##
==========================================================
Coverage ? 54.31%
==========================================================
Files ? 923
Lines ? 166617
Branches ? 0
==========================================================
Hits ? 90490
Misses ? 70414
Partials ? 5713
🚀 New features to boost your workflow:
|
This was referenced Aug 3, 2026
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 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
1 participant
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.
Summary
Adopts upstream #33153 (
965bd6b6a), EIP-7975 / eth/70 partial block receiptlists, which the previous PR in the stack deferred. This is a hand-written port
onto Bor's diverged receipt code, not a merge — which is exactly why it gets its
own PR rather than being folded into a merge batch.
eth/70 lets a peer answer a receipt request with part of a block when the
response would otherwise exceed the packet limit. The requester buffers the
partial lists in the peer and resumes from the next receipt index using the same
request ID; nothing reaches the downloader until the block is whole again.
Taken now, immediately after batch 20, so the tree still matches the state
upstream's commit expected — every further batch would have made this a
forward-port instead of an application.
[ETH70, ETH69, ETH68](Bor keeps eth/68, having declined #33511; upstream is[ETH70, ETH69])The deferral analysis was wrong, in our favour
The batch-20 note recorded a five-step adoption route and flagged a
consensus-relevant design decision as the blocker. Reading the code rather than
trusting the note, two steps evaporated and the blocker turned out not to exist:
ReceiptList70is needed. EIP-7975 changes the packet envelope, not thereceipt-list encoding, so eth/70 carries
ReceiptList69. Upstream likewisereuses its single
ReceiptListfor bothReceiptsPacket69andReceiptsPacket70.Bor never applies
ExcludeStateSyncReceipt()in the p2p handler.handleReceiptspasses a deliberately nilmetadatafunc because it has noblock number; the Madhugiri-gated exclusion runs later in the downloader queue
(
eth/downloader/bor_fetchers_concurrent_receipts.go→EncodeReceiptsAndPrepareHasher). Since partial lists are never sunk to thedownloader, the exclusion only ever sees a reassembled complete list —
unchanged from today.
Consensus-adjacent decisions — please review these first
TestPartialReceipts_StateSyncAcrossChunkBoundary, which splits every fixture at every size limit that splits it and asserts the reassembled receipt root on both sides of the Madhugiri gate.AmsterdamTime. Bor has no Amsterdam timestamp, so this is rewired to block-basedIsAmsterdam(number), andRequestReceiptscarries block numbers where upstream carries timestamps.AmsterdamBlockis nil, so the 21000 bound is what applies today; both sides of the gate are pinned byTestValidateLastBlockReceiptAmsterdamGate. No fork gate flipped.receipts <= gasUsed/minTxGas. A Bor block carries one receipt more than its gas can account for, because the state-sync transaction burns none, so the bound gained a+1tolerance. Without it, a peer serving a legitimate Bor block could be rejected.Deliberate scope beyond a straight port
Two existing eth/69 functions were refactored rather than cloned. Both came out
of running the Bor-only twin scan against my own diff, and both widen the eth/69
serving-path diff — flagging them because they are the parts most likely to want
splitting out.
blockReceiptsToNetwork69gained areceiptQueryParamsargument instead ofbeing cloned into a
…70. The first cut had the clone; it differed from itssibling only by the bounds, which is precisely the silent-drift pattern the twin
scan exists to catch. Upstream has one parameterised function, so this converges
rather than diverges. The zero value reproduces the previous whole-block
behaviour. One deliberate behaviour change on the eth/69 path, aligned with
upstream: a receipt with no corresponding body transaction is now an error
rather than being encoded as type 0 — reachable only from locally corrupt data,
and the caller skips the block.
gatherBlockReceiptsextracted fromServiceGetReceiptsQuery69. Thealternative was duplicating ~70 lines of pre-Madhugiri state-sync merge logic
into the eth/70 service function, i.e. a second new twin.
Divergences from upstream, kept on purpose
GetReceiptsPacketis not renamed to…69as upstream did, becausein Bor it serves both eth/68 and eth/69.
maxMessageSize(10 MiB) ratherthan adding upstream's duplicate
maxPacketSizeconstant.eth70handler map mirrors Bor'seth69map, which keepsNewBlockHashesMsg/NewBlockMsg; upstream's does not.dispatchRequestfails (upstream leaks it — its own TODO acknowledges theconcern), and
bufferReceiptsrejects a response claiming more blocks than wererequested before indexing into
gasUsed.Documentation
This PR also carries the v1.17.3 milestone documentation for batch 20 as well
as its own, since docs are excluded from the pure merge commits:
docs/upstream-merges/v1.17.4/{plan,ledger,fork-register,needs-wiring}.md.needs-wiring.md's EIP-7975 row movesdeferred→adoptedand records what theoriginal route got wrong;
fork-register.mdmovesdefer→adopt.Executed tests
New coverage in
eth/protocols/eth/receipt70_test.go:block, asserting the reassembled list is byte-identical to the whole-block
response in both network and storage encodings. Fails the test if no limit ever
splits, so it cannot silently go vacuous.
the Madhugiri gate.
firstIndexresumption,Append,LogsSize, bothresponse bounds, and the Amsterdam gate.
Plus an
ETH70case intestGetBlockReceiptscovering the full handler roundtrip and a resumed (
FirstBlockReceiptIndex: 1) request.Other gates, beyond CI's standard set:
go build ./...,go vet ./...— clean apart from the two pre-existing//nolintcopylocks;gofmt -lclean;go mod tidya no-op.go testgreen oneth/protocols/{eth,snap,wit},eth/downloader,eth/downloader/whitelist,eth,eth/fetcher.TestReinforceMultiClientPreCompilesTest,TestV2ForkParity.Known coverage gap, stated rather than hidden: the truncation path only fires
on blocks whose receipts exceed 10 MiB. No devnet produces those, so the split
behaviour is exercised by unit tests over synthetic size limits, not end to end.
It matters most on the largest blocks — plausibly more on Polygon than on L1 given
our gas limits and transaction counts. Upstream's
cmd/devp2p/internal/ethtesteth/70 suite and its regenerated testdata were not ported: the testdata is
specific to the geth test chain, and Bor's
ethtestis already a known-failingsurface.
Rollout notes
peer, not a state transition. The receipt root derivation path is untouched: the
Madhugiri-gated exclusion still runs on a complete list, in the downloader
queue, exactly as before.
older versions negotiate down and take the unchanged code paths.
AmsterdamBlockis nil on every Bor preset.Stacked PR — do not squash
Part of the v1.17.4 upstream sync stack. Squash-merging any PR in this stack
breaks every PR above it, because squashing rewrites commits into new SHAs, so
the PRs above would re-show all of this PR's changes and conflict against their
base. Team standard for upstream syncs is a merge commit, never squash.
Reviews are deliberately not being requested yet — the sync is mid-flight and
several decisions are still open. This will be marked ready once all six
milestones are complete.