diff --git a/contracts/invoice_liquidity/Cargo.toml b/contracts/invoice_liquidity/Cargo.toml index d030db4c..82b782f9 100644 --- a/contracts/invoice_liquidity/Cargo.toml +++ b/contracts/invoice_liquidity/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT OR Apache-2.0" crate-type = ["lib", "cdylib"] [dependencies] -insurance_pool = { path = "../insurance_pool" } +insurance_pool = { path = "../insurance_pool", version = "0.1.0" } soroban-sdk = { workspace = true } [dev-dependencies] diff --git a/contracts/invoice_liquidity/src/config.rs b/contracts/invoice_liquidity/src/config.rs index ffa8a373..b96c62cb 100644 --- a/contracts/invoice_liquidity/src/config.rs +++ b/contracts/invoice_liquidity/src/config.rs @@ -25,9 +25,36 @@ pub enum ConfigError { Unauthorized, InvalidBonusBps, InvalidMinDiscountRate, + /// decay_rate_bps exceeds the maximum allowed (Issue #604) — an + /// unbounded value (e.g. 10000 = 100%) would instantly zero all + /// reputation scores on the next decay application. + InvalidDecayRateBps, + /// decay_period_ledgers is below the minimum allowed (Issue #604) — a + /// value of 0 would disable decay entirely (guarded reads skip decay + /// when period is 0), silently breaking the reputation decay mechanism. + InvalidDecayPeriodLedgers, + /// dispute_timeout_ledgers is below the minimum allowed (Issue #604) — + /// a value of 0 would allow disputes to auto-resolve instantly, before + /// the payer has any opportunity to respond. + InvalidDisputeTimeoutLedgers, + /// high_rep_threshold is 0 (Issue #604) — this would make every LP + /// register as "high reputation" regardless of actual score. + InvalidHighRepThreshold, } const MAX_BONUS_BPS: u32 = 500; +/// Maximum decay_rate_bps (Issue #604): 5000 bps = 50% decay per period. +/// Bounding well below 10000 (100%) prevents a single governance call from +/// instantly zeroing every LP's reputation score. +const MAX_DECAY_RATE_BPS: u32 = 5000; +/// Minimum decay_period_ledgers (Issue #604): a period of 0 would disable +/// decay outright (see the `> 0` guards in invoice.rs / storage.rs), so a +/// floor prevents governance from silently neutering the decay mechanism. +const MIN_DECAY_PERIOD_LEDGERS: u64 = 100; +/// Minimum dispute_timeout_ledgers (Issue #604): ~1440 ledgers is roughly +/// one day at 5s/ledger — enough time for a payer to respond before a +/// dispute can be auto-resolved. +const MIN_DISPUTE_TIMEOUT_LEDGERS: u64 = 1440; #[allow(clippy::too_many_arguments)] pub fn update_config( @@ -56,6 +83,18 @@ pub fn update_config( if min_discount_rate_bps == 0 { return Err(ConfigError::InvalidMinDiscountRate); } + if decay_rate_bps == 0 || decay_rate_bps > MAX_DECAY_RATE_BPS { + return Err(ConfigError::InvalidDecayRateBps); + } + if decay_period_ledgers < MIN_DECAY_PERIOD_LEDGERS { + return Err(ConfigError::InvalidDecayPeriodLedgers); + } + if dispute_timeout_ledgers < MIN_DISPUTE_TIMEOUT_LEDGERS { + return Err(ConfigError::InvalidDisputeTimeoutLedgers); + } + if high_rep_threshold == 0 { + return Err(ConfigError::InvalidHighRepThreshold); + } let new_config = Config { high_rep_threshold, diff --git a/contracts/invoice_liquidity/src/errors.rs b/contracts/invoice_liquidity/src/errors.rs index bd63ebfd..496f0ee9 100644 --- a/contracts/invoice_liquidity/src/errors.rs +++ b/contracts/invoice_liquidity/src/errors.rs @@ -59,4 +59,38 @@ pub enum ContractError { Reentrancy = 37, /// Rate-limited function called before the cooldown period elapsed (Issue #541). RateLimited = 38, + /// Issue #604: bonus_bps exceeds the configured maximum. + InvalidBonusBps = 39, + /// Issue #604: min_discount_rate_bps is 0. + InvalidMinDiscountRate = 40, + /// Issue #604: decay_rate_bps is 0 or exceeds the configured maximum. + InvalidDecayRateBps = 41, + /// Issue #604: decay_period_ledgers is below the configured minimum. + InvalidDecayPeriodLedgers = 42, + /// Issue #604: dispute_timeout_ledgers is below the configured minimum. + InvalidDisputeTimeoutLedgers = 43, + /// Issue #604: high_rep_threshold is 0. + InvalidHighRepThreshold = 44, +} + +impl From for ContractError { + fn from(err: crate::config::ConfigError) -> Self { + match err { + crate::config::ConfigError::Unauthorized => ContractError::Unauthorized, + crate::config::ConfigError::InvalidBonusBps => ContractError::InvalidBonusBps, + crate::config::ConfigError::InvalidMinDiscountRate => { + ContractError::InvalidMinDiscountRate + } + crate::config::ConfigError::InvalidDecayRateBps => ContractError::InvalidDecayRateBps, + crate::config::ConfigError::InvalidDecayPeriodLedgers => { + ContractError::InvalidDecayPeriodLedgers + } + crate::config::ConfigError::InvalidDisputeTimeoutLedgers => { + ContractError::InvalidDisputeTimeoutLedgers + } + crate::config::ConfigError::InvalidHighRepThreshold => { + ContractError::InvalidHighRepThreshold + } + } + } } diff --git a/contracts/invoice_liquidity/src/lib.rs b/contracts/invoice_liquidity/src/lib.rs index 147c17bd..433663ce 100644 --- a/contracts/invoice_liquidity/src/lib.rs +++ b/contracts/invoice_liquidity/src/lib.rs @@ -2599,7 +2599,7 @@ impl InvoiceLiquidityContract { usdc_sac_address, eurc_sac_address, ) - .map_err(|_| ContractError::Unauthorized) + .map_err(ContractError::from) } pub fn get_config(env: Env) -> Result { diff --git a/contracts/invoice_liquidity/src/test.rs b/contracts/invoice_liquidity/src/test.rs index eba1748f..f9bb940e 100644 --- a/contracts/invoice_liquidity/src/test.rs +++ b/contracts/invoice_liquidity/src/test.rs @@ -1344,3 +1344,272 @@ fn test_get_version() { let version = t.contract.get_version(); assert_eq!(version, soroban_sdk::String::from_str(&t.env, "1.0.0")); } + +// ---------------------------------------------------------------- +// Issue #604: update_config parameter bounds validation +// ---------------------------------------------------------------- + +/// Builds a minimal admin-owned config directly in storage (bypassing the +/// public client) so each bounds test only needs to exercise +/// `crate::config::update_config` in isolation. +fn setup_config_for_bounds_test(env: &Env) -> (soroban_sdk::Address, soroban_sdk::Address) { + let admin = Address::generate(env); + let contract_id = env.register_contract(None, InvoiceLiquidityContract); + env.as_contract(&contract_id, || { + crate::storage::set_admin(env, &admin); + let config = crate::config::Config { + high_rep_threshold: 70, + bonus_bps: 100, + min_discount_rate_bps: 100, + decay_rate_bps: 50, + decay_period_ledgers: 10_000, + dispute_timeout_ledgers: 10_000, + xlm_sac_address: Address::generate(env), + usdc_sac_address: Address::generate(env), + eurc_sac_address: Address::generate(env), + price_oracle: None, + max_oracle_age_ledgers: 17_280, + }; + crate::storage::set_config(env, &config); + }); + (admin, contract_id) +} + +#[test] +fn test_update_config_valid_values_succeeds() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 50, + 10_000, + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert!(result.is_ok()); +} + +#[test] +fn test_update_config_rejects_decay_rate_bps_zero() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 0, // decay_rate_bps = 0 + 10_000, + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert_eq!(result, Err(crate::config::ConfigError::InvalidDecayRateBps)); +} + +#[test] +fn test_update_config_rejects_decay_rate_bps_at_10000() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 10_000, // 100% — would instantly zero all reputation scores + 10_000, + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert_eq!(result, Err(crate::config::ConfigError::InvalidDecayRateBps)); +} + +#[test] +fn test_update_config_accepts_decay_rate_bps_at_max_boundary() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 5_000, // exactly at the max boundary — should be accepted + 10_000, + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert!(result.is_ok()); +} + +#[test] +fn test_update_config_rejects_decay_period_ledgers_zero() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 50, + 0, // decay_period_ledgers = 0 — would cause division-by-zero risk + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert_eq!( + result, + Err(crate::config::ConfigError::InvalidDecayPeriodLedgers) + ); +} + +#[test] +fn test_update_config_accepts_decay_period_ledgers_at_min_boundary() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 50, + 100, // exactly at the min boundary — should be accepted + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert!(result.is_ok()); +} + +#[test] +fn test_update_config_rejects_dispute_timeout_ledgers_zero() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 50, + 10_000, + 0, // dispute_timeout_ledgers = 0 — allows instant auto-resolution + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert_eq!( + result, + Err(crate::config::ConfigError::InvalidDisputeTimeoutLedgers) + ); +} + +#[test] +fn test_update_config_accepts_dispute_timeout_ledgers_at_min_boundary() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 70, + 100, + 100, + 50, + 10_000, + 1_440, // exactly at the min boundary (~1 day) — should be accepted + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert!(result.is_ok()); +} + +#[test] +fn test_update_config_rejects_high_rep_threshold_zero() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 0, // high_rep_threshold = 0 — would make every LP "high reputation" + 100, + 100, + 50, + 10_000, + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert_eq!( + result, + Err(crate::config::ConfigError::InvalidHighRepThreshold) + ); +} + +#[test] +fn test_update_config_accepts_high_rep_threshold_at_min_boundary() { + let env = Env::default(); + env.mock_all_auths(); + let (admin, contract_id) = setup_config_for_bounds_test(&env); + let result = env.as_contract(&contract_id, || { + crate::config::update_config( + &env, + &admin, + 1, // exactly at the min boundary (just above 0) — should be accepted + 100, + 100, + 50, + 10_000, + 10_000, + Address::generate(&env), + Address::generate(&env), + Address::generate(&env), + ) + }); + assert!(result.is_ok()); +}