diff --git a/docs/PRICE_PROVIDERS.md b/docs/PRICE_PROVIDERS.md index b277dc3e..5733c367 100644 --- a/docs/PRICE_PROVIDERS.md +++ b/docs/PRICE_PROVIDERS.md @@ -353,7 +353,9 @@ of the last tick that produced a fresh aggregate for it. `max_price_staleness_seconds` defaults to **1800** (30 min) — long enough to ride out short API outages, short enough that nobody trades on an -hours-old quote. +hours-old quote. See §11.7 for `nostr_ingestion_budget_pct`, which carves a +fraction of this same TTL out for the Nostr provider's own ingestion gate +(issue #860) rather than letting `as_of` double-count it. ### 6.5 Per-provider health / circuit breaker @@ -891,6 +893,27 @@ for Venezuelan ISPs) but who can still reach a Nostr relay. - **Currency codes are re-upper-cased** (§6.6) even though a Mostro publisher already emits uppercase codes — a third-party trusted node is outside this codebase's control. +- **Ingestion budget, not the full TTL (issue #860).** A rate event's + `created_at` may be up to `max_price_staleness_seconds` old and still be + accepted, and once accepted the store (§6.4) allows serving it for + another full `max_price_staleness_seconds` before refusing it — stacking + the two would let a Nostr-sourced price be served for close to *twice* + the configured TTL, against a setting whose name implies one TTL. To + bound that, the provider's acceptance window is + `max_price_staleness_seconds × nostr_ingestion_budget_pct` (default + `0.5`, i.e. half the TTL) instead of the full TTL, so total age is + bounded at `(1 + nostr_ingestion_budget_pct) × max_price_staleness_seconds` + — 1.5x at the default, tunable down (e.g. `0.2` → 1.2x) if an operator + wants a tighter bound at the cost of the fallback provider rejecting more + still-useful-but-lagging events. This only affects currencies where + Nostr is a *contributor* this tick; a fiat-cross currency resolved + against a Nostr-tainted anchor (`nostr_anchor_dependent`, §6.3) benefits + transitively from a fresher anchor without needing separate handling. + The acceptance window is floored at one second, so this exact multiplier + only holds when `max_price_staleness_seconds × nostr_ingestion_budget_pct + ≥ 1`; a config below that (e.g. a 1s TTL with a 0.001 budget) gets a + 1-second window instead of its own smaller share, widening total age for + that edge case rather than shrinking it below a second. --- diff --git a/settings.tpl.toml b/settings.tpl.toml index ce21a5df..842f0752 100644 --- a/settings.tpl.toml +++ b/settings.tpl.toml @@ -148,6 +148,12 @@ port = 50051 # # Poll cadence and freshness budget. # update_interval_seconds = 300 # max_price_staleness_seconds = 1800 +# # Nostr provider only: fraction of max_price_staleness_seconds an event's +# # own created_at may consume before ingestion refuses it, leaving the +# # remainder as the store's serving budget (issue #860). Default 0.5. +# # Floored at 1 second, so a very low staleness/budget combo widens the +# # effective ingestion window instead of shrinking below one second. +# nostr_ingestion_budget_pct = 0.5 # # Discard a source whose value deviates more than this % from the median # # (only applies with >= 3 sources for a currency). # outlier_threshold_pct = 5.0 diff --git a/src/price/config.rs b/src/price/config.rs index 3676598d..ad821048 100644 --- a/src/price/config.rs +++ b/src/price/config.rs @@ -20,6 +20,19 @@ pub struct PriceSettings { /// Serve a currency's last-known-good value up to this age; then refuse. #[serde(default = "default_max_price_staleness_seconds")] pub max_price_staleness_seconds: i64, + /// Nostr provider only: fraction of `max_price_staleness_seconds` a + /// trusted-node event's own `created_at` may consume before ingestion + /// refuses it (issue #860). The remainder is left as the store's own + /// serving budget, so total age is bounded at + /// `(1 + nostr_ingestion_budget_pct) × max_price_staleness_seconds` + /// instead of accepting an event up to the full TTL old and then + /// serving it for another full TTL (~2x). The provider floors the + /// scaled product at one second, so that bound only holds exactly when + /// `max_price_staleness_seconds * nostr_ingestion_budget_pct >= 1`; + /// below that the ingestion window is one second regardless of how + /// small the product is. + #[serde(default = "default_nostr_ingestion_budget_pct")] + pub nostr_ingestion_budget_pct: f64, /// Discard a source whose value deviates more than this percent from the /// median (only applies with ≥ 3 sources for a currency). #[serde(default = "default_outlier_threshold_pct")] @@ -150,6 +163,15 @@ impl PriceSettings { self.outlier_threshold_pct )); } + if !(self.nostr_ingestion_budget_pct.is_finite() + && self.nostr_ingestion_budget_pct > 0.0 + && self.nostr_ingestion_budget_pct <= 1.0) + { + return Err(format!( + "price: nostr_ingestion_budget_pct must be in (0, 1], got {}", + self.nostr_ingestion_budget_pct + )); + } for (id, p) in &self.providers { p.validate(id)?; } @@ -163,6 +185,9 @@ fn default_update_interval_seconds() -> u64 { fn default_max_price_staleness_seconds() -> i64 { 1800 } +fn default_nostr_ingestion_budget_pct() -> f64 { + 0.5 +} fn default_outlier_threshold_pct() -> f64 { 5.0 } @@ -184,6 +209,7 @@ impl Default for PriceSettings { Self { update_interval_seconds: default_update_interval_seconds(), max_price_staleness_seconds: default_max_price_staleness_seconds(), + nostr_ingestion_budget_pct: default_nostr_ingestion_budget_pct(), outlier_threshold_pct: default_outlier_threshold_pct(), provider_timeout_seconds: default_provider_timeout_seconds(), provider_failure_threshold: default_provider_failure_threshold(), @@ -203,6 +229,7 @@ mod tests { let cfg = PriceSettings::default(); assert_eq!(cfg.update_interval_seconds, 300); assert_eq!(cfg.max_price_staleness_seconds, 1800); + assert_eq!(cfg.nostr_ingestion_budget_pct, 0.5); assert_eq!(cfg.outlier_threshold_pct, 5.0); assert_eq!(cfg.provider_timeout_seconds, 10); assert_eq!(cfg.provider_failure_threshold, 3); @@ -296,6 +323,21 @@ trusted_nodes = ["82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be883 with_pct(5.0).validate().unwrap(); } + #[test] + fn nostr_ingestion_budget_pct_out_of_range_is_rejected() { + let with_pct = |pct: f64| PriceSettings { + nostr_ingestion_budget_pct: pct, + ..Default::default() + }; + assert!(with_pct(0.0).validate().is_err()); + assert!(with_pct(-0.5).validate().is_err()); + assert!(with_pct(1.5).validate().is_err()); + // 1.0 is the inclusive upper boundary — full TTL as ingestion budget + // is wasteful (back to the old bug) but not itself invalid. + with_pct(1.0).validate().unwrap(); + with_pct(0.5).validate().unwrap(); + } + #[test] fn zero_update_interval_is_rejected() { let cfg = PriceSettings { diff --git a/src/price/manager.rs b/src/price/manager.rs index 981a8ae0..6c4537a5 100644 --- a/src/price/manager.rs +++ b/src/price/manager.rs @@ -115,6 +115,7 @@ impl PriceManager { cfg, settings.provider_timeout_seconds, settings.max_price_staleness_seconds, + settings.nostr_ingestion_budget_pct, )?; providers.push(EnabledProvider { id, @@ -725,6 +726,7 @@ fn build_provider( cfg: &ProviderConfig, provider_timeout_seconds: u64, max_price_staleness_seconds: i64, + nostr_ingestion_budget_pct: f64, ) -> Result, String> { match id { ProviderId::Yadio => Ok(Box::new(YadioProvider::new(cfg))), @@ -738,12 +740,16 @@ fn build_provider( // Nostr trusted-node relay mode (§11.7). `new` returns `Err` when // `trusted_nodes` is empty or contains an unparsable hex pubkey. // `provider_timeout_seconds` sizes its one-shot relay query; - // `max_price_staleness_seconds` is the freshness gate on event - // `created_at` so zombie relay data cannot refresh the store clock. + // `max_price_staleness_seconds * nostr_ingestion_budget_pct` is the + // freshness gate on event `created_at` so zombie relay data cannot + // refresh the store clock — only a fraction of the TTL, not the + // whole thing, is spent before the store's own window even starts + // (issue #860). ProviderId::Nostr => Ok(Box::new(NostrProvider::new( cfg, provider_timeout_seconds, max_price_staleness_seconds, + nostr_ingestion_budget_pct, )?)), } } diff --git a/src/price/providers/nostr.rs b/src/price/providers/nostr.rs index 7c55e231..faaa36c0 100644 --- a/src/price/providers/nostr.rs +++ b/src/price/providers/nostr.rs @@ -59,9 +59,18 @@ pub struct NostrProvider { /// timeout before they can complete or fail on their own (CodeRabbit, /// PR #841). query_timeout: Duration, - /// Maximum age of a trusted-node rate event (`created_at`), taken from - /// the shared `[price].max_price_staleness_seconds` so upstream Nostr - /// freshness uses the same TTL the store enforces on cached quotes. + /// Maximum age of a trusted-node rate event (`created_at`) accepted at + /// ingestion: `max_price_staleness_seconds * nostr_ingestion_budget_pct` + /// (issue #860), not the full TTL. The store then enforces its own full + /// `max_price_staleness_seconds` window on top of whatever age the event + /// already had when accepted — using the full TTL for both would let a + /// just-under-TTL-old event be served for another full TTL afterwards + /// (~2x total age). Splitting the budget bounds total age at + /// `(1 + nostr_ingestion_budget_pct) × max_price_staleness_seconds` — + /// except that the product is floored at one second (`new` below), so a + /// config where `max_price_staleness_seconds * nostr_ingestion_budget_pct + /// < 1` gets that one-second minimum instead of its own smaller share, + /// widening the effective bound for that edge case only. max_age: Duration, } @@ -81,12 +90,15 @@ impl NostrProvider { /// client's own per-attempt timeout. /// /// `max_price_staleness_seconds` is the same shared TTL used by - /// [`crate::price::store::PriceStore`]: events older than that are not - /// eligible as this tick's source. + /// [`crate::price::store::PriceStore`]; `nostr_ingestion_budget_pct` + /// (issue #860) carves out the fraction of it an event's own `created_at` + /// may consume at ingestion, so the store's later full-TTL serving + /// window is not stacked on top of an already-near-TTL-old event. pub fn new( cfg: &ProviderConfig, provider_timeout_seconds: u64, max_price_staleness_seconds: i64, + nostr_ingestion_budget_pct: f64, ) -> Result { if cfg.trusted_nodes.is_empty() { return Err( @@ -110,9 +122,13 @@ impl NostrProvider { Ok(Self { trusted_nodes, query_timeout: Duration::from_secs(provider_timeout_seconds.max(1)), - // `PriceSettings::validate` already rejects non-positive values; - // clamp here so a zero can never make every event look fresh. - max_age: Duration::from_secs(max_price_staleness_seconds.max(1) as u64), + // `PriceSettings::validate` already rejects non-positive + // staleness/budget values; clamp here so a zero (or a scaled + // result that rounds down to zero) can never make every event + // look fresh. + max_age: Duration::from_secs( + ((max_price_staleness_seconds as f64 * nostr_ingestion_budget_pct) as u64).max(1), + ), }) } @@ -637,22 +653,79 @@ mod tests { #[test] fn new_parses_valid_hex_trusted_nodes() { let cfg = sample_cfg(Keys::generate().public_key().to_hex()); - assert!(NostrProvider::new(&cfg, 10, 1_800).is_ok()); + assert!(NostrProvider::new(&cfg, 10, 1_800, 0.5).is_ok()); } #[test] fn new_derives_query_timeout_and_max_age_from_shared_settings() { let cfg = sample_cfg(Keys::generate().public_key().to_hex()); - let provider = NostrProvider::new(&cfg, 7, 1_800).unwrap(); + // max_age is now the ingestion *budget* carved out of the full TTL + // (issue #860), not the TTL itself: 1_800 * 0.5 = 900. + let provider = NostrProvider::new(&cfg, 7, 1_800, 0.5).unwrap(); assert_eq!(provider.query_timeout, Duration::from_secs(7)); - assert_eq!(provider.max_age, Duration::from_secs(1_800)); + assert_eq!(provider.max_age, Duration::from_secs(900)); // A misconfigured 0 must not produce a zero-duration timeout/age. - let provider = NostrProvider::new(&cfg, 0, 0).unwrap(); + let provider = NostrProvider::new(&cfg, 0, 0, 0.5).unwrap(); assert_eq!(provider.query_timeout, Duration::from_secs(1)); assert_eq!(provider.max_age, Duration::from_secs(1)); } + #[test] + fn new_scales_max_age_by_ingestion_budget() { + let cfg = sample_cfg(Keys::generate().public_key().to_hex()); + let provider = NostrProvider::new(&cfg, 10, 2_000, 0.25).unwrap(); + assert_eq!(provider.max_age, Duration::from_secs(500)); + } + + #[test] + fn new_floors_a_budget_that_would_round_to_zero() { + let cfg = sample_cfg(Keys::generate().public_key().to_hex()); + // 1 * 0.001 rounds down to 0 seconds before the floor — must not + // produce a zero-duration max_age (every event would look "too old" + // instantly, or worse, a zero-width window has surprising edge + // behavior). + let provider = NostrProvider::new(&cfg, 10, 1, 0.001).unwrap(); + assert_eq!(provider.max_age, Duration::from_secs(1)); + } + + /// Regression for issue #860: with the old unscaled gate, an event up + /// to the *full* `max_price_staleness_seconds` old was accepted at + /// ingestion and then served for another full TTL by the store — total + /// ~2x age. With the default 0.5 ingestion budget, an event at 1000s + /// old (well inside the old 1800s window) must now be **rejected**, + /// since the ingestion budget is only 1800 * 0.5 = 900s. + #[test] + fn new_default_budget_rejects_an_event_the_old_unscaled_gate_would_have_accepted() { + let keys = Keys::generate(); + let trusted = vec![keys.public_key()]; + let cfg = sample_cfg(keys.public_key().to_hex()); + + let provider = NostrProvider::new(&cfg, 10, 1_800, 0.5).unwrap(); + assert_eq!(provider.max_age, Duration::from_secs(900)); + + let now = Timestamp::from(10_000u64); + let event_1000s_old = signed_event(&keys, SAMPLE_CONTENT, 10_000 - 1_000); + + // Old behavior (pre-#860, unscaled 1800s window): would have kept it. + assert!(!NostrProvider::rank_candidates( + std::slice::from_ref(&event_1000s_old), + &trusted, + now, + Duration::from_secs(1_800), + ) + .is_empty()); + + // New behavior (scaled 900s ingestion budget): rejected. + assert!(NostrProvider::rank_candidates( + &[event_1000s_old], + &trusted, + now, + provider.max_age, + ) + .is_empty()); + } + #[test] fn build_filter_has_kind_authors_and_identifier() { let node_a = Keys::generate().public_key(); @@ -667,7 +740,7 @@ mod tests { except: None, trusted_nodes: vec![node_a.to_hex(), node_b.to_hex()], }; - let provider = NostrProvider::new(&cfg, 10, 1_800).unwrap(); + let provider = NostrProvider::new(&cfg, 10, 1_800, 0.5).unwrap(); let expected = Filter::new() .kind(Kind::Custom(NOSTR_EXCHANGE_RATES_EVENT_KIND)) @@ -695,13 +768,13 @@ mod tests { except: None, trusted_nodes: vec![], }; - assert!(NostrProvider::new(&cfg, 10, 1_800).is_err()); + assert!(NostrProvider::new(&cfg, 10, 1_800, 0.5).is_err()); } #[test] fn new_rejects_invalid_hex_pubkey() { let cfg = sample_cfg("not-a-pubkey".to_string()); - assert!(NostrProvider::new(&cfg, 10, 1_800).is_err()); + assert!(NostrProvider::new(&cfg, 10, 1_800, 0.5).is_err()); } /// Live-relay evidence for issue #697: exercises the real `fetch()` path @@ -739,7 +812,7 @@ mod tests { "00000235a3e904cfe1213a8a54d6f1ec1bef7cc6bfaabd6193e82931ccf1366a".to_string(), ], }; - let provider = NostrProvider::new(&cfg, 10, 1_800).expect("valid hex pubkeys"); + let provider = NostrProvider::new(&cfg, 10, 1_800, 0.5).expect("valid hex pubkeys"); let http = reqwest::Client::new(); let quotes = provider.fetch(&http).await.expect("live relay fetch");