Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion docs/PRICE_PROVIDERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.

---

Expand Down
6 changes: 6 additions & 0 deletions settings.tpl.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 42 additions & 0 deletions src/price/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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)?;
}
Expand All @@ -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
}
Expand All @@ -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(),
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions src/price/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -725,6 +726,7 @@ fn build_provider(
cfg: &ProviderConfig,
provider_timeout_seconds: u64,
max_price_staleness_seconds: i64,
nostr_ingestion_budget_pct: f64,
) -> Result<Box<dyn PriceProvider>, String> {
match id {
ProviderId::Yadio => Ok(Box::new(YadioProvider::new(cfg))),
Expand All @@ -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,
)?)),
}
}
Expand Down
105 changes: 89 additions & 16 deletions src/price/providers/nostr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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<Self, String> {
if cfg.trusted_nodes.is_empty() {
return Err(
Expand All @@ -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),
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})
}

Expand Down Expand Up @@ -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();
Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down