Narya Ed25519 integration + FD/Solana Conformance Suite refresh - #258
Merged
Conversation
Agave v4.2.0-beta.1 verifies every P2P/consensus signature — transactions, gossip (CRDS, ping/pong, prune), shreds, and repair — through solana-signature -> ed25519-dalek 2.2.0 verify_strict (confirmed uniform by reading each path). verify_strict rejects small-order public keys A and small-order signature points R; Go's crypto/ed25519.Verify does not (it never decodes R). Mithril used the standard library at every one of these sites, so it accepted an adversarial-only class of signatures mainnet rejects — a crafted block, gossip packet, or repair message could be admitted by Mithril and discarded by mainnet, splitting the node from consensus. Route all eight sites through narya.VerifyStrict, which enforces DalekStrict regardless of any global profile: gossip contact-info and CRDS value (contact_info.go), ping/pong (message.go), turbine shred reference and cache paths (shred.go, sigcache.go), and repair ping and signed request (protocol.go). The shred cache only inserts after a successful verify and never caches failures, so the single miss-path check is sufficient — a strict rejection can never be memoized as valid. Tests: repair carries the rigorous proof — real ed25519vectors that crypto/ed25519 accepts and verify_strict rejects (small-order A and R) are now rejected by VerifySignedRequest; gossip and turbine cover the honest happy path, small-order-key rejection, agreement with narya.VerifyStrict on both paths, and that the shred cache never caches a strict rejection. The transaction replay path is fixed separately in the narya sigverify integration. The ed25519 precompile (pkg/sealevel) is a separate, feature-gated path analyzed under its own task. narya is wired via a local replace directive (../narya) for now; it must become a published version pin before this branch is shared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New package owning backend selection and the batch entry point. Mithril verifies with stdlib today, which is non-strict, so it accepts small-order A and R that ed25519-dalek verify_strict -- and therefore mainnet -- rejects. Every path here applies the strict predicate. Batch is reusable worker-local scratch and Drain is the fill policy: block for the first item, non-blocking peek for the rest, no timer. Batch width is what sets cost per signature (a lone signature is roughly 3.7x the per-signature cost of one inside a group of eight), but a batching timer would trade tip latency away to buy that, so batches are whatever happened to be queued. The consensus claim is tested rather than asserted: the suite constructs a signature with a small-order public key that stdlib accepts and this package rejects, and pins that backend=stdlib faithfully reintroduces it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pool worker blocked for one job and verified it alone. One transaction carries one or two signatures, which is the worst shape for a backend that verifies eight per AVX-512 group: a lone signature pays for a whole group and uses one lane of it. Workers now drain what is already queued and verify the group in one call. Draining costs nothing on an empty queue -- the worker still takes exactly the job it blocked for -- and a backlog here means catch-up, which is when throughput matters and per-block latency does not. Attribution is preserved exactly. The backend reports a verdict per signature, so the failing signer is identified without re-verifying anything, and the panic still names it. Tests place the bad signature at every boundary of a full group, and cover release of jobs belonging to different blocks. Count on the Sigverify timing now counts groups rather than transactions; SumNanoseconds keeps its documented meaning as total async worker time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two TPU verifiers each marshalled the signed message themselves and so skipped the version-byte fixup. solana-go's MarshalV0 writes 0x7f, which is not the Solana wire encoding -- the signed prefix is 0x80 -- so a correctly signed versioned transaction was checked against bytes no client ever signs and was dropped at ingest. Both now go through txverify, which owns that fixup, and pick up the strict predicate at the same time. The pipeline worker verified one packet at a time. A transaction carries one or two signatures against a backend that verifies eight per group, so it paid for a group and used one lane. Workers now drain what is already queued; an empty queue still yields exactly the packet the worker blocked for, so quiet-ingress latency is unchanged. An unparseable packet contributes no signature lane, so the batch keeps a nil placeholder to stop verdicts sliding onto their neighbours -- covered by a test that interleaves garbage, corrupted, and honest packets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Draining unconditionally was a latency regression waiting to happen. Eight workers facing eight queued items would let the first worker take all eight and verify them as one group while the other seven idled -- and one group of eight costs more wall-clock than eight workers each verifying one. Batching buys throughput per core; it must not buy it with parallelism that was already there. A turbine cancellation regression test caught exactly this: one worker swallowed both jobs meant to occupy two workers. FairShare gives a worker one item plus an equal cut of what remains, so a shallow queue spreads and a deep queue -- catch-up, where every worker is saturated anyway -- gives everyone full groups. The floor is 1: the item already in hand is never given back. Turbine additionally admits workers*BatchTarget transactions per wave rather than one per worker, since a producer that meters work out one-per-worker makes groups unreachable no matter what the consumer does. The fairness bound it was protecting is unchanged in spirit: tens of jobs parked, not tens of thousands. Nothing waits for a target width anywhere, so a partly-filled group cannot strand work. That property is now pinned in all four pipelines with counts that divide badly into groups (1, 3, 7, 9, 33, 65, 129); a stranded item hangs the join and fails the test rather than passing quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A package-wide gofmt -w while iterating reformatted files this change does not touch. Whitespace-only, but it does not belong in a consensus-critical diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment said the test 'hangs rather than failing quietly', which reads backwards. The point is that asserting on the join completing -- rather than just on no error being returned -- means a stranded item surfaces as a timeout instead of slipping through as a pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
VerifySignedRequest authenticates repair requests from peers. Mithril is requester-side only today so nothing outside tests reaches it, but serving repair is normal validator work, and when it is wired up this becomes a packet-rate consumer on a socket loop -- the shape that wants Drain/FairShare rather than a one-at-a-time verify. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing selected a backend, so every verification ran on the portable pure-Go path: the strict predicate was live but none of the acceleration was, and the stdlib rollback switch was unreachable. This is the link that turns it on. --sigverify-backend / tuning.sigverify_backend resolves through the house flag-over-TOML precedence and installs the backend during config parsing, so a machine that cannot run the requested one fails at startup rather than at the first block. The resolved name is printed at startup because under 'auto' it is the only way an operator learns whether they got the accelerated path -- and because backend=stdlib deserves to say out loud that it accepts signatures mainnet rejects. Replay sigverify had no Prometheus series at all. It gets two: group duration and group width. Width is the one worth watching -- it is the difference between paying for a vector group and using it, and no backend setting can compensate for work arriving too thinly to fill one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The branch carried 'replace => ../narya' while the library was private, which made it unbuildable for anyone without that checkout sitting beside the repo -- CI included. narya-ed25519 is public now, so this pins an ordinary pseudo-version, matching how Overclock-Validator/crypto is consumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The strict-path options were built as a struct literal setting only AllowSmallOrderA, AllowSmallOrderR and CofactorlessVerify. Go zeroes every field a literal omits, so AllowNonCanonicalA silently became false and the precompile rejected public keys the reference predicate accepts. voi's own VerifyOptionsStdLib sets that field true, so the omission also inverted the library's default rather than merely leaving it. No constructible input separates the two behaviours: a non-canonical encoding requires y < 19, and every curve point with such a y whose discrete log is computable is small order, which AllowSmallOrderA already rejects. So this changes no verdict reachable today. It is worth fixing anyway, because the gap between "what we meant" and "what the literal said" is not visible at the call site, and the next field to go missing may not be unreachable. Moving the options into a named package-level value is the part that makes the invariant testable. The tests pin each field individually so a regression names the field it broke, and record why AllowNonCanonicalR stays unset: voi panics on AllowNonCanonicalR together with CofactorlessVerify, so setting it would crash on the first precompile instruction rather than loosen a check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The precompile was the last verification site still on curve25519-voi, and the only one that did not honour the backend selection or the stdlib rollback switch that pkg/sigverify owns. Route the strict path through sigverify.VerifyOne so it uses the same narya DalekStrict predicate, the same backend, and the same kill switch as replay, TPU, gossip, repair and turbine. The non-strict branch, which runs only when replaying history from before Ed25519PrecompileVerifyStrict activated, now calls crypto/ed25519 directly. The reference used plain non-strict verification there -- cofactorless, no small-order rejection, non-canonical A accepted, R compared as bytes -- which is exactly what the standard library does. voi's default options were none of those things: they are cofactored and reject a non-canonical A, so that branch had been running a predicate the reference never used. narya exposes no StdlibCompat entry point, so the stdlib is both correct and the smallest dependency for that path. Signatures are still verified one at a time. The reference walks entries in order and returns the first error, so batching would let a later entry's offset error preempt an earlier entry's signature error, and that error code reaches the ledger. The ordering is consensus-visible and not worth trading for the throughput of a batch that is usually one or two signatures deep. This supersedes the AllowNonCanonicalA fix in the previous commit; the option struct it repaired no longer exists. The tests now guard the wiring rather than the struct literal, since the predicate itself is covered by narya's CCTV, Wycheproof and edge corpora. Verified against Agave v4.2.0-rc.0: precompiles is still pinned to ed25519-dalek "=1.0.1" and the workspace to 2.2, unchanged from beta.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The precompile conformance tests read test-vectors/precompile/fixtures/<program>,
a layout that no longer exists upstream. ReadDir failed, the assertion failed,
and no fixture was ever executed. Firedancer now publishes every precompile
fixture in one flat instr/fixtures/precompile directory and distinguishes them
by the program id inside the fixture, so filtering has to parse rather than glob.
A second and quieter problem sat underneath. The fixture schema gained a
metadata message at field 1 and pushed input and output to fields 2 and 3, but
the generated bindings here still say input is 1 and output is 2. Protobuf does
not error on that: it reads the metadata submessage as an InstrContext and
yields a context with no program id and no accounts. Every test built on those
bindings would have compared nothing while reporting success. unmarshalInstrFixture
splits the wrapper by hand and decodes the two submessages with the existing
inner types, which have not drifted, and refuses a fixture with no metadata
field rather than guessing at an older shape.
parseAndConfigureFeatures dereferenced through Input.EpochContext
unconditionally and panicked on the precompile corpus, which mostly carries no
epoch context. It now treats a missing one as "defaults only". Its per-feature
logging moved behind MITHRIL_CONFORMANCE_VERBOSE, which the vm-programs test
already used; unguarded it emits a line per feature per fixture across thousands
of fixtures.
Results against firedancer-io/test-vectors:
ed25519 3316 / 3479
secp256k1 2523 / 2628
secp256r1 13185 / 13185
secp256r1 is the largest fixture set in the corpus and had no test at all.
The remaining ed25519 and secp256k1 failures share one cause, and it is not the
predicate. InstrContext.epoch_context also moved, from field 9 to field 10, so
the bindings read nil, parseAndConfigureFeatures sees no active features, and
every feature-gated branch evaluates as though nothing had ever activated. For
the ed25519 precompile that selects the pre-activation non-strict path, which
accepts signatures strict rejects -- exactly the observed "we accepted, fixture
expects an error". Field 10 is not universally an epoch context across the
corpus, so recovering it by hand is a guess rather than a fix; regenerating the
bindings from firedancer-io/protosol v5.3.0 is the correct repair and is left
as follow-up work.
Tests skip rather than fail when the corpus is absent. It is a ~7 GB external
checkout that is deliberately gitignored, so 'make conformance-vectors' fetches
or updates it and 'make test-conformance-precompiles' runs all three suites.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generated bindings described a schema several versions old. Three changes
mattered:
InstrFixture gained metadata at field 1, pushing input to 2 and output to 3
InstrContext dropped epoch_context and slot_context; the FeatureSet now sits
directly at field 10
AcctState dropped rent_epoch
Protobuf does not complain about any of that. It read the metadata submessage
as an InstrContext and produced a context whose program id was the ASCII of
"sol_compat_instr_execute_v1" and whose account list was empty, so the
precompile suites compared nothing. The previous commit worked around the outer
drift with a hand-rolled wrapper splitter; that shim is deleted here, since
correct descriptors make it unnecessary.
The field-10 FeatureSet is what the remaining failures were about. Reading the
old field number yielded nil, parseAndConfigureFeatures saw no active features,
and every feature-gated branch evaluated as though nothing had ever activated.
For the Ed25519 precompile that selected the pre-activation non-strict path,
which accepts signatures strict rejects.
ed25519 3316 / 3479 -> 3479 / 3479
secp256k1 2523 / 2628 -> 2628 / 2628
secp256r1 13185 / 13185 -> 13185 / 13185
So none of those 268 disagreements were predicate differences.
vm-programs is unchanged at 144/3398, verified by running the suite either side
of this commit. It already carried its own compatibility path for the feature
set, which is why it alone kept working; that path now goes through the
regenerated types like everything else.
Generated with protoc v3.21.12 and protoc-gen-go v1.34.2, matching the versions
recorded in the previous files so the diff shows schema movement rather than
codegen churn. elf.pb.go is untouched: protosol no longer ships elf.proto.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
backend=stdlib bypassed the library and called crypto/ed25519 directly. That is non-strict: it accepts small-order A and small-order R, which the strict predicate rejects and mainnet rejects. So an operator flag decided which signatures the node accepts. Two nodes on different settings would disagree on block validity, and it is the flag reached for under exactly the pressure that makes a silent fork worst. The package comment already said the predicate "is not optional and not configurable", and Configure sets DalekStrict before selecting anything. The bypass contradicted both. The library has its own crypto/ed25519-backed backend, and its rejection pre-pass runs before dispatch regardless of which backend is active. Selecting that instead keeps the diagnostic value -- it swaps out the r51 assembly, the comb tables and the batch kernels, which is where an implementation bug would realistically live -- while leaving acceptance untouched. The bypass flag is gone entirely; every verification now goes through the library. The test that pinned the divergence as deliberate is inverted to pin its absence. It has to re-execute itself in a child process: backend selection is one-shot per process by design, so the library can never hold key tables in two formats, which means any earlier Configure would make this test skip. A test that skips during "go test ./..." guards nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drives ProcessInstruction rather than the verifier underneath it, so offsets parsing, program-id routing and the predicate are covered together. Three cases that discriminate: a valid signature accepted, small-order A and small-order R rejected, and a tampered-but-well-formed signature failing on the equation rather than on a byte-level gate. The Firedancer corpus covers this path far more broadly at 3479 fixtures, but it needs a ~7 GB external checkout and skips without it. These run in an ordinary "go test ./..." and cost microseconds. Deliberately absent: a non-canonical-A case. verify_strict accepts a non-canonical A and hashes its original bytes, so pinning that looks like the obvious fourth case, but it cannot be built. A non-canonical encoding requires y < 19, and every curve point with such a y whose discrete log is computable is already small-order, so the small-order gate rejects it before canonicality is consulted. All 152 non-canonical-A fixtures in the Firedancer corpus expect an error for that reason, which also means they do not discriminate: they pass whether or not an implementation handles non-canonical A correctly. A test for that bullet would read as coverage and prove nothing, so the reasoning is recorded in a comment instead. Test files never enter the binary, so none of this costs the node anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The non-strict branch stays, so historical replay remains possible: blocks from before Ed25519PrecompileVerifyStrict activated were validated without it, and re-verifying them strictly would reject transactions the network accepted and produce a different bank hash. Keeping an untested branch is what made it dangerous, not the branch itself. One input, both feature states, opposite verdicts. The signature is a small-order construction that genuinely satisfies the stdlib equation -- A is the identity, so [s]B - [k]A collapses to [s]B, which is the R it carries. It is rejected only because strict verification refuses a small-order public key, which makes it the cleanest probe for which predicate ran. This also guards the failure mode that actually occurred. When the conformance fixtures parsed with an empty feature set, the gate read inactive, the non-strict branch ran, and 163 signatures were accepted that should have been rejected. A feature-plumbing bug silently became an acceptance change. With both directions pinned, that surfaces as a test failure rather than a quiet divergence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moves from 1625c183 to da0d045, about a hundred commits. The pin was an ancestor of main, so this is a fast-forward rather than a branch change. The reason to do it now is one commit in that range: 5e7ead9, "r51x5: stop dereferencing a skipped lane's nil table". A malformed lane in a warm group dereferenced a nil per-key table, which is reachable from network input. The pin predates that fix. Also picked up: 53868d9, which makes a nil Cache receiver fail closed uniformly instead of depending on the input, and 41720a2, which gates r51 availability on the kernels actually compiled in rather than on CPU features alone. The rest is the sigprep extraction, the fixed-base and x8 Niels work, and documentation. 118 files, none of which change the acceptance predicate. Verified after the bump: pkg/sigverify and the ed25519 precompile tests pass, and the Firedancer conformance suites are unchanged at ed25519 3479/3479, secp256k1 2628/2628, secp256r1 13185/13185. One pre-existing failure is untouched by this and should not be read as fallout from it: TestExecute_AddrLookupTable_Program_Test_Create_Lookup_Table_Not_Idempotent fails identically on the old pin, verified by reverting go.mod and rerunning it. It is in the address lookup table program and has no path to ed25519. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
resolveNativeProgramById had no case for AddressLookupTableAddr, so AddressLookupTableExecute was unreachable: every instruction for that program returned InstrErrUnsupportedProgramId and all 31 address-lookup unit tests failed. The implementation was complete and exported the whole time, which is why nothing caught it at build time. Adding the case makes those 31 pass. Config and Stake were missing the same way. Both have been migrated to BPF on mainnet, so their accounts are loader-owned and route through the loader; this resolver is only consulted when the program account's owner is NativeLoader, which is the pre-migration shape. Wiring them therefore changes nothing about current execution and restores historical replay. A resolver test now pins every native program to its case. This class of bug is silent by construction -- an implementation with no case still compiles, still exports, and simply never runs -- so the mapping needs an explicit assertion rather than relying on each program's own tests to notice. Two fixes from the same review: Conformance accounts get RentEpoch math.MaxUint64. protosol v5.4.0 removed rent_epoch from AcctState, and I had dropped the field with it, but a converted account's implied value is the maximum rather than zero. Leaving the Go zero modelled every account as rent-paying. The corpus revision is pinned at a87fc430 instead of tracking main. Both the schema and the per-suite counts move with the corpus, so following main makes a passing run unreproducible and makes a regression indistinguishable from an upstream edit. That revision is the one every count in these commits was measured against. Precompile suites unchanged: ed25519 3479/3479, secp256k1 2628/2628, secp256r1 13185/13185. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o be The doc comment said "Calling it twice returns an error rather than silently ignoring the second call". Nothing enforced that. A second call re-ran the whole path, and for BackendAuto it would return successfully while the library had already pinned a different backend, so the caller was told it got something it did not get. Three changes: Repeat calls are refused, with the already-resolved backend named in the error. Refusing a repeat of the *same* backend too is deliberate: treating it as a harmless no-op would make "Configure ran twice" invisible, and the second caller still has no way to learn its configuration was discarded. Validation now happens before anything is published. Cfg was assigned from the argument before the backend name was checked, so a rejected name still became visible to Backend() and to the startup log. Backend installation moved into a helper, leaving Configure as validate, install, publish. The r51 case grew a comment recording that the absence of a fallback is the contract, not an oversight. The tests run each case in a child process. Backend selection is one-shot by design, so an in-process table would let the first Configure win and turn every later case into a vacuous pass. That risk is not hypothetical: an earlier version of the stdlib test skipped whenever another test had configured first, which guarded nothing during "go test ./...". TestConfigureChildPlumbingActuallyRuns guards the harness itself. If the child marker or the -test.run pattern stopped matching, every subprocess test would spawn a child that ran nothing, exit zero, and report a pass. The r51 assertion holds on both kinds of machine: it accepts a resolution to r51 or a clear error, and rejects only the outcome that would be a bug, which is success while a different backend is actually active. Passes under -race with -count=3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things were unobservable at runtime: which backend actually resolved, whether the accelerated one ever fell back on an internal fault, and how wide the batches reaching it were. The third is the one that matters and the one a counter cannot answer. Cost per signature is a strong function of batch width -- eight signatures per AVX-512 group, so a stream of width-1 batches pays roughly 3.7x per signature what the same work costs at width 8. A million signatures arriving one at a time and the same million arriving in eights produce identical totals. Only the distribution separates them, so this is a histogram, with per-width buckets up to 8 where the group boundary sits and coarse buckets above it. Recorded in Batch.Verify rather than at each drain site, so a new caller cannot forget to instrument itself. It is an atomic add into a fixed array: no allocation, no lock, nothing that can fail on the verification path. pkg/sigverify keeps zero Mithril dependencies. It exposes Stats(); the reporter lives in cmd/mithril/node and owns the logging. Output goes to <run-dir>/sigverify.log via mlog.NamedFilef and nowhere else. Terminal output is unchanged -- the only edit to node.go is the two-line call that starts the reporter. Operator stderr already carries replay progress, and batch width is diagnostic rather than something to watch live. Lines are startup, per-interval and shutdown. Intervals report deltas, because counters are monotonic for the process lifetime and a cumulative-only view lets a long-healthy run hide a recent collapse in batch width. The shutdown line means a short run still leaves a record, which matters most when the run ended because of a verification problem. One exception to the file-only rule: a rise in InternalFaultFallbacks also warns to the operator log. It means the accelerated backend produced a result it could not trust and recomputed on the portable path. That should never happen, and it is a backend bug rather than an input condition, so silence is the wrong default. diffWidths matches buckets by upper bound rather than position, because Stats omits empty buckets and two snapshots need not share a shape. Getting that wrong would be quiet: the reporter would keep emitting plausible lines while misattributing counts across boundaries. Passes under -race. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
smcio
approved these changes
Jul 31, 2026
smcio
left a comment
smcio
left a comment
Collaborator
There was a problem hiding this comment.
Just made a few wording clarifications around what the stdlib backend does in Narya (still uses strictness checks).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Route all Ed25519 verification through narya, under the strict predicate
Mithril verified transaction signatures with Go's
crypto/ed25519, one signatureat a time, at four independent call sites. That was wrong on two axes at once.
It accepted signatures mainnet rejects. Solana verifies with
ed25519-dalek'sverify_strict, which is stdlibVerifyplus rejection ofsmall-order
Aand small-orderR.solana.PublicKey.Verifybottoms out atplain stdlib, so Mithril admitted a class of signature the network refuses — a
standing violation of the byte-identical-state invariant, and one an adversary
can construct cheaply.
It used the slowest possible shape. The library verifies eight signatures per
AVX-512 group, so per-signature cost is a strong function of how many arrive at
once: on Zen 5 a lone signature costs ~22.9 µs and a group of eight costs ~6.1 µs
each. Verifying one at a time gave up a ~3.7× factor before any kernel work.
This PR fixes the predicate everywhere, batches the two hot paths, and takes the
conformance harness from "never executed a fixture" to three suites at 100%.
Consensus-relevant behaviour changes
Reviewers should look hardest at these. Everything else is mechanical.
A/Rrejected--sigverify-backend stdlibExpect a real accept/reject delta on replay. Any transaction that only passed
under the non-strict predicate is now rejected. If a bankhash diverges after this
lands, that is the first hypothesis, not the last.
The backend flag was a consensus switch
backend=stdlibbypassed the library and calledcrypto/ed25519directly, whichis non-strict. An operator flag therefore decided which signatures the node
accepted — two nodes on different settings would disagree on block validity. It
is the flag you reach for under exactly the pressure that makes a silent fork
worst.
The package comment already said the predicate "is not optional and not
configurable", and
ConfiguresetDalekStrictbefore selecting anything. Thebypass contradicted both. It now selects narya's own
crypto/ed25519-backedarithmetic instead: the rejection pre-pass runs before backend dispatch, so the
diagnostic value survives — it still swaps out the r51 assembly, the comb tables
and the batch kernels — while acceptance is fixed.
The precompile ran a predicate the reference never used
pkg/sealevelbuilt its options as a struct literal setting three fields. Gozeroes what a literal omits, so
AllowNonCanonicalAsilently becamefalseandthe precompile rejected public keys the reference accepts — and voi's own
VerifyOptionsStdLibsets that fieldtrue, so the omission inverted thelibrary default rather than merely leaving it.
The non-strict branch was worse: voi's defaults are cofactored and reject
non-canonical
A, neither of which the reference does. That branch now callscrypto/ed25519, which is exactly what pre-activationdalekdid.Signatures are still verified one at a time here, deliberately. The reference
walks entries in order and returns the first error, so batching would let a
later entry's offset error preempt an earlier entry's signature error — and that
error code reaches the ledger.
What changed, by area
pkg/sigverify(new). Owns backend selection and the batch entry points.Configureis validated-then-published and refused on a second call.Batchisreusable worker-local scratch that allocates nothing after the first use.
Draintakes a fair share of the queue rather than the whole thing, so one worker cannot
strand the others.
Replay and TPU. Both worker loops moved from one-signature-at-a-time to
drain-and-batch. Failure attribution is preserved exactly: narya fills a per-item
verdict slice, so the arity-mismatch and invalid-signature panics still name the
precise signer via
diagContext(), and each job still completes its ownWaitGroup.Gossip, shreds, repair. Now on
narya.VerifyStrict.TPU versioned transactions.
txverify.MessageBytesapplies a version-bytefixup that solana-go's
MarshalV0gets wrong (it writes0x7f; the wireencoding is
0x80). Both TPU verifiers called rawMarshalBinaryand skippedit, so valid versioned transactions failed TPU sigverify. Consolidated onto
one helper.
Observability. The resolved backend,
InternalFaultFallbacks, and abatch-width histogram are recorded to
<run-dir>/sigverify.logeach run.Terminal output is unchanged. Width is a histogram rather than a counter because
a total cannot distinguish a million signatures arriving one at a time from the
same million arriving in eights, and those cost ~3.7x differently. Recorded
inside
Batch.Verifyso a new caller cannot forget to instrument itself.conformance/. See below.go.mod. narya-ed255191625c183→da0d045onmain(108 commits,fast-forward). The reason to move now is
5e7ead9, which fixes a nil per-keytable dereference in a warm group that is reachable from network input. Also
picks up a fail-closed nil
Cachereceiver and an r51 availability gate tied tocompiled kernels rather than CPU features alone.
The conformance harness was silently broken
This was not part of the plan and turned out to be the largest finding.
TestConformance_Precompile_Ed25519_Programreadtest-vectors/precompile/fixtures/ed25519, a layout that no longer existsupstream.
ReadDirfailed, the assertion failed, and no fixture was everexecuted. Firedancer now publishes every precompile fixture in one flat
directory keyed by the program id inside the fixture, so filtering has to parse
rather than glob.
Underneath that sat something quieter. The generated bindings described a schema
several versions old:
InstrFixturegainedmetadataat field 1, pushing input to 2 and output to 3InstrContextdroppedepoch_contextandslot_context; theFeatureSetnowsits directly at field 10
AcctStatedroppedrent_epochProtobuf does not complain about any of that. It read the metadata submessage as
an
InstrContextand produced a context whose program id was the ASCII of"sol_compat_instr_execute_v1"and whose account list was empty. Every testbuilt on those bindings compared nothing while reporting success.
The field-10 miss is what the remaining failures were about: reading the old
number yielded nil,
parseAndConfigureFeaturessaw no active features, and everyfeature-gated branch evaluated as though nothing had ever activated. For the
Ed25519 precompile that selected the pre-activation non-strict path.
Bindings regenerated from protosol v5.4.0 with protoc v3.21.12 and protoc-gen-go
v1.34.2, matching the versions recorded in the previous files so the diff shows
schema movement rather than codegen churn.
secp256r1 is the largest fixture set in the corpus and had no test at all.
The corpus revision is pinned at
a87fc430rather than trackingmain— boththe schema and the per-suite counts move with it, so following
mainmakes apassing run unreproducible and a regression indistinguishable from an upstream
edit.
make conformance-vectorsfetches it; suites skip when it is absent.Address lookup table, config and stake were undispatched
resolveNativeProgramByIdhad no case forAddressLookupTableAddr, soAddressLookupTableExecutewas unreachable and all 31 address-lookup unit testsfailed. The implementation was complete and exported the whole time, which is why
nothing caught it at build time.
Config and Stake were missing the same way. Both are BPF-migrated on mainnet, so
their accounts are loader-owned and route through the loader; this resolver is
only consulted when the owner is
NativeLoader, the pre-migration shape. Wiringthem changes nothing today and restores historical replay.
A resolver test now pins every native program to its case, because this bug class
is silent by construction: an implementation with no case still compiles, still
exports, and simply never runs.
Testing
Added tests avoid two failure modes that would make them decorative.
Nothing passes vacuously. Backend selection is one-shot per process by
design, so an in-process table over backends would let the first
Configurewinand turn every later case into a silent pass. Each case runs in a child process
instead.
TestConfigureChildPlumbingActuallyRunsguards the harness itself — ifthe child marker or
-test.runpattern stopped matching, every subprocess testwould spawn a child that ran nothing and reported PASS.
Assertions discriminate.
TestEd25519PrecompileFeatureGateSelectsThePredicatedrives one input through both feature states and requires opposite verdicts. A
test asserting "both reject" would pass under either branch and prove nothing.
The probe is a small-order construction that genuinely satisfies the stdlib
equation —
Ais the identity, so[s]B − [k]Acollapses to[s]B, exactly theRit carries — so it is rejected purely on the small-order gate, making it aclean signal for which predicate ran.
Coverage: predicate delta vs stdlib; batched-equals-per-item across mixed
validity and batch widths straddling 4 and 8; panic attribution inside a full
batch; every
Configurepath including unsupported CPU and repeat calls;end-to-end precompile through
ProcessInstruction; three conformance suites.pkg/sigverifypasses under-race.One case deliberately absent
verify_strictaccepts a non-canonicalAand hashes its original bytes, andpinning that looks like the obvious fourth precompile case. It cannot be
built. A non-canonical encoding requires
y < 19, and every curve point withsuch a
ywhose discrete log is computable is already small-order, so it isrejected before canonicality is consulted. All 152 non-canonical-
Afixtures inthe Firedancer corpus expect an error for that reason — which also means they do
not discriminate. A test for it would read as coverage and prove nothing, so the
reasoning is recorded in a comment instead.
Consequence worth stating plainly: the
AllowNonCanonicalAfix is correct byreading the reference, not by test. That is not fixable.
Not in this PR
warm group must be eight homogeneous warm lanes with no partial credit. Its
value rests on the fee-payer recurrence distribution, which nobody has
measured, and Alpenglow moving votes to BLS certificates plausibly removed the
dominant recurrence source. Cold-path batching is recurrence-independent and is
what this PR ships.
config and stake fixtures; native coverage lives in unit tests instead.
Known-red, and what this branch does to it
go test ./...is not green here, and was not green before. The failing packageset is identical on
origin/alpenglow-devand on this branch:conformance,pkg/genesis,pkg/lightbringer,pkg/sealevel. This branch adds no new one.At test granularity it is better than that, and the detail is worth stating
because a naive diff of failing tests looks alarming:
Both sets are internal unit tests in
pkg/sealevel(
address_lookup_table_test.go,sealevel_bpf_loader_test.go). Neither touchesthe Firedancer corpus, so neither is conformance breakage — worth saying because
"19 BPF-loader failures" reads that way otherwise.
The 19 are not regressions. On the base, the address-lookup test did not merely
fail — it panicked, which aborts the whole test binary, so every later test
in
pkg/sealevelnever ran. Verified by runninggo test ./pkg/sealevel/ -run TestExecute_Tx_BpfLoaderatorigin/alpenglow-dev, where all 19 fail identically in isolation. Unblockingthe panic simply let the rest of the package execute for the first time.
That is worth someone's attention on its own: a panicking test had been masking
19 real failures for as long as the dispatch case was missing. It is out of
scope here and wants its own change.
The conformance suite moves the other way. Three precompile suites went from
never executing a fixture to 100%, and vm-programs is unchanged at 144/3398,
verified either side of the protobuf regeneration.
conformancestays red onlybecause of that pre-existing vm-programs count.
Rollback
--sigverify-backend genericselects the portable pure-Go path;stdlibselectscrypto/ed25519-backed arithmetic. Neither changes the predicate. There is
deliberately no flag that restores the old non-strict behaviour, because that
behaviour was the bug.