From c12f7a910897137fbfb6df9ebc157166a946951b Mon Sep 17 00:00:00 2001 From: temycodes Date: Wed, 24 Jun 2026 13:02:20 +0100 Subject: [PATCH] fix(#351): empty recipients returns no-op Vec instead of panic --- acbu_burning/src/lib.rs | 8 ++-- acbu_burning/tests/fuzz_test.rs | 69 ++++++++++++++++++++------------- acbu_burning/tests/test.rs | 5 ++- 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/acbu_burning/src/lib.rs b/acbu_burning/src/lib.rs index 350ab18..b612264 100644 --- a/acbu_burning/src/lib.rs +++ b/acbu_burning/src/lib.rs @@ -5,8 +5,8 @@ use soroban_sdk::{ }; use shared::{ - calculate_fee, BurnEvent, ContractError, CurrencyCode, DataKey as SharedDataKey, - reentrancy_guard, BASIS_POINTS, CONTRACT_VERSION, DECIMALS, MIN_BURN_AMOUNT, + calculate_fee, reentrancy_guard, BurnEvent, ContractError, CurrencyCode, + DataKey as SharedDataKey, BASIS_POINTS, CONTRACT_VERSION, DECIMALS, MIN_BURN_AMOUNT, ORACLE_GET_ACBU_RATE_WITH_TS, ORACLE_GET_BASKET_WEIGHT, ORACLE_GET_CURRENCIES, ORACLE_GET_RATE_WITH_TS, ORACLE_GET_S_TOKEN_ADDR, RESERVE_IS_SUFFICIENT, TOKEN_GET_TOTAL_SUPPLY, UPDATE_INTERVAL_SECONDS, @@ -230,7 +230,8 @@ impl BurningContract { user.require_auth(); if recipients.is_empty() { - env.panic_with_error(ContractError::InvalidRecipient); + reentrancy_guard::release_guard(&env); + return Vec::new(&env); } // C-057: Enforce all recipient addresses are distinct. @@ -505,7 +506,6 @@ impl BurningContract { env.storage().instance().set(&DATA_KEY.vault, &new_vault); } - // ----------------------------------------------------------------------- // Two-step admin rotation // ----------------------------------------------------------------------- diff --git a/acbu_burning/tests/fuzz_test.rs b/acbu_burning/tests/fuzz_test.rs index 89d48d7..4e04bf1 100644 --- a/acbu_burning/tests/fuzz_test.rs +++ b/acbu_burning/tests/fuzz_test.rs @@ -1,29 +1,32 @@ #![cfg(test)] use acbu_burning::{BurningContract, BurningContractClient}; -use shared::{CurrencyCode, DECIMALS}; -use soroban_sdk::{ - testutils::Address as _, - Address, Env, Vec -}; use proptest::prelude::*; +use shared::{CurrencyCode, DECIMALS}; +use soroban_sdk::{testutils::Address as _, Address, Env, Vec}; mod mocks { - use soroban_sdk::{contract, contractimpl, Env, Address, Vec}; use shared::CurrencyCode; use shared::DECIMALS; + use soroban_sdk::{contract, contractimpl, Address, Env, Vec}; #[contract] pub struct MockOracle; #[contractimpl] impl MockOracle { - pub fn get_acbu_usd_rate(_env: Env) -> i128 { DECIMALS } + pub fn get_acbu_usd_rate(_env: Env) -> i128 { + DECIMALS + } pub fn get_acbu_usd_rate_with_timestamp(env: Env) -> (i128, u64) { (DECIMALS, env.ledger().timestamp()) } - pub fn get_basket_weight(_env: Env, _c: CurrencyCode) -> i128 { 10_000 } - pub fn get_rate(_env: Env, _c: CurrencyCode) -> i128 { DECIMALS } + pub fn get_basket_weight(_env: Env, _c: CurrencyCode) -> i128 { + 10_000 + } + pub fn get_rate(_env: Env, _c: CurrencyCode) -> i128 { + DECIMALS + } pub fn get_rate_with_timestamp(env: Env, _c: CurrencyCode) -> (i128, u64) { (DECIMALS, env.ledger().timestamp()) } @@ -33,10 +36,15 @@ mod mocks { v } pub fn get_s_token_address(env: Env, _c: CurrencyCode) -> Address { - env.storage().instance().get(&soroban_sdk::symbol_short!("STK")).unwrap() + env.storage() + .instance() + .get(&soroban_sdk::symbol_short!("STK")) + .unwrap() } pub fn seed_stoken(env: Env, stoken: Address) { - env.storage().instance().set(&soroban_sdk::symbol_short!("STK"), &stoken); + env.storage() + .instance() + .set(&soroban_sdk::symbol_short!("STK"), &stoken); } } @@ -56,15 +64,22 @@ mod mocks { #[contractimpl] impl MockACBUToken { pub fn get_total_supply(env: Env) -> i128 { - env.storage().instance().get(&soroban_sdk::symbol_short!("supply")).unwrap_or(0) + env.storage() + .instance() + .get(&soroban_sdk::symbol_short!("supply")) + .unwrap_or(0) } pub fn burn(env: Env, _from: Address, amount: i128) { let supply = Self::get_total_supply(env.clone()); - env.storage().instance().set(&soroban_sdk::symbol_short!("supply"), &(supply - amount)); + env.storage() + .instance() + .set(&soroban_sdk::symbol_short!("supply"), &(supply - amount)); } pub fn mint(env: Env, _to: Address, amount: i128) { let supply = Self::get_total_supply(env.clone()); - env.storage().instance().set(&soroban_sdk::symbol_short!("supply"), &(supply + amount)); + env.storage() + .instance() + .set(&soroban_sdk::symbol_short!("supply"), &(supply + amount)); } } } @@ -74,17 +89,17 @@ proptest! { fn fuzz_redeem_basket_recipients(num_recipients in 0usize..20usize) { let env = Env::default(); env.mock_all_auths(); - + let admin = Address::generate(&env); let oracle = env.register_contract(None, mocks::MockOracle); let reserve_tracker = env.register_contract(None, mocks::MockReserveTracker); let acbu_token = env.register_contract(None, mocks::MockACBUToken); let contract_id = env.register_contract(None, BurningContract); let client = BurningContractClient::new(&env, &contract_id); - + let processor = Address::generate(&env); let vault = Address::generate(&env); - + client.initialize( &admin, &oracle, @@ -95,35 +110,37 @@ proptest! { &300, &100, ); - + let user = Address::generate(&env); let acbu_client = mocks::MockACBUTokenClient::new(&env, &acbu_token); acbu_client.mint(&user, &(1_000_000 * DECIMALS)); - + let stoken = env.register_stellar_asset_contract_v2(admin.clone()).address(); let stoken_sac = soroban_sdk::token::StellarAssetClient::new(&env, &stoken); stoken_sac.mint(&vault, &(1_000_000 * DECIMALS)); - + let token = soroban_sdk::token::Client::new(&env, &stoken); token.approve(&vault, &contract_id, &1_000_000_000_000_000, &100u32); - + let oracle_client = mocks::MockOracleClient::new(&env, &oracle); oracle_client.seed_stoken(&stoken); - + let mut recipients = Vec::new(&env); for _ in 0..num_recipients { recipients.push_back(Address::generate(&env)); } - + let burn_amount = 100 * DECIMALS; - + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { client.redeem_basket(&user, &recipients, &burn_amount); })); - + // Since get_currencies returns exactly 1 item ("NGN"), the recipients list length must be exactly 1 // as per the contract logic for basket redemptions. - if num_recipients != 1 { + if num_recipients == 0 { + assert!(res.is_ok()); + } else if num_recipients != 1 { assert!(res.is_err()); } else { assert!(res.is_ok()); diff --git a/acbu_burning/tests/test.rs b/acbu_burning/tests/test.rs index 1e54c92..53aebf5 100644 --- a/acbu_burning/tests/test.rs +++ b/acbu_burning/tests/test.rs @@ -104,13 +104,14 @@ fn test_redeem_basket_requires_vault_allowance() { } #[test] -fn test_redeem_basket_rejects_empty_recipients() { +fn test_redeem_basket_allows_empty_recipients_as_noop() { let env = Env::default(); let ctx = setup_test(&env); let empty: Vec
= Vec::new(&env); let result = ctx.burning.try_redeem_basket(&ctx.user, &empty, &(100 * DECIMALS)); - assert!(result.is_err()); + assert!(result.is_ok()); + assert_eq!(result.unwrap().unwrap().len(), 0); } #[test]