currencyrate: fix cached currencies to still use median.
What the developer wrote
Authored by Rusty Russell
currencyrate: fix cached currencies to still use median.
It was using *only* the latest one, not all the fresh ones.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes a bug in Core Lightning's currency-rate plugin. Previously, when exchange-rate data was cached, the plugin used only the most recent single source instead of combining all still-fresh sources and taking the median. Using a single source makes the converted bitcoin amount easier to manipulate if one price feed is wrong or malicious. The fix restores the intended median behavior across all fresh cached prices.
Review how the median is computed and ensure outlier rejection is robust; confirm that SERVE_TTL and background refresh intervals prevent use of stale data; consider adding tests for single-source and multi-source cache scenarios including adversarial price feeds.
Security signals we found
Single-source dependency instead of multi-source median
Potential price oracle manipulation via stale or compromised feed
Test re-enabled after xfail indicates prior known incorrect behavior
Evidence from the diff
In plugins/currencyrate-plugin/src/oracle.rs, latest_fresh_price() returned one Option
Changed components
plugins/currencyrate-plugin/src/oracle.rsCurrencyCache::fresh_prices / latest_fresh_priceBtcPriceOracle::converttests/test_currencyrate.py::test_cached_medianInspect captured patch +5 / −5
diff --git a/plugins/currencyrate-plugin/src/oracle.rs b/plugins/currencyrate-plugin/src/oracle.rs
index 0a68d672..5712cfd7 100644
--- a/plugins/currencyrate-plugin/src/oracle.rs
+++ b/plugins/currencyrate-plugin/src/oracle.rs
@@ -178,15 +178,15 @@ impl CurrencyCache {
}
}
- fn latest_fresh_price(&self) -> Option<SourceResult> {
+ fn fresh_prices(&self) -> Vec<SourceResult> {
self.prices
.iter()
.filter(|(_, p)| p.timestamp + SERVE_TTL > Instant::now())
- .max_by_key(|(_, p)| p.timestamp)
.map(|(n, p)| SourceResult {
name: n.clone(),
price: p.price,
})
+ .collect()
}
fn is_drift_ok(&self) -> bool {
@@ -313,8 +313,9 @@ impl BtcPriceOracle {
pub async fn convert(&self, amount: f64, currency: &str) -> Result<u64, anyhow::Error> {
let inner = self.inner.lock().await;
let source_results = if let Some(currency_cache) = inner.currencies.get(currency) {
- if let Some(price) = currency_cache.latest_fresh_price() {
- vec![price]
+ let prices = currency_cache.fresh_prices();
+ if !prices.is_empty() {
+ prices
} else {
log::warn!("background task failed to keep currency `{currency}` up to date");
drop(inner);
diff --git a/tests/test_currencyrate.py b/tests/test_currencyrate.py
index 2c28390f..728fbd90 100644
--- a/tests/test_currencyrate.py
+++ b/tests/test_currencyrate.py
@@ -238,7 +238,6 @@ def fake_rateserver():
srv.join()
-@pytest.mark.xfail(strict=True)
def test_cached_median(node_factory, fake_rateserver):
"""This should use the median of available sources"""
opts = {
Why this scored 38/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.