fix(validator): preserve reviewed successor across head drift - #167
Conversation
The fixed two-miner successor now opts into the reviewed finalized-descendant allowance used by the one-miner launch. The lowest signing boundary revalidates the exact durable predecessor reservation, bounded canonical ancestry, latest chain policy and cooldown, dynamic UID mappings, UID30 permit and stake, and both reviewed raw Bittensor axons before nonce access or wallet unlock. Outcome target: a one- or two-block finalized-head advance no longer rejects the reviewed successor, while generic callers, recurring authority, burn-bearing modes, stale mappings, changed policy, or altered machines still fail closed. Tests cover the live race and adversarial no-touch mutations. No chain, wallet, cloud, or runtime artifacts were accessed or changed.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fbf7579b-977b-441a-a259-49e34d68723d) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
PM gate (no Astro ping):
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d06895042
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| or inclusion.expected_next_epoch_start_block - latest_finalized_block | ||
| < mortal_period_blocks * 3 |
There was a problem hiding this comment.
Accept valid two-era successor windows
When a reviewed successor starts with between 2 * mortal_period_blocks + drift and 3 * mortal_period_blocks - 1 blocks before the epoch, its preview is valid under validate_uid30_successor_state, and after a one- or two-block drift it still has the full mortal era required by the preceding valid_until_block check. This additional three-era requirement nevertheless rejects it before signing, so the new path still fails to preserve otherwise valid reviewed successors across the exact head drift it is intended to handle. Align this threshold with the existing preview/inclusion-policy room requirement rather than imposing a third era only on the descendant path.
Useful? React with 👍 / 👎.
Outcome target
Allow the fixed, digest-reviewed UID30 two-miner successor to cross only the existing one-or-two-finalized-block collection race, then sign only after every mutable launch gate is re-proven at the canonical descendant head.
The exact live target remains:
[[8, 65535], [124, 65535]]authority_boundedsuccessor attempt[[124, 65535]]Live failure this fixes
The first production successor invocation on merged main
a7115b5collected fresh evidence at finalized blocks 8,947,986 and 8,947,987, then refused with:SN39 finalized head advanced after preflight; refusing before signingThat refusal was safe. It returned
REFUSED_NO_CHAIN_WRITE, created no signed intent, consumed no nonce, submitted no extrinsic, and restored the canonical predecessor journal byte-for-byte at SHA-256700812f931a8c5c26bf35644e93a90c4ba4b15c5125ab22a231211777e647285.The one-miner launch already had a bounded reviewed-descendant path. The fixed successor caller did not request it, and the canonical descendant guard supported only the legacy one-miner contract.
What changes
two_miner_successorbranch from the exact durable journal marker.Before nonce lookup, wallet unlock, call composition, signing, intent journaling, or submission, the successor branch re-proves:
[65535, 65535]and zero burnEvery tested contradiction refuses before the irreversible boundary and leaves byte-exact unsigned abort available.
Verification
Exact commit:
4d06895042fb26babfd0d4574beaa2e004b96b1dgit diff --checkpassedFable recorded one P2 coverage gap: the axon mutation matrix lacks a dedicated live-IP-diff case. The exact IP comparison is present, while raw shape, IP type, port, protocol, and endpoint mutations are covered. This does not change the SHIP verdict.
Merge and live proof boundary
Merge success means the fail-closed descendant path is present on main. It is not proof of a chain write.
After merge, the operator must build the exact Linux/amd64 main image, create and independently review a new immutable preview, re-prove exclusive-writer status and both live machines, submit once, and confirm the exact row at inclusion and two later finalized heads. A signed or ambiguous result never authorizes a retry.
Note
High Risk
Changes pre-sign authorization for on-chain weight submission on SN39, including successor extrinsics; mistakes could sign wrong vectors or weaken fail-closed gates, though behavior is heavily tested and scoped to reviewed two-miner contracts.
Overview
Fixes production UID30 two-miner successor submissions that collected fresh evidence then refused with head-drift errors when the finalized chain moved one or two blocks before signing.
The successor submit path now passes
allow_reviewed_uid30_finalized_descendant=Trueinto_submit_exact_sn39_extrinsic, matching the existing one-miner launch behavior. Invalidator_thin, the canonical descendant guard branches when the durable journal marks atwo_miner_successorcontract (or the runtime carries the successor preview digest): it runs new_require_reviewed_uid30_successor_finalized_descendantinstead of the legacy one-miner checks, which stay unchanged.Before nonce, wallet unlock, or signing, the successor branch re-proves the reviewed digest, wire vector
[65535, 65535], bounded parent ancestry from the mapping block, inclusion/time/epoch room, chain policy, UID mappings, validator permit/stake/cooldown, predecessor weights row, and both miners’ reviewed IPv4 HTTPS axons. Callers without the exact reviewed successor contract still fail closed.Tests add
_descendant_preflightfixtures, acceptance for bounded drift, a large counterexample matrix that must refuse before irreversible steps, and an assertion that the generic flag does not authorize unreviewed callers.Reviewed by Cursor Bugbot for commit 4d06895. Bugbot is set up for automated code reviews on this repo. Configure here.