Merge pull request #616 from KorexOnchain/fix/reputation-decay-601-unbounded-loop

Author: Levi-Ojukwu

contracts/invoice_liquidity/src/constants.rs

@@ -25,6 +25,14 @@ pub const UPGRADE_COOLDOWN_LEDGERS: u64 = 1440;
 /// Rate limit cooldown for economic parameters — 30 minutes (360 ledgers).
 pub const ECONOMIC_PARAM_COOLDOWN_LEDGERS: u64 = 360;
 
+// ----------------------------------------------------------------
+// Reputation Decay Bounds (Issue #601)
+// ----------------------------------------------------------------
+
+/// Maximum number of decay periods `get_payer_score` will iterate before
+/// short-circuiting the score to zero. See invoice.rs for full rationale.
+pub const MAX_REPUTATION_DECAY_PERIODS: u64 = 1000;
+
 /// Minimum number of ledgers that must elapse between the first LP joining the
 /// fund queue and `resolve_fund_queue` being callable.  At ~5 s per ledger,
 /// 120 ledgers ≈ 10 minutes, giving other LPs a fair window to join.

contracts/invoice_liquidity/src/invoice.rs

@@ -481,16 +481,23 @@ pub fn get_payer_score(env: &Env, payer: &Address) -> u32 {
                         u64::from(ledgers_since_activity) / decay_config.decay_period_ledgers;
 
                     // Apply decay: score = score * (1 - decay_rate/10000)^periods
-                    let mut decayed_score = rep.score as u64;
-                    for _ in 0..periods_passed {
-                        // Decay: subtract decay_rate_bps basis points (min 1 point)
-                        let mut decay_amount =
-                            (decayed_score * decay_config.decay_rate_bps as u64) / 10_000;
-                        if decay_amount == 0 && decayed_score > 0 {
-                            decay_amount = 1;
+                    // Issue #601: periods_passed is unbounded (governance-
+                    // configurable decay_period_ledgers can be set to 1),
+                    // so cap iteration and short-circuit to 0 beyond that.
+                    let decayed_score: u64 = if periods_passed > crate::constants::MAX_REPUTATION_DECAY_PERIODS {
+                        0
+                    } else {
+                        let mut decayed_score = rep.score as u64;
+                        for _ in 0..periods_passed {
+                            let mut decay_amount =
+                                (decayed_score * decay_config.decay_rate_bps as u64) / 10_000;
+                            if decay_amount == 0 && decayed_score > 0 {
+                                decay_amount = 1;
+                            }
+                            decayed_score = decayed_score.saturating_sub(decay_amount);
                         }
-                        decayed_score = decayed_score.saturating_sub(decay_amount);
-                    }
+                        decayed_score
+                    };
 
                     let new_score = (decayed_score.min(100)) as u32;
                     if new_score != rep.score {

contracts/invoice_liquidity/src/test.rs

@@ -1207,6 +1207,84 @@ fn test_reputation_score_never_goes_below_zero() {
     assert_eq!(score, 0, "Score should floor at 0, not go negative");
 }
 
+// ----------------------------------------------------------------
+// Regression tests — issue #601: bound reputation decay loop
+// ----------------------------------------------------------------
+
+#[test]
+fn test_reputation_decay_bounded_for_extremely_long_inactivity() {
+    let t = setup();
+
+    t.env.as_contract(&t.contract.address, || {
+        invoice::set_payer_score(&t.env, &t.payer, 80);
+    });
+
+    let config = Config {
+        high_rep_threshold: 80,
+        bonus_bps: 200,
+        min_discount_rate_bps: 100,
+        decay_rate_bps: 100,
+        decay_period_ledgers: 2,
+        dispute_timeout_ledgers: 100,
+        xlm_sac_address: Address::generate(&t.env),
+        usdc_sac_address: Address::generate(&t.env),
+        eurc_sac_address: Address::generate(&t.env),
+        price_oracle: None,
+        max_oracle_age_ledgers: 17280,
+    };
+    t.env.as_contract(&t.contract.address, || {
+        crate::storage::set_config(&t.env, &config);
+        t.env.storage().instance().extend_ttl(1_000_000, 2_000_000);
+    });
+
+    // periods_passed = 2,500 / 2 = 1,250 (1.25x the cap) — sustained
+    // long-term inactivity under a normal (non-griefing) decay period.
+    let mut ledger = t.env.ledger().get();
+    ledger.sequence_number += 2_500;
+    t.env.ledger().set(ledger);
+
+    let score = t.contract.payer_score(&t.payer);
+
+    assert_eq!(score, 0, "Score for a long-inactive payer should floor at 0, not hang or panic");
+}
+
+#[test]
+fn test_reputation_decay_bounded_when_decay_period_is_one_ledger() {
+    let t = setup();
+
+    t.env.as_contract(&t.contract.address, || {
+        invoice::set_payer_score(&t.env, &t.payer, 80);
+    });
+
+    // The exact griefing scenario from issue #601: decay_period_ledgers=1.
+    let config = Config {
+        high_rep_threshold: 80,
+        bonus_bps: 200,
+        min_discount_rate_bps: 100,
+        decay_rate_bps: 100,
+        decay_period_ledgers: 1,
+        dispute_timeout_ledgers: 100,
+        xlm_sac_address: Address::generate(&t.env),
+        usdc_sac_address: Address::generate(&t.env),
+        eurc_sac_address: Address::generate(&t.env),
+        price_oracle: None,
+        max_oracle_age_ledgers: 17280,
+    };
+    t.env.as_contract(&t.contract.address, || {
+        crate::storage::set_config(&t.env, &config);
+        t.env.storage().instance().extend_ttl(1_000_000, 2_000_000);
+    });
+
+    // periods_passed = 1,500 (1.5x the cap) with decay_period_ledgers=1.
+    let mut ledger = t.env.ledger().get();
+    ledger.sequence_number += 1_500;
+    t.env.ledger().set(ledger);
+
+    let score = t.contract.payer_score(&t.payer);
+
+    assert_eq!(score, 0, "decay_period_ledgers=1 with a large gap should floor at 0, not hang or panic");
+}
+
 #[test]
 fn test_reputation_score_never_exceeds_100() {
     let t = setup();

contracts/invoice_liquidity/src/tests_new_features.rs

@@ -580,7 +580,7 @@ fn test_batch_submit_all_valid_invoices() {
     let result = t.contract.try_submit_invoices_batch(&batch);
     assert!(result.is_ok());
 
-    let ids = result.unwrap();
+    let ids = result.unwrap().unwrap();
     assert_eq!(ids.len(), 5);
 
     // Verify all invoices were created with sequential IDs
@@ -673,7 +673,7 @@ fn test_batch_submit_referral_tracking() {
     let result = t.contract.try_submit_invoices_batch(&batch);
     assert!(result.is_ok());
 
-    let ids = result.unwrap();
+    let ids = result.unwrap().unwrap();
     assert_eq!(ids.len(), 3);
 
     // Verify referral count was incremented
@@ -708,7 +708,7 @@ fn test_batch_submit_exact_10_invoices_succeeds() {
     let result = t.contract.try_submit_invoices_batch(&batch);
     assert!(result.is_ok());
 
-    let ids = result.unwrap();
+    let ids = result.unwrap().unwrap();
     assert_eq!(ids.len(), 10);
 }
 

contracts/invoice_liquidity/src/tests_storage_layout.rs

@@ -2,21 +2,22 @@
 mod tests {
     use crate::invoice::{InvoiceCore, InvoiceMetadata, Invoice, InvoiceStatus, ReferralCode};
     use soroban_sdk::testutils::Address as TestAddress;
-    use soroban_sdk::Address;
+    use soroban_sdk::{Address, Env};
 
     #[test]
     fn test_invoice_to_core_split() {
+        let env = Env::default();
         // Create a full invoice
         let invoice = Invoice {
             id: 123,
-            freelancer: TestAddress::random(),
-            payer: TestAddress::random(),
-            token: TestAddress::random(),
+            freelancer: Address::generate(&env),
+            payer: Address::generate(&env),
+            token: Address::generate(&env),
             amount: 1_000_000,
             due_date: 1234567890,
             discount_rate: 300,
             status: InvoiceStatus::Pending,
-            funder: Some(TestAddress::random()),
+            funder: Some(Address::generate(&env)),
             funded_at: Some(1234567800),
             amount_funded: 0,
             amount_paid: 0,
@@ -43,11 +44,12 @@ mod tests {
 
     #[test]
     fn test_invoice_core_with_metadata_roundtrip() {
+        let env = Env::default();
         // Create core and metadata
-        let freelancer = TestAddress::random();
-        let payer = TestAddress::random();
-        let token = TestAddress::random();
-        let funder = TestAddress::random();
+        let freelancer = Address::generate(&env);
+        let payer = Address::generate(&env);
+        let token = Address::generate(&env);
+        let funder = Address::generate(&env);
 
         let core = InvoiceCore {
             id: 456,
@@ -94,14 +96,15 @@ mod tests {
 
     #[test]
     fn test_invoice_hot_cold_separation_consistency() {
+        let env = Env::default();
         // Test that extracting hot/cold and recombining gives same result
         let invoice = Invoice {
             id: 789,
-            freelancer: TestAddress::random(),
-            payer: TestAddress::random(),
-            token: TestAddress::random(),
+            freelancer: Address::generate(&env),
+            payer: Address::generate(&env),
+            token: Address::generate(&env),
             amount: 5_000_000,
-            due_date: 9876543210,
+            due_date: 987654321,
             discount_rate: 100,
             status: InvoiceStatus::PartiallyFunded,
             funder: None,