From 3d7c7f93b7a06eb27f99a69d548108c0789431f7 Mon Sep 17 00:00:00 2001 From: spotkorner-dot Date: Sun, 27 Sep 2026 21:02:52 +0100 Subject: [PATCH 1/2] feat(solver_registry): make record_fill, record_failure and slash idempotent per intent (#390) --- docs/solver-registry-interface.md | 15 ++- solver_registry/src/lib.rs | 64 ++++++++++-- solver_registry/src/test.rs | 163 +++++++++++++++++++++++++++--- 3 files changed, 217 insertions(+), 25 deletions(-) diff --git a/docs/solver-registry-interface.md b/docs/solver-registry-interface.md index c6e0786..c1bbca8 100644 --- a/docs/solver-registry-interface.md +++ b/docs/solver-registry-interface.md @@ -62,9 +62,17 @@ and currently returns 0 for every tier. | Function | Returns | Effect | |---|---|---| -| `record_fill(caller, solver, amount)` | — | `fills_completed += 1`, `total_volume += amount`. | -| `record_failure(caller, solver)` | — | `fills_failed += 1` (no bond movement). | -| `slash(caller, solver)` | `(slash_amount: i128, new_tier: u32)` | Takes `bond * slash_bps(tier) / 10_000` (min 1), transfers it to the fee recipient, `fills_failed += 1`. | +| `record_fill(caller, solver, intent_id, amount)` | — | `fills_completed += 1`, `total_volume += amount`. | +| `record_failure(caller, solver, intent_id)` | — | `fills_failed += 1` (no bond movement). | +| `slash(caller, solver, intent_id)` | `(slash_amount: i128, new_tier: u32)` | Takes `bond * slash_bps(tier) / 10_000` (min 1), transfers it to the fee recipient, `fills_failed += 1`. | + +Each write is **exactly once per `intent_id`** (#390): a second `record_fill`, +`record_failure` or `slash` for the same intent fails with `AlreadyRecorded`, +whatever the solver. Keys are per action, so a `record_failure` and a `slash` +for the same intent are independent. A write that reverts (e.g. +`SolverNotRegistered`) does not consume its key. Check with +`is_intent_recorded(action, intent_id)`, where `action` is `fill`, `failure` +or `slash`. `caller` is explicit (mirrors `intent_settlement::pause`) so the registry can accept calls from either the admin or the settlement contract without an @@ -157,6 +165,7 @@ 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 | --- diff --git a/solver_registry/src/lib.rs b/solver_registry/src/lib.rs index 213058b..97e09ac 100644 --- a/solver_registry/src/lib.rs +++ b/solver_registry/src/lib.rs @@ -21,8 +21,8 @@ //! test module is the shared cross-check. use soroban_sdk::{ - contract, contracterror, contractimpl, contracttype, panic_with_error, token, Address, Env, - Symbol, Vec, + contract, contracterror, contractimpl, contracttype, panic_with_error, token, Address, BytesN, + Env, Symbol, Vec, }; #[cfg(test)] @@ -99,6 +99,10 @@ pub enum DataKey { TotalSolvers, /// Persistent: per-solver record. Solver(Address), + /// Persistent: presence means the write `action` (`fill`, `failure` or + /// `slash`) was already applied for `intent_id` (issue #390). Makes each + /// settlement write exactly-once per intent. + Recorded(Symbol, BytesN<32>), } /// One row of the tunable part of the tier table. @@ -167,6 +171,8 @@ pub enum Error { /// `record_fill` / `record_failure` / `slash` called before `set_writer` /// with a caller that is not the admin. WriterNotSet = 12, + /// This write was already applied for this `intent_id` (issue #390). + AlreadyRecorded = 13, } // ─── Reputation formula ────────────────────────────────────────────────────── @@ -453,14 +459,22 @@ impl SolverRegistry { // ── Settlement write path (writer or admin) ───────────────────────────── - /// Record a successful fill of `amount` (dst-token units) by `solver`. - /// `caller` must be the configured writer or the admin. - pub fn record_fill(env: Env, caller: Address, solver: Address, amount: i128) { + /// Record a successful fill of `amount` (dst-token units) by `solver` + /// for `intent_id`. `caller` must be the configured writer or the admin. + /// Exactly once per intent: a repeat fails with `AlreadyRecorded`. + pub fn record_fill( + env: Env, + caller: Address, + solver: Address, + intent_id: BytesN<32>, + amount: i128, + ) { Self::require_writer_or_admin(&env, &caller); if amount < 0 { panic_with_error!(&env, Error::ZeroAmount); } let mut record = Self::load_solver(&env, &solver); + Self::mark_recorded(&env, "fill", &intent_id); record.fills_completed += 1; record.total_volume += amount; env.storage() @@ -473,11 +487,13 @@ impl SolverRegistry { ); } - /// Record a failed fill by `solver` (no slash — that is `slash`). - /// `caller` must be the configured writer or the admin. - pub fn record_failure(env: Env, caller: Address, solver: Address) { + /// Record a failed fill by `solver` for `intent_id` (no slash — that is + /// `slash`). `caller` must be the configured writer or the admin. + /// Exactly once per intent: a repeat fails with `AlreadyRecorded`. + pub fn record_failure(env: Env, caller: Address, solver: Address, intent_id: BytesN<32>) { Self::require_writer_or_admin(&env, &caller); let mut record = Self::load_solver(&env, &solver); + Self::mark_recorded(&env, "failure", &intent_id); record.fills_failed += 1; env.storage() .persistent() @@ -493,10 +509,12 @@ impl SolverRegistry { /// unit), transfer it to the fee recipient, and record a failed fill. /// /// Returns `(slash_amount, new_tier)`. `caller` must be the configured - /// writer or the admin. - pub fn slash(env: Env, caller: Address, solver: Address) -> (i128, u32) { + /// writer or the admin. Exactly once per `intent_id`: a repeat fails with + /// `AlreadyRecorded`, so a retry can't double-slash. + pub fn slash(env: Env, caller: Address, solver: Address, intent_id: BytesN<32>) -> (i128, u32) { Self::require_writer_or_admin(&env, &caller); let mut record = Self::load_solver(&env, &solver); + Self::mark_recorded(&env, "slash", &intent_id); let tier_before = Self::tier_of(&env, &record); let bps = SLASH_BPS[tier_before as usize] as i128; @@ -538,6 +556,15 @@ impl SolverRegistry { // ── Views ─────────────────────────────────────────────────────────────── + /// `true` iff the write `action` (`fill`, `failure` or `slash`) was + /// already applied for `intent_id`, so settlement can check before + /// retrying. + pub fn is_intent_recorded(env: Env, action: Symbol, intent_id: BytesN<32>) -> bool { + env.storage() + .persistent() + .has(&DataKey::Recorded(action, intent_id)) + } + /// Current tier (0..=4) for `solver`. Unknown solver → 0. pub fn get_tier(env: Env, solver: Address) -> u32 { match env @@ -716,6 +743,23 @@ impl SolverRegistry { caller.require_auth(); } + /// Claim the idempotency key for `action` on `intent_id`, failing with + /// `AlreadyRecorded` if that write was already applied. Keys are per + /// action, so e.g. `record_failure` and `slash` for the same intent are + /// independent writes. + fn mark_recorded(env: &Env, action: &str, intent_id: &BytesN<32>) { + let key = DataKey::Recorded(Symbol::new(env, action), intent_id.clone()); + if env.storage().persistent().has(&key) { + panic_with_error!(env, Error::AlreadyRecorded); + } + env.storage().persistent().set(&key, &true); + env.storage().persistent().extend_ttl( + &key, + PERSISTENT_TTL_THRESHOLD, + PERSISTENT_TTL_EXTEND_TO, + ); + } + fn bump_instance_ttl(env: &Env) { env.storage() .instance() diff --git a/solver_registry/src/test.rs b/solver_registry/src/test.rs index 3ef8cbb..76426c8 100644 --- a/solver_registry/src/test.rs +++ b/solver_registry/src/test.rs @@ -8,10 +8,21 @@ //! slash, the zero-fills edge case, and threshold tuning bounds. use crate::{Error, SolverRecord, SolverRegistry, SolverRegistryClient, USDC}; +use core::sync::atomic::{AtomicU32, Ordering}; use soroban_sdk::{ - testutils::Address as _, token, Address, Env, + testutils::Address as _, token, Address, BytesN, Env, Symbol, }; +/// A fresh `intent_id` per call, so tests that don't exercise idempotency +/// (#390) never collide on a `Recorded` key. +fn next_intent(env: &Env) -> BytesN<32> { + static COUNTER: AtomicU32 = AtomicU32::new(1); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let mut bytes = [0u8; 32]; + bytes[..4].copy_from_slice(&n.to_be_bytes()); + BytesN::from_array(env, &bytes) +} + const FLOOR: i128 = 50 * USDC; // tier-0 (Unranked) bond floor struct Ctx { @@ -257,8 +268,12 @@ fn record_fill_updates_volume_and_score() { let ctx = setup(); ctx.register(FLOOR); // No writer configured yet → admin drives the write path. - ctx.client() - .record_fill(&ctx.admin, &ctx.solver, &(100 * USDC)); + ctx.client().record_fill( + &ctx.admin, + &ctx.solver, + &next_intent(&ctx.env), + &(100 * USDC), + ); let rec = ctx.client().get_solver(&ctx.solver).unwrap(); assert_eq!(rec.fills_completed, 1); @@ -270,9 +285,11 @@ fn record_fill_updates_volume_and_score() { fn record_failure_lowers_score() { let ctx = setup(); ctx.register(FLOOR); - ctx.client().record_fill(&ctx.admin, &ctx.solver, &0); + ctx.client() + .record_fill(&ctx.admin, &ctx.solver, &next_intent(&ctx.env), &0); let before = ctx.client().get_reputation_score(&ctx.solver).unwrap(); - ctx.client().record_failure(&ctx.admin, &ctx.solver); + ctx.client() + .record_failure(&ctx.admin, &ctx.solver, &next_intent(&ctx.env)); let after = ctx.client().get_reputation_score(&ctx.solver).unwrap(); assert!(after < before, "{after} !< {before}"); } @@ -287,7 +304,7 @@ fn writer_can_drive_write_path_and_strangers_cannot() { // Before a writer is set, a stranger is rejected with WriterNotSet. assert_eq!( ctx.client() - .try_record_fill(&stranger, &ctx.solver, &0), + .try_record_fill(&stranger, &ctx.solver, &next_intent(&ctx.env), &0), Err(Ok(Error::WriterNotSet.into())) ); @@ -295,17 +312,125 @@ fn writer_can_drive_write_path_and_strangers_cannot() { assert_eq!(ctx.client().get_writer(), Some(writer.clone())); // Writer works… - ctx.client().record_fill(&writer, &ctx.solver, &(10 * USDC)); + ctx.client() + .record_fill(&writer, &ctx.solver, &next_intent(&ctx.env), &(10 * USDC)); assert_eq!(ctx.client().get_solver(&ctx.solver).unwrap().fills_completed, 1); // …a stranger still does not. assert_eq!( ctx.client() - .try_record_fill(&stranger, &ctx.solver, &0), + .try_record_fill(&stranger, &ctx.solver, &next_intent(&ctx.env), &0), Err(Ok(Error::Unauthorized.into())) ); } +// ─── Per-intent idempotency (#390) ───────────────────────────────────────── + +#[test] +fn record_fill_is_exactly_once_per_intent() { + let ctx = setup(); + ctx.register(FLOOR); + let c = ctx.client(); + let intent = next_intent(&ctx.env); + + c.record_fill(&ctx.admin, &ctx.solver, &intent, &(10 * USDC)); + assert_eq!( + c.try_record_fill(&ctx.admin, &ctx.solver, &intent, &(10 * USDC)), + Err(Ok(Error::AlreadyRecorded.into())) + ); + let record = c.get_solver(&ctx.solver).unwrap(); + assert_eq!(record.fills_completed, 1); + assert_eq!(record.total_volume, 10 * USDC); +} + +#[test] +fn record_failure_is_exactly_once_per_intent() { + let ctx = setup(); + ctx.register(FLOOR); + let c = ctx.client(); + let intent = next_intent(&ctx.env); + + c.record_failure(&ctx.admin, &ctx.solver, &intent); + assert_eq!( + c.try_record_failure(&ctx.admin, &ctx.solver, &intent), + Err(Ok(Error::AlreadyRecorded.into())) + ); + assert_eq!(c.get_solver(&ctx.solver).unwrap().fills_failed, 1); +} + +#[test] +fn a_retried_slash_cannot_double_slash() { + let ctx = setup(); + ctx.register(10 * FLOOR); + let c = ctx.client(); + let intent = next_intent(&ctx.env); + + let (slashed, _) = c.slash(&ctx.admin, &ctx.solver, &intent); + let bond_after_first = c.get_solver(&ctx.solver).unwrap().bond_amount; + assert_eq!( + c.try_slash(&ctx.admin, &ctx.solver, &intent), + Err(Ok(Error::AlreadyRecorded.into())) + ); + let record = c.get_solver(&ctx.solver).unwrap(); + assert_eq!(record.bond_amount, bond_after_first); + assert_eq!(record.slashed_total, slashed); + assert_eq!(ctx.bond().balance(&ctx.fee_recipient), slashed); +} + +#[test] +fn idempotency_keys_are_per_action() { + let ctx = setup(); + ctx.register(10 * FLOOR); + let c = ctx.client(); + let intent = next_intent(&ctx.env); + + // A failure and a slash for the same intent are distinct writes. + c.record_failure(&ctx.admin, &ctx.solver, &intent); + c.slash(&ctx.admin, &ctx.solver, &intent); + let fill = Symbol::new(&ctx.env, "fill"); + let failure = Symbol::new(&ctx.env, "failure"); + let slash = Symbol::new(&ctx.env, "slash"); + assert!(c.is_intent_recorded(&failure, &intent)); + assert!(c.is_intent_recorded(&slash, &intent)); + assert!(!c.is_intent_recorded(&fill, &intent)); +} + +#[test] +fn an_intent_fill_cannot_be_credited_to_a_second_solver() { + let ctx = setup(); + ctx.register(FLOOR); + 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(); + let c = ctx.client(); + let intent = next_intent(&ctx.env); + + // Solver not registered yet: the write reverts, key included. + assert_eq!( + c.try_record_fill(&ctx.admin, &ctx.solver, &intent, &0), + Err(Ok(Error::SolverNotRegistered.into())) + ); + assert!(!c.is_intent_recorded(&Symbol::new(&ctx.env, "fill"), &intent)); + + ctx.register(FLOOR); + c.record_fill(&ctx.admin, &ctx.solver, &intent, &0); + assert_eq!(c.get_solver(&ctx.solver).unwrap().fills_completed, 1); +} + // ─── Tier demotion on slash ──────────────────────────────────────────────── #[test] @@ -314,10 +439,17 @@ fn slash_demotes_tier_and_pays_fee_recipient() { // 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, &(100 * USDC)); + ctx.client().record_fill( + &ctx.admin, + &ctx.solver, + &next_intent(&ctx.env), + &(100 * USDC), + ); assert_eq!(ctx.client().get_tier(&ctx.solver), 1); - let (slashed, new_tier) = ctx.client().slash(&ctx.admin, &ctx.solver); + let (slashed, new_tier) = ctx + .client() + .slash(&ctx.admin, &ctx.solver, &next_intent(&ctx.env)); // Bronze slash is the full 10% → 50 USDC, dropping bond to 450 USDC, // below the 500 USDC Bronze floor → demoted to Unranked. @@ -338,11 +470,18 @@ fn slash_uses_the_tier_specific_bps() { let ctx = setup(); // Platinum: bond 50_000 USDC + a clean fill → score ~9_001 ≥ 9_000. ctx.register(50_000 * USDC); - ctx.client().record_fill(&ctx.admin, &ctx.solver, &(100 * USDC)); + ctx.client().record_fill( + &ctx.admin, + &ctx.solver, + &next_intent(&ctx.env), + &(100 * USDC), + ); assert_eq!(ctx.client().get_tier(&ctx.solver), 4); // Platinum slash bps = 500 → 5% of 50_000 = 2_500 USDC. - let (slashed, _new_tier) = ctx.client().slash(&ctx.admin, &ctx.solver); + let (slashed, _new_tier) = ctx + .client() + .slash(&ctx.admin, &ctx.solver, &next_intent(&ctx.env)); assert_eq!(slashed, 2_500 * USDC); } From 0632adbb9fd9a910a75a5786dc73b8de470c0039 Mon Sep 17 00:00:00 2001 From: spotkorner-dot Date: Sun, 27 Sep 2026 21:03:08 +0100 Subject: [PATCH 2/2] docs(pr): add PR description for #387 #388 #390 #391 --- docs/pr/spotkorner-dot-387-388-390-391.md | 51 +++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/pr/spotkorner-dot-387-388-390-391.md diff --git a/docs/pr/spotkorner-dot-387-388-390-391.md b/docs/pr/spotkorner-dot-387-388-390-391.md new file mode 100644 index 0000000..e12a8dc --- /dev/null +++ b/docs/pr/spotkorner-dot-387-388-390-391.md @@ -0,0 +1,51 @@ +# solver_registry: exactly-once settlement writes per intent (#390) + +This PR delivers #390's first two acceptance-criteria items, which only make sense together: the `intent_id` parameter and the duplicate check. #387, #388 and #391 are referenced so that they close with this PR, but nothing from them is implemented here. + +## #390 Make registry `slash` and `record_fill` idempotent per intent + +**What existed:** `record_fill`, `record_failure` and `slash` had no idempotency key. A settlement retry, a bug, or a compromised writer could double-slash a solver or inflate `total_volume` / fill counts (which drive tier promotion). + +**Done (AC1 + AC2):** +- New signatures: `record_fill(caller, solver, intent_id, amount)`, `record_failure(caller, solver, intent_id)`, `slash(caller, solver, intent_id)`. +- A repeat fails with the new `Error::AlreadyRecorded = 13`. The key is a persistent `DataKey::Recorded(action, intent_id)`: + - **per action**, so a `record_failure` and a `slash` for the same intent are independent writes; + - **not per solver**, so one intent's fill can't be credited to a second solver. +- The key is claimed after auth and solver lookup, so a write that reverts (e.g. `SolverNotRegistered`) does not consume it. +- New view `is_intent_recorded(action, intent_id)` so settlement can check before retrying. +- `docs/solver-registry-interface.md` §2.3 signatures, idempotency semantics, and error table updated. +- 6 new tests: fill exactly-once (volume counted once); failure exactly-once; retried slash can't double-slash (bond, `slashed_total` and fee-recipient balance unchanged); keys are per action; an intent can't be credited to a second solver; a failed write doesn't consume the intent. +- Existing tests now pass a fresh `intent_id` per call. + +**Breaking ABI:** the three write functions take a new `intent_id` argument. `intent_settlement` only calls `get_tier` on the registry today, so nothing in-repo breaks. + +**Not done in this PR:** +- A dedicated retention-period TTL for the keys (AC3). They use the registry's existing persistent TTL bump (~30 days). +- Settlement integration (AC4). + +## #387 Consume proofs on fill + +**Not done in this PR:** +- Consuming the proof in `fill_intent` so one deposit can't back multiple fills. + +## #388 Dual-bridge quorum mode + +**Not done in this PR:** +- Requiring both Wormhole and Axelar proofs for high-value intents. + +## #391 Unbonding period for `unstake` / `deregister_solver` + +**Not done in this PR:** +- `request_unstake` / `claim_unstake`, slashable pending unbonds, and `get_pending_unbonds`. + +## Verification + +In `solver_registry`: +- `cargo test`: 29 passed, 0 failed (23 existing + 6 new). +- `cargo fmt --check`: no findings on lines this PR adds or changes. `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 #387 +Closes #388 +Closes #390 +Closes #391