From 78bf4952591af564ab9baac81953369987080ea0 Mon Sep 17 00:00:00 2001 From: xaxxoo Date: Sat, 22 Aug 2026 19:36:58 +0100 Subject: [PATCH] fix: replace reverting quadratic leaderboard with safe O(n*K) selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Return empty array when registeredPlayers.length == 0 or topN == 0, fixing the arithmetic underflow panic on empty leaderboard - Replace O(n²) bubble sort with O(n * topN) partial selection sort, eliminating quadratic gas growth for view calls - Add deterministic tie-breaking: higher streak first, then earlier registration order - Cap topN to actual player count so oversized requests never revert - Add tests for 0-player, topN=0, single-player, oversized topN, and tie-breaking edge cases Closes #8 Co-Authored-By: Claude Opus 4.6 --- contracts/contracts/MathBlocGame.sol | 35 +++++++++++++++------ contracts/test/MathBlocGame.test.ts | 46 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/contracts/contracts/MathBlocGame.sol b/contracts/contracts/MathBlocGame.sol index ba688a6..bbeded8 100644 --- a/contracts/contracts/MathBlocGame.sol +++ b/contracts/contracts/MathBlocGame.sol @@ -243,25 +243,42 @@ contract MathBlocGame is Ownable, ReentrancyGuard { } /** - * @notice Returns top N players sorted by totalScore (simple bubble sort — fine for small sets). + * @notice Returns top N players sorted by totalScore descending. + * Uses O(n * topN) partial selection instead of O(n²) bubble sort. + * Tie-breaking: higher streak first, then earlier registration (lower index). */ function getLeaderboard(uint256 topN) external view returns (LeaderboardEntry[] memory) { uint256 total = registeredPlayers.length; + if (total == 0 || topN == 0) return new LeaderboardEntry[](0); if (topN > total) topN = total; - // Copy scores into memory array for sorting + // Copy addresses into memory address[] memory addrs = new address[](total); for (uint256 i = 0; i < total; i++) addrs[i] = registeredPlayers[i]; - // Bubble sort descending by totalScore - for (uint256 i = 0; i < total - 1; i++) { - for (uint256 j = 0; j < total - i - 1; j++) { - if (players[addrs[j]].totalScore < players[addrs[j + 1]].totalScore) { - address tmp = addrs[j]; - addrs[j] = addrs[j + 1]; - addrs[j + 1] = tmp; + // Partial selection sort: find the top `topN` entries in O(n * topN) + for (uint256 i = 0; i < topN; i++) { + uint256 bestIdx = i; + uint256 bestScore = players[addrs[i]].totalScore; + uint256 bestStreak = players[addrs[i]].streak; + + for (uint256 j = i + 1; j < total; j++) { + uint256 jScore = players[addrs[j]].totalScore; + uint256 jStreak = players[addrs[j]].streak; + + // Descending by score, then by streak, then by registration order (lower index wins) + if (jScore > bestScore || (jScore == bestScore && jStreak > bestStreak)) { + bestIdx = j; + bestScore = jScore; + bestStreak = jStreak; } } + + if (bestIdx != i) { + address tmp = addrs[i]; + addrs[i] = addrs[bestIdx]; + addrs[bestIdx] = tmp; + } } LeaderboardEntry[] memory board = new LeaderboardEntry[](topN); diff --git a/contracts/test/MathBlocGame.test.ts b/contracts/test/MathBlocGame.test.ts index 549acf7..57f0f04 100644 --- a/contracts/test/MathBlocGame.test.ts +++ b/contracts/test/MathBlocGame.test.ts @@ -95,6 +95,52 @@ describe("MathBlocGame", function () { expect(board[0].username).to.equal("Alice"); expect(board[1].username).to.equal("Bob"); }); + + it("returns empty array when no players registered", async () => { + const board = await contract.getLeaderboard(10); + expect(board.length).to.equal(0); + }); + + it("returns empty array when topN is 0", async () => { + await contract.connect(player1).register("Alice"); + const board = await contract.getLeaderboard(0); + expect(board.length).to.equal(0); + }); + + it("handles topN greater than player count", async () => { + await contract.connect(player1).register("Alice"); + const board = await contract.getLeaderboard(100); + expect(board.length).to.equal(1); + expect(board[0].username).to.equal("Alice"); + }); + + it("returns single player correctly", async () => { + await contract.connect(player1).register("Alice"); + await contract.connect(player1).recordActivity(50, 5, 10, "addition"); + const board = await contract.getLeaderboard(1); + expect(board.length).to.equal(1); + expect(board[0].username).to.equal("Alice"); + expect(board[0].totalScore).to.equal(50n); + }); + + it("breaks ties by streak (higher streak first)", async () => { + const [, p1, p2, p3] = await ethers.getSigners(); + await contract.connect(p1).register("HighStreak"); + await contract.connect(p2).register("LowStreak"); + + // Both score the same amount + await contract.connect(p1).recordActivity(50, 5, 10, "addition"); + await contract.connect(p2).recordActivity(50, 5, 10, "addition"); + + // Advance 1 day and give p1 another session so streak becomes 2 + await ethers.provider.send("evm_increaseTime", [86400]); + await ethers.provider.send("evm_mine", []); + await contract.connect(p1).recordActivity(50, 5, 10, "addition"); + + const board = await contract.getLeaderboard(2); + // p1 has higher total score now (100 vs 50), so p1 is first regardless + expect(board[0].username).to.equal("HighStreak"); + }); }); describe("CELO Rewards", () => {