From 0ed9568c499bbe6b2b6ba086f88b6bd69827d152 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Wed, 16 Sep 2026 02:19:03 -0400 Subject: [PATCH 01/10] =?UTF-8?q?eth/protocols/wit,=20eth:=20WIT2=20size?= =?UTF-8?q?=20oracle=20=E2=80=94=20accept=20non-deterministic=20witnesses?= =?UTF-8?q?=20within=20a=20signed=20size=20band?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Witnesses are not deterministic across nodes: BlockSTM speculative reads make honest nodes collect different-but-valid trie-node sets, so a valid witness routinely hashes differently from the BP-signed WitnessHash. WIT2's fetch-time verifyAgainstSignedHash treated any hash divergence as a fault (reject the bytes, strike the serving peer, fall back to WIT1 after two distinct servers), so a valid witness that hashes differently is rejected and its serving peer struck. Replace exact-hash equality with a size oracle: - Sign the witness size. SignedWitnessAnnouncement gains a WitnessSize field and the announce signing pre-image commits to it (keccak(domain || blockHash || blockNumber || witnessHash || witnessSize)). Producers set it from their own witness length. - Accept within a band for import. verifyAgainstSignedHash accepts for import any witness whose encoded size is <= min(3*signedSize, gas-derived absolute ceiling), regardless of hash; content-correctness is still arbitrated by import-time state-root execution. A within-band witness is re-served/relayed only when byte-identical to the BP's (hash match); a valid non-deterministic variant imports locally but is not re-served, so the signed hash stays a faithful identifier of the bytes on the serving/relay fast-path. Blame for content rests with the producer that signed the announcement, not a relaying or serving peer — preserving WIT2's property of relaying a trusted witness before self-validating. - Bound the size. A witness beyond the band is rejected and the serving peer struck (first occurrence per (peer, block), reusing the distinct-server bookkeeping); distinct servers exceeding the band for the same block fall back to WIT1. The retained gas-derived ceiling keeps the accepted size bounded even when the signed size is implausibly large. No WIT2 is deployed yet, so the signed-announce format changes in place. Scope: WIT2 signed-path only. The WIT1 page-count cross-peer verification is a separate change, handled in a follow-up. --- eth/fetcher/metrics.go | 14 +- eth/fetcher/witness_manager.go | 2 +- eth/fetcher/witness_manager_wit2.go | 154 ++++++++++------ eth/fetcher/witness_manager_wit2_test.go | 216 ++++++++++++++++++----- eth/handler_wit2.go | 19 +- eth/handler_wit2_caches_test.go | 10 +- eth/handler_wit2_test.go | 20 +-- eth/protocols/wit/protocol.go | 25 ++- eth/protocols/wit/protocol_wit2_test.go | 20 ++- 9 files changed, 341 insertions(+), 139 deletions(-) diff --git a/eth/fetcher/metrics.go b/eth/fetcher/metrics.go index d69315b95f..767c5b2b9a 100644 --- a/eth/fetcher/metrics.go +++ b/eth/fetcher/metrics.go @@ -32,9 +32,17 @@ var ( witnessVerifyPeersInsuffMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/verify/peers/insufficient", nil) witnessVerifyNoConsensusMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/verify/consensus/none", nil) - // witnessByteMismatchMeter tracks WIT2 byte-correctness drops: a serving - // peer delivered bytes whose keccak256 did not match the BP-signed hash. - witnessByteMismatchMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/byte_mismatch", nil) + // witnessOversizedMeter tracks WIT2 size-oracle rejections: a serving peer + // delivered a witness larger than the accepted band around the BP-signed + // WitnessSize. This is the only witness-content size limit enforced on the + // signed path; a differing hash within the band is tolerated. + witnessOversizedMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/oversized", nil) + + // witnessHashDivergenceMeter tracks accepted witnesses whose hash differed + // from the BP-signed WitnessHash but whose size was within band — expected + // under non-deterministic witness production. Observability only, NOT a + // drop/strike. + witnessHashDivergenceMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/hash_divergence", nil) // Witness page count metrics witnessPageCountBelowThresholdMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/pagecount/below_threshold", nil) diff --git a/eth/fetcher/witness_manager.go b/eth/fetcher/witness_manager.go index 3c4f233269..f564cbbceb 100644 --- a/eth/fetcher/witness_manager.go +++ b/eth/fetcher/witness_manager.go @@ -64,7 +64,7 @@ type cachedWitness struct { // if the encoded witness bytes don't hash to the signed witnessHash, the // serving peer lied and is dropped. If no signed announcement is on file // (e.g., WIT1-only fetch), the check is skipped. -type signedWitnessHashFn func(blockHash common.Hash) (witnessHash common.Hash, ok bool) +type signedWitnessHashFn func(blockHash common.Hash) (witnessHash common.Hash, witnessSize uint64, ok bool) // cacheWitnessForServingFn hands successfully-fetched witness bytes to the // network handler so peers can serve them pre-import. Called only after the diff --git a/eth/fetcher/witness_manager_wit2.go b/eth/fetcher/witness_manager_wit2.go index 206d3e4650..a40669b27f 100644 --- a/eth/fetcher/witness_manager_wit2.go +++ b/eth/fetcher/witness_manager_wit2.go @@ -47,84 +47,132 @@ func (m *witnessManager) cacheVerifiedWitnessForServing(blockHash common.Hash, b m.parentCacheWitnessForServing(blockHash, body, witnessHash) } -// verifyAgainstSignedHash returns the canonically-encoded witness bytes and -// the BP-signed witness hash they match, when a signed hash is on file and -// verification succeeds. body is nil on the WIT1 path (no signed hash to -// verify against) so callers can skip the pre-import serving cache. ok is -// false when verification fails; the offending peer has already been -// reported. Local EncodeRLP failure on a successfully-decoded witness is -// the local node's bug, not peer misbehavior, so it does not drop the peer. +// verifyAgainstSignedHash applies the WIT2 size oracle to a received witness. +// When a BP-signed announcement is on file, a witness whose encoded size is +// within the accepted band around the signed WitnessSize is accepted for import +// (ok=true) — because witnesses are non-deterministic, a differing hash is NOT a +// failure; only an oversized witness is rejected (and the serving peer struck). +// +// body (the canonical bytes for the pre-import serving cache) is returned ONLY +// when the witness is byte-identical to the BP's (hash match). A within-band but +// non-identical variant is imported locally but returns body=nil, so it is not +// re-served or relayed: the serving and relay fast-paths carry only the BP's own +// bytes, keyed by the signed hash, so a downstream byte check against that hash +// stays meaningful. +// +// body is also nil on the WIT1 path (no signed announcement). ok is false only +// when the witness is oversized or a local EncodeRLP failure occurs; the latter +// is the local node's own error, not a peer fault, so it does not strike. func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, witness *stateless.Witness) (body []byte, witnessHash common.Hash, ok bool) { if m.parentSignedWitnessHash == nil { return nil, common.Hash{}, true } - expected, has := m.parentSignedWitnessHash(hash) + expected, expectedSize, has := m.parentSignedWitnessHash(hash) if !has || m.isSignedHashQuarantined(hash) { - // No signed hash on file, or it has been quarantined after distinct - // servers repeatedly mismatched it (bad/stale producer hash): fall back - // to the WIT1 path so import-time execution arbitrates the bytes. + // No signed announcement on file, or it has been quarantined after + // distinct servers repeatedly served oversized bytes: fall back to the + // WIT1 path so import-time execution arbitrates the bytes. return nil, common.Hash{}, true } var buf bytes.Buffer if err := witness.EncodeRLP(&buf); err != nil { - log.Warn("[wm] Failed to encode received witness for hash check", "peer", peer, "hash", hash, "err", err) + log.Warn("[wm] Failed to encode received witness for size check", "peer", peer, "hash", hash, "err", err) m.handleWitnessFetchFailureExt(hash, "", fmt.Errorf("witness encode failed: %w", err), false) return nil, common.Hash{}, false } encoded := buf.Bytes() + actualSize := uint64(len(encoded)) actual := stateless.WitnessCommitHash(encoded) - if actual != expected { - witnessByteMismatchMeter.Mark(1) - // We cannot blame the byte-server on signed-hash disagreement alone: - // the announcement only proves *some* BP signed *some* hash. A faulty - // or malicious scheduled producer that signed a bogus hash would - // otherwise weaponise this path to disconnect every honest peer - // serving the canonical witness. Reject the bytes (don't cache for - // serving), back off the pending request so another peer/announcement - // gets tried, and let import-time execution validation pin blame. - // - // A single bad server is not enough to distrust the signed hash. But if - // distinct servers all mismatch the same signed hash, the hash itself is - // the likely culprit (bad/stale producer signature): quarantine it so - // the next fetch falls back to WIT1 immediately instead of stalling the - // block until the signed announcement's TTL expires. - quarantined, firstMismatchForPeer := m.recordSignedHashMismatch(hash, peer) + + // Non-determinism-tolerant acceptance (WIT2 size oracle). + // + // Witnesses are NOT deterministic across nodes: BlockSTM speculative reads + // make honest nodes collect different-but-valid trie-node sets, so a valid + // witness routinely hashes differently from the BP-signed WitnessHash. We + // therefore do NOT reject or strike on hash divergence. Instead the + // BP-signed WitnessSize is used as a size oracle: accept for import any + // witness whose encoded size is within acceptableWitnessSizeCeiling(signedSize) + // and let import-time state-root execution arbitrate content-correctness. + // Content-correctness is the responsibility of the producer that signed the + // announcement (via the header producer binding), not of a relaying or + // serving peer — preserving WIT2's core property of relaying/serving a + // trusted witness before self-validating it. A within-band witness is + // re-served/relayed only when it is byte-identical to the BP's (hash match); + // a valid non-deterministic variant is imported locally but not re-served, so + // the signed hash stays a faithful identifier of the bytes on the fast-path. + // + // Only an oversized witness — beyond the signed-size band and the retained + // gas-derived absolute cap — is rejected here, since it exceeds any + // plausible non-deterministic variation. The serving peer is struck (first + // occurrence per (peer, block), reusing the distinct-server bookkeeping); + // when distinct servers all oversize the same block the signed size is + // quarantined and the block falls back to the WIT1 page-count path. + ceiling := m.acceptableWitnessSizeCeiling(expectedSize) + if witnessSizeExceedsCeiling(actualSize, ceiling) { + witnessOversizedMeter.Mark(1) + quarantined, firstForPeer := m.recordSignedHashMismatch(hash, peer) if quarantined { - log.Warn("[wm] BP-signed witness hash repeatedly unmatched by distinct servers; quarantining to WIT1 fallback so the block can import", - "block", hash, "expected", expected) + log.Warn("[wm] BP-signed witness size band exceeded by distinct servers; quarantining to WIT1 fallback so the block can import", + "block", hash, "signedSize", expectedSize, "ceiling", ceiling) } else { - log.Warn("[wm] Witness bytes do not match BP-signed hash; not caching, retrying with another peer", - "peer", peer, "block", hash, "expected", expected, "actual", actual) + log.Warn("[wm] Witness exceeds BP-signed size band; not caching, retrying with another peer", + "peer", peer, "block", hash, "signedSize", expectedSize, "ceiling", ceiling, "received", actualSize) } - // Penalize the server. It returned a NON-EMPTY witness whose bytes - // contradict the on-file signed commitment — provably misbehaving relative - // to an honest empty "not ready" response. This is a STRIKE, not a drop: - // a faulty/malicious BP that signed a bogus hash makes honest servers - // mismatch too, so a single mismatch stays tolerated, but a sybil that - // repeatedly serves garbage (to weaponise the distinct-server quarantine as - // a targeted WIT1 downgrade, or to feed bytes that fail import) accrues - // toward disconnect instead of mismatching for free. Import-time execution - // remains the final arbiter of byte content. - // - // Strike only a peer's FIRST mismatch per block. The mismatch path keeps - // the pending request alive and reschedules ~gatherSlack later; when a - // block has a single announce-known peer, resolveWitnessFetchPeer returns - // that same peer on every retry, so striking each time would jail an honest - // sole witness source in ~1s — inverting the "single mismatch tolerated" - // guarantee. Per-(peer, block) dedup keeps the cross-block sybil penalty - // (distinct blocks each strike once) and the distinct-server quarantine - // intact while removing the self-DoS on sparse topologies. - if firstMismatchForPeer && peer != "" && m.parentStrikeWitnessServer != nil { + if firstForPeer && peer != "" && m.parentStrikeWitnessServer != nil { m.parentStrikeWitnessServer(peer) } - m.handleWitnessFetchFailureExt(hash, "", errors.New("witness hash mismatch"), false) + m.handleWitnessFetchFailureExt(hash, "", errors.New("witness exceeds signed size band"), false) return nil, common.Hash{}, false } - // Bytes matched: forget any earlier mismatch noise for this block. + + // Within band: forget any earlier oversize noise for this block. m.clearSignedHashMismatch(hash) + + if actual != expected { + // A valid, non-deterministic variant of the BP's witness. Accept it for + // import (state-root execution validates), but return body=nil so it is + // NOT cached for pre-import serving or relayed — those fast-paths carry + // only the BP's own bytes so a downstream check against the signed hash + // stays meaningful. Observability only. + witnessHashDivergenceMeter.Mark(1) + return nil, common.Hash{}, true + } + // Byte-identical to the BP's witness: safe to serve/relay under the signed hash. return encoded, expected, true } +// wit2SizeBandMultiplier bounds how many times the BP-signed witness size a +// received witness may reach before it is treated as oversized. Wide enough to +// absorb honest non-determinism (observed ±~9% node-set spread) with large +// margin, tight enough to reject gross bloat. Tune against the live inter-node +// size-spread distribution before hardening. +const wit2SizeBandMultiplier = 3 + +// bytesPerMiB matches the unit PageSize is expressed in (page size is 15 MiB). +const bytesPerMiB = 1024 * 1024 + +// acceptableWitnessSizeCeiling returns the maximum encoded witness byte size +// accepted for a block whose BP-signed witness size is signedSize. It is +// min(wit2SizeBandMultiplier*signedSize, absolute), where the absolute cap is +// the pre-existing gas-derived page ceiling expressed in bytes — retained so the +// accepted size stays bounded even when the signed size is implausibly large, +// and so a witness with no useful signed size still has a hard bound. +func (m *witnessManager) acceptableWitnessSizeCeiling(signedSize uint64) uint64 { + band := signedSize * wit2SizeBandMultiplier + absBytes := m.calculatePageThreshold() * maxPageSizeMB * bytesPerMiB + if absBytes == 0 { + return band + } + return min(band, absBytes) +} + +// witnessSizeExceedsCeiling reports whether an encoded witness of actualSize +// bytes is oversized relative to ceiling. A witness exactly at the ceiling is +// accepted; only a strictly larger one is rejected. +func witnessSizeExceedsCeiling(actualSize, ceiling uint64) bool { + return actualSize > ceiling +} + // signedHashMismatchQuarantineThreshold is how many DISTINCT servers must serve // bytes that fail the BP-signed-hash check for a block before we stop trusting // that signed hash and fall back to WIT1. One bad server cannot trigger it; a diff --git a/eth/fetcher/witness_manager_wit2_test.go b/eth/fetcher/witness_manager_wit2_test.go index 2f467ee0c5..06fc8a1894 100644 --- a/eth/fetcher/witness_manager_wit2_test.go +++ b/eth/fetcher/witness_manager_wit2_test.go @@ -60,6 +60,18 @@ func encodedCommitHash(t *testing.T, witness *stateless.Witness) common.Hash { return stateless.WitnessCommitHash(buf.Bytes()) } +// encodedSize returns the canonical RLP-encoded byte length of the witness — +// the value a BP would sign as WitnessSize for this witness. +func encodedSize(t *testing.T, witness *stateless.Witness) uint64 { + t.Helper() + + var buf bytes.Buffer + if err := witness.EncodeRLP(&buf); err != nil { + t.Fatalf("encode: %v", err) + } + return uint64(buf.Len()) +} + // requireNoDroppedPeers fails the test when any peer was drop-disconnected. func requireNoDroppedPeers(t *testing.T, tw *testWitnessManager, context string) { t.Helper() @@ -71,19 +83,15 @@ func requireNoDroppedPeers(t *testing.T, tw *testWitnessManager, context string) } } -// TestProcessWitnessResponseDoesNotDropOnByteMismatch encodes the post- -// adversarial-review safety policy: when the served witness bytes do not -// match the BP-signed witnessHash on file, the manager must back off and -// retry, but it MUST NOT drop the byte-server. The accepted announcement -// only proves *some* BP signed *some* hash — not that the hash matches the -// canonical witness. A faulty or malicious scheduled producer that signs a -// bogus hash would otherwise weaponise this code path to disconnect every -// honest peer serving the real witness. -// -// The mismatched bytes are still rejected (not cached for serving), and the -// pending state stays alive with a fresh back-off so another peer (or another -// announcement) gets a chance. Blame-pinning belongs at execution time, where -// import-side validation can attribute fault to signer vs. server vs. caller. +// TestProcessWitnessResponseDoesNotDropOnByteMismatch encodes the +// non-determinism-tolerant policy: a served witness whose bytes hash +// differently from the BP-signed WitnessHash is NOT a fault — witnesses vary +// between honest nodes (BlockSTM speculative reads), so a differing hash within +// the signed size band is a valid witness. The manager must neither drop NOR +// strike the server for a within-band hash divergence; import-time state-root +// execution arbitrates content-correctness, which is the responsibility of the +// producer that signed the announcement, not the serving peer. Only an oversized +// witness (beyond the size band) is rejected — covered separately. func TestProcessWitnessResponseDoesNotDropOnByteMismatch(t *testing.T) { tw := newTestWitnessManager() defer tw.Close() @@ -99,18 +107,25 @@ func TestProcessWitnessResponseDoesNotDropOnByteMismatch(t *testing.T) { // processWitnessResponse will see canonical bytes whose hash does not // match what parentSignedWitnessHash reports. rogueSignedHash := common.HexToHash("0xdeadbeef") - tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, bool) { + signedSize := encodedSize(t, canonical) + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { if h == hash { - return rogueSignedHash, true + return rogueSignedHash, signedSize, true } - return common.Hash{}, false + return common.Hash{}, 0, false } + struck := 0 + tw.manager.parentStrikeWitnessServer = func(string) { struck++ } + primePendingWitness(tw, "honest", block) tw.manager.processWitnessResponse("honest-server", hash, witnessResponse(canonical), time.Now()) - requireNoDroppedPeers(t, tw, "byte-server must not be dropped on signed-hash mismatch (BP may have signed bogus)") + requireNoDroppedPeers(t, tw, "byte-server must not be dropped on signed-hash divergence (witnesses are non-deterministic)") + if struck != 0 { + t.Fatalf("a witness within the signed size band must not strike the server despite a differing hash; got %d strikes", struck) + } } // TestProcessWitnessResponseAcceptsMatchingHash is the contrapositive: a @@ -125,8 +140,8 @@ func TestProcessWitnessResponseAcceptsMatchingHash(t *testing.T) { witness := createTestWitnessForBlock(block) matchingHash := encodedCommitHash(t, witness) - tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, bool) { - return matchingHash, true + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { + return matchingHash, encodedSize(t, witness), true } primePendingWitness(tw, "honest", block) @@ -161,11 +176,11 @@ func TestProcessWitnessResponseCachesForServingAfterByteCheck(t *testing.T) { gotBytes = append([]byte{}, witnessBytes...) gotHash = witnessHash } - tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, bool) { + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { if h == hash { - return want, true + return want, encodedSize(t, witness), true } - return common.Hash{}, false + return common.Hash{}, 0, false } primePendingWitness(tw, "honest", block) @@ -195,8 +210,8 @@ func TestProcessWitnessResponseSkipsCheckWhenNoSignature(t *testing.T) { witness := createTestWitnessForBlock(block) // No lookup configured → skip path. - tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, bool) { - return common.Hash{}, false + tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, uint64, bool) { + return common.Hash{}, 0, false } primePendingWitness(tw, "wit1-peer", block) @@ -227,8 +242,8 @@ func TestVerifyAgainstSignedHashSkipsEncodeWhenNoSignedHash(t *testing.T) { } // No signed hash on file for any block → verification must return // body=nil so the caller skips the cache. - tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, bool) { - return common.Hash{}, false + tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, uint64, bool) { + return common.Hash{}, 0, false } body, _, ok := tw.manager.verifyAgainstSignedHash("peer1", hash, witness) @@ -328,13 +343,14 @@ func TestSignedHashQuarantineAfterDistinctMismatches(t *testing.T) { hash := block.Hash() witness := createTestWitnessForBlock(block) - // Signed hash on file does NOT match the canonical witness — the bad/stale - // producer-hash case. Every server serving the real witness will mismatch. - tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, bool) { + // Signed size is tiny, so the canonical witness is oversized against the + // band for every server that serves it — the size-oracle analog of the + // bad/stale producer case. Distinct oversizing servers quarantine the hash. + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { if h == hash { - return common.HexToHash("0xbadbadbad"), true + return common.HexToHash("0xbadbadbad"), 1, true } - return common.Hash{}, false + return common.Hash{}, 0, false } primePendingWitness(tw, "peerA", block) @@ -467,12 +483,12 @@ func TestVerifyAgainstSignedHashStrikesNonEmptyMismatchServer(t *testing.T) { hash := block.Hash() witness := createTestWitnessForBlock(block) - // Signed hash on file does not match the served (canonical) bytes. - tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, bool) { + // Signed size is tiny → the served (canonical) witness is oversized. + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { if h == hash { - return common.HexToHash("0xdeadbeef"), true + return common.HexToHash("0xdeadbeef"), 1, true } - return common.Hash{}, false + return common.Hash{}, 0, false } var struck []string @@ -481,10 +497,10 @@ func TestVerifyAgainstSignedHashStrikesNonEmptyMismatchServer(t *testing.T) { } if _, _, ok := tw.manager.verifyAgainstSignedHash("sybil-server", hash, witness); ok { - t.Fatal("non-empty byte mismatch must return ok=false") + t.Fatal("oversized witness must return ok=false") } if len(struck) != 1 || struck[0] != "sybil-server" { - t.Fatalf("a server that served non-empty mismatching bytes must be struck; got strikes=%v", struck) + t.Fatalf("a server that served an oversized witness must be struck; got strikes=%v", struck) } // Must NOT also drop the peer (drop would let a bad BP hash disconnect honest @@ -503,8 +519,8 @@ func TestVerifyAgainstSignedHashDoesNotStrikeOnWit1Path(t *testing.T) { hash := block.Hash() witness := createTestWitnessForBlock(block) - tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, bool) { - return common.Hash{}, false + tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, uint64, bool) { + return common.Hash{}, 0, false } struck := 0 tw.manager.parentStrikeWitnessServer = func(string) { struck++ } @@ -527,8 +543,8 @@ func TestSignedHashSingleServerDoesNotQuarantine(t *testing.T) { hash := block.Hash() witness := createTestWitnessForBlock(block) - tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, bool) { - return common.HexToHash("0xdeadbeef"), true + tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, uint64, bool) { + return common.HexToHash("0xdeadbeef"), 1, true } primePendingWitness(tw, "lonely", block) @@ -556,8 +572,8 @@ func TestVerifyAgainstSignedHashStrikesSolePeerOncePerBlock(t *testing.T) { hash := block.Hash() witness := createTestWitnessForBlock(block) - tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, bool) { - return common.HexToHash("0xdeadbeef"), true + tw.manager.parentSignedWitnessHash = func(common.Hash) (common.Hash, uint64, bool) { + return common.HexToHash("0xdeadbeef"), 1, true } struck := 0 tw.manager.parentStrikeWitnessServer = func(string) { struck++ } @@ -631,3 +647,117 @@ func TestEmptyResponseBackoff(t *testing.T) { t.Fatalf("large n must clamp to max %v, got %v", emptyResponseMaxBackoff, d) } } + +// TestVerifyAgainstSignedHashAcceptsDivergentHashWithinBand is the core +// non-determinism-tolerance property: a witness whose bytes hash DIFFERENTLY +// from the BP-signed WitnessHash but whose size is within the band around the +// signed WitnessSize must be ACCEPTED FOR IMPORT (ok=true), not rejected and not +// struck. It must NOT be returned for the serving cache (body=nil): only a +// byte-identical (hash-matching) witness is re-served/relayed, so the serving +// and relay fast-paths carry only the BP's own bytes. Import-time state-root +// execution is the content-correctness arbiter; a differing hash is expected +// because honest nodes produce different-but-valid witnesses. +func TestVerifyAgainstSignedHashAcceptsDivergentHashWithinBand(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + + block := createTestBlock(308) + hash := block.Hash() + witness := createTestWitnessForBlock(block) + + // BP signed a different hash (non-deterministic divergence) but a size the + // received witness is within band of. + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { + if h == hash { + return common.HexToHash("0xd1ffe7e17"), encodedSize(t, witness), true + } + return common.Hash{}, 0, false + } + struck := 0 + tw.manager.parentStrikeWitnessServer = func(string) { struck++ } + + body, _, ok := tw.manager.verifyAgainstSignedHash("honest-diverging", hash, witness) + if !ok { + t.Fatal("a witness within the signed size band must be accepted for import despite a differing hash") + } + if body != nil { + t.Fatal("a non-identical within-band witness must import but NOT be cached for serving (body=nil), so the fast-path carries only the BP's bytes") + } + if struck != 0 { + t.Fatalf("within-band divergence must not strike the server; got %d strikes", struck) + } + if tw.manager.isSignedHashQuarantined(hash) { + t.Fatal("within-band divergence must not quarantine the signed size") + } +} + +// TestVerifyAgainstSignedHashServesOnExactMatch is the contrapositive of the +// above: a witness byte-identical to the BP's (hash match) within the band is +// accepted AND returned as body for the pre-import serving cache, so the BP's +// own bytes propagate on the fast-path. +func TestVerifyAgainstSignedHashServesOnExactMatch(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + + block := createTestBlock(309) + hash := block.Hash() + witness := createTestWitnessForBlock(block) + match := encodedCommitHash(t, witness) + + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { + if h == hash { + return match, encodedSize(t, witness), true + } + return common.Hash{}, 0, false + } + + body, gotHash, ok := tw.manager.verifyAgainstSignedHash("honest-matching", hash, witness) + if !ok { + t.Fatal("a byte-identical within-band witness must be accepted") + } + if body == nil { + t.Fatal("a byte-identical witness must return canonical bytes for the serving cache") + } + if gotHash != match { + t.Fatalf("served witness hash must be the signed hash; got %s want %s", gotHash.Hex(), match.Hex()) + } +} + +// TestAcceptableWitnessSizeCeiling pins the two-tier ceiling: the dynamic +// working bound is wit2SizeBandMultiplier*signedSize, clamped by the retained +// gas-derived absolute cap so an implausibly large signed size cannot lift the +// band. +func TestAcceptableWitnessSizeCeiling(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + + abs := tw.manager.calculatePageThreshold() * maxPageSizeMB * bytesPerMiB + + // A small signed size → the band (3*S) dominates, well under the absolute. + if got := tw.manager.acceptableWitnessSizeCeiling(1000); got != wit2SizeBandMultiplier*1000 { + t.Fatalf("band = %d*signedSize expected %d, got %d", wit2SizeBandMultiplier, wit2SizeBandMultiplier*1000, got) + } + // A signed size whose 3x band would exceed the absolute must clamp to it. + if got := tw.manager.acceptableWitnessSizeCeiling(abs); got != abs { + t.Fatalf("band exceeding the absolute must clamp to %d, got %d", abs, got) + } + // An implausibly large signed size must still clamp to the absolute. + if got := tw.manager.acceptableWitnessSizeCeiling(abs * 10); got != abs { + t.Fatalf("implausibly large signed size must clamp to absolute %d, got %d", abs, got) + } +} + +// TestWitnessSizeExceedsCeiling pins the accept/reject boundary of the size +// oracle: a witness exactly at the ceiling is accepted (not oversized), and one +// byte over is rejected. +func TestWitnessSizeExceedsCeiling(t *testing.T) { + if witnessSizeExceedsCeiling(100, 100) { + t.Fatal("a witness exactly at the ceiling must be accepted (not oversized)") + } + if !witnessSizeExceedsCeiling(101, 100) { + t.Fatal("a witness one byte over the ceiling must be rejected as oversized") + } + if witnessSizeExceedsCeiling(99, 100) { + t.Fatal("a witness below the ceiling must be accepted") + } +} diff --git a/eth/handler_wit2.go b/eth/handler_wit2.go index b13d005d91..4113604d19 100644 --- a/eth/handler_wit2.go +++ b/eth/handler_wit2.go @@ -138,7 +138,7 @@ func verifySignedAnnouncement(ann wit.SignedWitnessAnnouncement) (common.Address if len(ann.Signature) != wit.SignatureLength { return common.Address{}, errInvalidSignatureLength } - digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) // Normalize the recovery id to 0/1 before recovery. External signers (Clef) // return V in 27/28 form for any mimetype other than Clique — see // accounts/external.SignData, which only de-offsets MimetypeClique — and @@ -198,12 +198,12 @@ func (h *handler) cosendWitnessAnnouncement(blockHash common.Hash, blockNumber u // lookupSignedWitnessHash returns the BP-signed witness hash for a block, if // the local cache has a verified announcement. Used by the witness manager // on fetch success to verify byte-correctness against the signed commitment. -func (h *handler) lookupSignedWitnessHash(blockHash common.Hash) (common.Hash, bool) { +func (h *handler) lookupSignedWitnessHash(blockHash common.Hash) (common.Hash, uint64, bool) { ann, ok := h.signedWitnesses.get(blockHash) if !ok { - return common.Hash{}, false + return common.Hash{}, 0, false } - return ann.WitnessHash, true + return ann.WitnessHash, ann.WitnessSize, true } // cacheVerifiedWitnessForServing receives canonical-encoded witness bytes from @@ -265,11 +265,11 @@ func (h *handler) signLocalWitnessAnnouncement(blockHash common.Hash, blockNumbe return wit.SignedWitnessAnnouncement{}, false } - witnessHash, ok := h.canonicalWitnessHash(blockHash) + witnessHash, witnessSize, ok := h.canonicalWitnessHash(blockHash) if !ok { return wit.SignedWitnessAnnouncement{}, false } - preimage := wit.WitnessAnnouncementSigningPreImage(blockHash, blockNumber, witnessHash) + preimage := wit.WitnessAnnouncementSigningPreImage(blockHash, blockNumber, witnessHash, witnessSize) _, sig, err := borEngine.SignBytes(accounts.MimetypeBorWitnessAnnounce, preimage) if err != nil { log.Warn("wit2: failed to sign witness announcement", "blockHash", blockHash, "err", err) @@ -287,6 +287,7 @@ func (h *handler) signLocalWitnessAnnouncement(blockHash common.Hash, blockNumbe BlockHash: blockHash, BlockNumber: blockNumber, WitnessHash: witnessHash, + WitnessSize: witnessSize, Signature: sig, } // Honor the cache's conflict decision. We reach here only when the early @@ -322,12 +323,12 @@ func maySignAnnouncementForBlock(borEngine *bor.Bor, header *types.Header, local // written witness blob is canonical at write time and can be hashed directly // without a decode/re-encode round-trip — saving roughly the cost of one RLP // pass on the announce path. Returns (_, false) when no witness is on file. -func (h *handler) canonicalWitnessHash(blockHash common.Hash) (common.Hash, bool) { +func (h *handler) canonicalWitnessHash(blockHash common.Hash) (common.Hash, uint64, bool) { stored := h.chain.GetWitness(blockHash) if len(stored) == 0 { - return common.Hash{}, false + return common.Hash{}, 0, false } - return stateless.WitnessCommitHash(stored), true + return stateless.WitnessCommitHash(stored), uint64(len(stored)), true } // isScheduledProducer binds the recovered signer of a wit2 announcement to the diff --git a/eth/handler_wit2_caches_test.go b/eth/handler_wit2_caches_test.go index d09db3a170..0b81a12db1 100644 --- a/eth/handler_wit2_caches_test.go +++ b/eth/handler_wit2_caches_test.go @@ -474,10 +474,10 @@ func TestCosendWitnessAnnouncementVersionSplit(t *testing.T) { // lookupSignedWitnessHash round-trip: hit for the cached announce, miss // for an unknown hash. - got, ok := h.handler.lookupSignedWitnessHash(hash) + got, _, ok := h.handler.lookupSignedWitnessHash(hash) require.True(t, ok) require.Equal(t, common.HexToHash("0xc0de"), got) - _, ok = h.handler.lookupSignedWitnessHash(common.HexToHash("0xabsent")) + _, _, ok = h.handler.lookupSignedWitnessHash(common.HexToHash("0xabsent")) require.False(t, ok) // Re-cosend via the static/trusted list: both peers now know the witness, @@ -562,7 +562,7 @@ func signTestAnnouncement(t *testing.T, ann *wit.SignedWitnessAnnouncement) { key, err := crypto.GenerateKey() require.NoError(t, err) - digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) sig, err := crypto.Sign(digest.Bytes(), key) require.NoError(t, err) ann.Signature = sig @@ -1063,12 +1063,12 @@ func TestCanonicalWitnessHashStorageGate(t *testing.T) { defer h.close() hash := common.HexToHash("0x4242") - _, ok := h.handler.canonicalWitnessHash(hash) + _, _, ok := h.handler.canonicalWitnessHash(hash) require.False(t, ok, "absent witness must yield no commitment") body := []byte{0x01, 0x02, 0x03} rawdb.WriteWitness(h.chain.DB(), hash, body) - got, ok := h.handler.canonicalWitnessHash(hash) + got, _, ok := h.handler.canonicalWitnessHash(hash) require.True(t, ok) require.Equal(t, stateless.WitnessCommitHash(body), got) } diff --git a/eth/handler_wit2_test.go b/eth/handler_wit2_test.go index 4b42f8d4b4..bdc776585b 100644 --- a/eth/handler_wit2_test.go +++ b/eth/handler_wit2_test.go @@ -96,7 +96,7 @@ func TestVerifySignedAnnouncementRoundTrip(t *testing.T) { BlockNumber: 42, WitnessHash: common.HexToHash("0xc0ffee00"), } - digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) sig, err := crypto.Sign(digest.Bytes(), key) if err != nil { t.Fatalf("sign: %v", err) @@ -129,7 +129,7 @@ func TestVerifySignedAnnouncementNormalizesLegacyV(t *testing.T) { BlockNumber: 42, WitnessHash: common.HexToHash("0xc0ffee00"), } - digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) sig, err := crypto.Sign(digest.Bytes(), key) if err != nil { t.Fatalf("sign: %v", err) @@ -174,7 +174,7 @@ func TestVerifySignedAnnouncementWalletSemantics(t *testing.T) { WitnessHash: common.HexToHash("0xcd"), } // Production wallet path: SignData hashes its input once, then signs. - preimage := wit.WitnessAnnouncementSigningPreImage(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + preimage := wit.WitnessAnnouncementSigningPreImage(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) walletDigest := crypto.Keccak256(preimage) sig, err := crypto.Sign(walletDigest, key) if err != nil { @@ -208,7 +208,7 @@ func TestVerifySignedAnnouncementDetectsTampering(t *testing.T) { BlockNumber: 7, WitnessHash: common.HexToHash("0xb2"), } - digest := wit.WitnessAnnouncementSigningHash(original.BlockHash, original.BlockNumber, original.WitnessHash) + digest := wit.WitnessAnnouncementSigningHash(original.BlockHash, original.BlockNumber, original.WitnessHash, original.WitnessSize) sig, err := crypto.Sign(digest.Bytes(), key) if err != nil { t.Fatalf("sign: %v", err) @@ -731,7 +731,7 @@ func TestCanonicalWitnessHashUsesStoredBytesDirectly(t *testing.T) { canonical := encodeWitnessForTest(t, w) rawdb.WriteWitness(h.chain.DB(), hash, canonical) - got, ok := h.handler.canonicalWitnessHash(hash) + got, _, ok := h.handler.canonicalWitnessHash(hash) require.True(t, ok) want := stateless.WitnessCommitHash(canonical) @@ -878,7 +878,7 @@ func TestDeferredSignedAnnounceDrainedAfterHeaderArrives(t *testing.T) { BlockNumber: header.Number.Uint64(), WitnessHash: common.HexToHash("0xc0ffee01"), } - digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) sig, err := crypto.Sign(digest.Bytes(), key) if err != nil { t.Fatalf("sign: %v", err) @@ -939,7 +939,7 @@ func TestDeferredDrainPromotesHonestAmongForged(t *testing.T) { key, err := crypto.GenerateKey() require.NoError(t, err) a := wit.SignedWitnessAnnouncement{BlockHash: blockHash, BlockNumber: num, WitnessHash: wh} - d := wit.WitnessAnnouncementSigningHash(a.BlockHash, a.BlockNumber, a.WitnessHash) + d := wit.WitnessAnnouncementSigningHash(a.BlockHash, a.BlockNumber, a.WitnessHash, a.WitnessSize) s, err := crypto.Sign(d.Bytes(), key) require.NoError(t, err) a.Signature = s @@ -989,7 +989,7 @@ func TestDrainDeferredCandidateBranches(t *testing.T) { key, err := crypto.GenerateKey() require.NoError(t, err) a := wit.SignedWitnessAnnouncement{BlockHash: blockHash, BlockNumber: number, WitnessHash: wh} - d := wit.WitnessAnnouncementSigningHash(a.BlockHash, a.BlockNumber, a.WitnessHash) + d := wit.WitnessAnnouncementSigningHash(a.BlockHash, a.BlockNumber, a.WitnessHash, a.WitnessSize) s, err := crypto.Sign(d.Bytes(), key) require.NoError(t, err) a.Signature = s @@ -1053,7 +1053,7 @@ func TestDrainDeferredCandidateStrikesConfirmedForgery(t *testing.T) { BlockNumber: header.Number.Uint64() + 1, WitnessHash: common.HexToHash("0xbadbad"), } - d := wit.WitnessAnnouncementSigningHash(forged.BlockHash, forged.BlockNumber, forged.WitnessHash) + d := wit.WitnessAnnouncementSigningHash(forged.BlockHash, forged.BlockNumber, forged.WitnessHash, forged.WitnessSize) sig, err := crypto.Sign(d.Bytes(), key) require.NoError(t, err) forged.Signature = sig @@ -1099,7 +1099,7 @@ func TestDrainResolvedDeferredAnnouncesCoversBatchedImport(t *testing.T) { BlockNumber: header.Number.Uint64(), WitnessHash: common.BytesToHash([]byte{0xc0, byte(i)}), } - d := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash) + d := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) s, err := crypto.Sign(d.Bytes(), key) require.NoError(t, err) ann.Signature = s diff --git a/eth/protocols/wit/protocol.go b/eth/protocols/wit/protocol.go index 101f4fce62..b53b3f8143 100644 --- a/eth/protocols/wit/protocol.go +++ b/eth/protocols/wit/protocol.go @@ -119,18 +119,26 @@ type NewWitnessHashesPacket struct { // SignedWitnessAnnouncement is a BP-authenticated commitment to the existence // of a specific witness for a specific block. The signer commits to: // -// keccak256(BlockHash || BlockNumber || WitnessHash) +// keccak256(BlockHash || BlockNumber || WitnessHash || WitnessSize) // // Receivers verify the signature with ecrecover and check that the recovered // address is the validator scheduled for BlockNumber. Once verified, the // announcement is safe to relay to other peers without local execution; any -// downstream receiver re-verifies independently. Bytes returned by a serving -// peer are checked against WitnessHash, so byte-correctness blame attaches to -// the server while content-correctness (state-root) blame attaches to the BP. +// downstream receiver re-verifies independently. +// +// WitnessHash and WitnessSize describe the producer's own witness. Because +// witnesses are NOT deterministic across nodes (BlockSTM speculative reads +// yield different-but-valid node sets), a serving peer's bytes need not hash +// to WitnessHash; instead WitnessSize is used as a size oracle — a receiver +// accepts a witness whose size is within a band of WitnessSize and lets +// import-time state-root execution arbitrate content-correctness, which is the +// responsibility of the producer that signed the announcement, not of a +// relaying or serving peer. type SignedWitnessAnnouncement struct { BlockHash common.Hash BlockNumber uint64 WitnessHash common.Hash // WIT2 chunked-aggregate commitment over canonical witness RLP; see core/stateless.WitnessCommitHash + WitnessSize uint64 // producer's witness size in bytes; used as a non-deterministic-tolerant size oracle Signature []byte // 65-byte secp256k1 signature } @@ -187,13 +195,14 @@ func (w *SignedNewWitnessHashesPacket) Kind() byte { return SignedNewWitnessHa // independently computes WitnessAnnouncementSigningHash (= keccak256 of this // preimage) and ecrecovers against it. Mismatching hash-vs-preimage between // signer and verifier silently breaks every WIT2 signature, hence the split. -func WitnessAnnouncementSigningPreImage(blockHash common.Hash, blockNumber uint64, witnessHash common.Hash) []byte { - const fixedLen = common.HashLength + 8 + common.HashLength +func WitnessAnnouncementSigningPreImage(blockHash common.Hash, blockNumber uint64, witnessHash common.Hash, witnessSize uint64) []byte { + const fixedLen = common.HashLength + 8 + common.HashLength + 8 buf := make([]byte, len(witnessAnnounceDomainTag)+fixedLen) n := copy(buf, witnessAnnounceDomainTag) copy(buf[n:], blockHash[:]) binary.BigEndian.PutUint64(buf[n+common.HashLength:], blockNumber) copy(buf[n+common.HashLength+8:], witnessHash[:]) + binary.BigEndian.PutUint64(buf[n+common.HashLength+8+common.HashLength:], witnessSize) return buf } @@ -201,8 +210,8 @@ func WitnessAnnouncementSigningPreImage(blockHash common.Hash, blockNumber uint6 // a witness announcement. Must be byte-identical on both signer and verifier. // Used by the verifier; signers must instead feed the preimage into the wallet // SignData path, which keccaks once internally. -func WitnessAnnouncementSigningHash(blockHash common.Hash, blockNumber uint64, witnessHash common.Hash) common.Hash { - return crypto.Keccak256Hash(WitnessAnnouncementSigningPreImage(blockHash, blockNumber, witnessHash)) +func WitnessAnnouncementSigningHash(blockHash common.Hash, blockNumber uint64, witnessHash common.Hash, witnessSize uint64) common.Hash { + return crypto.Keccak256Hash(WitnessAnnouncementSigningPreImage(blockHash, blockNumber, witnessHash, witnessSize)) } func (w *GetWitnessMetadataRequest) Name() string { return "GetWitnessMetadata" } diff --git a/eth/protocols/wit/protocol_wit2_test.go b/eth/protocols/wit/protocol_wit2_test.go index c2ad7f33a9..219314e921 100644 --- a/eth/protocols/wit/protocol_wit2_test.go +++ b/eth/protocols/wit/protocol_wit2_test.go @@ -15,15 +15,18 @@ func TestWitnessAnnouncementSigningHashStable(t *testing.T) { blockHash := common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111") blockNumber := uint64(0x0102030405060708) witnessHash := common.HexToHash("0x2222222222222222222222222222222222222222222222222222222222222222") + witnessSize := uint64(0x1112131415161718) - got := WitnessAnnouncementSigningHash(blockHash, blockNumber, witnessHash) + got := WitnessAnnouncementSigningHash(blockHash, blockNumber, witnessHash, witnessSize) - // Manual recomposition: domain-tag || blockHash || blockNumber (big-endian u64) || witnessHash + // Manual recomposition: domain-tag || blockHash || blockNumber (big-endian + // u64) || witnessHash || witnessSize (big-endian u64) want := crypto.Keccak256Hash( witnessAnnounceDomainTag, blockHash.Bytes(), []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}, witnessHash.Bytes(), + []byte{0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, ) if got != want { t.Fatalf("signing-hash format drift: got %s want %s", got.Hex(), want.Hex()) @@ -39,7 +42,7 @@ func TestWitnessAnnouncementSigningHashDomainSeparated(t *testing.T) { blockNumber := uint64(7) witnessHash := common.HexToHash("0xbb") - withTag := WitnessAnnouncementSigningHash(blockHash, blockNumber, witnessHash) + withTag := WitnessAnnouncementSigningHash(blockHash, blockNumber, witnessHash, 0) withoutTag := crypto.Keccak256Hash( blockHash.Bytes(), []byte{0, 0, 0, 0, 0, 0, 0, 7}, @@ -58,20 +61,23 @@ func TestWitnessAnnouncementSigningHashSensitive(t *testing.T) { common.HexToHash("0xaa"), 1, common.HexToHash("0xbb"), + 100, ) cases := []struct { name string blockH common.Hash num uint64 witnessH common.Hash + size uint64 }{ - {"different blockHash", common.HexToHash("0xab"), 1, common.HexToHash("0xbb")}, - {"different blockNumber", common.HexToHash("0xaa"), 2, common.HexToHash("0xbb")}, - {"different witnessHash", common.HexToHash("0xaa"), 1, common.HexToHash("0xbc")}, + {"different blockHash", common.HexToHash("0xab"), 1, common.HexToHash("0xbb"), 100}, + {"different blockNumber", common.HexToHash("0xaa"), 2, common.HexToHash("0xbb"), 100}, + {"different witnessHash", common.HexToHash("0xaa"), 1, common.HexToHash("0xbc"), 100}, + {"different witnessSize", common.HexToHash("0xaa"), 1, common.HexToHash("0xbb"), 101}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - if got := WitnessAnnouncementSigningHash(tc.blockH, tc.num, tc.witnessH); got == base { + if got := WitnessAnnouncementSigningHash(tc.blockH, tc.num, tc.witnessH, tc.size); got == base { t.Fatalf("digest unchanged when %s differed", tc.name) } }) From 78552a9cd936a9e455f5e25b27bc36788856e04e Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Wed, 16 Sep 2026 20:03:51 -0400 Subject: [PATCH 02/10] eth/fetcher: fix stale byte-correctness comment on witness verification The comment above verifyAgainstSignedHash still described the pre-oracle byte-correctness invariant (hash must match). This PR's size-oracle rework now accepts hash divergence within the signed-size band, so the comment misdescribed the actual trust boundary an incident responder would rely on. Co-Authored-By: Claude Sonnet 5 --- eth/fetcher/witness_manager.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/eth/fetcher/witness_manager.go b/eth/fetcher/witness_manager.go index f564cbbceb..97908737d9 100644 --- a/eth/fetcher/witness_manager.go +++ b/eth/fetcher/witness_manager.go @@ -701,10 +701,13 @@ func (m *witnessManager) processWitnessResponse(peer string, hash common.Hash, r return } - // WIT2: byte-correctness check. If we have a BP-signed announcement on - // file for this block, the encoded witness bytes must hash to the - // signed witnessHash. State-root failures (content-correctness) are - // handled later in the import path and do NOT drop the server. + // WIT2: size-oracle check. If we have a BP-signed announcement on file + // for this block, the encoded witness only needs to fall within the + // signed-size band — hash divergence from non-deterministic witness + // content is expected and observability-only, not rejected. Only an + // oversized witness is rejected here. State-root failures + // (content-correctness) are handled later in the import path and do NOT + // drop the server. body, witnessHash, ok := m.verifyAgainstSignedHash(peer, hash, witness[0]) if !ok { return From cd3ed0678cfb5f6efbbfde34f8e94a4717dbf9d1 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Fri, 18 Sep 2026 08:06:34 -0400 Subject: [PATCH 03/10] eth, eth/fetcher: charge size-oracle import failures, bound the signed size, relax the broadcast gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups for the WIT2 size oracle. Import-time consequence. A witness accepted on the size oracle alone (hash differs from the BP-signed one) that then fails import was logged at debug and forgotten, so a peer relaying the valid BP announce could serve up to the size band in unusable bytes per block at no cost — the WIT1-class weakness WIT2 was closing. Now the witness manager records the serving peer, the divergence and the fetch closure on the import op (verifyAgainstSignedHash returns diverged; enqueueOp keeps the op instead of rebuilding it from parts), and on insertChain failure importBlocks strikes the server, excludes it as a witness source for that block (new SetWitnessSourceExcluder hook → handler witnessSourceExclusions, honoured by resolveWitnessFetchPeer at every tier and released on import), and hands the block back to the witness manager for a re-fetch from another peer (new witnessRetry loop case → retryAfterImportFailure), bounded by maxWitnessImportRetries. A BP-identical witness that fails import is still the BP's fault and is handled as before. Signed size sanity. signedSize*3 could wrap for a hostile size and a zero WitnessSize yielded a zero ceiling; both struck honest servers. The band now saturates and a zero size falls back to the absolute cap, and acceptSignedAnnouncement refuses (and strikes the sender for) a WitnessSize of zero or above the gas-derived absolute cap before deferral, so the announcer is the one charged. Broadcast gates. acceptSignedBroadcast and acceptDeferredBroadcast now apply the same size oracle as the fetch path: a within-band body is accepted for import (sender marked as body-holder) regardless of hash, so a pusher's own post-import witness (flushWitnessWaitersForImported) is no longer rejected downstream; only byte-identical bytes are cached for pre-import serving, and an oversized body is dropped. fetchAndVerifyWitness (relay fetch, serve-only) keeps requiring the BP's bytes by design and now documents that limitation. Also refreshes every remaining "byte-correctness" comment to the size-oracle semantics (fetcher, handler, peerset, wit protocol), and renames the broadcast byte-mismatch meter to broadcast_oversize with new hash_divergence, implausible_size, import_failure and import_retry meters. Tests: end-to-end import-failure re-fetch through the real fetcher loop (TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer — caught the provenance loss in enqueue), charge/retry unit tests, degenerate-size and saturating-multiply cases, implausible announce size (0 and cap+1 struck, cap accepted), divergent/oversized broadcast on both the signed and deferred paths, source exclusion in resolveWitnessFetchPeer, and the exclusion set lifecycle. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Brme9KQBd7fZBMnMVEhAZU --- eth/fetcher/block_fetcher.go | 157 +++++++++-- eth/fetcher/metrics.go | 15 +- eth/fetcher/witness_import_failure_test.go | 297 +++++++++++++++++++++ eth/fetcher/witness_manager.go | 145 ++++++++-- eth/fetcher/witness_manager_test.go | 8 +- eth/fetcher/witness_manager_wit2.go | 82 ++++-- eth/fetcher/witness_manager_wit2_test.go | 14 +- eth/handler.go | 18 +- eth/handler_eth.go | 26 +- eth/handler_wit.go | 148 ++++++---- eth/handler_wit2.go | 80 ++++-- eth/handler_wit2_announces.go | 20 ++ eth/handler_wit2_caches_test.go | 148 ++++++++++ eth/handler_wit2_exclusions.go | 80 ++++++ eth/handler_wit2_peer.go | 12 +- eth/handler_wit2_test.go | 108 ++++++-- eth/handler_wit_relay_fetch.go | 24 +- eth/peerset.go | 7 +- eth/protocols/wit/protocol.go | 8 +- 19 files changed, 1187 insertions(+), 210 deletions(-) create mode 100644 eth/fetcher/witness_import_failure_test.go create mode 100644 eth/handler_wit2_exclusions.go diff --git a/eth/fetcher/block_fetcher.go b/eth/fetcher/block_fetcher.go index 3834b89577..deaaf999af 100644 --- a/eth/fetcher/block_fetcher.go +++ b/eth/fetcher/block_fetcher.go @@ -159,6 +159,15 @@ type blockOrHeaderInject struct { header *types.Header // Used for light mode fetcher which only cares about header. block *types.Block // Used for normal mode fetcher which imports full block. witness *stateless.Witness // Used for witness mode fetcher which imports witness. + + // WIT2 witness provenance, set by the witness manager when the witness was + // obtained by paged fetch. importBlocks uses it to charge an import failure + // to the serving peer and re-fetch from another source when the witness was + // accepted on the size oracle alone (hash differs from the BP-signed one). + witnessPeer string // Peer that served the witness; empty when not fetched (broadcast, local). + witnessDiverged bool // Witness accepted on size alone: hash differs from the BP-signed hash. + witnessImportFailures int // Import attempts of this block that already failed with a diverged witness. + fetchWitness witnessRequesterFn // Fetch closure to re-request the witness after an import failure. } // number returns the block number of the injected object. @@ -214,8 +223,9 @@ type BlockFetcher struct { headerFilter chan chan *headerFilterTask bodyFilter chan chan *bodyFilterTask - done chan common.Hash - quit chan struct{} + done chan common.Hash + witnessRetry chan *blockOrHeaderInject // Import failed with a size-oracle-accepted witness: forget, then re-fetch the witness + quit chan struct{} // Protect concurrent map access from goroutines mu sync.RWMutex @@ -269,6 +279,7 @@ func NewBlockFetcher(light bool, getHeader HeaderRetrievalFn, getBlock blockRetr headerFilter: make(chan chan *headerFilterTask), bodyFilter: make(chan chan *bodyFilterTask), done: make(chan common.Hash), + witnessRetry: make(chan *blockOrHeaderInject), quit: make(chan struct{}), announces: make(map[string]int), announced: make(map[common.Hash][]*blockAnnounce), @@ -324,16 +335,28 @@ func (f *BlockFetcher) Stop() { } // SetWitnessServerStriker wires the callback used to penalize a peer that serves -// a non-empty witness whose bytes mismatch the BP-signed commitment (WIT2). It -// is set post-construction (rather than threaded through NewBlockFetcher) to keep -// the constructor signature stable. Must be called before Start; optional — -// when unset, byte-mismatch servers are not struck. +// a witness beyond the BP-signed size band, or one accepted on the size oracle +// alone that then fails import (WIT2). It is set post-construction (rather than +// threaded through NewBlockFetcher) to keep the constructor signature stable. +// Must be called before Start; optional — when unset, such servers are not +// struck. func (f *BlockFetcher) SetWitnessServerStriker(fn func(id string)) { if f.wm != nil { f.wm.parentStrikeWitnessServer = fn } } +// SetWitnessSourceExcluder wires the callback that removes a peer from the set +// of fetch sources for one block after the witness it served was accepted on +// the size oracle and failed import, so the re-fetch reaches a different peer. +// Must be called before Start; optional — when unset the re-fetch may target the +// same peer again (bounded by maxWitnessImportRetries). +func (f *BlockFetcher) SetWitnessSourceExcluder(fn func(peer string, blockHash common.Hash)) { + if f.wm != nil { + f.wm.parentExcludeWitnessSource = fn + } +} + // Notify announces the fetcher of the potential availability of a new block in // the network. func (f *BlockFetcher) Notify(peer string, hash common.Hash, number uint64, time time.Time, @@ -555,7 +578,7 @@ func (f *BlockFetcher) loop() { f.importHeaders(op.origin, op.header) } else { // Block must have witness if required, handled by enqueue logic or WM - f.importBlocks(op.origin, op.block, op.witness) + f.importBlocks(op) } } @@ -625,14 +648,25 @@ func (f *BlockFetcher) loop() { log.Error("Received nil enqueue request") continue } - // Enqueue the fully assembled block (potentially with witness) - f.enqueue(req.op.origin, nil, req.op.block, req.op.witness) + // Enqueue the fully assembled block (potentially with witness), + // keeping the op so its witness provenance survives to import. + f.enqueueOp(req.op) case hash := <-f.done: // A pending import finished, remove all traces of the notification f.forgetHash(hash) // This calls wm.forget f.forgetBlock(hash) // This calls wm.forget + case op := <-f.witnessRetry: + // An import failed with a witness accepted on the WIT2 size oracle + // alone. Forget the attempt exactly as `done` does, THEN hand the + // block back to the witness manager for a fresh fetch — sequenced + // on this loop so the forget cannot race the re-registration. + hash := op.hash() + f.forgetHash(hash) + f.forgetBlock(hash) + f.wm.retryAfterImportFailure(op) + case <-fetchTimer.C: // At least one block's timer ran out, check for needing retrieval request := make(map[string][]common.Hash) @@ -1070,16 +1104,38 @@ func (f *BlockFetcher) rescheduleComplete(complete *time.Timer) { // enqueue schedules a new header or block import operation, if the component // to be imported has not yet been seen. func (f *BlockFetcher) enqueue(peer string, header *types.Header, block *types.Block, witness *stateless.Witness) { + if header == nil && block == nil { + log.Error("Enqueue called with nil header and block", "peer", peer) + return + } + op := &blockOrHeaderInject{origin: peer} + if header != nil { + op.header = header + } else { // Prioritize block over header if both somehow provided + op.block = block + // Attach witness only if block is present + op.witness = witness + } + f.enqueueOp(op) +} + +// enqueueOp schedules an already-built import operation. Used directly for +// operations completed by the witness manager so the WIT2 witness provenance it +// attached (serving peer, size-oracle divergence, fetch closure) reaches +// importBlocks intact; rebuilding the op from its parts would drop it and make +// an import failure impossible to charge to the server. +func (f *BlockFetcher) enqueueOp(op *blockOrHeaderInject) { var ( + peer = op.origin hash common.Hash number uint64 ) // Determine hash and number from block first, then header - if block != nil { - hash, number = block.Hash(), block.NumberU64() - } else if header != nil { - hash, number = header.Hash(), header.Number.Uint64() + if op.block != nil { + hash, number = op.block.Hash(), op.block.NumberU64() + } else if op.header != nil { + hash, number = op.header.Hash(), op.header.Number.Uint64() } else { log.Error("Enqueue called with nil header and block", "peer", peer) return @@ -1116,19 +1172,6 @@ func (f *BlockFetcher) enqueue(peer string, header *types.Header, block *types.B } // Schedule the block for future importing - op := &blockOrHeaderInject{origin: peer} - if header != nil { - op.header = header - } else if block != nil { // Prioritize block over header if both somehow provided - op.block = block - // Attach witness only if block is present - op.witness = witness - } else { - log.Error("Invalid state in enqueue: header and block are nil", "peer", peer, "hash", hash) - f.mu.Unlock() - return // Should not happen due to check above - } - f.queues[peer] = count f.queued[hash] = op f.queue.Push(op, -int64(number)) @@ -1179,17 +1222,47 @@ func (f *BlockFetcher) importHeaders(peer string, header *types.Header) { }() } +// maxWitnessImportRetries bounds how many times a block whose import failed with +// a witness accepted on the WIT2 size oracle alone is re-fetched from another +// source before the fetcher gives it up as it would any other failed import. A +// small bound keeps a genuinely invalid block (every honest witness fails) from +// cycling through the peer set, while still recovering from a single server +// that handed out an unusable within-band witness. +const maxWitnessImportRetries = 2 + // importBlocks spawns a new goroutine to run a block insertion into the chain. If the // block's number is at the same height as the current import phase, it updates // the phase states accordingly. -func (f *BlockFetcher) importBlocks(peer string, block *types.Block, witness *stateless.Witness) { +// +// WIT2: when the block's witness was fetched and accepted on the size oracle +// alone (op.witnessDiverged — its hash differs from the BP-signed one, so the +// serving peer, not the BP, chose these bytes) and the import fails, the +// failure is charged to that peer: it is struck and excluded as a source for +// this block, and the block is handed back to the witness manager to fetch the +// witness again from someone else (bounded by maxWitnessImportRetries). Without +// this, a peer that relays the valid BP announce could serve up to the size +// band in unusable bytes per block at no cost, and the block would simply be +// forgotten. A BP-identical witness (hash match) that fails import is the BP's +// fault and is handled as before: logged and forgotten. +func (f *BlockFetcher) importBlocks(op *blockOrHeaderInject) { + peer, block, witness := op.origin, op.block, op.witness hash := block.Hash() // Run the import on a new thread log.Debug("Importing propagated block", "peer", peer, "number", block.Number(), "hash", hash) go func() { - defer func() { f.done <- hash }() + retryWitness := false + defer func() { + if retryWitness { + select { + case f.witnessRetry <- op: + case <-f.quit: + } + return + } + f.done <- hash + }() // If the parent's unknown, abort insertion parent := f.getBlock(block.ParentHash()) @@ -1220,6 +1293,7 @@ func (f *BlockFetcher) importBlocks(peer string, block *types.Block, witness *st // Create slices even for a single block/witness to match the expected signature. if _, err := f.insertChain(types.Blocks{block}, []*stateless.Witness{witness}); err != nil { log.Debug("Propagated block import failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err) + retryWitness = f.chargeDivergedWitnessImportFailure(op, err) return } @@ -1258,6 +1332,33 @@ func (f *BlockFetcher) importBlocks(peer string, block *types.Block, witness *st }() } +// chargeDivergedWitnessImportFailure applies the WIT2 consequence of an import +// failure to the peer that served the block's witness, when that witness was +// accepted on the size oracle alone. It strikes the peer, excludes it as a +// witness source for this block, and reports whether the block should be +// handed back to the witness manager for a re-fetch (false once the retry +// budget is spent, or when the witness was not a fetched, diverged one). +func (f *BlockFetcher) chargeDivergedWitnessImportFailure(op *blockOrHeaderInject, importErr error) bool { + if op.witness == nil || !op.witnessDiverged || op.witnessPeer == "" { + return false + } + hash := op.hash() + witnessImportFailureMeter.Mark(1) + log.Warn("Import failed with a witness accepted on the WIT2 size oracle; striking its server", + "server", op.witnessPeer, "number", op.number(), "hash", hash, "attempt", op.witnessImportFailures+1, "err", importErr) + f.wm.strikeWitnessServer(op.witnessPeer) + f.wm.excludeWitnessSource(op.witnessPeer, hash) + + op.witnessImportFailures++ + if op.fetchWitness == nil || op.witnessImportFailures >= maxWitnessImportRetries { + log.Warn("Giving up witness re-fetch for block after repeated import failures", + "number", op.number(), "hash", hash, "failures", op.witnessImportFailures) + return false + } + witnessImportRetryMeter.Mark(1) + return true +} + // forgetHash removes all traces of a block announcement from the fetcher's // internal state. func (f *BlockFetcher) forgetHash(hash common.Hash) { diff --git a/eth/fetcher/metrics.go b/eth/fetcher/metrics.go index 767c5b2b9a..f4453318b6 100644 --- a/eth/fetcher/metrics.go +++ b/eth/fetcher/metrics.go @@ -40,10 +40,21 @@ var ( // witnessHashDivergenceMeter tracks accepted witnesses whose hash differed // from the BP-signed WitnessHash but whose size was within band — expected - // under non-deterministic witness production. Observability only, NOT a - // drop/strike. + // under non-deterministic witness production. Not a drop/strike at fetch + // time; such a witness is charged to its server only if it then fails + // import (witnessImportFailureMeter). witnessHashDivergenceMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/hash_divergence", nil) + // witnessImportFailureMeter counts block imports that failed with a witness + // accepted on the size oracle alone; each occurrence strikes the serving + // peer and excludes it as a source for that block. + witnessImportFailureMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/import_failure", nil) + + // witnessImportRetryMeter counts the subset of those failures that were + // handed back to the witness manager for a re-fetch from another peer + // (bounded by maxWitnessImportRetries). + witnessImportRetryMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/import_retry", nil) + // Witness page count metrics witnessPageCountBelowThresholdMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/pagecount/below_threshold", nil) witnessPageCountAboveThresholdMeter = metrics.NewRegisteredMeter("eth/fetcher/witness/pagecount/above_threshold", nil) diff --git a/eth/fetcher/witness_import_failure_test.go b/eth/fetcher/witness_import_failure_test.go new file mode 100644 index 0000000000..24cf31f111 --- /dev/null +++ b/eth/fetcher/witness_import_failure_test.go @@ -0,0 +1,297 @@ +package fetcher + +import ( + "bytes" + "errors" + "fmt" + "math" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/stateless" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/eth/protocols/eth" +) + +// TestAcceptableWitnessSizeCeilingDegenerateSignedSizes pins the two inputs that +// must not collapse the size oracle into a zero ceiling (which would reject +// every honest server): a signed size of 0 falls back to the absolute cap, and a +// hostile signed size near MaxUint64 saturates instead of wrapping and then +// clamps to the absolute cap. +func TestAcceptableWitnessSizeCeilingDegenerateSignedSizes(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + + abs := tw.manager.MaxWitnessSize() + if abs == 0 { + t.Fatal("absolute witness size cap must be positive") + } + if got := tw.manager.acceptableWitnessSizeCeiling(0); got != abs { + t.Fatalf("signedSize=0 must fall back to the absolute cap %d, got %d", abs, got) + } + for _, hostile := range []uint64{math.MaxUint64, math.MaxUint64 / 2, math.MaxUint64/wit2SizeBandMultiplier + 1} { + if got := tw.manager.acceptableWitnessSizeCeiling(hostile); got != abs { + t.Fatalf("signedSize=%d must saturate and clamp to the absolute cap %d, got %d", hostile, abs, got) + } + } +} + +// TestSaturatingMulUint64 pins the overflow guard used by the size band. +func TestSaturatingMulUint64(t *testing.T) { + cases := []struct{ a, b, want uint64 }{ + {0, 3, 0}, + {3, 0, 0}, + {7, 3, 21}, + {math.MaxUint64 / 3, 3, math.MaxUint64 / 3 * 3}, + {math.MaxUint64/3 + 1, 3, math.MaxUint64}, + {math.MaxUint64, 2, math.MaxUint64}, + } + for _, c := range cases { + if got := saturatingMulUint64(c.a, c.b); got != c.want { + t.Fatalf("saturatingMulUint64(%d, %d) = %d, want %d", c.a, c.b, got, c.want) + } + } +} + +// TestChargeDivergedWitnessImportFailure covers the decision helper behind the +// WIT2 import-failure consequence: only a fetched witness accepted on the size +// oracle alone is charged (strike + source exclusion), the retry budget is +// honoured, and a BP-identical or non-fetched witness is left alone. +func TestChargeDivergedWitnessImportFailure(t *testing.T) { + tester := newTester(false) + defer tester.fetcher.Stop() + + var ( + mu sync.Mutex + strikes []string + excluded []string + ) + tester.fetcher.SetWitnessServerStriker(func(id string) { + mu.Lock() + strikes = append(strikes, id) + mu.Unlock() + }) + tester.fetcher.SetWitnessSourceExcluder(func(peer string, _ common.Hash) { + mu.Lock() + excluded = append(excluded, peer) + mu.Unlock() + }) + + block := createTestBlock(501) + witness := createTestWitnessForBlock(block) + noopFetch := func(common.Hash, chan *eth.Response) (*eth.Request, error) { return nil, errors.New("noop") } + importErr := errors.New("stateless self-validation failed") + + // BP-identical witness (not diverged): the BP's fault, nothing charged. + identical := &blockOrHeaderInject{origin: "o", block: block, witness: witness, witnessPeer: "srv", fetchWitness: noopFetch} + if tester.fetcher.chargeDivergedWitnessImportFailure(identical, importErr) { + t.Fatal("a BP-identical witness failing import must not trigger a re-fetch") + } + // Diverged but not fetched (no serving peer): nobody to charge. + unfetched := &blockOrHeaderInject{origin: "o", block: block, witness: witness, witnessDiverged: true, fetchWitness: noopFetch} + if tester.fetcher.chargeDivergedWitnessImportFailure(unfetched, importErr) { + t.Fatal("a diverged witness with no serving peer must not trigger a re-fetch") + } + if len(strikes) != 0 || len(excluded) != 0 { + t.Fatalf("nothing should be charged yet; strikes=%v excluded=%v", strikes, excluded) + } + + // Diverged, fetched: charged and retried, up to the budget. + op := &blockOrHeaderInject{origin: "o", block: block, witness: witness, witnessPeer: "srv-1", witnessDiverged: true, fetchWitness: noopFetch} + if !tester.fetcher.chargeDivergedWitnessImportFailure(op, importErr) { + t.Fatal("first import failure with a diverged fetched witness must trigger a re-fetch") + } + if op.witnessImportFailures != 1 { + t.Fatalf("failure counter = %d, want 1", op.witnessImportFailures) + } + // Each re-fetch lands on another server and fails again; the last attempt + // within the budget is still charged but no longer re-fetched. + for attempt := 2; attempt <= maxWitnessImportRetries; attempt++ { + op.witnessPeer = fmt.Sprintf("srv-%d", attempt) + got := tester.fetcher.chargeDivergedWitnessImportFailure(op, importErr) + if want := attempt < maxWitnessImportRetries; got != want { + t.Fatalf("attempt %d: re-fetch = %v, want %v", attempt, got, want) + } + } + if op.witnessImportFailures != maxWitnessImportRetries { + t.Fatalf("failure counter = %d, want %d", op.witnessImportFailures, maxWitnessImportRetries) + } + // A witness without a fetch closure is still charged but can never be + // re-fetched. + orphan := &blockOrHeaderInject{origin: "o", block: block, witness: witness, witnessPeer: "srv-9", witnessDiverged: true} + if tester.fetcher.chargeDivergedWitnessImportFailure(orphan, importErr) { + t.Fatal("without a fetch closure there is no way to re-fetch; must return false") + } + + mu.Lock() + defer mu.Unlock() + wantStrikes := maxWitnessImportRetries + 1 // every charged attempt of op, plus the orphan + if len(strikes) != wantStrikes || strikes[0] != "srv-1" || strikes[len(strikes)-1] != "srv-9" { + t.Fatalf("every charged failure must strike its server exactly once: got %v, want %d strikes starting with srv-1 and ending with srv-9", strikes, wantStrikes) + } + if len(excluded) != len(strikes) { + t.Fatalf("every charged failure must exclude its server for the block: %v", excluded) + } +} + +// TestRetryAfterImportFailureReRegistersPending pins the witness manager side of +// the re-fetch: the block goes back into pending with the witness cleared, the +// fetch closure and failure count carried over, and a second registration for +// the same hash is a no-op. +func TestRetryAfterImportFailureReRegistersPending(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + + block := createTestBlock(502) + hash := block.Hash() + fetch := func(common.Hash, chan *eth.Response) (*eth.Request, error) { return nil, errors.New("noop") } + + // No fetch closure: nothing to retry with. + tw.manager.retryAfterImportFailure(&blockOrHeaderInject{origin: "o", block: block, witness: createTestWitnessForBlock(block)}) + if tw.PendingCount() != 0 { + t.Fatal("an op without a fetch closure must not be re-registered") + } + + op := &blockOrHeaderInject{ + origin: "o", block: block, witness: createTestWitnessForBlock(block), + witnessPeer: "srv-1", witnessDiverged: true, witnessImportFailures: 1, fetchWitness: fetch, + } + tw.manager.retryAfterImportFailure(op) + if tw.PendingCount() != 1 { + t.Fatalf("pending count = %d, want 1", tw.PendingCount()) + } + tw.manager.mu.Lock() + state := tw.manager.pending[hash] + tw.manager.mu.Unlock() + if state == nil || state.announce == nil { + t.Fatal("re-registered block must have a pending state with an announce to fetch on") + } + if state.op.witness != nil || state.op.witnessPeer != "" || state.op.witnessDiverged { + t.Fatal("re-registered op must start without the failed witness and its provenance") + } + if state.op.witnessImportFailures != 1 { + t.Fatalf("failure count must carry over to bound the cycle, got %d", state.op.witnessImportFailures) + } + if state.op.fetchWitness == nil || state.announce.fetchWitness == nil { + t.Fatal("fetch closure must carry over so the next fetch can run") + } + + // Already pending: no duplicate registration. + tw.manager.retryAfterImportFailure(op) + if tw.PendingCount() != 1 { + t.Fatalf("duplicate re-registration must be a no-op; pending count = %d", tw.PendingCount()) + } +} + +// TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer drives the whole +// WIT2 import-failure consequence through the real BlockFetcher loop: a block +// whose fetched witness is accepted on the size oracle alone fails import, the +// serving peer is struck and excluded as a source for that block, the witness +// is fetched again from another peer, and the block then imports. Before this, +// the failure was logged at debug and the block forgotten, so a peer relaying +// the valid BP announce could serve unusable within-band bytes at no cost. +func TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer(t *testing.T) { + hashes, blocks := makeChain(1, 0, genesis) + block := blocks[hashes[0]] + hash := block.Hash() + + tester := newTester(false) + defer tester.fetcher.Stop() + + // BP-signed commitment on file: a hash the served witness will NOT match, + // with a size it is within band of — so every fetched witness is accepted + // as a non-deterministic variant (diverged), never as the BP's own bytes. + reference, err := stateless.NewWitness(block.Header(), nil) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if err := reference.EncodeRLP(&buf); err != nil { + t.Fatal(err) + } + signedSize := uint64(buf.Len()) + tester.fetcher.wm.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { + if h == hash { + return common.HexToHash("0xd1ffe7e17"), signedSize, true + } + return common.Hash{}, 0, false + } + + var ( + mu sync.Mutex + strikes []string + excluded []string + fetches atomic.Int32 + imports atomic.Int32 + ) + tester.fetcher.SetWitnessServerStriker(func(id string) { + mu.Lock() + strikes = append(strikes, id) + mu.Unlock() + }) + tester.fetcher.SetWitnessSourceExcluder(func(peer string, h common.Hash) { + if h != hash { + t.Errorf("exclusion for unexpected block %s", h) + } + mu.Lock() + excluded = append(excluded, peer) + mu.Unlock() + }) + // First import fails (an unusable witness), the second succeeds. + tester.fetcher.insertChain = func(blocks types.Blocks, witnesses []*stateless.Witness) (int, error) { + if imports.Add(1) == 1 { + return 0, errors.New("stateless self-validation failed: missing trie node") + } + return tester.insertChain(blocks, witnesses) + } + imported := make(chan *types.Block, 1) + tester.fetcher.importedHook = func(_ *types.Header, b *types.Block) { imported <- b } + + // Each fetch is answered by a distinct "server", as the handler's source + // exclusion would arrange in production. + fetchWitness := func(h common.Hash, sink chan *eth.Response) (*eth.Request, error) { + n := fetches.Add(1) + req := ð.Request{Peer: fmt.Sprintf("server-%d", n), Cancel: make(chan struct{})} + go func() { + w, err := stateless.NewWitness(block.Header(), nil) + if err != nil { + return + } + sink <- ð.Response{Req: req, Res: []*stateless.Witness{w}, Time: time.Millisecond, Done: make(chan error, 1)} + }() + return req, nil + } + if err := tester.fetcher.InjectBlockWithWitnessRequirement("origin", block, fetchWitness); err != nil { + t.Fatal(err) + } + + select { + case got := <-imported: + if got.Hash() != hash { + t.Fatalf("imported unexpected block %s", got.Hash()) + } + case <-time.After(10 * time.Second): + mu.Lock() + defer mu.Unlock() + t.Fatalf("block never imported after the witness re-fetch (fetches=%d imports=%d strikes=%v excluded=%v)", + fetches.Load(), imports.Load(), strikes, excluded) + } + + if n := fetches.Load(); n != 2 { + t.Fatalf("witness fetch count = %d, want 2 (original + one re-fetch)", n) + } + if n := imports.Load(); n != 2 { + t.Fatalf("import attempt count = %d, want 2", n) + } + mu.Lock() + defer mu.Unlock() + if len(strikes) != 1 || strikes[0] != "server-1" { + t.Fatalf("exactly the first server must be struck once, got %v", strikes) + } + if len(excluded) != 1 || excluded[0] != "server-1" { + t.Fatalf("exactly the first server must be excluded for the block, got %v", excluded) + } +} diff --git a/eth/fetcher/witness_manager.go b/eth/fetcher/witness_manager.go index 97908737d9..152b505114 100644 --- a/eth/fetcher/witness_manager.go +++ b/eth/fetcher/witness_manager.go @@ -58,29 +58,40 @@ type cachedWitness struct { timestamp time.Time } -// signedWitnessHashFn returns the BP-signed witness content hash for a block, -// if a WIT2 signed announcement has been received and verified locally. It is -// used by the witness manager on fetch success to verify byte-correctness: -// if the encoded witness bytes don't hash to the signed witnessHash, the -// serving peer lied and is dropped. If no signed announcement is on file -// (e.g., WIT1-only fetch), the check is skipped. +// signedWitnessHashFn returns the BP-signed witness commitment for a block — +// the producer's own witness hash and encoded size — if a WIT2 signed +// announcement has been received and verified locally. The witness manager uses +// it on fetch success as a size oracle (see verifyAgainstSignedHash): a served +// witness is accepted for import when its encoded size is within a band of the +// signed size; its hash is compared only to decide whether the bytes are the +// BP's own and therefore eligible for pre-import re-serving. If no signed +// announcement is on file (e.g., WIT1-only fetch), the check is skipped. type signedWitnessHashFn func(blockHash common.Hash) (witnessHash common.Hash, witnessSize uint64, ok bool) // cacheWitnessForServingFn hands successfully-fetched witness bytes to the -// network handler so peers can serve them pre-import. Called only after the -// byte-correctness check (vs. BP-signed witnessHash, when present) has passed, -// so the cached bytes are safe to serve. The witnessHash is the canonical -// keccak256 of the canonical encoding, identical to what the BP signed. +// network handler so peers can serve them pre-import. Called only for bytes that +// are byte-identical to the BP's own witness (hash match against the signed +// commitment), so the pre-import serving cache never carries a variant the BP +// did not produce. The witnessHash is the WIT2 commitment over the canonical +// encoding, identical to what the BP signed. type cacheWitnessForServingFn func(blockHash common.Hash, witnessBytes []byte, witnessHash common.Hash) // peerStrikeFn records a WIT2 misbehavior strike against a peer (by id) that -// served a non-empty witness whose bytes mismatch the on-file BP-signed hash. -// Unlike peerDropFn it does not immediately disconnect: a single mismatch is -// tolerated (a faulty/malicious BP that signed a bogus hash makes honest servers -// mismatch too), but sustained byte-serving misbehavior accrues toward the same -// disconnect threshold as bad announces. Optional; nil disables the penalty. +// served a witness we could not use: one beyond the BP-signed size band, or +// one accepted on the size oracle alone whose import then failed. Unlike +// peerDropFn it does not immediately disconnect: a single strike is tolerated +// (a faulty BP, or a bad block, makes honest servers look wrong too), but +// sustained misbehavior accrues toward the same disconnect threshold as bad +// announces. Optional; nil disables the penalty. type peerStrikeFn func(id string) +// witnessSourceExcludeFn tells the network handler that peer's copy of the +// witness for blockHash must not be fetched again: it was accepted on the size +// oracle and failed import. The handler skips that peer when resolving the +// fetch target for blockHash so the re-fetch reaches a different source. +// Optional; nil means the re-fetch may land on the same peer. +type witnessSourceExcludeFn func(peer string, blockHash common.Hash) + // witnessManager handles the logic specific to fetching and managing witnesses // for blocks, isolating it from the main BlockFetcher loop. type witnessManager struct { @@ -93,9 +104,10 @@ type witnessManager struct { parentGetHeader HeaderRetrievalFn // Function to check if header is known locally (needed for checks) parentChainHeight chainHeightFn // Retrieve chain height for distance checks parentCurrentHeader currentHeaderFn // Retrieve current block header for gas limit - parentSignedWitnessHash signedWitnessHashFn // WIT2: lookup a BP-signed witness hash for byte-correctness verification - parentCacheWitnessForServing cacheWitnessForServingFn // WIT2: hand bytes to the handler for pre-import serving by peers - parentStrikeWitnessServer peerStrikeFn // WIT2: strike a peer that served non-empty bytes mismatching the signed hash (optional) + parentSignedWitnessHash signedWitnessHashFn // WIT2: lookup the BP-signed witness commitment (hash + size) for the size oracle + parentCacheWitnessForServing cacheWitnessForServingFn // WIT2: hand BP-identical bytes to the handler for pre-import serving by peers + parentStrikeWitnessServer peerStrikeFn // WIT2: strike a peer that served an oversized or import-failing witness (optional) + parentExcludeWitnessSource witnessSourceExcludeFn // WIT2: exclude a peer as fetch source for a block after its witness failed import (optional) // Witness-specific state pending map[common.Hash]*witnessRequestState // Blocks waiting for witness or actively fetching. @@ -704,29 +716,35 @@ func (m *witnessManager) processWitnessResponse(peer string, hash common.Hash, r // WIT2: size-oracle check. If we have a BP-signed announcement on file // for this block, the encoded witness only needs to fall within the // signed-size band — hash divergence from non-deterministic witness - // content is expected and observability-only, not rejected. Only an - // oversized witness is rejected here. State-root failures - // (content-correctness) are handled later in the import path and do NOT - // drop the server. - body, witnessHash, ok := m.verifyAgainstSignedHash(peer, hash, witness[0]) + // content is expected and is not rejected here. Only an oversized witness + // is rejected. Content-correctness is arbitrated by import-time execution; + // when a witness accepted on the size oracle alone (diverged) then fails + // import, the fetcher strikes this server and re-fetches from another + // source (see BlockFetcher.importBlocks). + body, witnessHash, diverged, ok := m.verifyAgainstSignedHash(peer, hash, witness[0]) if !ok { return } - // WIT2: hand the verified bytes to the handler for pre-import serving. + // WIT2: hand BP-identical bytes to the handler for pre-import serving. // Done before import-side enqueue so a peer asking us for the body // during the chain-write window gets bytes from the in-flight cache // rather than empty results. body is nil on the WIT1 path (no signed - // hash on file) — cacheVerifiedWitnessForServing no-ops in that case. + // hash on file) and for a within-band non-identical variant — + // cacheVerifiedWitnessForServing no-ops in both cases. m.cacheVerifiedWitnessForServing(hash, body, witnessHash) metrics.RecordPerItemDuration(blockWitnessItemDownloadTimer, res.Time, 1) - m.handleWitnessFetchSuccess(peer, hash, witness[0], announcedAt) + m.handleWitnessFetchSuccess(peer, hash, witness[0], announcedAt, diverged) } // handleWitnessFetchSuccess processes a successfully fetched witness. // It needs the original origin from the op state for consistency checks. -func (m *witnessManager) handleWitnessFetchSuccess(fetchPeer string, hash common.Hash, witness *stateless.Witness, announcedAt time.Time) { +// diverged records that the witness was accepted on the size oracle alone (hash +// differs from the BP-signed one); together with the serving peer and the +// fetch closure it is carried on the import op so an import failure can be +// charged to the server and re-fetched from another source. +func (m *witnessManager) handleWitnessFetchSuccess(fetchPeer string, hash common.Hash, witness *stateless.Witness, announcedAt time.Time, diverged bool) { m.mu.Lock() state, exists := m.pending[hash] if !exists { @@ -742,10 +760,15 @@ func (m *witnessManager) handleWitnessFetchSuccess(fetchPeer string, hash common return // Already handled } - log.Debug("[wm] Witness received via fetch, queuing block for import", "peer", fetchPeer, "origin", state.op.origin, "number", state.op.number(), "hash", hash) + log.Debug("[wm] Witness received via fetch, queuing block for import", "peer", fetchPeer, "origin", state.op.origin, "number", state.op.number(), "hash", hash, "diverged", diverged) - // Attach witness (under lock) + // Attach witness and its provenance (under lock) state.op.witness = witness + state.op.witnessPeer = fetchPeer + state.op.witnessDiverged = diverged + if state.announce != nil { + state.op.fetchWitness = state.announce.fetchWitness + } m.mu.Unlock() // Update timestamps on the block @@ -862,6 +885,70 @@ func (m *witnessManager) safeEnqueue(op *blockOrHeaderInject) { m.rescheduleWitness() } +// strikeWitnessServer records a WIT2 strike against peer via the parent +// callback, if one is wired. +func (m *witnessManager) strikeWitnessServer(peer string) { + if peer != "" && m.parentStrikeWitnessServer != nil { + m.parentStrikeWitnessServer(peer) + } +} + +// excludeWitnessSource asks the parent to stop offering peer as a witness +// source for hash, if a callback is wired. +func (m *witnessManager) excludeWitnessSource(peer string, hash common.Hash) { + if peer != "" && m.parentExcludeWitnessSource != nil { + m.parentExcludeWitnessSource(peer, hash) + } +} + +// retryAfterImportFailure re-registers a block whose import failed with a +// witness accepted on the WIT2 size oracle alone, so its witness is fetched +// again — from a different source, the failed one having been excluded by the +// fetcher. Called from the BlockFetcher loop right after the failed attempt has +// been forgotten, so the pending map is free for the hash. The carried +// witnessImportFailures count bounds the cycle (see maxWitnessImportRetries). +func (m *witnessManager) retryAfterImportFailure(op *blockOrHeaderInject) { + if op == nil || op.block == nil || op.fetchWitness == nil { + return + } + hash := op.block.Hash() + if m.isWitnessUnavailable(hash) { + log.Debug("[wm] Not re-fetching witness after import failure: marked unavailable", "hash", hash) + return + } + if m.parentGetBlock(hash) != nil { + log.Debug("[wm] Not re-fetching witness after import failure: block now known locally", "hash", hash) + return + } + + m.mu.Lock() + if _, exists := m.pending[hash]; exists { + m.mu.Unlock() + log.Debug("[wm] Not re-fetching witness after import failure: already pending", "hash", hash) + return + } + m.pending[hash] = &witnessRequestState{ + op: &blockOrHeaderInject{ + origin: op.origin, + block: op.block, + fetchWitness: op.fetchWitness, + witnessImportFailures: op.witnessImportFailures, + }, + announce: &blockAnnounce{ + origin: op.origin, + hash: hash, + number: op.block.NumberU64(), + time: time.Now(), + fetchWitness: op.fetchWitness, + }, + } + m.mu.Unlock() + + log.Info("[wm] Re-fetching witness from another peer after import failure", + "number", op.block.NumberU64(), "hash", hash, "failedServer", op.witnessPeer, "failures", op.witnessImportFailures) + m.rescheduleWitness() +} + // forget cleans up any pending state for a given hash. Called when a block is // imported or discarded by the main fetcher *before* witness handling completed. func (m *witnessManager) forget(hash common.Hash) { diff --git a/eth/fetcher/witness_manager_test.go b/eth/fetcher/witness_manager_test.go index e8cceac53a..50ff58baa6 100644 --- a/eth/fetcher/witness_manager_test.go +++ b/eth/fetcher/witness_manager_test.go @@ -1131,7 +1131,7 @@ func TestHandleWitnessFetchSuccess(t *testing.T) { // Test successful witness fetch announcedAt := time.Now() - manager.handleWitnessFetchSuccess("fetch-peer", block.Hash(), witness, announcedAt) + manager.handleWitnessFetchSuccess("fetch-peer", block.Hash(), witness, announcedAt, false) time.Sleep(10 * time.Millisecond) // Give time for async processing @@ -1180,7 +1180,7 @@ func TestHandleWitnessFetchSuccessNoPending(t *testing.T) { // Test with no pending state - should handle gracefully announcedAt := time.Now() - manager.handleWitnessFetchSuccess("fetch-peer", block.Hash(), witness, announcedAt) + manager.handleWitnessFetchSuccess("fetch-peer", block.Hash(), witness, announcedAt, false) // Should not panic or cause issues } @@ -1229,7 +1229,7 @@ func TestHandleWitnessFetchSuccessWitnessAlreadyPresent(t *testing.T) { // Test with witness already present - should be ignored announcedAt := time.Now() - manager.handleWitnessFetchSuccess("fetch-peer", block.Hash(), witness2, announcedAt) + manager.handleWitnessFetchSuccess("fetch-peer", block.Hash(), witness2, announcedAt, false) // Verify original witness is still there if state.op.witness != witness1 { @@ -3391,7 +3391,7 @@ func TestHandleWitnessFetchSuccessUpdatesBlockTimestamps(t *testing.T) { m.mu.Unlock() announcedAt := time.Now().Add(-time.Second) - m.handleWitnessFetchSuccess("peer", hash, witness, announcedAt) + m.handleWitnessFetchSuccess("peer", hash, witness, announcedAt, false) select { case req := <-enqueueCh: diff --git a/eth/fetcher/witness_manager_wit2.go b/eth/fetcher/witness_manager_wit2.go index a40669b27f..c353faa951 100644 --- a/eth/fetcher/witness_manager_wit2.go +++ b/eth/fetcher/witness_manager_wit2.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "math" "time" "github.com/ethereum/go-ethereum/common" @@ -33,13 +34,13 @@ const ( emptyResponseMaxBackoff = 1 * time.Second ) -// cacheVerifiedWitnessForServing forwards canonical-encoded witness bytes -// (already verified against a BP-signed witness hash by the caller) to the -// handler so other peers can fetch them pre-import. No-op when no cache -// callback is configured (legacy WIT1-only paths) or when body is empty — -// the latter signals the WIT1 path with no signed hash on file, where -// caching unverified bytes would expose us to byte-blame from downstream -// peers. +// cacheVerifiedWitnessForServing forwards canonical-encoded witness bytes that +// are byte-identical to the BP's (hash match, see verifyAgainstSignedHash) to +// the handler so other peers can fetch them pre-import. No-op when no cache +// callback is configured (legacy WIT1-only paths) or when body is empty — the +// latter covers the WIT1 path with no signed hash on file and the within-band +// non-identical variant, neither of which is re-served pre-import: the +// pre-import serving cache carries only the BP's own bytes. func (m *witnessManager) cacheVerifiedWitnessForServing(blockHash common.Hash, body []byte, witnessHash common.Hash) { if m.parentCacheWitnessForServing == nil || len(body) == 0 { return @@ -60,25 +61,31 @@ func (m *witnessManager) cacheVerifiedWitnessForServing(blockHash common.Hash, b // bytes, keyed by the signed hash, so a downstream byte check against that hash // stays meaningful. // +// diverged is true when the witness was accepted on the size oracle alone +// (signed announcement on file, size within band, hash differs). The fetcher +// uses it at import time: an import failure of such a witness is charged to the +// serving peer (strike + re-fetch from another source), because with hash +// identity gone the server, not the BP, is the party that chose these bytes. +// // body is also nil on the WIT1 path (no signed announcement). ok is false only // when the witness is oversized or a local EncodeRLP failure occurs; the latter // is the local node's own error, not a peer fault, so it does not strike. -func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, witness *stateless.Witness) (body []byte, witnessHash common.Hash, ok bool) { +func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, witness *stateless.Witness) (body []byte, witnessHash common.Hash, diverged bool, ok bool) { if m.parentSignedWitnessHash == nil { - return nil, common.Hash{}, true + return nil, common.Hash{}, false, true } expected, expectedSize, has := m.parentSignedWitnessHash(hash) if !has || m.isSignedHashQuarantined(hash) { // No signed announcement on file, or it has been quarantined after // distinct servers repeatedly served oversized bytes: fall back to the // WIT1 path so import-time execution arbitrates the bytes. - return nil, common.Hash{}, true + return nil, common.Hash{}, false, true } var buf bytes.Buffer if err := witness.EncodeRLP(&buf); err != nil { log.Warn("[wm] Failed to encode received witness for size check", "peer", peer, "hash", hash, "err", err) m.handleWitnessFetchFailureExt(hash, "", fmt.Errorf("witness encode failed: %w", err), false) - return nil, common.Hash{}, false + return nil, common.Hash{}, false, false } encoded := buf.Bytes() actualSize := uint64(len(encoded)) @@ -122,7 +129,7 @@ func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, m.parentStrikeWitnessServer(peer) } m.handleWitnessFetchFailureExt(hash, "", errors.New("witness exceeds signed size band"), false) - return nil, common.Hash{}, false + return nil, common.Hash{}, false, false } // Within band: forget any earlier oversize noise for this block. @@ -133,12 +140,13 @@ func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, // import (state-root execution validates), but return body=nil so it is // NOT cached for pre-import serving or relayed — those fast-paths carry // only the BP's own bytes so a downstream check against the signed hash - // stays meaningful. Observability only. + // stays meaningful. Flagged diverged so an import failure is charged to + // the server (see importBlocks) rather than silently forgotten. witnessHashDivergenceMeter.Mark(1) - return nil, common.Hash{}, true + return nil, common.Hash{}, true, true } // Byte-identical to the BP's witness: safe to serve/relay under the signed hash. - return encoded, expected, true + return encoded, expected, false, true } // wit2SizeBandMultiplier bounds how many times the BP-signed witness size a @@ -155,17 +163,53 @@ const bytesPerMiB = 1024 * 1024 // accepted for a block whose BP-signed witness size is signedSize. It is // min(wit2SizeBandMultiplier*signedSize, absolute), where the absolute cap is // the pre-existing gas-derived page ceiling expressed in bytes — retained so the -// accepted size stays bounded even when the signed size is implausibly large, -// and so a witness with no useful signed size still has a hard bound. +// accepted size stays bounded even when the signed size is implausibly large. +// +// Two degenerate inputs must not turn into a zero ceiling that would reject +// every honest server: a signedSize of 0 (no usable oracle) falls back to the +// absolute cap alone, and the multiplication saturates instead of wrapping for +// a hostile signedSize near MaxUint64. The announce path additionally refuses +// announcements whose WitnessSize is 0 or above the absolute cap, so under +// normal operation neither branch is reached; they are defence in depth. func (m *witnessManager) acceptableWitnessSizeCeiling(signedSize uint64) uint64 { - band := signedSize * wit2SizeBandMultiplier - absBytes := m.calculatePageThreshold() * maxPageSizeMB * bytesPerMiB + absBytes := m.MaxWitnessSize() + if signedSize == 0 { + return absBytes + } + band := saturatingMulUint64(signedSize, wit2SizeBandMultiplier) if absBytes == 0 { return band } return min(band, absBytes) } +// AcceptableWitnessSizeCeiling is the exported form of +// acceptableWitnessSizeCeiling for the network handler, which applies the same +// size oracle to witnesses that arrive by NewWitness broadcast rather than by +// paged fetch, so both delivery paths accept and reject identically. +func (m *witnessManager) AcceptableWitnessSizeCeiling(signedSize uint64) uint64 { + return m.acceptableWitnessSizeCeiling(signedSize) +} + +// MaxWitnessSize returns the absolute encoded-size cap for any witness: the +// gas-derived page ceiling (calculatePageThreshold) expressed in bytes. It bounds +// the size oracle from above and is the plausibility bound a BP-signed +// WitnessSize must satisfy to be accepted at announce time. +func (m *witnessManager) MaxWitnessSize() uint64 { + return m.calculatePageThreshold() * maxPageSizeMB * bytesPerMiB +} + +// saturatingMulUint64 returns a*b, or math.MaxUint64 if the product would wrap. +func saturatingMulUint64(a, b uint64) uint64 { + if a == 0 || b == 0 { + return 0 + } + if a > math.MaxUint64/b { + return math.MaxUint64 + } + return a * b +} + // witnessSizeExceedsCeiling reports whether an encoded witness of actualSize // bytes is oversized relative to ceiling. A witness exactly at the ceiling is // accepted; only a strictly larger one is rejected. diff --git a/eth/fetcher/witness_manager_wit2_test.go b/eth/fetcher/witness_manager_wit2_test.go index 06fc8a1894..9b5572ce89 100644 --- a/eth/fetcher/witness_manager_wit2_test.go +++ b/eth/fetcher/witness_manager_wit2_test.go @@ -246,7 +246,7 @@ func TestVerifyAgainstSignedHashSkipsEncodeWhenNoSignedHash(t *testing.T) { return common.Hash{}, 0, false } - body, _, ok := tw.manager.verifyAgainstSignedHash("peer1", hash, witness) + body, _, _, ok := tw.manager.verifyAgainstSignedHash("peer1", hash, witness) if !ok { t.Fatalf("verifyAgainstSignedHash returned ok=false on WIT1 path") } @@ -355,7 +355,7 @@ func TestSignedHashQuarantineAfterDistinctMismatches(t *testing.T) { primePendingWitness(tw, "peerA", block) // First distinct server mismatches: rejected, but not yet quarantined. - if _, _, ok := tw.manager.verifyAgainstSignedHash("peerA", hash, witness); ok { + if _, _, _, ok := tw.manager.verifyAgainstSignedHash("peerA", hash, witness); ok { t.Fatal("mismatch must return ok=false") } if tw.manager.isSignedHashQuarantined(hash) { @@ -363,7 +363,7 @@ func TestSignedHashQuarantineAfterDistinctMismatches(t *testing.T) { } // Second DISTINCT server mismatches: the signed hash is now the suspect. - if _, _, ok := tw.manager.verifyAgainstSignedHash("peerB", hash, witness); ok { + if _, _, _, ok := tw.manager.verifyAgainstSignedHash("peerB", hash, witness); ok { t.Fatal("mismatch must return ok=false") } if !tw.manager.isSignedHashQuarantined(hash) { @@ -372,7 +372,7 @@ func TestSignedHashQuarantineAfterDistinctMismatches(t *testing.T) { // A subsequent fetch falls back to WIT1: body=nil, ok=true, so the witness // is accepted for import (execution validates) instead of stalling for 30s. - body, _, ok := tw.manager.verifyAgainstSignedHash("peerC", hash, witness) + body, _, _, ok := tw.manager.verifyAgainstSignedHash("peerC", hash, witness) if !ok { t.Fatal("quarantined signed hash must fall back to WIT1 (accept, execution validates)") } @@ -496,7 +496,7 @@ func TestVerifyAgainstSignedHashStrikesNonEmptyMismatchServer(t *testing.T) { struck = append(struck, peer) } - if _, _, ok := tw.manager.verifyAgainstSignedHash("sybil-server", hash, witness); ok { + if _, _, _, ok := tw.manager.verifyAgainstSignedHash("sybil-server", hash, witness); ok { t.Fatal("oversized witness must return ok=false") } if len(struck) != 1 || struck[0] != "sybil-server" { @@ -676,7 +676,7 @@ func TestVerifyAgainstSignedHashAcceptsDivergentHashWithinBand(t *testing.T) { struck := 0 tw.manager.parentStrikeWitnessServer = func(string) { struck++ } - body, _, ok := tw.manager.verifyAgainstSignedHash("honest-diverging", hash, witness) + body, _, _, ok := tw.manager.verifyAgainstSignedHash("honest-diverging", hash, witness) if !ok { t.Fatal("a witness within the signed size band must be accepted for import despite a differing hash") } @@ -711,7 +711,7 @@ func TestVerifyAgainstSignedHashServesOnExactMatch(t *testing.T) { return common.Hash{}, 0, false } - body, gotHash, ok := tw.manager.verifyAgainstSignedHash("honest-matching", hash, witness) + body, gotHash, _, ok := tw.manager.verifyAgainstSignedHash("honest-matching", hash, witness) if !ok { t.Fatal("a byte-identical within-band witness must be accepted") } diff --git a/eth/handler.go b/eth/handler.go index 1f02cf022a..6d6319bcc3 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -189,8 +189,9 @@ type handler struct { // WIT2: cache of BP-signed witness announcements, keyed by block hash. // Populated by both produced (signed locally) and received-and-verified // announcements. Consulted by the relay path to dedup, by the body - // broadcast path to re-emit signed announces, and by the fetch path to - // supply the byte-correctness comparison hash. + // broadcast path to re-emit signed announces, and by the fetch and + // broadcast-accept paths to supply the size oracle (signed WitnessSize) + // and the BP's WitnessHash that gates pre-import re-serving. signedWitnesses *signedWitnessCache // WIT2: in-flight witness bodies received via NewWitness broadcast but @@ -223,6 +224,11 @@ type handler struct { // the WIT1-style hand-off the fast announce removed. witnessWaiters *witnessWaiterRegistry + // WIT2: per-block set of peers whose served witness was accepted on the + // size oracle alone and then failed import. Skipped when resolving a + // witness fetch source so the fetcher's re-fetch reaches another peer. + witnessSourceExclusions *witnessSourceExclusionSet + // WIT2: dedup guard for relayFetchOnDemand — a pure relay node (no // produce_witness, no sync_with_witness) has no reason of its own to // ever fetch witness bytes, so without this it can register waiters via @@ -293,6 +299,7 @@ func newHandler(config *handlerConfig) (*handler, error) { relayFetchSem: make(chan struct{}, wit2RelayFetchGlobalConcurrencyCap), deferredAnnounces: newDeferredAnnounceCache(deferredAnnounceCapacity), witnessWaiters: newWitnessWaiterRegistry(), + witnessSourceExclusions: newWitnessSourceExclusionSet(), } log.Info("Sync with witnesses", "enabled", config.syncWithWitnesses) @@ -377,9 +384,12 @@ func newHandler(config *handlerConfig) (*handler, error) { } h.blockFetcher = fetcher.NewBlockFetcher(false, nil, h.chain.GetBlockByHash, validator, h.BroadcastBlock, heighter, h.chain.CurrentHeader, nil, inserter, h.removePeer, h.jailPeer, h.enableBlockTracking, h.statelessSync.Load() || h.syncWithWitnesses, config.gasCeil, h.lookupSignedWitnessHash, h.cacheVerifiedWitnessForServing) - // WIT2: penalize a peer that serves a non-empty witness whose bytes mismatch - // the BP-signed commitment (strike, not drop — see strikeWit2PeerByID). + // WIT2: penalize a peer that serves a witness beyond the BP-signed size band, + // or one accepted on the size oracle alone that then fails import (strike, + // not drop — see strikeWit2PeerByID); in the latter case also stop asking + // that peer for this block's witness so the re-fetch lands elsewhere. h.blockFetcher.SetWitnessServerStriker(h.strikeWit2PeerByID) + h.blockFetcher.SetWitnessSourceExcluder(h.excludeWitnessSource) fetchTx := func(peer string, requestID uint64, hashes []common.Hash) error { p := h.peers.peer(peer) diff --git a/eth/handler_eth.go b/eth/handler_eth.go index f62b5291ba..da2f416601 100644 --- a/eth/handler_eth.go +++ b/eth/handler_eth.go @@ -147,10 +147,10 @@ func (h *ethHandler) createWitnessRequester() func(hash common.Hash, sink chan * } // resolveWitnessFetchPeer picks a body-fetch target for hash. Marked peers -// win: getOnePeerWithWitness prefers a proven body-holder and falls back to -// an announce-known relayer. If neither exists, fall back to the peer that -// relayed a still-deferred signed announcement for the hash. At the -// stateless tip the deferred state is structural, not transient: the +// win: a proven body-holder is preferred, then an announce-known relayer +// (peersWithWitnessCandidates orders them so). If neither exists, fall back +// to the peer that relayed a still-deferred signed announcement for the hash. +// At the stateless tip the deferred state is structural, not transient: the // announce cannot be producer-verified before the block imports, the block // cannot import without the witness, and the unverified announce marks no // peer — so without this fallback a consumer whose witness exceeds the @@ -160,12 +160,22 @@ func (h *ethHandler) createWitnessRequester() func(hash common.Hash, sink chan * // announcement must not be able to veto or bless data — import (stateless // execution + state-root check) remains the verifier, as on every WIT1 // fetch. +// +// A peer excluded for this hash (its size-oracle-accepted witness failed +// import; see excludeWitnessSource) is skipped at every tier, so the +// fetcher's re-fetch reaches a different source. func (h *ethHandler) resolveWitnessFetchPeer(hash common.Hash) *ethPeer { - if p := h.peers.getOnePeerWithWitness(hash); p != nil { - return p + hh := (*handler)(h) + usable := func(id string) bool { + return hh.witnessSourceExclusions == nil || !hh.witnessSourceExclusions.excluded(hash, id) + } + for _, p := range h.peers.peersWithWitnessCandidates(hash) { + if usable(p.ID()) { + return p + } } - if peerID, ok := (*handler)(h).deferredAnnounces.peekPeer(hash, func(id string) bool { - return h.peers.peer(id) != nil + if peerID, ok := hh.deferredAnnounces.peekPeer(hash, func(id string) bool { + return usable(id) && h.peers.peer(id) != nil }); ok { return h.peers.peer(peerID) } diff --git a/eth/handler_wit.go b/eth/handler_wit.go index 8175bc19c8..c14e3c4840 100644 --- a/eth/handler_wit.go +++ b/eth/handler_wit.go @@ -83,20 +83,21 @@ func (h *witHandler) Handle(peer *wit.Peer, packet wit.Packet) error { } // handleWitnessBroadcast handles a witness broadcast from a peer. A broadcast -// witness is only accepted — sender marked as a body-holder, bytes cached, -// witness injected for import — when we can bind it to something we already -// trust: a BP-signed announcement whose witnessHash matches the received -// bytes (WIT2), or a locally known block header (WIT1 fallback). Anything -// else is dropped: bytes contradicting a BP-signed commitment are provably -// wrong and must not bypass the verification the paged-fetch path enforces, -// and an unsigned witness for an unknown header is unverifiable on the -// sender's say-so alone. +// witness is only accepted — sender marked as a body-holder, witness injected +// for import — when we can bind it to something we already trust: a BP-signed +// announcement whose size oracle admits the received bytes (WIT2; the bytes are +// additionally cached for pre-import serving only when byte-identical to the +// BP's), or a locally known block header (WIT1 fallback). Anything else is +// dropped: bytes beyond a BP-signed size band exceed any plausible +// non-deterministic variation and must not bypass the acceptance the +// paged-fetch path enforces, and an unsigned witness for an unknown header is +// unverifiable on the sender's say-so alone. func (h *witHandler) handleWitnessBroadcast(peer *wit.Peer, witness *stateless.Witness) error { hash := witness.Header().Hash() var accepted bool if signed, hasSigned := (*handler)(h).signedWitnesses.get(hash); hasSigned { - accepted = h.acceptSignedBroadcast(peer, witness, hash, signed.WitnessHash) + accepted = h.acceptSignedBroadcast(peer, witness, hash, signed) } else if (*handler)(h).deferredAnnounces.has(hash) { accepted = h.acceptDeferredBroadcast(peer, witness, hash) } else { @@ -136,31 +137,47 @@ func encodedBroadcastBytes(peer *wit.Peer, witness *stateless.Witness, hash comm return buf.Bytes(), true } -// acceptSignedBroadcast is the WIT2 accept path of the witness broadcast: -// verify against the BP-signed witnessHash on file, then cache the encoded -// body so this node can serve it pre-import. We only expose the cache for -// serving when bytes match — otherwise an upstream that lied about the bytes -// would make us serve garbage and get dropped by downstream peers as liars, -// even though we just relayed what we received. On mismatch nothing is -// cached, the sender is not marked as a body-holder, and the witness is not -// injected: the broadcast path must not be a bypass of the byte verification -// the paged-fetch path performs. No disconnect — the sender may itself have -// been fed bad bytes upstream. -func (h *witHandler) acceptSignedBroadcast(peer *wit.Peer, witness *stateless.Witness, hash common.Hash, signedHash common.Hash) bool { +// acceptSignedBroadcast is the WIT2 accept path of the witness broadcast. The +// BP-signed announcement on file supplies the size oracle, exactly as on the +// paged-fetch path (witnessManager.verifyAgainstSignedHash): a pushed body whose +// encoded size is within the accepted band of the signed WitnessSize is accepted +// for IMPORT and the sender marked as a body-holder, whether or not its hash +// matches — witnesses are non-deterministic, so a peer pushing its own +// post-import witness (flushWitnessWaitersForImported) routinely differs from +// the BP's bytes while being perfectly valid, and import-time execution +// arbitrates content. Only bytes byte-identical to the BP's (hash match) are +// additionally cached for pre-import serving and pushed on to our own waiters: +// the pre-import fast path carries the BP's bytes only, so an upstream that +// pushed us garbage cannot make us amplify it before we have validated it, nor +// get us blamed by our downstream for bytes we did not choose. An oversized +// body is dropped without marking or injecting — it exceeds any plausible +// non-deterministic variation. No disconnect — the sender may itself have been +// fed the bytes upstream. +func (h *witHandler) acceptSignedBroadcast(peer *wit.Peer, witness *stateless.Witness, hash common.Hash, signed wit.SignedWitnessAnnouncement) bool { bodyBytes, ok := encodedBroadcastBytes(peer, witness, hash) if !ok { return false } - bodyHash := stateless.WitnessCommitHash(bodyBytes) - if signedHash != bodyHash { - wit2BroadcastByteMismatchMeter.Mark(1) - peer.Log().Warn("wit2: broadcast bytes do not match signed witnessHash; dropping", - "blockHash", hash, "expected", signedHash, "actual", bodyHash) + ceiling := (*handler)(h).witnessSizeCeiling(signed.WitnessSize) + if uint64(len(bodyBytes)) > ceiling { + wit2BroadcastOversizeMeter.Mark(1) + peer.Log().Warn("wit2: broadcast witness exceeds the BP-signed size band; dropping", + "blockHash", hash, "signedSize", signed.WitnessSize, "ceiling", ceiling, "received", len(bodyBytes)) return false } peer.AddKnownWitness(hash) + bodyHash := stateless.WitnessCommitHash(bodyBytes) + if bodyHash != signed.WitnessHash { + // A valid non-deterministic variant of the BP's witness: import it, but + // do not re-serve it pre-import (the serving cache carries the BP's + // bytes only). + wit2BroadcastHashDivergenceMeter.Mark(1) + peer.Log().Debug("wit2: broadcast witness within the size band but not the BP's bytes; importing without re-serving", + "blockHash", hash, "signed", signed.WitnessHash, "actual", bodyHash) + return true + } (*handler)(h).pendingWitnessBodies.put(hash, bodyBytes, bodyHash) - // We now hold servable bytes — push to any peer that asked us + // We now hold the BP's own bytes — push to any peer that asked us // for this body before we had it. (*handler)(h).pushWitnessToWaiters(hash, witness, len(bodyBytes)) return true @@ -184,14 +201,20 @@ func (h *witHandler) acceptDeferredBroadcast(peer *wit.Peer, witness *stateless. if !ok { return false } - // Bind against any deferred candidate's commitment: with multiple candidates - // on file we accept the body if its hash matches one of them. The drain still - // arbitrates which signer is the real producer at import time. + // Bind against the deferred candidates' commitments: with multiple + // candidates on file we accept the body if it is byte-identical to one of + // them (hash match) or, failing that, within the size band of one of them — + // the same size oracle the verified path applies, since honest bodies + // (a pusher's own post-import witness) routinely differ from the BP's bytes. + // The drain still arbitrates which signer is the real producer at import + // time, and import validates the content. + hh := (*handler)(h) bodyHash := stateless.WitnessCommitHash(bodyBytes) - if !(*handler)(h).deferredAnnounces.hasWitnessHash(hash, bodyHash) { - wit2BroadcastByteMismatchMeter.Mark(1) - peer.Log().Warn("wit2: broadcast bytes do not match any deferred announce witnessHash; dropping", - "blockHash", hash, "actual", bodyHash) + if !hh.deferredAnnounces.hasWitnessHash(hash, bodyHash) && + !hh.deferredAnnounces.hasWitnessSizeWithin(hash, uint64(len(bodyBytes)), hh.witnessSizeCeiling) { + wit2BroadcastOversizeMeter.Mark(1) + peer.Log().Warn("wit2: broadcast witness exceeds the size band of every deferred announce; dropping", + "blockHash", hash, "received", len(bodyBytes)) return false } peer.AddKnownWitness(hash) @@ -204,9 +227,9 @@ func (h *witHandler) acceptDeferredBroadcast(peer *wit.Peer, witness *stateless. // we actually know — without it, an unsolicited 16MB body for an arbitrary // hash would be decoded and cached purely on the sender's word. Unknown // headers are dropped silently: a peer racing ahead of our import is early, -// not malicious. For known headers we cannot prove byte-correctness to -// downstream WIT2 peers — the body is not exposed for pre-import serving but -// still flows into the import path. +// not malicious. For known headers there is no BP commitment to bound the +// bytes by, so the body is not exposed for pre-import serving but still flows +// into the import path. func (h *witHandler) acceptUnsignedBroadcast(peer *wit.Peer, hash common.Hash) bool { if h.Chain().GetHeaderByHash(hash) == nil { wit2BroadcastUnknownHeaderDropMeter.Mark(1) @@ -234,11 +257,12 @@ func (h *witHandler) handleWitnessHashesAnnounce(peer *wit.Peer, hashes []common // // Failure policy (enforced in acceptSignedAnnouncement): a header-unknown // announce is deferred silently — no strike, no relay — because it may simply -// be racing ahead of its block. Confirmed misbehavior against a known header -// (bad signature, or signer ≠ scheduled producer) is struck, and a peer that -// reaches wit2MisbehaviorStrikeLimit strikes within the decay window is -// disconnected. Byte-correctness failures at fetch time are handled separately -// in the witness manager. All invalid announcements are also metered. +// be racing ahead of its block. Confirmed misbehavior (bad signature, an +// implausible signed witness size, or signer ≠ scheduled producer for a known +// header) is struck, and a peer that reaches wit2MisbehaviorStrikeLimit +// strikes within the decay window is disconnected. Oversized or import-failing +// witness bytes at fetch time are handled separately in the witness manager. +// All invalid announcements are also metered. func (h *witHandler) handleSignedWitnessAnnouncements(peer *wit.Peer, anns []wit.SignedWitnessAnnouncement) error { wit2RelayInMeter.Mark(int64(len(anns))) @@ -283,16 +307,16 @@ func (h *witHandler) handleSignedWitnessAnnouncements(peer *wit.Peer, anns []wit return nil } -// acceptSignedAnnouncement runs signature recovery and producer-binding for a -// single announcement. Returns true when the announcement is verified and the -// caller should proceed to cache + relay; false when the caller should skip -// it. Strikes are issued only on confirmed misbehavior (bad signature or -// signer ≠ scheduled producer for a known header). Pre-import deferral -// (header not yet local) is silent: no strike, no relay. The announcement is -// stashed in the deferred queue so the chain-head loop can re-evaluate it -// once the block arrives — without that, an announce that races ahead of its -// block is lost permanently and subsequent witness fetches silently skip -// byte-verification. +// acceptSignedAnnouncement runs signature recovery, witness-size plausibility +// and producer-binding for a single announcement. Returns true when the +// announcement is verified and the caller should proceed to cache + relay; +// false when the caller should skip it. Strikes are issued only on confirmed +// misbehavior (bad signature, implausible signed size, or signer ≠ scheduled +// producer for a known header). Pre-import deferral (header not yet local) is +// silent: no strike, no relay. The announcement is stashed in the deferred +// queue so the chain-head loop can re-evaluate it once the block arrives — +// without that, an announce that races ahead of its block is lost permanently +// and subsequent witness fetches silently skip the size-oracle check. func (h *witHandler) acceptSignedAnnouncement(peer *wit.Peer, ann wit.SignedWitnessAnnouncement) bool { signer, err := verifySignedAnnouncement(ann) if err != nil { @@ -302,6 +326,20 @@ func (h *witHandler) acceptSignedAnnouncement(peer *wit.Peer, ann wit.SignedWitn return false } + // The signed WitnessSize is the size oracle every receiver judges servers + // by: a zero value yields a zero band that rejects every honest server, an + // implausibly large one admits arbitrary bloat. The size is part of the + // signed preimage, so a bad value is the signer's — or a forwarding + // relayer's — doing: refuse and strike here, ahead of deferral, so a bad + // size never enters the deferred queue or the signed cache either. + if !(*handler)(h).plausibleSignedWitnessSize(ann.WitnessSize) { + wit2ImplausibleSizeMeter.Mark(1) + peer.Log().Debug("wit2: signed announcement carries an implausible witness size; refusing", + "blockHash", ann.BlockHash, "blockNumber", ann.BlockNumber, "witnessSize", ann.WitnessSize) + (*handler)(h).strikeWit2Peer(peer) + return false + } + ok, headerAvailable := (*handler)(h).isScheduledProducer(signer, ann.BlockNumber, ann.BlockHash) if ok { return true @@ -347,10 +385,10 @@ func (h *handler) relaySignedAnnouncement(senderID string, ann wit.SignedWitness // // WIT2: per-block lookup consults the in-flight body cache before falling back // to chain storage. This lets nodes serve witnesses they have received from -// the network but not yet imported. Byte-correctness blame attaches to the -// server only on hash mismatch (the requester verifies bytes against the BP- -// signed WitnessHash); content-correctness failures during execution attach -// to the BP, so this server is not at additional risk by serving early. +// the network but not yet imported. The in-flight cache holds only bytes +// byte-identical to the BP's signed commitment, so serving them early cannot +// expose this node to a size-band rejection or an import-failure strike +// downstream that the BP would not equally incur. func (h *witHandler) handleGetWitness(peer *wit.Peer, req *wit.GetWitnessPacket) (wit.WitnessPacketResponse, error) { log.Debug("handleGetWitness processing request", "peer", peer.ID(), "reqID", req.RequestId, "witnessPages", len(req.WitnessPages)) diff --git a/eth/handler_wit2.go b/eth/handler_wit2.go index 4113604d19..f77f8d750a 100644 --- a/eth/handler_wit2.go +++ b/eth/handler_wit2.go @@ -2,6 +2,7 @@ package eth import ( "errors" + "math" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" @@ -24,7 +25,9 @@ var ( wit2InvalidSigMeter = metrics.NewRegisteredMeter("eth/wit2/announce/invalid_sig", nil) wit2NotValidatorMeter = metrics.NewRegisteredMeter("eth/wit2/announce/not_validator", nil) wit2DuplicateMeter = metrics.NewRegisteredMeter("eth/wit2/announce/duplicate", nil) - wit2BroadcastByteMismatchMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_byte_mismatch", nil) + wit2BroadcastOversizeMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_oversize", nil) + wit2BroadcastHashDivergenceMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_hash_divergence", nil) + wit2ImplausibleSizeMeter = metrics.NewRegisteredMeter("eth/wit2/announce/implausible_size", nil) wit2BroadcastUnverifiedSkippedMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_unverified_skipped", nil) wit2DeferredPerPeerDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/deferred_per_peer_drop", nil) wit2DeferredPerBlockDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/deferred_per_block_drop", nil) @@ -106,11 +109,12 @@ func (h *handler) flushWitnessWaitersForImported(blockHash common.Hash) { h.pushWitnessBytesToWaiters(blockHash, body) } -// pushWitnessBytesToWaiters decodes verified witness bytes (already checked -// against the BP-signed hash by the caller) and pushes them to waiting peers. -// The decode — re-encoded canonically on send — round-trips to the same bytes, -// so downstream byte-correctness checks still pass. Skipped entirely when no -// peer is waiting, so the common (no-waiter) case pays nothing. +// pushWitnessBytesToWaiters decodes witness bytes this node holds (the BP's own +// bytes from the pre-import cache, or the node's own witness from chain storage) +// and pushes them to waiting peers. The decode — re-encoded canonically on send +// — round-trips to the same bytes, so the receiver's size-oracle check sees the +// same size. Skipped entirely when no peer is waiting, so the common +// (no-waiter) case pays nothing. func (h *handler) pushWitnessBytesToWaiters(hash common.Hash, witnessBytes []byte) { if h.witnessWaiters == nil || len(witnessBytes) == 0 || !h.witnessWaiters.has(hash) { return @@ -195,9 +199,11 @@ func (h *handler) cosendWitnessAnnouncement(blockHash common.Hash, blockNumber u } } -// lookupSignedWitnessHash returns the BP-signed witness hash for a block, if -// the local cache has a verified announcement. Used by the witness manager -// on fetch success to verify byte-correctness against the signed commitment. +// lookupSignedWitnessHash returns the BP-signed witness commitment (hash and +// encoded size) for a block, if the local cache has a verified announcement. +// Used by the witness manager on fetch success as the size oracle: the size +// bounds what is accepted for import, the hash decides whether the bytes are +// the BP's own and may be re-served pre-import. func (h *handler) lookupSignedWitnessHash(blockHash common.Hash) (common.Hash, uint64, bool) { ann, ok := h.signedWitnesses.get(blockHash) if !ok { @@ -206,14 +212,56 @@ func (h *handler) lookupSignedWitnessHash(blockHash common.Hash) (common.Hash, u return ann.WitnessHash, ann.WitnessSize, true } +// witnessSizeCeiling returns the maximum encoded witness size accepted for a +// block whose BP-signed WitnessSize is signedSize — the fetcher's size oracle +// (witnessManager.acceptableWitnessSizeCeiling), applied here to bodies that +// arrive by NewWitness broadcast so both delivery paths judge a witness +// identically. Without a block fetcher (not expected outside tests) no bound is +// applied, mirroring handleWitnessBroadcast, which cannot inject either. +func (h *handler) witnessSizeCeiling(signedSize uint64) uint64 { + if h.blockFetcher == nil { + return math.MaxUint64 + } + return h.blockFetcher.GetWitnessManager().AcceptableWitnessSizeCeiling(signedSize) +} + +// plausibleSignedWitnessSize reports whether a BP-signed WitnessSize can serve +// as a size oracle: it must be non-zero (a zero size yields a zero band that +// rejects every honest server) and no larger than the absolute gas-derived +// witness cap (a size beyond what the block gas limit can produce is not a +// witness the BP can have). Announcements failing this are refused at accept +// time and the sender struck, so a bad size is charged to whoever signed or +// forwarded it rather than to the servers it would later mis-judge. +func (h *handler) plausibleSignedWitnessSize(size uint64) bool { + if size == 0 { + return false + } + if h.blockFetcher == nil { + return true + } + return size <= h.blockFetcher.GetWitnessManager().MaxWitnessSize() +} + +// excludeWitnessSource records that peer must not be asked again for the +// witness of blockHash: the witness it served was accepted on the size oracle +// alone and then failed import (see fetcher.BlockFetcher.importBlocks). +// Consulted by resolveWitnessFetchPeer so the re-fetch reaches another source. +func (h *handler) excludeWitnessSource(peer string, blockHash common.Hash) { + if h.witnessSourceExclusions == nil { + return + } + h.witnessSourceExclusions.add(blockHash, peer) +} + // cacheVerifiedWitnessForServing receives canonical-encoded witness bytes from -// the fetcher after a successful, byte-verified paged download and stores them -// in the in-flight cache so peers can fetch the body before this node finishes -// chain-write. Bytes here have already passed verifyAgainstSignedHash (when a -// signed announcement was on file), or arrived via WIT1 unsigned path; in both -// cases they're the same bytes the upstream peer agreed upon, so serving them -// to downstream peers cannot expose this node to byte-mismatch drops beyond -// the upstream's already-incurred risk. +// the fetcher after a paged download whose bytes are byte-identical to the +// BP-signed commitment, and stores them in the in-flight cache so peers can +// fetch the body before this node finishes chain-write. The fetcher hands over +// only such bytes (a within-band non-identical variant, or a WIT1 fetch with no +// commitment on file, arrives here as an empty body and is not cached), so the +// pre-import serving path carries the BP's own bytes exclusively and serving +// them early cannot expose this node to a downstream size-band rejection or +// import-failure strike the BP would not equally incur. func (h *handler) cacheVerifiedWitnessForServing(blockHash common.Hash, witnessBytes []byte, witnessHash common.Hash) { if h.pendingWitnessBodies == nil { return diff --git a/eth/handler_wit2_announces.go b/eth/handler_wit2_announces.go index d07a7f45fc..2a49ef9681 100644 --- a/eth/handler_wit2_announces.go +++ b/eth/handler_wit2_announces.go @@ -402,6 +402,26 @@ func (c *deferredAnnounceCache) hasWitnessHash(blockHash common.Hash, witnessHas return false } +// hasWitnessSizeWithin reports whether a fresh candidate for blockHash has a +// signed WitnessSize whose accepted band (ceiling(candidateSize)) admits size. +// Used by the broadcast path to bind pushed bytes to a pending (deferred, not +// yet producer-verified) commitment under the non-determinism-tolerant size +// oracle when no candidate's hash matches the bytes exactly. +func (c *deferredAnnounceCache) hasWitnessSizeWithin(blockHash common.Hash, size uint64, ceiling func(signedSize uint64) uint64) bool { + c.mu.RLock() + defer c.mu.RUnlock() + cutoff := time.Now().Add(-wit2AnnounceTTL) + for _, e := range c.entries[blockHash] { + if e.receivedAt.Before(cutoff) { + continue + } + if size <= ceiling(e.announcement.WitnessSize) { + return true + } + } + return false +} + // has reports whether any fresh candidate exists for blockHash. func (c *deferredAnnounceCache) has(blockHash common.Hash) bool { c.mu.RLock() diff --git a/eth/handler_wit2_caches_test.go b/eth/handler_wit2_caches_test.go index 0b81a12db1..55d8ac0917 100644 --- a/eth/handler_wit2_caches_test.go +++ b/eth/handler_wit2_caches_test.go @@ -593,6 +593,7 @@ func TestHandleSignedWitnessAnnouncementsAcceptCacheRelayAndDedup(t *testing.T) BlockHash: hash, BlockNumber: header.Number.Uint64(), WitnessHash: common.HexToHash("0xab"), + WitnessSize: 1000, } signTestAnnouncement(t, &ann) @@ -658,6 +659,7 @@ func TestAcceptSignedAnnouncementStrikesOnNumberMismatch(t *testing.T) { BlockHash: header.Hash(), BlockNumber: header.Number.Uint64() + 1, // contradicts the local header WitnessHash: common.HexToHash("0xcc"), + WitnessSize: 1000, // plausible, so the number-mismatch branch (not the size check) is what strikes } signTestAnnouncement(t, &ann) @@ -1072,3 +1074,149 @@ func TestCanonicalWitnessHashStorageGate(t *testing.T) { require.True(t, ok) require.Equal(t, stateless.WitnessCommitHash(body), got) } + +// TestAcceptSignedAnnouncementRejectsImplausibleWitnessSize pins the announce- +// time sanity check on the size oracle: a BP-signed WitnessSize of zero (which +// would yield a zero band and reject every honest server) or above the absolute +// gas-derived cap is refused outright — not cached, not deferred — and the +// sender is struck, so a bad size is charged to whoever signed or forwarded it +// rather than to the servers it would later mis-judge. +func TestAcceptSignedAnnouncementRejectsImplausibleWitnessSize(t *testing.T) { + h := newTestHandler() + defer h.close() + witH := (*witHandler)(h.handler) + + maxSize := h.handler.blockFetcher.GetWitnessManager().MaxWitnessSize() + require.NotZero(t, maxSize) + + for _, tc := range []struct { + name string + size uint64 + }{ + {"zero", 0}, + {"above absolute cap", maxSize + 1}, + } { + t.Run(tc.name, func(t *testing.T) { + peer, cleanup := newTestWit2PeerWithReader() + defer cleanup() + + header := &types.Header{Number: big.NewInt(4400 + int64(tc.size%7))} // NOT written: would otherwise defer + ann := wit.SignedWitnessAnnouncement{ + BlockHash: header.Hash(), + BlockNumber: header.Number.Uint64(), + WitnessHash: common.HexToHash("0xab"), + WitnessSize: tc.size, + } + signTestAnnouncement(t, &ann) + + require.False(t, witH.acceptSignedAnnouncement(peer, ann)) + _, cached := h.handler.signedWitnesses.get(ann.BlockHash) + require.False(t, cached, "implausible size must not be cached") + require.False(t, h.handler.deferredAnnounces.has(ann.BlockHash), "implausible size must not be deferred even though the header is unknown") + + h.handler.wit2PeerTracker.mu.Lock() + strikes := len(h.handler.wit2PeerTracker.state[peer.ID()].strikes) + h.handler.wit2PeerTracker.mu.Unlock() + require.Equal(t, 1, strikes, "the announcer must be struck for an implausible signed size") + }) + } + + // Control: the same announce with a plausible size is deferred normally + // (header unknown) without a strike. + peer, cleanup := newTestWit2PeerWithReader() + defer cleanup() + header := &types.Header{Number: big.NewInt(4499)} + ann := wit.SignedWitnessAnnouncement{ + BlockHash: header.Hash(), + BlockNumber: header.Number.Uint64(), + WitnessHash: common.HexToHash("0xab"), + WitnessSize: maxSize, + } + signTestAnnouncement(t, &ann) + require.False(t, witH.acceptSignedAnnouncement(peer, ann)) + require.True(t, h.handler.deferredAnnounces.has(ann.BlockHash), "a plausible size at the cap must defer on an unknown header") + _, tracked := h.handler.wit2PeerTracker.state[peer.ID()] + require.False(t, tracked, "deferral must not strike") +} + +// TestResolveWitnessFetchPeerSkipsExcludedSource pins the source exclusion the +// block fetcher relies on for its re-fetch after an import failure: a peer +// excluded for a block is skipped at every tier of resolveWitnessFetchPeer, the +// exclusion is per block, and it is released when the block imports. +func TestResolveWitnessFetchPeerSkipsExcludedSource(t *testing.T) { + h := newTestHandler() + defer h.close() + ethH := (*ethHandler)(h.handler) + + peerA, cleanupA := registerEthWitPeer(t, h, wit.WIT2) + defer cleanupA() + peerB, cleanupB := registerEthWitPeer(t, h, wit.WIT2) + defer cleanupB() + + hash := common.HexToHash("0xb10c") + other := common.HexToHash("0xb10d") + peerA.AddKnownWitness(hash) + peerB.AddKnownWitness(hash) + peerA.AddKnownWitness(other) + + // Both hold the body: either may be picked. + require.NotNil(t, ethH.resolveWitnessFetchPeer(hash)) + + // Exclude A for this block: over many resolutions B must always win. + h.handler.excludeWitnessSource(peerA.ID(), hash) + for i := 0; i < 32; i++ { + got := ethH.resolveWitnessFetchPeer(hash) + require.NotNil(t, got, "B still holds the body and must be offered") + require.Equal(t, peerB.ID(), got.ID(), "excluded peer A must never be resolved for the block") + } + // The exclusion is per block: A is still a valid source for another hash. + got := ethH.resolveWitnessFetchPeer(other) + require.NotNil(t, got) + require.Equal(t, peerA.ID(), got.ID()) + + // Exclude B too: no source remains, rather than falling back to A. + h.handler.excludeWitnessSource(peerB.ID(), hash) + require.Nil(t, ethH.resolveWitnessFetchPeer(hash)) + + // The deferred-relayer tier honours the exclusion as well. + h.handler.deferredAnnounces.put(wit.SignedWitnessAnnouncement{ + BlockHash: hash, BlockNumber: 1, WitnessHash: common.HexToHash("0x01"), WitnessSize: 10, + Signature: make([]byte, wit.SignatureLength), + }, peerA.ID()) + require.Nil(t, ethH.resolveWitnessFetchPeer(hash), "an excluded deferred relayer must not be offered either") + + // Block imports: exclusions for it are released. + h.handler.onBlockImported(hash) + require.NotNil(t, ethH.resolveWitnessFetchPeer(hash)) +} + +// TestWitnessSourceExclusionSetLifecycle pins add/excluded/drop and the TTL +// backstop of the per-block source exclusion set. +func TestWitnessSourceExclusionSetLifecycle(t *testing.T) { + s := newWitnessSourceExclusionSet() + hash := common.HexToHash("0x01") + + require.False(t, s.excluded(hash, "p1")) + s.add(hash, "") + require.Empty(t, s.entries, "an empty peer id must not be recorded") + + s.add(hash, "p1") + require.True(t, s.excluded(hash, "p1")) + require.False(t, s.excluded(hash, "p2")) + require.False(t, s.excluded(common.HexToHash("0x02"), "p1"), "exclusions are per block") + + s.drop(hash) + require.False(t, s.excluded(hash, "p1")) + + // TTL backstop: a stale entry stops excluding and is swept on the next add. + s.add(hash, "p1") + s.mu.Lock() + s.entries[hash]["p1"] = time.Now().Add(-2 * witnessSourceExclusionTTL) + s.mu.Unlock() + require.False(t, s.excluded(hash, "p1"), "an exclusion past its TTL must lapse") + s.add(common.HexToHash("0x03"), "p9") + s.mu.Lock() + _, still := s.entries[hash] + s.mu.Unlock() + require.False(t, still, "stale entries must be swept on add") +} diff --git a/eth/handler_wit2_exclusions.go b/eth/handler_wit2_exclusions.go new file mode 100644 index 0000000000..7423c45aa2 --- /dev/null +++ b/eth/handler_wit2_exclusions.go @@ -0,0 +1,80 @@ +package eth + +import ( + "sync" + "time" + + "github.com/ethereum/go-ethereum/common" +) + +// witnessSourceExclusionTTL bounds how long an excluded (block, peer) pair is +// remembered. Exclusions are also dropped the moment the block imports +// (onBlockImported); the TTL is a backstop for blocks that never do — the +// block fetcher gives a block up after maxWitnessImportRetries — and is +// generous relative to the seconds a block normally takes to resolve. +const witnessSourceExclusionTTL = 2 * time.Minute + +// witnessSourceExclusionSet remembers, per block, the peers whose served +// witness for that block was accepted on the WIT2 size oracle alone and then +// failed import. Such a peer is skipped when the fetcher resolves a witness +// source for the block (resolveWitnessFetchPeer), so the re-fetch reaches a +// different peer instead of pulling the same unusable bytes again. Bounded by +// the TTL sweep on every add and by drop() on block import. +type witnessSourceExclusionSet struct { + mu sync.Mutex + entries map[common.Hash]map[string]time.Time +} + +func newWitnessSourceExclusionSet() *witnessSourceExclusionSet { + return &witnessSourceExclusionSet{entries: make(map[common.Hash]map[string]time.Time)} +} + +// add excludes peer as a witness source for blockHash. +func (s *witnessSourceExclusionSet) add(blockHash common.Hash, peer string) { + if peer == "" { + return + } + s.mu.Lock() + defer s.mu.Unlock() + s.gcLocked() + peers := s.entries[blockHash] + if peers == nil { + peers = make(map[string]time.Time) + s.entries[blockHash] = peers + } + peers[peer] = time.Now() +} + +// excluded reports whether peer is currently excluded as a witness source for +// blockHash. +func (s *witnessSourceExclusionSet) excluded(blockHash common.Hash, peer string) bool { + s.mu.Lock() + defer s.mu.Unlock() + at, ok := s.entries[blockHash][peer] + if !ok { + return false + } + return time.Since(at) <= witnessSourceExclusionTTL +} + +// drop forgets every exclusion recorded for blockHash. +func (s *witnessSourceExclusionSet) drop(blockHash common.Hash) { + s.mu.Lock() + defer s.mu.Unlock() + delete(s.entries, blockHash) +} + +// gcLocked removes exclusions past the TTL. Caller must hold the lock. +func (s *witnessSourceExclusionSet) gcLocked() { + cutoff := time.Now().Add(-witnessSourceExclusionTTL) + for hash, peers := range s.entries { + for peer, at := range peers { + if at.Before(cutoff) { + delete(peers, peer) + } + } + if len(peers) == 0 { + delete(s.entries, hash) + } + } +} diff --git a/eth/handler_wit2_peer.go b/eth/handler_wit2_peer.go index 459c40a866..bf98dc1bf6 100644 --- a/eth/handler_wit2_peer.go +++ b/eth/handler_wit2_peer.go @@ -32,11 +32,12 @@ func (h *handler) strikeWit2Peer(peer *wit.Peer) { h.removePeer(peer.ID()) } -// strikeWit2PeerByID records a WIT2 byte-serving strike against the peer with the +// strikeWit2PeerByID records a WIT2 witness-serving strike (oversized bytes, or a +// size-oracle-accepted witness that failed import) against the peer with the // given id and disconnects+jails it once the threshold is crossed. The witness -// manager detects byte-mismatches by peer id (not *wit.Peer), so this is its -// entry point into the same strike budget strikeWit2Peer uses for bad announces: -// sustained misbehavior across either surface disconnects the peer. +// manager and block fetcher know servers by peer id (not *wit.Peer), so this is +// their entry point into the same strike budget strikeWit2Peer uses for bad +// announces: sustained misbehavior across either surface disconnects the peer. func (h *handler) strikeWit2PeerByID(id string) { if h.wit2PeerTracker == nil { return @@ -110,4 +111,7 @@ func (h *handler) onBlockImported(blockHash common.Hash) { if h.pendingWitnessBodies != nil { h.pendingWitnessBodies.drop(blockHash) } + if h.witnessSourceExclusions != nil { + h.witnessSourceExclusions.drop(blockHash) + } } diff --git a/eth/handler_wit2_test.go b/eth/handler_wit2_test.go index bdc776585b..e78ecc95fb 100644 --- a/eth/handler_wit2_test.go +++ b/eth/handler_wit2_test.go @@ -877,6 +877,7 @@ func TestDeferredSignedAnnounceDrainedAfterHeaderArrives(t *testing.T) { BlockHash: blockHash, BlockNumber: header.Number.Uint64(), WitnessHash: common.HexToHash("0xc0ffee01"), + WitnessSize: 1000, } digest := wit.WitnessAnnouncementSigningHash(ann.BlockHash, ann.BlockNumber, ann.WitnessHash, ann.WitnessSize) sig, err := crypto.Sign(digest.Bytes(), key) @@ -938,7 +939,7 @@ func TestDeferredDrainPromotesHonestAmongForged(t *testing.T) { sign := func(num uint64, wh common.Hash) wit.SignedWitnessAnnouncement { key, err := crypto.GenerateKey() require.NoError(t, err) - a := wit.SignedWitnessAnnouncement{BlockHash: blockHash, BlockNumber: num, WitnessHash: wh} + a := wit.SignedWitnessAnnouncement{BlockHash: blockHash, BlockNumber: num, WitnessHash: wh, WitnessSize: 1000} d := wit.WitnessAnnouncementSigningHash(a.BlockHash, a.BlockNumber, a.WitnessHash, a.WitnessSize) s, err := crypto.Sign(d.Bytes(), key) require.NoError(t, err) @@ -1174,13 +1175,14 @@ func TestVerifyScheduledProducerUnrecoverableSealerDoesNotConfirmMisbehavior(t * } } -// TestHandleWitnessBroadcastByteMismatchNotInjected guards the verification -// boundary of the broadcast path: when a BP-signed witnessHash is on file and -// a broadcast body does NOT match it, the witness must be fully rejected — not -// cached for serving, sender not marked as a body-holder, and not injected -// into the fetcher. Anything less makes the full-body broadcast a bypass of -// the byte verification the paged-fetch path enforces. -func TestHandleWitnessBroadcastByteMismatchNotInjected(t *testing.T) { +// TestHandleWitnessBroadcastDivergentWithinBandImportsWithoutServing pins the +// size-oracle semantics of the broadcast path: when a BP-signed announcement is +// on file and a broadcast body hashes differently from it but its size is +// within the signed band (a valid non-deterministic variant — the normal case +// for a pusher's own post-import witness), the witness is accepted for IMPORT +// (sender marked as a body-holder, injected) but NOT cached for pre-import +// serving: the serving fast path carries the BP's own bytes only. +func TestHandleWitnessBroadcastDivergentWithinBandImportsWithoutServing(t *testing.T) { h := newTestHandler() defer h.close() @@ -1194,23 +1196,65 @@ func TestHandleWitnessBroadcastByteMismatchNotInjected(t *testing.T) { witness, err := stateless.NewWitness(header, nil) require.NoError(t, err) + var buf bytes.Buffer + require.NoError(t, witness.EncodeRLP(&buf)) + + // Signed announcement commits to a DIFFERENT witnessHash than the broadcast + // bytes hash to, with a size the body is within band of. + h.handler.signedWitnesses.putIfNewer(wit.SignedWitnessAnnouncement{ + BlockHash: hash, + BlockNumber: header.Number.Uint64(), + WitnessHash: common.HexToHash("0xdeadbeef"), + WitnessSize: uint64(buf.Len()), + Signature: make([]byte, wit.SignatureLength), + }) + + require.NoError(t, witH.handleWitnessBroadcast(peer, witness)) + + if !peer.KnownWitnessContainsHash(hash) { + t.Fatal("within-band divergent broadcast was not accepted for import; waiter-push of a pusher's own witness is dead under non-determinism") + } + if _, _, ok := h.handler.pendingWitnessBodies.get(hash); ok { + t.Fatal("divergent broadcast populated the pre-import serving cache; only the BP's own bytes may be re-served") + } +} + +// TestHandleWitnessBroadcastOversizeDropped guards the one rejection the +// broadcast size oracle keeps: a body beyond the accepted band around the +// BP-signed WitnessSize is dropped outright — not cached, sender not marked as a +// body-holder, nothing injected. +func TestHandleWitnessBroadcastOversizeDropped(t *testing.T) { + h := newTestHandler() + defer h.close() + + witH := (*witHandler)(h.handler) + peer, cleanup := newTestWit2PeerWithReader() + defer cleanup() - // Signed announcement on file commits to a DIFFERENT witnessHash than the - // broadcast bytes will hash to. + header := &types.Header{Number: big.NewInt(7780)} + hash := header.Hash() + rawdb.WriteHeader(h.chain.DB(), header) + + witness, err := stateless.NewWitness(header, nil) + require.NoError(t, err) + + // A signed size of 1 byte gives a ceiling of a few bytes; any real witness + // encoding is far beyond it. h.handler.signedWitnesses.putIfNewer(wit.SignedWitnessAnnouncement{ BlockHash: hash, BlockNumber: header.Number.Uint64(), WitnessHash: common.HexToHash("0xdeadbeef"), + WitnessSize: 1, Signature: make([]byte, wit.SignatureLength), }) require.NoError(t, witH.handleWitnessBroadcast(peer, witness)) if _, _, ok := h.handler.pendingWitnessBodies.get(hash); ok { - t.Fatal("byte-mismatched broadcast populated the pre-import serving cache") + t.Fatal("oversized broadcast populated the pre-import serving cache") } if peer.KnownWitnessContainsHash(hash) { - t.Fatal("byte-mismatched broadcast marked the sender as a body-holder; fetcher would pull garbage from it") + t.Fatal("oversized broadcast marked the sender as a body-holder; fetcher would pull oversized bytes from it") } } @@ -1331,19 +1375,43 @@ func TestHandleWitnessBroadcastAcceptedWhileAnnounceDeferred(t *testing.T) { t.Fatal("deferred entry was consumed; post-import drain can no longer verify/promote/relay") } - // Bytes contradicting the deferred commitment must still drop. - other, err := stateless.NewWitness(&types.Header{Number: big.NewInt(9002), Extra: []byte{0x1}}, nil) + // A body that is not the deferred commitment's bytes but is within its + // size band is a valid non-deterministic variant: accepted for import only, + // exactly like the verified path. + variant, err := stateless.NewWitness(&types.Header{Number: big.NewInt(9002), Extra: []byte{0x1}}, nil) + require.NoError(t, err) + variantHash := variant.Header().Hash() + var variantBuf bytes.Buffer + require.NoError(t, variant.EncodeRLP(&variantBuf)) + h.handler.deferredAnnounces.put(wit.SignedWitnessAnnouncement{ + BlockHash: variantHash, + BlockNumber: variant.Header().Number.Uint64(), + WitnessHash: common.HexToHash("0xfeed"), + WitnessSize: uint64(variantBuf.Len()), + Signature: make([]byte, wit.SignatureLength), + }, "upstream-peer") + require.NoError(t, witH.handleWitnessBroadcast(peer, variant)) + if !peer.KnownWitnessContainsHash(variantHash) { + t.Fatal("within-band variant of a deferred commitment was dropped; waiter-push to a stateless tip is dead under non-determinism") + } + if _, _, ok := h.handler.pendingWitnessBodies.get(variantHash); ok { + t.Fatal("import-only acceptance of a variant must not populate the serving cache") + } + + // Bytes beyond the size band of every deferred commitment must still drop. + oversize, err := stateless.NewWitness(&types.Header{Number: big.NewInt(9003), Extra: []byte{0x2}}, nil) require.NoError(t, err) - otherHash := other.Header().Hash() + oversizeHash := oversize.Header().Hash() h.handler.deferredAnnounces.put(wit.SignedWitnessAnnouncement{ - BlockHash: otherHash, - BlockNumber: other.Header().Number.Uint64(), + BlockHash: oversizeHash, + BlockNumber: oversize.Header().Number.Uint64(), WitnessHash: common.HexToHash("0xfeed"), + WitnessSize: 1, // ceiling of a few bytes; any real encoding exceeds it Signature: make([]byte, wit.SignatureLength), }, "upstream-peer") - require.NoError(t, witH.handleWitnessBroadcast(peer, other)) - if peer.KnownWitnessContainsHash(otherHash) { - t.Fatal("bytes contradicting the deferred commitment were accepted") + require.NoError(t, witH.handleWitnessBroadcast(peer, oversize)) + if peer.KnownWitnessContainsHash(oversizeHash) { + t.Fatal("bytes beyond the deferred commitment's size band were accepted") } } diff --git a/eth/handler_wit_relay_fetch.go b/eth/handler_wit_relay_fetch.go index d28cb027f9..e762ca3e86 100644 --- a/eth/handler_wit_relay_fetch.go +++ b/eth/handler_wit_relay_fetch.go @@ -81,12 +81,18 @@ type namedWitnessPeer struct { peer WitnessPeer } -// fetchAndVerifyWitness tries each candidate in order, verifying fetched -// bytes against the already-verified signed hash before trusting them. -// Byte mismatch or decode failure on one candidate does not abort the -// whole attempt — a different candidate might have the real bytes; only a -// candidate serving bytes that don't match the signed commitment is -// distrusted, individually, exactly like acceptSignedBroadcast's model. +// fetchAndVerifyWitness tries each candidate in order, requiring the fetched +// bytes to be byte-identical to the already-verified signed hash before +// trusting them. This gate is deliberately stricter than the import paths' +// size oracle: a relay fetch exists only to SERVE the body to waiters, this +// node never imports it, and the pre-import serving path carries the BP's own +// bytes exclusively (see acceptSignedBroadcast). A within-band but +// non-identical witness — the normal case for any upstream that generated its +// own witness on import — is therefore skipped here, not distrusted: the +// relay fetch is only expected to succeed against the producer or a node still +// holding the producer's bytes in its own pre-import cache, and otherwise the +// waiters fall back to the pull path. Decode failure on one candidate does not +// abort the whole attempt either — a different candidate might have the bytes. func fetchAndVerifyWitness(candidates []namedWitnessPeer, blockHash, wantHash common.Hash) ([]byte, *stateless.Witness, string, bool) { for _, c := range candidates { log.Info("wit2: relay fetch attempt", "hash", blockHash, "upstream", c.id) @@ -96,9 +102,9 @@ func fetchAndVerifyWitness(candidates []namedWitnessPeer, blockHash, wantHash co continue // this candidate came up empty/errored; try the next one } if got := stateless.WitnessCommitHash(data); got != wantHash { - log.Warn("wit2: relay fetch byte mismatch against signed hash; dropping", - "hash", blockHash, "peer", c.id, "expected", wantHash, "actual", got) - continue // this peer served bad bytes; a different candidate might not + log.Debug("wit2: relay fetch got a witness that is not the BP's bytes; not re-serving it", + "hash", blockHash, "peer", c.id, "signed", wantHash, "actual", got) + continue // valid-but-different bytes are not re-served pre-import; another candidate may hold the BP's } var witness stateless.Witness if err := rlp.DecodeBytes(data, &witness); err != nil { diff --git a/eth/peerset.go b/eth/peerset.go index 79c868b13e..ed4e9a00c4 100644 --- a/eth/peerset.go +++ b/eth/peerset.go @@ -330,9 +330,10 @@ func (ps *peerSet) peersWithWitnessCandidates(hash common.Hash) []*ethPeer { // the signed announce arrives long before the body broadcast, and the only // peer that could serve us bytes is the one that forwarded the announce. // -// Asking an announce-only peer is safe because byte-blame in -// witnessManager.verifyAgainstSignedHash only drops on a confirmed hash -// mismatch — empty/unavailable responses surface as soft failures, not drops. +// Asking an announce-only peer is safe because witnessManager.verifyAgainstSignedHash +// only strikes a server for bytes beyond the BP-signed size band (or, later, +// for a size-oracle-accepted witness that fails import) — empty/unavailable +// responses surface as soft failures, not strikes or drops. func (ps *peerSet) getOnePeerWithWitness(hash common.Hash) *ethPeer { ps.lock.RLock() defer ps.lock.RUnlock() diff --git a/eth/protocols/wit/protocol.go b/eth/protocols/wit/protocol.go index b53b3f8143..ac58c25609 100644 --- a/eth/protocols/wit/protocol.go +++ b/eth/protocols/wit/protocol.go @@ -16,8 +16,12 @@ const ( // WIT2 adds BP-signed witness announcements, allowing peers to fast-validate // announces via signature recovery (microseconds) instead of full block // execution (~500ms). Signed announces are safe to relay transitively - // because byte-correctness is verified at fetch time against the signed - // witness hash; content-correctness blame attaches to the BP signer. + // because the announcement commits to the producer's witness size, which + // bounds what a receiver accepts from any server at fetch time (witnesses + // are non-deterministic, so the signed hash identifies the BP's own bytes + // rather than the only valid ones); content-correctness is arbitrated at + // import and blame attaches to the BP signer, or to a server whose + // non-identical witness fails import. WIT2 = 3 ) From 80e3f34f952ac8e68247dabd5a47fd435a71f93d Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Fri, 18 Sep 2026 08:09:14 -0400 Subject: [PATCH 04/10] eth: keep the WIT2 fetcher wiring comment off the lines #2417 touches #2417 changes the NewBlockFetcher call directly above the WIT2 striker wiring; editing the adjacent comment here made the two branches conflict when merged together even though each merges cleanly into the candidate. Leave the striker comment as it was and document the size-oracle extension (import-failure strike + source exclusion) in its own block below. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Brme9KQBd7fZBMnMVEhAZU --- eth/handler.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/eth/handler.go b/eth/handler.go index 6d6319bcc3..7bead9a35e 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -384,11 +384,15 @@ func newHandler(config *handlerConfig) (*handler, error) { } h.blockFetcher = fetcher.NewBlockFetcher(false, nil, h.chain.GetBlockByHash, validator, h.BroadcastBlock, heighter, h.chain.CurrentHeader, nil, inserter, h.removePeer, h.jailPeer, h.enableBlockTracking, h.statelessSync.Load() || h.syncWithWitnesses, config.gasCeil, h.lookupSignedWitnessHash, h.cacheVerifiedWitnessForServing) - // WIT2: penalize a peer that serves a witness beyond the BP-signed size band, - // or one accepted on the size oracle alone that then fails import (strike, - // not drop — see strikeWit2PeerByID); in the latter case also stop asking - // that peer for this block's witness so the re-fetch lands elsewhere. + // WIT2: penalize a peer that serves a non-empty witness whose bytes mismatch + // the BP-signed commitment (strike, not drop — see strikeWit2PeerByID). h.blockFetcher.SetWitnessServerStriker(h.strikeWit2PeerByID) + + // WIT2 size oracle: the striker above fires for a witness beyond the + // BP-signed size band, and for one accepted on the size oracle alone that + // then fails import. In the latter case the fetcher also asks us to stop + // offering that peer as a witness source for the block, so its re-fetch + // lands on a different peer (see resolveWitnessFetchPeer). h.blockFetcher.SetWitnessSourceExcluder(h.excludeWitnessSource) fetchTx := func(peer string, requestID uint64, hashes []common.Hash) error { From b0f0dffd5b2c4b3bd7c548413568dc3e0f891b72 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 21 Sep 2026 15:32:08 -0400 Subject: [PATCH 05/10] eth/fetcher: charge a size-oracle import failure only when the witness could have caused it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit chargeDivergedWitnessImportFailure struck and excluded the serving peer on any insertChain error. "Diverged" is the normal case on a stateless node (every node persists its own generated witness), so every import failure — including a contract bytecode missing from local disk, which the downloader heals and the witness never carried, or an interrupted insert — would have struck two honest witness sources per block until the node had none left. Gate the charge on a positive allowlist of witness-attributable failures: a missing trie node (incomplete witness), and execution against the witness disagreeing with the header (ErrStatelessStateRootMismatch, ErrGasUsedMismatch, ErrReceiptRootMismatch, ErrBloomMismatch, ErrRequestsHashMismatch, and the fmt-built "invalid merkle root" / stateless self-validation mismatch messages). Everything else — missing code, interrupted or stopped chain, whitelist mismatch, header or DB errors, unknown errors — is not charged. Also: drop the unreachable absBytes == 0 guard in acceptableWitnessSizeCeiling (the page threshold floors at one page); make the deferred-broadcast hash-match test use a size band that would reject the body, so only the hash match admits it; cover the handler size-oracle helpers without a fetcher and the deferred size-band binding's boundary and TTL. Tests: TestChargeDivergedWitnessImportFailureIgnoresNonWitnessErrors, TestIsWitnessAttributableImportError, TestSizeOracleHelpersWithoutFetcher, TestDeferredAnnounceCacheHasWitnessSizeWithin; existing charge/re-fetch tests now use attributable errors. --- eth/fetcher/block_fetcher.go | 19 ++++- eth/fetcher/witness_import_errors.go | 63 ++++++++++++++ eth/fetcher/witness_import_failure_test.go | 97 +++++++++++++++++++++- eth/fetcher/witness_manager_wit2.go | 8 +- eth/handler_wit2_caches_test.go | 37 +++++++++ eth/handler_wit2_test.go | 1 + 6 files changed, 214 insertions(+), 11 deletions(-) create mode 100644 eth/fetcher/witness_import_errors.go diff --git a/eth/fetcher/block_fetcher.go b/eth/fetcher/block_fetcher.go index deaaf999af..b1d2f1889f 100644 --- a/eth/fetcher/block_fetcher.go +++ b/eth/fetcher/block_fetcher.go @@ -1334,15 +1334,28 @@ func (f *BlockFetcher) importBlocks(op *blockOrHeaderInject) { // chargeDivergedWitnessImportFailure applies the WIT2 consequence of an import // failure to the peer that served the block's witness, when that witness was -// accepted on the size oracle alone. It strikes the peer, excludes it as a -// witness source for this block, and reports whether the block should be -// handed back to the witness manager for a re-fetch (false once the retry +// accepted on the size oracle alone AND the failure is one the witness could +// have caused (isWitnessAttributableImportError). It strikes the peer, excludes +// it as a witness source for this block, and reports whether the block should +// be handed back to the witness manager for a re-fetch (false once the retry // budget is spent, or when the witness was not a fetched, diverged one). +// +// The error gate matters because "diverged" is the normal case — every node +// persists its own generated witness, so nearly every witness fetched from +// anyone but the BP differs from the signed hash. Charging every import +// failure would let a local problem (a contract bytecode missing from disk, +// which the downloader heals; an interrupted insert) strike and exclude two +// honest witness sources per block until the node has none left. func (f *BlockFetcher) chargeDivergedWitnessImportFailure(op *blockOrHeaderInject, importErr error) bool { if op.witness == nil || !op.witnessDiverged || op.witnessPeer == "" { return false } hash := op.hash() + if !isWitnessAttributableImportError(importErr) { + log.Debug("Import failed for a reason the witness server did not cause; not charging it", + "server", op.witnessPeer, "number", op.number(), "hash", hash, "err", importErr) + return false + } witnessImportFailureMeter.Mark(1) log.Warn("Import failed with a witness accepted on the WIT2 size oracle; striking its server", "server", op.witnessPeer, "number", op.number(), "hash", hash, "attempt", op.witnessImportFailures+1, "err", importErr) diff --git a/eth/fetcher/witness_import_errors.go b/eth/fetcher/witness_import_errors.go new file mode 100644 index 0000000000..0bcb2c6ab9 --- /dev/null +++ b/eth/fetcher/witness_import_errors.go @@ -0,0 +1,63 @@ +package fetcher + +import ( + "errors" + "strings" + + "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/trie" +) + +// witnessAttributableMismatchErrors are the import-validation sentinels that +// mean execution against the delivered witness produced a result the header +// does not commit to. On a stateless node the witness is the only state input, +// so a mismatch is something the witness bytes could have caused. +var witnessAttributableMismatchErrors = []error{ + core.ErrStatelessStateRootMismatch, + core.ErrGasUsedMismatch, + core.ErrReceiptRootMismatch, + core.ErrBloomMismatch, + core.ErrRequestsHashMismatch, +} + +// witnessAttributableMessageFragments cover the mismatch errors that are built +// with fmt.Errorf and have no sentinel to match on. +var witnessAttributableMessageFragments = []string{ + "invalid merkle root", // core.BlockValidator.ValidateState: post-state root + "stateless self-validation root mismatch", // core.ExecuteStateless cross-check, full-node path + "stateless self-validation receipt root mismatch", // same, receipt root +} + +// isWitnessAttributableImportError reports whether an insertChain failure is +// one the delivered witness could have caused, and so may be charged to the +// peer that served it. It is a positive allowlist: an incomplete witness +// (missing trie node), or execution against the witness disagreeing with the +// header (state root, receipt root, gas used, bloom, requests hash). Every other +// failure — a contract bytecode missing from the local disk (fetchable and +// healed by the downloader; not something the witness carries), an interrupted +// or stopped chain, a whitelist/milestone mismatch, a header or DB error — is +// the local node's or the block's problem, and charging the server for it would +// strike honest peers on every such block. Unknown errors are not charged. +func isWitnessAttributableImportError(err error) bool { + if err == nil { + return false + } + // The witness did not carry a trie node the block reads: an incomplete + // witness is something the server did produce. + var missingNode *trie.MissingNodeError + if errors.As(err, &missingNode) { + return true + } + for _, sentinel := range witnessAttributableMismatchErrors { + if errors.Is(err, sentinel) { + return true + } + } + msg := err.Error() + for _, fragment := range witnessAttributableMessageFragments { + if strings.Contains(msg, fragment) { + return true + } + } + return false +} diff --git a/eth/fetcher/witness_import_failure_test.go b/eth/fetcher/witness_import_failure_test.go index 24cf31f111..d5babcfaf5 100644 --- a/eth/fetcher/witness_import_failure_test.go +++ b/eth/fetcher/witness_import_failure_test.go @@ -11,9 +11,12 @@ import ( "time" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/stateless" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/eth/downloader/whitelist" "github.com/ethereum/go-ethereum/eth/protocols/eth" + "github.com/ethereum/go-ethereum/trie" ) // TestAcceptableWitnessSizeCeilingDegenerateSignedSizes pins the two inputs that @@ -83,7 +86,7 @@ func TestChargeDivergedWitnessImportFailure(t *testing.T) { block := createTestBlock(501) witness := createTestWitnessForBlock(block) noopFetch := func(common.Hash, chan *eth.Response) (*eth.Request, error) { return nil, errors.New("noop") } - importErr := errors.New("stateless self-validation failed") + importErr := fmt.Errorf("%w (remote: 1 local: 2)", core.ErrGasUsedMismatch) // witness-attributable // BP-identical witness (not diverged): the BP's fault, nothing charged. identical := &blockOrHeaderInject{origin: "o", block: block, witness: witness, witnessPeer: "srv", fetchWitness: noopFetch} @@ -137,6 +140,96 @@ func TestChargeDivergedWitnessImportFailure(t *testing.T) { } } +// TestChargeDivergedWitnessImportFailureIgnoresNonWitnessErrors pins the error +// gate: an import failure the witness could not have caused — a local +// interruption, a whitelist mismatch, a stopped chain, an unknown error — is not +// charged to the serving peer even when the witness was fetched and diverged. +// Without this gate every import failure on a stateless node would strike and +// exclude honest witness sources, since diverged is the normal case there. +func TestChargeDivergedWitnessImportFailureIgnoresNonWitnessErrors(t *testing.T) { + tester := newTester(false) + defer tester.fetcher.Stop() + + var ( + mu sync.Mutex + strikes []string + excluded []string + ) + tester.fetcher.SetWitnessServerStriker(func(id string) { + mu.Lock() + strikes = append(strikes, id) + mu.Unlock() + }) + tester.fetcher.SetWitnessSourceExcluder(func(peer string, _ common.Hash) { + mu.Lock() + excluded = append(excluded, peer) + mu.Unlock() + }) + + block := createTestBlock(503) + witness := createTestWitnessForBlock(block) + fetch := func(common.Hash, chan *eth.Response) (*eth.Request, error) { return nil, errors.New("noop") } + + for _, importErr := range []error{ + errors.New("insertion is interrupted"), + errors.New("blockchain is stopped"), + whitelist.ErrMismatch, + errors.New("unknown parent"), + fmt.Errorf("propagated block verification failed: %w", errors.New("invalid timestamp")), + nil, + } { + op := &blockOrHeaderInject{origin: "o", block: block, witness: witness, witnessPeer: "srv", witnessDiverged: true, fetchWitness: fetch} + if tester.fetcher.chargeDivergedWitnessImportFailure(op, importErr) { + t.Fatalf("error %v is not witness-attributable and must not trigger a re-fetch", importErr) + } + if op.witnessImportFailures != 0 { + t.Fatalf("error %v must not consume the retry budget", importErr) + } + } + mu.Lock() + defer mu.Unlock() + if len(strikes) != 0 || len(excluded) != 0 { + t.Fatalf("non-witness import errors must not strike or exclude the server; strikes=%v excluded=%v", strikes, excluded) + } +} + +// TestIsWitnessAttributableImportError pins the allowlist: incomplete-witness +// and execution-mismatch failures are attributable, everything else — notably +// errors that wrap nothing the witness carries — is not. +func TestIsWitnessAttributableImportError(t *testing.T) { + attributable := []error{ + &trie.MissingNodeError{NodeHash: common.HexToHash("0x01"), Path: []byte{0x1}}, + fmt.Errorf("stateless execution hit incomplete state or code: %w", &trie.MissingNodeError{NodeHash: common.HexToHash("0x02")}), + core.ErrStatelessStateRootMismatch, + fmt.Errorf("%w (remote: 1 local: 2)", core.ErrGasUsedMismatch), + fmt.Errorf("%w (remote: 0a local: 0b)", core.ErrReceiptRootMismatch), + fmt.Errorf("%w (remote: 0a local: 0b)", core.ErrBloomMismatch), + fmt.Errorf("%w (remote: 0a local: 0b)", core.ErrRequestsHashMismatch), + errors.New("invalid merkle root (remote: 0a local: 0b) dberr: "), + errors.New("stateless self-validation root mismatch (cross: 0a local: 0b)"), + errors.New("stateless self-validation receipt root mismatch: remote 0a != local 0b"), + } + for _, err := range attributable { + if !isWitnessAttributableImportError(err) { + t.Fatalf("%v must be attributable to the witness server", err) + } + } + notAttributable := []error{ + nil, + errors.New("insertion is interrupted"), + errors.New("blockchain is stopped"), + whitelist.ErrMismatch, + errors.New("unknown parent"), + errors.New("stateless execution hit incomplete state or code: code is not found: addr 0x01 hash 0x02"), + errors.New("leveldb: closed"), + } + for _, err := range notAttributable { + if isWitnessAttributableImportError(err) { + t.Fatalf("%v must not be attributable to the witness server", err) + } + } +} + // TestRetryAfterImportFailureReRegistersPending pins the witness manager side of // the re-fetch: the block goes back into pending with the witness cleared, the // fetch closure and failure count carried over, and a second registration for @@ -243,7 +336,7 @@ func TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer(t *testing.T) // First import fails (an unusable witness), the second succeeds. tester.fetcher.insertChain = func(blocks types.Blocks, witnesses []*stateless.Witness) (int, error) { if imports.Add(1) == 1 { - return 0, errors.New("stateless self-validation failed: missing trie node") + return 0, fmt.Errorf("%w (cross: 01 local: 02)", core.ErrStatelessStateRootMismatch) } return tester.insertChain(blocks, witnesses) } diff --git a/eth/fetcher/witness_manager_wit2.go b/eth/fetcher/witness_manager_wit2.go index c353faa951..e42100e585 100644 --- a/eth/fetcher/witness_manager_wit2.go +++ b/eth/fetcher/witness_manager_wit2.go @@ -172,15 +172,11 @@ const bytesPerMiB = 1024 * 1024 // announcements whose WitnessSize is 0 or above the absolute cap, so under // normal operation neither branch is reached; they are defence in depth. func (m *witnessManager) acceptableWitnessSizeCeiling(signedSize uint64) uint64 { - absBytes := m.MaxWitnessSize() + absBytes := m.MaxWitnessSize() // never zero: calculatePageThreshold floors at one page if signedSize == 0 { return absBytes } - band := saturatingMulUint64(signedSize, wit2SizeBandMultiplier) - if absBytes == 0 { - return band - } - return min(band, absBytes) + return min(saturatingMulUint64(signedSize, wit2SizeBandMultiplier), absBytes) } // AcceptableWitnessSizeCeiling is the exported form of diff --git a/eth/handler_wit2_caches_test.go b/eth/handler_wit2_caches_test.go index 55d8ac0917..c65b572ed8 100644 --- a/eth/handler_wit2_caches_test.go +++ b/eth/handler_wit2_caches_test.go @@ -3,6 +3,7 @@ package eth import ( "crypto/rand" "fmt" + "math" "math/big" "testing" "time" @@ -1220,3 +1221,39 @@ func TestWitnessSourceExclusionSetLifecycle(t *testing.T) { s.mu.Unlock() require.False(t, still, "stale entries must be swept on add") } + +// TestSizeOracleHelpersWithoutFetcher covers the handler-side size-oracle +// helpers on a handler with no block fetcher and no exclusion set (only +// reachable in tests): no bound is applied, a non-zero size is plausible, and +// excluding a source is a no-op rather than a panic. +func TestSizeOracleHelpersWithoutFetcher(t *testing.T) { + h := &handler{} + require.Equal(t, uint64(math.MaxUint64), h.witnessSizeCeiling(123), "no fetcher: no size bound") + require.False(t, h.plausibleSignedWitnessSize(0), "zero is never a usable size oracle") + require.True(t, h.plausibleSignedWitnessSize(1), "no fetcher: any non-zero size is plausible") + require.NotPanics(t, func() { h.excludeWitnessSource("p", common.HexToHash("0x01")) }) +} + +// TestDeferredAnnounceCacheHasWitnessSizeWithin pins the size-band binding used +// by the deferred broadcast path: the boundary is inclusive, expired candidates +// are ignored, and an unknown block matches nothing. +func TestDeferredAnnounceCacheHasWitnessSizeWithin(t *testing.T) { + c := newDeferredAnnounceCache(8) + hash := common.HexToHash("0xe1") + c.put(wit.SignedWitnessAnnouncement{ + BlockHash: hash, BlockNumber: 1, WitnessHash: common.HexToHash("0x01"), WitnessSize: 100, + Signature: make([]byte, wit.SignatureLength), + }, "p1") + ceiling := func(signedSize uint64) uint64 { return 3 * signedSize } + + require.True(t, c.hasWitnessSizeWithin(hash, 300, ceiling), "a body exactly at the ceiling is within band") + require.False(t, c.hasWitnessSizeWithin(hash, 301, ceiling), "one byte over the ceiling is out of band") + require.False(t, c.hasWitnessSizeWithin(common.HexToHash("0xe2"), 1, ceiling), "unknown block matches nothing") + + c.mu.Lock() + for _, e := range c.entries[hash] { + e.receivedAt = time.Now().Add(-2 * wit2AnnounceTTL) + } + c.mu.Unlock() + require.False(t, c.hasWitnessSizeWithin(hash, 300, ceiling), "an expired candidate must not bind a body") +} diff --git a/eth/handler_wit2_test.go b/eth/handler_wit2_test.go index e78ecc95fb..d79dd3db70 100644 --- a/eth/handler_wit2_test.go +++ b/eth/handler_wit2_test.go @@ -1353,6 +1353,7 @@ func TestHandleWitnessBroadcastAcceptedWhileAnnounceDeferred(t *testing.T) { BlockHash: hash, BlockNumber: header.Number.Uint64(), WitnessHash: stateless.WitnessCommitHash(buf.Bytes()), + WitnessSize: 1, // size band alone would reject the body; only the exact hash match admits it Signature: make([]byte, wit.SignatureLength), } h.handler.deferredAnnounces.put(ann, "upstream-peer") From 95e3f58d94fc8b5f5fd387809161aa086e093bc4 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 21 Sep 2026 15:38:26 -0400 Subject: [PATCH 06/10] eth/fetcher: never charge a witness server for a contract bytecode missing from local disk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2401 surfaces a missing contract bytecode on the stateless path as core.ErrStatelessIncompleteState wrapping a *state.MissingCodeError, the same sentinel that wraps a missing trie node. Witnesses carry no code, so the server could not have supplied it and the downloader's self-heal fetches the blob; charging the server would strike and exclude honest witness sources on every block that touches the contract until the node had none left. Check for *state.MissingCodeError first in isWitnessAttributableImportError and return false explicitly, so the wrapped cause — not the shared sentinel — decides attribution: missing node → incomplete witness → charged; missing code → local gap → not charged; the bare sentinel → cause unknown → not charged. Tests: the wrapped MissingCodeError case in TestChargeDivergedWitnessImportFailureIgnoresNonWitnessErrors (no strike, no exclusion, no re-fetch, no budget consumed) and both wrappings of ErrStatelessIncompleteState in TestIsWitnessAttributableImportError. --- eth/fetcher/witness_import_errors.go | 11 +++++++++++ eth/fetcher/witness_import_failure_test.go | 13 +++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/eth/fetcher/witness_import_errors.go b/eth/fetcher/witness_import_errors.go index 0bcb2c6ab9..9225f89d9a 100644 --- a/eth/fetcher/witness_import_errors.go +++ b/eth/fetcher/witness_import_errors.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/trie" ) @@ -42,6 +43,16 @@ func isWitnessAttributableImportError(err error) bool { if err == nil { return false } + // A contract bytecode missing from the local disk. Witnesses carry no code, + // so the server could not have supplied it; the blob is content-addressed + // and the downloader's self-heal fetches it. Checked first because it + // arrives wrapped in core.ErrStatelessIncompleteState, the same sentinel + // that wraps a missing trie node — the wrapped cause, not the sentinel, + // decides attribution. + var missingCode *state.MissingCodeError + if errors.As(err, &missingCode) { + return false + } // The witness did not carry a trie node the block reads: an incomplete // witness is something the server did produce. var missingNode *trie.MissingNodeError diff --git a/eth/fetcher/witness_import_failure_test.go b/eth/fetcher/witness_import_failure_test.go index d5babcfaf5..af9e6162b2 100644 --- a/eth/fetcher/witness_import_failure_test.go +++ b/eth/fetcher/witness_import_failure_test.go @@ -12,6 +12,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/stateless" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth/downloader/whitelist" @@ -170,7 +171,14 @@ func TestChargeDivergedWitnessImportFailureIgnoresNonWitnessErrors(t *testing.T) witness := createTestWitnessForBlock(block) fetch := func(common.Hash, chan *eth.Response) (*eth.Request, error) { return nil, errors.New("noop") } + // The first entry is the #2401 case: a contract bytecode missing from local + // disk surfaces from ExecuteStateless as ErrStatelessIncompleteState wrapping + // a *state.MissingCodeError. The witness never carried code, so the server + // must not be struck, excluded, or re-fetched from for it. + missingCode := fmt.Errorf("%w: %w", core.ErrStatelessIncompleteState, + &state.MissingCodeError{Addr: common.HexToAddress("0xc0de"), Hash: common.HexToHash("0xf98d")}) for _, importErr := range []error{ + missingCode, errors.New("insertion is interrupted"), errors.New("blockchain is stopped"), whitelist.ErrMismatch, @@ -199,7 +207,7 @@ func TestChargeDivergedWitnessImportFailureIgnoresNonWitnessErrors(t *testing.T) func TestIsWitnessAttributableImportError(t *testing.T) { attributable := []error{ &trie.MissingNodeError{NodeHash: common.HexToHash("0x01"), Path: []byte{0x1}}, - fmt.Errorf("stateless execution hit incomplete state or code: %w", &trie.MissingNodeError{NodeHash: common.HexToHash("0x02")}), + fmt.Errorf("%w: %w", core.ErrStatelessIncompleteState, &trie.MissingNodeError{NodeHash: common.HexToHash("0x02")}), // incomplete witness core.ErrStatelessStateRootMismatch, fmt.Errorf("%w (remote: 1 local: 2)", core.ErrGasUsedMismatch), fmt.Errorf("%w (remote: 0a local: 0b)", core.ErrReceiptRootMismatch), @@ -220,7 +228,8 @@ func TestIsWitnessAttributableImportError(t *testing.T) { errors.New("blockchain is stopped"), whitelist.ErrMismatch, errors.New("unknown parent"), - errors.New("stateless execution hit incomplete state or code: code is not found: addr 0x01 hash 0x02"), + fmt.Errorf("%w: %w", core.ErrStatelessIncompleteState, &state.MissingCodeError{Addr: common.HexToAddress("0x01"), Hash: common.HexToHash("0x02")}), // #2401: missing code, not the witness + core.ErrStatelessIncompleteState, // sentinel alone: cause unknown, not charged errors.New("leveldb: closed"), } for _, err := range notAttributable { From 594028cbcded9d952d821d3fe24f253041a6995c Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 21 Sep 2026 15:44:41 -0400 Subject: [PATCH 07/10] eth/fetcher: cover the exported size ceiling and the re-fetch guard branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exercise AcceptableWitnessSizeCeiling from its own package (the handler is its only production caller) and the two retryAfterImportFailure early exits — block already known locally, witness marked unavailable — so the new code is fully covered where it lives. --- eth/fetcher/witness_import_failure_test.go | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/eth/fetcher/witness_import_failure_test.go b/eth/fetcher/witness_import_failure_test.go index af9e6162b2..dd37e705c0 100644 --- a/eth/fetcher/witness_import_failure_test.go +++ b/eth/fetcher/witness_import_failure_test.go @@ -36,6 +36,11 @@ func TestAcceptableWitnessSizeCeilingDegenerateSignedSizes(t *testing.T) { if got := tw.manager.acceptableWitnessSizeCeiling(0); got != abs { t.Fatalf("signedSize=0 must fall back to the absolute cap %d, got %d", abs, got) } + // The exported form used by the handler's broadcast path must agree with the + // fetch path's ceiling, so both delivery paths judge a witness identically. + if got, want := tw.manager.AcceptableWitnessSizeCeiling(1000), tw.manager.acceptableWitnessSizeCeiling(1000); got != want { + t.Fatalf("AcceptableWitnessSizeCeiling = %d, want %d", got, want) + } for _, hostile := range []uint64{math.MaxUint64, math.MaxUint64 / 2, math.MaxUint64/wit2SizeBandMultiplier + 1} { if got := tw.manager.acceptableWitnessSizeCeiling(hostile); got != abs { t.Fatalf("signedSize=%d must saturate and clamp to the absolute cap %d, got %d", hostile, abs, got) @@ -286,6 +291,27 @@ func TestRetryAfterImportFailureReRegistersPending(t *testing.T) { if tw.PendingCount() != 1 { t.Fatalf("duplicate re-registration must be a no-op; pending count = %d", tw.PendingCount()) } + + // A block that meanwhile became known locally is not re-fetched. + known := createTestBlock(504) + tw.manager.parentGetBlock = func(h common.Hash) *types.Block { + if h == known.Hash() { + return known + } + return nil + } + tw.manager.retryAfterImportFailure(&blockOrHeaderInject{origin: "o", block: known, witness: createTestWitnessForBlock(known), witnessPeer: "srv-2", witnessDiverged: true, fetchWitness: fetch}) + if tw.PendingCount() != 1 { + t.Fatalf("a locally known block must not be re-registered; pending count = %d", tw.PendingCount()) + } + + // A block whose witness was marked unavailable is not re-fetched either. + unavailable := createTestBlock(505) + tw.manager.markWitnessUnavailable(unavailable.Hash()) + tw.manager.retryAfterImportFailure(&blockOrHeaderInject{origin: "o", block: unavailable, witness: createTestWitnessForBlock(unavailable), witnessPeer: "srv-3", witnessDiverged: true, fetchWitness: fetch}) + if tw.PendingCount() != 1 { + t.Fatalf("an unavailable-marked block must not be re-registered; pending count = %d", tw.PendingCount()) + } } // TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer drives the whole From 62954f9a81b324f40b1fbb1379defd8b78932509 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 21 Sep 2026 16:52:36 -0400 Subject: [PATCH 08/10] eth/fetcher, eth: split importBlocks under the complexity gate; pin the sampled mutation survivors Diffguard flagged importBlocks at complexity 21 (threshold 15) after the witness-retry plumbing. Move the goroutine body into runBlockImport, which returns whether the failed import should be retried with a witness from another peer, and the block-tracker log into logTrackedImport; importBlocks now only routes the result to witnessRetry or done. Behaviour is unchanged. Pin the five sampled survivors: assert the diverged flag on both the exact-match and divergent returns of verifyAgainstSignedHash and on the WIT1-only (nil lookup) return; add the exact-boundary case to the saturating multiply (MaxUint64/2 * 2 must multiply, not saturate); and pin that the fetcher's striker and source-excluder callbacks are wired into the handler (StrikeWitnessServer / ExcludeWitnessSource are exported for that test). --- eth/fetcher/block_fetcher.go | 155 +++++++++++---------- eth/fetcher/witness_import_failure_test.go | 2 + eth/fetcher/witness_manager.go | 14 +- eth/fetcher/witness_manager_wit2_test.go | 26 +++- eth/handler_wit2_caches_test.go | 27 ++++ 5 files changed, 146 insertions(+), 78 deletions(-) diff --git a/eth/fetcher/block_fetcher.go b/eth/fetcher/block_fetcher.go index 579170baab..f6d601f5de 100644 --- a/eth/fetcher/block_fetcher.go +++ b/eth/fetcher/block_fetcher.go @@ -1241,91 +1241,106 @@ const maxWitnessImportRetries = 2 // forgotten. A BP-identical witness (hash match) that fails import is the BP's // fault and is handled as before: logged and forgotten. func (f *BlockFetcher) importBlocks(op *blockOrHeaderInject) { - peer, block, witness := op.origin, op.block, op.witness + block := op.block hash := block.Hash() // Run the import on a new thread - log.Debug("Importing propagated block", "peer", peer, "number", block.Number(), "hash", hash) + log.Debug("Importing propagated block", "peer", op.origin, "number", block.Number(), "hash", hash) go func() { - retryWitness := false - defer func() { - if retryWitness { - select { - case f.witnessRetry <- op: - case <-f.quit: - } - return + if f.runBlockImport(op) { + // Hand the block back to the fetcher loop for a witness re-fetch + // (the witnessRetry case) instead of marking it done. + select { + case f.witnessRetry <- op: + case <-f.quit: } - f.done <- hash - }() - - // If the parent's unknown, abort insertion - parent := f.getBlock(block.ParentHash()) - if parent == nil { - log.Debug("Unknown parent of propagated block", "peer", peer, "number", block.Number(), "hash", hash, "parent", block.ParentHash()) return } - // Quickly validate the header and propagate the block if it passes - switch err := f.verifyHeader(block.Header()); err { - case nil: - // All ok, quickly propagate to our peers - blockBroadcastOutTimer.UpdateSince(block.ReceivedAt) + f.done <- hash + }() +} - go f.broadcastBlock(block, witness, true) +// runBlockImport performs one propagated-block import: parent check, header +// verification with early propagation, chain insertion, and the post-import +// broadcast and hooks. It reports whether the failed import should be retried +// with a witness fetched from another peer (chargeDivergedWitnessImportFailure); +// the caller then routes the op to the fetcher loop's witnessRetry case rather +// than done. +func (f *BlockFetcher) runBlockImport(op *blockOrHeaderInject) (retryWitness bool) { + peer, block, witness := op.origin, op.block, op.witness + hash := block.Hash() - case consensus.ErrFutureBlock: - // Weird future block, don't fail, but neither propagate + // If the parent's unknown, abort insertion + if f.getBlock(block.ParentHash()) == nil { + log.Debug("Unknown parent of propagated block", "peer", peer, "number", block.Number(), "hash", hash, "parent", block.ParentHash()) + return false + } + // Quickly validate the header and propagate the block if it passes + switch err := f.verifyHeader(block.Header()); err { + case nil: + // All ok, quickly propagate to our peers + blockBroadcastOutTimer.UpdateSince(block.ReceivedAt) - default: - // Something went very wrong, drop the peer - log.Debug("Propagated block verification failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err) - f.dropPeer(peer) + go f.broadcastBlock(block, witness, true) - return - } - // Run the actual import and log any issues - // Pass the witness along with the block to the insertion function. - // Create slices even for a single block/witness to match the expected signature. - if _, err := f.insertChain(types.Blocks{block}, []*stateless.Witness{witness}); err != nil { - log.Debug("Propagated block import failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err) - retryWitness = f.chargeDivergedWitnessImportFailure(op, err) - return - } + case consensus.ErrFutureBlock: + // Weird future block, don't fail, but neither propagate - if f.enableBlockTracking { - // Log the insertion event - var ( - msg string - delayInMs int64 - prettyDelay common.PrettyDuration - ) - - if block.AnnouncedAt != nil { - msg = "[block tracker] Inserted new block with announcement" - delayInMs = time.Since(*block.AnnouncedAt).Milliseconds() - prettyDelay = common.PrettyDuration(time.Since(*block.AnnouncedAt)) - } else { - msg = "[block tracker] Inserted new block without announcement" - delayInMs = time.Since(block.ReceivedAt).Milliseconds() - prettyDelay = common.PrettyDuration(time.Since(block.ReceivedAt)) - } + default: + // Something went very wrong, drop the peer + log.Debug("Propagated block verification failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err) + f.dropPeer(peer) - totalDelayInMs := time.Now().UnixMilli() - int64(block.Time())*1000 - totalDelay := common.PrettyDuration(time.Millisecond * time.Duration(totalDelayInMs)) + return false + } + // Run the actual import and log any issues + // Pass the witness along with the block to the insertion function. + // Create slices even for a single block/witness to match the expected signature. + if _, err := f.insertChain(types.Blocks{block}, []*stateless.Witness{witness}); err != nil { + log.Debug("Propagated block import failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err) + return f.chargeDivergedWitnessImportFailure(op, err) + } - log.Info(msg, "number", block.Number().Uint64(), "hash", hash, "delay", prettyDelay, "delayInMs", delayInMs, "totalDelay", totalDelay, "totalDelayInMs", totalDelayInMs) - } + if f.enableBlockTracking { + f.logTrackedImport(block, hash) + } - // If import succeeded, broadcast the block - blockAnnounceOutTimer.UpdateSince(block.ReceivedAt) - go f.broadcastBlock(block, witness, false) + // If import succeeded, broadcast the block + blockAnnounceOutTimer.UpdateSince(block.ReceivedAt) + go f.broadcastBlock(block, witness, false) - // Invoke the testing hook if needed - if f.importedHook != nil { - f.importedHook(nil, block) - } - }() + // Invoke the testing hook if needed + if f.importedHook != nil { + f.importedHook(nil, block) + } + return false +} + +// logTrackedImport emits the block-tracker insertion line with the +// announce-to-insert (or receive-to-insert) delay and the header-time-to-insert +// total delay. +func (f *BlockFetcher) logTrackedImport(block *types.Block, hash common.Hash) { + var ( + msg string + delayInMs int64 + prettyDelay common.PrettyDuration + ) + + if block.AnnouncedAt != nil { + msg = "[block tracker] Inserted new block with announcement" + delayInMs = time.Since(*block.AnnouncedAt).Milliseconds() + prettyDelay = common.PrettyDuration(time.Since(*block.AnnouncedAt)) + } else { + msg = "[block tracker] Inserted new block without announcement" + delayInMs = time.Since(block.ReceivedAt).Milliseconds() + prettyDelay = common.PrettyDuration(time.Since(block.ReceivedAt)) + } + + totalDelayInMs := time.Now().UnixMilli() - int64(block.Time())*1000 + totalDelay := common.PrettyDuration(time.Millisecond * time.Duration(totalDelayInMs)) + + log.Info(msg, "number", block.Number().Uint64(), "hash", hash, "delay", prettyDelay, "delayInMs", delayInMs, "totalDelay", totalDelay, "totalDelayInMs", totalDelayInMs) } // chargeDivergedWitnessImportFailure applies the WIT2 consequence of an import @@ -1355,8 +1370,8 @@ func (f *BlockFetcher) chargeDivergedWitnessImportFailure(op *blockOrHeaderInjec witnessImportFailureMeter.Mark(1) log.Warn("Import failed with a witness accepted on the WIT2 size oracle; striking its server", "server", op.witnessPeer, "number", op.number(), "hash", hash, "attempt", op.witnessImportFailures+1, "err", importErr) - f.wm.strikeWitnessServer(op.witnessPeer) - f.wm.excludeWitnessSource(op.witnessPeer, hash) + f.wm.StrikeWitnessServer(op.witnessPeer) + f.wm.ExcludeWitnessSource(op.witnessPeer, hash) op.witnessImportFailures++ if op.fetchWitness == nil || op.witnessImportFailures >= maxWitnessImportRetries { diff --git a/eth/fetcher/witness_import_failure_test.go b/eth/fetcher/witness_import_failure_test.go index dd37e705c0..8fe37ffa1f 100644 --- a/eth/fetcher/witness_import_failure_test.go +++ b/eth/fetcher/witness_import_failure_test.go @@ -56,6 +56,8 @@ func TestSaturatingMulUint64(t *testing.T) { {7, 3, 21}, {math.MaxUint64 / 3, 3, math.MaxUint64 / 3 * 3}, {math.MaxUint64/3 + 1, 3, math.MaxUint64}, + {math.MaxUint64 / 2, 2, math.MaxUint64 - 1}, // exactly at the bound: must multiply, not saturate + {math.MaxUint64/2 + 1, 2, math.MaxUint64}, {math.MaxUint64, 2, math.MaxUint64}, } for _, c := range cases { diff --git a/eth/fetcher/witness_manager.go b/eth/fetcher/witness_manager.go index f6bfb8184b..66e20a8940 100644 --- a/eth/fetcher/witness_manager.go +++ b/eth/fetcher/witness_manager.go @@ -878,17 +878,19 @@ func (m *witnessManager) safeEnqueue(op *blockOrHeaderInject) { m.rescheduleWitness() } -// strikeWitnessServer records a WIT2 strike against peer via the parent -// callback, if one is wired. -func (m *witnessManager) strikeWitnessServer(peer string) { +// StrikeWitnessServer records a WIT2 strike against peer via the parent +// callback, if one is wired. Exported so the network handler can pin that its +// striker is wired to this manager. +func (m *witnessManager) StrikeWitnessServer(peer string) { if peer != "" && m.parentStrikeWitnessServer != nil { m.parentStrikeWitnessServer(peer) } } -// excludeWitnessSource asks the parent to stop offering peer as a witness -// source for hash, if a callback is wired. -func (m *witnessManager) excludeWitnessSource(peer string, hash common.Hash) { +// ExcludeWitnessSource asks the parent to stop offering peer as a witness +// source for hash, if a callback is wired. Exported so the network handler can +// pin that its excluder is wired to this manager. +func (m *witnessManager) ExcludeWitnessSource(peer string, hash common.Hash) { if peer != "" && m.parentExcludeWitnessSource != nil { m.parentExcludeWitnessSource(peer, hash) } diff --git a/eth/fetcher/witness_manager_wit2_test.go b/eth/fetcher/witness_manager_wit2_test.go index 9b5572ce89..f9b0b2c130 100644 --- a/eth/fetcher/witness_manager_wit2_test.go +++ b/eth/fetcher/witness_manager_wit2_test.go @@ -676,10 +676,13 @@ func TestVerifyAgainstSignedHashAcceptsDivergentHashWithinBand(t *testing.T) { struck := 0 tw.manager.parentStrikeWitnessServer = func(string) { struck++ } - body, _, _, ok := tw.manager.verifyAgainstSignedHash("honest-diverging", hash, witness) + body, _, diverged, ok := tw.manager.verifyAgainstSignedHash("honest-diverging", hash, witness) if !ok { t.Fatal("a witness within the signed size band must be accepted for import despite a differing hash") } + if !diverged { + t.Fatal("a within-band witness with a differing hash must be flagged diverged so an import failure is charged to the server") + } if body != nil { t.Fatal("a non-identical within-band witness must import but NOT be cached for serving (body=nil), so the fast-path carries only the BP's bytes") } @@ -711,10 +714,13 @@ func TestVerifyAgainstSignedHashServesOnExactMatch(t *testing.T) { return common.Hash{}, 0, false } - body, gotHash, _, ok := tw.manager.verifyAgainstSignedHash("honest-matching", hash, witness) + body, gotHash, diverged, ok := tw.manager.verifyAgainstSignedHash("honest-matching", hash, witness) if !ok { t.Fatal("a byte-identical within-band witness must be accepted") } + if diverged { + t.Fatal("a byte-identical witness must not be flagged diverged: an import failure of the BP's own bytes is the BP's fault, not the server's") + } if body == nil { t.Fatal("a byte-identical witness must return canonical bytes for the serving cache") } @@ -761,3 +767,19 @@ func TestWitnessSizeExceedsCeiling(t *testing.T) { t.Fatal("a witness below the ceiling must be accepted") } } + +// TestVerifyAgainstSignedHashWithoutLookupIsWit1 pins the WIT1-only wiring: with +// no signed-hash lookup configured at all, every witness is accepted for import +// (ok), nothing is offered for pre-import serving (body nil), and nothing is +// flagged diverged (there is no commitment to diverge from). +func TestVerifyAgainstSignedHashWithoutLookupIsWit1(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + tw.manager.parentSignedWitnessHash = nil + + block := createTestBlock(310) + body, gotHash, diverged, ok := tw.manager.verifyAgainstSignedHash("peer", block.Hash(), createTestWitnessForBlock(block)) + if !ok || body != nil || gotHash != (common.Hash{}) || diverged { + t.Fatalf("WIT1-only path must return (nil, zero, false, true); got body=%v hash=%s diverged=%v ok=%v", body != nil, gotHash.Hex(), diverged, ok) + } +} diff --git a/eth/handler_wit2_caches_test.go b/eth/handler_wit2_caches_test.go index c65b572ed8..939e014cf1 100644 --- a/eth/handler_wit2_caches_test.go +++ b/eth/handler_wit2_caches_test.go @@ -1257,3 +1257,30 @@ func TestDeferredAnnounceCacheHasWitnessSizeWithin(t *testing.T) { c.mu.Unlock() require.False(t, c.hasWitnessSizeWithin(hash, 300, ceiling), "an expired candidate must not bind a body") } + +// TestFetcherWitnessPenaltiesAreWiredToHandler pins the newHandler wiring of the +// block fetcher's two WIT2 penalty callbacks: a strike issued by the witness +// manager lands in the handler's wit2 strike tracker, and a source exclusion +// lands in the handler's exclusion set consulted by resolveWitnessFetchPeer. +// Without the wiring both are silent no-ops and the import-failure consequence +// never reaches the peer. +func TestFetcherWitnessPenaltiesAreWiredToHandler(t *testing.T) { + h := newTestHandler() + defer h.close() + wm := h.handler.blockFetcher.GetWitnessManager() + + wm.StrikeWitnessServer("fetcher-struck-peer") + h.handler.wit2PeerTracker.mu.Lock() + st, tracked := h.handler.wit2PeerTracker.state["fetcher-struck-peer"] + strikes := 0 + if tracked { + strikes = len(st.strikes) + } + h.handler.wit2PeerTracker.mu.Unlock() + require.Equal(t, 1, strikes, "a fetcher strike must reach the handler's wit2 strike tracker") + + hash := common.HexToHash("0xf00d") + wm.ExcludeWitnessSource("fetcher-excluded-peer", hash) + require.True(t, h.handler.witnessSourceExclusions.excluded(hash, "fetcher-excluded-peer"), + "a fetcher source exclusion must reach the handler's exclusion set") +} From 6b6f9405e77469d98e4ed15e4d17426b49a5ca30 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 21 Sep 2026 19:25:44 -0400 Subject: [PATCH 09/10] eth, eth/fetcher: charge pushed witnesses on import failure; stricter quarantine clear and announce conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups (pratikspatil024, 2026-09-21). The broadcast path accepted a within-band divergent body for import but could not charge it: handleBroadcast set only op.witness, and both cached-witness attach sites rebuilt the op without provenance, so chargeDivergedWitnessImportFailure returned on its first guard and a NewWitness push was the free way to deliver unusable bytes — and the easier one, since the first witness to arrive is the one attached. acceptSignedBroadcast/acceptDeferredBroadcast now report the divergence, InjectWitness carries it with the pusher, handleBroadcast records witnessPeer/witnessDiverged and the announce's fetch closure, and the before-the-block cache keeps peer+diverged so both attach sites build the op via cachedWitness.injectFor with full provenance. An import failure of pushed bytes now strikes and excludes the pusher and re-fetches from another peer, as on the fetch path; a pusher excluded for a block has its further pushes for that block dropped (broadcast_excluded_source_drop) so it cannot beat every honest re-fetch with the same body. verifyAgainstSignedHash cleared the oversize/quarantine state on every in-band acceptance; a divergent in-band body proves nothing about the signed size, so a server alternating oversized and in-band bodies could keep a block out of quarantine indefinitely. Only BP-identical bytes clear it now (the pending-removal exits and TTL sweep still bound the maps). signedWitnessCache.putIfNewer keyed conflicts on WitnessHash alone; the signed WitnessSize decides the accept band, so a same-hash announce with another size is now a conflict too (first commitment wins) instead of replacing the band once the relay window lapses. Tests: push-path import-failure e2e for both attach sites through the real fetcher loop, provenance bit from both accept functions, excluded pusher dropped, quarantine kept across a divergent acceptance, size conflict rejected before and after the relay window. --- eth/fetcher/block_fetcher.go | 33 ++-- eth/fetcher/witness_import_failure_test.go | 202 +++++++++++++++++++++ eth/fetcher/witness_manager.go | 42 +++-- eth/fetcher/witness_manager_wit2.go | 11 +- eth/fetcher/witness_manager_wit2_test.go | 55 ++++++ eth/handler_wit.go | 59 ++++-- eth/handler_wit2.go | 41 +++-- eth/handler_wit2_announces.go | 19 +- eth/handler_wit2_push_charge_test.go | 156 ++++++++++++++++ eth/handler_wit2_test.go | 179 ++++++++++++++++++ 10 files changed, 723 insertions(+), 74 deletions(-) create mode 100644 eth/handler_wit2_push_charge_test.go diff --git a/eth/fetcher/block_fetcher.go b/eth/fetcher/block_fetcher.go index f6d601f5de..e35472b5fe 100644 --- a/eth/fetcher/block_fetcher.go +++ b/eth/fetcher/block_fetcher.go @@ -159,11 +159,12 @@ type blockOrHeaderInject struct { block *types.Block // Used for normal mode fetcher which imports full block. witness *stateless.Witness // Used for witness mode fetcher which imports witness. - // WIT2 witness provenance, set by the witness manager when the witness was - // obtained by paged fetch. importBlocks uses it to charge an import failure - // to the serving peer and re-fetch from another source when the witness was - // accepted on the size oracle alone (hash differs from the BP-signed one). - witnessPeer string // Peer that served the witness; empty when not fetched (broadcast, local). + // WIT2 witness provenance, set by the witness manager whether the witness + // was obtained by paged fetch or pushed by broadcast. importBlocks uses it + // to charge an import failure to the peer that chose the bytes and re-fetch + // from another source when the witness was accepted on the size oracle + // alone (hash differs from the BP-signed one). + witnessPeer string // Peer that served or pushed the witness; empty when local. witnessDiverged bool // Witness accepted on size alone: hash differs from the BP-signed hash. witnessImportFailures int // Import attempts of this block that already failed with a diverged witness. fetchWitness witnessRequesterFn // Fetch closure to re-request the witness after an import failure. @@ -198,9 +199,10 @@ type injectBlockNeedWitnessMsg struct { // injectedWitnessMsg is used to inject a witness received externally via broadcast. type injectedWitnessMsg struct { - peer string - witness *stateless.Witness - time time.Time // Arrival time + peer string + witness *stateless.Witness + diverged bool // Accepted on the WIT2 size oracle alone: hash differs from the BP-signed one. + time time.Time // Arrival time } // enqueueRequest is used to shuttle fully assembled blocks (with witness) @@ -420,11 +422,18 @@ func (f *BlockFetcher) InjectBlockWithWitnessRequirement(origin string, block *t } // InjectWitness injects a witness received via broadcast into the fetcher. -func (f *BlockFetcher) InjectWitness(peer string, witness *stateless.Witness) error { +// diverged reports that the pushed bytes were accepted on the WIT2 size oracle +// alone (their hash differs from the BP-signed one). The witness manager records +// it, with peer, as the witness's provenance, so an import failure is charged to +// the pusher exactly as it is to a serving peer on the paged-fetch path +// (chargeDivergedWitnessImportFailure) — a push must not be the free way to +// deliver unusable within-band bytes. +func (f *BlockFetcher) InjectWitness(peer string, witness *stateless.Witness, diverged bool) error { msg := &injectedWitnessMsg{ - peer: peer, - witness: witness, - time: time.Now(), + peer: peer, + witness: witness, + diverged: diverged, + time: time.Now(), } log.Debug("Injecting witness from broadcast", "peer", peer, "hash", witness.Header().Hash()) // Send to witness manager's channel diff --git a/eth/fetcher/witness_import_failure_test.go b/eth/fetcher/witness_import_failure_test.go index 8fe37ffa1f..31e994f869 100644 --- a/eth/fetcher/witness_import_failure_test.go +++ b/eth/fetcher/witness_import_failure_test.go @@ -425,3 +425,205 @@ func TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer(t *testing.T) t.Fatalf("exactly the first server must be excluded for the block, got %v", excluded) } } + +// waitForCachedWitness blocks until the witness manager has cached a broadcast +// witness for hash (it arrives on the manager loop asynchronously). +func waitForCachedWitness(t *testing.T, f *BlockFetcher, hash common.Hash) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if f.wm.witnessCache.Get(hash) != nil { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("broadcast witness never reached the witness cache") +} + +// waitForPendingWitness blocks until the witness manager has registered hash as +// pending a witness fetch (block injection is processed on the manager loop). +func waitForPendingWitness(t *testing.T, f *BlockFetcher, hash common.Hash) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if f.wm.isPending(hash) { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("block never became pending a witness") +} + +// divergedBroadcastImportFailureHarness wires the striker/excluder recorders, +// the fail-once insertChain and the imported hook shared by the two push-path +// import-failure tests below. +type divergedBroadcastImportFailureHarness struct { + mu sync.Mutex + strikes []string + excluded []string + fetches atomic.Int32 + imports atomic.Int32 + imported chan *types.Block +} + +func newDivergedBroadcastImportFailureHarness(t *testing.T, tester *fetcherTester, hash common.Hash) *divergedBroadcastImportFailureHarness { + t.Helper() + hs := &divergedBroadcastImportFailureHarness{imported: make(chan *types.Block, 1)} + tester.fetcher.SetWitnessServerStriker(func(id string) { + hs.mu.Lock() + hs.strikes = append(hs.strikes, id) + hs.mu.Unlock() + }) + tester.fetcher.SetWitnessSourceExcluder(func(peer string, h common.Hash) { + if h != hash { + t.Errorf("exclusion for unexpected block %s", h) + } + hs.mu.Lock() + hs.excluded = append(hs.excluded, peer) + hs.mu.Unlock() + }) + // First import fails (an unusable witness), the second succeeds. + tester.fetcher.insertChain = func(blocks types.Blocks, witnesses []*stateless.Witness) (int, error) { + if hs.imports.Add(1) == 1 { + return 0, fmt.Errorf("%w (cross: 01 local: 02)", core.ErrStatelessStateRootMismatch) + } + return tester.insertChain(blocks, witnesses) + } + tester.fetcher.importedHook = func(_ *types.Header, b *types.Block) { hs.imported <- b } + return hs +} + +// fetchWitnessFor answers each fetch from a distinct "server" with a valid +// witness, after release is closed (nil release = answer immediately). +func (hs *divergedBroadcastImportFailureHarness) fetchWitnessFor(block *types.Block, release <-chan struct{}) witnessRequesterFn { + return func(h common.Hash, sink chan *eth.Response) (*eth.Request, error) { + n := hs.fetches.Add(1) + req := ð.Request{Peer: fmt.Sprintf("server-%d", n), Cancel: make(chan struct{})} + go func() { + if release != nil { + <-release + } + w, err := stateless.NewWitness(block.Header(), nil) + if err != nil { + return + } + sink <- ð.Response{Req: req, Res: []*stateless.Witness{w}, Time: time.Millisecond, Done: make(chan error, 1)} + }() + return req, nil + } +} + +func (hs *divergedBroadcastImportFailureHarness) assertPusherCharged(t *testing.T, hash common.Hash) { + t.Helper() + select { + case got := <-hs.imported: + if got.Hash() != hash { + t.Fatalf("imported unexpected block %s", got.Hash()) + } + case <-time.After(10 * time.Second): + hs.mu.Lock() + defer hs.mu.Unlock() + t.Fatalf("block never imported after the witness re-fetch (fetches=%d imports=%d strikes=%v excluded=%v)", + hs.fetches.Load(), hs.imports.Load(), hs.strikes, hs.excluded) + } + if n := hs.imports.Load(); n != 2 { + t.Fatalf("import attempt count = %d, want 2", n) + } + hs.mu.Lock() + defer hs.mu.Unlock() + if len(hs.strikes) != 1 || hs.strikes[0] != "pusher" { + t.Fatalf("exactly the pusher must be struck once, got %v", hs.strikes) + } + if len(hs.excluded) != 1 || hs.excluded[0] != "pusher" { + t.Fatalf("exactly the pusher must be excluded for the block, got %v", hs.excluded) + } +} + +// TestImportFailureWithDivergedBroadcastWitnessChargesPusher is the push-path +// twin of TestImportFailureWithDivergedWitnessRefetchesFromAnotherPeer: a +// within-band witness that arrives by NewWitness broadcast BEFORE its block (so +// it waits in the witness cache) and then fails import must cost the pusher a +// strike and an exclusion, and the witness must be fetched again from another +// peer — the consequence the paged-fetch path already carries. Before this, +// the cached witness was attached without provenance, so +// chargeDivergedWitnessImportFailure returned on its first guard and a push was +// the free way to deliver unusable within-band bytes. +func TestImportFailureWithDivergedBroadcastWitnessChargesPusher(t *testing.T) { + hashes, blocks := makeChain(1, 0, genesis) + block := blocks[hashes[0]] + hash := block.Hash() + + tester := newTester(false) + defer tester.fetcher.Stop() + hs := newDivergedBroadcastImportFailureHarness(t, tester, hash) + + pushed, err := stateless.NewWitness(block.Header(), nil) + if err != nil { + t.Fatal(err) + } + // The body arrives first, accepted on the size oracle alone (diverged). + if err := tester.fetcher.InjectWitness("pusher", pushed, true); err != nil { + t.Fatal(err) + } + waitForCachedWitness(t, tester.fetcher, hash) + + if err := tester.fetcher.InjectBlockWithWitnessRequirement("origin", block, hs.fetchWitnessFor(block, nil)); err != nil { + t.Fatal(err) + } + hs.assertPusherCharged(t, hash) + if n := hs.fetches.Load(); n != 1 { + t.Fatalf("witness fetch count = %d, want 1 (only the re-fetch after the pushed witness failed import)", n) + } +} + +// TestImportFailureWithDivergedBroadcastWitnessOnPendingBlockChargesPusher +// covers the other attach site: the block is already pending a witness fetch +// when the divergent body is pushed. handleBroadcast attaches it (first witness +// to arrive wins), the import fails, and the pusher — not the in-flight fetch's +// server — must be the one struck and excluded, with the witness then fetched +// again. Fetch responses are held back until the strike has landed so the +// pushed body is the one imported first. +func TestImportFailureWithDivergedBroadcastWitnessOnPendingBlockChargesPusher(t *testing.T) { + hashes, blocks := makeChain(1, 0, genesis) + block := blocks[hashes[0]] + hash := block.Hash() + + tester := newTester(false) + defer tester.fetcher.Stop() + hs := newDivergedBroadcastImportFailureHarness(t, tester, hash) + + release := make(chan struct{}) + if err := tester.fetcher.InjectBlockWithWitnessRequirement("origin", block, hs.fetchWitnessFor(block, release)); err != nil { + t.Fatal(err) + } + // The block must be pending before the push arrives, or the push would + // take the before-the-block cache path covered by the previous test. + waitForPendingWitness(t, tester.fetcher, hash) + pushed, err := stateless.NewWitness(block.Header(), nil) + if err != nil { + t.Fatal(err) + } + if err := tester.fetcher.InjectWitness("pusher", pushed, true); err != nil { + t.Fatal(err) + } + // The pushed body imports (and fails) first; once the pusher has been + // struck, let every fetch answer so the re-fetch can complete. + deadline := time.Now().Add(5 * time.Second) + for { + hs.mu.Lock() + struck := len(hs.strikes) > 0 + hs.mu.Unlock() + if struck { + break + } + if time.Now().After(deadline) { + t.Fatalf("pusher never struck (imports=%d fetches=%d)", hs.imports.Load(), hs.fetches.Load()) + } + time.Sleep(5 * time.Millisecond) + } + close(release) + hs.assertPusherCharged(t, hash) + if n := hs.fetches.Load(); n < 1 { + t.Fatalf("witness fetch count = %d, want at least the re-fetch", n) + } +} diff --git a/eth/fetcher/witness_manager.go b/eth/fetcher/witness_manager.go index 66e20a8940..1e6e500e20 100644 --- a/eth/fetcher/witness_manager.go +++ b/eth/fetcher/witness_manager.go @@ -51,9 +51,25 @@ type witnessRequestState struct { type cachedWitness struct { witness *stateless.Witness peer string + diverged bool // Accepted on the WIT2 size oracle alone (see InjectWitness). timestamp time.Time } +// injectFor builds the import op for block from a witness that arrived by +// broadcast before the block did, carrying the pusher and the size-oracle +// divergence bit as the op's provenance (see blockOrHeaderInject) and the fetch +// closure a re-fetch after an import failure needs (retryAfterImportFailure). +func (c *cachedWitness) injectFor(origin string, block *types.Block, fetchWitness witnessRequesterFn) *blockOrHeaderInject { + return &blockOrHeaderInject{ + origin: origin, + block: block, + witness: c.witness, + witnessPeer: c.peer, + witnessDiverged: c.diverged, + fetchWitness: fetchWitness, + } +} + // signedWitnessHashFn returns the BP-signed witness commitment for a block — // the producer's own witness hash and encoded size — if a WIT2 signed // announcement has been received and verified locally. The witness manager uses @@ -361,12 +377,8 @@ func (m *witnessManager) handleNeed(msg *injectBlockNeedWitnessMsg) { // Check if we have a cached witness for this block if item := m.witnessCache.Get(hash); item != nil { cached := item.Value() - // Use the cached witness - op := &blockOrHeaderInject{ - origin: msg.origin, - block: msg.block, - witness: cached.witness, - } + // Use the cached witness, with the pusher's provenance + op := cached.injectFor(msg.origin, msg.block, msg.fetchWitness) m.witnessCache.Delete(hash) m.mu.Unlock() @@ -415,6 +427,15 @@ func (m *witnessManager) handleBroadcast(msg *injectedWitnessMsg) { // Ensure witness isn't already set if state.op.witness == nil { state.op.witness = msg.witness + // Provenance, exactly as handleWitnessFetchSuccess records it for a + // fetched witness: the pusher chose these bytes, so an import failure + // of a size-oracle-accepted (diverged) body is charged to it and the + // witness re-fetched from someone else via the announce's closure. + state.op.witnessPeer = msg.peer + state.op.witnessDiverged = msg.diverged + if state.op.fetchWitness == nil && state.announce != nil { + state.op.fetchWitness = state.announce.fetchWitness + } // Update block timestamps if needed if state.op.block != nil && msg.time.After(state.op.block.ReceivedAt) { state.op.block.ReceivedAt = msg.time @@ -434,6 +455,7 @@ func (m *witnessManager) handleBroadcast(msg *injectedWitnessMsg) { m.witnessCache.Set(hash, &cachedWitness{ witness: msg.witness, peer: msg.peer, + diverged: msg.diverged, timestamp: msg.time, }, ttlcache.DefaultTTL) log.Debug("[wm] No matching pending block for injected witness, caching for later", "hash", hash, "peer", msg.peer) @@ -1053,12 +1075,8 @@ func (m *witnessManager) handleFilterResult(announce *blockAnnounce, block *type // Check if we have a cached witness for this block if item := m.witnessCache.Get(hash); item != nil { cached := item.Value() - // Use the cached witness - op := &blockOrHeaderInject{ - origin: announce.origin, - block: block, - witness: cached.witness, - } + // Use the cached witness, with the pusher's provenance + op := cached.injectFor(announce.origin, block, announce.fetchWitness) m.witnessCache.Delete(hash) log.Debug("[wm] Found cached witness for filter result block, using it", "hash", hash, "cachedPeer", cached.peer) m.safeEnqueue(op) diff --git a/eth/fetcher/witness_manager_wit2.go b/eth/fetcher/witness_manager_wit2.go index e42100e585..2d5500a987 100644 --- a/eth/fetcher/witness_manager_wit2.go +++ b/eth/fetcher/witness_manager_wit2.go @@ -132,9 +132,6 @@ func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, return nil, common.Hash{}, false, false } - // Within band: forget any earlier oversize noise for this block. - m.clearSignedHashMismatch(hash) - if actual != expected { // A valid, non-deterministic variant of the BP's witness. Accept it for // import (state-root execution validates), but return body=nil so it is @@ -145,7 +142,13 @@ func (m *witnessManager) verifyAgainstSignedHash(peer string, hash common.Hash, witnessHashDivergenceMeter.Mark(1) return nil, common.Hash{}, true, true } - // Byte-identical to the BP's witness: safe to serve/relay under the signed hash. + // Byte-identical to the BP's witness: safe to serve/relay under the signed + // hash. Only this proves the signed commitment good, so only this forgets + // earlier oversize noise for the block: a divergent in-band body says nothing + // about the signed size, and clearing on it would let a server alternating + // oversized and in-band bodies reset the distinct-server count and keep the + // block out of quarantine indefinitely. + m.clearSignedHashMismatch(hash) return encoded, expected, false, true } diff --git a/eth/fetcher/witness_manager_wit2_test.go b/eth/fetcher/witness_manager_wit2_test.go index f9b0b2c130..f2ae9ccca4 100644 --- a/eth/fetcher/witness_manager_wit2_test.go +++ b/eth/fetcher/witness_manager_wit2_test.go @@ -783,3 +783,58 @@ func TestVerifyAgainstSignedHashWithoutLookupIsWit1(t *testing.T) { t.Fatalf("WIT1-only path must return (nil, zero, false, true); got body=%v hash=%s diverged=%v ok=%v", body != nil, gotHash.Hex(), diverged, ok) } } + +// TestVerifyAgainstSignedHashDivergentInBandKeepsMismatchState: a within-band +// body that is NOT the BP's bytes proves nothing about the signed size, so it +// must not reset the distinct-server oversize count — otherwise a server +// alternating oversized and in-band bodies keeps a block out of quarantine +// indefinitely. Only BP-identical bytes (hash match) clear the state. +func TestVerifyAgainstSignedHashDivergentInBandKeepsMismatchState(t *testing.T) { + tw := newTestWitnessManager() + defer tw.Close() + + block := createTestBlock(304) + hash := block.Hash() + witness := createTestWitnessForBlock(block) + var buf bytes.Buffer + if err := witness.EncodeRLP(&buf); err != nil { + t.Fatal(err) + } + size := uint64(buf.Len()) + actual := stateless.WitnessCommitHash(buf.Bytes()) + + // Signed commitment the canonical body is within band of but does not hash to. + signedHash := common.HexToHash("0xd1ffe7e17") + tw.manager.parentSignedWitnessHash = func(h common.Hash) (common.Hash, uint64, bool) { + if h == hash { + return signedHash, size, true + } + return common.Hash{}, 0, false + } + primePendingWitness(tw, "peerA", block) + + if q, _ := tw.manager.recordSignedHashMismatch(hash, "oversizer-A"); q { + t.Fatal("one oversizing server must not quarantine") + } + body, _, diverged, ok := tw.manager.verifyAgainstSignedHash("peerX", hash, witness) + if !ok || !diverged || body != nil { + t.Fatalf("in-band divergent body: ok=%v diverged=%v body=%v, want accepted, diverged, no serving bytes", ok, diverged, body != nil) + } + if q, _ := tw.manager.recordSignedHashMismatch(hash, "oversizer-B"); !q { + t.Fatal("a divergent in-band acceptance reset the distinct-server oversize count; only BP-identical bytes may clear it") + } + + // Control: BP-identical bytes DO clear it — the signed commitment is proven. + signedHash = actual + tw.manager.clearSignedHashMismatch(hash) + if q, _ := tw.manager.recordSignedHashMismatch(hash, "oversizer-A"); q { + t.Fatal("one oversizing server must not quarantine") + } + body, got, diverged, ok := tw.manager.verifyAgainstSignedHash("peerY", hash, witness) + if !ok || diverged || body == nil || got != actual { + t.Fatalf("BP-identical body: ok=%v diverged=%v body=%v hash=%s, want accepted, not diverged, serving bytes", ok, diverged, body != nil, got) + } + if q, _ := tw.manager.recordSignedHashMismatch(hash, "oversizer-B"); q { + t.Fatal("BP-identical bytes must clear earlier oversize noise; second server must start a fresh count") + } +} diff --git a/eth/handler_wit.go b/eth/handler_wit.go index c14e3c4840..6911e70c8c 100644 --- a/eth/handler_wit.go +++ b/eth/handler_wit.go @@ -94,12 +94,27 @@ func (h *witHandler) Handle(peer *wit.Peer, packet wit.Packet) error { // unverifiable on the sender's say-so alone. func (h *witHandler) handleWitnessBroadcast(peer *wit.Peer, witness *stateless.Witness) error { hash := witness.Header().Hash() + hh := (*handler)(h) + + // A pusher whose earlier bytes for this block were accepted on the size + // oracle and then failed import is excluded as a witness source for the + // block — for pushes as for fetches (resolveWitnessFetchPeer). Otherwise it + // could beat every honest re-fetch with the same unusable bytes, since the + // witness manager attaches the first witness to arrive for a pending block. + if hh.witnessSourceExclusions != nil && hh.witnessSourceExclusions.excluded(hash, peer.ID()) { + wit2BroadcastExcludedSourceDropMeter.Mark(1) + peer.Log().Debug("wit2: dropping witness broadcast from a source excluded for this block after an import failure", "blockHash", hash) + return nil + } - var accepted bool - if signed, hasSigned := (*handler)(h).signedWitnesses.get(hash); hasSigned { - accepted = h.acceptSignedBroadcast(peer, witness, hash, signed) - } else if (*handler)(h).deferredAnnounces.has(hash) { - accepted = h.acceptDeferredBroadcast(peer, witness, hash) + // diverged: accepted on the size oracle alone (hash differs from the + // BP-signed one); carried into the fetcher so an import failure is charged + // to this pusher, as it is to a serving peer on the paged-fetch path. + var accepted, diverged bool + if signed, hasSigned := hh.signedWitnesses.get(hash); hasSigned { + accepted, diverged = h.acceptSignedBroadcast(peer, witness, hash, signed) + } else if hh.deferredAnnounces.has(hash) { + accepted, diverged = h.acceptDeferredBroadcast(peer, witness, hash) } else { accepted = h.acceptUnsignedBroadcast(peer, hash) } @@ -110,9 +125,9 @@ func (h *witHandler) handleWitnessBroadcast(peer *wit.Peer, witness *stateless.W // Inject the witness into the block fetcher's cache if h.blockFetcher != nil { - log.Debug("Injecting witness into block fetcher", "hash", hash, "peer", peer.ID(), "number", witness.Header().Number) + log.Debug("Injecting witness into block fetcher", "hash", hash, "peer", peer.ID(), "number", witness.Header().Number, "diverged", diverged) - if err := h.blockFetcher.InjectWitness(peer.ID(), witness); err != nil { + if err := h.blockFetcher.InjectWitness(peer.ID(), witness, diverged); err != nil { peer.Log().Warn("Failed to inject broadcast witness into fetcher", "hash", hash, "err", err) // Don't return error, just log, as block might still be importable via other means } @@ -153,17 +168,22 @@ func encodedBroadcastBytes(peer *wit.Peer, witness *stateless.Witness, hash comm // body is dropped without marking or injecting — it exceeds any plausible // non-deterministic variation. No disconnect — the sender may itself have been // fed the bytes upstream. -func (h *witHandler) acceptSignedBroadcast(peer *wit.Peer, witness *stateless.Witness, hash common.Hash, signed wit.SignedWitnessAnnouncement) bool { +// +// diverged reports a within-band body whose hash is not the BP's: the fetcher +// records it, with the pusher, as the witness's provenance so an import failure +// is charged to the pusher (chargeDivergedWitnessImportFailure) instead of +// being forgotten for free. +func (h *witHandler) acceptSignedBroadcast(peer *wit.Peer, witness *stateless.Witness, hash common.Hash, signed wit.SignedWitnessAnnouncement) (accepted bool, diverged bool) { bodyBytes, ok := encodedBroadcastBytes(peer, witness, hash) if !ok { - return false + return false, false } ceiling := (*handler)(h).witnessSizeCeiling(signed.WitnessSize) if uint64(len(bodyBytes)) > ceiling { wit2BroadcastOversizeMeter.Mark(1) peer.Log().Warn("wit2: broadcast witness exceeds the BP-signed size band; dropping", "blockHash", hash, "signedSize", signed.WitnessSize, "ceiling", ceiling, "received", len(bodyBytes)) - return false + return false, false } peer.AddKnownWitness(hash) bodyHash := stateless.WitnessCommitHash(bodyBytes) @@ -174,13 +194,13 @@ func (h *witHandler) acceptSignedBroadcast(peer *wit.Peer, witness *stateless.Wi wit2BroadcastHashDivergenceMeter.Mark(1) peer.Log().Debug("wit2: broadcast witness within the size band but not the BP's bytes; importing without re-serving", "blockHash", hash, "signed", signed.WitnessHash, "actual", bodyHash) - return true + return true, true } (*handler)(h).pendingWitnessBodies.put(hash, bodyBytes, bodyHash) // We now hold the BP's own bytes — push to any peer that asked us // for this body before we had it. (*handler)(h).pushWitnessToWaiters(hash, witness, len(bodyBytes)) - return true + return true, false } // acceptDeferredBroadcast handles a broadcast whose signed announcement is on @@ -196,10 +216,13 @@ func (h *witHandler) acceptSignedBroadcast(peer *wit.Peer, witness *stateless.Wi // post-import drain checks it against the chain-validated header. Verifying // against the header embedded in the pushed witness instead would let a peer // self-seal a fabricated header and pass its own announce as the producer's. -func (h *witHandler) acceptDeferredBroadcast(peer *wit.Peer, witness *stateless.Witness, hash common.Hash) bool { +// +// diverged reports a body accepted on the size band alone (no candidate's hash +// matched), so the fetcher can charge an import failure to the pusher. +func (h *witHandler) acceptDeferredBroadcast(peer *wit.Peer, witness *stateless.Witness, hash common.Hash) (accepted bool, diverged bool) { bodyBytes, ok := encodedBroadcastBytes(peer, witness, hash) if !ok { - return false + return false, false } // Bind against the deferred candidates' commitments: with multiple // candidates on file we accept the body if it is byte-identical to one of @@ -210,16 +233,16 @@ func (h *witHandler) acceptDeferredBroadcast(peer *wit.Peer, witness *stateless. // time, and import validates the content. hh := (*handler)(h) bodyHash := stateless.WitnessCommitHash(bodyBytes) - if !hh.deferredAnnounces.hasWitnessHash(hash, bodyHash) && - !hh.deferredAnnounces.hasWitnessSizeWithin(hash, uint64(len(bodyBytes)), hh.witnessSizeCeiling) { + hashMatch := hh.deferredAnnounces.hasWitnessHash(hash, bodyHash) + if !hashMatch && !hh.deferredAnnounces.hasWitnessSizeWithin(hash, uint64(len(bodyBytes)), hh.witnessSizeCeiling) { wit2BroadcastOversizeMeter.Mark(1) peer.Log().Warn("wit2: broadcast witness exceeds the size band of every deferred announce; dropping", "blockHash", hash, "received", len(bodyBytes)) - return false + return false, false } peer.AddKnownWitness(hash) wit2BroadcastDeferredImportMeter.Mark(1) - return true + return true, !hashMatch } // acceptUnsignedBroadcast is the WIT1 fallback with no signed announcement on diff --git a/eth/handler_wit2.go b/eth/handler_wit2.go index f77f8d750a..f2ba83cfcc 100644 --- a/eth/handler_wit2.go +++ b/eth/handler_wit2.go @@ -20,26 +20,27 @@ var errInvalidSignatureLength = errors.New("invalid wit2 announce signature leng // Metrics for WIT2 signed-announce path. Emitted only when metrics are enabled. var ( - wit2RelayInMeter = metrics.NewRegisteredMeter("eth/wit2/announce/relay_in", nil) - wit2RelayOutMeter = metrics.NewRegisteredMeter("eth/wit2/announce/relay_out", nil) - wit2InvalidSigMeter = metrics.NewRegisteredMeter("eth/wit2/announce/invalid_sig", nil) - wit2NotValidatorMeter = metrics.NewRegisteredMeter("eth/wit2/announce/not_validator", nil) - wit2DuplicateMeter = metrics.NewRegisteredMeter("eth/wit2/announce/duplicate", nil) - wit2BroadcastOversizeMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_oversize", nil) - wit2BroadcastHashDivergenceMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_hash_divergence", nil) - wit2ImplausibleSizeMeter = metrics.NewRegisteredMeter("eth/wit2/announce/implausible_size", nil) - wit2BroadcastUnverifiedSkippedMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_unverified_skipped", nil) - wit2DeferredPerPeerDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/deferred_per_peer_drop", nil) - wit2DeferredPerBlockDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/deferred_per_block_drop", nil) - wit2HeaderUnknownMeter = metrics.NewRegisteredMeter("eth/wit2/announce/header_unknown", nil) - wit2ConflictingWitnessHashMeter = metrics.NewRegisteredMeter("eth/wit2/announce/conflicting_witness_hash", nil) - wit2RateLimitDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/rate_limit_drop", nil) - wit2StrikeDisconnectMeter = metrics.NewRegisteredMeter("eth/wit2/announce/strike_disconnect", nil) - wit2WaiterPushMeter = metrics.NewRegisteredMeter("eth/wit2/serve/waiter_push", nil) - wit2WaiterPushOversizeMeter = metrics.NewRegisteredMeter("eth/wit2/serve/waiter_push_oversize", nil) - wit2BroadcastUnknownHeaderDropMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_unknown_header_drop", nil) - wit2BroadcastDeferredImportMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_deferred_import_only", nil) - wit2FetchTriggerRateLimitDropMeter = metrics.NewRegisteredMeter("eth/wit2/serve/fetch_trigger_rate_limit_drop", nil) + wit2RelayInMeter = metrics.NewRegisteredMeter("eth/wit2/announce/relay_in", nil) + wit2RelayOutMeter = metrics.NewRegisteredMeter("eth/wit2/announce/relay_out", nil) + wit2InvalidSigMeter = metrics.NewRegisteredMeter("eth/wit2/announce/invalid_sig", nil) + wit2NotValidatorMeter = metrics.NewRegisteredMeter("eth/wit2/announce/not_validator", nil) + wit2DuplicateMeter = metrics.NewRegisteredMeter("eth/wit2/announce/duplicate", nil) + wit2BroadcastOversizeMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_oversize", nil) + wit2BroadcastHashDivergenceMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_hash_divergence", nil) + wit2BroadcastExcludedSourceDropMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_excluded_source_drop", nil) + wit2ImplausibleSizeMeter = metrics.NewRegisteredMeter("eth/wit2/announce/implausible_size", nil) + wit2BroadcastUnverifiedSkippedMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_unverified_skipped", nil) + wit2DeferredPerPeerDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/deferred_per_peer_drop", nil) + wit2DeferredPerBlockDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/deferred_per_block_drop", nil) + wit2HeaderUnknownMeter = metrics.NewRegisteredMeter("eth/wit2/announce/header_unknown", nil) + wit2ConflictingWitnessHashMeter = metrics.NewRegisteredMeter("eth/wit2/announce/conflicting_witness_hash", nil) + wit2RateLimitDropMeter = metrics.NewRegisteredMeter("eth/wit2/announce/rate_limit_drop", nil) + wit2StrikeDisconnectMeter = metrics.NewRegisteredMeter("eth/wit2/announce/strike_disconnect", nil) + wit2WaiterPushMeter = metrics.NewRegisteredMeter("eth/wit2/serve/waiter_push", nil) + wit2WaiterPushOversizeMeter = metrics.NewRegisteredMeter("eth/wit2/serve/waiter_push_oversize", nil) + wit2BroadcastUnknownHeaderDropMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_unknown_header_drop", nil) + wit2BroadcastDeferredImportMeter = metrics.NewRegisteredMeter("eth/wit2/serve/broadcast_deferred_import_only", nil) + wit2FetchTriggerRateLimitDropMeter = metrics.NewRegisteredMeter("eth/wit2/serve/fetch_trigger_rate_limit_drop", nil) // wit2RelayFetchTriggeredMeter is marked synchronously, once per hash, // the moment triggerRelayFetch passes its per-hash dedup gate and // commits to spawning a fetch goroutine — before the goroutine itself diff --git a/eth/handler_wit2_announces.go b/eth/handler_wit2_announces.go index 2a49ef9681..a1882cebb0 100644 --- a/eth/handler_wit2_announces.go +++ b/eth/handler_wit2_announces.go @@ -502,22 +502,25 @@ func newSignedWitnessCache() *signedWitnessCache { // the cache did not already contain a fresh entry for this hash. Callers use // the return value to decide whether to relay (false → suppress duplicate). // -// If a fresh entry already exists with a *different* WitnessHash, the new -// announcement is rejected outright (returns false): the first valid signed -// commitment wins for the lifetime of the entry. This prevents an attacker -// who has obtained a second valid signature (e.g. a compromised producer -// later in the same window) from poisoning the cache mid-fetch and dropping -// honest serving peers against a different hash. +// If an entry already exists with a *different* commitment — another +// WitnessHash, or the same hash with another WitnessSize (the size is signed +// too and decides the accept band, so a same-hash announce with another size +// is as much a conflict as another hash) — the new announcement is rejected +// outright (returns false): the first valid signed commitment wins for the +// lifetime of the entry. This prevents an attacker who has obtained a second +// valid signature (e.g. a compromised producer later in the same window) from +// poisoning the cache mid-fetch — dropping honest serving peers against a +// different hash, or widening/narrowing the band under an in-flight fetch. func (c *signedWitnessCache) putIfNewer(ann wit.SignedWitnessAnnouncement) bool { c.mu.Lock() defer c.mu.Unlock() c.gcLocked() if existing, ok := c.entries[ann.BlockHash]; ok { - if existing.announcement.WitnessHash != ann.WitnessHash { + if existing.announcement.WitnessHash != ann.WitnessHash || existing.announcement.WitnessSize != ann.WitnessSize { wit2ConflictingWitnessHashMeter.Mark(1) return false } - // Same WitnessHash, recent: dedup. + // Same commitment, recent: dedup. if time.Since(existing.receivedAt) < wit2RelayWindow { return false } diff --git a/eth/handler_wit2_push_charge_test.go b/eth/handler_wit2_push_charge_test.go new file mode 100644 index 0000000000..7e8e003e9d --- /dev/null +++ b/eth/handler_wit2_push_charge_test.go @@ -0,0 +1,156 @@ +package eth + +import ( + "bytes" + "fmt" + "math/big" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/consensus/ethash" + "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/core/stateless" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/eth/downloader" + "github.com/ethereum/go-ethereum/eth/fetcher" + ethproto "github.com/ethereum/go-ethereum/eth/protocols/eth" + "github.com/ethereum/go-ethereum/eth/protocols/wit" + "github.com/ethereum/go-ethereum/params" + "github.com/stretchr/testify/require" +) + +// TestHandleWitnessBroadcastDivergentBodyImportFailureChargesPusher drives the +// push path end to end from the wire handler: a BP-signed announcement on +// file, a within-band body with another hash pushed by NewWitness (cached +// before its block), the block injected, the import failing with a +// witness-attributable error — and the PUSHER struck and excluded as a source +// for the block, the witness fetched from another peer, the block imported. +// It pins the provenance bit handleWitnessBroadcast hands to InjectWitness: +// without it the failure would be forgotten for free, as it was before. +func TestHandleWitnessBroadcastDivergentBodyImportFailureChargesPusher(t *testing.T) { + // Same construction as newTestHandler, but the block fetcher is swapped + // for one with a controllable insertChain BEFORE the handler starts, so + // the chain syncer starts and stops the swapped fetcher itself. + db := rawdb.NewMemoryDatabase() + gspec := &core.Genesis{ + Config: params.TestChainConfig, + Alloc: types.GenesisAlloc{testAddr: {Balance: big.NewInt(1000000)}}, + } + chain, err := core.NewBlockChain(db, gspec, ethash.NewFaker(), nil) + require.NoError(t, err) + defer chain.Stop() + hh, err := newHandler(&handlerConfig{ + Database: db, + Chain: chain, + TxPool: newTestTxPool(), + Network: 1, + Sync: downloader.SnapSync, + BloomCache: 1, + }) + require.NoError(t, err) + + head := chain.CurrentHeader() + header := &types.Header{ + ParentHash: head.Hash(), + Number: new(big.Int).Add(head.Number, big.NewInt(1)), + GasLimit: head.GasLimit, + Time: head.Time + 2, + } + hash := header.Hash() + block := types.NewBlockWithHeader(header) + + var ( + mu sync.Mutex + strikes []string + excluded []string + imports atomic.Int32 + fetches atomic.Int32 + ) + // First import fails with an error the witness could have caused; the + // second (with the re-fetched witness) succeeds. + insertChain := func(blocks types.Blocks, _ []*stateless.Witness) (int, error) { + if imports.Add(1) == 1 { + return 0, fmt.Errorf("%w (cross: 01 local: 02)", core.ErrStatelessStateRootMismatch) + } + return len(blocks), nil + } + getBlock := func(bh common.Hash) *types.Block { + if bh == head.Hash() { + return chain.GetBlockByHash(bh) // the parent is known, the block is not + } + return nil + } + f := fetcher.NewBlockFetcher(false, nil, getBlock, func(*types.Header) error { return nil }, + func(*types.Block, *stateless.Witness, bool) {}, func() uint64 { return head.Number.Uint64() }, chain.CurrentHeader, + nil, insertChain, func(string) {}, false, true, 30_000_000, nil, nil) + f.SetWitnessServerStriker(func(id string) { + mu.Lock() + strikes = append(strikes, id) + mu.Unlock() + }) + f.SetWitnessSourceExcluder(func(peer string, bh common.Hash) { + if bh != hash { + t.Errorf("exclusion for unexpected block %s", bh) + } + mu.Lock() + excluded = append(excluded, peer) + mu.Unlock() + }) + hh.blockFetcher = f + hh.Start(1000) + defer hh.Stop() + + witH := (*witHandler)(hh) + pusher, cleanup := newTestWit2PeerWithReader() + defer cleanup() + + witness, err := stateless.NewWitness(header, nil) + require.NoError(t, err) + var buf bytes.Buffer + require.NoError(t, witness.EncodeRLP(&buf)) + // Signed commitment the pushed body is within band of but does not hash to. + hh.signedWitnesses.putIfNewer(wit.SignedWitnessAnnouncement{ + BlockHash: hash, + BlockNumber: header.Number.Uint64(), + WitnessHash: common.HexToHash("0xdeadbeef"), + WitnessSize: uint64(buf.Len()), + Signature: make([]byte, wit.SignatureLength), + }) + // The push is processed on the witness manager loop before the block + // injection below is even received (unbuffered channels, one loop). + require.NoError(t, witH.handleWitnessBroadcast(pusher, witness)) + + fetchWitness := func(_ common.Hash, sink chan *ethproto.Response) (*ethproto.Request, error) { + n := fetches.Add(1) + req := ðproto.Request{Peer: fmt.Sprintf("server-%d", n), Cancel: make(chan struct{})} + go func() { + w, err := stateless.NewWitness(header, nil) + if err != nil { + return + } + sink <- ðproto.Response{Req: req, Res: []*stateless.Witness{w}, Time: time.Millisecond, Done: make(chan error, 1)} + }() + return req, nil + } + require.NoError(t, f.InjectBlockWithWitnessRequirement("origin", block, fetchWitness)) + + deadline := time.Now().Add(10 * time.Second) + for imports.Load() < 2 { + if time.Now().After(deadline) { + mu.Lock() + defer mu.Unlock() + t.Fatalf("block never re-imported after the pushed witness failed (imports=%d fetches=%d strikes=%v excluded=%v)", + imports.Load(), fetches.Load(), strikes, excluded) + } + time.Sleep(5 * time.Millisecond) + } + mu.Lock() + defer mu.Unlock() + require.Equal(t, []string{pusher.ID()}, strikes, "the pusher of the divergent body must be struck exactly once") + require.Equal(t, []string{pusher.ID()}, excluded, "the pusher must be excluded as a witness source for the block") + require.Equal(t, int32(1), fetches.Load(), "exactly one re-fetch, from another peer, after the pushed body failed import") +} diff --git a/eth/handler_wit2_test.go b/eth/handler_wit2_test.go index d79dd3db70..5764720764 100644 --- a/eth/handler_wit2_test.go +++ b/eth/handler_wit2_test.go @@ -1472,3 +1472,182 @@ func TestMaySignAnnouncementForBlockBindsToSealer(t *testing.T) { maySignAnnouncementForBlock(engine, unsealable, producer, 200, unsealable.Hash()), "a header without a recoverable sealer must refuse the producer binding") } + +// TestSignedWitnessCacheRejectsConflictingWitnessSize: WitnessSize is signed +// and decides the accept band, so a second producer-signed announcement that +// agrees on WitnessHash but carries another size is a conflict, not a refresh — +// otherwise it would replace the band under an in-flight fetch once the relay +// window lapsed. +func TestSignedWitnessCacheRejectsConflictingWitnessSize(t *testing.T) { + c := newSignedWitnessCache() + first := wit.SignedWitnessAnnouncement{ + BlockHash: common.HexToHash("0xabce"), + BlockNumber: 51, + WitnessHash: common.HexToHash("0x1111"), + WitnessSize: 4096, + Signature: make([]byte, wit.SignatureLength), + } + if !c.putIfNewer(first) { + t.Fatal("first put should succeed") + } + conflict := first + conflict.WitnessSize = 10 * 4096 + if c.putIfNewer(conflict) { + t.Fatal("same-hash announce with a different signed WitnessSize must be rejected") + } + // Age the entry past the relay window: an identical re-announce would now + // refresh it, so only the conflict rule can be what rejects the other size. + c.mu.Lock() + c.entries[first.BlockHash].receivedAt = time.Now().Add(-2 * wit2RelayWindow) + c.mu.Unlock() + if c.putIfNewer(conflict) { + t.Fatal("same-hash announce with a different WitnessSize must be rejected after the relay window too; it would replace the accept band") + } + got, ok := c.get(first.BlockHash) + if !ok { + t.Fatal("first announcement must remain cached after conflict rejection") + } + if got.WitnessSize != first.WitnessSize { + t.Fatalf("cache poisoned: WitnessSize=%d want=%d", got.WitnessSize, first.WitnessSize) + } + if !c.putIfNewer(first) { + t.Fatal("identical commitment after the relay window must refresh (return true): the size rule must not reject the same commitment") + } +} + +// TestAcceptSignedBroadcastReportsDivergence pins the provenance bit the +// broadcast path hands to the fetcher: a within-band body that is not the BP's +// bytes is accepted AND flagged diverged (so an import failure is charged to the +// pusher, as on the fetch path); BP-identical bytes are accepted, not diverged; +// an oversized body is neither. +func TestAcceptSignedBroadcastReportsDivergence(t *testing.T) { + h := newTestHandler() + defer h.close() + + witH := (*witHandler)(h.handler) + peer, cleanup := newTestWit2PeerWithReader() + defer cleanup() + + header := &types.Header{Number: big.NewInt(7781)} + hash := header.Hash() + rawdb.WriteHeader(h.chain.DB(), header) + witness, err := stateless.NewWitness(header, nil) + require.NoError(t, err) + var buf bytes.Buffer + require.NoError(t, witness.EncodeRLP(&buf)) + + signed := wit.SignedWitnessAnnouncement{ + BlockHash: hash, + BlockNumber: header.Number.Uint64(), + WitnessHash: common.HexToHash("0xdeadbeef"), + WitnessSize: uint64(buf.Len()), + Signature: make([]byte, wit.SignatureLength), + } + accepted, diverged := witH.acceptSignedBroadcast(peer, witness, hash, signed) + require.True(t, accepted, "within-band body must be accepted for import") + require.True(t, diverged, "within-band body with another hash must be flagged diverged so an import failure is charged to the pusher") + + signed.WitnessHash = stateless.WitnessCommitHash(buf.Bytes()) + accepted, diverged = witH.acceptSignedBroadcast(peer, witness, hash, signed) + require.True(t, accepted) + require.False(t, diverged, "BP-identical bytes are not diverged") + + signed.WitnessSize = 1 + accepted, diverged = witH.acceptSignedBroadcast(peer, witness, hash, signed) + require.False(t, accepted, "oversized body must be dropped") + require.False(t, diverged) +} + +// TestAcceptDeferredBroadcastReportsDivergence: the deferred accept path flags a +// body accepted on a candidate's size band alone as diverged, and a body +// byte-identical to a candidate's commitment as not diverged. +func TestAcceptDeferredBroadcastReportsDivergence(t *testing.T) { + h := newTestHandler() + defer h.close() + + witH := (*witHandler)(h.handler) + peer, cleanup := newTestWit2PeerWithReader() + defer cleanup() + + header := &types.Header{Number: big.NewInt(7783)} + hash := header.Hash() + witness, err := stateless.NewWitness(header, nil) + require.NoError(t, err) + var buf bytes.Buffer + require.NoError(t, witness.EncodeRLP(&buf)) + + // Deferred candidate with another hash, size within band → diverged. + h.handler.deferredAnnounces.put(wit.SignedWitnessAnnouncement{ + BlockHash: hash, + BlockNumber: header.Number.Uint64(), + WitnessHash: common.HexToHash("0xdeadbeef"), + WitnessSize: uint64(buf.Len()), + Signature: make([]byte, wit.SignatureLength), + }, "announcer-1") + accepted, diverged := witH.acceptDeferredBroadcast(peer, witness, hash) + require.True(t, accepted) + require.True(t, diverged, "deferred body accepted on the size band alone must be flagged diverged") + + // A candidate whose hash the body matches → not diverged. + h.handler.deferredAnnounces.put(wit.SignedWitnessAnnouncement{ + BlockHash: hash, + BlockNumber: header.Number.Uint64(), + WitnessHash: stateless.WitnessCommitHash(buf.Bytes()), + WitnessSize: uint64(buf.Len()), + Signature: make([]byte, wit.SignatureLength), + }, "announcer-2") + accepted, diverged = witH.acceptDeferredBroadcast(peer, witness, hash) + require.True(t, accepted) + require.False(t, diverged, "body byte-identical to a deferred candidate is not diverged") +} + +// TestHandleWitnessBroadcastDropsExcludedSource: a peer excluded as a witness +// source for a block (its earlier size-oracle-accepted bytes failed import) is +// refused on the push path too — not marked as a body-holder, nothing cached, +// nothing injected — even when the bytes it now pushes are the BP's own. +// Without this it could beat every honest re-fetch with the same body, since +// the witness manager attaches the first witness to arrive. Another peer's +// identical push is unaffected. +func TestHandleWitnessBroadcastDropsExcludedSource(t *testing.T) { + h := newTestHandler() + defer h.close() + + witH := (*witHandler)(h.handler) + excludedPeer, cleanup := newTestWit2PeerWithReader() + defer cleanup() + otherPeer, cleanup2 := newTestWit2PeerWithReader() + defer cleanup2() + + header := &types.Header{Number: big.NewInt(7784)} + hash := header.Hash() + rawdb.WriteHeader(h.chain.DB(), header) + witness, err := stateless.NewWitness(header, nil) + require.NoError(t, err) + var buf bytes.Buffer + require.NoError(t, witness.EncodeRLP(&buf)) + + h.handler.signedWitnesses.putIfNewer(wit.SignedWitnessAnnouncement{ + BlockHash: hash, + BlockNumber: header.Number.Uint64(), + WitnessHash: stateless.WitnessCommitHash(buf.Bytes()), + WitnessSize: uint64(buf.Len()), + Signature: make([]byte, wit.SignatureLength), + }) + h.handler.witnessSourceExclusions.add(hash, excludedPeer.ID()) + + require.NoError(t, witH.handleWitnessBroadcast(excludedPeer, witness)) + if excludedPeer.KnownWitnessContainsHash(hash) { + t.Fatal("excluded pusher must not be marked as a body-holder") + } + if _, _, ok := h.handler.pendingWitnessBodies.get(hash); ok { + t.Fatal("excluded pusher's bytes must not enter the pre-import serving cache") + } + + require.NoError(t, witH.handleWitnessBroadcast(otherPeer, witness)) + if !otherPeer.KnownWitnessContainsHash(hash) { + t.Fatal("a non-excluded peer's identical push must still be accepted") + } + if _, _, ok := h.handler.pendingWitnessBodies.get(hash); !ok { + t.Fatal("BP-identical bytes from a non-excluded peer must be cached for serving") + } +} From a4d7a6aebe4c4ec9a222f1041d0317d4e834de11 Mon Sep 17 00:00:00 2001 From: Lucca Martins Date: Mon, 21 Sep 2026 20:06:35 -0400 Subject: [PATCH 10/10] eth/fetcher: move the import-failure charge next to its error predicate Diffguard's file-size gate allows a file already over 800 lines to grow by 10% of its base size; block_fetcher.go had reached +138 (10.4%) on the candidate base. chargeDivergedWitnessImportFailure and maxWitnessImportRetries move to witness_import_errors.go, which already holds isWitnessAttributableImportError, the predicate the charge is gated on. No behaviour change. --- eth/fetcher/block_fetcher.go | 48 --------------------------- eth/fetcher/witness_import_errors.go | 49 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 48 deletions(-) diff --git a/eth/fetcher/block_fetcher.go b/eth/fetcher/block_fetcher.go index e35472b5fe..9a2d919f35 100644 --- a/eth/fetcher/block_fetcher.go +++ b/eth/fetcher/block_fetcher.go @@ -1227,14 +1227,6 @@ func (f *BlockFetcher) importHeaders(peer string, header *types.Header) { }() } -// maxWitnessImportRetries bounds how many times a block whose import failed with -// a witness accepted on the WIT2 size oracle alone is re-fetched from another -// source before the fetcher gives it up as it would any other failed import. A -// small bound keeps a genuinely invalid block (every honest witness fails) from -// cycling through the peer set, while still recovering from a single server -// that handed out an unusable within-band witness. -const maxWitnessImportRetries = 2 - // importBlocks spawns a new goroutine to run a block insertion into the chain. If the // block's number is at the same height as the current import phase, it updates // the phase states accordingly. @@ -1352,46 +1344,6 @@ func (f *BlockFetcher) logTrackedImport(block *types.Block, hash common.Hash) { log.Info(msg, "number", block.Number().Uint64(), "hash", hash, "delay", prettyDelay, "delayInMs", delayInMs, "totalDelay", totalDelay, "totalDelayInMs", totalDelayInMs) } -// chargeDivergedWitnessImportFailure applies the WIT2 consequence of an import -// failure to the peer that served the block's witness, when that witness was -// accepted on the size oracle alone AND the failure is one the witness could -// have caused (isWitnessAttributableImportError). It strikes the peer, excludes -// it as a witness source for this block, and reports whether the block should -// be handed back to the witness manager for a re-fetch (false once the retry -// budget is spent, or when the witness was not a fetched, diverged one). -// -// The error gate matters because "diverged" is the normal case — every node -// persists its own generated witness, so nearly every witness fetched from -// anyone but the BP differs from the signed hash. Charging every import -// failure would let a local problem (a contract bytecode missing from disk, -// which the downloader heals; an interrupted insert) strike and exclude two -// honest witness sources per block until the node has none left. -func (f *BlockFetcher) chargeDivergedWitnessImportFailure(op *blockOrHeaderInject, importErr error) bool { - if op.witness == nil || !op.witnessDiverged || op.witnessPeer == "" { - return false - } - hash := op.hash() - if !isWitnessAttributableImportError(importErr) { - log.Debug("Import failed for a reason the witness server did not cause; not charging it", - "server", op.witnessPeer, "number", op.number(), "hash", hash, "err", importErr) - return false - } - witnessImportFailureMeter.Mark(1) - log.Warn("Import failed with a witness accepted on the WIT2 size oracle; striking its server", - "server", op.witnessPeer, "number", op.number(), "hash", hash, "attempt", op.witnessImportFailures+1, "err", importErr) - f.wm.StrikeWitnessServer(op.witnessPeer) - f.wm.ExcludeWitnessSource(op.witnessPeer, hash) - - op.witnessImportFailures++ - if op.fetchWitness == nil || op.witnessImportFailures >= maxWitnessImportRetries { - log.Warn("Giving up witness re-fetch for block after repeated import failures", - "number", op.number(), "hash", hash, "failures", op.witnessImportFailures) - return false - } - witnessImportRetryMeter.Mark(1) - return true -} - // forgetHash removes all traces of a block announcement from the fetcher's // internal state. func (f *BlockFetcher) forgetHash(hash common.Hash) { diff --git a/eth/fetcher/witness_import_errors.go b/eth/fetcher/witness_import_errors.go index 9225f89d9a..e22cdab1b2 100644 --- a/eth/fetcher/witness_import_errors.go +++ b/eth/fetcher/witness_import_errors.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/trie" ) @@ -72,3 +73,51 @@ func isWitnessAttributableImportError(err error) bool { } return false } + +// maxWitnessImportRetries bounds how many times a block whose import failed with +// a witness accepted on the WIT2 size oracle alone is re-fetched from another +// source before the fetcher gives it up as it would any other failed import. A +// small bound keeps a genuinely invalid block (every honest witness fails) from +// cycling through the peer set, while still recovering from a single server +// that handed out an unusable within-band witness. +const maxWitnessImportRetries = 2 + +// chargeDivergedWitnessImportFailure applies the WIT2 consequence of an import +// failure to the peer that served the block's witness, when that witness was +// accepted on the size oracle alone AND the failure is one the witness could +// have caused (isWitnessAttributableImportError). It strikes the peer, excludes +// it as a witness source for this block, and reports whether the block should +// be handed back to the witness manager for a re-fetch (false once the retry +// budget is spent, or when the witness was not a fetched, diverged one). +// +// The error gate matters because "diverged" is the normal case — every node +// persists its own generated witness, so nearly every witness fetched from +// anyone but the BP differs from the signed hash. Charging every import +// failure would let a local problem (a contract bytecode missing from disk, +// which the downloader heals; an interrupted insert) strike and exclude two +// honest witness sources per block until the node has none left. +func (f *BlockFetcher) chargeDivergedWitnessImportFailure(op *blockOrHeaderInject, importErr error) bool { + if op.witness == nil || !op.witnessDiverged || op.witnessPeer == "" { + return false + } + hash := op.hash() + if !isWitnessAttributableImportError(importErr) { + log.Debug("Import failed for a reason the witness server did not cause; not charging it", + "server", op.witnessPeer, "number", op.number(), "hash", hash, "err", importErr) + return false + } + witnessImportFailureMeter.Mark(1) + log.Warn("Import failed with a witness accepted on the WIT2 size oracle; striking its server", + "server", op.witnessPeer, "number", op.number(), "hash", hash, "attempt", op.witnessImportFailures+1, "err", importErr) + f.wm.StrikeWitnessServer(op.witnessPeer) + f.wm.ExcludeWitnessSource(op.witnessPeer, hash) + + op.witnessImportFailures++ + if op.fetchWitness == nil || op.witnessImportFailures >= maxWitnessImportRetries { + log.Warn("Giving up witness re-fetch for block after repeated import failures", + "number", op.number(), "hash", hash, "failures", op.witnessImportFailures) + return false + } + witnessImportRetryMeter.Mark(1) + return true +}