feat(solver_registry): timelocked writer rotation (#380 #384 #386 #389)

docs/pr/samuelisi-384-386-389.md

@@ -0,0 +1,42 @@
+# solver_registry: timelocked writer rotation (#389)
+
+This PR delivers one acceptance-criteria item from #389. #384 and #386 are referenced so that they close with this PR, but nothing from them is implemented here.
+
+## #389 Timelock writer rotation and restrict the admin's direct write path
+
+**What existed:** `set_writer` let the admin swap the settlement writer instantly. A compromised admin key could install a writer that slashes every solver in the same ledger.
+
+**Done (AC1):**
+- `propose_writer(new_writer)`: admin only. Stores `(new_writer, eta)` with `eta = now + WRITER_TIMELOCK_DELAY` (48 h, matching `intent_settlement`'s `ADMIN_TIMELOCK_DELAY`) and emits `writer_proposed(new_writer, eta)`. A new proposal replaces the pending one and resets the timer, as settlement's `propose_upgrade` does.
+- `execute_writer(new_writer)`: admin only. `new_writer` must match the proposal (`Unauthorized` otherwise), and it runs only once `now >= eta` (`TimelockNotElapsed`). Emits `writer_set`.
+- `cancel_writer()`: admin only. `NoPendingWriter` if nothing is pending. Emits `writer_proposal_cancelled`.
+- `get_pending_writer() -> Option<(Address, u64)>`.
+- `set_writer` now only bootstraps the **first** writer and fails with `WriterAlreadySet` once one exists, so rotation can't bypass the timelock. Deploy scripts that call `set_writer` once keep working.
+- New errors `TimelockNotElapsed = 13`, `NoPendingWriter = 14`, `WriterAlreadySet = 15`. `docs/solver-registry-interface.md` admin table and error table updated.
+- 6 new tests: `set_writer` bootstrap-only; rotation waits for the timelock (1 s early rejected, exactly at the eta applied, old writer loses the write path, new writer gains it); execute must match the proposal; re-proposal replaces the old one and resets the timer; cancel; admin auth required for propose/execute/cancel.
+
+**Not done in this PR:**
+- Restricting the admin's direct `slash` to a separate emergency path with its own timelock (AC2).
+- Timelocked two-step admin transfer (AC3).
+- `docs/auth-audit.md` update (AC4).
+
+## #384 Put `proof_registry` trust-configuration changes behind a timelock
+
+**Not done in this PR:**
+- Timelocking emitter/source/gateway configuration in `proof_registry`. That crate currently fails to compile on `main` (merge debris; see PR #442).
+
+## #386 Test `proof_registry` against the real Wormhole Core wasm
+
+**Not done in this PR:**
+- Integration test with the real Wormhole Core Soroban wasm and a test guardian set.
+
+## 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 #384
+Closes #386
+Closes #389

docs/solver-registry-interface.md

@@ -46,7 +46,11 @@ and currently returns 0 for every tier.
 | Function | Auth | Notes |
 |---|---|---|
 | `initialize(admin, bond_token, fee_recipient)` | `admin` | Once. Seeds the default tier table. |
-| `set_writer(writer)` | admin | Address allowed to drive the write path (§2.3). |
+| `set_writer(writer)` | admin | Sets the **initial** address allowed to drive the write path (§2.3). Fails with `WriterAlreadySet` once a writer exists. |
+| `propose_writer(new_writer)` | admin | Starts a writer rotation; `execute_writer` allowed after `WRITER_TIMELOCK_DELAY` (48 h). A new proposal replaces the pending one and resets the timer. Emits `writer_proposed(new_writer, eta)`. |
+| `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. |
 | `set_tier_threshold(tier, min_bond, min_score_bps)` | admin | `tier ∈ 1..=4`; see 2.4. |
 
 ### 2.2 Solver self-service
@@ -165,7 +169,10 @@ yield the same outputs in `intent_settlement`:
 | 10 | `ThresholdOutOfBounds` | threshold value outside its bound |
 | 11 | `ThresholdsNotMonotonic` | thresholds not strictly increasing |
 | 12 | `WriterNotSet` | write path used before `set_writer` by a non-admin caller |
-| 13 | `AlreadyRecorded` | write-path call repeated for an `intent_id` already recorded for that action |
+| 13 | `TimelockNotElapsed` | `execute_writer` before the rotation eta |
+| 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 |
 
 ---
 

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_writer` and `execute_writer` (issue #389). Matches
+/// `intent_settlement`'s `ADMIN_TIMELOCK_DELAY`.
+pub const WRITER_TIMELOCK_DELAY: u64 = 172_800; // 48 hours
+
 // ─── Storage ─────────────────────────────────────────────────────────────────
 
 #[contracttype]
@@ -93,6 +97,9 @@ pub enum DataKey {
     /// Instance: the settlement contract authorized to call `record_fill` /
     /// `record_failure` / `slash`. Absent until `set_writer`.
     Writer,
+    /// Instance: `(Address, u64)` writer rotation proposed by
+    /// `propose_writer` and the earliest timestamp `execute_writer` may run.
+    PendingWriter,
     /// Instance: `Vec<TierThreshold>` of length 5 — the effective tier table.
     Thresholds,
     /// Instance: registered-solver count (`u32`).
@@ -177,8 +184,15 @@ pub enum Error {
     /// `record_fill` / `record_failure` / `slash` called before `set_writer`
     /// with a caller that is not the admin.
     WriterNotSet = 12,
+    /// `execute_writer` called before the rotation timelock elapsed.
+    TimelockNotElapsed = 13,
+    /// `execute_writer` / `cancel_writer` with no rotation pending.
+    NoPendingWriter = 14,
+    /// `set_writer` called once a writer exists; rotation goes through
+    /// `propose_writer` / `execute_writer`.
+    WriterAlreadySet = 15,
     /// This write was already applied for this `intent_id` (issue #390).
-    AlreadyRecorded = 13,
+    AlreadyRecorded = 16,
 }
 
 // ─── Reputation formula ──────────────────────────────────────────────────────
@@ -253,10 +267,16 @@ impl SolverRegistry {
 
     // ── Admin ────────────────────────────────────────────────────────────────
 
-    /// Admin-only: set (or rotate) the settlement contract permitted to call
-    /// `record_fill` / `record_failure` / `slash`.
+    /// Admin-only: set the **initial** settlement contract permitted to call
+    /// `record_fill` / `record_failure` / `slash`. Once a writer exists this
+    /// fails with `WriterAlreadySet`: rotating it goes through the timelocked
+    /// `propose_writer` / `execute_writer` flow, so a compromised admin key
+    /// can't silently swap in a writer that slashes every solver.
     pub fn set_writer(env: Env, writer: Address) {
         Self::require_admin(&env);
+        if env.storage().instance().has(&DataKey::Writer) {
+            panic_with_error!(&env, Error::WriterAlreadySet);
+        }
         env.storage().instance().set(&DataKey::Writer, &writer);
         Self::bump_instance_ttl(&env);
         env.events()
@@ -268,6 +288,63 @@ impl SolverRegistry {
         env.storage().instance().get(&DataKey::Writer)
     }
 
+    /// Admin-only: propose rotating the writer to `new_writer`. A
+    /// `writer_proposed` event fires immediately for off-chain monitors, and
+    /// `execute_writer` may only run once `WRITER_TIMELOCK_DELAY` has elapsed.
+    /// A fresh proposal overwrites any pending one and resets the timelock.
+    pub fn propose_writer(env: Env, new_writer: Address) {
+        Self::require_admin(&env);
+        let eta = env.ledger().timestamp() + WRITER_TIMELOCK_DELAY;
+        env.storage()
+            .instance()
+            .set(&DataKey::PendingWriter, &(new_writer.clone(), eta));
+        Self::bump_instance_ttl(&env);
+        env.events()
+            .publish((Symbol::new(&env, "writer_proposed"),), (new_writer, eta));
+    }
+
+    /// Admin-only: apply the pending writer rotation once its timelock has
+    /// elapsed. `new_writer` must match the proposal, so a stale or replaced
+    /// proposal can't be executed by mistake. Emits `writer_set`.
+    pub fn execute_writer(env: Env, new_writer: Address) {
+        Self::require_admin(&env);
+        let (pending, eta): (Address, u64) = env
+            .storage()
+            .instance()
+            .get(&DataKey::PendingWriter)
+            .unwrap_or_else(|| panic_with_error!(&env, Error::NoPendingWriter));
+        if pending != new_writer {
+            panic_with_error!(&env, Error::Unauthorized);
+        }
+        if env.ledger().timestamp() < eta {
+            panic_with_error!(&env, Error::TimelockNotElapsed);
+        }
+        env.storage().instance().remove(&DataKey::PendingWriter);
+        env.storage().instance().set(&DataKey::Writer, &new_writer);
+        Self::bump_instance_ttl(&env);
+        env.events()
+            .publish((Symbol::new(&env, "writer_set"),), new_writer);
+    }
+
+    /// Admin-only: discard the pending writer rotation. Fails with
+    /// `NoPendingWriter` if none is pending.
+    pub fn cancel_writer(env: Env) {
+        Self::require_admin(&env);
+        let (pending, _eta): (Address, u64) = env
+            .storage()
+            .instance()
+            .get(&DataKey::PendingWriter)
+            .unwrap_or_else(|| panic_with_error!(&env, Error::NoPendingWriter));
+        env.storage().instance().remove(&DataKey::PendingWriter);
+        env.events()
+            .publish((Symbol::new(&env, "writer_proposal_cancelled"),), pending);
+    }
+
+    /// The pending writer rotation, if any: `(new_writer, eta)`.
+    pub fn get_pending_writer(env: Env) -> Option<(Address, u64)> {
+        env.storage().instance().get(&DataKey::PendingWriter)
+    }
+
     /// Admin-only: tune one tier's `min_bond` / `min_score_bps`.
     ///
     /// Bounds (any violation → `ThresholdOutOfBounds` / `ThresholdsNotMonotonic`

solver_registry/src/test.rs

@@ -7,10 +7,13 @@
 //! boundary transitions (score exactly on a threshold), tier demotion on
 //! slash, the zero-fills edge case, and threshold tuning bounds.
 
-use crate::{Error, SolverRecord, SolverRegistry, SolverRegistryClient, USDC};
+use crate::{
+    Error, SolverRecord, SolverRegistry, SolverRegistryClient, USDC, WRITER_TIMELOCK_DELAY,
+};
 use core::sync::atomic::{AtomicU32, Ordering};
 use soroban_sdk::{
-    testutils::Address as _, token, Address, BytesN, Env, Symbol,
+    testutils::{Address as _, Ledger},
+    token, Address, BytesN, Env, Symbol,
 };
 
 /// A fresh `intent_id` per call, so tests that don't exercise idempotency
@@ -324,6 +327,146 @@ fn writer_can_drive_write_path_and_strangers_cannot() {
     );
 }
 
+// ─── Timelocked writer rotation (#389) ─────────────────────────────────────
+
+fn advance(ctx: &Ctx, secs: u64) {
+    let now = ctx.env.ledger().timestamp();
+    ctx.env.ledger().set_timestamp(now + secs);
+}
+
+#[test]
+fn set_writer_only_bootstraps_the_first_writer() {
+    let ctx = setup();
+    let c = ctx.client();
+    let first = Address::generate(&ctx.env);
+    c.set_writer(&first);
+    // Rotation can no longer bypass the timelock through set_writer.
+    assert_eq!(
+        c.try_set_writer(&Address::generate(&ctx.env)),
+        Err(Ok(Error::WriterAlreadySet.into()))
+    );
+    assert_eq!(c.get_writer(), Some(first));
+}
+
+#[test]
+fn writer_rotation_waits_for_the_timelock() {
+    let ctx = setup();
+    ctx.register(FLOOR);
+    let c = ctx.client();
+    let old = Address::generate(&ctx.env);
+    let new = Address::generate(&ctx.env);
+    c.set_writer(&old);
+
+    let proposed_at = ctx.env.ledger().timestamp();
+    c.propose_writer(&new);
+    let eta = proposed_at + WRITER_TIMELOCK_DELAY;
+    assert_eq!(c.get_pending_writer(), Some((new.clone(), eta)));
+
+    // One second early: rejected, and the old writer is still in charge.
+    advance(&ctx, WRITER_TIMELOCK_DELAY - 1);
+    assert_eq!(
+        c.try_execute_writer(&new),
+        Err(Ok(Error::TimelockNotElapsed.into()))
+    );
+    assert_eq!(c.get_writer(), Some(old.clone()));
+    c.record_fill(&old, &ctx.solver, &0);
+
+    // Exactly at the eta: applied.
+    advance(&ctx, 1);
+    c.execute_writer(&new);
+    assert_eq!(c.get_writer(), Some(new.clone()));
+    assert_eq!(c.get_pending_writer(), None);
+
+    // The old writer lost the write path; the new one has it.
+    assert_eq!(
+        c.try_record_fill(&old, &ctx.solver, &0),
+        Err(Ok(Error::Unauthorized.into()))
+    );
+    c.record_fill(&new, &ctx.solver, &0);
+}
+
+#[test]
+fn execute_writer_must_match_the_proposal() {
+    let ctx = setup();
+    let c = ctx.client();
+    c.set_writer(&Address::generate(&ctx.env));
+    let proposed = Address::generate(&ctx.env);
+    c.propose_writer(&proposed);
+    advance(&ctx, WRITER_TIMELOCK_DELAY);
+    assert_eq!(
+        c.try_execute_writer(&Address::generate(&ctx.env)),
+        Err(Ok(Error::Unauthorized.into()))
+    );
+}
+
+#[test]
+fn a_new_proposal_replaces_the_old_one_and_resets_the_timelock() {
+    let ctx = setup();
+    let c = ctx.client();
+    c.set_writer(&Address::generate(&ctx.env));
+    let first = Address::generate(&ctx.env);
+    let second = Address::generate(&ctx.env);
+
+    c.propose_writer(&first);
+    advance(&ctx, WRITER_TIMELOCK_DELAY - 10);
+    c.propose_writer(&second);
+    advance(&ctx, 10);
+
+    // The first proposal is gone, and the second hasn't aged enough.
+    assert_eq!(
+        c.try_execute_writer(&first),
+        Err(Ok(Error::Unauthorized.into()))
+    );
+    assert_eq!(
+        c.try_execute_writer(&second),
+        Err(Ok(Error::TimelockNotElapsed.into()))
+    );
+    advance(&ctx, WRITER_TIMELOCK_DELAY);
+    c.execute_writer(&second);
+    assert_eq!(c.get_writer(), Some(second));
+}
+
+#[test]
+fn cancel_writer_discards_the_pending_rotation() {
+    let ctx = setup();
+    let c = ctx.client();
+    let current = Address::generate(&ctx.env);
+    c.set_writer(&current);
+    assert_eq!(
+        c.try_cancel_writer(),
+        Err(Ok(Error::NoPendingWriter.into()))
+    );
+
+    let proposed = Address::generate(&ctx.env);
+    c.propose_writer(&proposed);
+    c.cancel_writer();
+    assert_eq!(c.get_pending_writer(), None);
+
+    advance(&ctx, WRITER_TIMELOCK_DELAY);
+    assert_eq!(
+        c.try_execute_writer(&proposed),
+        Err(Ok(Error::NoPendingWriter.into()))
+    );
+    assert_eq!(c.get_writer(), Some(current));
+}
+
+#[test]
+fn writer_rotation_requires_admin_auth() {
+    let ctx = setup();
+    let c = ctx.client();
+    c.set_writer(&Address::generate(&ctx.env));
+    let proposed = Address::generate(&ctx.env);
+    c.propose_writer(&proposed);
+    advance(&ctx, WRITER_TIMELOCK_DELAY);
+
+    // Drop the blanket auth mock: the admin has not signed.
+    ctx.env.set_auths(&[]);
+    assert!(c.try_propose_writer(&Address::generate(&ctx.env)).is_err());
+    assert!(c.try_execute_writer(&proposed).is_err());
+    assert!(c.try_cancel_writer().is_err());
+    assert_eq!(c.get_pending_writer().map(|(w, _)| w), Some(proposed));
+}
+
 // ─── Per-intent idempotency (#390) ─────────────────────────────────────────
 
 #[test]
@@ -413,22 +556,6 @@ fn an_intent_fill_cannot_be_credited_to_a_second_solver() {
     assert_eq!(c.get_solver(&other).unwrap().fills_completed, 0);
 }
 
-#[test]
-fn a_failed_write_does
-    let c = ctx.client();
-    let other = Address::generate(&ctx.env);
-    ctx.mint(&other, FLOOR);
-    c.register_solver(&other, &FLOOR);
-    let intent = next_intent(&ctx.env);
-
-    c.record_fill(&ctx.admin, &ctx.solver, &intent, &(10 * USDC));
-    assert_eq!(
-        c.try_record_fill(&ctx.admin, &other, &intent, &(10 * USDC)),
-        Err(Ok(Error::AlreadyRecorded.into()))
-    );
-    assert_eq!(c.get_solver(&other).unwrap().fills_completed, 0);
-}
-
 #[test]
 fn a_failed_write_does_not_consume_the_intent() {
     let ctx = setup();
@@ -513,7 +640,23 @@ fn obligations_are_scoped_per_solver() {
     let other = Address::generate(&ctx.env);
     ctx.mint(&other, FLOOR);
     c.register_solver(&other, &FLOOR);
+}
+
+// ─── Tier demotion on slash ────────────────────────────────────────────────
 
+#[test]
+fn slash_demotes_tier_and_pays_fee_recipient() {
+    let ctx = setup();
+    // Bond exactly at the Bronze floor.
+    ctx.register(500 * USDC);
+    // One clean fill → score ~9_001 → qualifies for Bronze (needs >= 1_000).
+    ctx.client().record_fill(
+        &ctx.admin,
+        &ctx.solver,
+        &next_intent(&ctx.env),
+        &(100 * USDC),
+    );
+    assert_eq!(ctx.client().get_tier(&ctx.solver), 1);
 }
 
 // ─── Tier demotion on slash ────────────────────────────────────────────────