Skip to content

Validate the proposed admin differs from any active pending admin in propose_admin #556

Description

@mikewheeleer

Validate the proposed admin differs from any active pending admin in propose_admin

Description

propose_admin rejects a proposal where new_admin == current_admin (NewAdminSameAsCurrent), but it does not check the new proposal against any already-pending proposal. Re-proposing the identical pending address silently resets PendingAdminExpiry to a fresh window with no event distinction from a genuine change, and silently overwrites a different in-flight successor. Both cases deserve explicit, observable handling.

This issue tightens propose_admin so re-proposing the exact pending address is rejected as a no-op typed error, and replacing a different pending proposal emits a distinct supersede signal in the event payload — making handover races auditable.

Requirements and context

  • Repository scope: Liquifact/Liquifact-contracts only.
  • In propose_admin, read any existing DataKey::PendingAdmin; if it equals new_admin, reject with a new typed error (e.g. PendingAdminUnchanged), keeping codes append-only.
  • When a different pending proposal exists and is being replaced, include the superseded address in the emitted AdminProposedEvent (or a companion event) so observers can detect the overwrite.
  • Preserve the existing NewAdminSameAsCurrent guard, admin require_auth, and expiry-window behavior for genuinely new proposals.
  • Do not weaken the ability to legitimately change the pending successor.

Suggested execution

  • Fork the repo and create a branch
  • git checkout -b security/contracts-propose-admin-pending-guard
  • Implement changes
    • Write code in: escrow/src/lib.rs — the new guard, error variant, and event field in propose_admin.
    • Write comprehensive tests in: escrow/src/tests/admin.rs — assert rejection of an identical re-proposal, the supersede event on a changed proposal, the preserved same-as-current guard, and expiry refresh semantics.
    • Add documentation: update the two-step handover docs to describe the new guard and supersede signal.
    • Include NatSpec-style doc comments (///) describing the new error path.
    • Validate security assumptions: no silent overwrite, observable supersede, admin-only.
  • Test and commit

Test and commit

  • Run cargo fmt --all -- --check, cargo build, and cargo test.
  • Cover edge cases and failure paths: identical re-proposal, different successor replacement, same-as-current, non-admin caller, expiry refresh.
  • Include the full cargo test output and a short security notes section in the PR description.

Example commit message

fix: reject identical pending re-proposal and signal supersede in propose_admin with tests

Guidelines

  • Minimum 95 percent test coverage for impacted modules.
  • Clear, reviewer-focused documentation.
  • Timeframe: 96 hours.

Community & contribution rewards

  • 💬 Join the Liquifact community on Discord for questions, reviews, and faster merges: https://discord.gg/JrGPH4V3
  • ⭐ This is a GrantFox OSS / Official Campaign task and may be rewarded. When your PR is merged you'll be prompted to rate the project — if this issue and the maintainers helped you ship, we'd be grateful for a 5-star rating. Clear questions in Discord and tidy, well-tested PRs are the fastest path to a merge and a reward.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions