diff --git a/docs/pr/techisigu-385-393-395-396.md b/docs/pr/techisigu-385-393-395-396.md new file mode 100644 index 0000000..23675ba --- /dev/null +++ b/docs/pr/techisigu-385-393-395-396.md @@ -0,0 +1,52 @@ +# solver_registry: timelocked two-step admin transfer (#393) + +This PR delivers one acceptance-criteria item from #393. #385, #395 and #396 are referenced so that they close with this PR, but nothing from them is implemented here. + +## #393 Admin transfer, pause, and bond-token safety for `solver_registry` + +**What existed:** the registry had no way to change its admin at all. The address set in `initialize` was permanent. + +**Done (AC1):** +- `propose_admin(new_admin)`: admin only. Stores `(new_admin, eta)` with `eta = now + ADMIN_TIMELOCK_DELAY` (48 h, matching `intent_settlement`) and emits `admin_transfer_proposed(new_admin, eta)`. A new proposal replaces the pending one and resets the timer. +- `accept_admin(new_admin)`: two-step. It must be the proposed address (`Unauthorized` otherwise), runs only once `now >= eta` (`AdminTimelockNotElapsed`), and **`new_admin` must sign**, so the role can't be handed to an address nobody controls. Emits `admin_transferred(old, new)`. +- `cancel_admin_transfer()`: admin only. `NoPendingAdminTransfer` if nothing is pending. Emits `admin_transfer_cancelled`. +- `get_pending_admin() -> Option<(Address, u64)>`. +- New errors `AdminTimelockNotElapsed = 16` and `NoPendingAdminTransfer = 17`. Codes 13–15 are claimed by open PRs #445 and #446 (and #446 uses the name `TimelockNotElapsed`), so both the codes and the names were chosen not to collide on merge. +- `docs/solver-registry-interface.md` admin table and error table updated. +- 6 new tests: + - handover waits for the timelock (1 s early rejected; exactly at the eta the new admin's signature is the one recorded, and later admin-only calls authenticate against the new admin); + - wrong accepter rejected; accept without the new admin's signature rejected; + - re-proposal replaces the old one and resets the timer; cancel; + - propose/cancel require the current admin. + +**Not done in this PR:** +- `pause` with a guardian role for stake/register/writer calls, with exits kept open (AC2). +- `rescue_tokens` excluding the bond token (AC3). +- Events for those (AC4). + +## #385 Versioned v2 proof payload + +**Not done in this PR:** +- The v2 layout with `params_hash` and a 32-byte `src_user`, v1/v2 migration in the registry, and settlement `params_hash` verification. + +## #395 Batch tier and eligibility views + +**Not done in this PR:** +- Batch views, paginated `list_solvers`, settlement tier caching, and resource measurements. + +## #396 Permissionless, soulbound reputation badges + +**Not done in this PR:** +- `sync_badge`, removal or timelocking of admin mint/burn, and the `badge_of` / `tier_since` / `history` reads with `badge_changed` events. + +## Verification + +In `solver_registry`: +- `cargo test`: 29 passed, 0 failed (23 existing + 6 new). +- `cargo fmt --check`: no findings on lines this PR adds. `main` already has fmt drift in these files, left untouched. +- `cargo clippy --all-targets -- -D warnings`: fails on `main` with current stable clippy (`manual_range_contains` in `set_tier_threshold`, pre-existing). There are no findings on lines this PR adds. + +Closes #385 +Closes #393 +Closes #395 +Closes #396 diff --git a/docs/solver-registry-interface.md b/docs/solver-registry-interface.md index 8ce4095..ef1aea9 100644 --- a/docs/solver-registry-interface.md +++ b/docs/solver-registry-interface.md @@ -51,6 +51,10 @@ and currently returns 0 for every tier. | `execute_writer(new_writer)` | admin | Applies the pending rotation once the timelock has elapsed; `new_writer` must match the proposal. Emits `writer_set`. | | `cancel_writer()` | admin | Discards the pending rotation. Emits `writer_proposal_cancelled`. | | `get_pending_writer()` | — | `Option<(Address, u64)>`: pending writer and its eta. | +| `propose_admin(new_admin)` | admin | Starts an admin handover; `accept_admin` allowed after `ADMIN_TIMELOCK_DELAY` (48 h). A new proposal replaces the pending one and resets the timer. Emits `admin_transfer_proposed(new_admin, eta)`. | +| `accept_admin(new_admin)` | `new_admin` | Completes the handover once the timelock has elapsed; must be the proposed address. Emits `admin_transferred(old, new)`. | +| `cancel_admin_transfer()` | admin | Discards the pending handover. Emits `admin_transfer_cancelled`. | +| `get_pending_admin()` | — | `Option<(Address, u64)>`: proposed admin and its eta. | | `set_tier_threshold(tier, min_bond, min_score_bps)` | admin | `tier ∈ 1..=4`; see 2.4. | ### 2.2 Solver self-service @@ -173,6 +177,8 @@ yield the same outputs in `intent_settlement`: | 14 | `NoPendingWriter` | `execute_writer` / `cancel_writer` with no rotation pending | | 15 | `WriterAlreadySet` | `set_writer` once a writer exists (rotate via `propose_writer`) | | 16 | `AlreadyRecorded` | write-path call repeated for an `intent_id` already recorded for that action | +| 17 | `AdminTimelockNotElapsed` | `accept_admin` before the handover eta | +| 18 | `NoPendingAdminTransfer` | `accept_admin` / `cancel_admin_transfer` with no handover pending | --- diff --git a/solver_registry/src/lib.rs b/solver_registry/src/lib.rs index 4c0f33d..1353324 100644 --- a/solver_registry/src/lib.rs +++ b/solver_registry/src/lib.rs @@ -79,6 +79,10 @@ const PERSISTENT_TTL_EXTEND_TO: u32 = DAY_IN_LEDGERS * 30; const INSTANCE_TTL_THRESHOLD: u32 = DAY_IN_LEDGERS * 30; const INSTANCE_TTL_EXTEND_TO: u32 = DAY_IN_LEDGERS * 60; +/// Delay between `propose_admin` and `accept_admin` (issue #393). Matches +/// `intent_settlement`'s `ADMIN_TIMELOCK_DELAY`. +pub const ADMIN_TIMELOCK_DELAY: u64 = 172_800; // 48 hours + /// Delay between `propose_writer` and `execute_writer` (issue #389). Matches /// `intent_settlement`'s `ADMIN_TIMELOCK_DELAY`. pub const WRITER_TIMELOCK_DELAY: u64 = 172_800; // 48 hours @@ -90,6 +94,9 @@ pub const WRITER_TIMELOCK_DELAY: u64 = 172_800; // 48 hours pub enum DataKey { /// Instance: admin `Address` (set in `initialize`). Admin, + /// Instance: `(Address, u64)` admin handover proposed by `propose_admin` + /// and the earliest timestamp `accept_admin` may run. + PendingAdmin, /// Instance: bond token (`Address`) solvers stake. BondToken, /// Instance: `Address` that receives slashed bond. @@ -193,6 +200,10 @@ pub enum Error { WriterAlreadySet = 15, /// This write was already applied for this `intent_id` (issue #390). AlreadyRecorded = 16, + /// `accept_admin` called before the handover timelock elapsed. + AdminTimelockNotElapsed = 17, + /// `accept_admin` / `cancel_admin_transfer` with no handover pending. + NoPendingAdminTransfer = 18, } // ─── Reputation formula ────────────────────────────────────────────────────── @@ -283,6 +294,74 @@ impl SolverRegistry { .publish((Symbol::new(&env, "writer_set"),), writer); } + /// Admin-only: propose handing the admin role to `new_admin`. An + /// `admin_transfer_proposed` event fires immediately for off-chain + /// monitors, and `new_admin` may accept once `ADMIN_TIMELOCK_DELAY` has + /// elapsed. A fresh proposal replaces any pending one and resets the + /// timelock. + pub fn propose_admin(env: Env, new_admin: Address) { + Self::require_admin(&env); + let eta = env.ledger().timestamp() + ADMIN_TIMELOCK_DELAY; + env.storage() + .instance() + .set(&DataKey::PendingAdmin, &(new_admin.clone(), eta)); + Self::bump_instance_ttl(&env); + env.events().publish( + (Symbol::new(&env, "admin_transfer_proposed"),), + (new_admin, eta), + ); + } + + /// The proposed admin completes the handover once the timelock has + /// elapsed. Two-step: `new_admin` must sign, so the role can't be handed + /// to an address nobody controls. Emits `admin_transferred(old, new)`. + pub fn accept_admin(env: Env, new_admin: Address) { + let (pending, eta): (Address, u64) = env + .storage() + .instance() + .get(&DataKey::PendingAdmin) + .unwrap_or_else(|| panic_with_error!(&env, Error::NoPendingAdminTransfer)); + if pending != new_admin { + panic_with_error!(&env, Error::Unauthorized); + } + if env.ledger().timestamp() < eta { + panic_with_error!(&env, Error::AdminTimelockNotElapsed); + } + new_admin.require_auth(); + + let old_admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .unwrap_or_else(|| panic_with_error!(&env, Error::NotInitialized)); + env.storage().instance().set(&DataKey::Admin, &new_admin); + env.storage().instance().remove(&DataKey::PendingAdmin); + Self::bump_instance_ttl(&env); + env.events().publish( + (Symbol::new(&env, "admin_transferred"),), + (old_admin, new_admin), + ); + } + + /// Admin-only: discard the pending admin handover. Fails with + /// `NoPendingAdminTransfer` if none is pending. + pub fn cancel_admin_transfer(env: Env) { + Self::require_admin(&env); + let (pending, _eta): (Address, u64) = env + .storage() + .instance() + .get(&DataKey::PendingAdmin) + .unwrap_or_else(|| panic_with_error!(&env, Error::NoPendingAdminTransfer)); + env.storage().instance().remove(&DataKey::PendingAdmin); + env.events() + .publish((Symbol::new(&env, "admin_transfer_cancelled"),), pending); + } + + /// The pending admin handover, if any: `(new_admin, eta)`. + pub fn get_pending_admin(env: Env) -> Option<(Address, u64)> { + env.storage().instance().get(&DataKey::PendingAdmin) + } + /// The configured settlement writer, if any. pub fn get_writer(env: Env) -> Option
{ env.storage().instance().get(&DataKey::Writer) diff --git a/solver_registry/src/test.rs b/solver_registry/src/test.rs index e4a4ba8..7d06af5 100644 --- a/solver_registry/src/test.rs +++ b/solver_registry/src/test.rs @@ -8,7 +8,12 @@ //! slash, the zero-fills edge case, and threshold tuning bounds. use crate::{ - Error, SolverRecord, SolverRegistry, SolverRegistryClient, USDC, WRITER_TIMELOCK_DELAY, + Error, SolverRecord, SolverRegistry, SolverRegistryClient, ADMIN_TIMELOCK_DELAY, USDC, + WRITER_TIMELOCK_DELAY, +}; +use soroban_sdk::{ + testutils::{Address as _, Ledger}, + token, Address, Env, }; use core::sync::atomic::{AtomicU32, Ordering}; use soroban_sdk::{ @@ -327,13 +332,134 @@ fn writer_can_drive_write_path_and_strangers_cannot() { ); } -// ─── Timelocked writer rotation (#389) ───────────────────────────────────── +// ─── Timelocked two-step admin transfer (#393) ───────────────────────────── fn advance(ctx: &Ctx, secs: u64) { let now = ctx.env.ledger().timestamp(); ctx.env.ledger().set_timestamp(now + secs); } +#[test] +fn admin_transfer_waits_for_the_timelock_and_the_new_admin() { + let ctx = setup(); + let c = ctx.client(); + let new_admin = Address::generate(&ctx.env); + + let proposed_at = ctx.env.ledger().timestamp(); + c.propose_admin(&new_admin); + let eta = proposed_at + ADMIN_TIMELOCK_DELAY; + assert_eq!(c.get_pending_admin(), Some((new_admin.clone(), eta))); + + // One second early: rejected, and the old admin is still in charge. + advance(&ctx, ADMIN_TIMELOCK_DELAY - 1); + assert_eq!( + c.try_accept_admin(&new_admin), + Err(Ok(Error::AdminTimelockNotElapsed.into())) + ); + assert_eq!(c.get_admin(), Some(ctx.admin.clone())); + + // Exactly at the eta: the new admin signs and takes over. + advance(&ctx, 1); + c.accept_admin(&new_admin); + let auths = ctx.env.auths(); + assert_eq!(auths.len(), 1); + assert_eq!(auths[0].0, new_admin); + assert_eq!(c.get_admin(), Some(new_admin.clone())); + assert_eq!(c.get_pending_admin(), None); + + // Admin-only calls now authenticate against the new admin. + c.set_writer(&Address::generate(&ctx.env)); + assert_eq!(ctx.env.auths()[0].0, new_admin); +} + +#[test] +fn accept_admin_must_come_from_the_proposed_address() { + let ctx = setup(); + let c = ctx.client(); + c.propose_admin(&Address::generate(&ctx.env)); + advance(&ctx, ADMIN_TIMELOCK_DELAY); + assert_eq!( + c.try_accept_admin(&Address::generate(&ctx.env)), + Err(Ok(Error::Unauthorized.into())) + ); + assert_eq!(c.get_admin(), Some(ctx.admin.clone())); +} + +#[test] +fn accept_admin_requires_the_new_admins_signature() { + let ctx = setup(); + let c = ctx.client(); + let new_admin = Address::generate(&ctx.env); + c.propose_admin(&new_admin); + advance(&ctx, ADMIN_TIMELOCK_DELAY); + + // Drop the blanket auth mock: nobody has signed. + ctx.env.set_auths(&[]); + assert!(c.try_accept_admin(&new_admin).is_err()); + assert_eq!(c.get_admin(), Some(ctx.admin.clone())); +} + +#[test] +fn a_new_admin_proposal_replaces_the_old_one_and_resets_the_timelock() { + let ctx = setup(); + let c = ctx.client(); + let first = Address::generate(&ctx.env); + let second = Address::generate(&ctx.env); + + c.propose_admin(&first); + advance(&ctx, ADMIN_TIMELOCK_DELAY - 10); + c.propose_admin(&second); + advance(&ctx, 10); + + assert_eq!( + c.try_accept_admin(&first), + Err(Ok(Error::Unauthorized.into())) + ); + assert_eq!( + c.try_accept_admin(&second), + Err(Ok(Error::AdminTimelockNotElapsed.into())) + ); + advance(&ctx, ADMIN_TIMELOCK_DELAY); + c.accept_admin(&second); + assert_eq!(c.get_admin(), Some(second)); +} + +#[test] +fn cancel_admin_transfer_discards_the_pending_handover() { + let ctx = setup(); + let c = ctx.client(); + assert_eq!( + c.try_cancel_admin_transfer(), + Err(Ok(Error::NoPendingAdminTransfer.into())) + ); + + let proposed = Address::generate(&ctx.env); + c.propose_admin(&proposed); + c.cancel_admin_transfer(); + assert_eq!(c.get_pending_admin(), None); + + advance(&ctx, ADMIN_TIMELOCK_DELAY); + assert_eq!( + c.try_accept_admin(&proposed), + Err(Ok(Error::NoPendingAdminTransfer.into())) + ); + assert_eq!(c.get_admin(), Some(ctx.admin.clone())); +} + +#[test] +fn propose_and_cancel_require_the_current_admin() { + let ctx = setup(); + let c = ctx.client(); + let proposed = Address::generate(&ctx.env); + c.propose_admin(&proposed); + assert_eq!(ctx.env.auths()[0].0, ctx.admin); + + ctx.env.set_auths(&[]); + assert!(c.try_propose_admin(&Address::generate(&ctx.env)).is_err()); + assert!(c.try_cancel_admin_transfer().is_err()); + assert_eq!(c.get_pending_admin().map(|(a, _)| a), Some(proposed)); +} + #[test] fn set_writer_only_bootstraps_the_first_writer() { let ctx = setup();