feat(criteria)!: compose gate rules with all/any, and report every failure - #35
feat(criteria)!: compose gate rules with all/any, and report every failure#35Rinse12 wants to merge 2 commits into
Conversation
…ilure
BREAKING CHANGE: `criteria.rule` becomes `criteria.gate`, a boolean tree
over rule references. This re-CIDs every criteria document and so
re-topics every contest; the identity migration is
`gate: { rule: <the old rule ref> }`. `Contest.checkEligibility` returns
a new shape, and `BundleVerifier.checkGate` becomes `checkGates`.
A client could not answer "which requirements is this wallet missing?"
because a contest had exactly one gate rule, so listing failures was
listing one thing. Expressing "the Pass, or a moderator, and not banned"
needed a bespoke rule per combination — code every participant must
implement to say what a document could say.
The gate is now `{ rule } | { all: [...] } | { any: [...] }`. A rule
still answers one question about one wallet and never sees the gate it
sits in; `src/rules/gate.ts` folds the answers, reading the tree's
structure and the results' discriminants but never which rule produced
them. Canonicity constrains the shape, because the topic is the CID of
these bytes and two spellings of one meaning is a silent topic fork: a
branch takes >= 2 children, the leaf is wrapped (a rule ref is loose, so
an option named `all`/`any` would make a bare leaf ambiguous), depth and
leaf count are capped, and every leaf must read the contest's one clock
chain (`GateChainMismatchError`).
Three things fold, and only the first is obvious:
- the score: min across an `all` (the binding constraint), max across a
satisfied `any` (the best qualification);
- the blame set: the failures that EXPLAIN a refusal, which is NOT every
failed leaf. One inside a satisfied `any` cost the wallet nothing, and
telling it to acquire an asset it does not need is worse than silence;
- `penalize`: an `all` is attributable if ANY failing child is (that
child alone closes the gate everywhere), an `any` only if EVERY child
is (one unprovable failure means a peer with a fresher view may see a
wallet this gate admits, and reject-scoring it punishes honest
relaying). The inline gate short-circuits and can only under-report
attributability, which is the fail-safe direction.
`checkEligibility` now returns `{ eligible, score | error, checks,
failures, gate }` — every rule in document order, the blame set, and the
tree with per-node verdicts so a client renders the real requirement.
Rows key by `ruleId` (the leaf's canonical hash, which is also its cache
namespace), because one gate may name a rule twice on different options.
The forward gate evaluates lazily; the background verifier scores every
leaf because its batching axis is the rule — one `evaluateMany` per leaf
per round — so collecting all is cheaper there and yields the complete
blame set for free.
Benchmark: re-run against the WAN host with a same-session control on
master. `verify+merge` is flat (0.31/0.32/0.33/0.47/1.96 on master vs
0.32/0.32/0.32/0.47/1.97 here) and gate-RPC counts are identical; the
end-to-end spread between the two runs sits entirely in `connect` and
`fetch`, which drifted on master too. RESULTS.md is left alone rather
than re-baselined on today's link conditions.
|
Warning Review limit reached
Next review available in: 20 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (24)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The ≥2-children rule closed only one of the ways a gate tree can mean
what a shorter tree means while encoding to different bytes. Two more
were open, and both derive a DIFFERENT topic for the same contest:
{ all: [X, X] } ≡ { rule: X }
{ all: [{ all: [A, B] }, C] } ≡ { all: [A, B, C] }
The second is redundant in every fold, not just in admission: min and
`some` are associative, so nesting a branch inside a branch of its own
kind changes no score, no blame set and no penalize verdict.
Both are now refused, duplicates compared by canonical bytes so two
leaves of one rule type on different options stay distinct requirements.
Normalizing them instead — flatten, dedupe, sort at parse time — was
rejected: validation here is a check and never a transform, or the
document a client ships stops being the document whose bytes derive the
topic, and out-of-band verification of a published manifest stops meaning
anything. Child ORDER therefore stays significant, which is defensible on
its own terms: it is the order the lazy forward gate evaluates in, so it
decides which rule's chain read is paid first.
Also closes the coverage gaps this audit surfaced:
- composite gates through the INLINE forward gate (the fold's disposition
and blame set, the folded score under all/any, and that it stops at the
deciding leaf so a composite costs only the rules it needed);
- per-leaf memo isolation: two same-type leaves on different options must
not read each other's answers;
- `VerifyFail.failures` reaching the publisher on VoteEvictedError;
- checkEligibility's tree projection over a NESTED gate;
- `evaluateGate` asking each leaf at most once, the contract its callers
memoize on.
|
Audited the new surface for gaps. One turned out to be a defect, not just missing coverage — pushed in 4456304. The canonicity rule was incompleteThe ≥2-children rule closed one way a gate tree can mean what a shorter tree means while encoding to different bytes. Two more were open, and each derives a different topic for the same contest: I verified the first against Both are now rejected, duplicates compared by canonical bytes so two leaves of one rule type on different options stay distinct requirements. Why reject rather than normalize. Flattening, deduping and sorting children at parse time would make any two equivalent trees encode identically — but validation here is deliberately a check and never a transform (the same property Coverage gaps closed
472 unit tests, 10 integration, typechecks clean. Docs updated in all four places. |
|
@CodeRabbit review |
|
1 participant
Builds on the question that started this: a UI wants to list which gate rules a wallet fails. That is only meaningful once a contest can have more than one, so this ships the composition and the reporting together.
The shape
A rule still answers one question about one wallet and never sees the gate it sits in. Composition is the document's — which matters because a rule is code every participant must implement, while a document is bytes they merely share.
The leaf is wrapped rather than bare because
RuleRefSchemais loose: a custom rule may carry an option namedallorany, and a bare leaf would be structurally ambiguous with a branch exactly when someone writes one. The rest of the shape rules are canonicity, not style — the topic is the CID of these bytes, so two spellings of one meaning is a silent topic fork: a branch needs ≥ 2 children ({ all: [X] }must not coexist withX), depth ≤ 4 and leaves ≤ 8 (attacker-supplied input every peer parses), and every leaf must read the contest's one clock chain (GateChainMismatchError— bucket boundaries, a ballot'sblockNumberand the sample block each rule is handed are all numbers on that chain).What folds, and the part that is not obvious
src/rules/gate.tsreads the tree's structure and the results' discriminants, never which rule produced them.allanypenalizeThe last row is the load-bearing one and it is not a simple OR. An
allfails as soon as one child does, so one attributable failure makes the refusal attributable — that child alone closes the gate identically on every honest verifier. Ananyfails only when every alternative does, so it is attributable only if every one is: a single unprovable failure means a peer with a fresher chain view may be looking at a wallet this gate would admit, and reject-scoring it would punish honest relaying.The blame set is the other one. It is not every failed leaf: a leaf that failed inside a satisfied
anycost the wallet nothing, and telling someone to go and acquire an asset they do not need is worse than saying nothing.The inline forward gate evaluates lazily (one wallet, every leaf a chain read) and so can only under-report attributability — the fail-safe direction, costing at worst a spammer we merely
ignoreinstead of reject-scoring an honest relayer on evidence never gathered. The background verifier does the opposite and scores every leaf, because its batching axis is the rule: oneevaluateManyper leaf covers a whole round, so collecting all is cheaper there than short-circuiting, and the complete blame set comes free.checkEligibilitychecks{ ruleId, type, satisfied, score, error? }failuresgatecriteria.gate, each node carryingsatisfiederrorfailuresjoined, for a caller that only wants a stringKey rows by
ruleId— the leaf's canonical hash, which is also its cache namespace — never bytype, since one gate may name a rule twice on different options.Per-leaf memos follow the same identity, so two leaves of one rule type on different options can never read each other's answers, and two contests sharing a gate rule still share every read.
Tests
src/rules/gate.test.ts(18) — the fold, offline: leaf numbering, short-circuit vs collect-all, the score fold, the blame set (including the satisfied-anycase), and each branch of thepenalizecomposition plus the under-report property.src/schema/criteria.test.ts(7) — every canonicity rule, and thatallvsanyover the same rules forks the topic.src/verify/background.test.ts— composite gates through the real background verifier: an attributable failure inside anallevicts terminal+cached with only the explaining rule named; ananyof one attributable and one not is heldignore-class and uncached.src/client/voter.test.ts—checkEligibilityover composites: the full failure list, the partial one, the satisfied-anycase that blames nothing, and distinctruleIds for same-type leaves.npm test465 passing,npm run test:integration10 passing, all three typechecks clean.Benchmark
Re-run against the WAN host with a same-session control on master, since the committed baseline was measured on another day.
Flat, with identical RPC counts — which is the phase this change can touch. The end-to-end spread between the two runs sits entirely in
connectandfetch, and master today is likewise slower than the committed table (connect1.67s vs 1.56s), so it is the link, not the code.benchmark/RESULTS.mdis therefore left alone rather than re-baselined on today's conditions.Docs
README (the
gateshape, the newcheckEligibilitycontract, the compositepenalizefold), DESIGN.md (a new "The gate is a tree, and the pipeline folds it", plus therule→gatecutover note), AGENTS.md, and ROADMAP.md — where "Gate combining" was a planned flat AND-array and is now marked done as a tree, with a note on whyanyearned its place.