From 83d83f727f8dd22da4e5f885e1fbc0483b551a79 Mon Sep 17 00:00:00 2001 From: goldandrew Date: Mon, 27 Jul 2026 13:29:02 +0100 Subject: [PATCH] fix: oracle ops config and error hygiene - Add configurable SET_PRICES_TX_FEE / KEEPER_TX_FEE env vars, replacing hardcoded transaction inclusion fees in the keeper loop. - Fix config.rs env-parsing blocks that used bare `{}` blocks with `return`/`?` targeting the wrong Result type (EnvErrors vs EnvError), which failed to compile; wrap them in closures so early returns resolve correctly. - Fix ConfigError -> EnvError conversion for price_feed loading. - Carry truncated HTTP response bodies into Binance/Coinbase/Pyth/RPC error variants for better diagnostics. - De-duplicate keeper low-balance log spam: only emit `error!` on the transition into a below-minimum state, `debug!` while it persists, and `info!` on recovery. - Fix build_spot_price_url ignoring the caller's base_url, which caused Binance wiremock tests to hit the live API instead of the mock server. - Remove dead/duplicated code in binance.rs and signing.rs. - Add MIT license field to oracle/shared-config crates and LICENSE file. --- LICENSE | 21 ++++++++++++++ README.md | 3 ++ oracle/Cargo.toml | 1 + oracle/src/binance.rs | 47 +++++++++++++++++++++--------- oracle/src/coinbase.rs | 30 ++++++++++++++++---- oracle/src/config.rs | 60 ++++++++++++++++++++++++++++++++++----- oracle/src/http.rs | 12 ++++++++ oracle/src/keeper.rs | 51 ++++++++++++++++++++++++--------- oracle/src/keeper_loop.rs | 6 ++-- oracle/src/pyth.rs | 33 +++++++++++++++------ oracle/src/signing.rs | 1 - oracle/src/stellar_rpc.rs | 24 ++++++++++++---- oracle/src/submit.rs | 5 +++- shared/config/Cargo.toml | 1 + 14 files changed, 235 insertions(+), 60 deletions(-) create mode 100644 LICENSE diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..2166540 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 SO4 Markets + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index e09ad57..4bd3f79 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,9 @@ KEEPER_SECRET_KEY= KEEPER_ACCOUNT_ID= KEEPER_INDEX=0 MIN_KEEPER_BALANCE_XLM=10 +# Optional inclusion fees (stroops); defaults match historical hardcoded values +SET_PRICES_TX_FEE=1000000 +KEEPER_TX_FEE=2000000 # API configuration BIND_ADDR=0.0.0.0:8080 diff --git a/oracle/Cargo.toml b/oracle/Cargo.toml index 91b1da8..c2fc49c 100644 --- a/oracle/Cargo.toml +++ b/oracle/Cargo.toml @@ -3,6 +3,7 @@ name = "oracle" version = "0.1.0" edition = "2021" authors = ["0xsteins"] +license = "MIT" [lints] workspace = true diff --git a/oracle/src/binance.rs b/oracle/src/binance.rs index bbebe9f..0f04b40 100644 --- a/oracle/src/binance.rs +++ b/oracle/src/binance.rs @@ -6,7 +6,7 @@ pub const FLOAT_PRECISION: i128 = 1_000_000_000_000_000_000_000_000_000_000; #[derive(Debug, Clone, PartialEq, Eq)] pub enum BinancePriceError { NetworkError(String), - HttpError(u16), + HttpError { status: u16, body: String }, JsonError(String), PriceParseError(String), } @@ -15,7 +15,13 @@ impl std::fmt::Display for BinancePriceError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::NetworkError(error) => write!(f, "Binance network error: {error}"), - Self::HttpError(status) => write!(f, "Binance returned HTTP {status}"), + Self::HttpError { status, body } => { + if body.is_empty() { + write!(f, "Binance returned HTTP {status}") + } else { + write!(f, "Binance returned HTTP {status}: {body}") + } + } Self::JsonError(error) => write!(f, "invalid Binance response: {error}"), Self::PriceParseError(error) => write!(f, "invalid Binance price: {error}"), } @@ -65,7 +71,10 @@ pub fn parse_ticker_http_response( symbols: &[String], ) -> Result, BinancePriceError> { if status_code != 200 { - return Err(BinancePriceError::HttpError(status_code)); + return Err(BinancePriceError::HttpError { + status: status_code, + body: crate::http::truncate_error_body(body), + }); } parse_ticker_response_body(body, symbols) } @@ -79,12 +88,16 @@ pub fn parse_ticker_http_result( } fn build_spot_price_url(symbols: &[String]) -> String { + build_spot_price_url_for(BINANCE_TICKER_PRICE_URL, symbols) +} + +fn build_spot_price_url_for(base_url: &str, symbols: &[String]) -> String { if symbols.len() == 1 { - format!("{}?symbol={}", BINANCE_TICKER_PRICE_URL, symbols[0]) + format!("{}?symbol={}", base_url, symbols[0]) } else { format!( "{}?symbols={}", - BINANCE_TICKER_PRICE_URL, + base_url, serde_json::to_string(symbols).unwrap() ) } @@ -100,12 +113,7 @@ pub(crate) async fn fetch_spot_prices_with_url( base_url: &str, symbols: &[String], ) -> Result, BinancePriceError> { - let url = if symbols.len() == 1 { - format!("{}?symbol={}", base_url, symbols[0]) - } else { - base_url.to_string() - }; - let url = build_spot_price_url(symbols); + let url = build_spot_price_url_for(base_url, symbols); let response = crate::http::client() .get(&url) .send() @@ -279,7 +287,10 @@ mod tests { fn parse_ticker_http_response_non_200_returns_error() { let symbols = vec!["BTCUSDT".to_string()]; let err = parse_ticker_http_response(503, "[]", &symbols).unwrap_err(); - assert_eq!(err, BinancePriceError::HttpError(503)); + assert!(matches!( + err, + BinancePriceError::HttpError { status: 503, .. } + )); } #[test] @@ -353,7 +364,10 @@ mod tests { let err = super::fetch_spot_prices_with_url(&server.uri(), &symbols) .await .unwrap_err(); - assert_eq!(err, BinancePriceError::HttpError(404)); + assert!(matches!( + err, + BinancePriceError::HttpError { status: 404, .. } + )); } #[tokio::test] @@ -372,7 +386,12 @@ mod tests { let err = super::fetch_spot_prices_with_url(&server.uri(), &symbols) .await .unwrap_err(); - assert_eq!(err, BinancePriceError::HttpError(500)); + assert!(matches!( + err, + BinancePriceError::HttpError { status: 500, .. } + )); + } + #[test] fn build_spot_price_url_includes_symbols_parameter_for_multiple_symbols() { let symbols = vec!["BTCUSDT".to_string(), "ETHUSDT".to_string()]; diff --git a/oracle/src/coinbase.rs b/oracle/src/coinbase.rs index 13c2cd5..47a1d4a 100644 --- a/oracle/src/coinbase.rs +++ b/oracle/src/coinbase.rs @@ -6,7 +6,7 @@ pub const COINBASE_EXCHANGE_RATES_URL: &str = #[derive(Debug, Clone, PartialEq, Eq)] pub enum CoinbasePriceError { NetworkError(String), - HttpError(u16), + HttpError { status: u16, body: String }, JsonError(String), PriceParseError(String), MissingUsdRate, @@ -16,7 +16,13 @@ impl std::fmt::Display for CoinbasePriceError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::NetworkError(error) => write!(f, "Coinbase network error: {error}"), - Self::HttpError(status) => write!(f, "Coinbase returned HTTP {status}"), + Self::HttpError { status, body } => { + if body.is_empty() { + write!(f, "Coinbase returned HTTP {status}") + } else { + write!(f, "Coinbase returned HTTP {status}: {body}") + } + } Self::JsonError(error) => write!(f, "invalid Coinbase response: {error}"), Self::PriceParseError(error) => write!(f, "invalid Coinbase price: {error}"), Self::MissingUsdRate => f.write_str("Coinbase response has no USD rate"), @@ -60,7 +66,10 @@ pub fn parse_coinbase_http_response( body: &str, ) -> Result { if status_code != 200 { - return Err(CoinbasePriceError::HttpError(status_code)); + return Err(CoinbasePriceError::HttpError { + status: status_code, + body: crate::http::truncate_error_body(body), + }); } parse_coinbase_response_body(body) } @@ -152,7 +161,10 @@ mod tests { #[test] fn test_parse_coinbase_http_response_non_200() { let err = parse_coinbase_http_response(404, "{}").unwrap_err(); - assert_eq!(err, CoinbasePriceError::HttpError(404)); + assert!(matches!( + err, + CoinbasePriceError::HttpError { status: 404, .. } + )); } #[test] @@ -376,7 +388,10 @@ mod tests { let err = super::fetch_spot_price_with_url(&base_url, "BTC") .await .unwrap_err(); - assert_eq!(err, CoinbasePriceError::HttpError(404)); + assert!(matches!( + err, + CoinbasePriceError::HttpError { status: 404, .. } + )); } #[tokio::test] @@ -395,6 +410,9 @@ mod tests { let err = super::fetch_spot_price_with_url(&base_url, "BTC") .await .unwrap_err(); - assert_eq!(err, CoinbasePriceError::HttpError(500)); + assert!(matches!( + err, + CoinbasePriceError::HttpError { status: 500, .. } + )); } } diff --git a/oracle/src/config.rs b/oracle/src/config.rs index b5d15b0..7e05861 100644 --- a/oracle/src/config.rs +++ b/oracle/src/config.rs @@ -13,6 +13,10 @@ pub const DEFAULT_TESTNET_HORIZON_URL: &str = "https://horizon-testnet.stellar.o pub const DEFAULT_MAINNET_HORIZON_URL: &str = "https://horizon.stellar.org"; pub const DEFAULT_PRICE_LOOP_MS: u64 = 1_000; pub const DEFAULT_KEEPER_LOOP_MS: u64 = 1_500; +/// Default inclusion fee (stroops) for `set_prices` transactions. +pub const DEFAULT_SET_PRICES_TX_FEE: u32 = 1_000_000; +/// Default inclusion fee (stroops) for keeper handler transactions. +pub const DEFAULT_KEEPER_TX_FEE: u32 = 2_000_000; /// Oracle-specific view of a token feed config. /// Re-exports fields from `TokenConfig` for backward compatibility with @@ -80,6 +84,10 @@ pub struct Config { /// API key used to authenticate requests to the production Hermes endpoint. pub pyth_api_key: Option, pub min_keeper_balance_xlm: f64, + /// Inclusion fee (stroops) for oracle `set_prices` transactions. + pub set_prices_tx_fee: u32, + /// Inclusion fee (stroops) for keeper handler execute/freeze transactions. + pub keeper_tx_fee: u32, pub price_loop_interval: Duration, pub keeper_loop_interval: Duration, pub price_feed: PriceFeedConfig, @@ -198,7 +206,7 @@ impl Config { }; let price_feed = collect_or_default!( - load_price_feed_config(lookup("PRICE_FEED_CONFIG").as_deref()), + load_price_feed_config(lookup("PRICE_FEED_CONFIG").as_deref()).map_err(EnvError::from), PriceFeedConfig { tokens: vec![] } ); @@ -279,7 +287,7 @@ impl Config { .map(SecretString::new); let min_keeper_balance_xlm: f64 = collect_or_default!( - { + (|| { let value: f64 = parse_or_default( &mut lookup, "MIN_KEEPER_BALANCE_XLM", @@ -292,12 +300,48 @@ impl Config { }); } Ok(value) - }, + })(), DEFAULT_MIN_KEEPER_BALANCE_XLM ); + let set_prices_tx_fee: u32 = collect_or_default!( + (|| { + let fee: u32 = parse_or_default( + &mut lookup, + "SET_PRICES_TX_FEE", + &DEFAULT_SET_PRICES_TX_FEE.to_string(), + )?; + if fee == 0 { + return Err(EnvError::InvalidVar { + var: "SET_PRICES_TX_FEE", + reason: "must be greater than 0".to_string(), + }); + } + Ok(fee) + })(), + DEFAULT_SET_PRICES_TX_FEE + ); + + let keeper_tx_fee: u32 = collect_or_default!( + (|| { + let fee: u32 = parse_or_default( + &mut lookup, + "KEEPER_TX_FEE", + &DEFAULT_KEEPER_TX_FEE.to_string(), + )?; + if fee == 0 { + return Err(EnvError::InvalidVar { + var: "KEEPER_TX_FEE", + reason: "must be greater than 0".to_string(), + }); + } + Ok(fee) + })(), + DEFAULT_KEEPER_TX_FEE + ); + let price_loop_interval = collect_or_default!( - { + (|| { let ms: u64 = parse_or_default( &mut lookup, "PRICE_LOOP_MS", @@ -310,12 +354,12 @@ impl Config { }); } Ok(Duration::from_millis(ms)) - }, + })(), Duration::from_millis(DEFAULT_PRICE_LOOP_MS) ); let keeper_loop_interval = collect_or_default!( - { + (|| { let ms: u64 = parse_or_default( &mut lookup, "KEEPER_LOOP_MS", @@ -328,7 +372,7 @@ impl Config { }); } Ok(Duration::from_millis(ms)) - }, + })(), Duration::from_millis(DEFAULT_KEEPER_LOOP_MS) ); @@ -356,6 +400,8 @@ impl Config { admin_api_token, pyth_api_key, min_keeper_balance_xlm, + set_prices_tx_fee, + keeper_tx_fee, price_loop_interval, keeper_loop_interval, price_feed, diff --git a/oracle/src/http.rs b/oracle/src/http.rs index 19750b8..c81728e 100644 --- a/oracle/src/http.rs +++ b/oracle/src/http.rs @@ -7,6 +7,18 @@ use std::sync::OnceLock; use std::time::Duration; +/// Max chars of an HTTP error body retained in error variants / logs. +pub const HTTP_ERROR_BODY_MAX_CHARS: usize = 512; + +/// Truncate a response body for inclusion in error types. +pub fn truncate_error_body(body: &str) -> String { + let trimmed = body.trim(); + if trimmed.chars().count() <= HTTP_ERROR_BODY_MAX_CHARS { + return trimmed.to_string(); + } + trimmed.chars().take(HTTP_ERROR_BODY_MAX_CHARS).collect() +} + /// Returns the process-wide shared HTTP client, initializing it on first use (resolves #355). pub fn client() -> &'static reqwest::Client { static CLIENT: OnceLock = OnceLock::new(); diff --git a/oracle/src/keeper.rs b/oracle/src/keeper.rs index 59e10a7..20326de 100644 --- a/oracle/src/keeper.rs +++ b/oracle/src/keeper.rs @@ -1,3 +1,5 @@ +use std::sync::atomic::{AtomicBool, Ordering}; + use crate::stellar_rpc::{get_account_balance_stroops, RpcError}; /// 1 XLM expressed in stroops. @@ -6,6 +8,10 @@ pub const XLM_IN_STROOPS: i64 = 10_000_000; /// Default minimum keeper balance: 10 XLM. pub const DEFAULT_MIN_KEEPER_BALANCE_XLM: f64 = 10.0; +/// Tracks whether the keeper was already below the minimum so `/ready` probes +/// do not re-emit `error!` on every poll for a sustained low-balance condition. +static KEEPER_BALANCE_BELOW_MIN: AtomicBool = AtomicBool::new(false); + pub struct KeeperBalanceConfig { pub horizon_url: String, pub account_id: String, @@ -15,30 +21,49 @@ pub struct KeeperBalanceConfig { /// Check the keeper balance. Returns the current balance in stroops. /// -/// Logs a critical warning and optionally returns the balance even when below -/// threshold so the caller can decide whether to skip submission. +/// Logs `error!` only on the transition into the low-balance state (and +/// `info!` on recovery). Subsequent checks while the balance remains low +/// use `debug!` so readiness probes do not flood logs. pub async fn check_keeper_balance(cfg: &KeeperBalanceConfig) -> Result { let stroops = get_account_balance_stroops(&cfg.horizon_url, &cfg.account_id).await?; let xlm = stroops as f64 / XLM_IN_STROOPS as f64; if xlm < cfg.min_balance_xlm { - tracing::error!( - balance_xlm = xlm, - min_balance_xlm = cfg.min_balance_xlm, - account_id = cfg.account_id, - "keeper balance below minimum" - ); + let was_below = KEEPER_BALANCE_BELOW_MIN.swap(true, Ordering::Relaxed); + if was_below { + tracing::debug!( + balance_xlm = xlm, + min_balance_xlm = cfg.min_balance_xlm, + account_id = cfg.account_id, + "keeper balance still below minimum" + ); + } else { + tracing::error!( + balance_xlm = xlm, + min_balance_xlm = cfg.min_balance_xlm, + account_id = cfg.account_id, + "keeper balance below minimum" + ); + } return Err(RpcError::BalanceBelowMinimum { balance_xlm: xlm, min_xlm: cfg.min_balance_xlm, }); } - tracing::info!( - balance_xlm = xlm, - min_balance_xlm = cfg.min_balance_xlm, - "keeper balance ok" - ); + if KEEPER_BALANCE_BELOW_MIN.swap(false, Ordering::Relaxed) { + tracing::info!( + balance_xlm = xlm, + min_balance_xlm = cfg.min_balance_xlm, + "keeper balance recovered above minimum" + ); + } else { + tracing::debug!( + balance_xlm = xlm, + min_balance_xlm = cfg.min_balance_xlm, + "keeper balance ok" + ); + } Ok(stroops) } diff --git a/oracle/src/keeper_loop.rs b/oracle/src/keeper_loop.rs index 86986c5..3ceb71a 100644 --- a/oracle/src/keeper_loop.rs +++ b/oracle/src/keeper_loop.rs @@ -9,8 +9,6 @@ use crate::chain::scval; use crate::chain::tx_builder; use crate::state::{AppState, CachedPrice, FailedSubmission, KeeperExecution}; -const KEEPER_TX_FEE: u32 = 2_000_000; - pub async fn run_keeper_loop(state: Arc) { let mut ticker = interval(state.config.keeper_loop_interval); ticker.set_missed_tick_behavior(MissedTickBehavior::Delay); @@ -331,7 +329,7 @@ async fn set_prices_on_chain( &state.config.oracle_contract_id, "set_prices", vec![prices_scval], - 1_000_000, + state.config.set_prices_tx_fee, sequence, )?; @@ -374,7 +372,7 @@ async fn execute_handler( )?), key_scval, ], - KEEPER_TX_FEE, + state.config.keeper_tx_fee, sequence, )?; diff --git a/oracle/src/pyth.rs b/oracle/src/pyth.rs index 7b563df..7322195 100644 --- a/oracle/src/pyth.rs +++ b/oracle/src/pyth.rs @@ -8,7 +8,7 @@ pub const FLOAT_PRECISION: i128 = 1_000_000_000_000_000_000_000_000_000_000; #[derive(Debug, Clone, PartialEq)] pub enum PythPriceError { NetworkError(String), - HttpError(u16), + HttpError { status: u16, body: String }, JsonError(String), PriceParseError(String), MissingFeedId(String), @@ -27,7 +27,13 @@ impl std::fmt::Display for PythPriceError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::NetworkError(error) => write!(f, "Pyth network error: {error}"), - Self::HttpError(status) => write!(f, "Pyth returned HTTP {status}"), + Self::HttpError { status, body } => { + if body.is_empty() { + write!(f, "Pyth returned HTTP {status}") + } else { + write!(f, "Pyth returned HTTP {status}: {body}") + } + } Self::JsonError(error) => write!(f, "invalid Pyth response: {error}"), Self::PriceParseError(error) => write!(f, "invalid Pyth price: {error}"), Self::MissingFeedId(id) => write!(f, "Pyth response is missing feed {id}"), @@ -203,15 +209,18 @@ pub async fn fetch_pyth_prices( .map_err(|err| PythPriceError::NetworkError(err.to_string()))?; let status = response.status().as_u16(); - if status != 200 { - return Err(PythPriceError::HttpError(status)); - } - let body = response .text() .await .map_err(|err| PythPriceError::NetworkError(err.to_string()))?; + if status != 200 { + return Err(PythPriceError::HttpError { + status, + body: crate::http::truncate_error_body(&body), + }); + } + let response: HermesResponse = serde_json::from_str(&body).map_err(|err| PythPriceError::JsonError(err.to_string()))?; let feeds = match response { @@ -586,7 +595,10 @@ mod tests { let err = super::fetch_pyth_price_with_url(&server.uri(), "feed-1", 60, 50) .await .unwrap_err(); - assert_eq!(err, PythPriceError::HttpError(404)); + assert!(matches!( + err, + PythPriceError::HttpError { status: 404, .. } + )); } #[tokio::test] @@ -604,7 +616,12 @@ mod tests { let err = super::fetch_pyth_price_with_url(&server.uri(), "feed-1", 60, 50) .await .unwrap_err(); - assert_eq!(err, PythPriceError::HttpError(500)); + assert!(matches!( + err, + PythPriceError::HttpError { status: 500, .. } + )); + } + // #532 — array branch must match the requested feed_id, not blindly pop #[test] fn hermes_array_with_wrong_feed_id_returns_missing_feed_id_error() { diff --git a/oracle/src/signing.rs b/oracle/src/signing.rs index 60fbda4..3998ed3 100644 --- a/oracle/src/signing.rs +++ b/oracle/src/signing.rs @@ -62,7 +62,6 @@ pub fn sign_price( let key_array: [u8; 32] = key_bytes.try_into().unwrap(); let signing_key = SigningKey::from_bytes(&key_array); - let payload = build_price_message(network_passphrase, ledger_seq, token_strkey, min, max, timestamp); let payload = build_price_message( network_passphrase, ledger_seq, diff --git a/oracle/src/stellar_rpc.rs b/oracle/src/stellar_rpc.rs index 009edad..623a246 100644 --- a/oracle/src/stellar_rpc.rs +++ b/oracle/src/stellar_rpc.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Clone)] pub enum RpcError { NetworkError(String), - HttpError(u16), + HttpError { status: u16, body: String }, JsonError(String), RpcFault { code: i64, message: String }, BalanceBelowMinimum { balance_xlm: f64, min_xlm: f64 }, @@ -15,7 +15,13 @@ impl std::fmt::Display for RpcError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { RpcError::NetworkError(msg) => write!(f, "network error: {msg}"), - RpcError::HttpError(code) => write!(f, "HTTP {code}"), + RpcError::HttpError { status, body } => { + if body.is_empty() { + write!(f, "HTTP {status}") + } else { + write!(f, "HTTP {status}: {body}") + } + } RpcError::JsonError(msg) => write!(f, "JSON parse error: {msg}"), RpcError::RpcFault { code, message } => { write!(f, "RPC fault {code}: {message}") @@ -123,7 +129,10 @@ pub(crate) async fn rpc_post(rpc_url: &str, payload: String) -> Result