Skip to content

refactor(safrole): cache ring verifier by validator-set hash, not epoch#1041

Merged
TwEricShen merged 1 commit into
mainfrom
perf/ring-verifier-cache-by-validator-set
Jul 10, 2026
Merged

refactor(safrole): cache ring verifier by validator-set hash, not epoch#1041
TwEricShen merged 1 commit into
mainfrom
perf/ring-verifier-cache-by-validator-set

Conversation

@YCC3741

@YCC3741 YCC3741 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Closes #1040

Why

GetVerifier cached one Bandersnatch ring verifier keyed by epoch, and ChainState.restoreWithState (fork-restore) called ClearVerifierCache() on every restore. The next GetVerifier then rebuilt the verifier via vrf.NewVerifier (the expensive KZG ring construction). In fork-heavy fuzzing this rebuild recurs on nearly every block and dominates Safrole import time.

The blanket clear existed only because the epoch key was too coarse: same-epoch forks can carry different validator sets, so an epoch-only cache could return a verifier built from the wrong fork's keys.

What changed

  • internal/blockchain/ring_verifier.go: the cache is now a bounded map[OpaqueHash]*vrf.Verifier keyed by Blake2bHash(concatenated bandersnatch keys). GetVerifier drops the epoch parameter; double-checked locking; clear-all eviction at 32 entries. Clear/evict only drop Go references (never Free a verifier a reader may hold), preserving the prior finalizer-based use-after-free safety.
  • internal/blockchain/chain_state.go: restoreWithState no longer clears the cache — the content-hash key makes same-validator-set forks hit and different-validator-set forks miss, so clearing is unnecessary.
  • internal/safrole/safrole.go: call site updated to GetVerifier(postGammaK).
  • internal/blockchain/ring_verifier_test.go: new unit tests (hit / rebuild / eviction / clear / concurrent / wrong-size).

The ring commitment is a pure function of the next epoch's validator bandersnatch keys (GP 0.8.0, gamma_z = O([k_b | k in gamma'_P]); RFC-0026 §6.6), so the epoch is irrelevant to the verifier's identity and is dropped from the key.

Correctness

  • go vet clean; unit tests pass under -race (hit / rebuild / eviction / clear / concurrent / wrong-size).
  • jam-conformance test_folder (0.7.2): 1179 PASSED / 0 FAILED.
  • jam-test-vectors fuzzy: 201 PASSED / 0 FAILED.
  • jam-testing minifuzz (forks, no_forks, fallback, safrole, storage, storage_light) and picofuzz (fallback, safrole, storage, storage_light): all pass.

Improvement

picofuzz (tiny spec, median of 3). Numbers are per-block import latency aggregated across all blocks in a suite (microseconds per block), not the suite's total wall-clock time:

suite mean before -> after mean p99 before -> after p99
fallback 4082 -> 1917 us -53% 29531 -> 2884 us -90%
safrole 5839 -> 3723 us -36% 32621 -> 7015 us -78%
storage 6856 -> 4732 us -31% 33673 -> 12998 us -61%
storage_light 5770 -> 3644 us -37% 31319 -> 7254 us -77%

Median is essentially unchanged (typical blocks never rebuilt); the gain is in the tail — periodic epoch-boundary / fork blocks that previously rebuilt the ring verifier. stdDev roughly halves (more consistent import latency). This is tiny mode (ring = 6); full mode (ring = 1023) rebuilds are far more expensive, so the effect scales up.

End-to-end whole-suite wall time (median of 3) also drops. This is a supporting signal only: most of it is fixed overhead (target container startup, per-block socket round-trips, debug logging), so it is noisier and dilutes the effect compared with the per-block numbers above.

suite wall time before -> after change
fallback 17.08 -> 14.14 s -17.2%
safrole 18.78 -> 16.66 s -11.3%
storage 19.80 -> 17.72 s -10.5%
storage_light 18.75 -> 16.59 s -11.5%

Test plan

  • go vet + unit tests under -race
  • jam-conformance test_folder: 1179 PASSED / 0 FAILED
  • jam-test-vectors fuzzy: 201 PASSED / 0 FAILED
  • jam-testing minifuzz (6) + picofuzz (4): all pass
  • picofuzz before/after, median of 3

Fork-restore (restoreWithState) called ClearVerifierCache() on every block,
so the next GetVerifier had to rebuild the Bandersnatch ring verifier (the
expensive KZG ring construction) from scratch. The verifier is fully
determined by the validator set's bandersnatch keys (GP 0.8.0:
z = O([k_b | k in gamma'_P])), so key the cache by a Blake2b hash of those
keys in a small bounded map instead of by epoch.

Same-validator-set forks and stable-validator epoch boundaries now hit the
cache, and the restore no longer needs to clear it. Correctness across forks
is guaranteed by the key: a different validator set hashes differently and
misses. Eviction and clear only drop Go references (never Free the Rust
verifier under a reader), preserving the existing use-after-free safety.

picofuzz (tiny, median-of-3): per-block import p99 -61..90%, mean -31..53%,
stdDev roughly halved; median unchanged. Correctness unchanged: conformance
1179/0, fuzzy 201/0, minifuzz (6) and picofuzz (4) all green, unit tests
(hit/rebuild/eviction/clear/concurrent) pass under -race.
@YCC3741
YCC3741 requested review from TwEricShen and yu2C July 7, 2026 22:03
@YCC3741 YCC3741 self-assigned this Jul 7, 2026
@YCC3741 YCC3741 changed the title perf(safrole): cache ring verifier by validator-set hash, not epoch refactor(safrole): cache ring verifier by validator-set hash, not epoch Jul 7, 2026
@TwEricShen

Copy link
Copy Markdown
Contributor

3 suggestion:

  1. This cache can also benefit CE131: verifySafroleTicketProof currently rebuilds a ring verifier via vrf.NewVerifier(...) for every incoming ticket message, repeating the expensive KZG construction this PR avoids. It could call blockchain.GetVerifier(validators) instead. Note the defer verifier.Free() must go — cached verifiers are shared and reclaimed by the GC finalizer.

  2. Building on 1: once CE131 goes through GetVerifier, the cache is accessed concurrently, which exposes a contention issue.
    On a cache miss, vrf.NewVerifier (potentially hundreds of milliseconds in full mode) runs while holding the global write lock.
    Every other caller — even cache hits for different validator sets — blocks behind the rebuild, so concurrent network messages are effectively serialized.
    I'd suggest per-key singleflight requests for the same key share one construction, other keys proceed unblocked, and the map lock is only held for cheap reads and writes.

  3. The inputs to ticket verification (the ring and η₂') only change once per epoch, so within an epoch the same ticket always verifies to the same result — including across forks. This makes the verification itself cacheable:

key = hash(ringKey ‖ context ‖ signature)
  value = 32-byte VRF output

Only successful verifications are stored, so garbage signatures cannot pollute the cache; a miss simply falls back to normal verification.

@TwEricShen
TwEricShen merged commit 7203337 into main Jul 10, 2026
1 check passed
@YCC3741
YCC3741 deleted the perf/ring-verifier-cache-by-validator-set branch July 15, 2026 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(safrole): ring verifier cache keyed by epoch forces rebuild on every fork-restore

3 participants