From bb0079fe00ddc4c24f4084a7d71e4d0bb20c9769 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Fri, 28 Aug 2026 06:00:53 -0700 Subject: [PATCH] Update agent file. --- AGENTS.md | 92 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 27 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 23035ca..cbda5c7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,10 +23,30 @@ trailing argument. `verify_blocks` is the Nano account-chain case — many block hashes against one public key. It is correct across the full range and has vector coverage. The -public-key decompression is hoisted into `crypto_verify_pubkey` and run once per -batch, which saves `(1 - 1/n) * k` where `k` is the key work's share of one -verify: **measured k ≈ 8.5%** (byte inputs) or **≈9.7%** (hex). At the cap of 32 -that realises **~8.1% / ~9.6%**, i.e. 97% of everything available. +public-key decompression is hoisted into `crypto_verify_decodepubkey` and run +once per batch, which saves `(1 - 1/n) * k` where `k` is the key work's share of +one verification: **measured k = 8.84%**, realising **8.51%** at the cap of 32. + +⚠️ **`verify` and `verify_blocks` have different acceptance sets, on purpose.** + +| | `verify()` | `verify_blocks()` | +| --- | --- | --- | +| target | strictest available | ed25519-donna, i.e. what nano_node accepts | +| scalar `S` | `S < L` | `S < 2^253` | +| public key | canonical, non-small-order | decodable only | +| nonce `R` | decoded, rejected if small order | never decoded | +| equation | cofactored | cofactorless byte-compare | +| small-order key (`PROBLEM_VECTOR`) | rejects | **accepts** | +| `R` paired with `S+L` | rejects | **accepts** | + +They will disagree about real cemented Nano blocks. That is the design, not a +bug: `verify_blocks` answers *"does the ledger accept this"*, `verify` answers +*"is this a sound Ed25519 signature"*. Do not "fix" one to match the other. + +`crypto_verify_relaxed` **requires** `crypto_verify_decodepubkey` to have run — +it reuses the module-level `A` that call leaves behind, which is what makes the +batch hoist possible. Call it cold and you verify against whichever key ran last, +with no error. `crypto_verify_strict` does its own key handling inline. **Do not raise the cap to chase throughput.** `(1 - 1/n)` is 96.9% at n=32 and 99.7% at n=341 — the remaining 2.8% of an 8.5% prize is under a quarter of a @@ -96,6 +116,13 @@ byte per block, *and* at least 64 bytes for a signature. Only the first was ever written down, so cutting the batch cap to 32 made `sign` overrun it. If you ever merge them again, both constraints have to be stated together. +`INPUT_MSG` still has this problem. It holds **either** a message for +`sign`/`verify` (up to `MAX_MESSAGE_BYTELENGTH`) **or** a packed batch +(`MAX_VERIFY_BLOCKS * SIGNEDBLOCK_BYTELENGTH` = 3,072 B). Only the first is +named. `1 << 15` satisfies both with room to spare, but it was briefly `1 << 10` +and a full batch overran it. **Before shrinking `MAX_MESSAGE_BYTELENGTH` or +raising `MAX_VERIFY_BLOCKS`, check both roles still fit.** + Records in `INPUT_MSG` are **`signature` then `hash`** (64 B + 32 B), libsodium combined-mode order. Nothing validates this; both sides just have to agree. @@ -167,6 +194,15 @@ col)` against static string data — the wat shows `call $abort` and store, and makes AssemblyScript pass a null message pointer, so the error text is lost before it reaches the host. +**A comparison function needs a test that makes things unequal.** `equalbytes` +shipped comparing 12 of 32 bytes — the 12 being the FieldElement limb count, +copied into a byte count. It reduced a 256-bit signature check to 96 bits, and +the full suite passed the whole time, because valid signatures pass under the bug +and random ones fail. Any function whose job is *"return false unless equal"* +needs a test that flips **every** byte position, not one that checks the equal +case. The same applies to a result buffer: assert the failure path, not just the +success path. + **Raw memory intrinsics are unguarded in every build.** `memory.copy`, `memory.fill` and friends are not covered by `noAssert` or `uncheckedBehavior` — turning the safety net back on does **not** make them bounds-checked. A wrong @@ -190,17 +226,10 @@ Setting the default in the failure branch instead is the bug this shape avoids: a result buffer is only fail-closed if the default is written before anything can skip past it. -**`crypto_verify_relaxed` requires a prepared key.** It does not validate the -public key; it reuses the module-level `A` that `crypto_verify_pubkey` left -behind, which is what makes the batch hoist possible. Call `crypto_verify_pubkey` -first, or you will verify a signature against whichever key ran last, with no -error. `crypto_verify_strict` does both itself. Neither actually relaxes any -rule despite the name — see the `PROBLEM_VECTOR` note under Testing. - **Counts, lengths, and end offsets are not interchangeable, and confusing them -is silent here.** This project has produced the same bug three separate times: a -byte length passed where an element count was wanted, and a length passed where -an end offset was wanted. With bounds checks compiled out on the wasm side and +is silent here.** This project has produced the same bug four separate times: a +byte length passed where an element count was wanted, a length passed where an +end offset was wanted, and a limb count passed where a byte count was wanted. With bounds checks compiled out on the wasm side and typed-array writes silently discarded on the JS side, every instance compiled, ran, and returned plausible answers. When touching either side of the boundary, name the quantity in the variable (`…_count`, `…_bytes`, `…_end`) and check each @@ -242,6 +271,13 @@ a conclusion: it is most of the difference between the ≈10% and 13–16% figur that both circulate for `verify_blocks`. Build inputs the way a caller would, and keep the same distinctness on both sides. +**Benchmark the same code path on both sides.** `verify_blocks` was compared +against `verify()` for as long as both ran the same algorithm; they no longer do, +so that comparison now measures the hoist *plus* the algorithm difference. The +same-algorithm baseline is `verify_blocks(1)` repeated n times. Keep an n=1 row +in every sweep — with nothing to amortise it must read ~0% (it reads 0.04%), and +it is the row that tells you whether you measured what you named. + Useful scale anchors: a wasm call boundary is ~2 ns, a 32 KiB `memory.fill` is ~0.1 µs, one `verify` is ~73 µs with `Uint8Array` inputs and ~80 µs with hex strings, one `sign` is ~49 µs, and the hoisted per-key work is ~6.2 µs. Buffer scrubbing and boundary crossing never @@ -249,8 +285,9 @@ show up on the clock; only the point arithmetic does. ## Testing -`node ./test/node.mjs` after a build. Current state is **6176 passing, 1 -failing**, and that one failure is expected. +`node ./test/node.mjs` after a build. Current state is **6178 passing, 0 +failing**. There is no longer an expected failure — if anything fails, it is a +regression. `verify_blocks` now has vector coverage — a single block, a full 32-block batch, both range errors, and the three block-shape `TypeError`s. Coverage still @@ -260,17 +297,18 @@ whole range rather than at one size, and always corrupt at least one signature in the batch — the bug this feature has actually shipped is reporting invalid signatures as valid, which an all-valid batch cannot catch. -The failing case is `PROBLEM_VECTOR`: a live cemented Nano block from a -small-order account (`nano_11a11…`, public key `0100…00`) that network -consensus accepted but strict Ed25519 rejects. The test asserts `true` to match -the ledger; `verify()` returns `false` to match the spec. Do not resolve this by -weakening the small-order check in strict verification. The intended shape is a -relaxed variant for Nano blocks alongside a strict one for general use. -`crypto_verify_relaxed` and `crypto_verify_strict` are no longer identical — -relaxed now means "the key was already validated" — but **neither relaxes the -small-order rule**, so the ledger-versus-spec decision is still open. Both paths -reach it through the single `crypto_verify_pubkey`, so whichever way it goes, -it changes in one place. +`PROBLEM_VECTOR` is a live cemented Nano block from a small-order account +(`nano_11a11…`, public key `0100…00`) that network consensus accepted and strict +Ed25519 rejects. It is now asserted **twice**, once per path: `verify()` must +reject it, `verify_blocks()` must accept it. Both assertions pass. Do not +"simplify" this into one expectation — it is the regression test for the whole +strict/permissive split. + +Coverage still runs thin in the middle: `verify_blocks` bugs have twice been +invisible below a boundary (64, then 256) and appeared only past it, so exercise +changes across the whole range, and always corrupt at least one signature in the +batch — the bug this feature has actually shipped is reporting invalid signatures +as valid, which an all-valid batch cannot catch. ## Style -- 2.52.0