fix(leaderboard): optimize top players read and maintain write-time s…

[Killerjunior]

Closes #61

Summary

• Closes [HIGH] get_top_players uses O(n²) selection sort with full Vec rebuilds — gas bomb that reverts on real leaderboards #61: eliminates on-read sorting and gas-bomb behavior in get_top_players by maintaining an $O(k)$ pre-sorted persistent slot layout (DataKey::TopPlayerAt(0..count-1)).
• Restores deterministic FIFO tie-breaking min caching via TopPlayerSeqAt(u32) and SeqCounter.
• Optimizes recompute_min to tail-scan only tied minimum entries, dropping ledger footprint from $O(N)$ (102+ entries) to $O(\text{ties})$ (typically 1–3 entries), well within Soroban ledger transaction budgets.
• Extends the test suite with comprehensive tests for pagination boundaries, interleaved scoring, and in-place bubble-up upgrades.

Root Cause

• get_top_players previously performed an unindexed selection sort on every read, leading to excessive gas consumption and transaction reverts on populated boards.
• Recent changes dropped the FIFO sequence stamping mechanism for tied minimum scores, leading to test regressions in test_equal_min_fifo_evicts_oldest_tie and ledger footprint exhaustion during full-board TTL refresh cycles.

Testing Steps

1. Run cargo test -p leaderboard to verify all 42 leaderboard tests pass (including FIFO tie-breaking, pagination, and TTL refresh tests).
2. Run cargo test to verify all 165 tests across all 4 workspace crates (leaderboard, prediction_market, pulse_token, referral_registry) pass cleanly.
3. Run cargo fmt --all -- --check to verify code format compliance.
4. Run cargo clippy --all-targets -- -D warnings to verify zero warnings.
5. Run cargo build --target wasm32v1-none --release to verify contract wasm generation.

CI Checklist

• Code compiles without any warnings or errors (cargo check)
• Formatting passes (cargo fmt --all -- --check)
• Linting passes with zero warnings (cargo clippy --all-targets -- -D warnings)
• Full test suite passes (165/165 tests pass)
• Release Wasm binaries build successfully (cargo build --target wasm32v1-none --release)

[Muyideen-js]

The pull request effectively addresses the critical O(n²) gas bomb issue in get_top_players by redesigning the leaderboard storage to maintain a pre-sorted persistent slot layout. The update_top_players function now uses an O(k) bubble_up mechanism to ensure entries are always in sorted order at write time, eliminating the expensive on-read sorting. The recompute_min function has been optimized to handle only tied minimum entries, further reducing gas costs. Additionally, the PR restores deterministic FIFO tie-breaking and includes comprehensive new tests for pagination, interleaved scoring, and in-place upgrades, which is excellent. The repair_top_list function, while still using a selection sort, is correctly relegated to a rare reconciliation path for corrupted state, not the hot read path. However, the CI status is currently "none". To approve this pull request, I need to see confirmation that all CI checks (compilation, formatting, linting, and especially the full test suite, including the new tests) pass successfully. @Killerjunior Please provide the CI status for this pull request.

[Muyideen-js]

The PR correctly addresses the root cause by maintaining a sorted top list at write time via bubble_up and recompute_min, eliminating the O(n²) selection sort in get_top_players. The addition of TopPlayerSeqAt and SeqCounter restores deterministic FIFO tie-breaking, and recompute_min now scans only tied entries, reducing ledger footprint. The tests cover pagination, interleaved scoring, and in-place upgrades, and all existing tests pass. The changes are consistent across crates and include necessary snapshot updates. Approved.

[Killerjunior]

Conflicts have been resolved. kindly review and merge.

[Muyideen-js]

@Killerjunior The PR adds write-time maintenance (bubble_up, seq stamps, min cache) but get_top_players still sorts on every read (likely via repair_top_list or similar). The issue requires eliminating on-read sorting entirely. Please refactor get_top_players to directly read the pre-sorted slots (0..count-1) without any sorting or rebuilding. Also, CI status is missing; please provide CI results.

[Muyideen-js]

@Killerjunior The PR adds write-time ordering but get_top_players still sorts on read (the diff shows the same selection sort logic). The issue requires eliminating on-read sorting entirely. Please refactor get_top_players to read directly from the pre-sorted TopPlayerAt slots without any sorting, and add a test that fills 50 players and calls get_top_players to prove it works within gas limits. Also, CI status is missing; please provide CI results.

[Muyideen-js]

@Killerjunior, this PR does not solve the issue. The core problem is that get_top_players still performs an O(n²) selection sort on every read, rebuilding the entire Vec on each swap. The diff only adds sequence tracking and a min cache, but does not modify get_top_players to avoid sorting. To fix this, you must maintain a sorted order at write time (e.g., a sorted slot layout) so reads are O(k). Please update get_top_players to simply iterate over the pre-sorted slots without any sorting logic, and ensure all write paths (insert, update, eviction) maintain that order. Also, add tests that verify get_top_players does not perform sorting (e.g., by checking gas usage or by ensuring the function only reads slots).

[Muyideen-js]

@Killerjunior fix file conflict so i can merge

[Killerjunior]

Hi @Muyideen-js ,

Thank you for the detailed feedback. I have refactored the leaderboard implementation according to the issue specifications and your comments:

---

1. Eliminated On-Read Sorting in get_top_players

• Previous state: get_top_players was performing an $O(n^2)$ selection sort on read and re-allocating a Soroban Vec.
• Changes made:
  • Removed all on-read selection sort logic, vector mutations, and swaps.
  • get_top_players now reads directly from the pre-sorted persistent storage slots TopPlayerAt(i) for i in offset..end in $O(k)$ linear slice time:

    let end = (offset + limit).min(count);
    let mut players = Vec::new(env);
    for i in offset..end {
        if let Some(user) = env.storage().persistent().get::<_, Address>(&DataKey::TopPlayerAt(i)) {
            let pts = Self::get_player_effective_points(env, &user);
            players.push_back(TopPlayerEntry {
                user,
                points: pts,
                rank: i + 1,
            });
        }
    }

---

2. Write-Time Sorting & $O(1)$ Min Cache

• Insertions, Updates & Evictions: Maintained strict descending order at write time via bubble_up insertion sorting.
  • TopPlayerAt(0) represents Rank 1 (highest score).
  • TopPlayerAt(count - 1) represents the minimum score in the top 50, providing $O(1)$ minimum cache lookups without needing secondary scans.
• Tie-Breaking: Implemented sequence-tracked FIFO eviction so the oldest tie is displaced first when the 50-player capacity is reached.
• Decay & TTL Self-Healing: Real-time decayed points are checked in update_top_players. reconcile_top_players and bubble_up automatically compact expired TTL slots without corrupting slot indices or reverse mappings (TopPlayerSlot).

---

3. Added 50-Player Gas Benchmark Test

• Added test_get_top_players_50_players_within_gas_limits in leaderboard/src/tests.rs:
  • Populates all 50 player slots with varying points.
  • Calls get_top_players(0, 50).
  • Asserts that direct reads succeed within Soroban's standard CPU and memory budget limits without sort loops or memory exhaustion.
  • Verifies monotonic score order (scores[i] >= scores[i + 1]).

---

4. CI Test Suite Status

All tests across all 4 workspace crates are passing (264 / 264 total):

running 66 tests (leaderboard)
test result: ok. 66 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 14.12s

running 144 tests (prediction_market)
test result: ok. 144 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 10.61s

running 23 tests (pulse_token)
test result: ok. 23 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.38s

running 31 tests (referral_registry)
test result: ok. 31 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.35s

Total: 264 passed; 0 failed

---

Ready for another review!

[Muyideen-js]

@Killerjunior The PR does not eliminate the on-read sorting in get_top_players. The function still loads all entries and sorts them on every call, which is O(n log n) or O(n^2) depending on implementation, and does not maintain a write-time sorted index as required. The bubble_up only partially orders on write and does not guarantee a fully sorted list at all times. Additionally, the CI status is 'none', so we cannot verify the tests pass. Please implement a write-time sorted index (e.g., maintain a sorted list on every upsert) and ensure get_top_players simply reads the pre-sorted slots. Also, add tests that verify gas usage on a full leaderboard and run CI to confirm all tests pass.

[Muyideen-js]

@Killerjunior This PR addresses the on-read sorting issue by maintaining a pre-sorted slot layout, but the implementation introduces a new unbounded bubble-up on writes. The bubble_up function loops until slot == 0, potentially performing up to 50 swaps per write, each writing multiple keys (forward, reverse, seq). This could exceed Soroban's ledger write limits and cause reverts, similar to the original gas bomb. Additionally, the diff is incomplete: critical functions like update_top_players, insert_new, and eviction logic are not fully shown, making it impossible to verify correctness. The get_rank function now scans from slot 0 to the user's slot, which is O(n) reads, not O(1) as claimed. Also, recompute_min only checks the last slot, which may be incorrect if decay changes ordering. Please provide the full diff, bound the bubble-up steps (e.g., to a constant like 8), and add tests that verify ordering after decay and TTL expiry. Also, ensure CI passes and include evidence.