Skip to content

Comments

Fix: Jackpot Calculation and Comprehensive Test Coverage for Prize Accumulation#550

Merged
kimcascante merged 8 commits intoFutureMindsTeam:mainfrom
davidmelendez:fix/Jackpot-Validation-CreateNewDraw
Oct 13, 2025
Merged

Fix: Jackpot Calculation and Comprehensive Test Coverage for Prize Accumulation#550
kimcascante merged 8 commits intoFutureMindsTeam:mainfrom
davidmelendez:fix/Jackpot-Validation-CreateNewDraw

Conversation

@davidmelendez
Copy link
Collaborator

Fix: Jackpot Calculation and Comprehensive Test Coverage for Prize Accumulation

📌 Description

This PR fixes critical issues in jackpot calculation logic and adds comprehensive test coverage for lottery fund accumulation across multiple draws. The main fix ensures that when creating a new lottery draw after prize distribution, the jackpot is calculated correctly based on the previous draw's accumulated prize minus distributed prizes, rather than using the entire vault balance which incorrectly included administrative fees (45%).

Additionally, this PR refactors the test suite to properly validate jackpot accumulation scenarios and makes the Ticket struct public to enable better testing capabilities.

🎯 Motivation and Context

Problem 1: Jackpot Duplication
When creating a new lottery after distributing prizes, the jackpot was calculated as vault_balance - prizes_distributed. This incorrectly included the 45% administrative fees stored in the vault, causing jackpot values to be inflated. For example:

  • Expected: 11 STRKP - 1.65 STRKP = 9.35 STRKP
  • Actual: 18.35 STRKP (incorrect, including fees)

Problem 2: Insufficient Test Coverage
The existing tests only verified jackpot >= 0 without validating actual accumulation logic. Critical scenarios like prize distribution impact, carry-over between draws, and progressive accumulation were not tested.

Closes #455

🛠️ How to Test the Change

Testing Jackpot Calculation Fix:

  1. 🔹 Deploy lottery contract and initialize first draw
  2. 🔹 Buy tickets to accumulate jackpot (e.g., 2 tickets = 5.5 STRKP)
  3. 🔹 Execute DrawNumbers and DistributePrizes (e.g., 1.65 STRKP distributed)
  4. 🔹 Create new draw with CreateNewDrawWithDuration
  5. 🔹 Verify new jackpot = 5.5 - 1.65 = 3.85 STRKP (not including vault fees)

Testing New Test Suite:

cd packages/snfoundry
yarn test

Key tests to verify:

  • test_get_accumulated_prize_after_create_new_draw - Validates 2.75 STRKP carry-over
  • test_jackpot_accumulation_across_three_draws - Validates progression: 5.5 → 8.25 → 11 STRKP
  • test_jackpot_carryover_without_distribution - Validates full carry-over when prizes not distributed
  • test_jackpot_after_prize_distribution - Validates jackpot reduction after distribution
  • test_progressive_jackpot_five_draws - Validates 5-draw accumulation
  • test_jackpot_edge_cases - Validates zero jackpot scenarios

🖼️ Screenshots

First lottery: two tickets were purchased, leaving 5.5 STRKP in the prize pool.

Screenshot 2025-10-12 005632

In the first lottery, there were no winners, so the pool rolled over to the second lottery, totaling 11 STRKP after purchasing two new tickets.

Screenshot 2025-10-12 010157

The following image shows the winning numbers for the second lottery.

Screenshot 2025-10-12 010238

Based on the winning numbers, this ticket has three matching numbers, earning a reward of 1.1 STRKP.

Screenshot 2025-10-12 010458

Finally, in the third lottery, the prize pool decreased from 11 STRKP to 9.9 STRKP due to the previous 1.1 STRKP payout, visually demonstrating that the pool accumulation and reward distribution logic works consistently.

Screenshot 2025-10-12 010624

Additionally, the modified tests implementing the new pool accumulation logic and the new test cases were executed successfully, with no errors or warnings.

Screenshot 2025-10-12 151536

🔍 Type of Change

  • 🐞 Bugfix - Fixes jackpot duplication issue in CreateNewDrawWithDuration
  • New Feature - Public Ticket struct enables better testing
  • 🔄 Refactoring - Comprehensive test suite refactoring for jackpot accumulation
  • 🚀 Hotfix
  • 📖 Documentation
  • Other

✅ Checklist Before Merging

  • 🧪 I have tested the code and it works as expected.
  • 🎨 My changes follow the project's coding style.
  • 📖 I have updated the documentation if necessary.
  • ⚠️ No new warnings or errors were introduced.
  • 🔍 I have reviewed and approved my own code before submitting.

📌 Additional Notes

Core Contract Changes (Lottery.cairo)

1. Fixed Jackpot Calculation Logic (lines 788-815)

// Previous logic (incorrect):
if previous_draw.distribution_done {
    vault_balance - prizes_distributed  // ❌ Included 45% admin fees
}

// New logic (correct):
if previous_draw.distribution_done {
    previous_draw.accumulatedPrize - prizes_distributed  // ✅ Only uses legitimate jackpot
}

2. Made Ticket Struct Public (lines 17-29)

  • Changed struct Ticket to pub struct Ticket
  • Made all fields public to enable test validation of prize distribution

Test Suite Refactoring

Deleted 5 Duplicate/Unnecessary Tests:

  • test_get_jackpot_history_performance (test_jackpot_history.cairo)
  • test_jackpot_entry_getters_after_drawing (test_lottery_getters.cairo)
  • test_get_jackpot_history_initial (test_lottery_getters.cairo)
  • test_get_jackpot_history_multiple_draws (test_lottery_getters.cairo)
  • test_get_jackpot_history_after_completing_draws (test_lottery_getters.cairo)

Modified 6 Existing Tests:

  1. test_get_accumulated_prize_after_create_new_draw - Now validates exact 2.75 STRKP carry-over
  2. test_prize_distribution_with_updated_jackpot - Renamed and adapted to work with random winning numbers
  3. test_get_jackpot_entry_amount - Now validates exact 2.75 STRKP with ticket purchase
  4. test_get_jackpot_entry_start_block - Changed from timestamps to blocks
  5. test_get_jackpot_entry_end_block - Changed from timestamps to blocks
  6. test_get_jackpot_history_multiple_draws - Added ticket purchases to validate progressive accumulation

Created 7 New Critical Scenario Tests:

  1. test_jackpot_accumulation_across_three_draws - Validates 5.5 → 8.25 → 11 STRKP progression
  2. test_jackpot_multiple_purchases_same_draw - Validates 3 tickets = 8.25 STRKP
  3. test_jackpot_carryover_without_distribution - Validates full carry-over (5.5 STRKP)
  4. test_jackpot_after_prize_distribution - Validates jackpot reduction after DistributePrizes
  5. test_progressive_jackpot_five_draws - Validates 5 consecutive draws: 2.75 → 5.5 → 8.25 → 11 → 13.75 STRKP
  6. test_external_funds_addition_to_jackpot - Documents AddExternalFunds behavior
  7. test_jackpot_edge_cases - Validates zero jackpot scenarios
  8. test_vault_balance_matches_jackpot_allocation - Validates 55% allocation (8.25 of 15 STRKP)

Test Infrastructure Improvements

Added helper functions to test_jackpot_history.cairo:

  • create_valid_numbers() - Generates valid lottery numbers
  • create_valid_numbers_array(quantity) - Creates multiple ticket number sets
  • setup_mocks_for_buy_ticket() - Configures ERC20 mocks for ticket purchases
  • setup_mocks_for_multiple_tickets() - Simplified mock setup for multiple tickets
  • cleanup_mocks() - Cleans up mock calls
  • Updated deploy_lottery() to return mock ERC20 address for proper test setup

Key Validations

  • ✅ All tests pass without linter errors
  • ✅ Jackpot calculation now correctly uses only legitimate jackpot funds (55% of ticket sales)
  • ✅ Administrative fees (45%) are properly excluded from jackpot calculations
  • ✅ Comprehensive coverage of all jackpot accumulation scenarios
  • ✅ Tests work correctly with random winning numbers from Randomness contract

…tions

- Removed accumulatedPrize parameter from Initialize and CreateNewDraw functions.
- Introduced GetVaultBalance function to calculate jackpot automatically from vault balance.
- Added JackpotCalculated event to emit jackpot details upon draw creation.
- Updated totalPrizesDistributed mapping to track prizes distributed per draw.
- Enhanced jackpot calculation logic to ensure accurate prize distribution.
… users from the /dapp route to /dapp/dashboard.

- Updated Navbar and HeroSection components to point to the new /dapp/dashboard route instead of /dapp.
- Modified deployed contracts to reflect changes in jackpot calculation and prize distribution logic.
- Enhanced lottery contract functions to streamline prize distribution and jackpot management.
- Updated tests to align with the new jackpot calculation and removed unnecessary parameters from initialization functions.
…ze distribution handling. Updated jackpot determination based on previous draw's prize distribution status, ensuring accurate carryover of funds and safety checks for sufficient jackpot amounts. Enhanced comments for clarity.
…rove test coverage for jackpot calculations. Updated tests to validate jackpot accumulation across multiple draws and ensure accurate prize distribution handling. Introduced helper functions for ticket number generation and mock setups to streamline testing processes.
@davidmelendez davidmelendez self-assigned this Oct 12, 2025
Copy link
Collaborator

@kimcascante kimcascante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kimcascante kimcascante merged commit 488742f into FutureMindsTeam:main Oct 13, 2025
2 checks passed
@davidmelendez davidmelendez deleted the fix/Jackpot-Validation-CreateNewDraw branch October 13, 2025 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ISSUE-CU03-104: Jackpot Validation in CreateNewDraw

4 participants