Skip to content

Extract the duplicated require_admin storage-read-and-auth block into a helper #92

Description

@mikewheeleer

Refactor the repeated admin read-and-require_auth idiom into require_admin

Description

At least a dozen entrypoints in contracts/escrow/src/lib.rsset_service_price, remove_service_price, register_service, unregister_service, set_agent_allowed, set_agent_blocked, set_service_disabled, set_service_metadata, register_service_with_metadata, the bounds/window setters, pause, unpause, migrate_v1_to_v2, and others — open with the identical block: let admin = env.storage().persistent().get(&DataKey::Admin).unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth();. This copy-paste is the single most-repeated pattern in the file and an easy place to introduce an ordering bug in a new entrypoint. This issue extracts it into one private helper.

Requirements and context

  • Repository scope: Agentpay-Org/Agentpay-contracts only.
  • Add a private fn require_admin(env: &Env) -> Address that reads DataKey::Admin, panics NotInitialized (#3) if absent, calls require_auth(), and returns the admin address; route every admin entrypoint through it.
  • Behaviour must be byte-for-byte identical: same #3 error, same auth-then-pause ordering at each call site (the helper performs only the admin read + auth; the ensure_not_paused call remains where it currently is).
  • This is a pure refactor — no public API or semantic change; the existing test suite must pass unchanged.
  • Add a rationale comment so new entrypoints reuse the helper rather than re-inlining the block.

Suggested execution

  • Fork the repo and create a branch
  • git checkout -b refactor/contracts-require-admin-helper
  • Implement changes
    • Write code in: contracts/escrow/src/lib.rsrequire_admin helper, replace duplicated blocks.
    • Write comprehensive tests in: contracts/escrow/src/test.rs — existing tests pass; add a regression test that a representative entrypoint still panics #3 when uninitialized and rejects a wrong signer.
    • Add documentation: note the helper convention in a module comment.
    • Include NatSpec-style doc comments (///) on the helper.
    • Validate security: no auth check dropped, ordering preserved relative to pause checks.
  • Test and commit

Test and commit

  • Run cargo fmt --all -- --check, cargo build, and cargo test.
  • Cover edge cases: uninitialized contract panics #3, wrong signer rejected, correct admin succeeds.
  • Include the full cargo test output and a short security notes section in the PR description.

Example commit message

refactor: extract require_admin helper for the admin read-and-auth idiom

Guidelines

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

Community & contribution rewards

  • 💬 Join the AgentPay community on Discord for questions, reviews, and faster merges: https://discord.gg/eXvRKkgcv
  • ⭐ 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
    No fields configured for Feature.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions