feat(claim): add read-only view - #447
Merged
mikewheeleer merged 2 commits intoJul 30, 2026
Merged
Conversation
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.
Closes #429
Summary
The issue asks for "a read-only view exposing the current claim state... a sane default before init... covered in tests." A view that satisfies this already exists in
src/lib.rs:get_pending_admin_info(env: Env) -> PendingAdminInfo, returning:— an aggregated, single-call snapshot of the queued admin-transfer ("claim") state, defaulting to
pending: None, eta: Nonewhen nothing is queued. Its own doc comment already describes it as "a consistent snapshot" of the two individual getters (get_pending_admin,get_pending_admin_eta).The actual gap: this function had zero test coverage anywhere in the suite — grep confirms it was never called outside its own definition. This PR adds that missing coverage instead of introducing a duplicate/competing view, since one already exists and matches the issue's ask exactly.
Changes
Three new tests in
mod test:test_pending_admin_info_default_before_any_transfer— saneNone/Nonedefault before any transfer is proposed.test_pending_admin_info_reflects_queued_transfer— afterpropose_admin_transfer(with a timelock set), the aggregated view matches both individual getters (info.pending == get_pending_admin(),info.eta == get_pending_admin_eta()) and the expected values.test_pending_admin_info_clears_after_accept— reverts to the default once the transfer completes, mirroring the individual-getter coverage in the existingtest_admin_transfer_flow.No production code changed.
Note on this repo's domain terminology
Same reinterpretation as #422/#432: "claim" maps onto the existing two-step admin-transfer flow, whose own
accept_admin_transferdoc comment already calls it the step where "the pending admin claims the role."Verification blocker (pre-existing, unrelated to this change)
Same pre-existing, unrelated compile break documented in #422 / PR #444 (90 errors from duplicate function definitions in
impl StableRouteRouter, confirmed viagit stashA/B rebuild onmain) —cargo testcannot actually run these new tests. What I could verify:cargo fmt -- --check: no new formatting diffs near the added lines.get_pending_admin_info's implementation (env.storage().instance().get(&DataKey::PendingAdmin)/env.storage().persistent().get(&DataKey::PendingAdminEta)) and against the existing, passingtest_admin_transfer_flow/test_timelock_allows_accept_after_delaytests that exercise the same storage slots with the same timestamps/timelock values (e.g.proposeatt=1_000withtimelock=100⇒eta = Some(1_100), matchingtest_timelock_blocks_early_accept's already-passing assertion).Flagging this so a maintainer can re-run
cargo testand confirm these three pass once the pre-existing duplicate-definition issue is fixed.