fix(test): make the shard-split authorization gate live in the fixture - #618
Open
blacks1ne wants to merge 1 commit into
Open
Conversation
`split_fixture` never installs the intrinsic's hypergraph slot — `with_kick_verify_deps` is its only setter and the fixture calls only `with_frame_header_deps`. `proposer_is_active_global` returns true unconditionally when that slot is empty, so the authorization check is a no-op and the non-global proposer is accepted. Both halves of the test were inert, not just the failing one. The positive half was passing for the wrong reason and could not have passed for the right one: `seed_global_prover` stores the allocation vertex at an arbitrary flat key, while `verify_shard_op_signer_is_active_global` reads the signer's PublicKey out of the prover tree and looks up `allocation_address(pubkey, <empty filter>)`. The registry path this replaced tolerated the flat key because `refresh_from_store` parses the blob and keys by the tree's ProverAddress field. Install the crdt, and seed the allocation at the derived address — taking it from the prover's own filter is what makes the non-global case miss the empty-filter lookup and be rejected. Also pin the rejection reason: bare `is_err()` passes just as well when the op is refused for an unrelated reason, which is how this dimension went silently inert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Base:
2b96656eshard_split_by_global_prover_registers_enumerable_child_and_rejects_non_globalfails — a non-global proposer is accepted.proposer_is_active_globalauthorizes against committed hypergraph state and skips itself when that slot is empty:split_fixturenever installs it —with_kick_verify_depsis the only setter, and the fixture calls onlywith_frame_header_deps— so the gate returnstrueunconditionally.The skip is deliberate and consistent with validate (
if let Some(hg)), so invoke and validate still agree on the same node. What no longer holds is the fixture's node shape: the previous implementation readprover_registry, which the fixture does install, and failed closed when it was absent.Both halves of the test were inert, not only the failing one. The positive half was passing for the wrong reason, and could not have passed for the right one:
seed_global_proverstores the allocation vertex at an arbitrary flat key, whileverify_shard_op_signer_is_active_globalreads the signer's PublicKey out of the prover tree and looks upallocation_address(pubkey, <empty filter>). The registry path tolerated the flat key becauserefresh_from_storekeys by the tree's ProverAddress field rather than by the storage key.Fix: install the crdt, and seed the allocation at the derived address — deriving it from the prover's own filter is what makes the non-global case miss the empty-filter lookup and be rejected. The assertion now pins the rejection reason; a bare
is_err()passes just as well when the op is refused for an unrelated reason, which is how this dimension went silently inert.cargo nextest run -p quil-engine --test materialize_harness→ 9/9 pass. The four othersplit_fixtureusers pass with the gate now live, so it accepts the legitimate global prover rather than merely rejecting everything.Out of scope: whether the gate should fail open when the hypergraph slot is empty. That is a product decision; this only makes the test exercise the behaviour as written.