From fc99b5041f677e32eec4ceb0435c8b27240b1c6e Mon Sep 17 00:00:00 2001 From: grunch Date: Fri, 8 May 2026 19:46:50 -0300 Subject: [PATCH 1/2] =?UTF-8?q?feat(bond):=20align=20AntiAbuseBondSettings?= =?UTF-8?q?=20with=20spec=20=E2=80=94=20slash=20split,=20claim=20window,?= =?UTF-8?q?=20drop=20unused=20dispute=20flag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes to `[anti_abuse_bond]` to match the `docs/ANTI_ABUSE_BOND.md` spec: - Add `slash_node_share_pct: f64` (default `0.5`). Fraction of a slashed bond the node retains; the rest goes to the winning counterparty. Funds solver compensation for dispute work without introducing a new payment rail (the slash already routes through Mostro's wallet on settle). `0.0` keeps the legacy winner-takes-all semantics; `1.0` makes the bond a pure sybil/abuse cost. - Add `payout_claim_window_days: u32` (default `15`). Long-stop forfeiture: if the wronged counterparty never submits a payout invoice within this window, the bond closes as `forfeited` and the node retains the counterparty share too. Removes the indefinite- hold failure mode without operator intervention. - Remove `slash_on_lost_dispute: bool`. The spec drops it explicitly in §3 ("there is no `slash_on_lost_dispute` flag"); dispute slashes are expressed by the solver per-resolution via the `BondResolution` payload (Phase 2). The flag was opt-in dead code with no behaviour wired up, so removing it does not change runtime. A toml file with `slash_on_lost_dispute = true` from before this lands continues to parse — serde silently ignores unknown fields on this struct — so no operator action is needed at upgrade time. Doc comments on `slash_on_waiting_timeout` and `payout_max_retries` gain context: the former notes the `BondResolution` design for disputes; the latter clarifies it counts only `send_payment` retries against an already-received invoice (independent from the claim window). Tests cover the new defaults and a toml round-trip for both new knobs. `settings.tpl.toml` updated to match. No behaviour change in Phase 0/1 — the new fields are read by Phase 3+ scheduler logic that has not landed yet. Co-Authored-By: Claude Opus 4.7 (1M context) --- settings.tpl.toml | 9 ++++++- src/config/types.rs | 63 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/settings.tpl.toml b/settings.tpl.toml index af9a3574..026f85d1 100644 --- a/settings.tpl.toml +++ b/settings.tpl.toml @@ -100,7 +100,14 @@ port = 50051 # base_amount_sats = 1000 # # "take" | "create" | "both" # apply_to = "take" -# slash_on_lost_dispute = true +# # Note: there is no `slash_on_lost_dispute` flag — dispute slashes are +# # solver-directed via the `BondResolution` payload (Phase 2). # slash_on_waiting_timeout = false +# # Fraction of a slashed bond that the node retains (the rest goes to the +# # winning counterparty). Funds solver compensation for dispute work. +# slash_node_share_pct = 0.5 # payout_invoice_window_seconds = 300 # payout_max_retries = 5 +# # Days the winner has to claim their share by submitting a bolt11; after +# # this the bond closes as `forfeited` and the node retains everything. +# payout_claim_window_days = 15 diff --git a/src/config/types.rs b/src/config/types.rs index b3f2b352..aa9dbbe6 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -55,22 +55,40 @@ pub struct AntiAbuseBondSettings { /// Which trade flow(s) require the bond. #[serde(default)] pub apply_to: BondApplyTo, - /// Slash the bond when the bonded party loses a dispute. - #[serde(default)] - pub slash_on_lost_dispute: bool, /// Slash the bond when the bonded party lets the waiting-state timeout /// actually elapse. A cancellation before the timeout MUST always /// release the bond regardless of this flag; see §Phase 4 of the spec. + /// + /// Note: there is intentionally no `slash_on_lost_dispute` flag. + /// Dispute slashes are expressed by the solver per-resolution via the + /// `BondResolution` payload (see §3 / Phase 2 of the spec). #[serde(default)] pub slash_on_waiting_timeout: bool, + /// Fraction of a slashed bond that the node retains. The remainder is + /// paid out to the winning counterparty. The node share is meant to + /// fund solver compensation for dispute work; see §15.4 of the spec. + /// `0.0` = full payout to counterparty (legacy behaviour); + /// `1.0` = node keeps everything. Used by Phase 3. + #[serde(default = "default_slash_node_share_pct")] + pub slash_node_share_pct: f64, /// How long (seconds) Mostro waits between payout-invoice retries /// when asking the winning counterparty for a bolt11. Used by Phase 3. #[serde(default = "default_payout_invoice_window_seconds")] pub payout_invoice_window_seconds: u64, - /// Maximum number of payout-invoice retries before a bond transitions - /// to `failed` state. Used by Phase 3. + /// Maximum number of `send_payment` retries against an invoice the + /// counterparty has already submitted before a bond transitions to + /// `failed`. Independent from how long we wait for the invoice itself + /// (that is governed by `payout_claim_window_days`). Used by Phase 3. #[serde(default = "default_payout_max_retries")] pub payout_max_retries: u32, + /// How many days the winning counterparty has, from the moment the + /// bond is slashed, to claim their share by submitting a payout + /// bolt11. If the window elapses without an invoice ever being + /// received, the bond transitions to `forfeited` and the node retains + /// the counterparty share too (long-stop forfeiture; see §15.4). + /// Used by Phase 3. + #[serde(default = "default_payout_claim_window_days")] + pub payout_claim_window_days: u32, } fn default_bond_amount_pct() -> f64 { @@ -89,6 +107,14 @@ fn default_payout_max_retries() -> u32 { 5 } +fn default_slash_node_share_pct() -> f64 { + 0.5 +} + +fn default_payout_claim_window_days() -> u32 { + 15 +} + impl Default for AntiAbuseBondSettings { fn default() -> Self { Self { @@ -96,10 +122,11 @@ impl Default for AntiAbuseBondSettings { amount_pct: default_bond_amount_pct(), base_amount_sats: default_bond_base_amount(), apply_to: BondApplyTo::default(), - slash_on_lost_dispute: false, slash_on_waiting_timeout: false, + slash_node_share_pct: default_slash_node_share_pct(), payout_invoice_window_seconds: default_payout_invoice_window_seconds(), payout_max_retries: default_payout_max_retries(), + payout_claim_window_days: default_payout_claim_window_days(), } } } @@ -367,13 +394,14 @@ mod anti_abuse_bond_tests { fn defaults_are_off() { let cfg = AntiAbuseBondSettings::default(); assert!(!cfg.enabled); - assert!(!cfg.slash_on_lost_dispute); assert!(!cfg.slash_on_waiting_timeout); assert_eq!(cfg.apply_to, BondApplyTo::Take); assert_eq!(cfg.amount_pct, 0.01); assert_eq!(cfg.base_amount_sats, 1_000); assert_eq!(cfg.payout_invoice_window_seconds, 300); assert_eq!(cfg.payout_max_retries, 5); + assert_eq!(cfg.slash_node_share_pct, 0.5); + assert_eq!(cfg.payout_claim_window_days, 15); } #[test] @@ -420,10 +448,27 @@ mod anti_abuse_bond_tests { r#"[anti_abuse_bond] enabled = true apply_to = "both" -slash_on_lost_dispute = true"#, +slash_on_waiting_timeout = true"#, ) .expect("toml parses"); assert_eq!(parsed.anti_abuse_bond.apply_to, BondApplyTo::Both); - assert!(parsed.anti_abuse_bond.slash_on_lost_dispute); + assert!(parsed.anti_abuse_bond.slash_on_waiting_timeout); + } + + #[test] + fn toml_slash_node_share_pct_and_claim_window_override() { + #[derive(serde::Deserialize)] + struct Stub { + anti_abuse_bond: AntiAbuseBondSettings, + } + let parsed: Stub = toml::from_str( + r#"[anti_abuse_bond] +enabled = true +slash_node_share_pct = 0.25 +payout_claim_window_days = 30"#, + ) + .expect("toml parses"); + assert_eq!(parsed.anti_abuse_bond.slash_node_share_pct, 0.25); + assert_eq!(parsed.anti_abuse_bond.payout_claim_window_days, 30); } } From 350b0af3bcd920bace741f3d9f1447920148a39f Mon Sep 17 00:00:00 2001 From: grunch Date: Sat, 9 May 2026 10:07:55 -0300 Subject: [PATCH 2/2] chore(bond): validate slash_node_share_pct range and lock down legacy-config compat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review nits on PR 728: - `slash_node_share_pct` now validates at deserialization: values outside `[0.0, 1.0]` (and NaN) are rejected with a descriptive serde error naming the field and the valid range. Out-of-range values would corrupt the `node_share_sats = floor(amount_sats * pct)` math (negative counterparty share, or node retention exceeding the bond), so failing fast at startup is safer than silent misbehaviour at slash time. The default `0.5` is in range so existing configs are unaffected. - New backward-compatibility test `toml_legacy_slash_on_lost_dispute_parses` locks down the contract the PR description claimed: a toml carrying the now-removed `slash_on_lost_dispute = true` parses without error and the surrounding fields deserialize normally. The test also flags the invariant for future maintainers — a later `#[serde(deny_unknown_fields)]` would silently break upgrade paths and this test would catch it. Three additional tests round out the validation surface: boundary values `0.0` and `1.0` are accepted; `-0.1` and `1.5` are rejected with the expected error message. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/config/types.rs | 102 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/src/config/types.rs b/src/config/types.rs index aa9dbbe6..3c24337a 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -69,7 +69,16 @@ pub struct AntiAbuseBondSettings { /// fund solver compensation for dispute work; see §15.4 of the spec. /// `0.0` = full payout to counterparty (legacy behaviour); /// `1.0` = node keeps everything. Used by Phase 3. - #[serde(default = "default_slash_node_share_pct")] + /// + /// Validated at deserialization: rejected if outside `[0.0, 1.0]`. + /// Out-of-range values would corrupt the + /// `node_share_sats = floor(amount_sats * pct)` math (negative + /// counterparty share, or node retention exceeding the bond), so the + /// daemon refuses to start rather than silently misbehave. + #[serde( + default = "default_slash_node_share_pct", + deserialize_with = "deserialize_slash_node_share_pct" + )] pub slash_node_share_pct: f64, /// How long (seconds) Mostro waits between payout-invoice retries /// when asking the winning counterparty for a bolt11. Used by Phase 3. @@ -111,6 +120,24 @@ fn default_slash_node_share_pct() -> f64 { 0.5 } +/// Validating deserializer for `slash_node_share_pct`. Rejects anything +/// outside `[0.0, 1.0]` (including NaN) with a descriptive serde error +/// so a typo in the operator's `settings.toml` fails fast at startup +/// rather than producing nonsense math at slash time. +fn deserialize_slash_node_share_pct<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + use serde::de::Error as _; + let v = f64::deserialize(deserializer)?; + if !(0.0..=1.0).contains(&v) { + return Err(D::Error::custom(format!( + "slash_node_share_pct must be in [0.0, 1.0], got {v}" + ))); + } + Ok(v) +} + fn default_payout_claim_window_days() -> u32 { 15 } @@ -471,4 +498,77 @@ payout_claim_window_days = 30"#, assert_eq!(parsed.anti_abuse_bond.slash_node_share_pct, 0.25); assert_eq!(parsed.anti_abuse_bond.payout_claim_window_days, 30); } + + #[test] + fn toml_slash_node_share_pct_boundaries_accepted() { + #[derive(serde::Deserialize)] + struct Stub { + anti_abuse_bond: AntiAbuseBondSettings, + } + for (pct, expected) in [("0.0", 0.0), ("1.0", 1.0)] { + let toml_str = format!("[anti_abuse_bond]\nslash_node_share_pct = {pct}"); + let parsed: Stub = toml::from_str(&toml_str).expect("boundary value should parse"); + assert_eq!(parsed.anti_abuse_bond.slash_node_share_pct, expected); + } + } + + #[test] + fn toml_slash_node_share_pct_below_zero_rejected() { + #[derive(Debug, serde::Deserialize)] + struct Stub { + #[allow(dead_code)] + anti_abuse_bond: AntiAbuseBondSettings, + } + let err = toml::from_str::("[anti_abuse_bond]\nslash_node_share_pct = -0.1") + .expect_err("negative pct must be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("slash_node_share_pct") && msg.contains("[0.0, 1.0]"), + "error message should name the field and the valid range, got: {msg}" + ); + } + + #[test] + fn toml_slash_node_share_pct_above_one_rejected() { + #[derive(Debug, serde::Deserialize)] + struct Stub { + #[allow(dead_code)] + anti_abuse_bond: AntiAbuseBondSettings, + } + let err = toml::from_str::("[anti_abuse_bond]\nslash_node_share_pct = 1.5") + .expect_err("pct above 1.0 must be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("slash_node_share_pct") && msg.contains("[0.0, 1.0]"), + "error message should name the field and the valid range, got: {msg}" + ); + } + + /// Backward compatibility: an operator who upgrades from a pre-spec-cleanup + /// build may still have `slash_on_lost_dispute = true` in their + /// `settings.toml`. The field has been removed from `AntiAbuseBondSettings`, + /// but `deny_unknown_fields` is intentionally NOT set on the struct so the + /// legacy line is silently ignored — no operator action required at upgrade + /// time. This test locks that contract down so a future + /// `#[serde(deny_unknown_fields)]` addition cannot accidentally break + /// existing configs without an explicit migration. + #[test] + fn toml_legacy_slash_on_lost_dispute_parses() { + #[derive(serde::Deserialize)] + struct Stub { + anti_abuse_bond: AntiAbuseBondSettings, + } + let parsed: Stub = toml::from_str( + r#"[anti_abuse_bond] +enabled = true +slash_on_lost_dispute = true +slash_node_share_pct = 0.25 +payout_claim_window_days = 30"#, + ) + .expect("legacy slash_on_lost_dispute should be silently ignored"); + assert!(parsed.anti_abuse_bond.enabled); + // Other fields on the same block must still deserialize correctly. + assert_eq!(parsed.anti_abuse_bond.slash_node_share_pct, 0.25); + assert_eq!(parsed.anti_abuse_bond.payout_claim_window_days, 30); + } }