fix(contracts): separate admin, keeper and fee-recipient roles, add pause

[waledev1]

Overview

This splits one key that currently does everything.

Today locked_in, target_savings, spend_save and mock_pool each keep a single Admin address in their own DataKey. target_savings stores that same address as both its Keeper and the default FeeRecipient, and the scheduled entrypoints (process_period, deposit_to_blend, withdraw_from_blend) authenticate that one address. The hot key the keeper service holds is therefore also the cold key that rewrites APY/yield config, replaces the fee recipient, grants itself more roles and — once pause exists — freezes the vault. A compromise of the keeper server is a total compromise of the protocol, and every fee/forfeit flow pays the admin.

This PR adopts OpenZeppelin stellar-access (AccessControl) and the OZ pausable module across all four in-scope contracts:

Role	Can do	Key lives
Admin	config changes, role grants, pause/unpause	cold, offline
Keeper	only the scheduled functions (process_period, blend sweeps)	hot, on the keeper server
Fee recipient	receives income, no powers at all	anywhere

Two-step admin transfer comes along with AccessControl on all four contracts, and every state-changing user-facing function is now pause-guarded while every read stays callable.

A note on OZ tags, because it is the one line outside contracts/: tags are per-package in OpenZeppelin/stellar-contracts. The standalone stellar-pausable / stellar-pausable-macros crates only exist on the v0.3.0 line, whose workspace pins soroban-sdk = "22.0.6". These contracts are on the soroban-sdk = "23.0.3" line, so that crate cannot be linked here — the two Env types are incompatible and it does not compile. On the v0.5.1 line — the tag stellar-access and stellar-macros already sit on — the pausable module ships inside stellar-contract-utils, which is pinned to soroban-sdk 23.0.2 and links cleanly. That is what this PR adopts; the workspace's stellar-pausable entries are left declared and unused, and unifying every OZ dependency onto one tag stays #35's job.

Related Issue

Closes #39

Changes

Workspace

• [MODIFY] Cargo.toml

  • Adds stellar-contract-utils at tag v0.5.1 — the pausable module for the soroban-sdk 23 line, matching the tag stellar-access / stellar-macros are already on.
  • Documents why the existing stellar-pausable / stellar-pausable-macros (v0.3.0, soroban-sdk 22) entries cannot be linked from these contracts. They are left declared exactly as before.

• [MODIFY] Cargo.lock

  • Picked up by the cargo test run below: the three OZ v0.5.1 packages that are now referenced, plus their entries in the four crates' dependency lists. The diff against staging is additive only — 38 lines added, 0 removed.

locked_in

• [MODIFY] contracts/locked_in/src/lib.rs

  • Constructor is now __constructor(env, admin, fee_recipient: Option<Address>, usdc_token): the admin is registered with OZ access_control::set_admin, and the fee recipient is stored only when Some — it never falls back to the admin.
  • set_apy_tier, set_pool and the new set_fee_recipient are admin-gated through require_admin, which compares the explicit caller against access_control::get_admin and authenticates — so a valid keeper key is rejected. Reading the live admin also means a completed two-step transfer takes effect immediately.
  • deposit_to_blend / withdraw_from_blend are #[only_role(caller, "keeper")].
  • lock / unlock carry #[when_not_paused]. get_lock, get_user_locks, blend_position, get_apy_tiers, get_apy_for_duration, pool, usdc_token, fee_recipient stay callable while paused.
  • Implements OZ Pausable (pause / unpause are admin-gated and never pause-blocked) and #[default_impl] AccessControl for role grants and the two-step admin transfer.

• [MODIFY] contracts/locked_in/src/types.rs

  • Drops DataKey::Admin (OZ now owns the admin) and adds DataKey::FeeRecipient, holding Option<Address>.

• [MODIFY] contracts/locked_in/src/test.rs

  • test_fee_recipient_is_distinct_and_not_the_admin, test_fee_recipient_defaults_to_nothing_not_admin
  • test_keeper_cannot_call_admin_functions, test_keeper_can_call_keeper_functions, test_unprivileged_caller_cannot_call_privileged_functions
  • test_paused_rejects_deposits_and_withdrawals_but_not_reads
  • test_two_step_admin_transfer
  • Existing tests updated for the new constructor and the caller-taking privileged functions.

target_savings

• [MODIFY] contracts/target_savings/src/lib.rs

  • Constructor is now __constructor(env, admin, fee_recipient: Option<Address>, usdc_token).
  • set_pool, set_fee_recipient, set_yield_apy are admin-gated via require_admin; process_period, deposit_to_blend and withdraw_from_blend are #[only_role(caller, "keeper")] — process_period now takes the caller, so the scheduled pull is signed by the keeper role instead of the admin.
  • create_target, manual_deposit, withdraw carry #[when_not_paused]; every read (get_target, get_user_goals, get_goal_yield, blend_position, pool, yield_apy_bps, fee_recipient) stays callable while paused.
  • The early-withdrawal forfeit is paid to the configured fee recipient when one is set and otherwise stays in the vault — it is never sent to the admin implicitly.
  • Implements OZ Pausable and #[default_impl] AccessControl.

• [MODIFY] contracts/target_savings/src/types.rs

  • Drops DataKey::Admin and the bespoke Keeper entry; the admin, the keeper role and the optional FeeRecipient now come from OZ AccessControl.

• [MODIFY] contracts/target_savings/src/test.rs

  • The same seven role / pause / transfer tests as locked_in.
  • test_early_withdrawal_charges_forfeit now asserts the forfeit lands on the distinct fee recipient.

spend_save

• [MODIFY] contracts/spend_save/src/lib.rs

  • Constructor is now __constructor(env, admin, fee_recipient: Option<Address>, usdc_token); the new set_fee_recipient is admin-gated.
  • enroll, spend, withdraw carry #[when_not_paused]; get_position, is_withdrawal_day_now, current_day_utc, blend_position, fee_recipient stay callable while paused.
  • deposit_to_blend / withdraw_from_blend move from "admin signs" to #[only_role(caller, "keeper")].
  • Implements OZ Pausable and #[default_impl] AccessControl.

• [MODIFY] contracts/spend_save/src/types.rs

  • Drops DataKey::Admin and adds DataKey::FeeRecipient.

• [MODIFY] contracts/spend_save/src/test.rs

  • Role, pause and two-step-transfer coverage as above, with the TS_28TH withdrawal path exercised while paused.

mock_pool

• [MODIFY] contracts/mock_pool/src/lib.rs

  • Constructor is now __constructor(env, admin, fee_recipient: Option<Address>, usdc_token, apy_bps).
  • set_apy, fund_reserve and the new set_fee_recipient are admin-gated via require_admin.
  • supply / withdraw — the pool's only state-changing user-facing functions — carry #[when_not_paused]; get_position, get_principal, apy_bps, reserve_balance, fee_recipient stay callable while paused.
  • The pool has no scheduled task of its own, so it defines no keeper-role function; a keeper key is still rejected from every admin function.
  • Implements OZ Pausable and #[default_impl] AccessControl.

• [MODIFY] contracts/mock_pool/src/types.rs

  • Drops DataKey::Admin and adds DataKey::FeeRecipient.

• [MODIFY] contracts/mock_pool/src/test.rs

  • Existing supply / withdraw / yield tests updated for the new constructor and caller-taking admin functions, plus role, pause and two-step-transfer coverage.

ABI impact and follow-ups

The role model needs a caller on every privileged entrypoint, so this is a breaking change to four contract interfaces rather than a purely internal refactor:

• Constructors take an extra fee_recipient: Option<Address>: __constructor(admin, fee_recipient, usdc_token[, apy_bps]).
• Admin functions gained a leading caller: Address: set_apy_tier, set_pool in locked_in; set_pool, set_yield_apy in target_savings; set_apy, fund_reserve in mock_pool. set_fee_recipient is new on locked_in, spend_save and mock_pool, and on target_savings it changes from set_fee_recipient(recipient) to set_fee_recipient(caller, Option<Address>).
• deposit_to_blend / withdraw_from_blend gained a leading caller: Address, and so did target_savings::process_period — the scheduled pull is now signed by the keeper role, so the keeper service must pass its own address.
• fee_recipient() now returns Option<Address>, the per-contract admin() getter is gone (OZ get_admin() returns Option<Address>), and target_savings' set_keeper / keeper() are replaced by grant_role / has_role on the "keeper" symbol.
• mock_pool gains set_fee_recipient / fee_recipient; it had no fee-recipient concept before, and the issue requires one on all four.
• New on all four: pause / unpause / paused, plus OZ grant_role, revoke_role, renounce_role, set_role_admin, has_role, get_admin, transfer_admin_role, accept_admin_transfer, renounce_admin.

packages/* bindings and the keeper/ call sites are consequently stale and need regeneration plus a call-site update for process_period. They are deliberately untouched here: the issue scopes the change to the contracts and asks for packages/ to be left alone.

Verification Results

Implemented via GitHub Contents/Git API (no local clone).
For a real signal, the four modified crates plus the untouched `plans` crate
were mirrored into a scratch workspace outside the repo (same workspace
Cargo.toml, same Cargo.lock) and this was run there against the exact bytes
committed on this branch:

  $ rustc --version
  rustc 1.89.0 (29483883e 2025-08-04)      <- the version rust-toolchain.toml pins

  $ cargo test --workspace --locked
      Running unittests src/lib.rs (locked_in)
  test result: ok. 15 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
      Running unittests src/lib.rs (mock_pool)
  test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
      Running unittests src/lib.rs (plans)          <- untouched, out of scope
  test result: ok. 26 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
      Running unittests src/lib.rs (spend_save)
  test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
      Running unittests src/lib.rs (target_savings)
  test result: ok. 17 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

  -> 84 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (workspace exit code 0)

  $ cargo build --workspace --target wasm32v1-none --release --locked
  Finished `release` profile [optimized] target(s) in 1m 14s  (exit code 0, no errors, no warnings)
  locked_in.wasm  target_savings.wasm  spend_save.wasm  mock_pool.wasm  plans.wasm

The deployable artifacts therefore build too — the pause and role macros expand
cleanly for `wasm32v1-none`, not just for the host test profile.

Both runs used the toolchain `rust-toolchain.toml` pins: `rustc 1.89.0
(29483883e 2025-08-04)`.

Cargo.lock is the lock that run produced: additive against staging's (38 lines
added, 0 removed), so `--locked` is satisfied by both commands above.

The tests that prove the criteria are on this branch:

  locked_in / target_savings / spend_save
    test_keeper_cannot_call_admin_functions
    test_keeper_can_call_keeper_functions
    test_unprivileged_caller_cannot_call_privileged_functions
    test_paused_rejects_deposits_and_withdrawals_but_not_reads
    test_fee_recipient_is_distinct_and_not_the_admin
    test_fee_recipient_defaults_to_nothing_not_admin
    test_two_step_admin_transfer
  mock_pool
    test_keeper_key_cannot_call_admin_functions
    test_unprivileged_caller_cannot_call_privileged_functions
    test_paused_rejects_deposits_and_withdrawals_but_not_reads
    test_fee_recipient_is_distinct_and_not_the_admin
    test_fee_recipient_defaults_to_nothing_not_admin
    test_two_step_admin_transfer

Two details worth flagging to a reviewer:

* `set_role_admin` and `renounce_admin` take no caller argument by design — OZ
  gates them on the stored admin's own signature. The unprivileged-caller tests
  therefore prove that gate by dropping every authorization
  (`env.mock_auths(&[])`) and asserting both calls fail.
* `grant_role` / `revoke_role` do take a caller, and the same tests assert an
  unrelated address is rejected from both.

Acceptance Criteria	Status
locked_in, target_savings, spend_save, mock_pool use OZ AccessControl	Met — each ends with #[default_impl] #[contractimpl] impl AccessControl for <Contract> {}, and each constructor calls access_control::set_admin
Keeper role can call keeper functions and nothing else — test proving a keeper key is rejected from every admin function	Met — #[only_role(caller, "keeper")] on every scheduled entrypoint; test_keeper_cannot_call_admin_functions (locked_in / target_savings / spend_save) and test_keeper_key_cannot_call_admin_functions (mock_pool) assert the keeper key is rejected from every admin function
Fee recipient is a distinct address set at construction, defaulting to nothing rather than to the admin	Met — fee_recipient: Option<Address> on every constructor, stored only when Some; test_fee_recipient_defaults_to_nothing_not_admin asserts fee_recipient().is_none() and test_fee_recipient_is_distinct_and_not_the_admin asserts it differs from get_admin()
Two-step admin transfer available on all four	Met — OZ transfer_admin_role / accept_admin_transfer via #[default_impl]; test_two_step_admin_transfer in all four modules proves the old admin keeps control until the nominee accepts and loses it afterwards
Pause guard on every state-changing user-facing function; reads unaffected	Met — #[when_not_paused] on lock/unlock, create_target/manual_deposit/withdraw, enroll/spend/withdraw, supply/withdraw; admin functions and every read stay open while paused
Test proving a paused contract rejects deposits and withdrawals	Met — test_paused_rejects_deposits_and_withdrawals_but_not_reads in all four modules, which also asserts reads still succeed while paused and that unpausing restores writes
Test proving an unprivileged caller is rejected from every privileged function	Met — test_unprivileged_caller_cannot_call_privileged_functions in all four modules: every admin function, every keeper function, grant_role, revoke_role, pause, unpause, and (with authorizations dropped) set_role_admin / renounce_admin
cargo test --workspace passes	Met — 84 passed; 0 failed across the five workspace members (see Verification Results)

Closes #39