Harden propose_admin_transfer against degenerate proposals
Description
propose_admin_transfer in contracts/escrow/src/lib.rs stores any new_admin under DataKey::PendingAdmin without validating it. Proposing the current admin as the new admin is accepted, creating a pointless pending entry that an off-chain monitor will flag as an in-flight handover that never changes anything. There is also no guard that a stale PendingAdmin is cleared when a handover completes via a different path. This issue adds input validation so only meaningful proposals are stored.
Requirements and context
- Repository scope:
Agentpay-Org/Agentpay-contracts only.
- In
propose_admin_transfer, reject a new_admin equal to the current admin with a new typed error (e.g. InvalidAdminProposal, next free code, append-only).
- Confirm
accept_admin_transfer clears PendingAdmin (it does) and add a guard/test that no stale pending entry can survive a successful rotation.
- Decide and document whether re-proposing the same pending address is a no-op or an error; keep the two-step handover semantics intact.
- Keep all existing handover behaviour and error codes unchanged for the valid paths.
Suggested execution
- Fork the repo and create a branch
git checkout -b security/contracts-admin-proposal-validation- Implement changes
- Write code in:
contracts/escrow/src/lib.rs — validation in propose_admin_transfer + new error variant.
- Write comprehensive tests in:
contracts/escrow/src/test.rs — proposing current admin panics with the new code; valid proposal still works; pending cleared after accept.
- Add documentation: document the proposal validation rules in
README.md.
- Include NatSpec-style doc comments (
///) matching the existing style in lib.rs.
- Validate security: no lockout path introduced, two-step handover still completable.
- Test and commit
Test and commit
- Run
cargo fmt --all -- --check, cargo build, and cargo test.
- Cover edge cases: propose self, propose then re-propose a different address, accept after a valid proposal.
- Include the full
cargo test output and a short security notes section in the PR description.
Example commit message
security: reject self-targeted proposals in propose_admin_transfer
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.
Harden propose_admin_transfer against degenerate proposals
Description
propose_admin_transferincontracts/escrow/src/lib.rsstores anynew_adminunderDataKey::PendingAdminwithout validating it. Proposing the current admin as the new admin is accepted, creating a pointless pending entry that an off-chain monitor will flag as an in-flight handover that never changes anything. There is also no guard that a stalePendingAdminis cleared when a handover completes via a different path. This issue adds input validation so only meaningful proposals are stored.Requirements and context
Agentpay-Org/Agentpay-contractsonly.propose_admin_transfer, reject anew_adminequal to the current admin with a new typed error (e.g.InvalidAdminProposal, next free code, append-only).accept_admin_transferclearsPendingAdmin(it does) and add a guard/test that no stale pending entry can survive a successful rotation.Suggested execution
git checkout -b security/contracts-admin-proposal-validationcontracts/escrow/src/lib.rs— validation inpropose_admin_transfer+ new error variant.contracts/escrow/src/test.rs— proposing current admin panics with the new code; valid proposal still works; pending cleared after accept.README.md.///) matching the existing style inlib.rs.Test and commit
cargo fmt --all -- --check,cargo build, andcargo test.cargo testoutput and a short security notes section in the PR description.Example commit message
security: reject self-targeted proposals in propose_admin_transferGuidelines
Community & contribution rewards