Add a dedicated Unauthorized error instead of overloading NotPendingAdmin
Description
Both settle and transfer_service_ownership in contracts/escrow/src/lib.rs reject an unauthorized caller by panicking with EscrowError::NotPendingAdmin (#6) — an error whose name and doc comment are specifically about the two-step admin handover in accept_admin_transfer. A code comment even flags the reuse (// reuse: unauthorized caller). Client SDKs decoding #6 cannot tell a failed admin acceptance from a generic unauthorized settle/ownership attempt, and operators reading logs are misled. This issue introduces a precise error while keeping codes append-only.
Requirements and context
- Repository scope:
Agentpay-Org/Agentpay-contracts only.
- Add a new
EscrowError::Unauthorized variant (next free code, append-only) and use it for the unauthorized-caller branch in both settle and transfer_service_ownership.
- Do not renumber or change the meaning of
#6; accept_admin_transfer keeps NotPendingAdmin for its genuine wrong-pending-caller case.
- Update the doc comments on
settle and transfer_service_ownership (which currently document the #6 reuse) to reference the new error.
- Keep all valid-path behaviour unchanged.
Suggested execution
- Fork the repo and create a branch
git checkout -b security/contracts-unauthorized-error
- Implement changes
- Write code in:
contracts/escrow/src/lib.rs — new Unauthorized variant + swap in settle and transfer_service_ownership.
- Write comprehensive tests in:
contracts/escrow/src/test.rs — unauthorized settle and transfer now panic the new code; accept_admin_transfer wrong-caller still panics #6.
- Add documentation: note the error semantics in
README.md.
- Include NatSpec-style doc comments (
///) matching the existing style in lib.rs.
- Validate security: no code reuse, stable SDK decoding for the admin-handover path.
- Test and commit
Test and commit
- Run
cargo fmt --all -- --check, cargo build, and cargo test.
- Cover edge cases: stranger settles (rejected new code), stranger transfers (rejected new code), wrong pending admin accepts (still
#6).
- Include the full
cargo test output and a short security notes section in the PR description.
Example commit message
security: add dedicated Unauthorized error for settle and ownership callers
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.
Add a dedicated Unauthorized error instead of overloading NotPendingAdmin
Description
Both
settleandtransfer_service_ownershipincontracts/escrow/src/lib.rsreject an unauthorized caller by panicking withEscrowError::NotPendingAdmin (#6)— an error whose name and doc comment are specifically about the two-step admin handover inaccept_admin_transfer. A code comment even flags the reuse (// reuse: unauthorized caller). Client SDKs decoding#6cannot tell a failed admin acceptance from a generic unauthorized settle/ownership attempt, and operators reading logs are misled. This issue introduces a precise error while keeping codes append-only.Requirements and context
Agentpay-Org/Agentpay-contractsonly.EscrowError::Unauthorizedvariant (next free code, append-only) and use it for the unauthorized-caller branch in bothsettleandtransfer_service_ownership.#6;accept_admin_transferkeepsNotPendingAdminfor its genuine wrong-pending-caller case.settleandtransfer_service_ownership(which currently document the#6reuse) to reference the new error.Suggested execution
git checkout -b security/contracts-unauthorized-errorcontracts/escrow/src/lib.rs— newUnauthorizedvariant + swap insettleandtransfer_service_ownership.contracts/escrow/src/test.rs— unauthorized settle and transfer now panic the new code;accept_admin_transferwrong-caller still panics#6.README.md.///) matching the existing style inlib.rs.Test and commit
cargo fmt --all -- --check,cargo build, andcargo test.#6).cargo testoutput and a short security notes section in the PR description.Example commit message
security: add dedicated Unauthorized error for settle and ownership callersGuidelines
Community & contribution rewards