Feature/best vs ranked bid tests

docs/contracts/bid-ranking.md

@@ -15,8 +15,11 @@ Bids on an invoice are ranked by a deterministic algorithm so that businesses ca
 3. **Bid amount** – `bid_amount`  
    If profit and expected return are equal, higher bid amount ranks higher.
 
-4. **Timestamp (first-come)** – `timestamp`  
-   If all of the above are equal, the earlier bid (smaller timestamp) ranks higher.
+4. **Timestamp (recency preference)** – `timestamp`  
+   If all of the above are equal, the newer bid (larger timestamp) ranks higher.
+
+5. **Bid ID (deterministic final tie-breaker)** – `bid_id`  
+   If economics and timestamp are equal, lexicographically larger `bid_id` ranks higher.
 
 Comparison uses saturating arithmetic for profit to avoid overflow. The order is deterministic and does not depend on storage iteration order.
 
@@ -37,8 +40,16 @@ Returns all bids for the invoice that are in `Placed` status, sorted from best t
 - **Consistency**: `get_best_bid(invoice_id)` is equal to the first element of `get_ranked_bids(invoice_id)` when the latter is non-empty.
 - **Determinism**: Same set of bids always produces the same ranking; tie-breaks are fully specified above.
 
+## Regression Guarantees
+
+- Tie scenarios on each ordering layer (profit, expected return, bid amount,
+   timestamp, and bid ID) are covered by dedicated tests.
+- Insertion-order variance tests ensure `get_best_bid` remains identical to the
+   first element of `get_ranked_bids` even when tied bids are written in
+   different storage orders.
+
 ## Security and Testing
 
-- Ranking logic is unit-tested in `test_bid_ranking.rs` (empty list, single bid, multiple bids, equal bids, best-bid selection, non-existent invoice).
+- Ranking logic is unit-tested in `test_bid_ranking.rs` (empty list, single bid, multiple bids, equal bids, tie-layer regressions, best-vs-ranked invariant, non-existent invoice).
 - `compare_bids` uses `saturating_sub` for profit to avoid overflow (see `test_overflow.rs`).
 - No external or mutable state is used for ordering beyond bid fields and ledger timestamp for expiration.

quicklendx-contracts/src/bid.rs

@@ -469,8 +469,14 @@ impl BidStorage {
         }
         Ordering::Equal
     }
-    pub fn get_best_bid(env: &Env, invoice_id: &BytesN<32>) -> Option<Bid> {
-        let records = Self::get_bid_records_for_invoice(env, invoice_id);
+
+    /// Select the best placed bid from a bid list using `compare_bids`.
+    ///
+    /// # Security
+    /// This helper is used by both `get_best_bid` and `rank_bids` so they
+    /// cannot drift on tie handling. Any ordering change flows through one
+    /// path, preserving the invariant that best bid == first ranked bid.
+    fn select_best_placed_bid(records: &Vec<Bid>) -> Option<Bid> {
         let mut best: Option<Bid> = None;
         let mut idx: u32 = 0;
         while idx < records.len() {
@@ -493,6 +499,42 @@ impl BidStorage {
         }
         best
     }
+
+    /// Return the index of the best bid inside `records` using `compare_bids`.
+    fn select_best_index(records: &Vec<Bid>) -> Option<u32> {
+        if records.len() == 0 {
+            return None;
+        }
+
+        let mut best_idx: u32 = 0;
+        let mut best_bid = records.get(0).unwrap();
+        let mut idx: u32 = 1;
+        while idx < records.len() {
+            let candidate = records.get(idx).unwrap();
+            if Self::compare_bids(&candidate, &best_bid) == Ordering::Greater {
+                best_idx = idx;
+                best_bid = candidate;
+            }
+            idx += 1;
+        }
+        Some(best_idx)
+    }
+
+    /// Return the highest-ranked placed bid for an invoice.
+    ///
+    /// # Invariant
+    /// When `rank_bids` is non-empty, this method always returns the same bid
+    /// as `rank_bids(...).get(0)`.
+    pub fn get_best_bid(env: &Env, invoice_id: &BytesN<32>) -> Option<Bid> {
+        let records = Self::get_bid_records_for_invoice(env, invoice_id);
+        Self::select_best_placed_bid(&records)
+    }
+
+    /// Return all placed bids sorted from best to worst.
+    ///
+    /// # Invariant
+    /// If this function returns at least one bid, the first element equals the
+    /// value returned by `get_best_bid` for the same invoice and ledger state.
     pub fn rank_bids(env: &Env, invoice_id: &BytesN<32>) -> Vec<Bid> {
         let records = Self::get_bid_records_for_invoice(env, invoice_id);
         let mut remaining = Vec::new(env);
@@ -508,17 +550,8 @@ impl BidStorage {
         let mut ranked = Vec::new(env);
 
         while remaining.len() > 0 {
-            let mut best_idx: u32 = 0;
-            let mut best_bid = remaining.get(0).unwrap();
-            let mut search_idx: u32 = 1;
-            while search_idx < remaining.len() {
-                let candidate = remaining.get(search_idx).unwrap();
-                if Self::compare_bids(&candidate, &best_bid) == Ordering::Greater {
-                    best_idx = search_idx;
-                    best_bid = candidate;
-                }
-                search_idx += 1;
-            }
+            let best_idx = Self::select_best_index(&remaining).unwrap();
+            let best_bid = remaining.get(best_idx).unwrap();
             ranked.push_back(best_bid);
 
             let mut new_remaining = Vec::new(env);

quicklendx-contracts/src/test_bid_ranking.rs

@@ -42,6 +42,19 @@ fn persist_bid(env: &Env, bid: &Bid) {
     BidStorage::add_bid_to_invoice(env, &bid.invoice_id, &bid.bid_id);
 }
 
+fn assert_best_matches_first_ranked(env: &Env, invoice: &BytesN<32>) {
+    let ranked = BidStorage::rank_bids(env, invoice);
+    let best = BidStorage::get_best_bid(env, invoice);
+
+    if ranked.len() == 0 {
+        assert!(best.is_none());
+        return;
+    }
+
+    let best_bid = best.expect("best bid must exist when ranking is non-empty");
+    assert_eq!(best_bid.bid_id, ranked.get(0).unwrap().bid_id);
+}
+
 #[test]
 fn rank_bids_orders_by_profit_and_expected_return() {
     let env = Env::default();
@@ -211,3 +224,217 @@ fn get_best_bid_aligns_with_ranking_and_filters_non_placed() {
     let best = BidStorage::get_best_bid(&env, &invoice).unwrap();
     assert_eq!(best.bid_id, placed.bid_id);
 }
+
+#[test]
+fn best_bid_matches_first_ranked_on_expected_return_tie() {
+    let env = Env::default();
+    env.ledger().with_mut(|li| li.timestamp = 5_000);
+    let invoice = invoice_id(&env, 5);
+
+    let investor1 = Address::generate(&env);
+    let investor2 = Address::generate(&env);
+
+    // Equal profit (1000), different expected_return.
+    let lower_expected = build_bid(
+        &env,
+        &invoice,
+        &investor1,
+        5_000,
+        6_000,
+        10,
+        BidStatus::Placed,
+        1,
+    );
+    let higher_expected = build_bid(
+        &env,
+        &invoice,
+        &investor2,
+        6_000,
+        7_000,
+        20,
+        BidStatus::Placed,
+        2,
+    );
+
+    persist_bid(&env, &lower_expected);
+    persist_bid(&env, &higher_expected);
+
+    let ranked = BidStorage::rank_bids(&env, &invoice);
+    assert_eq!(ranked.get(0).unwrap().bid_id, higher_expected.bid_id);
+    assert_best_matches_first_ranked(&env, &invoice);
+}
+
+#[test]
+fn best_bid_matches_first_ranked_on_bid_amount_tie_breaker() {
+    let env = Env::default();
+    env.ledger().with_mut(|li| li.timestamp = 6_000);
+    let invoice = invoice_id(&env, 6);
+
+    let investor1 = Address::generate(&env);
+    let investor2 = Address::generate(&env);
+
+    // Equal profit and expected_return, different bid_amount.
+    let lower_amount = build_bid(
+        &env,
+        &invoice,
+        &investor1,
+        4_000,
+        6_000,
+        10,
+        BidStatus::Placed,
+        1,
+    );
+    let higher_amount = build_bid(
+        &env,
+        &invoice,
+        &investor2,
+        5_000,
+        6_000,
+        20,
+        BidStatus::Placed,
+        2,
+    );
+
+    persist_bid(&env, &lower_amount);
+    persist_bid(&env, &higher_amount);
+
+    let ranked = BidStorage::rank_bids(&env, &invoice);
+    assert_eq!(ranked.get(0).unwrap().bid_id, higher_amount.bid_id);
+    assert_best_matches_first_ranked(&env, &invoice);
+}
+
+#[test]
+fn best_bid_matches_first_ranked_on_timestamp_tie_breaker() {
+    let env = Env::default();
+    env.ledger().with_mut(|li| li.timestamp = 7_000);
+    let invoice = invoice_id(&env, 7);
+
+    let investor1 = Address::generate(&env);
+    let investor2 = Address::generate(&env);
+
+    // Equal economics and bid amount, timestamp decides.
+    let older = build_bid(
+        &env,
+        &invoice,
+        &investor1,
+        5_000,
+        6_000,
+        10,
+        BidStatus::Placed,
+        1,
+    );
+    let newer = build_bid(
+        &env,
+        &invoice,
+        &investor2,
+        5_000,
+        6_000,
+        20,
+        BidStatus::Placed,
+        2,
+    );
+
+    persist_bid(&env, &older);
+    persist_bid(&env, &newer);
+
+    let ranked = BidStorage::rank_bids(&env, &invoice);
+    assert_eq!(ranked.get(0).unwrap().bid_id, newer.bid_id);
+    assert_best_matches_first_ranked(&env, &invoice);
+}
+
+#[test]
+fn best_bid_matches_first_ranked_on_bid_id_final_tie_breaker() {
+    let env = Env::default();
+    env.ledger().with_mut(|li| li.timestamp = 8_000);
+    let invoice = invoice_id(&env, 8);
+
+    let investor = Address::generate(&env);
+
+    // Full tie except bid_id.
+    let lower_id = build_bid(
+        &env,
+        &invoice,
+        &investor,
+        5_000,
+        6_000,
+        15,
+        BidStatus::Placed,
+        1,
+    );
+    let higher_id = build_bid(
+        &env,
+        &invoice,
+        &investor,
+        5_000,
+        6_000,
+        15,
+        BidStatus::Placed,
+        9,
+    );
+
+    persist_bid(&env, &lower_id);
+    persist_bid(&env, &higher_id);
+
+    let ranked = BidStorage::rank_bids(&env, &invoice);
+    assert_eq!(ranked.get(0).unwrap().bid_id, higher_id.bid_id);
+    assert_best_matches_first_ranked(&env, &invoice);
+}
+
+#[test]
+fn best_bid_matches_first_ranked_independent_of_insertion_order_on_ties() {
+    // Dataset: equal profit and expected_return, timestamp/bid_id ties decide.
+    for &order in &[0u8, 1u8] {
+        let env = Env::default();
+        env.ledger().with_mut(|li| li.timestamp = 9_000);
+        let invoice = invoice_id(&env, 9u8.saturating_add(order));
+
+        let investor1 = Address::generate(&env);
+        let investor2 = Address::generate(&env);
+        let investor3 = Address::generate(&env);
+
+        let bid_a = build_bid(
+            &env,
+            &invoice,
+            &investor1,
+            5_000,
+            6_000,
+            10,
+            BidStatus::Placed,
+            1,
+        );
+        let bid_b = build_bid(
+            &env,
+            &invoice,
+            &investor2,
+            5_000,
+            6_000,
+            20,
+            BidStatus::Placed,
+            2,
+        );
+        let bid_c = build_bid(
+            &env,
+            &invoice,
+            &investor3,
+            5_000,
+            6_000,
+            20,
+            BidStatus::Placed,
+            9,
+        );
+
+        if order == 0 {
+            persist_bid(&env, &bid_a);
+            persist_bid(&env, &bid_b);
+            persist_bid(&env, &bid_c);
+        } else {
+            persist_bid(&env, &bid_c);
+            persist_bid(&env, &bid_b);
+            persist_bid(&env, &bid_a);
+        }
+
+        let ranked = BidStorage::rank_bids(&env, &invoice);
+        assert_eq!(ranked.get(0).unwrap().bid_id, bid_c.bid_id);
+        assert_best_matches_first_ranked(&env, &invoice);
+    }
+}