fix(wpi-token): authenticate admin from stored state, not caller-supplied argument

[dannyy2000]

Summary

• Closes Issue 17 — Admin functions authenticate the passed-in argument instead of stored state #17. mint, mint_from_deposit, burn, set_admin, set_paused, set_volume_limit_admin, configure_volume_limits, override_volume_limit, and upgrade all took an admin: Address parameter, compared it against read_admin(&env) (or read_volume_limit_admin), and only then called .require_auth() on it. That couples correctness to argument order and leaves room for a caller-supplied address to be silently authenticated in place of the real admin.
• These functions now drop the admin parameter entirely. Internally they call require_admin(&env) / require_volume_limit_admin(&env), which read the address from storage and call .require_auth() on that — there is nothing left for a caller to pass in that could stand in for the real admin.
• Error::NotAdmin is kept (unused) so the existing contract error-code numbering doesn't shift for anyone decoding error 1.

Test plan

• cargo build (whole workspace) — compiles cleanly.
• Added tests asserting a non-admin signer is rejected regardless of what address it attempts to authenticate as (non_admin_signer_cannot_authenticate_mint, ..._configure_volume_limits, ..._override_volume_limit, bridge_admin_cannot_authenticate_as_volume_limit_admin_after_rotation), using explicit mock_auths for a specific non-admin address rather than mock_all_auths.
• Updated existing tests (only_override_can_lift_a_tripped_circuit_breaker, volume_limit_admin_is_independent_from_bridge_admin, proptest property tests) to the new call signatures.
• cargo test -p wpi-token — could not run in my sandbox: soroban-sdk/soroban-env-host 23.0.1 pull both ed25519-dalek 2.2.0 and 3.0.0 into the dependency graph with incompatible rand_core requirements, which fails to compile under the testutils feature. This reproduces identically on main before this change (verified via git stash), so it's a pre-existing lockfile/dependency issue unrelated to this PR, and I had no network access to bump the lock. Please run the test suite in CI/locally to confirm.
• Updated scripts/quickstart.sh, testnet_e2e_simulation.sh, and README.md invocations to drop the now-removed --admin flags (kept it where initialize still legitimately takes it).

Summary by CodeRabbit

• New Features

  • Simplified privileged token operations by removing the need to provide administrator addresses in minting, burning, pausing, upgrades, and volume-limit actions.
  • Administrative actions now authenticate against the configured administrator automatically.

• Bug Fixes

  • Updated bridge and testnet workflows to use the revised token operation format, preserving mint, burn, transfer, and balance verification behavior.

• Documentation

  • Updated deployment and invocation examples to reflect the streamlined authorization process.

[coderabbitai]

📝 Walkthrough

Walkthrough

WpiToken privileged entrypoints now authenticate stored administrators instead of caller-supplied admin arguments. Tests and README, quickstart, and testnet invocation examples were updated to match the revised contract interface.

Changes

Stored-admin authorization

Layer / File(s)	Summary
Stored-admin entrypoint authorization Stellar-contracts-v1/wpi-token/src/lib.rs	Admin helpers authenticate stored administrators, and privileged mint, burn, volume-limit, pause, role-management, and upgrade methods no longer accept explicit admin parameters.
Authorization and behavior test updates Stellar-contracts-v1/wpi-token/src/test.rs	Tests update client calls, mocked authorization inputs, role rotation checks, idempotency coverage, property tests, and balance assertions for the revised signatures.
Script and documentation invocation updates Stellar-contracts-v1/README.md, Stellar-contracts-v1/scripts/quickstart.sh, Stellar-contracts-v1/testnet_e2e_simulation.sh	Operational examples remove explicit admin arguments from volume-limit, mint, and burn invocations.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• Pi-Defi-world/Wpi#41: Updates the same testnet simulation script’s wPi mint and burn invocations.
• Pi-Defi-world/Wpi#51: Adds the volume-limit circuit-breaker interfaces that this change updates.
• Pi-Defi-world/Wpi#52: Modifies related wpi-token entrypoint signatures and privileged flows.

Suggested reviewers: funds0033-cmyk

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR removes admin parameters from the admin-gated functions and updates tests to verify non-admin rejection under the new auth flow.
Out of Scope Changes check	✅ Passed	The documentation, scripts, and tests all support the same admin-authentication refactor, with no clear unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: WPI token admin actions now authenticate from stored contract state instead of caller-supplied admin arguments.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

Stellar-contracts-v1/wpi-token/src/test.rs (1)

166-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extend non-admin-spoof coverage to the remaining admin-gated entrypoints.

Only mint, configure_volume_limits, and override_volume_limit get a dedicated "attacker mocks their own auth" test. burn, mint_from_deposit, set_admin, set_paused, set_volume_limit_admin, and upgrade were changed identically but have no equivalent regression test proving a non-admin signer can't authenticate them.

♻️ Suggested pattern to replicate for the remaining entrypoints

#[test]
#[should_panic]
fn non_admin_signer_cannot_authenticate_burn() {
    let env = Env::default();
    let (_admin, client, user) = setup(&env, 10, 10, 10);
    let destination = BytesN::from_array(&env, &[9; 32]);
    let attacker = Address::generate(&env);

    client
        .mock_auths(&[MockAuth {
            address: &attacker,
            invoke: &MockAuthInvoke {
                contract: &client.address,
                fn_name: "burn",
                args: (&user, &1i128, &destination).into_val(&env),
                sub_invokes: &[],
            },
        }])
        .burn(&user, &1, &destination);
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Stellar-contracts-v1/wpi-token/src/test.rs` around lines 166 - 230, Add
#[test] and #[should_panic] regression tests matching the existing non-admin
signer patterns for burn, mint_from_deposit, set_admin, set_paused,
set_volume_limit_admin, and upgrade. In each test, use setup, generate an
attacker address, configure mock_auths for that entrypoint with matching
arguments, and invoke it through the client so a non-admin signer is rejected.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Stellar-contracts-v1/testnet_e2e_simulation.sh`:
- Around line 91-98: Update the wPi `burn` invocation in the Bridge Withdrawal
step to provide the required `pi_destination` argument in addition to `--from
$ALICE_PUB` and `--amount $SWAP_AMOUNT`, using Alice’s intended Pi destination
so the simulation completes successfully.

---

Nitpick comments:
In `@Stellar-contracts-v1/wpi-token/src/test.rs`:
- Around line 166-230: Add #[test] and #[should_panic] regression tests matching
the existing non-admin signer patterns for burn, mint_from_deposit, set_admin,
set_paused, set_volume_limit_admin, and upgrade. In each test, use setup,
generate an attacker address, configure mock_auths for that entrypoint with
matching arguments, and invoke it through the client so a non-admin signer is
rejected.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f6deb509-3132-4734-a787-1fbb23f621d4

📥 Commits

Reviewing files that changed from the base of the PR and between 8e27491 and 7bfb3a6.

📒 Files selected for processing (5)

• Stellar-contracts-v1/README.md
• Stellar-contracts-v1/scripts/quickstart.sh
• Stellar-contracts-v1/testnet_e2e_simulation.sh
• Stellar-contracts-v1/wpi-token/src/lib.rs
• Stellar-contracts-v1/wpi-token/src/test.rs

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

burn invocation is missing the required pi_destination argument.

burn requires (from, amount, pi_destination); this call only supplies --from and --amount, so it will fail at invocation time and the "Bridge Withdrawal" step of the simulation won't complete.

🐛 Proposed fix

 soroban contract invoke --id $WPI_ID --source relayer --network $NETWORK -- \
-  burn --from $ALICE_PUB --amount $SWAP_AMOUNT
+  burn --from $ALICE_PUB --amount $SWAP_AMOUNT --pi_destination <32-byte-hex-destination>

🧰 Tools

🪛 Shellcheck (0.11.0)

[info] 97-97: Double quote to prevent globbing and word splitting.

(SC2086)

---

[info] 98-98: Double quote to prevent globbing and word splitting.

(SC2086)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Stellar-contracts-v1/testnet_e2e_simulation.sh` around lines 91 - 98, Update
the wPi `burn` invocation in the Bridge Withdrawal step to provide the required
`pi_destination` argument in addition to `--from $ALICE_PUB` and `--amount
$SWAP_AMOUNT`, using Alice’s intended Pi destination so the simulation completes
successfully.