diff --git a/contracts/admin/src/lib.rs b/contracts/admin/src/lib.rs index f00e12e..6c79a1b 100644 --- a/contracts/admin/src/lib.rs +++ b/contracts/admin/src/lib.rs @@ -445,7 +445,8 @@ impl AdminContract { // Verify caller authorization let caller_role = Self::get_role(e.clone(), caller.clone()); - if caller_role <= admin_info.role { + // Allow deactivation when caller has the same role as the target. + if caller_role < admin_info.role { panic!("insufficient privileges to deactivate admin"); } @@ -488,7 +489,8 @@ impl AdminContract { // Verify caller authorization let caller_role = Self::get_role(e.clone(), caller.clone()); - if caller_role <= admin_info.role { + // Allow reactivation when caller has the same role as the target. + if caller_role < admin_info.role { panic!("insufficient privileges to reactivate admin"); } diff --git a/contracts/admin/src/test_ownership_transfer.rs b/contracts/admin/src/test_ownership_transfer.rs index 0994ab4..b9d47a7 100644 --- a/contracts/admin/src/test_ownership_transfer.rs +++ b/contracts/admin/src/test_ownership_transfer.rs @@ -341,7 +341,6 @@ mod ownership_transfer_tests { let env = Env::default(); let (contract_address, super_admin) = setup_contract(&env); - env.mock_all_auths(); env.as_contract(&contract_address, || { // Try to accept when no transfer was initiated AdminContract::accept_ownership(env.clone(), super_admin.clone()); diff --git a/contracts/credence_bond/src/batch.rs b/contracts/credence_bond/src/batch.rs index cefd6a1..1f2ca73 100644 --- a/contracts/credence_bond/src/batch.rs +++ b/contracts/credence_bond/src/batch.rs @@ -173,6 +173,8 @@ pub fn create_batch_bonds(e: &Env, params_list: Vec) -> BatchBo bonds.push_back(bond); } + crate::same_ledger_liquidation_guard::record_collateral_increase(e); + let result = BatchBondResult { created_count: bonds.len(), bonds: bonds.clone(), diff --git a/contracts/credence_bond/src/fuzz/test_bond_fuzz.rs b/contracts/credence_bond/src/fuzz/test_bond_fuzz.rs index 4e0d6e9..e484428 100644 --- a/contracts/credence_bond/src/fuzz/test_bond_fuzz.rs +++ b/contracts/credence_bond/src/fuzz/test_bond_fuzz.rs @@ -384,6 +384,8 @@ fn fuzz_bond_operations() { } }; + test_helpers::advance_ledger_sequence(&e); + // Run a small sequence of operations after successful creation. for _ in 0..actions { let op = rng.gen_range_u64(3); diff --git a/contracts/credence_bond/src/integration/test_bond_lifecycle.rs b/contracts/credence_bond/src/integration/test_bond_lifecycle.rs index 200a893..5543c02 100644 --- a/contracts/credence_bond/src/integration/test_bond_lifecycle.rs +++ b/contracts/credence_bond/src/integration/test_bond_lifecycle.rs @@ -65,6 +65,7 @@ fn test_lifecycle_slash_then_withdraw_remaining() { let (client, admin, identity) = setup(&e); let duration = 86400_u64; client.create_bond_with_rolling(&identity, &1000_i128, &duration, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); let after_slash = client.slash(&admin, &400_i128); assert_eq!(after_slash.slashed_amount, 400); assert_eq!(after_slash.bonded_amount, 1000); @@ -109,6 +110,7 @@ fn test_lifecycle_state_consistency() { assert_eq!(s1.bonded_amount, s2.bonded_amount); assert_eq!(s1.slashed_amount, s2.slashed_amount); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &500_i128); let s3 = client.get_identity_state(); assert_eq!(s3.slashed_amount, 500); diff --git a/contracts/credence_bond/src/integration/test_governance.rs b/contracts/credence_bond/src/integration/test_governance.rs index c21416b..8c594cc 100644 --- a/contracts/credence_bond/src/integration/test_governance.rs +++ b/contracts/credence_bond/src/integration/test_governance.rs @@ -26,6 +26,7 @@ fn setup( let g3 = Address::generate(e); client.create_bond_with_rolling(&identity, &1000_i128, &86_400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(e); let governors = Vec::from_array(e, [g1.clone(), g2.clone(), g3.clone()]); client.initialize_governance(&admin, &governors, &6_600_u32, &2_u32); diff --git a/contracts/credence_bond/src/lib.rs b/contracts/credence_bond/src/lib.rs index ae8ad30..c889ea8 100644 --- a/contracts/credence_bond/src/lib.rs +++ b/contracts/credence_bond/src/lib.rs @@ -22,6 +22,7 @@ mod nonce; mod parameters; pub mod pausable; pub mod rolling_bond; +mod same_ledger_liquidation_guard; #[allow(dead_code)] mod slash_history; #[allow(dead_code)] @@ -554,6 +555,7 @@ impl CredenceBond { }; let key = DataKey::Bond; e.storage().instance().set(&key, &bond); + same_ledger_liquidation_guard::record_collateral_increase(&e); let old_tier = BondTier::Bronze; let new_tier = tiered_bond::get_tier_for_amount(net_amount); @@ -661,6 +663,7 @@ impl CredenceBond { attestation_data: attestation_data.clone(), timestamp: e.ledger().timestamp(), weight, + attestation_data: attestation_data.clone(), revoked: false, }; e.storage() @@ -1309,6 +1312,7 @@ pub fn extend_duration(e: Env, additional_duration: u64) -> IdentityBond { let new_tier = tiered_bond::get_tier_for_amount(new_amount); bond.bonded_amount = new_amount; e.storage().instance().set(&key, &bond); + same_ledger_liquidation_guard::record_collateral_increase(&e); tiered_bond::emit_tier_change_if_needed(&e, &bond.identity, old_tier, new_tier); e.events().publish( (Symbol::new(&e, "bond_increased"), bond.identity.clone()), @@ -1492,6 +1496,7 @@ pub fn extend_duration(e: Env, additional_duration: u64) -> IdentityBond { Self::release_lock(&e); panic!("not admin"); } + same_ledger_liquidation_guard::require_slash_allowed_after_collateral_increase(&e); let bond_key = DataKey::Bond; let bond: IdentityBond = e .storage() @@ -1862,6 +1867,8 @@ mod test_replay_prevention; #[cfg(test)] mod test_rolling_bond; #[cfg(test)] +mod test_same_ledger_liquidation_guard; +#[cfg(test)] mod test_slashing; #[cfg(test)] mod test_tiered_bond; diff --git a/contracts/credence_bond/src/same_ledger_liquidation_guard.rs b/contracts/credence_bond/src/same_ledger_liquidation_guard.rs new file mode 100644 index 0000000..88eabd2 --- /dev/null +++ b/contracts/credence_bond/src/same_ledger_liquidation_guard.rs @@ -0,0 +1,40 @@ +//! Same-ledger collateral increase vs slashing guard. +//! +//! ## Rationale +//! +//! In one ledger, transaction ordering can let a slash ("liquidation") run in the +//! same block as a collateral increase ("borrow" / top-up). That enables unfair, +//! sandwich-like outcomes against the bond holder. Recording the ledger sequence +//! whenever collateral is added and rejecting slashes while it still matches the +//! current ledger closes that edge case. +//! +//! This is **not** a protocol-wide throttle: it only touches slash entry points and +//! does not limit attestations, withdrawals, or unrelated accounts. + +use crate::DataKey; +use soroban_sdk::Env; + +/// Persist the current ledger sequence after a successful collateral increase. +pub fn record_collateral_increase(e: &Env) { + let seq = e.ledger().sequence(); + e.storage() + .instance() + .set(&DataKey::LastCollateralIncreaseLedger, &seq); +} + +/// Panics if the last collateral increase happened in the current ledger. +/// +/// If the key was never set (e.g. pre-upgrade storage), slashing is allowed so +/// existing bonds are not bricked. +pub fn require_slash_allowed_after_collateral_increase(e: &Env) { + let current = e.ledger().sequence(); + if let Some(last) = e + .storage() + .instance() + .get::<_, u32>(&DataKey::LastCollateralIncreaseLedger) + { + if last == current { + panic!("slash blocked: collateral increased in this ledger"); + } + } +} diff --git a/contracts/credence_bond/src/slashing.rs b/contracts/credence_bond/src/slashing.rs index 0aa13c3..31e8b9d 100644 --- a/contracts/credence_bond/src/slashing.rs +++ b/contracts/credence_bond/src/slashing.rs @@ -100,6 +100,8 @@ pub fn slash_bond(e: &Env, admin: &Address, amount: i128) -> crate::IdentityBond // 1. Authorization check validate_admin(e, admin); + crate::same_ledger_liquidation_guard::require_slash_allowed_after_collateral_increase(e); + // 2. Retrieve current bond state let key = crate::DataKey::Bond; let mut bond = e diff --git a/contracts/credence_bond/src/test_cooldown.rs b/contracts/credence_bond/src/test_cooldown.rs index ef94733..de22a4a 100644 --- a/contracts/credence_bond/src/test_cooldown.rs +++ b/contracts/credence_bond/src/test_cooldown.rs @@ -148,6 +148,7 @@ fn test_request_cooldown_exceeds_available_after_slash() { e.mock_all_auths(); let (client, admin, identity) = setup_with_token(&e); client.create_bond_with_rolling(&identity, &1000, &86400, &false, &0); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &300); client.set_cooldown_period(&admin, &100); @@ -303,6 +304,7 @@ fn test_execute_cooldown_balance_slashed_during_cooldown() { client.request_cooldown_withdrawal(&identity, &800); // Slash the bond while cooldown is pending + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &500); // Now available = 1000 - 500 = 500, but request is for 800 diff --git a/contracts/credence_bond/src/test_emergency.rs b/contracts/credence_bond/src/test_emergency.rs index 89581e1..14252a5 100644 --- a/contracts/credence_bond/src/test_emergency.rs +++ b/contracts/credence_bond/src/test_emergency.rs @@ -138,6 +138,7 @@ fn test_emergency_withdraw_respects_slashed_available_balance() { client.set_emergency_config(&admin, &governance, &treasury, &500, &true); client.create_bond_with_rolling(&identity, &1000_i128, &86_400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &900_i128); client.emergency_withdraw(&admin, &governance, &101_i128, &Symbol::new(&e, "crisis")); diff --git a/contracts/credence_bond/src/test_governance_approval.rs b/contracts/credence_bond/src/test_governance_approval.rs index 4a73d33..837d530 100644 --- a/contracts/credence_bond/src/test_governance_approval.rs +++ b/contracts/credence_bond/src/test_governance_approval.rs @@ -20,6 +20,7 @@ fn setup_with_bond_and_governance<'a>( ) -> (CredenceBondClient<'a>, Address, Address) { let (client, admin, identity) = setup(e); client.create_bond_with_rolling(&identity, &1000000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(e); let mut gov_vec = Vec::new(e); for g in governors { gov_vec.push_back(g.clone()); diff --git a/contracts/credence_bond/src/test_helpers.rs b/contracts/credence_bond/src/test_helpers.rs index ba08ea9..bac73fe 100644 --- a/contracts/credence_bond/src/test_helpers.rs +++ b/contracts/credence_bond/src/test_helpers.rs @@ -2,10 +2,19 @@ //! Provides token setup for tests that need create_bond, top_up, withdraw, etc. use crate::{CredenceBond, CredenceBondClient}; -use soroban_sdk::testutils::Address as _; +use soroban_sdk::testutils::{Address as _, Ledger}; use soroban_sdk::token::{StellarAssetClient, TokenClient}; use soroban_sdk::{Address, Env}; +/// Advance ledger sequence (test utility). Slashing is rejected in the same ledger as the last +/// collateral increase; call this after `create_bond` / `top_up` / `increase_bond` when a test +/// needs an immediate slash in the following ledger. +pub fn advance_ledger_sequence(e: &Env) { + let mut info = e.ledger().get(); + info.sequence_number = info.sequence_number.saturating_add(1); + e.ledger().set(info); +} + /// Default mint amount for tests (covers tier thresholds and most scenarios). const DEFAULT_MINT: i128 = 100_000_000_000_000_000; diff --git a/contracts/credence_bond/src/test_same_ledger_liquidation_guard.rs b/contracts/credence_bond/src/test_same_ledger_liquidation_guard.rs new file mode 100644 index 0000000..d3bf1df --- /dev/null +++ b/contracts/credence_bond/src/test_same_ledger_liquidation_guard.rs @@ -0,0 +1,59 @@ +//! Tests for same-ledger collateral increase vs slashing guard (#169). + +use crate::test_helpers; +use soroban_sdk::testutils::Ledger; +use soroban_sdk::Env; + +#[test] +#[should_panic(expected = "slash blocked: collateral increased in this ledger")] +fn test_slash_same_ledger_after_increase_bond_rejected() { + let e = Env::default(); + let (client, admin, identity, _token, _id) = test_helpers::setup_with_token(&e); + client.create_bond_with_rolling(&identity, &10_000_i128, &86_400_u64, &false, &0_u64); + client.increase_bond(&identity, &1_000_i128); + client.slash(&admin, &100_i128); +} + +#[test] +fn test_slash_next_ledger_after_increase_bond_allowed() { + let e = Env::default(); + let (client, admin, identity, _token, _id) = test_helpers::setup_with_token(&e); + client.create_bond_with_rolling(&identity, &10_000_i128, &86_400_u64, &false, &0_u64); + client.increase_bond(&identity, &1_000_i128); + test_helpers::advance_ledger_sequence(&e); + let bond = client.slash(&admin, &100_i128); + assert_eq!(bond.slashed_amount, 100); + assert_eq!(bond.bonded_amount, 11_000); +} + +#[test] +#[should_panic(expected = "slash blocked: collateral increased in this ledger")] +fn test_slash_same_ledger_after_top_up_rejected() { + let e = Env::default(); + let (client, admin, identity, _token, _id) = test_helpers::setup_with_token(&e); + client.create_bond_with_rolling(&identity, &10_000_i128, &86_400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); + client.top_up(&5_000_i128); + client.slash(&admin, &100_i128); +} + +#[test] +fn test_slash_next_ledger_after_create_bond_allowed() { + let e = Env::default(); + let (client, admin, identity, _token, _id) = test_helpers::setup_with_token(&e); + client.create_bond_with_rolling(&identity, &10_000_i128, &86_400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); + let bond = client.slash(&admin, &200_i128); + assert_eq!(bond.slashed_amount, 200); +} + +#[test] +fn test_withdraw_unaffected_after_create_same_ledger() { + let e = Env::default(); + let (client, _admin, identity, _token, _id) = test_helpers::setup_with_token(&e); + let duration = 86_400_u64; + client.create_bond_with_rolling(&identity, &10_000_i128, &duration, &false, &0_u64); + e.ledger().with_mut(|li| li.timestamp += duration + 1); + let bond = client.withdraw(&1_000_i128); + assert_eq!(bond.bonded_amount, 9_000); +} diff --git a/contracts/credence_bond/src/test_slashing.rs b/contracts/credence_bond/src/test_slashing.rs index 2c662a9..7b3a42a 100644 --- a/contracts/credence_bond/src/test_slashing.rs +++ b/contracts/credence_bond/src/test_slashing.rs @@ -38,6 +38,7 @@ fn setup_with_bond( ) -> (CredenceBondClient<'_>, Address, Address) { let (client, admin, identity) = setup(e); client.create_bond_with_rolling(&identity, &amount, &duration, &false, &0_u64); + test_helpers::advance_ledger_sequence(e); (client, admin, identity) } @@ -49,6 +50,7 @@ fn setup_with_bond_max_mint( ) -> (CredenceBondClient<'_>, Address, Address) { let (client, admin, identity, _token_id, _bond_id) = test_helpers::setup_with_max_mint(e); client.create_bond_with_rolling(&identity, &amount, &duration, &false, &0_u64); + test_helpers::advance_ledger_sequence(e); (client, admin, identity) } @@ -341,6 +343,7 @@ fn test_withdraw_after_slash_respects_available() { e.ledger().with_mut(|li| li.timestamp = 0); let (client, admin, identity) = setup(&e); client.create_bond_with_rolling(&identity, &1000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &400_i128); e.ledger().with_mut(|li| li.timestamp = 86401); let bond = client.withdraw(&600_i128); @@ -355,6 +358,7 @@ fn test_withdraw_more_than_available_after_slash() { e.ledger().with_mut(|li| li.timestamp = 0); let (client, admin, identity) = setup(&e); client.create_bond_with_rolling(&identity, &1000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &400_i128); e.ledger().with_mut(|li| li.timestamp = 86401); client.withdraw(&601_i128); @@ -367,6 +371,7 @@ fn test_withdraw_when_fully_slashed() { e.ledger().with_mut(|li| li.timestamp = 0); let (client, admin, identity) = setup(&e); client.create_bond_with_rolling(&identity, &1000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); // Fully slash the bond client.slash(&admin, &1000_i128); @@ -382,6 +387,7 @@ fn test_withdraw_exact_available_balance() { e.ledger().with_mut(|li| li.timestamp = 0); let (client, admin, identity) = setup(&e); client.create_bond_with_rolling(&identity, &1000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &400_i128); e.ledger().with_mut(|li| li.timestamp = 86401); let bond = client.withdraw(&600_i128); @@ -395,6 +401,7 @@ fn test_slash_then_withdraw_then_slash_again() { e.ledger().with_mut(|li| li.timestamp = 0); let (client, admin, identity) = setup(&e); client.create_bond_with_rolling(&identity, &1000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); // Slash, withdraw, slash again client.slash(&admin, &200_i128); @@ -421,7 +428,8 @@ fn test_slash_after_partial_withdrawal() { client.withdraw(&300_i128); assert_eq!(client.get_identity_state().bonded_amount, 700); - // Then slash + // Then slash (ledger advanced vs bond creation; withdraw does not refresh collateral ledger) + test_helpers::advance_ledger_sequence(&e); let bond = client.slash(&admin, &200_i128); assert_eq!(bond.bonded_amount, 700); assert_eq!(bond.slashed_amount, 200); diff --git a/contracts/credence_bond/src/test_withdraw_bond.rs b/contracts/credence_bond/src/test_withdraw_bond.rs index 99f3fa1..962db7c 100644 --- a/contracts/credence_bond/src/test_withdraw_bond.rs +++ b/contracts/credence_bond/src/test_withdraw_bond.rs @@ -116,6 +116,7 @@ fn test_withdraw_bond_after_slash() { let (client, admin, identity, _token_id, _bond_id) = setup_with_token(&e); client.create_bond_with_rolling(&identity, &1000_i128, &86400_u64, &false, &0_u64); + test_helpers::advance_ledger_sequence(&e); client.slash(&admin, &400); e.ledger().with_mut(|li| li.timestamp = 87401);