feat(w2a): finalized update convergence — canonical contracts + a genuinely per-chain finalized-read lane - #2051
Conversation
W2 chunk 10.1. Adds the package-neutral half of W2: scope identity, exact finalized-event identity, the ordered raw-log commitment, page-assurance proofs, the two-cursor model, the scoped KA candidate parser, and the closed outcome vocabularies. No chain client, no store, no I/O. Two rules are load-bearing and are pinned by tests that can fail: - `scopeId` is derived from the four IDENTITY fields only, never from `deploymentBlock`. Including the anchor would make an anchor correction mint a new scope instead of triggering revision reset and replay inside the old one, orphaning every persisted cursor. - The candidate parser refuses to guess. `did:dkg:<chain>/<KA-storage>/7` round-trips under both the legacy and rootless forms, so it returns both and lets the store resolve which is live against chain provenance. Choosing either here would attribute a graph write to the wrong KA. `buildScopedKnowledgeAssetUal` restates the rule owned by `buildReconciledKnowledgeAssetUal`, because core cannot import agent. The new cross-package parity suite is the external anchor for that restatement; it is the only place both implementations are visible at once. Verification: 39 core tests + 10 parity tests green. Reachability sentinels proved both suites execute the intended module (core source, and the rebuilt core dist that the agent suite loads). 8 behavioural mutants applied serially with hash-verified restore; all 8 killed. One earlier mutant survived and exposed two vacuous boundary-collision assertions of mine — both compared inputs of differing total content, which any encoding distinguishes. They now compare tuples with identical concatenations and differing field boundaries, and the corrected mutant (lengthPrefixed collapsed to a plain join) kills exactly those two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…ly per chain W2 chunk 10.2, admission half. This fixes a live defect it uncovered. `CURRENT_FINALIZED_EVM_SNAPSHOT_MAX_CONCURRENT_PER_CHAIN_V1 = 1` has always documented "one heavyweight pinned scan per chain", but it was enforced by a gate constructed INSIDE `createStrictFinalizedEndpointRunnerV1` — one gate per transport instance. RFC64 builds its snapshot scope inside the precommit handler (`finalized-vm-agent-precommit-v1.ts:94`), so it built a fresh gate per invocation and contended with nothing: two concurrent precommits on one chain both admitted, each running a full pinned multi-batch scan. Permit state moves to module scope in `finalized-chain-read-admission.ts`, keyed on the canonical chain id ALONE. `owner` is carried for attribution only — putting it in the key would produce one lane per owner, which is the bug being removed. It is REQUIRED on the snapshot config, not defaulted: an ownerless path cannot be attributed and would quietly recreate the unattributed lane. RFC64 now passes `owner: 'rfc64'`; W2's page scanner will pass `w2-page` and share the same permit. The one-shot read primitive deliberately keeps its own gate: its limit is 4, and folding it into the snapshot's single lane would throttle an unrelated path. Verification: 42 chain tests green. The decisive check is negative — routing admission back to the per-instance gate fails EXACTLY ONE test, the new "contends across INDEPENDENTLY constructed scopes" case. The two pre-existing saturation tests stay green under that mutant, because both reuse a single handle and so could never discriminate per-instance from per-chain. That is why the defect survived. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…n does Two corrections found by re-reading the shipped contracts rather than my own tests. **1. The scope's chain id is namespaced, not a bare decimal.** `ChainAdapter.chainId` is `base:84532` / `otp:20430` / `evm:31337`, and its own doc comment says it is "not directly parseable with `BigInt()`" — `getEvmChainId()` is the numeric one. UALs are built from the namespaced form (`chain-adapter.ts:960,1534-1540`). The scope validator required a canonical decimal, so `canonicalScopedKaCandidatesFromVerifiedUal` would have thrown `noncanonical-scalar` for every real mainnet and testnet UAL. Every existing test used `84532`, so the suite was green against code that could not run anywhere real. `canonicalUalChainId` now accepts an optional lowercase namespace plus a canonical decimal tail, still rejecting `base:084532` so one chain cannot have two spellings. Note this makes `base:84532` and `84532` DIFFERENT scope ids, which is correct: they are different chain identifiers, not two ways of writing one. Proven negative: restoring the bare-decimal validator fails exactly the four new namespaced tests and leaves the other 41 green — which is why the gap survived the first round. **2. Owner vocabulary aligned to plan §5.2/§8.3.** `current-finalized` was invented; the plan specifies `foreground | rfc64 | w2-page | w2-target`, and §8.3's `owner` metric dimension uses exactly those values. Renamed before anything depends on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
| throw new TypeError('Strict finalized snapshot RPC config must be a plain data record'); | ||
| } | ||
| const { owner, ...rest } = input; | ||
| if (!FINALIZED_CHAIN_READ_OWNERS.includes(owner as FinalizedChainReadOwnerV1)) { |
There was a problem hiding this comment.
🔴 Bug: Snapshot factory now rejects previously valid public configs
What's wrong
This is an API/contract break for callers of the published chain package. The monorepo call sites were updated, but external users that construct finalized snapshot scopes without the new owner field will fail at compile time and at runtime.
Example
Existing consumer code such as createStrictCurrentFinalizedEvmSnapshotScopeV1({ chainId: '84532', endpoints: ['https://rpc.example'] }) now reaches FINALIZED_CHAIN_READ_OWNERS.includes(undefined) and throws Strict finalized snapshot RPC config requires a known owner...; previously it constructed a snapshot scope.
Suggested direction
Keep the old public call shape working while still assigning an owner internally for the shared admission registry.
For Agents
Look at packages/chain/src/strict-current-finalized-evm-snapshot-factory.ts and packages/chain/src/strict-current-finalized-evm-config.ts. Preserve the existing public config shape, likely by defaulting omitted snapshot owners to a public owner such as foreground or by adding an owner-specific factory while keeping the old one source- and runtime-compatible. Prove old { chainId, endpoints } callers still work and new RFC64/W2 callers still contend in the shared registry.
There was a problem hiding this comment.
Confirmed and fixed in 977e02a4. You're right that this is a published-package break — @origintrail-official/dkg-chain is on npm at 10.0.11 and not private, so a required field breaks every external { chainId, endpoints } caller at compile time and at runtime.
owner is now optional and defaults to foreground. That was the right call for a reason worth stating: foreground is a real owner label, not an "unknown" bucket — external callers genuinely are foreground — so nothing about attribution is lost. I had over-applied a no-compat-shims habit to a case where the default is semantically correct.
The important part is that the default must not become an escape hatch, so there is now a test for exactly that: does NOT let the default owner bypass the shared lane — an ownerless scope takes the same process-wide permit and refuses a concurrent w2-page scope with held by foreground.
Proven negative: reverting to const resolved = owner; fails two tests by name — keeps the pre-existing ownerless public call shape working and does NOT let the default owner bypass the shared lane. A second mutant that routes only the default owner to a private gate fails the latter alone.
| parts.push( | ||
| canonicalEvmAddress(log.address, 'log.address'), | ||
| String(log.topics.length), | ||
| ...log.topics.map((topic, index) => canonicalDigest32(topic, `log.topics[${index}]`)), |
There was a problem hiding this comment.
🔴 Bug: Raw-log commitment does not bound topics per log
What's wrong
The new commitment helper is meant to canonicalize untrusted page data, but it only limits the number of logs and scalar sizes. A single malformed log can still drive unbounded memory and CPU through the topics array.
Example
A faulty or hostile RPC page with one log and topics: Array.from({ length: 1_000_000 }, () => HASH_A) passes the page-length guard because there is only one log, then allocates and hashes a million topic entries. A valid EVM log should have at most 4 topics and should be rejected before building the commitment.
Suggested direction
Reject malformed logs with too many topics before expanding them into the commitment input.
Confidence note
I did not find current non-test call sites for this new W2 helper in the diff, so the risk depends on the planned scanner feeding raw RPC logs into this exported API.
For Agents
In packages/core/src/vm-update-convergence.ts, validate each raw log's topics before mapping: require an array, require a dense data-only sequence, and cap it at the EVM topic bound of 4. Preserve the commitment bytes for valid logs. Add a failing case with one log carrying more than 4 topics or a very large topic array.
There was a problem hiding this comment.
Confirmed and fixed in 977e02a4. The bound is real: MAX_UPDATE_PAGE_EVENTS constrains the number of logs, so one log with a million topics passes it and then allocates and hashes a million entries — from an untrusted RPC response.
Now capped at MAX_LOG_TOPICS = 4 (LOG0..LOG4), and the check runs before the topics.map, which is load-bearing: after the map, the allocation this guard exists to prevent has already happened. A non-array topics is also rejected as page-malformed rather than throwing a raw TypeError out of the canonicalizer.
On your confidence note — correct, there are no non-test callers yet. This helper is what the §10.3 page transport will feed raw RPC logs into, which is exactly why it fails closed now rather than later.
Two regression mutants, both killed: raising the bound to 100M, and downgrading the array check so it no longer reports page-malformed.
There was a problem hiding this comment.
🔴 Bug: Page proof origin lists are unbounded before normalization
What's wrong
The proof contract says a page can carry at most two origins, but the implementation normalizes the whole array before enforcing that bound. A malformed or hostile proof can force unnecessary URL parsing, allocation, and Set construction, which is exactly the kind of untrusted-input resource exposure this module otherwise guards against.
Example
canonicalPageProof({ assurance: 'unattested', normalizedOrigins: Array.from({ length: 1_000_000 }, (_, i) => https://${i}.example.com), ... }) will allocate and parse every URL before rejecting that a proof carries more than two origins. Expected behavior is to reject the length before mapping.
Suggested direction
Enforce the maximum origin count before normalizing or allocating derived arrays.
For Agents
In canonicalPageProof, check input.normalizedOrigins.length > 2 immediately after confirming it is a non-empty array, before map() or Set construction. Preserve the existing dual-origin rule that corroborated proofs require exactly two distinct normalized origins, and add a test with a very large origins array proving it fails before normalization.
| const from = canonicalBlockRef(input.from, 'proof.from'); | ||
| const through = canonicalBlockRef(input.through, 'proof.through'); | ||
| const finalizedAnchor = canonicalBlockRef(input.finalizedAnchor, 'proof.finalizedAnchor'); | ||
| if (through.blockNumber < from.blockNumber) { |
There was a problem hiding this comment.
🔴 Bug: Page proof accepts impossible same-height block hashes
What's wrong
A proof can be marked corroborated and later used to advance coverage even when its own block references describe an impossible chain state. This weakens the integrity guarantee for empty or sparse pages where log positions may not otherwise expose the inconsistency.
Example
canonicalPageProof({ from: { blockNumber: 100, blockHash: A }, through: { blockNumber: 100, blockHash: B }, finalizedAnchor: { blockNumber: 100, blockHash: C }, assurance: 'dual-origin-corroborated', normalizedOrigins: ['https://a.example','https://b.example'], orderedLogCommitment }) is accepted even though block 100 cannot simultaneously have hashes A, B, and C. A page over one block should require the same hash for all equal block numbers.
Suggested direction
Add same-height hash consistency checks between from, through, and finalizedAnchor.
Confidence note
This finding assumes from, through, and finalizedAnchor are intended to be internally consistent block references, which matches the surrounding proof and coverage comments.
For Agents
In canonicalPageProof, after canonicalizing refs, reject any pair of refs with the same blockNumber but different blockHash before returning the frozen proof. Preserve existing range checks. Add cases for from == through and through == finalizedAnchor with mismatched hashes.
There was a problem hiding this comment.
Confirmed and fixed in 977e02a4. Your reading of the intent is right: from, through and finalizedAnchor are meant to be internally consistent references to one chain.
canonicalPageProof now rejects any pair with the same blockNumber and a different blockHash, across all three pairs. The case that makes it matter is the one you identified — a single-block page has from === through, and a page reaching the anchor has through === finalizedAnchor, so empty and sparse pages are precisely where no log position would ever expose the contradiction.
Tests cover from == through mismatched, through == finalizedAnchor mismatched, the three-way case, and the consistent single-block page still being accepted. Disabling the check fails them.
There was a problem hiding this comment.
🔴 Bug: Non-RPC URL schemes can satisfy dual-origin page assurance
What's wrong
The page-proof validator treats distinct URL origins as corroborating evidence but does not restrict them to RPC endpoint schemes. That allows a malformed or forged proof to claim authoritative two-origin corroboration using schemes such as file:, data:, or ftp:, which do not represent independent finalized RPC providers.
Example
A proof with assurance: 'dual-origin-corroborated' and normalizedOrigins: ['file:///tmp/a', 'data:text/plain,x'] normalizes to two distinct strings (file:// and data://). With valid block refs and commitment fields, isAuthoritativePage() would return true even though neither origin can be a configured RPC endpoint.
Suggested direction
Restrict proof origins to the same HTTP(S), host-bearing endpoint domain accepted by the RPC configuration, or require callers to pass an already validated normalized HTTP(S) origin type.
Confidence note
This matters if W2 page proofs can enter this exported validator from persisted or cross-layer data rather than being constructed only from already validated HTTP(S) endpoint configuration.
For Agents
In packages/core/src/vm-update-convergence.ts, update normalizeEndpointOrigin to reject non-HTTP(S) schemes and empty hostnames before returning the origin. Preserve normalization of valid HTTP(S) URLs and add tests for file:, data:, and ftp: rejection.
| /** | ||
| * W2 — finalized update convergence: canonical value contracts. | ||
| * | ||
| * This module owns the package-neutral half of W2: scope identity, exact |
There was a problem hiding this comment.
🟡 Issue: Split the W2 convergence contracts before this becomes a grab-bag module
What's wrong
The new module starts by centralizing too many independent concepts in one place. That makes the package-neutral layer easy to import but hard to evolve: unrelated changes will churn the same file, readers must load several domains at once, and the broad barrel export makes internal helpers feel like stable public API.
Example
A change to only UAL candidate parsing currently lives in the same public module as endpoint-origin proof validation, cursor movement rules, error vocabularies, and the store contract. There is no runtime example because this is a structural maintainability issue, not a behavior defect.
Suggested direction
Preserve behavior but decompose by ownership boundary. The current section comments are already the module map; turn those sections into files so future W2 work extends the right concept instead of growing one central contract file.
For Agents
Split packages/core/src/vm-update-convergence.ts into focused modules such as errors/vocabularies, scalars, scope, events/log-commitment, page-proof, cursor, UAL candidates, and store contracts. Keep a barrel export if consumers need the same public surface, and run the existing convergence and parity tests unchanged.
There was a problem hiding this comment.
Deferring this one, with reasoning rather than a dismissal — and I'd genuinely welcome pushback if you disagree.
The concern is fair: the module is broad, and the barrel export does make internals look like stable API. Two things argue for waiting:
- Plan §10.1 specifies this single file, and §10.2–§10.6 have not landed. The section comments are the module map, as you say — but they are my predicted seams. Once the chain transport (§10.3), the SQLite store (§10.4) and the runtime (§10.6) are consuming these contracts, the real seams will be visible, and some sections will likely merge or move rather than split 1:1.
- Splitting now means churning the same files twice, during a sequence of PRs where the reviewable unit is already large.
Concretely I'd propose doing it at the end of W2a, when every consumer exists, and it can be a pure no-behaviour-change move validated by the unchanged convergence and parity suites. Happy to do it now instead if you think the ordering risk runs the other way.
One thing I took from this immediately: the public-surface concern is real, so that follow-up should also decide what stays exported rather than re-exporting everything through a barrel by default.
There was a problem hiding this comment.
🟡 Issue: Decompose the new W2 convergence god module before it hardens
What's wrong
This PR introduces a broad coordination module that owns several independent concepts. The section markers help, but they do not reduce coupling: unrelated contracts now share one file, one export surface, and one growth path. That makes the next W2 increment likely to push this file past 1k lines and makes local reasoning harder than necessary.
Example
A future change to only page-proof normalization has to scan through UAL parsing, store interfaces, hash-domain helpers, cursor rules, event reducers, and scalar adapters in the same module. The file is already close to the 1k-line decomposition threshold before the rest of W2 lands.
Suggested direction
Keep the package-neutral boundary, but divide it by concept. The code already has section markers that map cleanly to modules, so this can be a mechanical split without changing behavior.
For Agents
Split this into cohesive core modules such as vm-update-scalars, vm-update-scope, vm-update-events, vm-update-page-proof, vm-update-cursor, vm-update-ual, and vm-update-store-contracts, then keep vm-update-convergence.ts as a barrel if the public import path matters. Preserve behavior and existing exports through the barrel.
There was a problem hiding this comment.
🟡 Issue: Split the near-1k W2 convergence module before it becomes the catch-all owner
What's wrong
The file is cohesive only at the feature-label level. It combines scalar adaptation, domain hashing, page proof validation, log commitment construction, cursor policy, UAL parsing, closed vocabularies, and store-facing target shapes in one new 988-line module. Landing just below 1k on day one is a strong signal that the next W2 rule will push this into a sprawling canonical dumping ground.
Example
Changing endpoint-origin normalization in canonicalPageProof now happens in the same file a reader must scan for UAL candidate parsing, cursor resume semantics, raw-log hashing, scalar bounds, event identity, vocabularies, and target interfaces. Those concerns do not need to change together, but the module makes them one review and ownership unit.
Suggested direction
Turn this into a small folder with a barrel export and one module per contract family. That keeps the package-neutral boundary intact while reducing the number of unrelated invariants a maintainer must hold to change any one rule.
For Agents
Keep the public exports stable through packages/core/src/index.ts, but split vm-update-convergence.ts into focused modules such as errors.ts, vocabularies.ts, scalars.ts, scope.ts, event-position.ts, ordered-log-commitment.ts, page-proof.ts, coverage-cursor.ts, and scoped-ka-ual.ts. Move the tests into matching focused files or at least separate describe blocks by imported module, preserving behavior and exported names.
There was a problem hiding this comment.
🟡 Issue: The W2 convergence module is doing too many jobs
What's wrong
This adds a near-1k-line, multi-domain module to core, which is otherwise organized around smaller single-purpose modules. That makes the core W2 model harder to review, extend, and safely publish because unrelated changes will keep landing in one broad file instead of in the layer that owns each concept.
Example
A caller looking for canonicalScopedKaCandidatesFromVerifiedUal has to scan past scalar wrappers, closed vocabularies, hashing helpers, page assurance, cursor logic, and future store DTOs in the same file. The next W2 slice will almost certainly push this over 1k lines.
Suggested direction
Decompose this before merging. The behavior can stay identical while each concept gets its own file and the public barrel re-exports only the stable pieces.
For Agents
Preserve exported behavior, but split vm-update-convergence.ts into focused modules such as vm-update/errors.ts, scope.ts, log-commitment.ts, page-proof.ts, cursor.ts, and ual-candidates.ts; keep a small barrel for the intended public API and run the existing convergence/parity tests.
There was a problem hiding this comment.
🟡 Issue: Split the W2 convergence monolith before it hardens
What's wrong
This introduces a near-thousand-line public core module that explicitly aggregates several separate domains. That makes the W2 contracts harder to scan, encourages unrelated additions in one file, and weakens ownership boundaries before the store/scanner layers even arrive.
Example
A future change to page-proof validation now has to live beside scope-id derivation, cursor math, UAL parsing, and target DTOs, making this file the default place for unrelated W2 concepts and pushing it past the 1k-line boundary immediately.
Suggested direction
Decompose by concept now, while the contracts are new. The current file is already just under 1k lines and mixes independent responsibilities that should evolve separately.
For Agents
Split packages/core/src/vm-update-convergence.ts into focused modules such as errors/vocabularies, scope identity, event position, log commitment, page proof, cursor, and scoped KA identity. Preserve the current public exports through packages/core/src/index.ts or a small barrel, and keep the existing behavior covered by vm-update-convergence.test.ts.
There was a problem hiding this comment.
🟡 Issue: Split the W2 convergence god module before exporting it
What's wrong
This creates a broad core module that is hard to scan, hard to review, and likely to become the default dumping ground for the rest of W2. The abstractions are not naturally one unit: page proof validation, raw-log commitments, scope identity, cursor math, and UAL candidate parsing have different ownership boundaries and different reasons to change.
Example
orderedLogCommitment() and canonicalScopedKaCandidatesFromVerifiedUal() share only scalar/error plumbing, but future changes to either now happen in the same near-1k-line public module.
Suggested direction
Keep the shared error/scalar adapter as a small common layer, but move each independent value contract into its own module. The current file starts just under the 1k-line smell threshold and already bundles too many unrelated concepts, so the clean move is decomposition before more W2 store/reducer code lands.
For Agents
In packages/core/src/vm-update-convergence.ts, split the public surface into focused files such as vm-update/errors, scope, events, log-commitment, page-proof, cursor, and ka-candidates, then preserve the current exported names through a small barrel. Keep behavior unchanged and run the existing core W2 tests against the re-exported API.
| type EvmAddressV1, | ||
| } from './sync-wire-scalars.js'; | ||
|
|
||
| const EVM_ADDRESS = /^0x[0-9a-f]{40}$/; |
There was a problem hiding this comment.
🟡 Issue: Reuse the canonical scalar layer instead of restating its regexes
What's wrong
This module imports canonical scalar helpers but still copies their regex-level implementation. The duplicated validators add drift risk and make scalar policy harder to change safely across the codebase.
Example
If sync-wire-scalars.ts changes the canonical address or digest rule, the existing sync/control-object path will follow the canonical helper while W2 keeps validating against the copied regexes here. That creates two canonical scalar definitions inside core.
Suggested direction
Let sync-wire-scalars.ts remain the single source of truth for EVM addresses, digests, unsigned decimals, and hex bytes. Add a narrow zero-permitting author helper there if needed, then adapt errors at the W2 boundary.
For Agents
Move missing return-style or nullable-author helpers into packages/core/src/sync-wire-scalars.ts, or wrap the existing assertions in W2-specific error translation. Preserve W2 error codes while deleting the duplicate regex constants from vm-update-convergence.ts.
There was a problem hiding this comment.
Agreed, and fixed in 977e02a4 — this was the strongest of the yellow findings.
EVM_ADDRESS, CANONICAL_DIGEST_32, CANONICAL_UNSIGNED_DECIMAL and LOWER_HEX_BYTES are gone. The module now adapts the shipped assertions (assertCanonicalEvmAddress, assertCanonicalDigest, assertCanonicalHexBytes, parseCanonicalDecimalU256) into W2's error codes through one small adapt() helper, and holds no validators of its own. The UAL parser's address and number segments go through the same path.
One constant remains — the zero-address literal — and only because W2 must accept it for author where every shipped helper rejects it. It's a literal, not a validator, and the comment says so. A noncanonical zero like 0X00… misses the equality and falls through to the shipped assertion, so permitting zero does not also permit sloppy zero.
Worth noting this is the same defect class the plan itself flags in §8.3.1: a second transcription reproduces the problem one level up.
There was a problem hiding this comment.
🟡 Issue: Make the UAL builder single-source instead of mirrored with a parity test
What's wrong
The comment correctly identifies a drift risk, but the implementation accepts that risk by restating the canonical UAL rule in a second package. The cross-package test is a guardrail around duplicated logic, not a structural fix.
Example
Today, changing the rootless UAL spelling requires editing the core builder and the agent builder, plus maintaining a cross-package parity test. That test catches drift after the fact, but the simpler structure is to make drift impossible.
Suggested direction
Use the existing dependency direction: agent already depends on core, so core can own this neutral identity function and agent can wrap/re-export it. That deletes a whole drift class instead of institutionalizing it with a parity suite.
For Agents
Move the canonical legacy/rootless UAL construction into packages/core/src/vm-update-convergence.ts or a smaller core identity module, then change packages/agent/src/ka-identity.ts so buildReconciledKnowledgeAssetUal delegates to it while preserving its exported API. Keep/adjust a focused agent compatibility test if needed, but remove the duplicated rule.
There was a problem hiding this comment.
🟡 Issue: Reuse one dense-data array helper instead of adding another validator
What's wrong
The PR duplicates an existing low-level validation abstraction instead of moving it to the canonical layer. That creates two versions of a subtle invariant involving holes, accessors, inherited indices, and extra array keys, which is exactly the kind of helper that should not drift across packages.
Example
orderedLogCommitment and canonicalPageProof now depend on denseDataArray, while chain config/read validation depend on snapshotDenseDataArray. Both encode the same dense, enumerable data-array invariant, but future fixes have to be made in two places and may not stay semantically aligned.
Suggested direction
Promote the existing dense-data array snapshot concept into core and reuse it from both packages, with thin error adaptation where W2 needs its own error codes.
For Agents
Move the descriptor-based array snapshot helper to a canonical core utility, likely next to sync-wire-objects.ts, and have both W2 and chain import that helper. Preserve W2's typed error mapping by catching utility errors at the W2 boundary rather than reimplementing the array walk.
| * set it: its limit is 4 and folding it into the snapshot's single lane | ||
| * would throttle an unrelated path. | ||
| */ | ||
| readonly sharedOwner?: FinalizedChainReadOwnerV1; |
There was a problem hiding this comment.
🟡 Issue: Inject admission policy instead of teaching the endpoint runner about snapshot owners
What's wrong
The endpoint runner used to own endpoint lifecycle. This change makes it also understand a process-wide snapshot admission mode, an owner vocabulary, and an optional holder dimension. That is a boundary leak and a classic mode flag: the runner now has to know which callers are snapshots instead of just running under a supplied admission policy.
Example
Adding another shared finalized snapshot caller now means editing the chain-layer owner vocabulary and carrying that type through the generic endpoint lifecycle. The one-shot read path also has to accept an optional holder in messages.saturated even though it never uses that concept.
Suggested direction
Replace the sharedOwner mode flag with a small admission abstraction. This would delete the runner-level branch, keep maxConcurrentPerChain meaningful for local gates, and stop feature-specific owner labels from leaking into the generic endpoint runner.
For Agents
Refactor createStrictFinalizedEndpointRunnerV1 to accept an admission strategy or runWithAdmission callback. Have the one-shot and snapshot factories supply either the local nonqueueing gate or the process-wide snapshot gate. Keep saturation messages and holder attribution in the admission adapter, not in the generic runner contract.
There was a problem hiding this comment.
Agreed, and refactored in 977e02a4.
sharedOwner is gone. createStrictFinalizedEndpointRunnerV1 now takes a FinalizedReadAdmissionV1 and runs it without knowing what kind it is. The snapshot factory injects createSharedFinalizedReadAdmissionV1(owner); the one-shot read injects createLocalNonqueueingAdmissionV1(4). Concretely:
maxConcurrentPerChainis out of the runner contract entirely — it now lives with the local gate, where it is meaningful.FinalizedChainReadOwnerV1no longer crosses into the generic lifecycle;messages.saturated(active, holder?)takes a plainstring, and the local gate simply passes none.- Adding another shared caller is now an owner-tuple entry plus a factory call, with no edit to the endpoint lifecycle.
Your framing that it was "a classic mode flag" is accurate — it branched on caller identity inside code whose job is endpoint lifecycle.
There was a problem hiding this comment.
🟡 Issue: Keep the generic endpoint lifecycle independent from the shared finalized-read registry
What's wrong
The refactor tries to prevent feature-specific owner knowledge from leaking into the generic endpoint runner, but the interface that runner consumes now lives in the feature-specific registry module. That preserves a boundary leak at the module level and makes the local one-shot read path depend on snapshot admission concepts for a type-only reason.
Example
The endpoint runner is described as generic lifecycle code, but its profile type now depends on a module whose primary job is the process-wide finalized-read registry. A reader following the type has to understand snapshot owner labels just to reason about the one-shot read transport's local admission policy.
Suggested direction
Invert the dependency: the shared registry should adapt to a generic admission-policy contract, not define the contract that the generic lifecycle imports.
For Agents
Move the small admission-policy interface to the generic layer, likely nonqueueing-admission.ts or strict-current-finalized-evm-lifecycle.ts, and have both createLocalNonqueueingAdmissionV1 and createSharedFinalizedReadAdmissionV1 implement that interface. Keep owner vocabulary and process-wide registry isolated in finalized-chain-read-admission.ts.
| ]); | ||
| }); | ||
|
|
||
| it('contends across INDEPENDENTLY constructed scopes on one chain', async () => { |
There was a problem hiding this comment.
🟡 Issue: Split the snapshot tests before accepting the 1k-line crossing
What's wrong
The PR pushes an already-large test file past the 1000-line threshold without a strong structural reason. The newly added admission scenario is valuable, but it is a separate concern from the existing snapshot behavior matrix and makes the file harder to scan and maintain.
Example
Line 504 starts a new process-wide admission/concurrency scenario inside a file that already covers endpoint pinning, preflight, budgets, cancellation, forged selectors, and snapshot session behavior.
Suggested direction
Keep the new coverage, but put admission-specific tests in a focused file near finalized-chain-read-admission.unit.test.ts or extract common harness setup. The main snapshot behavior test should not keep absorbing every new concern.
For Agents
Move the new cross-scope/process-wide admission scenario into a dedicated snapshot-admission test file, or extract shared loopback/request helpers so this file drops back below 1000 lines. Preserve the same assertions and fixture behavior.
There was a problem hiding this comment.
Agreed, done in 977e02a4. The file is back to 978 lines.
Shared loopback/request helpers moved to packages/chain/test/snapshot-test-fixtures.ts, and admission now has its own file, strict-finalized-snapshot-admission.unit.test.ts, which also picked up three new cases (published-shape compatibility, the default owner not bypassing the shared lane, permit release on consumer throw).
That file header states why the separation matters rather than just that it exists: the two saturation cases in the behaviour matrix both reuse a single handle, so they pass identically whether the permit is per-instance or per-chain — they cannot discriminate the property the admission file is there to pin.
| ).toBe('page-malformed'); | ||
| }); | ||
|
|
||
| it('freezes the proof and its origin list', () => { |
There was a problem hiding this comment.
🟡 Issue: Nested mutation-safety contract is only partially verified
What's wrong
The change relies on canonicalized values being immutable after validation, but the tests only prove shallow freezing for a few outputs. That leaves a data-integrity regression unguarded: future code could accidentally stop freezing nested evidence while the suite stays green.
Example
A regression that returned mutable from, through, finalizedAnchor, coveredThrough, resumeAfter, or candidate objects would still pass these tests. A focused assertion would be expect(Object.isFrozen(frozen.from)).toBe(true) and an attempted mutation should fail or leave the value unchanged.
Suggested direction
Add behavior-level assertions for the nested returned records, especially page block refs, cursor positions/observations, and individual KA candidate objects.
For Agents
In packages/core/test/vm-update-convergence.test.ts, extend the mutation-safety coverage around canonicalPageProof, canonicalCoverageCursor, and canonicalScopedKaCandidatesFromVerifiedUal. Preserve canonical return values and prove all nested returned records/arrays that can be reused after validation are frozen.
There was a problem hiding this comment.
Agreed, and fixed in 977e02a4. Your diagnosis is exactly right — the code already froze the nested records; the tests only checked the top level, so a regression returning mutable nested values would have stayed green. That is a check that cannot fail.
Added assertions for proof.from / through / finalizedAnchor, cursor.coveredThrough / resumeAfter / scannedThroughUnattested, and each individual KA candidate (the array being frozen says nothing about the objects inside it). They are behavioural, not just Object.isFrozen flags: an attempted write must throw and leave the value unchanged.
Proven negative: dropping Object.freeze from canonicalBlockRef fails the new page-proof case.
**🔴 Snapshot factory broke published callers.** `@origintrail-official/dkg-chain`
is on npm (10.0.11, not private) and this factory is public surface; requiring
`owner` broke every external `{ chainId, endpoints }` caller. Now defaults to
`foreground`, which is a real owner label — those callers genuinely ARE
foreground — not an "unknown" bucket. Attribution survives because the registry
is process-wide regardless of holder, and RFC64/W2 pass their owner explicitly.
A test pins that the default does NOT get a private gate, so the default cannot
become an escape hatch from the shared lane. I had over-applied "no compat
shims" to a case where the default is semantically correct.
**🔴 Raw-log commitment did not bound topics per log.** `MAX_UPDATE_PAGE_EVENTS`
constrains logs, not topics: ONE log with a million topics passed it, then
allocated and hashed a million entries — from an untrusted RPC response. Now
capped at the EVM bound of 4, checked BEFORE the map, with a non-array rejected
as `page-malformed` rather than throwing a raw TypeError.
**🔴 Page proof accepted impossible same-height hashes.** `from{100,A}`,
`through{100,B}`, `finalizedAnchor{100,C}` was accepted and could then advance
coverage. A single-block page has `from === through`, and a page reaching the
anchor has `through === finalizedAnchor` — empty and sparse pages are exactly
where no log position would expose the contradiction. All three pairs are now
checked.
**🟡 Duplicated canonical-scalar regexes removed.** The module had copies of
`EVM_ADDRESS`, `CANONICAL_DIGEST_32`, `CANONICAL_UNSIGNED_DECIMAL` and
`LOWER_HEX_BYTES` from `sync-wire-scalars.ts` — two canonical-scalar policies
inside one package. It now adapts the shipped assertions into W2's error codes
and holds no validators of its own. Only the zero-address literal remains, and
only because W2 must ACCEPT it for `author` where every shipped helper rejects
it. This is the same defect class as the plan's own B6 finding.
**🟡 Admission policy is injected, not a mode flag.** `sharedOwner` made the
generic endpoint runner understand a snapshot-specific owner vocabulary — a
boundary leak that would grow an enum in the lifecycle for every future shared
caller. The runner now takes a `FinalizedReadAdmissionV1` and runs it without
knowing which kind it is; the snapshot factory injects the process-wide policy,
the one-shot read injects a local gate. `maxConcurrentPerChain` is gone from the
runner contract, and `holder` is a plain string so the owner type no longer
crosses that boundary.
**🟡 Admission tests split out.** The snapshot behaviour matrix is back under
1000 lines. Shared loopback/request helpers moved to `snapshot-test-fixtures.ts`;
admission scenarios now live in `strict-finalized-snapshot-admission.unit.test.ts`.
**🟡 Nested freezing now actually asserted.** The code already froze block refs,
cursor positions and candidates; the tests only checked the top level, so a
regression that returned mutable nested records would have stayed green.
Assertions are behavioural — an attempted write must throw and leave the value
unchanged.
**Deferred, with rationale: splitting the module into ~8 files.** The plan's
§10.1 names one file and the sections are already the module map. Doing an
8-way split mid-W2, before §10.3–§10.6 land and show where the seams actually
fall, would churn the same file twice. Worth revisiting once W2a is complete.
Verification: 49 core + 841 chain (whole unit lane) + 20 agent tests green.
6 regression mutants, one per accepted fix, all killed and restored
byte-identical — each confirmed to kill the intended test BY NAME, including
re-running the one whose naive form would have failed to compile rather than
failing an assertion.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
| if (!isPlainRecord(input)) { | ||
| throw new TypeError('Strict finalized snapshot RPC config must be a plain data record'); | ||
| } | ||
| const { owner, ...rest } = input; |
There was a problem hiding this comment.
🔴 Bug: Snapshot config validation now executes accessor fields before rejecting them
What's wrong
The snapshot factory is meant to preserve the strict plain-data config contract, but the new owner-peeling step reads and copies all enumerable properties first. That weakens the trust boundary: accessor-backed configs can run code during validation and can be accepted after being converted into a plain rest object.
Example
A config object with an enumerable getter for chainId or endpoints now has that getter executed and the copied value validated. Before this wrapper, the same object would be rejected as not having enumerable data properties before any field read.
Suggested direction
Do not use object rest until after descriptor validation has proved the input is data-only; default owner only after that validation.
For Agents
In snapshotStrictFinalizedSnapshotConfigV1, validate the original input's own keys/descriptors with a snapshot-specific allowlist that includes owner before reading or spreading fields. Then read owner via the validated data property and pass a plain { chainId, endpoints, blockReferenceProfile } object to the existing validator. Add a regression test with an accessor-backed chainId proving it is rejected without invoking the getter.
There was a problem hiding this comment.
Confirmed and fixed in 36332221f, with the remaining half in 3c5ebb995.
I measured it rather than reasoning about it, and it was worse than the finding states — the wrapper did not merely run the getter, it accepted a config the base validator rejects untouched:
base validator : rejected, getter invoked 0 times
my wrapper : ACCEPTED, getter invoked 1 time
Object rest is a read: it executed the caller's enumerable getters and flattened them into data properties, so the descriptor check downstream saw a clean object. A weaker door into the same room, opened while fixing round 1.
assertConfigDataProperties now takes the optional-key set, and the snapshot path calls it on the original input with owner allowlisted, before anything is read. The base config is rebuilt explicitly from the proven data properties, and blockReferenceProfile is added only when the caller actually set it (the base allowlist rejects an explicit undefined key).
You also flagged owner: null being treated as omission by ??. Fixed: omission means foreground; an explicitly present owner must be a known value, so null and undefined are now rejected rather than silently defaulted.
Tests: strict-finalized-snapshot-config.unit.test.ts (8 cases), including one asserting the wrapper and the base validator produce an identical error rather than merely both throwing. Restoring the object-rest shape kills 5 of them by name.
Review round 2, and this one is a regression I introduced in `977e02a4`.
`snapshotStrictFinalizedSnapshotConfigV1` destructured `{ owner, ...rest }`
before validating. Object rest is a READ: it executed the caller's enumerable
getters and flattened them into plain data properties, so the descriptor check
downstream saw a clean object and passed. Measured, not inferred:
base validator : rejected, getter invoked 0 times
my wrapper : ACCEPTED, getter invoked 1 time
So a config the shipped validator refuses without touching was executed and
accepted — a weaker door into the same room, opened while fixing round 1.
`assertConfigDataProperties` now takes the optional-key set, and the snapshot
path calls it on the ORIGINAL input, with `owner` allowlisted, before anything
is read. The base config is then rebuilt explicitly from the proven data
properties rather than spread from the caller's object, and
`blockReferenceProfile` is added only when the caller actually set it — the base
allowlist rejects an explicit `undefined` key.
The ordering is the contract, not an implementation detail, and the comment at
the call site says so: object rest cannot precede the check that decides whether
reading is allowed at all.
Verification: wrapper and base validator now agree exactly — same verdict, same
message, zero getter invocations. 7 new tests in
`strict-finalized-snapshot-config.unit.test.ts`, including one that asserts the
two produce an IDENTICAL error rather than merely both throwing. Restoring the
object-rest shape kills 5 of them by name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
PR #2051 — W2a foundations merge-readiness reviewPR: OriginTrail/dkg#2051 The implementation is thoughtful and the existing suite is strong, but the new trust boundary can still manufacture authoritative evidence from malformed arrays, rejects valid shipped-contract events, and creates a normal production contention path that can lose an RFC64 head until some unrelated future trigger arrives. Those are W2 correctness and convergence failures, not style concerns. Ground truth and scopeI reviewed the exact merge delta
Every finding below is introduced by this PR. Where an older component participates—most notably the RFC64 receiver retry policy—the regression is the new interaction created by routing RFC64 through the process-wide nonqueueing permit. Findings, prioritizedP1 — Sparse arrays can mint false
|
…ree P2s **P1 — sparse arrays could mint false `dual-origin-corroborated` evidence.** `map` skips holes while `new Set` observes them as `undefined`, so `['https://a', <hole>]` had length 2 AND set size 2 and was accepted as two distinct origins; the frozen proof serialized as one URL plus a null. Adds `denseDataArray`: length checked before any iteration, then own-descriptor inspection per index, rejecting holes, inherited entries and accessors WITHOUT invoking getters (`getOwnPropertyDescriptor` does not read). Applied to `normalizedOrigins`, the log page, and each log's `topics`. `Array.from` is deliberately not used — it materializes holes as `undefined` and can invoke getters. `isAuthoritativePage` now canonicalizes instead of reading the assurance string, so authority cannot be claimed by declaration. **P1 — the 4,096-byte scalar cap rejected valid shipped-contract events.** `log.data` shared the identity-scalar cap, which permits 2,047 payload bytes. A legal 21-entry `MerkleRoot[]` from `setMerkleRoots` is 64 + 21*96 = 2,080 bytes and was rejected — and that event is a BLOCKING mutation, so W2 would have halted before persisting the latch that must fail closed. Raw log bytes now use a per-page `MAX_ENCODED_UPDATE_LOG_BYTES_PER_PAGE = 8 MiB` budget charged while walking; identity scalars keep the tight cap. **P1 — normal same-chain contention could discard a valid RFC64 head.** The process-wide lane is right, but it is nonqueueing and the receiver had no admission-aware deferral: a 60s pinned scan for one CG made another CG's receiver burn three attempts in ~1.75s, mark the task failed, and delete its pending key. A nested `concurrency-saturated` is now classified as a detached deferral that gives the attempt back, releases both the concurrency slot and the per-scope semantic lock before waiting, keeps the task pending, is bounded (`maxAdmissionDeferrals`), unrefs its timer, and is cleared on close. The classifier walks `cause` rather than matching message text, and stays narrow — an ordinary error keeps the existing fail-fast path. **P1 — resume filtering discarded a conflicting identity.** At equal ordering `isDiscardedByResume` ignored `blockHash`/`transactionHash`, silently treating a different event at the same position as already reduced and destroying the only signal that would catch a reorg at the resume boundary. Equality now requires `sameEventIdentity` and otherwise raises the new typed `resume-identity-conflict`. Hashes are still NOT in the ordering comparator: lexicographic hash order would invent a position the chain never assigned. **P1 — `VmUpdateConvergenceStoreV1` removed.** It had no implementation or consumer and could not express the plan's expected-revision PLUS expected-prior-cursor CAS; two stale callers can hold the same revision. Publishing it as `V1` would have made a placeholder an API promise. It lands with the store slice. **P1 — snapshot config getter execution** was fixed in `36332221f`; this adds the remaining half the review identified: an explicitly present `owner: null` or `undefined` is now rejected rather than silently treated as omission by `??`. **P2** — the five core vocabularies and `FINALIZED_CHAIN_READ_OWNERS` are `Object.freeze`d (`as const` is compile-time only, and owners had validator/ enforcer split-brain); `buildScopedKnowledgeAssetUal` validates chain id, address and `0 <= kaId <= uint256.max` before formatting instead of emitting well-formed UALs that denote nothing. Verification: 67 core, 850 chain (whole unit lane), 25 focused agent tests green. Regression mutants for each fix, restored byte-identical: disabling the contention classifier fails 4 of 5 deferral tests and correctly leaves the "ordinary error must not be deferred" negative case passing. Note on the outstanding P2 (no test reaches the real RFC64 factory call): the owner label is now covered indirectly — the config layer rejects an explicit bad owner and the shared-lane tests prove owner attribution end to end — but a test that drives the production factory itself is still owed and is tracked for the next round. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
Merge-readiness review — all six P1s and all three P2s addressedHead is now
Specifics worth flaggingYour direction on the sparse arrays was better than the online reviewer's, and better than what I would have written. I would have reached for a length bound, which as you note still accepts the false two-origin proof. Descriptors are the only check that distinguishes a hole from a value, and The 4,096 cap. Your arithmetic is right and the consequence is the part that matters: The contention lifecycle. Implemented as you specified: no provider attempt consumed, no receiver slot or semantic lock retained, task stays pending, bounded by What I did not doThe real-factory owner test (P2) is not done, and I am not claiming it is. Owner attribution is now covered indirectly — the config layer rejects an explicit bad owner, and the shared-lane tests assert holder attribution end to end — but your point stands that the production line in The core module split (raised by the online reviewer) remains deferred with reasoning on its thread: §10.3–§10.6 have not landed, and the current section comments are my predicted seams rather than observed ones. On the local Windows failures you sawI reached the same place, and then went one step further rather than leaving it at "unrelated". I re-ran the six RFC64 files on a clean build and read the actual failure text: Verification67 core, 850 chain (whole unit lane), 25 focused agent tests green. Regression mutants for each accepted fix, restored byte-identical; disabling the contention classifier fails 4 of 5 deferral tests and correctly leaves the "ordinary error must not be deferred" negative case passing. The PR description has been rewritten to match this head — |
Closes the one item I left open from the merge-readiness review: no test reached the real `createStrictCurrentFinalizedEvmSnapshotScopeV1` call, so changing `owner: 'rfc64'` to `'foreground'` survived the focused precommit and runtime suites 10/10 — the line that establishes attribution was unobserved. The new suite drives the real `createRfc64FinalizedVmAgentPrecommitV1` against an RPC endpoint that accepts the connection and never answers, so the precommit genuinely takes and HOLDS the process-wide permit. While held, a second owner probes the registry and is told who the holder is. That single assertion proves three things at once: the production path uses the shared registry, it labels itself `rfc64`, and the test reaches production code rather than a hand-built chain fixture. The second case pins release: an aborted precommit must return the permit, or a leak would wedge every other caller on that chain for the life of the process. Getting here required matching the canonical runtime request exactly. Two things bit, and both are recorded in the test so the next person does not repeat them: the accepted policy must be the full `ContextGraphPolicyV1` shape (a thinner fake is rejected before the chain is touched), and `roster` must be EXPLICITLY `null` — the guard is `!== null`, so an omitted field is `undefined` and fails it. Verification: the reviewer's own mutant is now killed. `owner: 'rfc64'` → `'foreground'` fails exactly one test — this one — while the ten pre-existing RFC64 precommit/runtime tests still pass, reproducing the coverage gap and closing it. Source restored byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
The last open item is closed —
|
| // before waiting, and keeps the pending key so a duplicate announcement | ||
| // still dedupes onto this task instead of creating a second writer. | ||
| if (outcome === 'defer-admission' && !this.#closed) { | ||
| this.#scheduleAdmissionRetry(task); |
There was a problem hiding this comment.
🔴 Bug: Deferred admission makes whenIdle() report idle while work is still pending
What's wrong
The new admission deferral path releases the receiver slot by design, but it also removes the task from both structures used by #isIdle(). That changes the receiver drain contract: callers waiting for all accepted reconciliation work to settle can proceed while the head is merely sleeping for admission retry.
Example
A receiver hits concurrency-saturated, returns defer-admission, and schedules a 500ms retry. During that 500ms window #active.size === 0 and #queue.length === 0, so whenIdle() resolves immediately even though the pending key is still retained and the task will run later. An explicit synchronizeCurrentCatalogHead() call can therefore report that scheduling has drained before the discovered head is applied, staged, not-found, or failed.
Suggested direction
Track deferred tasks as outstanding work for idle/drain purposes, and resolve idle only after their retry timer has either requeued and completed the task or the task is dropped on close/failure.
For Agents
In packages/agent/src/rfc64/public-catalog-receiver-v1.ts, include deferred-admission timers/pending deferred tasks in the idle predicate and stats, or keep deferred tasks in a queue state that whenIdle() observes. Preserve duplicate dedupe while deferred, and add a test where whenIdle() is called immediately after a contention deferral and must not resolve until the retry reaches a terminal outcome.
There was a problem hiding this comment.
🔴 Bug: Admission deferrals make whenIdle() resolve while work is still pending
What's wrong
The new deferral path removes the task from the active set and leaves the queue empty while a retry timer owns the pending head. Because idle detection ignores those timers and the pending-key map, public idle waiters can observe the receiver as drained even though a valid head is still waiting to be retried.
Example
If reconcileHead() throws a concurrency-saturated error once, #runTask() returns defer-admission, line 301 schedules a timer, and the active promise is removed while the queue is empty. A concurrent await receiver.whenIdle() resolves immediately even though the same head will be re-queued later and has not applied or failed yet. synchronizeCurrentCatalogHead() can therefore return before the discovered head has actually been reconciled.
Suggested direction
Track deferred retries as pending work for idle purposes, and resolve idle waiters only after the deferred task is re-run to a terminal outcome or dropped on close.
For Agents
In Rfc64PublicCatalogReceiverV1, include deferred admission timers/tasks in idle accounting, or store deferred tasks in a state that #isIdle() observes. Preserve the desired release of the concurrency slot and scope lock while waiting. Add a test that schedules a head whose first reconcile throws concurrency-saturated, calls whenIdle() before admissionDeferralMs expires, and proves it does not resolve until the retry applies or fails.
There was a problem hiding this comment.
Confirmed and fixed in 4c874a582. This is a regression I introduced one commit earlier while fixing the merge-readiness P1, and your reading of it is exactly right: the deferral releases the slot and the queue entry by design, which left the task in #pendingByKey plus a detached timer — invisible to #isIdle(), which tests active === 0 && queue === 0.
There is now an explicit #deferred set: registered before the timer is armed (between those two statements the task must never be invisible), observed by #isIdle(), reported as stats().deferred, cleared on close(), and cleared on the bounded give-up path — which now also resolves idle, since that path previously deleted the pending key without re-checking.
Worth admitting: my own releases the concurrency slot and scope lock while waiting test called whenIdle() and also slept afterwards, so it was green for the wrong reason and could never have caught this. The new test calls whenIdle() and asserts it has not resolved 120ms into a 300ms deferral, then that it resolves only once the work reaches a terminal outcome.
Proven negative: removing #deferred.size === 0 from the idle predicate fails exactly that test.
| * The counter bounds it so a permanently wedged lane degrades into a normal | ||
| * failure instead of looping forever. | ||
| */ | ||
| #scheduleAdmissionRetry(task: ReceiverTaskV1): void { |
There was a problem hiding this comment.
🟡 Issue: Admission deferral is bolted on as a hidden scheduler state
What's wrong
The receiver used to have two visible work states: queued and active. This change creates a third state, pending-but-not-queued-or-active, spread across #deferralTimers, a mutable optional field on the task, a magic 'defer-admission' return value, and a chain-specific error crawler. That makes lifecycle reasoning harder and already leaves the idle contract disconnected from the task model.
Example
A task that hits chain admission contention returns defer-admission, schedules a timer, leaves pendingByKey populated, then removes itself from #active while #queue is empty. At that point whenIdle() can resolve even though the task is intentionally still pending and will be requeued later.
Suggested direction
Model deferred work inside the scheduler’s existing state machine instead of as detached timers plus a string outcome and error-shape classifier. The cleaner move is for the reconciler to return a declared deferral result, then for the receiver to track queued/active/deferred tasks uniformly.
For Agents
In public-catalog-receiver-v1.ts, preserve dedupe, provider-attempt accounting, and the fact that admission waits do not occupy a worker slot. Rework the deferral path into a first-class scheduler state, for example queue entries with readyAt plus one pump timer, or a tracked deferred set included in idle/close. Move contention classification into the reconciler or inject a typed deferral policy so the receiver no longer cause-walks chain-layer errors. Add a test proving whenIdle() does not resolve while a deferred task is waiting.
There was a problem hiding this comment.
🟡 Issue: Keep admission deferrals inside the receiver lifecycle model
What's wrong
The new deferral path splits one task across #pendingByKey, #deferralTimers, #queue, and #active. That makes the scheduler harder to reason about and creates partial-state lifecycle semantics: the receiver can look idle even though it is deliberately holding a pending task for retry.
Example
After reconcileHead() throws concurrency-saturated, #runTask() returns defer-admission; the task is removed from #active, not in #queue, and only exists behind the timer until it fires. A caller observing whenIdle() in that window sees the receiver as idle while the pending key is still retained and work is scheduled to resume.
Suggested direction
Represent delayed admission retry as a first-class queue/task state instead of a detached setTimeout side channel, so idle, stats, close, dedupe, and retry ownership all reason over one state machine.
For Agents
In packages/agent/src/rfc64/public-catalog-receiver-v1.ts, preserve the behavior that contention waits outside a concurrency slot, but fold deferred tasks into the receiver's single work model. Consider a queued task state/availableAt delay or make deferred tasks part of idle/stats/pending cleanup. Add a focused assertion that whenIdle() does not resolve while an admission-deferred task is still pending.
There was a problem hiding this comment.
Agreed, and largely resolved by the 🔴 fix in 4c874a582 — they were the same defect seen from two angles, which is itself the argument for your framing.
Queued, active and deferred are now three named states rather than two plus a timer and a mutable field, and deferred appears in stats() so the state is observable rather than inferred.
On the classifier: moved out of the receiver entirely. isFinalizedChainAdmissionContention now lives in packages/chain/src/finalized-chain-read-admission.ts — the package whose code concurrency-saturated actually is — and the receiver takes an injectable isDeferrableError that defaults to it. So the receiver no longer cause-walks errors from a layer it does not own, and a different admission source can supply its own policy.
I did not take the larger step of moving deferral into the reconciler's return type ('applied' | 'not-found' | 'staged-only' | 'deferred'). The precommit throws from deep inside the chain stack, so something still has to classify a thrown error; that would relocate the classification rather than remove it, and it changes a contract shared with the native receiver. If you would rather have it there I will do it — say so and it goes in the next round.
readyAt-on-queue-entries plus one pump timer is the other shape you suggested. I kept detached timers because the retry interval is per-task and the queue is scanned by scope-key eligibility rather than time; a single pump timer would need its own ordering. Happy to revisit if you think the timer set will not age well.
There was a problem hiding this comment.
🟡 Issue: The receiver deferral path adds a detached third scheduler state
What's wrong
The new deferral mechanism is bolted beside the existing queue rather than represented as a first-class queue state. That spreads the same invariant across multiple fields and methods, making the scheduler much harder to reason about than the original queued/active model.
Example
A deferred task is not in the queue and not active; close(), stats(), #isIdle(), timer cleanup, and duplicate scheduling all need separate branches to keep that invisible third state coherent.
Suggested direction
Fold deferred work back into the scheduler model instead of tracking it with an out-of-band set plus per-task timers.
For Agents
Preserve the same deferral semantics, but model receiver work with one scheduler-owned task queue that includes a readyAt/deferredUntil field and one wake timer, or extract a small scheduler class that owns queued/deferred/active transitions atomically. Prove the existing deferral, idle, close, and duplicate-announcement tests still pass.
There was a problem hiding this comment.
🟡 Issue: Model admission deferral directly instead of adding scheduler side channels
What's wrong
The new admission-deferral flow is bolted onto the existing scheduler as a side channel. It returns a magic outcome from #runTask, keeps a mutable counter on the task, parks tasks in a separate #deferred set, tracks timers in another set, and special-cases pending-key cleanup. That spreads one lifecycle invariant across several structures and makes the receiver harder to reason about.
Example
Adding another wait reason, changing idle semantics, or changing close behavior requires touching the outcome string in #pump, the detached timer path, the deferred set, the timer set, pending-key cleanup, and stats. That is a sign the scheduler state is not modeled directly enough.
Suggested direction
Turn deferral into a first-class scheduler state rather than a special outcome plus detached timer set. A centralized state transition path would delete much of the manual cleanup and idle bookkeeping.
Confidence note
The current behavior may be correct; this is a maintainability concern about the state model introduced by the diff.
For Agents
Refactor the receiver scheduler around explicit task states or a small scheduler helper. Keep one authoritative task record with state like queued | active | deferred, a retry timestamp/timer handle on the task, and centralized transition/finish helpers. Preserve synchronous schedule, per-scope serialization, duplicate-provider dedupe, bounded deferral, and close semantics.
There was a problem hiding this comment.
🟡 Issue: Admission deferral is bolted across the scheduler instead of owned by a task state model
What's wrong
The implementation adds the deferral behavior by threading new state through several unrelated parts of the receiver. The behavior is understandable today, but the state model is now implicit and distributed, which makes future scheduler changes fragile.
Example
A future scheduler outcome now has to know which containers to update: #active, #activeScopeKeys, #queue, #deferred, #deferralTimers, and #pendingByKey. Missing one of those updates creates invisible work, duplicate work, or stuck idle waiters.
Suggested direction
Centralize task state transitions so queued, active, deferred, pending, and idle resolution are updated atomically in one place. That would delete much of the cross-method bookkeeping and make the new third scheduler state explicit without threading special cases through the existing pump/finally path.
For Agents
In packages/agent/src/rfc64/public-catalog-receiver-v1.ts, extract task-state transitions into a small scheduler/delay-queue abstraction or represent delayed admission retries as queued tasks with a single owner for timer/pending/idle cleanup. Preserve: contention does not consume provider attempts, releases concurrency and scope locks, keeps duplicate dedupe while deferred, and closes without leaking timers.
| * leak that would grow an enum in the generic lifecycle every time another | ||
| * shared caller appeared. It now runs whatever policy its factory supplies. | ||
| */ | ||
| export interface FinalizedReadAdmissionV1 { |
There was a problem hiding this comment.
🟡 Issue: The generic admission policy type lives in the shared-registry module
What's wrong
The PR correctly tries to keep the endpoint runner policy-agnostic, but then declares the policy abstraction inside finalized-chain-read-admission.ts, which is specifically the process-wide shared registry with owner vocabulary. That leaves the local gate factory returning an unnamed structural clone and makes the lifecycle import a generic type from a feature-specific module.
Example
A future local admission policy has two poor options: import its type from finalized-chain-read-admission.ts, which makes a local policy depend conceptually on the shared registry, or restate the anonymous run shape again. Both hide that the real abstraction belongs to the endpoint lifecycle/admission layer, not to the process-wide registry implementation.
Suggested direction
Put the neutral policy interface in the neutral admission/lifecycle layer and let both local and shared factories implement it. That removes the anonymous duplicate shape and keeps the process-wide registry from becoming the owner of every endpoint-admission concept.
Confidence note
This is a structure concern rather than a behavior problem; the current structural typing works, but the ownership boundary is already awkward in the new code.
For Agents
Move FinalizedReadAdmissionV1 to a neutral owner, likely nonqueueing-admission.ts or strict-current-finalized-evm-lifecycle.ts, and consider renaming it to something like EndpointAdmissionPolicyV1. Have createLocalNonqueueingAdmissionV1 and createSharedFinalizedReadAdmissionV1 both return that named type. Keep the shared registry module focused on owners, lane state, and process-wide acquisition.
There was a problem hiding this comment.
🟡 Issue: Move the generic admission policy out of the snapshot registry module
What's wrong
FinalizedReadAdmissionV1 is a general runner dependency, but it lives in the same module as the owner vocabulary and process-wide lane registry. That keeps a feature-specific module in the import path of generic endpoint lifecycle code and forces a thin local-admission adapter around the existing nonqueueing gate.
Example
The one-shot read path has no owner concept, but its generic endpoint runner now depends on a type exported from the owner-aware process-wide snapshot admission module. That makes the module boundary read backwards: the generic lifecycle depends on the specialized registry it is supposed to be abstracted away from.
Suggested direction
Put the generic admission abstraction in the generic admission/lifecycle layer, and let the process-wide finalized-read registry be one implementation of it rather than the module that owns the interface.
For Agents
Move the generic policy interface to nonqueueing-admission.ts or strict-current-finalized-evm-lifecycle.ts, make createNonqueueingAdmissionGateV1 and createSharedFinalizedReadAdmissionV1 implement that interface, and keep FINALIZED_CHAIN_READ_OWNERS plus registry state isolated to the shared snapshot admission module.
There was a problem hiding this comment.
Agreed, done in 4c874a582.
FinalizedReadAdmissionV1 is now EndpointAdmissionPolicyV1<K> in nonqueueing-admission.ts — the neutral admission layer. createLocalNonqueueingAdmissionV1 and createSharedFinalizedReadAdmissionV1 both return it, so the local gate no longer returns an unnamed structural clone, and strict-current-finalized-evm-lifecycle.ts imports its policy type from a module that knows nothing about owners or lane state.
Your point that the working structural typing was hiding the ownership problem is the part I want to acknowledge — it compiled fine, which is exactly why it would have calcified.
There was a problem hiding this comment.
🟡 Issue: Caller-specific owner labels leak feature knowledge into the chain package
What's wrong
The chain package now owns a closed list of RFC64/W2 caller names. That makes a general-purpose admission primitive depend on higher-level feature vocabulary, so every new user of the lane creates package churn and broadens the chain API for reasons unrelated to chain behavior.
Example
A future w2-reconcile or catalog-repair snapshot caller cannot just pass its attribution label; it must first modify @origintrail-official/dkg-chain to know that feature name.
Suggested direction
Separate the generic admission mechanism from the feature taxonomy used for attribution/metrics.
Confidence note
The closed owner set may be driven by an external metrics contract, but that contract is not visible in this diff; from the package boundary shown here, the labels are feature-level concepts inside a generic chain admission utility.
For Agents
Keep the process-wide lane in packages/chain, but move caller-specific labels out of the chain layer. Consider accepting a branded/nonempty attribution string, or define the closed metric vocabulary in the higher-level package that owns RFC64/W2 and pass it into the chain policy.
There was a problem hiding this comment.
🟡 Issue: Caller-specific owner labels leak RFC64/W2 concerns into the chain package
What's wrong
This couples a general chain admission primitive to specific agent/W2 features. The chain package should not need to know the product taxonomy of every caller that might use a pinned finalized read; that is a boundary smell and makes future extension more expensive than necessary.
Example
Adding a new consumer such as w2-repair or another agent feature would require editing packages/chain/src/finalized-chain-read-admission.ts even though the admission invariant is only one lane per chain and the owner is just an attribution label.
Suggested direction
Let the chain layer own the admission policy and holder plumbing, but avoid making it own every higher-level caller name. A branded nonempty owner label, or an injected validator at the product boundary, keeps attribution without forcing the published chain package to change for each new feature.
Confidence note
The closed owner list may be intentional for metrics, but it still puts caller vocabulary in the lower-level chain package and should be justified explicitly if kept.
For Agents
Review packages/chain/src/finalized-chain-read-admission.ts and packages/chain/src/strict-current-finalized-evm-config.ts. Preserve process-wide per-chain admission and holder reporting, but move feature owner vocabulary to the agent/W2 telemetry boundary or accept a canonical branded owner label from callers.
| // A deferral releases the concurrency slot AND the semantic scope lock | ||
| // before waiting, and keeps the pending key so a duplicate announcement | ||
| // still dedupes onto this task instead of creating a second writer. | ||
| if (outcome === 'defer-admission' && !this.#closed) { |
There was a problem hiding this comment.
🟡 Issue: Deferred-head dedupe is not verified
What's wrong
The tests cover that a deferred head eventually retries, but not the changed pending-key contract that prevents duplicate announcements from becoming separate tasks while the retry timer is detached. That contract protects the receiver from concurrent semantic writers for the same head.
Example
A regression that moved this.#pendingByKey.delete(task.key) back into the finally path would still let the single-announcement deferral tests apply after the retry. But if the same head is announced again while the deferral timer is pending, schedule() would not find the existing task and could create a second writer for the same head.
Suggested direction
Cover the deferred-but-not-requeued state with a duplicate announcement test so the pending-key lifecycle is pinned, not just the eventual retry.
For Agents
Add a regression case in packages/agent/test/rfc64-receiver-admission-deferral.test.ts: trigger one concurrency-saturated deferral, wait until admissionDeferred is 1 and inFlight is 0, schedule the same announcement from another peer before the retry fires, then assert dedupedInFlight increments and only one task ultimately reconciles/applies.
There was a problem hiding this comment.
🟡 Issue: Deferral dedupe during the retry delay is not verified
What's wrong
The change relies on keeping the pending key while a task waits outside the receiver slot, but the added tests do not exercise duplicate announcements during that waiting state. A future change could drop the pending key on deferral and still pass the current suite, while production would be able to create duplicate work for one exact catalog head.
Example
Failing-test sketch: schedule head A so reconcileHead throws a nested concurrency-saturated; before admissionDeferralMs fires, call schedule() for the same announcement from peer B; assert dedupedInFlight === 1, the pending retry remains a single task, and only one reconciliation path can apply that head.
Suggested direction
Add a focused deferral-window duplicate test so a regression that deletes #pendingByKey on deferral cannot enqueue a second writer for the same head.
For Agents
Look at packages/agent/src/rfc64/public-catalog-receiver-v1.ts around the defer-admission branch and add a receiver deferral regression test in packages/agent/test/rfc64-receiver-admission-deferral.test.ts that schedules a duplicate while the task is deferred. Preserve dedupe semantics and prove the pending key is retained across the timer delay.
There was a problem hiding this comment.
Agreed, added in 4c874a582, and your failure scenario is the test verbatim: one contention deferral, wait until deferred === 1 and inFlight === 0, schedule the same head from a second peer while the retry timer is pending, assert dedupedInFlight increments and only one writer ultimately reconciles (calls.reconcile === 2, i.e. the retry — not a second task).
This is the case that distinguishes the contract from the observable outcome: moving pendingByKey.delete back into the finally path leaves every single-announcement test green while forking a second semantic writer for one head. That is precisely the class of thing those tests could not see.
There was a problem hiding this comment.
🟡 Issue: Deferral coverage does not prove only the contention attempt is refunded
What's wrong
The new deferral path is meant to treat chain-lane contention as unrelated to the head or provider, but the current tests would still pass if a deferral reset all provider retry state. That leaves a risky scheduler contract unverified: provider failures and not-found results that happened before the lane contention should not be erased merely because the task stepped aside.
Example
Use maxAttempts: 1 with two providers: peer A throws a normal transient error, peer B then throws the deferrable contention error, and after the deferral peer B applies. The test should assert peer A is not tried again after the deferral and any prior not-found provider remains excluded.
Suggested direction
Add a mixed-case deferral test that records per-peer calls across a deferral and asserts normal provider attempt limits/failover state are still honored after the task is requeued.
Confidence note
This assumes ordinary provider attempt/not-found history before a deferral is meant to remain counted; the new comment says contention gives only the contention attempt back, not the whole task history.
For Agents
Add a receiver regression test in packages/agent/test/rfc64-receiver-admission-deferral.test.ts that mixes ordinary provider history with an admission deferral. Preserve existing behavior that contention does not consume the current attempt, while proving previous non-deferrable attempts/not-found decisions survive the deferral retry.
| * | ||
| * Matching is on the chain layer's own typed code, not on message text. | ||
| */ | ||
| function isChainAdmissionContention(error: unknown): boolean { |
There was a problem hiding this comment.
🟡 Issue: Do not classify chain admission errors by local shape matching
What's wrong
This pushes chain-layer error semantics into the RFC64 receiver as a magic structural check. The receiver now needs to know the exact code string and how errors are nested, which is a boundary leak and makes the deferral policy brittle.
Example
The receiver now treats any nested object with { code: 'concurrency-saturated' } as chain admission contention. That duplicates chain error knowledge in an agent-local helper and makes future error wrapping or code movement depend on this private structural search continuing to match.
Suggested direction
Let the chain layer expose the typed admission-contention predicate or translate this at the reconciler boundary, then have the receiver depend on that explicit abstraction rather than peeking through arbitrary cause chains.
For Agents
Move the classification boundary to packages/chain, for example by exporting a predicate like isCurrentFinalizedEvmCallErrorCode(error, 'concurrency-saturated'), or have the RFC64 reconciler return an explicit admission-contention outcome. Preserve the current deferral behavior, but remove agent-owned cause spelunking and string-code matching.
| } from './canonical-graph-scoped-author-seal.js'; | ||
| export * from './catalog-seal-binding.js'; | ||
| export * from './transferred-catalog-bundle.js'; | ||
| export * from './vm-update-convergence.js'; |
There was a problem hiding this comment.
🟡 Issue: Avoid publishing the entire W2 convergence grab bag as one public barrel export
What's wrong
This creates a very wide public core surface from a near-1k-line module that mixes several independent concepts. Because it is exported wholesale, implementation helpers, future-facing types, and closed vocabularies all become API before the owning store/chain integration exists.
Example
FinalizedKaTargetV1 and the outcome vocabularies become public through export * even though the durable store contract is explicitly deferred and there is no production consumer yet. Any later split or narrowing now risks becoming a public API change instead of an internal cleanup.
Suggested direction
Decompose the module into focused files and replace export * with explicit exports for the stable contracts this PR actually needs.
Confidence note
I verified with rg that the new convergence API has no production consumer in this diff outside tests/parity coverage.
For Agents
In packages/core/src/index.ts and packages/core/src/vm-update-convergence.ts, split the convergence code by responsibility and export only the contracts needed by the slice landing in this PR. Keep future store/target/outcome shapes internal until their implementing package lands, then add explicit barrel exports with a narrower surface.
There was a problem hiding this comment.
🟡 Issue: Avoid publishing the entire W2 scratch surface through the core barrel
What's wrong
The wildcard export turns all implementation helpers and future DTOs into public core API at once. That makes later restructuring harder and contradicts the local comment that the durable store contract should not be committed as a V1 surface yet.
Example
FinalizedKaTargetV1, VM_UPDATE_SCAN_OUTCOMES, and VM_UPDATE_COVERAGE_STATES become part of the package barrel even though this PR only uses the UAL helpers externally in tests.
Suggested direction
Use explicit exports and hold back speculative/future-facing contracts until the code that owns them lands.
For Agents
Replace the wildcard export with an explicit export list for the APIs needed now. Keep future store/coverage/result DTOs module-private or in a non-barrel internal module until their owning implementation lands.
There was a problem hiding this comment.
🟡 Issue: Avoid publishing W2 as one near-1k-line catch-all module
What's wrong
This PR introduces a new core module just below the 1k-line smell threshold and gives it seven separate responsibilities, then exposes every export through the package barrel. That is a structural boundary problem: the module is hard to scan, hard to extend without adding more sections, and its internal helper names become API commitments.
Example
A future cursor-only change has to edit the same file that owns UAL parsing and log commitments, and because of export *, helpers like canonicalBlockNumber, normalizeEndpointOrigin, and FinalizedKaTargetV1 become part of the package surface by default.
Suggested direction
Decompose the W2 value-contract code before making it public. Use explicit index exports so internal helpers can stay internal and the public API does not accidentally freeze every implementation detail.
For Agents
Split packages/core/src/vm-update-convergence.ts into cohesive modules such as W2 scalars/errors, scope identity, event positions, page proofs, log commitments, cursors, and KA UAL candidates. Keep behavior and existing tests intact, then replace the barrel export * with explicit exports for the intended public contracts only.
| chainId: config.chainId, | ||
| endpoints: config.endpoints, | ||
| maxConcurrentPerChain: CURRENT_FINALIZED_EVM_READ_MAX_CONCURRENT_PER_CHAIN_V1, | ||
| admission: createLocalNonqueueingAdmissionV1<ChainIdV1>( |
There was a problem hiding this comment.
🟡 Issue: The local read admission split is not verified against the shared snapshot lane
What's wrong
This line preserves a user-facing distinction introduced by the refactor: snapshot scans are process-wide single-lane, but ordinary finalized reads should keep their local four-call gate. The new tests prove the shared snapshot behavior, but not that the read path remains uncoupled from it, so a regression could throttle ordinary reads behind long pinned snapshots without being caught.
Example
Failing-test sketch: start a snapshot scope and park it while it holds the shared finalized snapshot permit; then call createStrictCurrentFinalizedEvmReadV1 for the same chain and assert the read still succeeds. A mistaken switch from local read admission to shared snapshot admission would fail with concurrency-saturated.
Suggested direction
Add an integration-style unit test covering a held snapshot plus a concurrent one-shot read on the same chain, proving the read path still uses local admission rather than the process-wide snapshot registry.
For Agents
Add a chain transport regression test near strict-finalized-snapshot-admission.unit.test.ts or strict-current-finalized-evm-rpc.unit.test.ts that holds a shared snapshot permit and verifies a one-shot read on the same chain is not refused. Keep the existing same-handle read saturation tests intact.
Round 4 review, all four findings — and the 🔴 is a regression I introduced one commit ago while fixing the merge-readiness P1. **🔴 `whenIdle()` reported idle while a head was still pending.** The deferral releases the receiver slot and the queue entry by design, but that left the task in `#pendingByKey` plus a detached timer only — invisible to `#isIdle()`, which tests `active === 0 && queue === 0`. During the retry window `whenIdle()` resolved, so a caller draining the receiver (`synchronizeCurrentCatalogHead()`) could conclude scheduling had settled before the head was applied, staged, not-found or failed. Worth noting my own "releases the slot" test passed through this: it called `whenIdle()` and then also slept, so it was green for the wrong reason. There is now an explicit `#deferred` set — the third state named rather than implied — registered BEFORE the timer is armed, observed by `#isIdle()`, reported in `stats().deferred`, cleared on close, and cleared on the bounded give-up path (which now also resolves idle). **🟡 Deferral was bolted on as hidden state.** Addressed by the above: queued, active and deferred are now three visible states rather than two plus a timer and a mutable field. Contention classification moved OUT of the receiver: the predicate is `isFinalizedChainAdmissionContention`, owned by the chain package whose code `concurrency-saturated` is, and injectable here via `isDeferrableError`. The receiver no longer crawls the shape of errors from a layer it does not own. **🟡 The neutral policy type lived in the shared-registry module.** `FinalizedReadAdmissionV1` is now `EndpointAdmissionPolicyV1<K>` in `nonqueueing-admission.ts`. Both factories return it, so the local gate no longer returns an unnamed structural clone and a future local policy does not have to import its type from the process-wide registry. **🟡 Deferred-head dedupe unverified.** Added: same head re-announced by a second peer while the retry timer is pending must increment `dedupedInFlight` and still produce ONE writer. This pins the `#pendingByKey` lifecycle that the single-announcement tests could not — a regression moving the delete back into `finally` would pass those and fork a second semantic writer here. Verification: 850 chain (whole unit lane) and 29 focused agent tests green. Two mutants, restored byte-identical, with the profiles you would want: removing `#deferred.size === 0` from the idle predicate fails exactly the new idle test; disabling the deferral dispatch fails 6 of 7 and correctly leaves the "ordinary error must not be deferred" negative case passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
Round 4 — all four findings addressed,
|
| cause !== undefined && cause !== null && depth < 8; | ||
| depth += 1) { | ||
| if (typeof cause === 'object' | ||
| && (cause as { code?: unknown }).code === 'concurrency-saturated') { |
There was a problem hiding this comment.
🔴 Bug: The admission classifier matches unrelated concurrency saturation errors
What's wrong
The new deferral path is meant only for the process-wide finalized chain-read lane being busy. Matching every concurrency-saturated error also catches local read limits and snapshot-session reentrancy errors, so real integration bugs or local overload are treated as harmless lane contention, delayed for up to the admission deferral bound, and reported with a misleading failure.
Example
If reconcileHead accidentally starts two session.read() calls at once, the second one rejects with CurrentFinalizedEvmCallErrorV1('concurrency-saturated', 'Current-finalized snapshot permits only one dynamic batch at a time'). The receiver will defer it as a busy chain lane, repeatedly retry without consuming attempts, and eventually report a generic lane wait failure instead of surfacing the real snapshot-session misuse.
Suggested direction
Do not classify solely on the shared concurrency-saturated code. Tag or specialize errors emitted by the process-wide finalized-chain admission path, then have isFinalizedChainAdmissionContention match only that tag.
For Agents
In packages/chain/src/finalized-chain-read-admission.ts, make admission contention distinguishable from other concurrency-saturated errors, for example by marking errors thrown by acquireFinalizedChainRead with a private WeakSet/symbol or a more specific code. Preserve deferral for the shared snapshot lane, and add a test proving snapshot-session read reentrancy and one-shot read saturation are not classified as admission contention by the RFC64 receiver.
There was a problem hiding this comment.
Confirmed and fixed in 80d53fc3e. Verified both emitters you predicted, by reading rather than assuming:
strict-current-finalized-evm-snapshot-rpc.ts:61-64—'Current-finalized snapshot permits only one dynamic batch at a time'current-finalized-evm-call.ts:100-103— the one-shot read's own limit-of-4 gate
So the classifier would have taken a genuine integration bug (two overlapping session.read() calls), deferred it as harmless contention, retried it for the whole deferral bound without consuming attempts, and finally reported "gave up waiting for the lane". A real defect converted into a slow, misleading one — worse than the failure it replaced.
acquireFinalizedChainRead now marks the refusals it actually throws in a module-private WeakSet, and isFinalizedChainAdmissionContention matches membership while walking cause. Identity, not shape, exactly as you suggested: unforgeable from outside, and it holds nothing once the error is collected.
Your finding also invalidated my own test fixture, correctly. The receiver suite forged a concurrency-saturated object, which the new classifier refuses — as it should. Those tests now inject isDeferrableError, which is the receiver's real contract ("defer when the policy says so"). That would have left the default wiring unproven, so there is a new end-to-end test: capture a refusal that genuinely came out of acquireFinalizedChainRead, wrap it the way the precommit does, and assert a receiver with no injected policy defers it — then assert the same receiver fails fast on a look-alike carrying the identical code. That is the pair you asked for.
Proven negative: reverting to code-matching fails exactly three tests — snapshot reentrancy, one-shot read limit, and the forgery case — and nothing else.
854 chain (whole unit lane) and 30 focused agent tests green.
…or code
Round 5 review, one 🔴 — and it is right in a way that matters more than the
code change.
`concurrency-saturated` is a SHARED code with at least two other emitters:
* the snapshot session's reentrancy guard — "permits only one dynamic batch
at a time" (`strict-current-finalized-evm-snapshot-rpc.ts:61-64`);
* the one-shot read's own limit-of-4 gate (`current-finalized-evm-call.ts:100-103`).
Matching on the code therefore classified a genuine integration bug — two
overlapping `session.read()` calls — as harmless lane contention. The RFC64
receiver would defer it, retry it for the whole deferral bound without
consuming attempts, and finally report "gave up waiting for the lane" instead
of surfacing the misuse. A real defect converted into a slow, misleading one.
`acquireFinalizedChainRead` now marks the refusals it actually throws in a
module-private `WeakSet`, and `isFinalizedChainAdmissionContention` matches
membership while walking `cause`. Identity, not shape: a caller constructing a
look-alike cannot be mistaken for the shared lane, and the set holds nothing
once the error is collected.
**This also invalidated my own test fixture, correctly.** The receiver suite
forged a `concurrency-saturated` object, which the new classifier refuses — as
it should. Those tests now inject `isDeferrableError`, which is the receiver's
actual contract ("defer when the policy says so") and decouples them from the
chain package. Because that would have left the DEFAULT wiring unproven, one
new test drives it end to end: capture a refusal that genuinely came out of
`acquireFinalizedChainRead`, wrap it the way the precommit does, and assert a
receiver with no injected policy defers it — then assert the same receiver
fails fast on a look-alike carrying the identical code.
Verification: 854 chain (whole unit lane) and 30 focused agent tests green.
Reverting the classifier to code-matching fails exactly three tests — snapshot
reentrancy, one-shot read limit, and the forgery case — and nothing else.
Source restored byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
Round 5 — one 🔴, fixed in
|
| * the external anchor: without it this is a transcription that can drift | ||
| * silently, and the drift would make W2's fence select the wrong KA. | ||
| */ | ||
| export function buildScopedKnowledgeAssetUal( |
There was a problem hiding this comment.
🟡 Issue: Make the KA UAL rule single-owned instead of parity-tested duplication
What's wrong
This PR adds a second production implementation of the same rootless/legacy KA UAL rule. The dependency direction does not require duplication: agent already depends on core, so core can own the rule and agent can delegate downward. A parity test is useful, but it is a weaker substitute for deleting the duplicate rule entirely.
Example
If the packed-id boundary or UAL spelling changes, maintainers now have to update the agent builder, the new core builder, and the parity test. The test detects drift after the fact but leaves two production implementations to keep in sync.
Suggested direction
Use core as the owner of the shared rootless/legacy UAL model and make agent import that, rather than copying the rule into core and adding a cross-package parity test as the guardrail.
For Agents
Move the resolved legacy/rootless UAL construction into the canonical core KA identity area, preferably near ka-content-scope.ts, and have buildReconciledKnowledgeAssetUal delegate to it. Reuse existing parser/unpack constants where possible, and keep the parity cases as direct unit coverage of the single implementation.
There was a problem hiding this comment.
Agreed, done in 8ef662661 — and my original deferral rationale was wrong, which is worth saying plainly since I argued it on a thread two rounds ago.
I claimed the clean move was out of scope because core cannot import the agent builder. True, but beside the point, exactly as you say: agent → chain → core, so core can own the rule and agent delegates downward. I had conflated that with moving buildKnowledgeAssetUal out of chain — 62 references across ~25 files — when the real change is one function with one production caller (dkg-agent-swm-host.ts:3911).
packages/core/src/ka-ual-identity.ts now owns it. buildReconciledKnowledgeAssetUal and W2's buildScopedKnowledgeAssetUal both delegate; there is no second production implementation left.
One behaviour I deliberately preserved, reversing an earlier choice of mine. The owner lowercases the storage address rather than rejecting mixed case. I had made the W2 builder strict about case — that would have broken the sole caller, which passes the address straight from getDKGKnowledgeAssetsAddress(), and ethers returns the checksummed form (the shipped buildKnowledgeAssetUal has always lowercased it). I read the call site rather than assuming. A builder should emit canonical output; the strictness belongs on the parse side, where accepting mixed case really would let two spellings denote one KA, and it is still enforced there.
Something worth flagging about this refactor, because the naive version silently loses coverage. Deleting the duplicate also deletes the thing the parity test compared against. With one owner, comparing the two entry points is vacuous for any change to the rule itself — mutating the owner moves both sides together and they still agree. I proved it: changing kaId >> 96n to >> 95n in the owner left all 11 tests green.
So the suite now pins the boundary with absolute expected strings at the four ids where the 96-bit split decides the form, and that same mutant is killed. Relative agreement had to become absolute expectation for the coverage to survive the consolidation.
67 core, 854 chain (whole unit lane), 32 focused agent tests green. Core rebuilt before and after the mutant so the agent suite loaded the mutated dist.
Round 6 review, one 🟡 — and my original deferral rationale was wrong. I claimed the clean move was out of scope because `core` cannot import the agent builder. True, but beside the point: the dependency direction is `agent → chain → core`, so `core` can OWN the rule and `agent` can delegate downward. I had conflated that with moving `buildKnowledgeAssetUal` out of `chain` — 62 references across ~25 files — when the actual change is one function with ONE production caller (`dkg-agent-swm-host.ts:3911`). `packages/core/src/ka-ual-identity.ts` now owns the legacy/rootless rule. `buildReconciledKnowledgeAssetUal` and W2's `buildScopedKnowledgeAssetUal` both delegate to it, so there is no second production implementation to keep in step. **Behaviour deliberately preserved, against my own earlier choice.** The owner LOWERCASES the storage address rather than rejecting mixed case. I had made the W2 builder strict about case; that would have broken the sole caller, which passes the address straight from `getDKGKnowledgeAssetsAddress()` — ethers returns the checksummed form, and the shipped `buildKnowledgeAssetUal` has always lowercased it. Verified by reading the call site, not assumed. A builder should emit canonical output; the strictness belongs on the PARSE side, where accepting mixed case really would let two spellings denote one KA, and it is still enforced there. **A mutant caught a defect in my own restructured test.** With the duplicate deleted, comparing the two entry points is VACUOUS for any change to the rule: mutating the owner moves both sides together and they still agree. Changing `kaId >> 96n` to `>> 95n` in the owner left all 11 tests green. The suite now pins the classifier boundary with ABSOLUTE expected strings at the four ids where the 96-bit split decides the form — and that same mutant is killed. That trade-off is inherent to the reviewer's (correct) direction: removing the duplicate removes the thing a parity test compared against, so the coverage has to become absolute rather than relative. Worth stating plainly, because the naive version of this refactor silently loses coverage. Verification: 67 core, 854 chain (whole unit lane), 32 focused agent tests green. Owner-boundary mutant killed, source restored byte-identical, core rebuilt before and after so the agent suite loaded the mutated `dist`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
Round 6 —
|
|
|
||
| function announcement(headDigest: string): Rfc64PublicCatalogHeadAnnouncementV1 { | ||
| return { | ||
| networkId: 'test-network', |
There was a problem hiding this comment.
🔴 Bug: Receiver deferral tests use malformed announcements, collapsing production keys
What's wrong
The tests meant to verify pending-key and scope-lock behavior are not exercising the same identity fields production uses. Because the fixture omits those fields, the scheduler sees undefined for most key components, which can make distinct test announcements collapse into the same synthetic key and gives false confidence about the changed deferral lifecycle.
Example
A regression that only mishandles real catalogHeadObjectDigest or same-scope/different-head scheduling can still pass here, because announcement('0xaa') and announcement('0xbb') do not vary the production head-key fields at all. A stronger failing-test sketch would build two valid announcements with the same scope fields but different catalogHeadObjectDigest, defer the first, schedule the second during the deferral window, and assert the second reconciles before the first retry delay.
Suggested direction
Use a type-correct fixture without as unknown as, vary catalogHeadObjectDigest/signatureVariantDigest for distinct heads, and explicitly assert both duplicate-head dedupe and same-scope different-head scheduling during admission deferral.
For Agents
In packages/agent/test/rfc64-receiver-admission-deferral.test.ts, replace the casted announcement fixture with a real Rfc64PublicCatalogHeadAnnouncementV1 builder using the current wire fields. Preserve tests for duplicate same-head dedupe, and add a same-scope/different-head case that proves the scope lock is released while a task is deferred.
There was a problem hiding this comment.
Confirmed and fixed in da575b511. This one is worse than "weak fixture" — the dedupe test could not fail.
headKey() is built from catalogHeadObjectDigest, signatureVariantDigest, authorAddress, catalogEra and catalogVersion. My fixture was cast with as unknown as and set headDigest/authorDid, which are not fields on the wire type at all. So every announcement produced the same key of undefineds: announcement('0xaa') and announcement('0xbb') were literally the same head to the scheduler, and the pending-key test passed regardless of what the deferral lifecycle did to #pendingByKey.
Now a type-correct Rfc64PublicCatalogHeadAnnouncementV1 with no cast, varying catalogHeadObjectDigest. I also added a test that guards the premise directly rather than trusting it — two announcements must differ in the field the key actually reads — because the failure mode here was precisely that the premise was false and nothing said so.
Your suggested case is in as well: same scope, different head, scheduled during the deferral window; it must not dedupe and must run before the first head's retry. That is the assertion that proves the deferral releases the SCOPE lock and not just the concurrency slot.
Proven negative: making the fixture stop varying that field fails two tests, where before it would have failed none.
| // Not this head's fault and not this provider's fault: give the | ||
| // attempt back and let the task wait for the lane outside the slot. | ||
| attemptsByProvider.set(provider.key, providerAttempt - 1); | ||
| return 'defer-admission'; |
There was a problem hiding this comment.
🟡 Issue: Admission deferral resets provider retry state
What's wrong
The new deferral path exits #runTask and requeues the same task, but provider retry bookkeeping is local to that invocation. That means a busy finalized-read lane can erase earlier ordinary provider failures, so maxAttempts is no longer a true per-provider bound whenever contention happens between failures.
Example
With maxAttempts = 3, a provider can fail twice with an ordinary error, then hit chain-lane contention. The deferral path requeues the same task, but the next run starts with attemptsByProvider back at 0, so that provider can get three more ordinary attempts instead of the one remaining attempt. Repeated contention can multiply provider retries far beyond the configured limit.
Suggested direction
Keep attemptsByProvider, notFoundProviders, and the provider cursor with the task across deferrals, while still rolling back only the deferrable attempt.
Confidence note
The issue depends on a task seeing both ordinary provider failures and an admission-contention deferral, but that is exactly the mixed failure mode this scheduler now handles.
For Agents
Move per-task retry state that must survive admission deferrals out of #runTask and onto ReceiverTaskV1, or otherwise carry it through #scheduleAdmissionRetry. Preserve the rule that admission contention itself does not consume an attempt, and add a test where ordinary failures before and after a deferral still total at most maxAttempts per provider.
There was a problem hiding this comment.
Confirmed and fixed in da575b511. Your confidence note is right that it needs the mixed sequence — and that sequence is exactly what this scheduler now invites, so it is worth closing rather than filing.
attemptsByProvider, notFoundProviders and the provider cursor were locals in #runTask. A deferral exits and requeues, so the next run started them at zero: with maxAttempts = 3, a provider could fail twice, hit a busy lane, and receive a fresh three attempts. Repeated contention multiplied retries without bound, which is the opposite of what the deferral was for.
They now live on ReceiverTaskV1 and survive deferrals. Contention still gives back only its own attempt — and when that was the provider's first, the entry is deleted rather than left at zero, so the tally is identical whichever way it got there.
The new test drives the exact sequence you described: two ordinary failures, one contention deferral, then completion at the configured bound. Total ordinary attempts is 3, not 2 + 3. Reverting the state to per-run fails exactly that test.
…re that could not fail
Round 7 review, both findings on code I added.
**🔴 The deferral tests used malformed announcements, so the dedupe test could
not fail.** `headKey()` is built from `catalogHeadObjectDigest`,
`signatureVariantDigest`, `authorAddress`, `catalogEra` and `catalogVersion`.
My fixture was cast with `as unknown as` and set `headDigest`/`authorDid` —
fields that do not exist on the wire type — so every announcement produced the
same key of `undefined`s. `announcement('0xaa')` and `announcement('0xbb')` were
the SAME head to the scheduler. The pending-key test therefore passed no matter
what the deferral lifecycle did.
Replaced with a type-correct `Rfc64PublicCatalogHeadAnnouncementV1` that varies
`catalogHeadObjectDigest`, plus a test that guards the premise directly: two
announcements must differ in the field the key actually reads. Zeroing that
field again now fails two tests instead of silently passing all of them.
Added the same-scope/different-head case: a second head in the same catalog
scope, scheduled during the deferral window, must not dedupe and must run before
the first head's retry — proving the deferral releases the SCOPE lock, not only
the concurrency slot.
**🟡 Deferral reset provider retry state.** `attemptsByProvider`,
`notFoundProviders` and the provider cursor were locals in `#runTask`. A
deferral exits and requeues, so the next run restarted them at zero: with
`maxAttempts = 3` a provider could fail twice, hit a busy lane, and then get a
fresh three attempts. Repeated contention multiplied retries without bound.
They now live on the task and survive deferrals, while contention still gives
back only its own attempt — and if that was the provider's first, the entry is
deleted rather than left at zero, so the tally is identical either way. The new
test drives exactly the mixed sequence: two ordinary failures, one contention,
then completion at the configured bound — 3 total attempts, not 2 + 3.
Verification: 51 agent tests across six suites green, including the receiver's
pre-existing suite. Two mutants, restored byte-identical: reverting the retry
state to per-run fails exactly the bound test; making the fixture stop varying
the key field fails two.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
Round 7 —
|
…construct RFC64's finalized-VM precommit could not construct its snapshot scope on ANY shipped EVM network. Measured against the merged validator: testnet pool=3 THROWS -> Strict current-finalized RPC requires 1..2 distinct endpoints mainnet-base pool=3 THROWS -> ... mainnet-gnosis pool=3 THROWS -> ... The finalized-read pool is `resolveRpcUrls(chain.rpcUrl, chain.rpcUrls)` — primary PLUS backups (`dkg-agent-rfc64-catalog.ts:777`) — which is three URLs on every EVM network. `snapshotNormalizedEndpoints` rejects more than `CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1 = 2` distinct endpoints, and the precommit passes the pool through unsliced. No test covered it: grepping the error message across the chain and agent suites returned zero hits. The fix is what plan §5.2 always specified — session selection. A new `selectStrictFinalizedEndpointSessionV1` reduces the pool to at most two distinct provider ORIGINS before the ceiling applies, so the ceiling is satisfied by construction rather than by rejection. Provider identity is core's `normalizeEndpointOrigin` — imported, not copied. It is the same predicate `canonicalPageProof` uses to decide `dual-origin-corroborated`, so the two layers cannot drift. (A working revision of this branch forked that rule into chain on the theory that core was not importable; it never reached a commit, so the diff below is purely additive. Core exports the function from the package root and chain already depends on core in 26 files, so the premise was simply wrong.) Note what this is NOT: base never computed an origin at all. `url.href` was the DEDUP key, and that href dedup is still here, unchanged. Origin identity is a coarser layer added on top of it, not a replacement. What inheriting core's rule actually changes, through the only production caller, is the byte bound and the error class below — NOT parsing strictness. The config's `normalizeEndpoint` runs `input.trim()` then `new URL(...)` and returns `url.href`, so the selector only ever sees an already-parsed canonical href; endpoints prefixed with U+00A0/U+FEFF/U+2028/U+3000 all normalize to `https://a.example.com/` and reach selection identically before and after. Those inputs could only have mattered to a direct caller of the exported selector, which is surface this change removes. The attempt ceiling is asserted where it is attested. `CONTROL_EIP1271_MAX_ATTEMPTS_V1` is verified at `current-finalized-evm-call.ts:182`, but it is NOT self-enforcing: the runner never reads `maxAttempts` and instead attempts one endpoint per entry of `profile.endpoints` (`strict-current-finalized-evm-lifecycle.ts:104`). Since this change removes the validator's count rejection, the config keeps an explicit postcondition on the selection result — otherwise an attested "2" would depend on a `>=` inside another module. (A separate constant, `CONTROL_EIP1271_ENDPOINT_ATTEMPT_POLICY_V1`, checked at `:185`, is the policy STRING; an earlier version of this message and both docblocks conflated the two.) The bound is NOT a parameter. Only one caller exists and it always passes the canonical constant, so a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only ABOVE the shipped value — which then needed an explicit ordering repair to fix. Reading the constant directly deletes the parameter, its validation guard, the ordering repair, and that whole failure mode. At a fixed bound of 2 the repair is provably unnecessary: pass 2 runs only when pass 1 selected fewer than the bound, which means the pool held exactly one distinct origin, so pass 1 holds index 0 and pass 2 appends index 1. Ordering is the contract: selection runs AFTER `assertConfigDataProperties` and BEFORE normalization. Before descriptor validation it would re-execute attacker-supplied getters (the hole closed in #2051); after normalization it would never run, because normalization already threw. Both directions have a test and a mutant. Two behaviour changes beyond the intended widening, both from adopting core's bounded rule, both stated because neither is obvious from the diff: * Endpoints are now capped at 4096 bytes. The boundary is exact and pinned by test: 4096 accepted, 4097 rejected. Base had NO length bound and accepted 4097, so this is a narrowing. No real RPC URL is affected. * The configured pool is now capped at 32 entries. Removing the `> 2` rejection removed the only bound on pool size, and both the href dedup and selection scan linearly inside a loop (~6s at 16k entries, measured), on the synchronous config-construction path. Consequences stated rather than buried: * This WIDENS accepted configs. A 3-URL pool with two same-origin aliases was rejected and now constructs. Asserted deliberately. * Selection backfills spare slots with same-origin URLs rather than shrinking the pool, so a load-balancer config carrying per-key URLs keeps its failover. Callers needing real provider diversity must count `selectedOrigins`, never `selected.length`. This function deliberately exposes NO diversity scalar: an earlier revision returned `distinctOriginCount` computed over the whole pool BEFORE truncation while the module header pointed corroboration consumers straight at it — on every shipped EVM network that reported 3 for a session that dials 2 origins, and `canonicalPageProof` requires EXACTLY two distinct origins. A derived value that can disagree with `selectedOrigins` is a trap, so the array is now the single truth. * `selectStrictFinalizedEndpointSessionV1` is NOT exported from the package root. Every consumer imports it by relative path, so exporting it would have published surface with no caller. * The third shipped endpoint (`*.drpc.org`) is now permanently unreachable by strict finalized reads. Selection is health-blind and configuration-ordered, so a degraded primary is still dialled first. Pinned by assertion in the new agent test so it is a known limitation rather than a silent one. * `/api/status` still reports `resolveRpcUrls(...).length` = 3 while a strict session dials 2. Not reconciled here. This does NOT enable dual-origin corroboration. A scope still pins one endpoint for its lifetime; holding two under one lease needs a runner change, deferred to the page-transport PR so §10.3 is not planned against a capability that does not exist. `strict-current-finalized-evm-rpc.unit.test.ts` had one case asserting a 3-endpoint config throws — that case encoded the defect. It is now a positive assertion that selection happens; its other five rejection cases are unchanged. No `instanceof TypeError` rethrow guard sits in the conversion: core routes every fault through `fail()`, including the `new URL()` parse failure it catches and re-fails, so there is no native `TypeError` path to preserve. A guard for it was dead code — deleting it left the suite green. Element faults are reported as `TypeError`. Core throws `VmUpdateConvergenceError`, which extends `Error`, and the config's translating try/catch closes BEFORE the selection call — so without conversion, core's error escaped a validator whose entire documented contract is `TypeError`, carrying a `vm-update:` message for an RPC-config fault. The pre-existing `toThrow(TypeError)` assertion could not catch this: no fixture in any suite reached the 4096-byte bound. The new case is message-pinned rather than type-pinned for that reason. Only ONE fault class actually reaches that conversion from the config. Chain's own `normalizeEndpoint` catches non-strings, null/undefined, objects and unparseable URLs inside the loop before selection runs, each with its own `TypeError`; measured end-to-end, the byte bound is the only element fault that gets through. So the conversion is defense-in-depth rather than the contract's load-bearing layer — and the message-pinned oversize row is not merely good practice, it is the ONLY end-to-end assertion that exercises the conversion at all. Without it the conversion would be covered solely by direct-selector tests. The conversion is at the core call site rather than a pre-check in chain's own `normalizeEndpoint`, which would avoid the conversion entirely by rejecting oversize endpoints before core sees them. That was considered and rejected: core's `MAX_SCALAR_BYTES` is module-private, so a pre-check must hardcode 4096 and would silently disagree the moment core changes it — reintroducing the exact drift class that importing core's function removes. Converting whatever core decides needs no knowledge of the bound. An empty pool now fails closed too; it was the one input the function did not guard. Verification: * Chain unit lane 44 files, 882 tests, 1 skipped. A rotating handful of real-timer loopback tests fail under machine load: FIVE different tests observed across runs, each passing on a later run. They are load-dependent rather than lane-dependent — running the file alone does not reliably fix them, so "passes solo" is not the argument. Not attributable here for two independent reasons. First, selection is the IDENTITY function for every config involved, checked individually: three carry a single endpoint, one carries two distinct-port origins, and `applies the two-endpoint ceiling after normalized endpoint deduplication` carries three entries that href-dedup to two distinct origins. None exceeds the bound, so no truncation occurs and the endpoint list is byte-identical before and after. Second, independently reproduced in review: the file passes at `fa9dca15e` (30/30) and at `c557f1a61` (31/31) on one machine minutes apart, which rules the change out directly rather than arguing from timing. Later commits add only input guards and touch no timing path. * Nine mutants at this revision (fifteen across the earlier rounds, two of which targeted code this simplification deletes), each killed by its NAMED test, every restore byte-identical, baseline green before each — plus independent auditor mutants from review. Three assertions added in the final round because a mutant proved they could not fail: the pool cap was one-sided (any value in [3,32] was indistinguishable), the `{ cause }` fix was entirely unasserted (deleting it left the suite green), and the default-port test asserted only `toHaveLength(2)`, which backfill guarantees whether or not the port is part of provider identity. * The runner refuses to start unless the tree is green first. It fired: a twelve-mutant run aborted with "tree not green before mutation" because a concurrent lane had pushed a load-sensitive test to 14.3s. Without it that run would have produced six results indistinguishable from real kills. * Agent seam suites green against a force-rebuilt `dist` (`tsbuildinfo` removed — `composite: true` otherwise skips emit and can leave stale or mutated code in `dist` while reporting success). * New agent test drives the REAL precommit with the REAL shipped testnet pool and stubs only global `fetch`. Fail-before proved by reverting the config to its base body: reverted -> exit 1 with "requires 1..2 distinct endpoints" and zero dials; restored byte-identical -> exit 0, two endpoints dialled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…construct RFC64's finalized-VM precommit could not construct its snapshot scope on ANY shipped EVM network. Measured against the merged validator: testnet pool=3 THROWS -> Strict current-finalized RPC requires 1..2 distinct endpoints mainnet-base pool=3 THROWS -> ... mainnet-gnosis pool=3 THROWS -> ... The finalized-read pool is `resolveRpcUrls(chain.rpcUrl, chain.rpcUrls)` — primary PLUS backups (`dkg-agent-rfc64-catalog.ts:777`) — which is three URLs on every EVM network. `snapshotNormalizedEndpoints` rejects more than `CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1 = 2` distinct endpoints, and the precommit passes the pool through unsliced. No test covered it: grepping the error message across the chain and agent suites returned zero hits. The fix is what plan §5.2 always specified — session selection. A new `selectStrictFinalizedEndpointSessionV1` reduces the pool to at most two distinct provider ORIGINS before the ceiling applies, so the ceiling is satisfied by construction rather than by rejection. Provider identity is core's `normalizeEndpointOrigin` — imported, not copied. It is the same predicate `canonicalPageProof` uses to decide `dual-origin-corroborated`, so the two layers cannot drift. (A working revision of this branch forked that rule into chain on the theory that core was not importable; it never reached a commit, so the diff below is purely additive. Core exports the function from the package root and chain already depends on core in 26 files, so the premise was simply wrong.) Note what this is NOT: base never computed an origin at all. `url.href` was the DEDUP key, and that href dedup is still here, unchanged. Origin identity is a coarser layer added on top of it, not a replacement. What inheriting core's rule actually changes, through the only production caller, is the byte bound and the error class below — NOT parsing strictness. The config's `normalizeEndpoint` runs `input.trim()` then `new URL(...)` and returns `url.href`, so the selector only ever sees an already-parsed canonical href; endpoints prefixed with U+00A0/U+FEFF/U+2028/U+3000 all normalize to `https://a.example.com/` and reach selection identically before and after. Those inputs could only have mattered to a direct caller of the exported selector, which is surface this change removes. The attempt ceiling is asserted where it is attested. `CONTROL_EIP1271_MAX_ATTEMPTS_V1` is verified at `current-finalized-evm-call.ts:182`, but it is NOT self-enforcing: the runner never reads `maxAttempts` and instead attempts one endpoint per entry of `profile.endpoints` (`strict-current-finalized-evm-lifecycle.ts:104`). Since this change removes the validator's count rejection, the config keeps an explicit postcondition on the selection result — otherwise an attested "2" would depend on a `>=` inside another module. (A separate constant, `CONTROL_EIP1271_ENDPOINT_ATTEMPT_POLICY_V1`, checked at `:185`, is the policy STRING; an earlier version of this message and both docblocks conflated the two.) The bound is NOT a parameter. Only one caller exists and it always passes the canonical constant, so a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only ABOVE the shipped value — which then needed an explicit ordering repair to fix. Reading the constant directly deletes the parameter, its validation guard, the ordering repair, and that whole failure mode. At a fixed bound of 2 the repair is provably unnecessary: pass 2 runs only when pass 1 selected fewer than the bound, which means the pool held exactly one distinct origin, so pass 1 holds index 0 and pass 2 appends index 1. Ordering is the contract: selection runs AFTER `assertConfigDataProperties` and BEFORE normalization. Before descriptor validation it would re-execute attacker-supplied getters (the hole closed in #2051); after normalization it would never run, because normalization already threw. Both directions have a test and a mutant. Core's predicate is handed the ORIGIN, never the full dial URL. Core bounds its input at the canonical scalar limit, so passing the whole URL imposed a 4096-byte cap that RPC endpoints never had — a config carrying a long path or query credential constructed at base and would have begun throwing. Deriving the origin from the already-parsed URL keeps core as the single source of the origin rule while confining its bound to a string that is always short. Regression-tested in both directions, and a mutant that reinstates the full-URL form dies on it. One behaviour change beyond the intended widening: * The configured pool is now capped at 32 entries. Removing the `> 2` rejection removed the only bound on pool size, and both the href dedup and selection scan linearly inside a loop (~6s at 16k entries, measured), on the synchronous config-construction path. Selection is a TWO-SLOT policy, not a generic selector: first configured URL, then the first later URL with a different origin, else the second configured URL. The bound is fixed, there is one caller, and a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only above the shipped value. A test pins the ceiling at 2 so raising it fails loudly rather than silently leaving this policy at two. Preferring a distinct origin CAN skip an earlier same-origin URL — `[KEY_A, KEY_B, backup]` selects `[KEY_A, backup]` and never attempts `KEY_B`. That is a deliberate trade, now pinned by test rather than left implicit: two credentials behind one load balancer share a provider, so a provider outage or account-level rate limit takes both out together and leaves a same-origin pair with nothing working, while a distinct origin survives it. With two slots and three endpoints, every possible policy skips some configured URL. Consequences stated rather than buried: * This WIDENS accepted configs. A 3-URL pool with two same-origin aliases was rejected and now constructs. Asserted deliberately. * Selection backfills spare slots with same-origin URLs rather than shrinking the pool, so a load-balancer config carrying per-key URLs keeps its failover. Callers needing real provider diversity must count `selectedOrigins`, never `selected.length`. This function deliberately exposes NO diversity scalar: an earlier revision returned `distinctOriginCount` computed over the whole pool BEFORE truncation while the module header pointed corroboration consumers straight at it — on every shipped EVM network that reported 3 for a session that dials 2 origins, and `canonicalPageProof` requires EXACTLY two distinct origins. A derived value that can disagree with `selectedOrigins` is a trap, so the array is now the single truth. * `selectStrictFinalizedEndpointSessionV1` is NOT exported from the package root. Every consumer imports it by relative path, so exporting it would have published surface with no caller. * The third shipped endpoint (`*.drpc.org`) is now permanently unreachable by strict finalized reads. Selection is health-blind and configuration-ordered, so a degraded primary is still dialled first. Pinned by assertion in the new agent test so it is a known limitation rather than a silent one. * `/api/status` still reports `resolveRpcUrls(...).length` = 3 while a strict session dials 2. Not reconciled here. This does NOT enable dual-origin corroboration. A scope still pins one endpoint for its lifetime; holding two under one lease needs a runner change, deferred to the page-transport PR so §10.3 is not planned against a capability that does not exist. `strict-current-finalized-evm-rpc.unit.test.ts` had one case asserting a 3-endpoint config throws — that case encoded the defect. It is now a positive assertion that selection happens; its other five rejection cases are unchanged. No `instanceof TypeError` rethrow guard sits in the conversion: core routes every fault through `fail()`, including the `new URL()` parse failure it catches and re-fails, so there is no native `TypeError` path to preserve. A guard for it was dead code — deleting it left the suite green. Element faults are reported as `TypeError`. Core throws `VmUpdateConvergenceError`, which extends `Error`, and the config's translating try/catch closes BEFORE the selection call — so without conversion, core's error escaped a validator whose entire documented contract is `TypeError`, carrying a `vm-update:` message for an RPC-config fault. The pre-existing `toThrow(TypeError)` assertion could not catch this: no fixture in any suite reached the 4096-byte bound. The new case is message-pinned rather than type-pinned for that reason. Only ONE fault class actually reaches that conversion from the config. Chain's own `normalizeEndpoint` catches non-strings, null/undefined, objects and unparseable URLs inside the loop before selection runs, each with its own `TypeError`; measured end-to-end, the byte bound is the only element fault that gets through. So the conversion is defense-in-depth rather than the contract's load-bearing layer — and the message-pinned oversize row is not merely good practice, it is the ONLY end-to-end assertion that exercises the conversion at all. Without it the conversion would be covered solely by direct-selector tests. The conversion is at the core call site rather than a pre-check in chain's own `normalizeEndpoint`, which would avoid the conversion entirely by rejecting oversize endpoints before core sees them. That was considered and rejected: core's `MAX_SCALAR_BYTES` is module-private, so a pre-check must hardcode 4096 and would silently disagree the moment core changes it — reintroducing the exact drift class that importing core's function removes. Converting whatever core decides needs no knowledge of the bound. An empty pool now fails closed too; it was the one input the function did not guard. Verification: * Chain unit lane 44 files, 882 tests, 1 skipped. A rotating handful of real-timer loopback tests fail under machine load: FIVE different tests observed across runs, each passing on a later run. They are load-dependent rather than lane-dependent — running the file alone does not reliably fix them, so "passes solo" is not the argument. Not attributable here for two independent reasons. First, selection is the IDENTITY function for every config involved, checked individually: three carry a single endpoint, one carries two distinct-port origins, and `applies the two-endpoint ceiling after normalized endpoint deduplication` carries three entries that href-dedup to two distinct origins. None exceeds the bound, so no truncation occurs and the endpoint list is byte-identical before and after. Second, independently reproduced in review: the file passes at `fa9dca15e` (30/30) and at `c557f1a61` (31/31) on one machine minutes apart, which rules the change out directly rather than arguing from timing. Later commits add only input guards and touch no timing path. * Ten mutants at this revision (fifteen across the earlier rounds, two of which targeted code this simplification deletes), each killed by its NAMED test, every restore byte-identical, baseline green before each — plus independent auditor mutants from review. Three assertions added in the final round because a mutant proved they could not fail: the pool cap was one-sided (any value in [3,32] was indistinguishable), the `{ cause }` fix was entirely unasserted (deleting it left the suite green), and the default-port test asserted only `toHaveLength(2)`, which backfill guarantees whether or not the port is part of provider identity. * The runner refuses to start unless the tree is green first. It fired: a twelve-mutant run aborted with "tree not green before mutation" because a concurrent lane had pushed a load-sensitive test to 14.3s. Without it that run would have produced six results indistinguishable from real kills. * Agent seam suites green against a force-rebuilt `dist` (`tsbuildinfo` removed — `composite: true` otherwise skips emit and can leave stale or mutated code in `dist` while reporting success). * New agent test drives the REAL precommit with the REAL shipped testnet pool and stubs only global `fetch`. Fail-before proved by reverting the config to its base body: reverted -> exit 1 with "requires 1..2 distinct endpoints" and zero dials; restored byte-identical -> exit 0, two endpoints dialled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…construct RFC64's finalized-VM precommit could not construct its snapshot scope on ANY shipped EVM network. Measured against the merged validator: testnet pool=3 THROWS -> Strict current-finalized RPC requires 1..2 distinct endpoints mainnet-base pool=3 THROWS -> ... mainnet-gnosis pool=3 THROWS -> ... The finalized-read pool is `resolveRpcUrls(chain.rpcUrl, chain.rpcUrls)` — primary PLUS backups (`dkg-agent-rfc64-catalog.ts:777`) — which is three URLs on every EVM network. `snapshotNormalizedEndpoints` rejects more than `CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1 = 2` distinct endpoints, and the precommit passes the pool through unsliced. No test covered it: grepping the error message across the chain and agent suites returned zero hits. The fix is what plan §5.2 always specified — session selection. A new `selectStrictFinalizedEndpointSessionV1` reduces the pool to at most two distinct provider ORIGINS before the ceiling applies, so the ceiling is satisfied by construction rather than by rejection. Provider identity is core's `normalizeEndpointOrigin` — imported, not copied. It is the same predicate `canonicalPageProof` uses to decide `dual-origin-corroborated`, so the two layers cannot drift. (A working revision of this branch forked that rule into chain on the theory that core was not importable; it never reached a commit, so the diff below is purely additive. Core exports the function from the package root and chain already depends on core in 26 files, so the premise was simply wrong.) Note what this is NOT: base never computed an origin at all. `url.href` was the DEDUP key, and that href dedup is still here, unchanged. Origin identity is a coarser layer added on top of it, not a replacement. What inheriting core's rule actually changes, through the only production caller, is the byte bound and the error class below — NOT parsing strictness. The config's `normalizeEndpoint` runs `input.trim()` then `new URL(...)` and returns `url.href`, so the selector only ever sees an already-parsed canonical href; endpoints prefixed with U+00A0/U+FEFF/U+2028/U+3000 all normalize to `https://a.example.com/` and reach selection identically before and after. Those inputs could only have mattered to a direct caller of the exported selector, which is surface this change removes. The attempt ceiling is asserted where it is attested. `CONTROL_EIP1271_MAX_ATTEMPTS_V1` is verified at `current-finalized-evm-call.ts:182`, but it is NOT self-enforcing: the runner never reads `maxAttempts` and instead attempts one endpoint per entry of `profile.endpoints` (`strict-current-finalized-evm-lifecycle.ts:104`). Since this change removes the validator's count rejection, the config keeps an explicit postcondition on the selection result — otherwise an attested "2" would depend on a `>=` inside another module. (A separate constant, `CONTROL_EIP1271_ENDPOINT_ATTEMPT_POLICY_V1`, checked at `:185`, is the policy STRING; an earlier version of this message and both docblocks conflated the two.) The bound is NOT a parameter. Only one caller exists and it always passes the canonical constant, so a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only ABOVE the shipped value — which then needed an explicit ordering repair to fix. Reading the constant directly deletes the parameter, its validation guard, the ordering repair, and that whole failure mode. At a fixed bound of 2 the repair is provably unnecessary: pass 2 runs only when pass 1 selected fewer than the bound, which means the pool held exactly one distinct origin, so pass 1 holds index 0 and pass 2 appends index 1. Ordering is the contract: selection runs AFTER `assertConfigDataProperties` and BEFORE normalization. Before descriptor validation it would re-execute attacker-supplied getters (the hole closed in #2051); after normalization it would never run, because normalization already threw. Both directions have a test and a mutant. Core's predicate is handed the ORIGIN, never the full dial URL. Core bounds its input at the canonical scalar limit, so passing the whole URL imposed a 4096-byte cap that RPC endpoints never had — a config carrying a long path or query credential constructed at base and would have begun throwing. Deriving the origin from the already-parsed URL keeps core as the single source of the origin rule while confining its bound to a string that is always short. Regression-tested in both directions, and a mutant that reinstates the full-URL form dies on it. One behaviour change beyond the intended widening: * The configured pool is now capped at 32 entries. Removing the `> 2` rejection removed the only bound on pool size, and both the href dedup and selection scan linearly inside a loop (~6s at 16k entries, measured), on the synchronous config-construction path. Selection is a TWO-SLOT policy, not a generic selector: first configured URL, then the first later URL with a different origin, else the second configured URL. The bound is fixed, there is one caller, and a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only above the shipped value. A test pins the ceiling at 2 so raising it fails loudly rather than silently leaving this policy at two. Preferring a distinct origin CAN skip an earlier same-origin URL — `[KEY_A, KEY_B, backup]` selects `[KEY_A, backup]` and never attempts `KEY_B`. That is a deliberate trade, now pinned by test rather than left implicit: two credentials behind one load balancer share a provider, so a provider outage or account-level rate limit takes both out together and leaves a same-origin pair with nothing working, while a distinct origin survives it. With two slots and three endpoints, every possible policy skips some configured URL. Consequences stated rather than buried: * This WIDENS accepted configs. A 3-URL pool with two same-origin aliases was rejected and now constructs. Asserted deliberately. * Selection backfills spare slots with same-origin URLs rather than shrinking the pool, so a load-balancer config carrying per-key URLs keeps its failover. Callers needing real provider diversity must count `selectedOrigins`, never `selected.length`. This function deliberately exposes NO diversity scalar: an earlier revision returned `distinctOriginCount` computed over the whole pool BEFORE truncation while the module header pointed corroboration consumers straight at it — on every shipped EVM network that reported 3 for a session that dials 2 origins, and `canonicalPageProof` requires EXACTLY two distinct origins. A derived value that can disagree with `selectedOrigins` is a trap, so the array is now the single truth. * `selectStrictFinalizedEndpointSessionV1` is NOT exported from the package root. Every consumer imports it by relative path, so exporting it would have published surface with no caller. * The third shipped endpoint (`*.drpc.org`) is now permanently unreachable by strict finalized reads. Selection is health-blind and configuration-ordered, so a degraded primary is still dialled first. Pinned by assertion in the new agent test so it is a known limitation rather than a silent one. * `/api/status` still reports `resolveRpcUrls(...).length` = 3 while a strict session dials 2. Not reconciled here. This does NOT enable dual-origin corroboration. A scope still pins one endpoint for its lifetime; holding two under one lease needs a runner change, deferred to the page-transport PR so §10.3 is not planned against a capability that does not exist. `strict-current-finalized-evm-rpc.unit.test.ts` had one case asserting a 3-endpoint config throws — that case encoded the defect. It is now a positive assertion that selection happens; its other five rejection cases are unchanged. No `instanceof TypeError` rethrow guard sits in the conversion: core routes every fault through `fail()`, including the `new URL()` parse failure it catches and re-fails, so there is no native `TypeError` path to preserve. A guard for it was dead code — deleting it left the suite green. Element faults are reported as `TypeError`. Core throws `VmUpdateConvergenceError`, which extends `Error`, and the config's translating try/catch closes BEFORE the selection call — so without conversion, core's error escaped a validator whose entire documented contract is `TypeError`, carrying a `vm-update:` message for an RPC-config fault. The pre-existing `toThrow(TypeError)` assertion could not catch this: no fixture in any suite reached the 4096-byte bound. The new case is message-pinned rather than type-pinned for that reason. Only ONE fault class actually reaches that conversion from the config. Chain's own `normalizeEndpoint` catches non-strings, null/undefined, objects and unparseable URLs inside the loop before selection runs, each with its own `TypeError`; measured end-to-end, the byte bound is the only element fault that gets through. So the conversion is defense-in-depth rather than the contract's load-bearing layer — and the message-pinned oversize row is not merely good practice, it is the ONLY end-to-end assertion that exercises the conversion at all. Without it the conversion would be covered solely by direct-selector tests. The conversion is at the core call site rather than a pre-check in chain's own `normalizeEndpoint`, which would avoid the conversion entirely by rejecting oversize endpoints before core sees them. That was considered and rejected: core's `MAX_SCALAR_BYTES` is module-private, so a pre-check must hardcode 4096 and would silently disagree the moment core changes it — reintroducing the exact drift class that importing core's function removes. Converting whatever core decides needs no knowledge of the bound. An empty pool now fails closed too; it was the one input the function did not guard. RFC64 precommit fixtures are shared, not copied. The shipped-pool regression varies exactly one field (`rpcEndpoints`), so the plan, accepted-policy snapshot and base options now live in `test/support/rfc64-finalized-vm-precommit-fixture.ts` and both precommit suites consume them: -161 lines across the two test files. Previously a change to the plan or policy shape needed synchronized edits in two places before either suite's actual assertion could run. Verification: * Chain unit lane 44 files, 882 tests, 1 skipped. A rotating handful of real-timer loopback tests fail under machine load: FIVE different tests observed across runs, each passing on a later run. They are load-dependent rather than lane-dependent — running the file alone does not reliably fix them, so "passes solo" is not the argument. Not attributable here for two independent reasons. First, selection is the IDENTITY function for every config involved, checked individually: three carry a single endpoint, one carries two distinct-port origins, and `applies the two-endpoint ceiling after normalized endpoint deduplication` carries three entries that href-dedup to two distinct origins. None exceeds the bound, so no truncation occurs and the endpoint list is byte-identical before and after. Second, independently reproduced in review: the file passes at `fa9dca15e` (30/30) and at `c557f1a61` (31/31) on one machine minutes apart, which rules the change out directly rather than arguing from timing. Later commits add only input guards and touch no timing path. * Ten mutants at this revision (fifteen across the earlier rounds, two of which targeted code this simplification deletes), each killed by its NAMED test, every restore byte-identical, baseline green before each — plus independent auditor mutants from review. Three assertions added in the final round because a mutant proved they could not fail: the pool cap was one-sided (any value in [3,32] was indistinguishable), the `{ cause }` fix was entirely unasserted (deleting it left the suite green), and the default-port test asserted only `toHaveLength(2)`, which backfill guarantees whether or not the port is part of provider identity. * The runner refuses to start unless the tree is green first. It fired: a twelve-mutant run aborted with "tree not green before mutation" because a concurrent lane had pushed a load-sensitive test to 14.3s. Without it that run would have produced six results indistinguishable from real kills. * Agent seam suites green against a force-rebuilt `dist` (`tsbuildinfo` removed — `composite: true` otherwise skips emit and can leave stale or mutated code in `dist` while reporting success). * New agent test drives the REAL precommit with the REAL shipped testnet pool and stubs only global `fetch`. Fail-before proved by reverting the config to its base body: reverted -> exit 1 with "requires 1..2 distinct endpoints" and zero dials; restored byte-identical -> exit 0, two endpoints dialled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…construct RFC64's finalized-VM precommit could not construct its snapshot scope on ANY shipped EVM network. Measured against the merged validator: testnet pool=3 THROWS -> Strict current-finalized RPC requires 1..2 distinct endpoints mainnet-base pool=3 THROWS -> ... mainnet-gnosis pool=3 THROWS -> ... The finalized-read pool is `resolveRpcUrls(chain.rpcUrl, chain.rpcUrls)` — primary PLUS backups (`dkg-agent-rfc64-catalog.ts:777`) — which is three URLs on every EVM network. `snapshotNormalizedEndpoints` rejects more than `CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1 = 2` distinct endpoints, and the precommit passes the pool through unsliced. No test covered it: grepping the error message across the chain and agent suites returned zero hits. The fix is what plan §5.2 always specified — session selection. A new `selectStrictFinalizedEndpointSessionV1` reduces the pool to at most two distinct provider ORIGINS before the ceiling applies, so the ceiling is satisfied by construction rather than by rejection. Provider identity is core's `normalizeEndpointOrigin` — imported, not copied. It is the same predicate `canonicalPageProof` uses to decide `dual-origin-corroborated`, so the two layers cannot drift. (A working revision of this branch forked that rule into chain on the theory that core was not importable; it never reached a commit, so the diff below is purely additive. Core exports the function from the package root and chain already depends on core in 26 files, so the premise was simply wrong.) Note what this is NOT: base never computed an origin at all. `url.href` was the DEDUP key, and that href dedup is still here, unchanged. Origin identity is a coarser layer added on top of it, not a replacement. What inheriting core's rule actually changes, through the only production caller, is the byte bound and the error class below — NOT parsing strictness. The config's `normalizeEndpoint` runs `input.trim()` then `new URL(...)` and returns `url.href`, so the selector only ever sees an already-parsed canonical href; endpoints prefixed with U+00A0/U+FEFF/U+2028/U+3000 all normalize to `https://a.example.com/` and reach selection identically before and after. Those inputs could only have mattered to a direct caller of the exported selector, which is surface this change removes. The attempt ceiling is asserted where it is attested. `CONTROL_EIP1271_MAX_ATTEMPTS_V1` is verified at `current-finalized-evm-call.ts:182`, but it is NOT self-enforcing: the runner never reads `maxAttempts` and instead attempts one endpoint per entry of `profile.endpoints` (`strict-current-finalized-evm-lifecycle.ts:104`). Since this change removes the validator's count rejection, the config keeps an explicit postcondition on the selection result — otherwise an attested "2" would depend on a `>=` inside another module. (A separate constant, `CONTROL_EIP1271_ENDPOINT_ATTEMPT_POLICY_V1`, checked at `:185`, is the policy STRING; an earlier version of this message and both docblocks conflated the two.) The bound is NOT a parameter. Only one caller exists and it always passes the canonical constant, so a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only ABOVE the shipped value — which then needed an explicit ordering repair to fix. Reading the constant directly deletes the parameter, its validation guard, the ordering repair, and that whole failure mode. At a fixed bound of 2 the repair is provably unnecessary: pass 2 runs only when pass 1 selected fewer than the bound, which means the pool held exactly one distinct origin, so pass 1 holds index 0 and pass 2 appends index 1. Ordering is the contract: selection runs AFTER `assertConfigDataProperties` and BEFORE normalization. Before descriptor validation it would re-execute attacker-supplied getters (the hole closed in #2051); after normalization it would never run, because normalization already threw. Both directions have a test and a mutant. Core's predicate is handed the ORIGIN, never the full dial URL. Core bounds its input at the canonical scalar limit, so passing the whole URL imposed a 4096-byte cap that RPC endpoints never had — a config carrying a long path or query credential constructed at base and would have begun throwing. Deriving the origin from the already-parsed URL keeps core as the single source of the origin rule while confining its bound to a string that is always short. Regression-tested in both directions, and a mutant that reinstates the full-URL form dies on it. No bound is placed on the RAW configured array. An earlier revision capped it at 32 entries, which was a compatibility break: base deduplicated by href BEFORE its count check, so 33 identical URLs — or 40 entries collapsing to two — normalized to a valid pool and constructed. A raw cap turns those into a construction failure and silently narrows the contract from "at most two distinct normalized endpoints" to "at most N raw entries". Both cases are now regression-tested, and the performance motive for the cap is gone: dedup is a `Set` (O(n)) and selection is a fixed two-slot scan, so an oversized array costs a linear pass. Endpoint normalization is the single boundary. `normalizeEndpoint` now yields `{ href, origin }` — the dial URL and provider identity from one parse — and the selector is policy over that model. It performs no validation, no parsing and no deduplication, because the states a second validator would defend against cannot reach it. That removed the duplicate URL parse, the defensive branches and the error-translation the selector previously carried. Selection is a TWO-SLOT policy, not a generic selector: first configured URL, then the first later URL with a different origin, else the second configured URL. The bound is fixed, there is one caller, and a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only above the shipped value. The slot count is a LITERAL, not an alias of the attempt ceiling — aliasing would export a value that can change while the behaviour does not — and a module-load assertion fails fast if the ceiling stops matching it. That is a code-level invariant rather than one a test has to notice, and it is itself covered by mocking the read profile. Preferring a distinct origin CAN skip an earlier same-origin URL — `[KEY_A, KEY_B, backup]` selects `[KEY_A, backup]` and never attempts `KEY_B`. That is a deliberate trade, now pinned by test rather than left implicit: two credentials behind one load balancer share a provider, so a provider outage or account-level rate limit takes both out together and leaves a same-origin pair with nothing working, while a distinct origin survives it. With two slots and three endpoints, every possible policy skips some configured URL. Consequences stated rather than buried: * This WIDENS accepted configs. A 3-URL pool with two same-origin aliases was rejected and now constructs. Asserted deliberately. * Selection backfills spare slots with same-origin URLs rather than shrinking the pool, so a load-balancer config carrying per-key URLs keeps its failover. Callers needing real provider diversity must count `selectedOrigins`, never `selected.length`. This function deliberately exposes NO diversity scalar: an earlier revision returned `distinctOriginCount` computed over the whole pool BEFORE truncation while the module header pointed corroboration consumers straight at it — on every shipped EVM network that reported 3 for a session that dials 2 origins, and `canonicalPageProof` requires EXACTLY two distinct origins. A derived value that can disagree with `selectedOrigins` is a trap, so the array is now the single truth. * `selectStrictFinalizedEndpointSessionV1` is NOT exported from the package root. Every consumer imports it by relative path, so exporting it would have published surface with no caller. * The third shipped endpoint (`*.drpc.org`) is now permanently unreachable by strict finalized reads. Selection is health-blind and configuration-ordered, so a degraded primary is still dialled first. Pinned by assertion in the new agent test so it is a known limitation rather than a silent one. * `/api/status` still reports `resolveRpcUrls(...).length` = 3 while a strict session dials 2. Not reconciled here. This does NOT enable dual-origin corroboration. A scope still pins one endpoint for its lifetime; holding two under one lease needs a runner change, deferred to the page-transport PR so §10.3 is not planned against a capability that does not exist. `strict-current-finalized-evm-rpc.unit.test.ts` had one case asserting a 3-endpoint config throws — that case encoded the defect. It is now a positive assertion that selection happens; its other five rejection cases are unchanged. No `instanceof TypeError` rethrow guard sits in the conversion: core routes every fault through `fail()`, including the `new URL()` parse failure it catches and re-fails, so there is no native `TypeError` path to preserve. A guard for it was dead code — deleting it left the suite green. Element faults are reported as `TypeError`. Core throws `VmUpdateConvergenceError`, which extends `Error`, and the config's translating try/catch closes BEFORE the selection call — so without conversion, core's error escaped a validator whose entire documented contract is `TypeError`, carrying a `vm-update:` message for an RPC-config fault. The pre-existing `toThrow(TypeError)` assertion could not catch this: no fixture in any suite reached the 4096-byte bound. The new case is message-pinned rather than type-pinned for that reason. Only ONE fault class actually reaches that conversion from the config. Chain's own `normalizeEndpoint` catches non-strings, null/undefined, objects and unparseable URLs inside the loop before selection runs, each with its own `TypeError`; measured end-to-end, the byte bound is the only element fault that gets through. So the conversion is defense-in-depth rather than the contract's load-bearing layer — and the message-pinned oversize row is not merely good practice, it is the ONLY end-to-end assertion that exercises the conversion at all. Without it the conversion would be covered solely by direct-selector tests. The conversion is at the core call site rather than a pre-check in chain's own `normalizeEndpoint`, which would avoid the conversion entirely by rejecting oversize endpoints before core sees them. That was considered and rejected: core's `MAX_SCALAR_BYTES` is module-private, so a pre-check must hardcode 4096 and would silently disagree the moment core changes it — reintroducing the exact drift class that importing core's function removes. Converting whatever core decides needs no knowledge of the bound. An empty pool now fails closed too; it was the one input the function did not guard. RFC64 precommit fixtures are shared, not copied. The shipped-pool regression varies exactly one field (`rpcEndpoints`), so the plan, accepted-policy snapshot and base options now live in `test/support/rfc64-finalized-vm-precommit-fixture.ts` and both precommit suites consume them: -161 lines across the two test files. Previously a change to the plan or policy shape needed synchronized edits in two places before either suite's actual assertion could run. Verification: * Chain unit lane 44 files, 882 tests, 1 skipped. A rotating handful of real-timer loopback tests fail under machine load: FIVE different tests observed across runs, each passing on a later run. They are load-dependent rather than lane-dependent — running the file alone does not reliably fix them, so "passes solo" is not the argument. Not attributable here for two independent reasons. First, selection is the IDENTITY function for every config involved, checked individually: three carry a single endpoint, one carries two distinct-port origins, and `applies the two-endpoint ceiling after normalized endpoint deduplication` carries three entries that href-dedup to two distinct origins. None exceeds the bound, so no truncation occurs and the endpoint list is byte-identical before and after. Second, independently reproduced in review: the file passes at `fa9dca15e` (30/30) and at `c557f1a61` (31/31) on one machine minutes apart, which rules the change out directly rather than arguing from timing. Later commits add only input guards and touch no timing path. * Nine mutants at this revision (fifteen across the earlier rounds, two of which targeted code this simplification deletes), each killed by its NAMED test, every restore byte-identical, baseline green before each — plus independent auditor mutants from review. Three assertions added in the final round because a mutant proved they could not fail: the pool cap was one-sided (any value in [3,32] was indistinguishable), the `{ cause }` fix was entirely unasserted (deleting it left the suite green), and the default-port test asserted only `toHaveLength(2)`, which backfill guarantees whether or not the port is part of provider identity. * The runner refuses to start unless the tree is green first. It fired: a twelve-mutant run aborted with "tree not green before mutation" because a concurrent lane had pushed a load-sensitive test to 14.3s. Without it that run would have produced six results indistinguishable from real kills. * Agent seam suites green against a force-rebuilt `dist` (`tsbuildinfo` removed — `composite: true` otherwise skips emit and can leave stale or mutated code in `dist` while reporting success). * New agent test drives the REAL precommit with the REAL shipped testnet pool and stubs only global `fetch`. Fail-before proved by reverting the config to its base body: reverted -> exit 1 with "requires 1..2 distinct endpoints" and zero dials; restored byte-identical -> exit 0, two endpoints dialled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…construct RFC64's finalized-VM precommit could not construct its snapshot scope on ANY shipped EVM network. Measured against the merged validator: testnet pool=3 THROWS -> Strict current-finalized RPC requires 1..2 distinct endpoints mainnet-base pool=3 THROWS -> ... mainnet-gnosis pool=3 THROWS -> ... The finalized-read pool is `resolveRpcUrls(chain.rpcUrl, chain.rpcUrls)` — primary PLUS backups (`dkg-agent-rfc64-catalog.ts:777`) — which is three URLs on every EVM network. `snapshotNormalizedEndpoints` rejects more than `CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1 = 2` distinct endpoints, and the precommit passes the pool through unsliced. No test covered it: grepping the error message across the chain and agent suites returned zero hits. The fix is what plan §5.2 always specified — session selection. A new `selectStrictFinalizedEndpointSessionV1` reduces the pool to at most two distinct provider ORIGINS before the ceiling applies, so the ceiling is satisfied by construction rather than by rejection. Provider identity is core's `normalizeEndpointOrigin` — imported, not copied. It is the same predicate `canonicalPageProof` uses to decide `dual-origin-corroborated`, so the two layers cannot drift. (A working revision of this branch forked that rule into chain on the theory that core was not importable; it never reached a commit, so the diff below is purely additive. Core exports the function from the package root and chain already depends on core in 26 files, so the premise was simply wrong.) Note what this is NOT: base never computed an origin at all. `url.href` was the DEDUP key, and that href dedup is still here, unchanged. Origin identity is a coarser layer added on top of it, not a replacement. What inheriting core's rule actually changes, through the only production caller, is the byte bound and the error class below — NOT parsing strictness. The config's `normalizeEndpoint` runs `input.trim()` then `new URL(...)` and returns `url.href`, so the selector only ever sees an already-parsed canonical href; endpoints prefixed with U+00A0/U+FEFF/U+2028/U+3000 all normalize to `https://a.example.com/` and reach selection identically before and after. Those inputs could only have mattered to a direct caller of the exported selector, which is surface this change removes. The attempt ceiling is asserted where it is attested. `CONTROL_EIP1271_MAX_ATTEMPTS_V1` is verified at `current-finalized-evm-call.ts:182`, but it is NOT self-enforcing: the runner never reads `maxAttempts` and instead attempts one endpoint per entry of `profile.endpoints` (`strict-current-finalized-evm-lifecycle.ts:104`). Since this change removes the validator's count rejection, the config keeps an explicit postcondition on the selection result — otherwise an attested "2" would depend on a `>=` inside another module. (A separate constant, `CONTROL_EIP1271_ENDPOINT_ATTEMPT_POLICY_V1`, checked at `:185`, is the policy STRING; an earlier version of this message and both docblocks conflated the two.) The bound is NOT a parameter. Only one caller exists and it always passes the canonical constant, so a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only ABOVE the shipped value — which then needed an explicit ordering repair to fix. Reading the constant directly deletes the parameter, its validation guard, the ordering repair, and that whole failure mode. At a fixed bound of 2 the repair is provably unnecessary: pass 2 runs only when pass 1 selected fewer than the bound, which means the pool held exactly one distinct origin, so pass 1 holds index 0 and pass 2 appends index 1. Ordering is the contract: selection runs AFTER `assertConfigDataProperties` and BEFORE normalization. Before descriptor validation it would re-execute attacker-supplied getters (the hole closed in #2051); after normalization it would never run, because normalization already threw. Both directions have a test and a mutant. Core's predicate is handed the ORIGIN, never the full dial URL. Core bounds its input at the canonical scalar limit, so passing the whole URL imposed a 4096-byte cap that RPC endpoints never had — a config carrying a long path or query credential constructed at base and would have begun throwing. Deriving the origin from the already-parsed URL keeps core as the single source of the origin rule while confining its bound to a string that is always short. Regression-tested in both directions, and a mutant that reinstates the full-URL form dies on it. No bound is placed on the RAW configured array. An earlier revision capped it at 32 entries, which was a compatibility break: base deduplicated by href BEFORE its count check, so 33 identical URLs — or 40 entries collapsing to two — normalized to a valid pool and constructed. A raw cap turns those into a construction failure and silently narrows the contract from "at most two distinct normalized endpoints" to "at most N raw entries". Both cases are now regression-tested, and the performance motive for the cap is gone: dedup is a `Set` (O(n)) and selection is a fixed two-slot scan, so an oversized array costs a linear pass. Endpoint normalization is the single boundary. `normalizeEndpoint` now yields `{ href, origin }` — the dial URL and provider identity from one parse — and the selector is policy over that model. It performs no validation, no parsing and no deduplication, because the states a second validator would defend against cannot reach it. That removed the duplicate URL parse, the defensive branches and the error-translation the selector previously carried. Selection is a TWO-SLOT policy, not a generic selector: first configured URL, then the first later URL with a different origin, else the second configured URL. The bound is fixed, there is one caller, and a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only above the shipped value. The slot count is a LITERAL, not an alias of the attempt ceiling — aliasing would export a value that can change while the behaviour does not — and a COMPILE-TIME assertion fails `tsc` if the ceiling stops matching it. The constant is declared `= 2`, so its type is the literal `2`; raising it makes the assertion a type error. No runtime cost, no import-time side effect, and it reports at the place the mistake is made. Proven by flipping the ceiling to 3 and confirming `tsc` errors, then restoring byte-identically. Preferring a distinct origin CAN skip an earlier same-origin URL — `[KEY_A, KEY_B, backup]` selects `[KEY_A, backup]` and never attempts `KEY_B`. That is a deliberate trade, now pinned by test rather than left implicit: two credentials behind one load balancer share a provider, so a provider outage or account-level rate limit takes both out together and leaves a same-origin pair with nothing working, while a distinct origin survives it. With two slots and three endpoints, every possible policy skips some configured URL. Consequences stated rather than buried: * This WIDENS accepted configs. A 3-URL pool with two same-origin aliases was rejected and now constructs. Asserted deliberately. * Selection backfills spare slots with same-origin URLs rather than shrinking the pool, so a load-balancer config carrying per-key URLs keeps its failover. Callers needing real provider diversity must count `selectedOrigins`, never `selected.length`. This function deliberately exposes NO diversity scalar: an earlier revision returned `distinctOriginCount` computed over the whole pool BEFORE truncation while the module header pointed corroboration consumers straight at it — on every shipped EVM network that reported 3 for a session that dials 2 origins, and `canonicalPageProof` requires EXACTLY two distinct origins. A derived value that can disagree with `selectedOrigins` is a trap, so the array is now the single truth. * `selectStrictFinalizedEndpointSessionV1` is NOT exported from the package root. Every consumer imports it by relative path, so exporting it would have published surface with no caller. * The third shipped endpoint (`*.drpc.org`) is now permanently unreachable by strict finalized reads. Selection is health-blind and configuration-ordered, so a degraded primary is still dialled first. Pinned by assertion in the new agent test so it is a known limitation rather than a silent one. * `/api/status` still reports `resolveRpcUrls(...).length` = 3 while a strict session dials 2. Not reconciled here. This does NOT enable dual-origin corroboration. A scope still pins one endpoint for its lifetime; holding two under one lease needs a runner change, deferred to the page-transport PR so §10.3 is not planned against a capability that does not exist. `strict-current-finalized-evm-rpc.unit.test.ts` had one case asserting a 3-endpoint config throws — that case encoded the defect. It is now a positive assertion that selection happens; its other five rejection cases are unchanged. No `instanceof TypeError` rethrow guard sits in the conversion: core routes every fault through `fail()`, including the `new URL()` parse failure it catches and re-fails, so there is no native `TypeError` path to preserve. A guard for it was dead code — deleting it left the suite green. Element faults are reported as `TypeError`. Core throws `VmUpdateConvergenceError`, which extends `Error`, and the config's translating try/catch closes BEFORE the selection call — so without conversion, core's error escaped a validator whose entire documented contract is `TypeError`, carrying a `vm-update:` message for an RPC-config fault. The pre-existing `toThrow(TypeError)` assertion could not catch this: no fixture in any suite reached the 4096-byte bound. The new case is message-pinned rather than type-pinned for that reason. Only ONE fault class actually reaches that conversion from the config. Chain's own `normalizeEndpoint` catches non-strings, null/undefined, objects and unparseable URLs inside the loop before selection runs, each with its own `TypeError`; measured end-to-end, the byte bound is the only element fault that gets through. So the conversion is defense-in-depth rather than the contract's load-bearing layer — and the message-pinned oversize row is not merely good practice, it is the ONLY end-to-end assertion that exercises the conversion at all. Without it the conversion would be covered solely by direct-selector tests. The conversion is at the core call site rather than a pre-check in chain's own `normalizeEndpoint`, which would avoid the conversion entirely by rejecting oversize endpoints before core sees them. That was considered and rejected: core's `MAX_SCALAR_BYTES` is module-private, so a pre-check must hardcode 4096 and would silently disagree the moment core changes it — reintroducing the exact drift class that importing core's function removes. Converting whatever core decides needs no knowledge of the bound. An empty pool now fails closed too; it was the one input the function did not guard. RFC64 precommit fixtures are shared, not copied. The shipped-pool regression varies exactly one field (`rpcEndpoints`), so the plan, accepted-policy snapshot and base options now live in `test/support/rfc64-finalized-vm-precommit-fixture.ts` and both precommit suites consume them: -161 lines across the two test files. Previously a change to the plan or policy shape needed synchronized edits in two places before either suite's actual assertion could run. Verification: * Chain unit lane 44 files, 882 tests, 1 skipped. A rotating handful of real-timer loopback tests fail under machine load: FIVE different tests observed across runs, each passing on a later run. They are load-dependent rather than lane-dependent — running the file alone does not reliably fix them, so "passes solo" is not the argument. Not attributable here for two independent reasons. First, selection is the IDENTITY function for every config involved, checked individually: three carry a single endpoint, one carries two distinct-port origins, and `applies the two-endpoint ceiling after normalized endpoint deduplication` carries three entries that href-dedup to two distinct origins. None exceeds the bound, so no truncation occurs and the endpoint list is byte-identical before and after. Second, independently reproduced in review: the file passes at `fa9dca15e` (30/30) and at `c557f1a61` (31/31) on one machine minutes apart, which rules the change out directly rather than arguing from timing. Later commits add only input guards and touch no timing path. * Nine mutants at this revision (fifteen across the earlier rounds, two of which targeted code this simplification deletes), each killed by its NAMED test, every restore byte-identical, baseline green before each — plus independent auditor mutants from review. Three assertions added in the final round because a mutant proved they could not fail: the pool cap was one-sided (any value in [3,32] was indistinguishable), the `{ cause }` fix was entirely unasserted (deleting it left the suite green), and the default-port test asserted only `toHaveLength(2)`, which backfill guarantees whether or not the port is part of provider identity. * The runner refuses to start unless the tree is green first. It fired: a twelve-mutant run aborted with "tree not green before mutation" because a concurrent lane had pushed a load-sensitive test to 14.3s. Without it that run would have produced six results indistinguishable from real kills. * Agent seam suites green against a force-rebuilt `dist` (`tsbuildinfo` removed — `composite: true` otherwise skips emit and can leave stale or mutated code in `dist` while reporting success). * New agent test drives the REAL precommit with the REAL shipped testnet pool and stubs only global `fetch`. Fail-before proved by reverting the config to its base body: reverted -> exit 1 with "requires 1..2 distinct endpoints" and zero dials; restored byte-identical -> exit 0, two endpoints dialled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
…construct RFC64's finalized-VM precommit could not construct its snapshot scope on ANY shipped EVM network. Measured against the merged validator: testnet pool=3 THROWS -> Strict current-finalized RPC requires 1..2 distinct endpoints mainnet-base pool=3 THROWS -> ... mainnet-gnosis pool=3 THROWS -> ... The finalized-read pool is `resolveRpcUrls(chain.rpcUrl, chain.rpcUrls)` — primary PLUS backups (`dkg-agent-rfc64-catalog.ts:777`) — which is three URLs on every EVM network. `snapshotNormalizedEndpoints` rejects more than `CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1 = 2` distinct endpoints, and the precommit passes the pool through unsliced. No test covered it: grepping the error message across the chain and agent suites returned zero hits. The fix is what plan §5.2 always specified — session selection. A new `selectStrictFinalizedEndpointSessionV1` reduces the pool to at most two distinct provider ORIGINS before the ceiling applies, so the ceiling is satisfied by construction rather than by rejection. Provider identity is core's `normalizeEndpointOrigin` — imported, not copied. It is the same predicate `canonicalPageProof` uses to decide `dual-origin-corroborated`, so the two layers cannot drift. (A working revision of this branch forked that rule into chain on the theory that core was not importable; it never reached a commit, so the diff below is purely additive. Core exports the function from the package root and chain already depends on core in 26 files, so the premise was simply wrong.) Note what this is NOT: base never computed an origin at all. `url.href` was the DEDUP key, and that href dedup is still here, unchanged. Origin identity is a coarser layer added on top of it, not a replacement. What inheriting core's rule actually changes, through the only production caller, is the byte bound and the error class below — NOT parsing strictness. The config's `normalizeEndpoint` runs `input.trim()` then `new URL(...)` and returns `url.href`, so the selector only ever sees an already-parsed canonical href; endpoints prefixed with U+00A0/U+FEFF/U+2028/U+3000 all normalize to `https://a.example.com/` and reach selection identically before and after. Those inputs could only have mattered to a direct caller of the exported selector, which is surface this change removes. The attempt ceiling is asserted where it is attested. `CONTROL_EIP1271_MAX_ATTEMPTS_V1` is verified at `current-finalized-evm-call.ts:182`, but it is NOT self-enforcing: the runner never reads `maxAttempts` and instead attempts one endpoint per entry of `profile.endpoints` (`strict-current-finalized-evm-lifecycle.ts:104`). Since this change removes the validator's count rejection, the config keeps an explicit postcondition on the selection result — otherwise an attested "2" would depend on a `>=` inside another module. (A separate constant, `CONTROL_EIP1271_ENDPOINT_ATTEMPT_POLICY_V1`, checked at `:185`, is the policy STRING; an earlier version of this message and both docblocks conflated the two.) The bound is NOT a parameter. Only one caller exists and it always passes the canonical constant, so a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only ABOVE the shipped value — which then needed an explicit ordering repair to fix. Reading the constant directly deletes the parameter, its validation guard, the ordering repair, and that whole failure mode. At a fixed bound of 2 the repair is provably unnecessary: pass 2 runs only when pass 1 selected fewer than the bound, which means the pool held exactly one distinct origin, so pass 1 holds index 0 and pass 2 appends index 1. Ordering is the contract: selection runs AFTER `assertConfigDataProperties` and BEFORE normalization. Before descriptor validation it would re-execute attacker-supplied getters (the hole closed in #2051); after normalization it would never run, because normalization already threw. Both directions have a test and a mutant. Core's predicate is handed the ORIGIN, never the full dial URL. Core bounds its input at the canonical scalar limit, so passing the whole URL imposed a 4096-byte cap that RPC endpoints never had — a config carrying a long path or query credential constructed at base and would have begun throwing. Deriving the origin from the already-parsed URL keeps core as the single source of the origin rule while confining its bound to a string that is always short. Regression-tested in both directions, and a mutant that reinstates the full-URL form dies on it. No bound is placed on the RAW configured array. An earlier revision capped it at 32 entries, which was a compatibility break: base deduplicated by href BEFORE its count check, so 33 identical URLs — or 40 entries collapsing to two — normalized to a valid pool and constructed. A raw cap turns those into a construction failure and silently narrows the contract from "at most two distinct normalized endpoints" to "at most N raw entries". Both cases are now regression-tested, and the performance motive for the cap is gone: dedup is a `Set` (O(n)) and selection is a fixed two-slot scan, so an oversized array costs a linear pass. Endpoint normalization is the single boundary. `normalizeEndpoint` now yields `{ href, origin }` — the dial URL and provider identity from one parse — and the selector is policy over that model. It performs no validation, no parsing and no deduplication, because the states a second validator would defend against cannot reach it. That removed the duplicate URL parse, the defensive branches and the error-translation the selector previously carried. Selection is a TWO-SLOT policy, not a generic selector: first configured URL, then the first later URL with a different origin, else the second configured URL. The bound is fixed, there is one caller, and a caller-supplied bound bought nothing while introducing a reordering failure mode reachable only above the shipped value. The slot count is a LITERAL, not an alias of the attempt ceiling — aliasing would export a value that can change while the behaviour does not — and a COMPILE-TIME assertion fails `tsc` if the ceiling stops matching it. The constant is declared `= 2`, so its type is the literal `2`; raising it makes the assertion a type error. No runtime cost, no import-time side effect, and it reports at the place the mistake is made. Proven by flipping the ceiling to 3 and confirming `tsc` errors, then restoring byte-identically. Preferring a distinct origin CAN skip an earlier same-origin URL — `[KEY_A, KEY_B, backup]` selects `[KEY_A, backup]` and never attempts `KEY_B`. That is a deliberate trade, now pinned by test rather than left implicit: two credentials behind one load balancer share a provider, so a provider outage or account-level rate limit takes both out together and leaves a same-origin pair with nothing working, while a distinct origin survives it. With two slots and three endpoints, every possible policy skips some configured URL. Consequences stated rather than buried: * This WIDENS accepted configs. A 3-URL pool with two same-origin aliases was rejected and now constructs. Asserted deliberately. * Selection backfills spare slots with same-origin URLs rather than shrinking the pool, so a load-balancer config carrying per-key URLs keeps its failover. Callers needing real provider diversity must count `selectedOrigins`, never `selected.length`. This function deliberately exposes NO diversity scalar: an earlier revision returned `distinctOriginCount` computed over the whole pool BEFORE truncation while the module header pointed corroboration consumers straight at it — on every shipped EVM network that reported 3 for a session that dials 2 origins, and `canonicalPageProof` requires EXACTLY two distinct origins. A derived value that can disagree with `selectedOrigins` is a trap, so the array is now the single truth. * `selectStrictFinalizedEndpointSessionV1` is NOT exported from the package root. Every consumer imports it by relative path, so exporting it would have published surface with no caller. * The third shipped endpoint (`*.drpc.org`) is now permanently unreachable by strict finalized reads. Selection is health-blind and configuration-ordered, so a degraded primary is still dialled first. Pinned by assertion in the new agent test so it is a known limitation rather than a silent one. * `/api/status` still reports `resolveRpcUrls(...).length` = 3 while a strict session dials 2. Not reconciled here. This does NOT enable dual-origin corroboration. A scope still pins one endpoint for its lifetime; holding two under one lease needs a runner change, deferred to the page-transport PR so §10.3 is not planned against a capability that does not exist. `strict-current-finalized-evm-rpc.unit.test.ts` had one case asserting a 3-endpoint config throws — that case encoded the defect. It is now a positive assertion that selection happens; its other five rejection cases are unchanged. No `instanceof TypeError` rethrow guard sits in the conversion: core routes every fault through `fail()`, including the `new URL()` parse failure it catches and re-fails, so there is no native `TypeError` path to preserve. A guard for it was dead code — deleting it left the suite green. Element faults are reported as `TypeError`. Core throws `VmUpdateConvergenceError`, which extends `Error`, and the config's translating try/catch closes BEFORE the selection call — so without conversion, core's error escaped a validator whose entire documented contract is `TypeError`, carrying a `vm-update:` message for an RPC-config fault. The pre-existing `toThrow(TypeError)` assertion could not catch this: no fixture in any suite reached the 4096-byte bound. The new case is message-pinned rather than type-pinned for that reason. Only ONE fault class actually reaches that conversion from the config. Chain's own `normalizeEndpoint` catches non-strings, null/undefined, objects and unparseable URLs inside the loop before selection runs, each with its own `TypeError`; measured end-to-end, the byte bound is the only element fault that gets through. So the conversion is defense-in-depth rather than the contract's load-bearing layer — and the message-pinned oversize row is not merely good practice, it is the ONLY end-to-end assertion that exercises the conversion at all. Without it the conversion would be covered solely by direct-selector tests. The conversion is at the core call site rather than a pre-check in chain's own `normalizeEndpoint`, which would avoid the conversion entirely by rejecting oversize endpoints before core sees them. That was considered and rejected: core's `MAX_SCALAR_BYTES` is module-private, so a pre-check must hardcode 4096 and would silently disagree the moment core changes it — reintroducing the exact drift class that importing core's function removes. Converting whatever core decides needs no knowledge of the bound. An empty pool now fails closed too; it was the one input the function did not guard. RFC64 precommit fixtures are shared, not copied. The shipped-pool regression varies exactly one field (`rpcEndpoints`), so the plan, accepted-policy snapshot and base options now live in `test/support/rfc64-finalized-vm-precommit-fixture.ts` and both precommit suites consume them: -161 lines across the two test files. Previously a change to the plan or policy shape needed synchronized edits in two places before either suite's actual assertion could run. Verification: * Chain unit lane 44 files, 882 tests, 1 skipped. A rotating handful of real-timer loopback tests fail under machine load: FIVE different tests observed across runs, each passing on a later run. They are load-dependent rather than lane-dependent — running the file alone does not reliably fix them, so "passes solo" is not the argument. Not attributable here for two independent reasons. First, selection is the IDENTITY function for every config involved, checked individually: three carry a single endpoint, one carries two distinct-port origins, and `applies the two-endpoint ceiling after normalized endpoint deduplication` carries three entries that href-dedup to two distinct origins. None exceeds the bound, so no truncation occurs and the endpoint list is byte-identical before and after. Second, independently reproduced in review: the file passes at `fa9dca15e` (30/30) and at `c557f1a61` (31/31) on one machine minutes apart, which rules the change out directly rather than arguing from timing. Later commits add only input guards and touch no timing path. * Nine mutants at this revision (fifteen across the earlier rounds, two of which targeted code this simplification deletes), each killed by its NAMED test, every restore byte-identical, baseline green before each — plus independent auditor mutants from review. Three assertions added in the final round because a mutant proved they could not fail: the pool cap was one-sided (any value in [3,32] was indistinguishable), the `{ cause }` fix was entirely unasserted (deleting it left the suite green), and the default-port test asserted only `toHaveLength(2)`, which backfill guarantees whether or not the port is part of provider identity. * The runner refuses to start unless the tree is green first. It fired: a twelve-mutant run aborted with "tree not green before mutation" because a concurrent lane had pushed a load-sensitive test to 14.3s. Without it that run would have produced six results indistinguishable from real kills. * Agent seam suites green against a force-rebuilt `dist` (`tsbuildinfo` removed — `composite: true` otherwise skips emit and can leave stale or mutated code in `dist` while reporting success). * New agent test drives the REAL precommit with the REAL shipped testnet pool and stubs only global `fetch`. Fail-before proved by reverting the config to its base body: reverted -> exit 1 with "requires 1..2 distinct endpoints" and zero dials; restored byte-identical -> exit 0, two endpoints dialled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1
2 participants
Summary
W2a part 1 of N. W2 is the update-convergence half of D1 — making a node converge on finalized KA updates it never saw. This PR lands the two foundation chunks (plan §10.1 and the admission half of §10.2) and, along the way, fixes a live defect it uncovered. It is deliberately scoped so each piece is independently reviewable; §10.3 onward follow as separate PRs (list at the bottom).
1.
packages/core/src/vm-update-convergence.ts— the canonical value contracts (§10.1). Scope identity, exact finalized-event identity, the ordered raw-log commitment, page-assurance proofs, the two-cursor model, the scoped KA candidate parser, and the closed outcome vocabularies. No chain client, no store, no I/O — every rule is testable without a node, an RPC endpoint, or a database.Two rules in it are load-bearing and are pinned by tests that can fail:
scopeIdis derived from the four IDENTITY fields only, never fromdeploymentBlock. Including the anchor would make an anchor correction mint a new scope instead of triggering revision reset and replay inside the old one — orphaning every persisted cursor for that chain.did:dkg:<chain>/<KA-storage>/7round-trips byte-for-byte under both the legacy-sequential and rootless-packed forms. It returns both and lets the store resolve which is live against chain provenance. Choosing either here would attribute a graph write to the wrong KA.2.
packages/chain/src/finalized-chain-read-admission.ts— a per-chain limit that is actually per chain (§10.2, admission half). This is a live bug fix, not just W2 scaffolding.CURRENT_FINALIZED_EVM_SNAPSHOT_MAX_CONCURRENT_PER_CHAIN_V1 = 1has always documented "one heavyweight pinned scan per chain" — but it was enforced by a gate constructed insidecreateStrictFinalizedEndpointRunnerV1, i.e. one gate per transport instance. RFC64 builds its snapshot scope inside the precommit handler (finalized-vm-agent-precommit-v1.ts:94), so it built a fresh gate per invocation and contended with nothing. Two concurrent precommits on one chain both admitted, each running a full pinned multi-batch scan against the same RPC pool.Permit state moves to module scope, keyed on the canonical chain id alone.
owner(foreground | rfc64 | w2-page | w2-target) is carried for attribution only — putting it in the key would produce one lane per owner, which is the exact bug being removed. It is optional and defaults toforeground— a real label, not an "unknown" bucket — because this factory is published API; a test pins that the default takes the same shared permit rather than escaping it.The one-shot read primitive deliberately keeps its own gate: its limit is 4, and folding it into the snapshot's single lane would throttle an unrelated path.
3.
packages/core/src/ka-ual-identity.ts— one owner for the legacy/rootless KA UAL rule.buildReconciledKnowledgeAssetUal(agent) andbuildScopedKnowledgeAssetUal(W2) both delegate to it, so there is no second production implementation of an identity rule. An earlier revision of this PR restated the rule incoreand guarded the copy with a cross-package parity test; that was corrected in review — the dependency direction (agent → chain → core) never required the duplication.4. A second real defect, caught by re-reading the shipped contracts rather than my own tests.
ChainAdapter.chainIdis namespaced —base:84532,otp:20430,evm:31337— and its own doc comment says it is "not directly parseable withBigInt()" (getEvmChainId()is the numeric one). UALs are built from the namespaced form. My scope validator required a canonical decimal, socanonicalScopedKaCandidatesFromVerifiedUalwould have thrown for every real mainnet and testnet UAL. Every test used84532, so the suite was green against code that could not run anywhere real. Fixed, with the namespaced forms now covered explicitly.Related
agent-docs/plans/2026-08-02-w2-update-convergence.md(v13, local-only —agent-docs/is git-excluded).W2b is NOT in this PR and is gated on two open questions, both of the same shape — a typed, designed, green outcome that satisfies every acceptance criterion while the feature does nothing:
eth_callat the covered anchor, on shipped non-archive pools. If they will not serve it, W2b is permanentlyfinalized-state-unavailableand every zero-side-effect criterion still passes. Needs one probe.processBoundMutationCompletionV1. The plan excludes HTTP Blazegraph, managed Oxigraph, and generic SPARQL — andoxigraph-serveris managed Oxigraph oversparql-http. So the only family that can declare it isoxigraph/oxigraph-worker/oxigraph-persistent, which is being retired. Needs a named successor backend, or W2b needs a design change.W2a is unaffected by both: it is backend-agnostic and installs no mutation hook.
Diagrams
Finalized-read admission on one chain
Before:
sequenceDiagram participant P1 as RFC64 precommit #1 participant P2 as RFC64 precommit #2 participant F as createSnapshotScope() participant RPC as Chain RPC pool P1->>F: build scope (per invocation) F-->>P1: transport + its OWN gate(limit=1) P2->>F: build scope (per invocation) F-->>P2: transport + a SECOND gate(limit=1) P1->>RPC: pinned multi-batch scan P2->>RPC: pinned multi-batch scan (also admitted) Note over RPC: two heavyweight scans, limit says "1 per chain"After:
sequenceDiagram participant P1 as RFC64 precommit #1 participant P2 as W2 page scanner participant R as finalized-chain-read-admission (module scope) participant RPC as Chain RPC pool P1->>R: acquire(chainId, owner=rfc64) R-->>P1: permit P1->>RPC: pinned multi-batch scan P2->>R: acquire(chainId, owner=w2-page) R-->>P2: refused — concurrency-saturated (held by rfc64) P1->>R: release P2->>R: acquire(chainId, owner=w2-page) R-->>P2: permitResolving which KA a verified UAL denotes
Before: no W2 path existed — the UAL's last segment was the identity.
After:
sequenceDiagram participant C as Caller (verified UAL) participant P as canonicalScopedKaCandidates… participant S as W2 store (later PR) C->>P: scope + did:dkg:base:84532/<KA-storage>/7 P->>P: round-trip BOTH forms byte-for-byte P-->>C: [rootless-packed, legacy-sequential] C->>S: intersect candidates with this revision's chain provenance alt exactly one live S-->>C: selected KA + open mutation intent else two live S-->>C: ambiguous-w2-identity — mutation fails CLOSED endFiles changed
packages/core/src/vm-update-convergence.tspackages/core/test/vm-update-convergence.test.tspackages/core/src/index.tspackages/chain/src/finalized-chain-read-admission.tsownerfor attributionpackages/chain/test/finalized-chain-read-admission.unit.test.tsconcurrency-saturatedcodepackages/chain/src/strict-current-finalized-evm-lifecycle.tsEndpointAdmissionPolicyV1<K>instead of knowing about owners;maxConcurrentPerChainleft the runner contract;saturatedreports the holder as a plain stringpackages/chain/src/strict-current-finalized-evm-types.tsowner(defaults toforeground)packages/chain/src/strict-current-finalized-evm-config.tspackages/chain/src/strict-current-finalized-evm-snapshot-{transport,rpc,factory}.tspackages/chain/src/index.tspackages/chain/test/strict-current-finalized-evm-snapshot.unit.test.tsowneron 23 existing sitespackages/chain/test/finalized-vm-chain-scanner.unit.test.tsowneron the one construction sitepackages/agent/src/rfc64/finalized-vm-agent-precommit-v1.tsowner: 'rfc64'— this is the per-invocation construction sitepackages/core/src/ka-ual-identity.tspackages/agent/src/ka-identity.tsbuildReconciledKnowledgeAssetUaldelegates to that owner; behaviour unchanged, including lowercasing the checksummed address its sole caller passespackages/agent/test/w2-ual-parity.test.tskaId >> 96boundary with absolute expected strings — with one owner, comparing the two entry points is vacuouspackages/chain/src/nonqueueing-admission.tsEndpointAdmissionPolicyV1<K>, pluscreateLocalNonqueueingAdmissionV1— the per-instance policy for the one-shot read, whose limit is genuinely localpackages/chain/test/snapshot-test-fixtures.tspackages/chain/test/strict-finalized-snapshot-admission.unit.test.tspackages/chain/test/strict-finalized-snapshot-config.unit.test.tsnullowner policypackages/chain/src/finalized-chain-read-admission.tsisFinalizedChainAdmissionContention, which matches refusals this module actually threw (aWeakSet) rather than a code three other paths also emitpackages/agent/src/rfc64/public-catalog-receiver-v1.tsdeferredis an explicit scheduler state observed bywhenIdle(); provider retry bookkeeping moved onto the task so a deferral cannot resetmaxAttempts; the deferral predicate is injectablepackages/agent/test/rfc64-receiver-admission-deferral.test.tswhenIdle()does not resolve while deferred; duplicate-head dedupe and same-scope/different-head scheduling during a deferral;maxAttemptsstays a true per-provider bound across deferrals; bounded give-up; ordinary errors still fail fastpackages/agent/test/rfc64-precommit-owner-attribution.test.tsrfc64— the only test that fails if the production owner label changespackages/agent/vitest.unit.config.tsincludeis explicit — an unlisted file runs nothing)Test plan
Run from the repo root.
packages/agentresolvesdkg-corethroughdist, so build first.pnpm --filter @origintrail-official/dkg-core build && pnpm --filter @origintrail-official/dkg-chain build && pnpm --filter @origintrail-official/dkg-agent buildpnpm --filter @origintrail-official/dkg-core exec vitest run test/vm-update-convergence.test.ts→ 67 passedpnpm --filter @origintrail-official/dkg-chain exec vitest run --config vitest.unit.config.ts(whole unit lane) → 854 passed, 1 skippedpnpm --filter @origintrail-official/dkg-agent exec vitest run --config vitest.unit.config.ts test/w2-ual-parity.test.ts test/rfc64-receiver-admission-deferral.test.ts test/rfc64-precommit-owner-attribution.test.ts test/rfc64-finalized-vm-agent-precommit-v1.test.ts test/rfc64-finalized-vm-runtime-v1.test.ts test/rfc64-public-catalog-receiver-v1.test.ts→ 51 passed (six suites, including the receiver's pre-existing one)pnpm --filter @origintrail-official/dkg-chain exec tsc --noEmit -p tsconfig.json→ cleanEvidence the tests can fail
Green counts prove little on their own, so each claim below is a recorded negative.
throwinderiveVmUpdateScopeIdfails 16 core tests; athrowinjected into the rebuiltpackages/core/dist/…fails 9 of the 10 agent parity tests — proving the agent suite executes the rebuiltdistand not a stale copy. Both restored byte-identical (hash-verified).deploymentBlock;lengthPrefixed→ plain join; commitment dropping the raw payload; corroboration accepting one origin; resume skipping the partial block; the parser returning one candidate instead of the ambiguous pair; the author canonicalizer rejecting zero; the cursor guard weakened<=→<. All 8 killed.One finding worth flagging to the reviewer
A mutant survived during §10.1, and it was my own test's fault: two "boundary collision" assertions compared inputs of different total content, which any encoding distinguishes — they could never have failed for the property they claimed to pin. They now compare tuples with identical concatenations and differing field boundaries (
'84532'+'abc'vs'8453'+'2abc'; adjacent variable-widthtxIndex/logIndex(1,23)vs(12,3)). The original mutant was also wrong — zeroing a length field leaves four constant NUL bytes still acting as a delimiter — so the corrected mutant collapseslengthPrefixedto a plain join, and kills exactly those two tests.Known, and not caused by this PR
packages/core/test/{project-ontology,sync-control-object}.test.tsfail locally with 5 s timeouts. Verified pre-existing by removing this PR's single barrel-export line and re-running — both still red with the branch's only shared-surface change reverted.What follows
Remaining W2a chunks, in order. Each is its own PR.
CURRENT_FINALIZED_EVM_READ_MAX_ATTEMPTS_V1is 2 while shipped pools carry 3 URLs, and origin normalization currently dedups on fullurl.href, not origin); the fairness/reservation contractknowledgeAssetStorageDeploymentanchors, which no network overlay carries todayEXPECTED_CORROBORATING, notEXPECTED_INSTRUMENTS— the latter reds the build), docsReview rounds applied
This description reflects head
3c5ebb995. Three rounds have landed since the PR opened; the sections above are current, and the substantive changes are:Round 1 —
otReviewAgent, 8 findings, 7 accepted (977e02a43). TheownerAPI break (now defaults toforeground), an unboundedtopicsarray, a page proof accepting impossible same-height hashes, duplicated canonical-scalar regexes, thesharedOwnermode flag replaced by an injected admission policy, admission tests split out, and shallow freeze assertions made behavioural. Deferred: splitting the core module into ~8 files, with reasoning on the thread.Round 2 —
otReviewAgent, 1 finding (36332221f). A regression I introduced in round 1:{ owner, ...rest }ran before validation, so object rest executed the caller's enumerable getters and flattened them into data properties — the wrapper then accepted a config the base validator rejects untouched (measured: base rejected with 0 getter invocations, wrapper accepted with 1). Descriptors are now validated on the original input first.Round 3 — merge-readiness review (
3c5ebb995). All six P1s and all three P2s:denseDataArray— length before iteration, own-descriptor per index, getters never invoked; applied to origins, the page, and topics.isAuthoritativePagecanonicalizes instead of trusting the assurance stringMerkleRoot[](2,080 bytes) now passes. Identity scalars keep the tight capconcurrency-saturatedbecomes a detached, bounded deferral that gives the attempt back and releases both the slot and the scope locksameEventIdentity; otherwise the new typedresume-identity-conflict. Hashes stay out of the comparatornull/undefinedowner policyObject.freezed0 <= kaId <= uint256.max1999de2bc— see round 3b belowRound 3b — the item round 3 left open (
1999de2bc). The merge-readiness review showedowner: 'rfc64'→'foreground'surviving the focused RFC64 suites 10/10, because nothing reached the real factory.rfc64-precommit-owner-attribution.test.tsnow drives the real precommit against an RPC that accepts and never answers, so the permit is genuinely held; a second owner then probes the registry for the holder. That mutant now fails exactly one test.Round 4 — 4 findings, 1 🔴 (
4c874a582). The 🔴 was a regression from round 3: the deferral released the slot and the queue entry, so a deferred head was invisible to#isIdle()andwhenIdle()resolved mid-retry — a caller draining the receiver could conclude scheduling had settled before the head applied.deferredis now an explicit state observed by idle, reported instats(), cleared on close and on give-up. Also: contention classification moved out of the receiver into the chain package that owns the code (injectable viaisDeferrableError);EndpointAdmissionPolicyV1moved to the neutral module; deferred-head dedupe covered. One suggestion declined with reasoning on its thread (moving deferral into the reconciler's return type).Round 5 — 1 🔴 (
80d53fc3e). The classifier matched theconcurrency-saturatedcode, which is shared: the snapshot session's reentrancy guard and the one-shot read's limit gate both emit it. A genuine integration bug would have been deferred as contention and reported as a lane-wait failure. Now matched by identity — aWeakSetof refusalsacquireFinalizedChainReadactually threw. This also invalidated my own test fixture, correctly; those tests now inject the policy, and a new test proves the default wiring end to end against a real refusal plus a look-alike that must still fail fast.Round 6 — 1 🟡 (
8ef662661). The KA UAL rule now has a single owner incore; my earlier deferral rationale was wrong (I had conflated it with movingbuildKnowledgeAssetUalout ofchain— 62 references — when the real change is one function with one production caller). Two notes: I reversed my own mixed-case rejection, because the sole caller passes the checksummed address fromgetDKGKnowledgeAssetsAddress()and builders should normalize; and the consolidation silently weakened the test until a mutant caught it — with one owner, comparing the two entry points is vacuous, so the boundary is now pinned with absolute expected strings.Round 7 — 2 findings, 1 🔴 (
da575b511). The 🔴: my deferral tests used a casted fixture setting fields that do not exist on the wire type, so every announcement collapsed to oneheadKeyofundefineds — the dedupe test could not fail. Replaced with a type-correct fixture, plus a test that asserts the premise itself, plus the same-scope/different-head case proving the scope lock is released. The 🟡: provider retry bookkeeping was local to#runTask, so a deferral reset it andmaxAttemptsstopped bounding anything once contention interleaved with ordinary failures; it now lives on the task, with contention refunding only its own attempt.Open by choice
Two items are deliberately not done, both argued on their threads rather than quietly dropped:
On the local Windows test noise the merge-readiness review also saw: I re-ran the six RFC64 files that fail locally, on a clean build, and read the actual failure text rather than inferring. It is
EBUSY: resource busy or locked, unlink '…inventory-v1.lease.sqlite3'plusAbortError: The operation was aborted due to timeoutin the protocol router — Windows file locking and libp2p dial timeouts, in code paths this PR does not touch. Applying the receiver deferral did not change them, which further separates them from the contention finding. Linux CI is green on those same files.🤖 Generated with Claude Code
https://claude.ai/code/session_01SaXdkmteqStxGBpXcUPGd1