From d8b736a7cc11c55cbc0bf7f51b1ce146e61e4166 Mon Sep 17 00:00:00 2001 From: hartz0 Date: Thu, 2 Jul 2026 14:46:23 +0000 Subject: [PATCH 1/2] fix(#319,#333): oracle quorum check + reserve tracker duplicate currency guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #319 — update_rate now enforces max(min_signatures, MIN_ORACLE_SOURCE_FEEDS=3) sources when sources are provided. A single-validator submission with < required feeds is rejected with InsufficientOracleSources (#7009). Zero-source bypass is preserved. Issue #333 — add_currency checks the stored list before push and panics with DuplicateCurrency (#8008) on a duplicate, preventing double-counting of reserves. Adds get_currencies() to expose the tracked list. Also: - Resolves DataKey merge conflict: both last_verify_call (rate-limit) and currencies (dedup) are present in the struct - Adds tests for both fixes in their respective test files - Regenerates docs/ERROR_CODES.md with DuplicateCurrency = 8008 --- acbu_oracle/src/lib.rs | 12 ++- acbu_oracle/tests/test.rs | 139 +++++++++++++++++++++++++++++ acbu_reserve_tracker/src/lib.rs | 36 ++++++++ acbu_reserve_tracker/tests/test.rs | 93 +++++++++++++++++++ docs/ERROR_CODES.md | 6 +- 5 files changed, 280 insertions(+), 6 deletions(-) diff --git a/acbu_oracle/src/lib.rs b/acbu_oracle/src/lib.rs index 0487d28..4085334 100644 --- a/acbu_oracle/src/lib.rs +++ b/acbu_oracle/src/lib.rs @@ -419,8 +419,16 @@ impl OracleContract { } } - if sources.len() > 0 && sources.len() < MIN_ORACLE_SOURCE_FEEDS { - env.panic_with_error(OracleError::InsufficientOracleSources); + if sources.len() > 0 { + let min_sigs: u32 = env + .storage() + .instance() + .get(&DATA_KEY.min_signatures) + .unwrap(); + let required = min_sigs.max(MIN_ORACLE_SOURCE_FEEDS); + if sources.len() < required { + env.panic_with_error(OracleError::InsufficientOracleSources); + } } // Bypass median and outlier calculation workflows if 0 or 1 submissions exist diff --git a/acbu_oracle/tests/test.rs b/acbu_oracle/tests/test.rs index 50875d1..8b1b824 100644 --- a/acbu_oracle/tests/test.rs +++ b/acbu_oracle/tests/test.rs @@ -699,3 +699,142 @@ fn test_rate_not_initialized_before_first_submission() { let result = client.try_get_acbu_usd_rate_with_timestamp(); assert!(result.is_err(), "get_acbu_usd_rate_with_timestamp should fail before first submission"); } + +// ─── Oracle quorum / InsufficientOracleSources tests (#319) ────────────────── + +/// Submitting sources.len() > 0 but below max(min_signatures, MIN_ORACLE_SOURCE_FEEDS=3) +/// must panic with InsufficientOracleSources (#7009). +#[test] +#[should_panic] +fn test_update_rate_too_few_sources_panics() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().with_mut(|l| l.timestamp = 1_000_000); + + let admin = Address::generate(&env); + let validator = Address::generate(&env); + let mut validators = Vec::new(&env); + validators.push_back(validator.clone()); + + let ngn = CurrencyCode::new(&env, "NGN"); + let mut currencies = Vec::new(&env); + currencies.push_back(ngn.clone()); + let mut basket_weights = Map::new(&env); + basket_weights.set(ngn.clone(), 10_000i128); + + let contract_id = env.register_contract(None, OracleContract); + let client = OracleContractClient::new(&env, &contract_id); + // min_signatures = 1, but MIN_ORACLE_SOURCE_FEEDS = 3, so required = max(1,3) = 3 + client.initialize(&admin, &validators, &1u32, ¤cies, &basket_weights); + + let mut two_sources = Vec::new(&env); + two_sources.push_back(1_000_000i128); + two_sources.push_back(1_000_000i128); + // 2 sources < 3 required → must panic + client.update_rate(&validator, &ngn, &1_000_000i128, &two_sources, &env.ledger().timestamp()); +} + +/// Submitting exactly MIN_ORACLE_SOURCE_FEEDS sources must succeed. +#[test] +fn test_update_rate_exactly_min_sources_succeeds() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().with_mut(|l| l.timestamp = 1_000_000); + + let admin = Address::generate(&env); + let validator = Address::generate(&env); + let mut validators = Vec::new(&env); + validators.push_back(validator.clone()); + + let ngn = CurrencyCode::new(&env, "NGN"); + let mut currencies = Vec::new(&env); + currencies.push_back(ngn.clone()); + let mut basket_weights = Map::new(&env); + basket_weights.set(ngn.clone(), 10_000i128); + + let contract_id = env.register_contract(None, OracleContract); + let client = OracleContractClient::new(&env, &contract_id); + client.initialize(&admin, &validators, &1u32, ¤cies, &basket_weights); + + // Exactly 3 sources (MIN_ORACLE_SOURCE_FEEDS) — must not panic. + let mut three_sources = Vec::new(&env); + three_sources.push_back(1_000_000i128); + three_sources.push_back(1_001_000i128); + three_sources.push_back(999_000i128); + client.update_rate(&validator, &ngn, &1_000_000i128, &three_sources, &env.ledger().timestamp()); + + let stored = client.get_rate(&ngn); + // median of [999_000, 1_000_000, 1_001_000] = 1_000_000 + assert_eq!(stored, 1_000_000, "median of 3 sources should be 1_000_000"); +} + +/// Submitting zero sources must still be accepted (bypass path — rate param is used directly). +#[test] +fn test_update_rate_zero_sources_bypasses_quorum_check() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().with_mut(|l| l.timestamp = 1_000_000); + + let admin = Address::generate(&env); + let validator = Address::generate(&env); + let mut validators = Vec::new(&env); + validators.push_back(validator.clone()); + + let ngn = CurrencyCode::new(&env, "NGN"); + let mut currencies = Vec::new(&env); + currencies.push_back(ngn.clone()); + let mut basket_weights = Map::new(&env); + basket_weights.set(ngn.clone(), 10_000i128); + + let contract_id = env.register_contract(None, OracleContract); + let client = OracleContractClient::new(&env, &contract_id); + client.initialize(&admin, &validators, &1u32, ¤cies, &basket_weights); + + // Empty sources: quorum guard is skipped entirely; rate param is stored as-is. + let empty_sources = Vec::new(&env); + let submitted_rate = 1_234_567i128; + client.update_rate(&validator, &ngn, &submitted_rate, &empty_sources, &env.ledger().timestamp()); + + assert_eq!(client.get_rate(&ngn), submitted_rate, "empty sources should store the rate param directly"); +} + +/// When min_signatures > MIN_ORACLE_SOURCE_FEEDS the larger threshold applies. +#[test] +#[should_panic] +fn test_update_rate_min_sigs_larger_than_min_feeds_is_enforced() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().with_mut(|l| l.timestamp = 1_000_000); + + let admin = Address::generate(&env); + let v1 = Address::generate(&env); + let v2 = Address::generate(&env); + let v3 = Address::generate(&env); + let v4 = Address::generate(&env); + let v5 = Address::generate(&env); + + let mut validators = Vec::new(&env); + validators.push_back(v1.clone()); + validators.push_back(v2.clone()); + validators.push_back(v3.clone()); + validators.push_back(v4.clone()); + validators.push_back(v5.clone()); + + let ngn = CurrencyCode::new(&env, "NGN"); + let mut currencies = Vec::new(&env); + currencies.push_back(ngn.clone()); + let mut basket_weights = Map::new(&env); + basket_weights.set(ngn.clone(), 10_000i128); + + let contract_id = env.register_contract(None, OracleContract); + let client = OracleContractClient::new(&env, &contract_id); + // min_signatures = 5 > MIN_ORACLE_SOURCE_FEEDS = 3, so required = 5 + client.initialize(&admin, &validators, &5u32, ¤cies, &basket_weights); + + // Only 3 sources — below min_signatures=5 threshold → must panic + let mut three_sources = Vec::new(&env); + three_sources.push_back(1_000_000i128); + three_sources.push_back(1_001_000i128); + three_sources.push_back(999_000i128); + client.update_rate(&v1, &ngn, &1_000_000i128, &three_sources, &env.ledger().timestamp()); +} diff --git a/acbu_reserve_tracker/src/lib.rs b/acbu_reserve_tracker/src/lib.rs index 309066e..e4b640c 100644 --- a/acbu_reserve_tracker/src/lib.rs +++ b/acbu_reserve_tracker/src/lib.rs @@ -21,6 +21,7 @@ pub enum ReserveTrackerError { AdminTimelockNotElapsed = 8005, NoPendingAdminToCancel = 8006, Unauthorized = 8007, + DuplicateCurrency = 8008, Unknown = 8999, } @@ -34,6 +35,7 @@ impl Display for ReserveTrackerError { Self::AdminTimelockNotElapsed => "admin timelock has not elapsed", Self::NoPendingAdminToCancel => "no pending admin to cancel", Self::Unauthorized => "unauthorized", + Self::DuplicateCurrency => "currency already tracked", Self::Unknown => "unknown reserve tracker error", }; f.write_str(message) @@ -52,6 +54,7 @@ pub struct DataKey { pub pending_admin: Symbol, pub pending_admin_eligible_at: Symbol, pub last_verify_call: Symbol, + pub currencies: Symbol, } const DATA_KEY: DataKey = DataKey { @@ -64,6 +67,7 @@ const DATA_KEY: DataKey = DataKey { pending_admin: symbol_short!("PEND_ADM"), pending_admin_eligible_at: symbol_short!("PEND_ETA"), last_verify_call: symbol_short!("LAST_VFY"), + currencies: symbol_short!("CURRNCYS"), }; /// Admin rotation timelock: the pending admin must wait this long before @@ -302,6 +306,38 @@ impl ReserveTrackerContract { env.storage().instance().set(&DATA_KEY.reserves, &empty); } + // ----------------------------------------------------------------------- + // Currency list management (admin only) + // ----------------------------------------------------------------------- + + /// Add a currency to the tracked list. Panics with `DuplicateCurrency` if + /// the currency is already present, preventing double-counting of reserves. + pub fn add_currency(env: Env, currency: CurrencyCode) { + Self::check_admin(&env); + let mut currencies: Vec = env + .storage() + .instance() + .get(&DATA_KEY.currencies) + .unwrap_or(Vec::new(&env)); + for c in currencies.iter() { + if c == currency { + env.panic_with_error(ReserveTrackerError::DuplicateCurrency); + } + } + currencies.push_back(currency); + env.storage() + .instance() + .set(&DATA_KEY.currencies, ¤cies); + } + + /// Return the list of tracked currencies. + pub fn get_currencies(env: Env) -> Vec { + env.storage() + .instance() + .get(&DATA_KEY.currencies) + .unwrap_or(Vec::new(&env)) + } + // ----------------------------------------------------------------------- // Dependency address updaters (admin only) // ----------------------------------------------------------------------- diff --git a/acbu_reserve_tracker/tests/test.rs b/acbu_reserve_tracker/tests/test.rs index bf6c071..f444869 100644 --- a/acbu_reserve_tracker/tests/test.rs +++ b/acbu_reserve_tracker/tests/test.rs @@ -328,3 +328,96 @@ fn test_update_reserve_without_admin_auth_fails() { "update_reserve must reject callers that are not the admin" ); } + +// ─── Currency list management tests (#333) ─────────────────────────────────── + +#[test] +fn test_add_currency_and_get_currencies() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().with_mut(|l| l.timestamp = 1); + + let admin = Address::generate(&env); + let oracle = env.register_contract(None, MockOracle); + let acbu_token = Address::generate(&env); + + let contract_id = env.register_contract(None, ReserveTrackerContract); + let client = ReserveTrackerContractClient::new(&env, &contract_id); + client.initialize(&admin, &oracle, &acbu_token, &10_000i128); + + assert_eq!(client.get_currencies().len(), 0, "currencies should start empty"); + + let ngn = CurrencyCode::new(&env, "NGN"); + client.add_currency(&ngn); + assert_eq!(client.get_currencies().len(), 1, "one currency after first add"); + + let kes = CurrencyCode::new(&env, "KES"); + client.add_currency(&kes); + assert_eq!(client.get_currencies().len(), 2, "two currencies after second add"); + + let stored = client.get_currencies(); + assert!(stored.contains(ngn.clone()), "NGN must be in list"); + assert!(stored.contains(kes.clone()), "KES must be in list"); +} + +#[test] +#[should_panic(expected = "#8008")] +fn test_add_currency_duplicate_panics() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().with_mut(|l| l.timestamp = 1); + + let admin = Address::generate(&env); + let oracle = env.register_contract(None, MockOracle); + let acbu_token = Address::generate(&env); + + let contract_id = env.register_contract(None, ReserveTrackerContract); + let client = ReserveTrackerContractClient::new(&env, &contract_id); + client.initialize(&admin, &oracle, &acbu_token, &10_000i128); + + let ngn = CurrencyCode::new(&env, "NGN"); + client.add_currency(&ngn); + // Second add of same currency must panic with DuplicateCurrency = 8008 + client.add_currency(&ngn); +} + +#[test] +fn test_add_currency_without_admin_auth_fails() { + let env = Env::default(); + env.ledger().with_mut(|l| l.timestamp = 1); + + let admin = Address::generate(&env); + let attacker = Address::generate(&env); + let oracle = env.register_contract(None, MockOracle); + let acbu_token = Address::generate(&env); + + let contract_id = env.register_contract(None, ReserveTrackerContract); + let client = ReserveTrackerContractClient::new(&env, &contract_id); + + env.mock_all_auths(); + client.initialize(&admin, &oracle, &acbu_token, &10_000i128); + + use soroban_sdk::testutils::MockAuth; + use soroban_sdk::testutils::MockAuthInvoke; + use soroban_sdk::IntoVal; + let ngn = CurrencyCode::new(&env, "NGN"); + env.mock_auths(&[MockAuth { + address: &attacker, + invoke: &MockAuthInvoke { + contract: &contract_id, + fn_name: "add_currency", + args: (ngn.clone(),).into_val(&env), + sub_invokes: &[], + }, + }]); + + let result = client.try_add_currency(&ngn); + assert!(result.is_err(), "add_currency must reject non-admin callers"); + + env.mock_all_auths(); + assert_eq!( + client.get_currencies().len(), + 0, + "currency list must be unchanged after failed add" + ); +} diff --git a/docs/ERROR_CODES.md b/docs/ERROR_CODES.md index cb143e8..f9ed04a 100644 --- a/docs/ERROR_CODES.md +++ b/docs/ERROR_CODES.md @@ -20,10 +20,7 @@ Clients map `invoke_contract` / simulation failures using the contract error `u3 | 10 | `InsufficientBalance` | The account's token balance is too low to complete the transfer/operation. | | 11 | `InvalidRecipient` | The recipient address is invalid for this operation (e.g. a contract address where a classic account is required). | | 12 | `InvalidVersion` | WASM upgrade rejected: `new_version` must be greater than the stored version. | -| 13 | `NoPendingAdmin` | No admin transfer is in progress, so `accept_admin` has nothing to claim. | -| 14 | `AdminTimelockNotElapsed` | The two-step admin transfer timelock has not yet elapsed; the pending admin must wait before calling `accept_admin`. | -| 15 | `NoPendingAdminToCancel` | No admin transfer is in progress, so `cancel_admin_transfer` has nothing to cancel. | -| 9999 | `Unknown` | Catch-all for an unexpected/unclassified failure. Should not occur in normal operation; treat as an internal error. | +| 9999 | `Unknown` | unknown error | ## `shared / reentrancy guard` - `ReentrancyError` @@ -191,4 +188,5 @@ Clients map `invoke_contract` / simulation failures using the contract error `u3 | 8005 | `AdminTimelockNotElapsed` | admin timelock has not elapsed | | 8006 | `NoPendingAdminToCancel` | no pending admin to cancel | | 8007 | `Unauthorized` | unauthorized | +| 8008 | `DuplicateCurrency` | currency already tracked | | 8999 | `Unknown` | unknown reserve tracker error | From c7dc1c4d1c6d172f22d6cf14dce153fd51d1a70c Mon Sep 17 00:00:00 2001 From: hartz0 Date: Thu, 2 Jul 2026 14:54:15 +0000 Subject: [PATCH 2/2] fix: declare missing CurrencyNotRegistered = 7024 in OracleError enum The variant was referenced in assert_currency_registered() but never declared in the enum, causing a compile error whenever acbu_oracle is built. Adds the variant and its Display arm, then regenerates ERROR_CODES.md. --- acbu_oracle/src/lib.rs | 2 ++ docs/ERROR_CODES.md | 1 + 2 files changed, 3 insertions(+) diff --git a/acbu_oracle/src/lib.rs b/acbu_oracle/src/lib.rs index 4085334..f817197 100644 --- a/acbu_oracle/src/lib.rs +++ b/acbu_oracle/src/lib.rs @@ -38,6 +38,7 @@ pub enum OracleError { MaxValidatorsReached = 7021, TimestampRollback = 7022, RateNotInitialized = 7023, + CurrencyNotRegistered = 7024, Unknown = 7999, } @@ -67,6 +68,7 @@ impl Display for OracleError { Self::MaxValidatorsReached => "maximum validators reached", Self::TimestampRollback => "timestamp rollback", Self::RateNotInitialized => "rate not initialized - no submissions yet", + Self::CurrencyNotRegistered => "currency not registered", Self::Unknown => "unknown oracle error", }; f.write_str(message) diff --git a/docs/ERROR_CODES.md b/docs/ERROR_CODES.md index f9ed04a..a43513c 100644 --- a/docs/ERROR_CODES.md +++ b/docs/ERROR_CODES.md @@ -175,6 +175,7 @@ Clients map `invoke_contract` / simulation failures using the contract error `u3 | 7021 | `MaxValidatorsReached` | maximum validators reached | | 7022 | `TimestampRollback` | timestamp rollback | | 7023 | `RateNotInitialized` | rate not initialized - no submissions yet | +| 7024 | `CurrencyNotRegistered` | currency not registered | | 7999 | `Unknown` | unknown oracle error | ## `acbu_reserve_tracker` - `ReserveTrackerError`