merge v2 #10
merge v2 #10
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe updates to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Out of diff range and nitpick comments (3)
contracts/prudent-pots/src/tests/prepare_next_game.rs (1)
Line range hint
16-56: Enhance test coverage and clarity inprepare_next_game_no_raffle_works.
- Consider adding more detailed comments to explain each step of the test setup and assertions. This will help other developers understand the purpose and expected behavior of the test.
- Review the commented-out assertions on lines 45-47. If these checks are no longer relevant due to changes in responsibilities, remove the comments or update them to reflect the current logic.
contracts/prudent-pots/src/tests/instantiate.rs (1)
Line range hint
12-60: Review and optimize thesetup_game_no_raffle_worksfunction.
- Ensure that the game configuration setup in this function aligns with the expected game rules and parameters. For instance, validate that the
game_cw721address is always correct and that thedecay_factoris set to a sensible default.- Consider refactoring the function to improve readability and reduce complexity. For example, the loop for setting up pot allocations could be extracted into a separate helper function.
contracts/prudent-pots/src/query.rs (1)
Line range hint
18-115: Review and optimize query functions inquery.rs.
- Ensure that all query functions correctly access and return the intended data. For example, the
query_game_configfunction should accurately load and return the game configuration.- Consider adding error handling for cases where data cannot be loaded or calculations fail. This will improve the robustness of the query functions.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
contracts/prudent-pots/Cargo.tomlis excluded by!**/*.toml
Files selected for processing (19)
- contracts/prudent-pots/NOTICE (1 hunks)
- contracts/prudent-pots/src/contract.rs (3 hunks)
- contracts/prudent-pots/src/error.rs (2 hunks)
- contracts/prudent-pots/src/execute.rs (5 hunks)
- contracts/prudent-pots/src/helpers/game_end.rs (1 hunks)
- contracts/prudent-pots/src/helpers/mod.rs (1 hunks)
- contracts/prudent-pots/src/helpers/pot.rs (1 hunks)
- contracts/prudent-pots/src/helpers/validate.rs (1 hunks)
- contracts/prudent-pots/src/lib.rs (1 hunks)
- contracts/prudent-pots/src/msg.rs (1 hunks)
- contracts/prudent-pots/src/query.rs (2 hunks)
- contracts/prudent-pots/src/reply.rs (1 hunks)
- contracts/prudent-pots/src/state.rs (1 hunks)
- contracts/prudent-pots/src/tests/calculate_total_losing_tokens.rs (4 hunks)
- contracts/prudent-pots/src/tests/distribute_tokens.rs (3 hunks)
- contracts/prudent-pots/src/tests/instantiate.rs (4 hunks)
- contracts/prudent-pots/src/tests/is_winning_pot.rs (5 hunks)
- contracts/prudent-pots/src/tests/mod.rs (1 hunks)
- contracts/prudent-pots/src/tests/prepare_next_game.rs (3 hunks)
Files not reviewed due to errors (2)
- contracts/prudent-pots/src/helpers/pot.rs (no review received)
- contracts/prudent-pots/src/execute.rs (no review received)
Files skipped from review due to trivial changes (3)
- contracts/prudent-pots/NOTICE
- contracts/prudent-pots/src/helpers/mod.rs
- contracts/prudent-pots/src/lib.rs
Additional Context Used
GitHub Check Runs (1)
build-and-test failure (13)
contracts/prudent-pots/src/tests/mod.rs: [failure] 5-5:
file not found for moduleintegration
Additional comments not posted (24)
contracts/prudent-pots/src/tests/distribute_tokens.rs (1)
38-38: Test setup and assertions appear appropriate for the described scenario.contracts/prudent-pots/src/helpers/validate.rs (7)
8-24: Function correctly checks if a given address is the contract admin.
26-38: Function correctly checks if the initial amount per pot meets the minimum requirement.
40-60: Function correctly checks if a player has already allocated tokens to a specified pot.
62-90: Function correctly checks if adding a new amount to a pot exceeds the sum of all other pots.
92-119: Function correctly checks if the game has already ended and extends the game time if necessary.
121-130: Function correctly checks if the game is still active.
133-148: Function correctly validates and sums the funds in the specified denomination.contracts/prudent-pots/src/contract.rs (7)
Line range hint
18-53: Function correctly initializes the contract with the provided configuration and prepares the game for the next round.
69-74:UpdateConfigcase correctly updates the contract configuration based on the provided parameters.
92-122:AllocateTokenscase correctly handles the allocation of tokens to a specified pot and updates the player's allocation and pot state.
Line range hint
136-177:ReallocateTokenscase correctly handles the reallocation of tokens from one pot to another and updates the allocations and pot states.
Line range hint
186-253:GameEndcase correctly handles the end of the game, determines the winning pots, and redistributes the tokens. It also resets and prepares for the next game.
104-108: Function correctly handles reply messages based on their ID.
114-128: Function correctly handles different query messages and returns the requested data.contracts/prudent-pots/src/tests/calculate_total_losing_tokens.rs (5)
9-37: Test setup and assertions appear appropriate for the scenario with a single winner.
45-67: Test setup and assertions appear appropriate for the scenario with multiple winners.
Line range hint
74-94: Test setup and assertions appear appropriate for the scenario with no winners.
Line range hint
102-134: Test setup and assertions appear appropriate for the scenario where all pots are winners.
Line range hint
131-156: Test setup and assertions appear appropriate for the scenario with player allocations.contracts/prudent-pots/src/tests/is_winning_pot.rs (3)
Line range hint
9-53: Test setup and assertions appear appropriate for identifying the pot with the lowest token count as a winning pot.
Line range hint
53-90: Test setup and assertions appear appropriate for identifying the pot with an even token count as a winning pot.
Line range hint
90-127: Test setup and assertions appear appropriate for identifying the pot with the median token count as a winning pot.contracts/prudent-pots/src/helpers/game_end.rs (1)
134-135: Clarify the intended behavior in tie situations. The current implementation sets the winner toNone, which might not be the desired outcome.Verification successful
The review comment raises concerns about the handling of tie situations in the
get_raffle_winnerfunction, specifically pointing out that the winner is set toNonein certain scenarios. The provided script output shows that theget_raffle_winnerfunction is used in multiple places within thecontracts/prudent-pots/srcdirectory, including bothquery.rsandhelpers/game_end.rs.From the output, it's clear that the function is indeed being utilized, but the script does not provide information on how ties are handled within the function itself or whether setting the winner to
Noneis consistent with the intended behavior across the codebase.To fully address the review comment, we need to examine the implementation of
get_raffle_winnerto understand how it handles tie situations and whether setting the winner toNoneis a deliberate and consistent choice. This requires looking at the function's logic and any related comments or documentation within the code.Given the current information, we need to delve deeper into the
get_raffle_winnerfunction's implementation to verify the tie handling logic.
The provided script output has given us a partial view of the
get_raffle_winnerfunction's implementation. From the snippet, we can see that the function iterates over player allocations, calculating the total tokens each player has in the winning pots. The player with the highest total is considered for the winner.However, the script output was truncated before we could see how ties (situations where multiple players have the same maximum total) are handled. The review comment specifically concerns the behavior in tie situations, where the winner is set to
None. To fully address the review comment, we need to see the rest of the function to understand how it concludes, particularly how it handles cases where two or more players have the same maximum token total.Given the current information, we need to retrieve the remaining part of the
get_raffle_winnerfunction to verify how tie situations are handled and whether setting the winner toNoneis consistent and intentional.
The extended script output has provided the necessary details on how the
get_raffle_winnerfunction handles tie situations. The logic within the function checks if a player's total tokens in the winning pots exceed the current maximum. If so, that player becomes the current winner. If there is a tie (i.e., another player has the same total as the current maximum), the function attempts to determine the winner by checking who bid first among the tied players.However, if there is another tie in terms of the bidding time (i.e., two players bid at the exact same earliest time), the function sets the winner to
Noneand specifies that the raffle should not be distributed in that round. This is a deliberate mechanism to handle tie situations where no clear winner can be determined based on the provided criteria (total tokens and bidding time).Given this information, the behavior described in the review comment (setting the winner to
Nonein tie situations) is indeed the intended outcome as per the current implementation. This behavior is consistent and deliberate, designed to handle cases where a clear winner cannot be determined.Therefore, the review comment is consistent with the codebase, and the implementation aligns with the described behavior.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the tie handling logic in `get_raffle_winner`. # Test: Search for the function usage and check if tie handling is consistent across the codebase. rg --type rust $'get_raffle_winner'Length of output: 456
Script:
#!/bin/bash # Description: Examine the implementation of `get_raffle_winner` to understand how tie situations are handled. # Test: Retrieve the content of the function `get_raffle_winner` from `game_end.rs`. rg --type rust --context 20 'fn get_raffle_winner' contracts/prudent-pots/src/helpers/game_end.rsLength of output: 1407
Script:
#!/bin/bash # Description: Retrieve the remaining part of the `get_raffle_winner` function to understand tie handling. # Test: Continue retrieving the content of the function `get_raffle_winner` from `game_end.rs` starting from the line where the previous output was truncated. rg --type rust --context 20 --after-context 40 'if total_in_winning_pots > max_total' contracts/prudent-pots/src/helpers/game_end.rsLength of output: 1885
| // This reply function is called by reply_always. This should only do nothing, or throw an error, foreach NFT send during game_end workflow to the raflle winner. | ||
| pub fn game_end_reply(msg: SubMsgResult) -> Result<Response, ContractError> { | ||
| match msg.into() { | ||
| Ok(_data) => Ok(Response::new().add_attributes(vec![ | ||
| attr("method", "reply"), | ||
| attr("action", "game_end_reply"), | ||
| ])), | ||
| _ => Err(ContractError::Cw721TokenNotReceived {}), | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure proper documentation and error handling in game_end_reply.
- The comment on line 5 could be improved for clarity. Consider specifying what "do nothing" means in this context and elaborate on the conditions under which an error should be thrown.
- The error handling on line 12 only covers a generic failure case. It might be beneficial to handle different types of errors specifically if applicable.
contracts/prudent-pots/src/msg.rs
Outdated
| UpdateConfig { | ||
| fee: Option<u64>, | ||
| fee_reallocation: Option<u64>, | ||
| fee_address: Option<Addr>, | ||
| game_denom: Option<String>, | ||
| game_cw721: Option<Addr>, // this is the cw721 collection addy we use as optional raffle prize | ||
| game_duration: Option<u64>, | ||
| game_extend: Option<u64>, | ||
| min_pot_initial_allocation: Option<Uint128>, | ||
| decay_factor: Option<Uint128>, | ||
| }, | ||
| AllocateTokens { | ||
| pot_id: u8, | ||
| }, | ||
| ReallocateTokens { | ||
| from_pot_id: u8, | ||
| to_pot_id: u8, | ||
| }, | ||
| GameEnd { | ||
| raffle_cw721_token_id: Option<String>, | ||
| }, | ||
| } | ||
|
|
||
| #[cw_serde] | ||
| pub struct MigrateMsg {} | ||
| /// Reply | ||
|
|
||
| #[derive(FromPrimitive, IntoPrimitive)] | ||
| #[repr(u64)] | ||
| pub enum ReplyMsg { | ||
| GameEnd = 1, | ||
| #[default] | ||
| Unknown, | ||
| } | ||
|
|
||
| #[cw_serde] | ||
| #[derive(QueryResponses)] | ||
| pub enum QueryMsg { | ||
| #[returns(QueryGameConfigResponse)] | ||
| QueryGameConfig {}, | ||
| #[returns(QueryGameStateResponse)] | ||
| QueryGameState {}, | ||
| #[returns(QueryBidRangeResponse)] | ||
| QueryBidRange {}, | ||
| #[returns(QueryPotStateResponse)] | ||
| QueryPotState { pot_id: u8 }, | ||
| #[returns(QueryPotsStateResponse)] | ||
| QueryPotsState {}, | ||
| #[returns(QueryWinningPotsReponse)] | ||
| QueryWinningPots {}, | ||
| #[returns(QueryPlayerAllocationsResponse)] | ||
| QueryPlayerAllocations { address: Addr }, | ||
| #[returns(QueryReallocationFeePoolResponse)] | ||
| QueryReallocationFeePool {}, | ||
| } | ||
|
|
||
| #[cw_serde] | ||
| pub struct QueryGameConfigResponse { | ||
| #[returns(GameConfigResponse)] | ||
| GameConfig {}, | ||
| #[returns(GameStateResponse)] | ||
| GameState {}, | ||
| #[returns(BidRangeResponse)] | ||
| BidRange { cw721_count: usize }, | ||
| #[returns(PotStateResponse)] | ||
| PotState { pot_id: u8 }, | ||
| #[returns(PotsStateResponse)] | ||
| PotsState {}, | ||
| #[returns(WinningPotsResponse)] | ||
| WinningPots {}, | ||
| #[returns(PlayerAllocationsResponse)] | ||
| PlayerAllocations { address: String }, | ||
| #[returns(AllPlayersAllocationsResponse)] | ||
| AllPlayersAllocations {}, | ||
| #[returns(ReallocationFeePoolResponse)] | ||
| ReallocationFeePool {}, | ||
| #[returns(RaffleResponse)] | ||
| Raffle {}, | ||
| #[returns(RaffleWinnerResponse)] | ||
| RaffleWinner {}, | ||
| #[returns(RaffleDenomSplitResponse)] | ||
| RaffleDenomSplit {}, |
There was a problem hiding this comment.
Review and validate updates to message structures in msg.rs.
- Ensure that all new fields and options in message structures are necessary and correctly used throughout the contract. For example, the optional fields in
UpdateConfigshould have corresponding logic to handle cases where they are not provided. - Consider adding more documentation to new fields to explain their purpose and how they should be used.
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (6)
contracts/prudent-pots/src/tests/integration/reallocate_tokens.rs (1)
29-53: Check the correctness of the expected pots state calculation.The calculation for
amountinTokenAllocationforpot_id: 2seems complex and error-prone. Consider simplifying or adding a comment explaining the logic.contracts/prudent-pots/src/tests/integration/update_config.rs (1)
39-52: Check the correctness of the expected new configuration.The configuration values are hardcoded in the test. Consider using variables or constants to make the test more maintainable and readable.
contracts/prudent-pots/src/tests/integration/helpers.rs (1)
37-53: Check the logic of thereallocate_tokensfunction.The logic for calculating the reallocation seems complex. Consider adding comments or simplifying the logic to improve readability and maintainability.
contracts/prudent-pots/src/tests/integration/allocate_tokens.rs (1)
17-34: Check the correctness of the expected pots state after token allocation.The expected pots state is hardcoded. Consider using variables or constants to make the test more maintainable and readable.
contracts/prudent-pots/src/tests/integration/fixtures.rs (1)
64-220: Check the setup logic indefault_with_balances.The function setup is complex and involves many steps. Consider breaking it down into smaller functions or adding detailed comments to improve readability and maintainability.
contracts/prudent-pots/src/tests/integration/game_end.rs (1)
155-418: Check the correctness and complexity of the raffle logic intest_game_end_one_winner_raffle_both_works.The raffle logic is complex and involves multiple steps and calculations. Consider simplifying the logic or adding detailed comments to improve readability and maintainability.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- contracts/prudent-pots/src/tests/integration/allocate_tokens.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/fixtures.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/game_end.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/helpers.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/instantiate.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/mod.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/reallocate_tokens.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/update_config.rs (1 hunks)
Files skipped from review due to trivial changes (2)
- contracts/prudent-pots/src/tests/integration/instantiate.rs
- contracts/prudent-pots/src/tests/integration/mod.rs
Additional comments not posted (5)
contracts/prudent-pots/src/tests/integration/reallocate_tokens.rs (1)
18-20: Ensure thereallocate_tokensfunction handles errors gracefully.Verification successful
The
reallocate_tokensfunction in theexecute.rsfile does indeed handle errors gracefully. It uses Rust's error propagation mechanism (?operator) to handle potential errors during the updating of pot states. This confirms that the function is designed to manage errors effectively as expected in robust software practices.
- File:
contracts/prudent-pots/src/execute.rs- Method:
reallocate_tokensScripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `reallocate_tokens` function has proper error handling. # Test: Search for error handling in the `reallocate_tokens` function. Expect: Error handling code. rg --type rust 'reallocate_tokens' --context 5Length of output: 9316
contracts/prudent-pots/src/tests/integration/helpers.rs (2)
10-21: Check the use ofADMIN_ADDRESSinupdate_config.Verification successful
The review comment asked to verify the use of
ADMIN_ADDRESSin theupdate_configfunction within thehelpers.rsfile. The provided shell script output shows multiple instances ofADMIN_ADDRESSbeing used across different files in the project, specifically within thehelpers.rs,game_end.rs, andfixtures.rsfiles.From the output, we can observe the following:
ADMIN_ADDRESSis defined infixtures.rsas a constant string"admin_address".- It is used in
helpers.rsin theupdate_configfunction to execute a contract, which matches the usage pattern expected in the context of the review comment.- The usage of
ADMIN_ADDRESSis consistent across other test functions and files, indicating it is a commonly used fixture for administrative actions within tests.Given this information, the usage of
ADMIN_ADDRESSin theupdate_configfunction appears to be consistent with its definition and usage in other parts of the test suite. There are no indications of incorrect or inconsistent usage based on the script output.Therefore, the review comment concerning the verification of
ADMIN_ADDRESSusage inupdate_configcan be considered verified.The usage of `ADMIN_ADDRESS` in the `update_config` function is consistent with its definition and usage across the test suite.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `ADMIN_ADDRESS` is correctly defined and used. # Test: Search for the definition and usage of `ADMIN_ADDRESS`. Expect: Consistent and correct usage. rg --type rust 'ADMIN_ADDRESS' --context 5Length of output: 12535
23-35: Ensure theallocate_tokensfunction handles errors gracefully.Verification successful
The
allocate_tokensfunction in the codebase appears to handle errors gracefully by returning aResulttype in both its main implementation and its test helpers. This is a standard and effective way to manage errors in Rust, allowing them to be propagated and handled appropriately. Here are the specific implementations:
- In
contracts/prudent-pots/src/execute.rs, the function returnsResult<Response, ContractError>.- In
contracts/prudent-pots/src/tests/integration/helpers.rs, the function returnsResult<AppResponse, AnyError>.Both instances indicate proper error handling mechanisms are in place.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `allocate_tokens` function has proper error handling. # Test: Search for error handling in the `allocate_tokens` function. Expect: Error handling code. rg --type rust 'allocate_tokens' --context 5Length of output: 21572
contracts/prudent-pots/src/tests/integration/allocate_tokens.rs (1)
47-75: Ensure theallocate_tokensfunction handles insufficient funds correctly.The test correctly checks for the
BidOutOfRangeerror when insufficient funds are sent. Good use of detailed assertions to validate the error type and expected values.contracts/prudent-pots/src/tests/integration/game_end.rs (1)
22-107: Ensure thegame_endfunction handles complex scenarios correctly.Verification successful
The review comment suggests verifying the handling of complex scenarios by the
game_endfunction, particularly focusing on error handling and logic. The provided script output shows various parts of the code wheregame_endis implemented or referenced, including error handling and validation checks.Analysis of
game_endFunctionality Based on Script Output:
Error Handling and Validation:
- The function
validate_game_end_timeis used withingame_endto ensure that the game can only end after the designated time has passed. If the game is still active, it throws aGameStillActiveerror.- The function
validate_is_contract_adminchecks if the caller is the contract administrator, adding a layer of access control.Complex Logic Handling:
- The function calculates the total losing tokens and determines the winning pots.
- It processes the raffle winner and prepares messages for token distribution.
- It handles the transition to the next game round by preparing the game state and potentially resetting allocations.
Integration with Other Components:
- The
game_endfunction interacts with multiple helpers likeget_winning_pots,calculate_total_losing_tokens, andprocess_raffle_winnerto manage different aspects of the game logic.- It uses sub-messages for operations like sending tokens, which indicates integration with the broader contract ecosystem.
Conclusion:
The
game_endfunction incorporates detailed error handling, validations, and complex logic to manage the end of a game round, including raffle processing and preparation for the next round. The function's integration with various helpers and its use of contract features like sub-messages further affirm its robust handling of complex scenarios.Based on the analysis, the review comment is consistent with the codebase, confirming that the
game_endfunction is designed to handle complex scenarios effectively.Final Response:
The
game_endfunction is well-equipped with error handling, validations, and complex logic to manage game endings and transitions effectively. It integrates multiple components to ensure robust game management, aligning with the expectations set in the review comment.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `game_end` function handles complex scenarios with proper error handling and logic. # Test: Search for error handling and complex logic in the `game_end` function. Expect: Detailed error handling and logic handling code. rg --type rust 'game_end' --context 10Length of output: 49097
| let res = update_config( | ||
| &mut app, | ||
| &pp_addr, | ||
| &ExecuteMsg::UpdateConfig { | ||
| fee: Some(10), | ||
| fee_reallocation: Some(10), | ||
| fee_address: Some(Addr::unchecked("new_address")), | ||
| game_denom: Some("new_denom".to_string()), | ||
| game_cw721: Some(Addr::unchecked("test")), | ||
| game_duration: Some(GAME_DURATION * 2), | ||
| game_extend: Some(600 * 3), | ||
| min_pot_initial_allocation: Some(Uint128::new(1_000_000u128)), | ||
| decay_factor: Some(Uint128::new(50u128)), | ||
| }, | ||
| ) | ||
| .unwrap(); | ||
| assert!(!res.events.is_empty()); |
There was a problem hiding this comment.
Tip
Codebase Verification
The review comment suggests verifying whether the update_config function in the Rust codebase handles errors gracefully. The provided shell script output shows various instances of the update_config function being called and defined, but crucially, it does not explicitly show any error handling mechanisms directly within the update_config function itself.
From the output:
- The
update_configfunction inexecute.rsdoes not show any explicit error handling within the function's body. It performs some operations and returns aResponseobject, but there's no indication of error catching or handling. - The usage of
update_configin other parts of the code (like incontract.rsand test files) also does not show any error handling related to the function call itself.
Given this information, it appears that the update_config function might not have explicit error handling within its implementation. However, error handling could potentially be managed by the calling functions or through the broader system's error management strategy.
Since the script output does not provide a complete view of how errors might be handled or propagated when update_config fails (e.g., due to database issues, invalid parameters, etc.), further investigation into how errors are managed in the broader context of the application might be necessary.
Conclusion
The verification suggests that the update_config function itself does not include explicit error handling within its body. It is recommended to review the broader error management strategy of the application to ensure robust error handling around configuration updates.
- Review the broader error management strategy in the application.
- Consider implementing explicit error handling in the
update_configfunction if it aligns with the overall error strategy.
Analysis chain
Ensure the update_config function handles errors gracefully.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the `update_config` function has proper error handling.
# Test: Search for error handling in the `update_config` function. Expect: Error handling code.
rg --type rust 'update_config' --context 5
Length of output: 8815
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (2)
contracts/prudent-pots/src/execute.rs (2)
Line range hint
92-115: Consider adding comments to explain the logic behind dynamic bid constraints for better maintainability.
159-172: Consider adding comments to explain the impact of fees on the reallocation process for better clarity.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- contracts/prudent-pots/src/execute.rs (5 hunks)
- contracts/prudent-pots/src/state.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/prudent-pots/src/state.rs
Additional comments not posted (4)
contracts/prudent-pots/src/execute.rs (4)
37-74: LGTM! Good use of optional parameters and proper admin validation.
Line range hint
84-122: LGTM! Proper validations and updates are performed.
136-177: LGTM! Correct handling of token reallocation with appropriate validations and updates.
186-253: LGTM! Well-structured handling of game end operations.
| //attr("treasury_outgoing_tokens", todo!()), | ||
| //attr("winner_outgoing_tokens", todo!()), | ||
| //attr("raffle_outgoing_nft", todo!()), | ||
| //attr("raffle_outgoing_tokens", todo!()), | ||
| //attr("next_round_tokens", todo!()), |
There was a problem hiding this comment.
Complete the implementation of attributes using actual values instead of placeholders.
There was a problem hiding this comment.
Actionable comments posted: 18
Out of diff range and nitpick comments (2)
contracts/prudent-pots/src/execute.rs (1)
Line range hint
102-130: Review the implementation ofallocate_tokensfor correctness and efficiency.Review the
allocate_tokensfunction to ensure it is implemented correctly and efficiently, particularly in how it handles dynamic bid constraints and updates player allocations.contracts/prudent-pots/src/tests/integration/game_end.rs (1)
22-107: Consider adding more detailed comments explaining the logic behind each major step or calculation.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- contracts/prudent-pots/src/contract.rs (3 hunks)
- contracts/prudent-pots/src/error.rs (2 hunks)
- contracts/prudent-pots/src/execute.rs (5 hunks)
- contracts/prudent-pots/src/helpers/game_end.rs (1 hunks)
- contracts/prudent-pots/src/helpers/pot.rs (1 hunks)
- contracts/prudent-pots/src/msg.rs (1 hunks)
- contracts/prudent-pots/src/query.rs (2 hunks)
- contracts/prudent-pots/src/state.rs (1 hunks)
- contracts/prudent-pots/src/tests/instantiate.rs (4 hunks)
- contracts/prudent-pots/src/tests/integration/fixtures.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/game_end.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/helpers.rs (1 hunks)
- contracts/prudent-pots/src/tests/integration/update_config.rs (1 hunks)
- contracts/prudent-pots/src/tests/prepare_next_game.rs (3 hunks)
Files skipped from review as they are similar to previous changes (8)
- contracts/prudent-pots/src/contract.rs
- contracts/prudent-pots/src/error.rs
- contracts/prudent-pots/src/helpers/pot.rs
- contracts/prudent-pots/src/tests/instantiate.rs
- contracts/prudent-pots/src/tests/integration/fixtures.rs
- contracts/prudent-pots/src/tests/integration/helpers.rs
- contracts/prudent-pots/src/tests/integration/update_config.rs
- contracts/prudent-pots/src/tests/prepare_next_game.rs
Additional comments not posted (9)
contracts/prudent-pots/src/state.rs (3)
25-29: Check the consistency ofGameStatestruct fields with the game logic.Ensure that the fields in the
GameStatestruct are consistent with the game logic, particularly the handling ofextend_countand its impact on game dynamics.Verification successful
The usage of the
extend_countfield in theGameStatestruct appears to be consistent with the game logic. It is used to influence game dynamics, such as applying decay factors and modifying reward distributions, and is properly handled and tested across various parts of the codebase. Here are the key points:
- Validation:
extend_countis safely incremented invalidate.rs.- Game Dynamics: It influences decay factors in
game_end.rs.- Testing: Its impact and reset are tested in
integration/game_end.rs.Based on this analysis, the fields in the
GameStatestruct, particularlyextend_count, are consistent with the game logic as expected.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `GameState` fields in game logic. # Test: Search for the usage of `extend_count`. Expect: Proper handling in game logic. rg --type rust $'extend_count'Length of output: 1522
40-40: EnsurePlayerAllocationsstruct is used effectively across the contract.Verify that the
PlayerAllocationsstruct is being used effectively and consistently across the contract, particularly in functions that handle player data and token allocations.Verification successful
The
PlayerAllocationsstruct is effectively and consistently used across the contract. It is defined in the state module, utilized in query and message handling, and is also present in tests, which suggests thorough testing and integration into the contract's functionality. Based on this, the review comment is verified as accurate.
- State Module: The struct is defined and serves as the basis for player data handling.
- Query and Contract Modules: It is used to handle queries related to player allocations, indicating its integration into the operational logic of the contract.
- Tests: The presence in test scripts ensures that its functionality is verified under various scenarios.
- Message Handling: It is used in defining messages related to player allocations, which is crucial for operations on player data.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the usage of `PlayerAllocations` struct across the contract. # Test: Search for the usage of `PlayerAllocations`. Expect: Consistent and effective use. rg --type rust $'PlayerAllocations'Length of output: 1315
45-49: Confirm the role and usage ofFirstBidderstruct in the bidding process.Confirm the role and usage of the
FirstBidderstruct within the bidding process, and ensure it aligns with the intended game mechanics.Verification successful
The output from the executed script provides several references to the
FirstBidderstruct within the codebase, particularly in thecontracts/prudent-pots/src/helpers/pot.rsfile. Here's a breakdown of the findings:
Definition and Storage:
FirstBidderis defined incontracts/prudent-pots/src/state.rsand is stored in aMapdata structure, which suggests that instances of this struct are saved and likely retrieved during the game's execution.Usage in Game Mechanics: The comments and function signatures in
contracts/prudent-pots/src/helpers/pot.rsindicate thatFirstBidderis used to handle situations where there might be a tie in raffle winner selection. The struct is used to record the first bidder for a pot during a round, which includes the bidder's identifier and the timestamp of the bid.Functionality: The functions in
contracts/prudent-pots/src/helpers/pot.rscheck for the existence of aFirstBidderfor a given pot and set it if it does not already exist. This functionality is crucial for determining the outcome in case of a tie, ensuring that the first bidder has a recorded advantage.From this analysis, it is evident that the
FirstBidderstruct plays a significant role in the bidding process, specifically in managing tie situations in raffle draws. This aligns with the intended game mechanics as described in the review comment.Based on the information gathered, the usage of
FirstBidderwithin the bidding process is confirmed to align with the game mechanics as intended.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Confirm the usage of `FirstBidder` struct in the bidding process. # Test: Search for the usage of `FirstBidder`. Expect: Alignment with game mechanics. rg --type rust $'FirstBidder'Length of output: 898
contracts/prudent-pots/src/msg.rs (1)
53-77: Ensure new query messages are implemented correctly and integrated with the contract's logic.Verify that all new query messages are implemented correctly and integrated with the contract's logic, particularly those that handle complex queries like
RaffleDenomSplit.Verification successful
The implementation and integration of the
RaffleDenomSplitquery message appear to be correctly handled within the contract's logic. The function is defined, connected to the corresponding query message, and covered by integration tests, which suggests thorough implementation. Here are the specific points of integration:
- Definition and Response Structure: Defined in
msg.rsand has a corresponding response structure.- Function Implementation: Implemented in
query.rswherequery_raffle_denom_splitfunction returns aRaffleDenomSplitResponse.- Contract Integration: Integrated in
contract.rswhere the query message triggers the corresponding function.- Testing: Covered by integration tests in
game_end.rs, indicating that the functionality is tested.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of new query messages. # Test: Search for the implementation of `RaffleDenomSplit`. Expect: Correct implementation and integration. rg --type rust $'RaffleDenomSplit'Length of output: 1064
contracts/prudent-pots/src/query.rs (4)
18-20: Validate the implementation ofquery_game_configfunction.Ensure that the
query_game_configfunction is implemented correctly and returns the expected results based on the game configuration.Verification successful
The implementation of the
query_game_configfunction has been verified successfully. It correctly loads the game configuration from storage and returns it wrapped in aGameConfigResponse, which is the expected behavior. The function is also appropriately used in the contract to handleQueryMsg::GameConfig {}.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Validate the implementation of `query_game_config`. # Test: Search for the usage of `query_game_config`. Expect: Correct implementation and expected results. rg --type rust $'query_game_config'Length of output: 387
36-38: Ensure correct implementation ofquery_pot_statefunction.Verify that the
query_pot_statefunction is implemented correctly and accurately retrieves the state of a specified pot.Verification successful
The
query_pot_statefunction is implemented correctly in thecontracts/prudent-pots/src/query.rsfile and is accurately used in thecontracts/prudent-pots/src/contract.rsfile to handle theQueryMsg::PotStatemessage. This confirms that the function retrieves the state of a specified pot as intended.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure correct implementation of `query_pot_state`. # Test: Search for the usage of `query_pot_state`. Expect: Correct implementation and accurate retrieval. rg --type rust $'query_pot_state'Length of output: 395
53-56: Check the implementation ofquery_winning_potsfunction for correctness.Check that the
query_winning_potsfunction is implemented correctly and returns the expected results based on the winning pots.Verification successful
The review comment asks to verify the correctness of the
query_winning_potsfunction in thecontracts/prudent-pots/src/query.rsfile. The function is expected to return the correct results based on the winning pots.Based on the provided code and usage context, the
query_winning_potsfunction appears to be implemented correctly. It fetches the winning pots usingget_winning_potsand handles potential errors withunwrap_or_default(). This function is also appropriately used incontract.rsto respond to a specific query message, which aligns with typical usage patterns in smart contract development.
- The function is defined and used within
contracts/prudent-pots/src/query.rs.- It is also referenced in
contracts/prudent-pots/src/contract.rsto handle a query message.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of `query_winning_pots`. # Test: Search for the usage of `query_winning_pots`. Expect: Correct implementation and expected results. rg --type rust $'query_winning_pots'Length of output: 489
71-89: Review the implementation ofquery_reallocation_fee_poolandquery_rafflefunctions for accuracy.Review the implementation of the
query_reallocation_fee_poolandquery_rafflefunctions to ensure they accurately reflect the state of the reallocation fee pool and the raffle, respectively.contracts/prudent-pots/src/tests/integration/game_end.rs (1)
155-428: Verify the correctness of the reallocation logic and ensure it aligns with the intended contract rules.Verification successful
The search results from the
rgcommand indicate that thereallocate_tokensfunction is indeed used within the codebase, specifically in thecontracts/prudent-pots/src/execute.rsandcontracts/prudent-pots/src/tests/integration/game_end.rsfiles among others. This confirms that the function is part of the contract's execution logic and is also utilized in integration tests, which is expected for a function that handles token reallocation logic in a blockchain contract scenario.Given this information, the usage of
reallocate_tokensin the test functiontest_game_end_one_winner_raffle_both_worksappears to be consistent with its intended purpose in the contract. The function is being tested in a scenario that involves reallocating tokens, which aligns with the contract rules regarding token management during game state changes.Since the function is correctly implemented and used according to the contract rules, and the test scenario in
game_end.rsis designed to verify this logic under game-ending conditions, the review comment about verifying the correctness of the reallocation logic and ensuring it aligns with the intended contract rules is consistent with the findings.Therefore, the review comment is accurate and the implementation in the test aligns with the intended behavior of the contract.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the reallocation logic in the contract. # Test: Search for the reallocation logic usage. Expect: Correct implementation according to contract rules. rg --type rust $'reallocate_tokens'Length of output: 1186
| #[test] | ||
| fn test_game_end_one_winner_simple_works() { | ||
| let (mut app, pp_addr, _cw721_addr) = | ||
| default_with_balances(5, vec![coin(100_000_000u128, DENOM_GAME.to_string())], None); | ||
|
|
||
| // Game state extend_count after | ||
| let game_state: GameStateResponse = app | ||
| .wrap() | ||
| .query_wasm_smart(&pp_addr, &QueryMsg::GameState {}) | ||
| .unwrap(); | ||
| assert_eq!(game_state.state.extend_count, 0); | ||
| assert_eq!(game_state.state.round_count, 1); // this is a side effect of raffle which can be set only on 2nd and next rounds | ||
|
|
||
| // Allocate tokens with 5 users, starting from a minBid of 1 $DENOM (so also 1 $DENOM per pot) | ||
| let info_1 = mock_info("user1", &coins(2_000_000, DENOM_GAME)); | ||
| allocate_tokens(&mut app, &pp_addr, &info_1, 1).unwrap(); // tot. 7.0, minBid is now 1.4 - Pot 1 has 3.0 tokens (L) | ||
| let info_2 = mock_info("user2", &coins(1_400_001, DENOM_GAME)); | ||
| allocate_tokens(&mut app, &pp_addr, &info_2, 2).unwrap(); // tot. 8,400001, minBid is now 1_680_000(2) - Pot 2 has 2.400001 (L) | ||
| let info_3 = mock_info("user3", &coins(1_680_000, DENOM_GAME)); | ||
| allocate_tokens(&mut app, &pp_addr, &info_3, 3).unwrap(); // tot. 10,080001, minBid is now 2,016000(2) - Pot 3 has 2,68 (L) | ||
| let info_4 = mock_info("user4", &coins(2_016_000, DENOM_GAME)); | ||
| allocate_tokens(&mut app, &pp_addr, &info_4, 4).unwrap(); // tot. 12,096001, minBid is now 2,419200(24) - Pot 4 has 3,016 (L) | ||
| let info_5 = mock_info("user5", &coins(2_419_200, DENOM_GAME)); | ||
| allocate_tokens(&mut app, &pp_addr, &info_5, 5).unwrap(); // tot. 14,515201, minBid is now 2,903040(2) - Pot 5 has 3.4192 (W) | ||
|
|
||
| // Total tokens: 14,515201 / Winning 3.4192 / Losing 11,096001.div(2) === 5,548000(5) | ||
| let total_tokens = Uint128::new(5_000_000) // raffle is adding 5_000_000 to the contract balance | ||
| + info_1.funds[0].amount | ||
| + info_2.funds[0].amount | ||
| + info_3.funds[0].amount | ||
| + info_4.funds[0].amount | ||
| + info_5.funds[0].amount; | ||
| assert_eq!(total_tokens.u128(), 14_515_201u128); | ||
|
|
||
| // Winning pot 5 has initial balance (1) + user 5 allocation | ||
| let winning_tokens = 1_000_000 + info_5.funds[0].amount.u128(); | ||
| assert_eq!(winning_tokens, 3_419_200u128); | ||
|
|
||
| // Losing pots 1, 2, 3, 4 have initial balance (4) + losers allocation | ||
| let losing_tokens = total_tokens.u128() - winning_tokens; | ||
| assert_eq!(losing_tokens, 11_096_001u128); | ||
|
|
||
| // we only pay 50% of the losing tokens, the other 50% goes to next round | ||
| let winning_pots_get = losing_tokens / 2; | ||
| assert_eq!(winning_pots_get, 5_548_000u128); | ||
|
|
||
| // user 5 should receive 95% of the winning tokens + (losing tokens / 2) | ||
| let winner_gets = (winning_tokens + winning_pots_get) * 95 / 100; | ||
| assert_eq!(winner_gets, 8_518_840u128); | ||
|
|
||
| // Get user balance after allocating funds, before game_end | ||
| let user5_balance_before = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | ||
|
|
||
| // Assert get_winning_pots only returns pot 5 | ||
| let winning_pots: WinningPotsResponse = app | ||
| .wrap() | ||
| .query_wasm_smart(&pp_addr, &QueryMsg::WinningPots {}) | ||
| .unwrap(); | ||
| assert_eq!(winning_pots.pots.len(), 1); // only one winner | ||
| assert_eq!(winning_pots.pots[0], 5); // pot id 5 | ||
|
|
||
| // Increase time by GAME_DURATION + 1 second to make the game expire | ||
| increase_app_time(&mut app, GAME_DURATION + 1); | ||
|
|
||
| // Game end and new raffles | ||
| let info = mock_info(ADMIN_ADDRESS, &vec![]); | ||
| game_end(&mut app, &pp_addr, &info, None, None).unwrap(); | ||
|
|
||
| // Get user balance after game_end | ||
| let user5_balance_after = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | ||
| assert_eq!( | ||
| user5_balance_after.amount.u128(), | ||
| user5_balance_before.amount.u128() + 8_518_840u128 | ||
| ); | ||
|
|
||
| // Query pots_state and sum all of them | ||
| let pots_state: PotsStateResponse = app | ||
| .wrap() | ||
| .query_wasm_smart(&pp_addr, &QueryMsg::PotsState {}) | ||
| .unwrap(); | ||
| let pots_state_total_amount: Uint128 = pots_state.pots.iter().map(|pot| pot.amount).sum(); | ||
| // Query contract balance and compare with summed pots_state after new round is started | ||
| let contract_balance = app.wrap().query_balance(&pp_addr, DENOM_GAME).unwrap(); | ||
| // Assert contract balance is consistent with new pot_state | ||
| assert_approx_eq!(contract_balance.amount, pots_state_total_amount, "0.000001"); | ||
| } |
There was a problem hiding this comment.
Ensure consistent use of Uint128 for token calculations to prevent overflow errors.
- let total_tokens = Uint128::new(5_000_000) // raffle is adding 5_000_000 to the contract balance
+ let total_tokens = Uint128::new(5_000_000u128) // raffle is adding 5_000_000 to the contract balanceCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| #[test] | |
| fn test_game_end_one_winner_simple_works() { | |
| let (mut app, pp_addr, _cw721_addr) = | |
| default_with_balances(5, vec![coin(100_000_000u128, DENOM_GAME.to_string())], None); | |
| // Game state extend_count after | |
| let game_state: GameStateResponse = app | |
| .wrap() | |
| .query_wasm_smart(&pp_addr, &QueryMsg::GameState {}) | |
| .unwrap(); | |
| assert_eq!(game_state.state.extend_count, 0); | |
| assert_eq!(game_state.state.round_count, 1); // this is a side effect of raffle which can be set only on 2nd and next rounds | |
| // Allocate tokens with 5 users, starting from a minBid of 1 $DENOM (so also 1 $DENOM per pot) | |
| let info_1 = mock_info("user1", &coins(2_000_000, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_1, 1).unwrap(); // tot. 7.0, minBid is now 1.4 - Pot 1 has 3.0 tokens (L) | |
| let info_2 = mock_info("user2", &coins(1_400_001, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_2, 2).unwrap(); // tot. 8,400001, minBid is now 1_680_000(2) - Pot 2 has 2.400001 (L) | |
| let info_3 = mock_info("user3", &coins(1_680_000, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_3, 3).unwrap(); // tot. 10,080001, minBid is now 2,016000(2) - Pot 3 has 2,68 (L) | |
| let info_4 = mock_info("user4", &coins(2_016_000, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_4, 4).unwrap(); // tot. 12,096001, minBid is now 2,419200(24) - Pot 4 has 3,016 (L) | |
| let info_5 = mock_info("user5", &coins(2_419_200, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_5, 5).unwrap(); // tot. 14,515201, minBid is now 2,903040(2) - Pot 5 has 3.4192 (W) | |
| // Total tokens: 14,515201 / Winning 3.4192 / Losing 11,096001.div(2) === 5,548000(5) | |
| let total_tokens = Uint128::new(5_000_000) // raffle is adding 5_000_000 to the contract balance | |
| + info_1.funds[0].amount | |
| + info_2.funds[0].amount | |
| + info_3.funds[0].amount | |
| + info_4.funds[0].amount | |
| + info_5.funds[0].amount; | |
| assert_eq!(total_tokens.u128(), 14_515_201u128); | |
| // Winning pot 5 has initial balance (1) + user 5 allocation | |
| let winning_tokens = 1_000_000 + info_5.funds[0].amount.u128(); | |
| assert_eq!(winning_tokens, 3_419_200u128); | |
| // Losing pots 1, 2, 3, 4 have initial balance (4) + losers allocation | |
| let losing_tokens = total_tokens.u128() - winning_tokens; | |
| assert_eq!(losing_tokens, 11_096_001u128); | |
| // we only pay 50% of the losing tokens, the other 50% goes to next round | |
| let winning_pots_get = losing_tokens / 2; | |
| assert_eq!(winning_pots_get, 5_548_000u128); | |
| // user 5 should receive 95% of the winning tokens + (losing tokens / 2) | |
| let winner_gets = (winning_tokens + winning_pots_get) * 95 / 100; | |
| assert_eq!(winner_gets, 8_518_840u128); | |
| // Get user balance after allocating funds, before game_end | |
| let user5_balance_before = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | |
| // Assert get_winning_pots only returns pot 5 | |
| let winning_pots: WinningPotsResponse = app | |
| .wrap() | |
| .query_wasm_smart(&pp_addr, &QueryMsg::WinningPots {}) | |
| .unwrap(); | |
| assert_eq!(winning_pots.pots.len(), 1); // only one winner | |
| assert_eq!(winning_pots.pots[0], 5); // pot id 5 | |
| // Increase time by GAME_DURATION + 1 second to make the game expire | |
| increase_app_time(&mut app, GAME_DURATION + 1); | |
| // Game end and new raffles | |
| let info = mock_info(ADMIN_ADDRESS, &vec![]); | |
| game_end(&mut app, &pp_addr, &info, None, None).unwrap(); | |
| // Get user balance after game_end | |
| let user5_balance_after = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | |
| assert_eq!( | |
| user5_balance_after.amount.u128(), | |
| user5_balance_before.amount.u128() + 8_518_840u128 | |
| ); | |
| // Query pots_state and sum all of them | |
| let pots_state: PotsStateResponse = app | |
| .wrap() | |
| .query_wasm_smart(&pp_addr, &QueryMsg::PotsState {}) | |
| .unwrap(); | |
| let pots_state_total_amount: Uint128 = pots_state.pots.iter().map(|pot| pot.amount).sum(); | |
| // Query contract balance and compare with summed pots_state after new round is started | |
| let contract_balance = app.wrap().query_balance(&pp_addr, DENOM_GAME).unwrap(); | |
| // Assert contract balance is consistent with new pot_state | |
| assert_approx_eq!(contract_balance.amount, pots_state_total_amount, "0.000001"); | |
| } | |
| #[test] | |
| fn test_game_end_one_winner_simple_works() { | |
| let (mut app, pp_addr, _cw721_addr) = | |
| default_with_balances(5, vec![coin(100_000_000u128, DENOM_GAME.to_string())], None); | |
| // Game state extend_count after | |
| let game_state: GameStateResponse = app | |
| .wrap() | |
| .query_wasm_smart(&pp_addr, &QueryMsg::GameState {}) | |
| .unwrap(); | |
| assert_eq!(game_state.state.extend_count, 0); | |
| assert_eq!(game_state.state.round_count, 1); // this is a side effect of raffle which can be set only on 2nd and next rounds | |
| // Allocate tokens with 5 users, starting from a minBid of 1 $DENOM (so also 1 $DENOM per pot) | |
| let info_1 = mock_info("user1", &coins(2_000_000, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_1, 1).unwrap(); // tot. 7.0, minBid is now 1.4 - Pot 1 has 3.0 tokens (L) | |
| let info_2 = mock_info("user2", &coins(1_400_001, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_2, 2).unwrap(); // tot. 8,400001, minBid is now 1_680_000(2) - Pot 2 has 2.400001 (L) | |
| let info_3 = mock_info("user3", &coins(1_680_000, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_3, 3).unwrap(); // tot. 10,080001, minBid is now 2,016000(2) - Pot 3 has 2,68 (L) | |
| let info_4 = mock_info("user4", &coins(2_016_000, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_4, 4).unwrap(); // tot. 12,096001, minBid is now 2,419200(24) - Pot 4 has 3,016 (L) | |
| let info_5 = mock_info("user5", &coins(2_419_200, DENOM_GAME)); | |
| allocate_tokens(&mut app, &pp_addr, &info_5, 5).unwrap(); // tot. 14,515201, minBid is now 2,903040(2) - Pot 5 has 3.4192 (W) | |
| // Total tokens: 14,515201 / Winning 3.4192 / Losing 11,096001.div(2) === 5,548000(5) | |
| let total_tokens = Uint128::new(5_000_000u128) // raffle is adding 5_000_000 to the contract balance | |
| + info_1.funds[0].amount | |
| + info_2.funds[0].amount | |
| + info_3.funds[0].amount | |
| + info_4.funds[0].amount | |
| + info_5.funds[0].amount; | |
| assert_eq!(total_tokens.u128(), 14_515_201u128); | |
| // Winning pot 5 has initial balance (1) + user 5 allocation | |
| let winning_tokens = 1_000_000 + info_5.funds[0].amount.u128(); | |
| assert_eq!(winning_tokens, 3_419_200u128); | |
| // Losing pots 1, 2, 3, 4 have initial balance (4) + losers allocation | |
| let losing_tokens = total_tokens.u128() - winning_tokens; | |
| assert_eq!(losing_tokens, 11_096_001u128); | |
| // we only pay 50% of the losing tokens, the other 50% goes to next round | |
| let winning_pots_get = losing_tokens / 2; | |
| assert_eq!(winning_pots_get, 5_548_000u128); | |
| // user 5 should receive 95% of the winning tokens + (losing tokens / 2) | |
| let winner_gets = (winning_tokens + winning_pots_get) * 95 / 100; | |
| assert_eq!(winner_gets, 8_518_840u128); | |
| // Get user balance after allocating funds, before game_end | |
| let user5_balance_before = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | |
| // Assert get_winning_pots only returns pot 5 | |
| let winning_pots: WinningPotsResponse = app | |
| .wrap() | |
| .query_wasm_smart(&pp_addr, &QueryMsg::WinningPots {}) | |
| .unwrap(); | |
| assert_eq!(winning_pots.pots.len(), 1); // only one winner | |
| assert_eq!(winning_pots.pots[0], 5); // pot id 5 | |
| // Increase time by GAME_DURATION + 1 second to make the game expire | |
| increase_app_time(&mut app, GAME_DURATION + 1); | |
| // Game end and new raffles | |
| let info = mock_info(ADMIN_ADDRESS, &vec![]); | |
| game_end(&mut app, &pp_addr, &info, None, None).unwrap(); | |
| // Get user balance after game_end | |
| let user5_balance_after = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | |
| assert_eq!( | |
| user5_balance_after.amount.u128(), | |
| user5_balance_before.amount.u128() + 8_518_840u128 | |
| ); | |
| // Query pots_state and sum all of them | |
| let pots_state: PotsStateResponse = app | |
| .wrap() | |
| .query_wasm_smart(&pp_addr, &QueryMsg::PotsState {}) | |
| .unwrap(); | |
| let pots_state_total_amount: Uint128 = pots_state.pots.iter().map(|pot| pot.amount).sum(); | |
| // Query contract balance and compare with summed pots_state after new round is started | |
| let contract_balance = app.wrap().query_balance(&pp_addr, DENOM_GAME).unwrap(); | |
| // Assert contract balance is consistent with new pot_state | |
| assert_approx_eq!(contract_balance.amount, pots_state_total_amount, "0.000001"); | |
| } |
| .unwrap(); | ||
|
|
||
| // Game state extend_count after | ||
| let game_state: GameStateResponse = app | ||
| .wrap() | ||
| .query_wasm_smart(&pp_addr, &QueryMsg::GameState {}) | ||
| .unwrap(); | ||
| assert_eq!(game_state.state.extend_count, 0); | ||
| assert_eq!(game_state.state.round_count, 3); | ||
|
|
||
| // Get user balance after game_end | ||
| let user5_balance_after = app.wrap().query_balance("user5", DENOM_GAME).unwrap(); | ||
| assert_eq!( | ||
| (user5_balance_after.amount - user5_balance_before.amount).u128(), | ||
| (1_000_000+info_5.funds[0].amount.u128()) | ||
| - 192_500u128 // fee paid over winning amount to treasury | ||
| - reallocation_fee_pool.reallocation_fee_pool.u128() // fee paid on reallocating to pot 2 over player's funds | ||
| + 5_750_000u128 // losing amount / 2 | ||
| - 287_500u128 // fee on losing amount / 2 | ||
| + raffle_denom_split.prize_to_distribute.u128() | ||
| ); | ||
|
|
||
| // Assert NFT balance for user5, it should have the NFT #1 | ||
| let raffle_winner_nft_balance: TokensResponse = app | ||
| .wrap() | ||
| .query_wasm_smart( | ||
| &cw721_addr, | ||
| &Cw721QueryMsg::Tokens { | ||
| owner: "user5".to_string(), | ||
| start_after: None, | ||
| limit: None, | ||
| }, | ||
| ) | ||
| .unwrap(); | ||
| assert_eq!(raffle_winner_nft_balance.tokens.len(), 1); // check length 1 | ||
| assert_eq!(raffle_winner_nft_balance.tokens[0], "1"); | ||
|
|
||
| // Assert NFT balance for contract pp_addr, it should have the NFT #2 | ||
| let contract_new_nft_balance: TokensResponse = app | ||
| .wrap() | ||
| .query_wasm_smart( | ||
| &cw721_addr, | ||
| &Cw721QueryMsg::Tokens { | ||
| owner: pp_addr.to_string(), | ||
| start_after: None, | ||
| limit: None, | ||
| }, | ||
| ) | ||
| .unwrap(); | ||
| assert_eq!(contract_new_nft_balance.tokens.len(), 1); | ||
| assert_eq!(contract_new_nft_balance.tokens[0], "2"); | ||
|
|
||
| // Assert raffle state | ||
| let raffle: RaffleResponse = app | ||
| .wrap() | ||
| .query_wasm_smart(&pp_addr, &QueryMsg::Raffle {}) | ||
| .unwrap(); | ||
| assert_eq!( | ||
| raffle.raffle, | ||
| Raffle { | ||
| cw721_token_id: Some("2".to_string()), | ||
| cw721_addr: Some(cw721_addr.to_string()), | ||
| denom_amount: Uint128::new(200_000_000u128) | ||
| } | ||
| ); | ||
|
|
||
| // Query pots_state and sum all of them | ||
| let pots_state: PotsStateResponse = app | ||
| .wrap() | ||
| .query_wasm_smart(&pp_addr, &QueryMsg::PotsState {}) | ||
| .unwrap(); | ||
| let pots_state_total_amount: Uint128 = pots_state.pots.iter().map(|pot| pot.amount).sum(); | ||
|
|
||
| // Query contract balance and compare with summed pots_state after new round is started | ||
| let contract_balance = app.wrap().query_balance(&pp_addr, DENOM_GAME).unwrap(); | ||
| // Assert contract balance is consistent with new pot_state | ||
| assert_approx_eq!( | ||
| contract_balance | ||
| .amount | ||
| .checked_sub(Uint128::new(200_000_000u128)) | ||
| .unwrap(), | ||
| pots_state_total_amount, | ||
| "0.000001" | ||
| ); | ||
|
|
||
| // Assert treasury balance: sources are (fee, realloc_fee, raffle_denom_prize_split) | ||
| let treasury_balance = app | ||
| .wrap() | ||
| .query_balance("treasury_addr", DENOM_GAME) | ||
| .unwrap(); | ||
|
|
||
| // TOOD: Fix this assert | ||
| assert_eq!( | ||
| treasury_balance.amount.u128(), | ||
| 150_000u128 // reallocation fee from player5 realloc from pot 1 to 2 | ||
| + 192_500u128 // fee on winning pot (init alloc + player 5 bet) | ||
| + 287_500u128 // fee on total losing / 2 | ||
| + 5_000_000u128 // from raffle split denom amount | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add unit tests for edge cases in the reallocation logic, such as reallocations that result in no change or minimal changes.
Would you like me to help by adding these tests or should I open a GitHub issue to track this task?
| // TODO: fn test_game_end_multiple_winners_raffle_both_works | ||
|
|
||
| // TODO: fn test_game_end_multiple_winners_raffle_both_tie_works |
There was a problem hiding this comment.
Implement the remaining test functions to ensure comprehensive coverage of all game end scenarios.
Would you like assistance in implementing these tests, or should I open a GitHub issue to track this task?
| #[cw_serde] | ||
| pub struct GameConfig { | ||
| pub fee: u64, | ||
| pub fee_address: Addr, | ||
| pub fee_reallocation: u64, | ||
| pub fee_address: Addr, | ||
| pub game_denom: String, | ||
| pub game_cw721_addrs: Vec<Addr>, // this is the cw721 collection addy we use as optional raffle prize | ||
| pub game_duration: u64, | ||
| pub game_extend: u64, | ||
| pub min_bid: Uint128, | ||
| pub min_pot_initial_allocation: Uint128, // i.e. 1000000 for 1 $OSMO | ||
| pub decay_factor: Uint128, // i.e. 95 as 95% | ||
| } |
There was a problem hiding this comment.
Ensure that all new fields in GameConfig are properly documented and validated.
Consider adding detailed comments for each field in the GameConfig struct to explain their purpose and usage within the system. Additionally, ensure that these new fields are validated wherever they are used to maintain data integrity.
| #[cw_serde] | ||
| pub struct Raffle { | ||
| pub cw721_token_id: Option<String>, // the tokenId of the raffle nft to be won | ||
| pub cw721_addr: Option<String>, // one of the whitelisted addys | ||
| pub denom_amount: Uint128, // this is limited to the same game_config.game_denom for now |
There was a problem hiding this comment.
Validate the optional fields in Raffle struct to handle cases where they are not provided.
The fields cw721_token_id and cw721_addr in the Raffle struct are optional. Ensure that there is logic to handle cases where these fields are not provided to avoid runtime errors.
| pub fn game_end( | ||
| deps: DepsMut, | ||
| env: Env, | ||
| info: MessageInfo, | ||
| new_raffle_cw721_id: Option<String>, | ||
| new_raffle_cw721_addr: Option<String>, | ||
| ) -> Result<Response, ContractError> { | ||
| validate_is_contract_admin(&deps.querier, &env, &info.sender)?; | ||
| validate_game_end_time(deps.storage, &env)?; | ||
|
|
||
| // Determine the winning pot(s) based on the unique rules for each pot | ||
| let mut winning_pots = Vec::new(); | ||
| for pot_id in 1..=5 { | ||
| if is_winning_pot(deps.storage, pot_id)? { | ||
| winning_pots.push(pot_id); | ||
| } | ||
| // Ensure both or neither options are provided | ||
| if new_raffle_cw721_id.is_some() != new_raffle_cw721_addr.is_some() { | ||
| return Err(ContractError::InvalidRaffleNft {}); | ||
| } | ||
|
|
||
| // Calculate the total amount in losing pots to be redistributed | ||
| // Determine the winning pots and calculate total losing tokens | ||
| let winning_pots = get_winning_pots(deps.storage)?; | ||
| let total_losing_tokens = calculate_total_losing_tokens(deps.storage, &winning_pots)?; | ||
|
|
||
| // Redistribute the tokens from losing to winning pots, and also winning pots amount with users' allocations based on contributors shares: | ||
| let bank_msgs = get_distribute_bank_msgs(deps.storage, &winning_pots, total_losing_tokens)?; | ||
| // Process raffle winner and prepare distribution messages | ||
| let ( | ||
| mut msgs, | ||
| raffle_submsgs, | ||
| new_raffle_denom_amount, | ||
| updated_new_raffle_cw721_id, | ||
| updated_new_raffle_cw721_addr, | ||
| ) = process_raffle_winner( | ||
| &deps.as_ref(), | ||
| &env, | ||
| &info.funds, | ||
| &winning_pots, | ||
| new_raffle_cw721_id, | ||
| new_raffle_cw721_addr, | ||
| )?; | ||
|
|
||
| // Prepare for the next game | ||
| prepare_next_game(deps, &env, &bank_msgs)?; | ||
| // Add messages for redistributing tokens from losing to winning pots | ||
| msgs.extend(get_distribution_send_msgs( | ||
| &deps.as_ref(), | ||
| &winning_pots, | ||
| total_losing_tokens, | ||
| )?); | ||
|
|
||
| // Construct the response with appropriate attributes | ||
| Ok(Response::new().add_messages(bank_msgs).add_attributes(vec![ | ||
| attr("method", "execute"), | ||
| attr("action", "game_end"), | ||
| attr("winning_pots", format!("{:?}", winning_pots)), | ||
| attr("total_losing_tokens", total_losing_tokens), | ||
| ])) | ||
| // Iterate again the msgs generated to know how much tokens effectively we send, | ||
| // as total_losing_tokens contains also next game funds we want to preserve. | ||
| let total_outgoing_tokens = msgs | ||
| .iter() | ||
| .filter_map(|msg| { | ||
| if let CosmosMsg::Bank(BankMsg::Send { amount, .. }) = msg { | ||
| Some(amount) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .flatten() | ||
| .map(|coin| coin.amount) | ||
| .sum(); | ||
|
|
||
| // Reset and prepare for the next game | ||
| let (old_round_count, _new_round_count) = prepare_next_game( | ||
| deps, | ||
| &env, | ||
| total_outgoing_tokens, | ||
| updated_new_raffle_cw721_id, | ||
| updated_new_raffle_cw721_addr, | ||
| Some(new_raffle_denom_amount), | ||
| )?; | ||
|
|
||
| Ok(Response::new() | ||
| .add_messages(msgs) | ||
| .add_submessages(raffle_submsgs) | ||
| .add_attributes(vec![ | ||
| attr("method", "execute"), | ||
| attr("action", "game_end"), | ||
| attr("round_count", old_round_count.to_string()), | ||
| attr("winning_pots", format!("{:?}", winning_pots)), | ||
| //attr("treasury_outgoing_tokens", todo!()), | ||
| //attr("winner_outgoing_tokens", todo!()), | ||
| //attr("raffle_outgoing_nft", todo!()), | ||
| //attr("raffle_outgoing_tokens", todo!()), | ||
| //attr("next_round_tokens", todo!()), | ||
| ])) |
There was a problem hiding this comment.
Validate the implementation of game_end function for correctness and completeness.
Ensure that the game_end function is implemented correctly and completely, particularly in how it handles the distribution of tokens and the preparation for the next game.
| // Helper to prepare for the next game | ||
| pub fn prepare_next_game( | ||
| deps: DepsMut, | ||
| env: &Env, | ||
| total_outgoing_tokens: Uint128, | ||
| raffle_cw721_token_id: Option<String>, | ||
| raffle_cw721_addr: Option<String>, | ||
| raffle_denom_amount: Option<Uint128>, | ||
| ) -> Result<(u64, u64), ContractError> { | ||
| let config = GAME_CONFIG.load(deps.storage)?; | ||
| let game_state = GAME_STATE.may_load(deps.storage)?.unwrap_or_default(); | ||
|
|
||
| // Start the next game 1 second in the future | ||
| let game_duration = config.game_duration; | ||
| let next_game_start = env.block.time.seconds() + 1; | ||
| let next_game_end = next_game_start + game_duration; | ||
|
|
||
| // Validate game start and end times | ||
| if next_game_start >= next_game_end { | ||
| return Err(ContractError::InvalidInput {}); | ||
| } | ||
|
|
||
| // Safely increment round_count | ||
| let new_round_count = game_state | ||
| .round_count | ||
| .checked_add(1) | ||
| .ok_or(ContractError::InvalidInput {})?; | ||
|
|
||
| // Update the game state with new values | ||
| let new_game_state = GameState { | ||
| start_time: next_game_start, | ||
| end_time: next_game_end, | ||
| round_count: new_round_count, | ||
| extend_count: 0, | ||
| }; | ||
| GAME_STATE.save(deps.storage, &new_game_state)?; | ||
|
|
||
| // Reset player allocations for the next game | ||
| PLAYER_ALLOCATIONS.clear(deps.storage); | ||
|
|
||
| // Start initial tokens allocation workflow by querying the contract balance | ||
| let net_contract_balance = deps | ||
| .querier | ||
| .query_balance(&env.contract.address, &config.game_denom)? | ||
| .amount | ||
| .checked_sub(total_outgoing_tokens)? // Subtract outgoing tokens from the total tokens | ||
| .checked_sub(raffle_denom_amount.unwrap_or_default())?; // Subtract the new amount sent in this tx as info.funds reserved for next round denom raffle prize | ||
|
|
||
| // Calculate the initial tokens for each pot after subtracting outgoing tokens and next round game_denom raffle prize amount. | ||
| let initial_tokens_per_pot = net_contract_balance.checked_div(Uint128::from(5u128))?; // Divide the result by 5 to get tokens per pot | ||
|
|
||
| // This will make the restart fail till someone funds the contract balance via bank send | ||
| validate_pot_initial_amount(&config.min_pot_initial_allocation, &net_contract_balance)?; | ||
|
|
||
| // Distribute the initial tokens to the pots for the next game | ||
| for pot_id in 1..=5 { | ||
| POT_STATES.save( | ||
| deps.storage, | ||
| pot_id, | ||
| &TokenAllocation { | ||
| pot_id, | ||
| amount: initial_tokens_per_pot, | ||
| }, | ||
| )?; | ||
| } | ||
|
|
||
| // Save new raffle prizes for next game | ||
| RAFFLE.save( | ||
| deps.storage, | ||
| &Raffle { | ||
| cw721_token_id: raffle_cw721_token_id, | ||
| cw721_addr: raffle_cw721_addr, | ||
| denom_amount: raffle_denom_amount.unwrap_or_default(), | ||
| }, | ||
| )?; | ||
|
|
||
| Ok((game_state.round_count, new_game_state.round_count)) | ||
| } |
There was a problem hiding this comment.
Review the implementation of prepare_next_game for correctness and efficiency.
Review the prepare_next_game function to ensure it is implemented correctly and efficiently, particularly in how it handles the transition between games and the distribution of initial tokens.
| /// Compute the raffle winner based on the total tokens allocated among the winning pots. | ||
| pub fn get_raffle_winner( | ||
| storage: &dyn Storage, | ||
| winning_pots: &Vec<u8>, | ||
| ) -> Result<Option<String>, ContractError> { | ||
| let mut max_total = Uint128::zero(); | ||
| let mut winner: Option<String> = None; | ||
|
|
||
| // TODO: Early return if there is not raffle.denom_amount nor raffle.cw721_id | ||
|
|
||
| // Traverse all player allocations | ||
| let all_allocations = | ||
| PLAYER_ALLOCATIONS.range(storage, None, None, cosmwasm_std::Order::Ascending); | ||
| for item in all_allocations { | ||
| let (addr, player_allocations) = item?; | ||
|
|
||
| let total_in_winning_pots: Uint128 = player_allocations | ||
| .iter() | ||
| .filter(|allocation| winning_pots.contains(&allocation.pot_id)) | ||
| .map(|allocation| allocation.amount) | ||
| .sum(); | ||
|
|
||
| // Update the totals and check for the highest | ||
| if total_in_winning_pots > max_total { | ||
| max_total = total_in_winning_pots; | ||
| winner = Some(addr.to_string()); | ||
| } else if total_in_winning_pots == max_total { | ||
| // In case of a tie, find the earliest bidder among the winning pots | ||
| let mut earliest_bid_time = u64::MAX; | ||
| // TODO: handle unwrap safely, but I think this should never happen | ||
| let mut current_winner = winner; | ||
|
|
||
| for &pot_id in winning_pots.iter() { | ||
| // if there is any first bidder | ||
| if let Ok(fb) = FIRST_BIDDER.load(storage, pot_id) { | ||
| // if the current user is first better, and its earlier than the previously iterated one among wining pots | ||
| if fb.time < earliest_bid_time && fb.bidder == addr { | ||
| // so the new earliest is the current one | ||
| earliest_bid_time = fb.time; | ||
| current_winner = Some(fb.bidder); // we have a new winner | ||
| } else if fb.time == earliest_bid_time && fb.bidder == addr { | ||
| // otherwise if its equal, meaning we have another tie situation, | ||
| // set it to None and DO NOT distribute the raffle this round! | ||
| current_winner = None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| winner = current_winner; | ||
| } | ||
| } | ||
|
|
||
| // If no allocations to winning pots were made, or all were zero, there's no raffle winner | ||
| if max_total == Uint128::zero() { | ||
| Ok(None) | ||
| } else { | ||
| Ok(winner) | ||
| } | ||
| } |
There was a problem hiding this comment.
Check the logic and error handling in get_raffle_winner.
Ensure that the get_raffle_winner function contains robust logic and error handling, particularly in how it determines the winner based on allocations in winning pots.
| ContractError, | ||
| > { | ||
| let game_config = GAME_CONFIG.load(deps.storage)?; | ||
| let raffle = RAFFLE.load(deps.storage)?; | ||
|
|
||
| let mut msgs = Vec::new(); | ||
| let mut submsgs = Vec::new(); | ||
|
|
||
| // this is common for yes_raffle and no_raffle scenarios | ||
| let mut new_raffle_denom_amount = | ||
| validate_funds(funds, &game_config.game_denom).unwrap_or_default(); | ||
|
|
||
| let raffle_winner = get_raffle_winner(deps.storage, &winning_pots)?; | ||
|
|
||
| match raffle_winner { | ||
| Some(recipient) => { | ||
| if let Some(cw721_id) = &raffle.cw721_token_id { | ||
| let transfer_nft_msg = SubMsg::reply_always( | ||
| WasmMsg::Execute { | ||
| contract_addr: raffle.cw721_addr.unwrap(), // TODO: Better handle this unwrap | ||
| msg: to_json_binary(&cw721::Cw721ExecuteMsg::TransferNft { | ||
| recipient: recipient.clone(), | ||
| token_id: cw721_id.clone(), | ||
| })?, | ||
| funds: vec![], | ||
| }, | ||
| ReplyMsg::GameEnd as u64, | ||
| ); | ||
| submsgs.push(transfer_nft_msg); | ||
| } | ||
|
|
||
| let (prize_to_distribute, prize_to_treasury) = get_raffle_denom_prize_amounts(deps)?; | ||
| if !prize_to_distribute.is_zero() { | ||
| let send_msg = CosmosMsg::Bank(BankMsg::Send { | ||
| to_address: recipient, | ||
| amount: coins(prize_to_distribute.u128(), game_config.game_denom.clone()), | ||
| }); | ||
| msgs.push(send_msg); | ||
| } | ||
| if !prize_to_treasury.is_zero() { | ||
| let send_msg = CosmosMsg::Bank(BankMsg::Send { | ||
| to_address: game_config.fee_address.to_string(), | ||
| amount: coins(prize_to_treasury.u128(), game_config.game_denom.clone()), | ||
| }); | ||
| msgs.push(send_msg); | ||
| } | ||
| } | ||
| None => { | ||
| // Scenarios are: | ||
| // - Admin sends new NFT, but old is unsent (ok) | ||
| // - Admin sends new NFT, but old is sent (ok) | ||
| // - Admin do not send new NFT, old is sent (ok) | ||
| // - Admin do not send new NFT, but old is unsent (ok) | ||
| // If there was a not-won Raffle NFT, and admin is trying to send a new one, throw error. | ||
| if raffle.cw721_token_id.is_some() && new_raffle_cw721_id.is_some() { | ||
| // we dont want any new NFT from admin if the previous one is unwon | ||
| return Err(ContractError::PreviousRaffleNftIsUnwon {}); | ||
| } | ||
| // if there is still the previous one, override the value | ||
|
|
||
| // if there is still the previous one | ||
| if raffle.cw721_token_id.is_some() { | ||
| new_raffle_cw721_id = raffle.cw721_token_id; | ||
| new_raffle_cw721_addr = raffle.cw721_addr; | ||
| } | ||
| // else we set either the new one already passed from above, or None | ||
|
|
||
| // here for denom prize we increment the old value (possibly 0 or positive) with the new info.funds sent | ||
| new_raffle_denom_amount = new_raffle_denom_amount.checked_add(raffle.denom_amount)?; | ||
| } | ||
| } | ||
|
|
||
| // Send the new NFTs for the next raffle prize if any to process | ||
| // TODO: Check if new_raffle_cw721_id is different from the old one and ensure the address is valid | ||
| // if new_raffle_cw721_id != raffle.cw721_token_id || new_raffle_cw721_addr != raffle.cw721_addr {} | ||
| if let (Some(cw721_id), Some(cw721_addr)) = | ||
| (new_raffle_cw721_id.clone(), new_raffle_cw721_addr.clone()) | ||
| { | ||
| let transfer_nft_msg = SubMsg::reply_always( | ||
| WasmMsg::Execute { | ||
| contract_addr: cw721_addr, | ||
| msg: to_json_binary(&cw721::Cw721ExecuteMsg::TransferNft { | ||
| recipient: env.contract.address.to_string(), | ||
| token_id: cw721_id, | ||
| })?, | ||
| funds: vec![], | ||
| }, | ||
| ReplyMsg::GameEnd as u64, | ||
| ); | ||
| submsgs.push(transfer_nft_msg); | ||
| } | ||
|
|
||
| Ok(( | ||
| msgs, | ||
| submsgs, | ||
| new_raffle_denom_amount, | ||
| new_raffle_cw721_id, | ||
| new_raffle_cw721_addr, | ||
| )) | ||
| } |
There was a problem hiding this comment.
Validate the implementation of process_raffle_winner for correctness and completeness.
Ensure that the process_raffle_winner function is implemented correctly and completely, particularly in how it handles the distribution of raffle prizes and the preparation for the next raffle.
| /// Helper to calculate the prize amount for raffle distribution based on game extensions. | ||
| pub fn get_raffle_denom_prize_amounts(deps: &Deps) -> Result<(Uint128, Uint128), ContractError> { | ||
| let game_config: GameConfig = GAME_CONFIG.load(deps.storage)?; | ||
| let game_state: GameState = GAME_STATE.load(deps.storage)?; | ||
| let raffle: Raffle = RAFFLE.load(deps.storage)?; | ||
|
|
||
| // Apply the decay factor iteratively based on extend_count | ||
| let mut prize_percentage = Uint128::from(100u128); // Starting from 100% | ||
| for _ in 0..game_state.extend_count { | ||
| prize_percentage = | ||
| prize_percentage.multiply_ratio(game_config.decay_factor, Uint128::from(100u128)); | ||
| } | ||
|
|
||
| let distributed_prize = raffle | ||
| .denom_amount | ||
| .multiply_ratio(prize_percentage, Uint128::from(100u128)); | ||
| let remaining_prize = raffle.denom_amount.checked_sub(distributed_prize)?; | ||
|
|
||
| Ok((distributed_prize, remaining_prize)) | ||
| } | ||
|
|
||
| pub fn get_distribution_send_msgs( | ||
| deps: &Deps, | ||
| winning_pots: &[u8], | ||
| total_losing_tokens: Uint128, | ||
| ) -> Result<Vec<CosmosMsg>, ContractError> { | ||
| let game_config = GAME_CONFIG.load(deps.storage)?; | ||
| let total_distribution_amount = total_losing_tokens.multiply_ratio(1u128, 2u128); | ||
|
|
||
| let mut pot_contributions: Vec<Uint128> = vec![Uint128::zero(); 5]; // Assumes 5 pots | ||
| let mut total_winning_tokens = Uint128::zero(); | ||
|
|
||
| // Calculate total token amounts for each winning pot and store them | ||
| for &pot_id in winning_pots { | ||
| if has_player_allocations(deps.storage, pot_id)? { | ||
| let pot_state = POT_STATES.load(deps.storage, pot_id)?; | ||
| pot_contributions[pot_id as usize - 1] = pot_state.amount; | ||
| total_winning_tokens += pot_state.amount; | ||
| } | ||
| } | ||
|
|
||
| let mut messages: Vec<CosmosMsg> = Vec::new(); | ||
| let mut total_fee = Uint128::zero(); | ||
|
|
||
| // Distribute tokens to winning pots based on their contribution to the total | ||
| for &pot_id in winning_pots { | ||
| if pot_contributions[pot_id as usize - 1].is_zero() { | ||
| continue; // Skip pots without player allocations or tokens | ||
| } | ||
|
|
||
| let pot_share = total_distribution_amount | ||
| .multiply_ratio(pot_contributions[pot_id as usize - 1], total_winning_tokens); | ||
| let pot_state = POT_STATES.load(deps.storage, pot_id)?; | ||
| let pot_total_distribution_amount = pot_share + pot_state.amount; | ||
| let fee = pot_total_distribution_amount.multiply_ratio(game_config.fee, 100u128); | ||
| total_fee += fee; | ||
| let net_distribution_amount = pot_total_distribution_amount.checked_sub(fee)?; | ||
|
|
||
| distribute_tokens_to_players( | ||
| deps.storage, | ||
| &game_config, | ||
| pot_id, | ||
| net_distribution_amount, | ||
| &mut messages, | ||
| )?; | ||
| } | ||
|
|
||
| // Send reallocation_fee_pool amount to treasury, we will reset it later on to avoid passing DepsMut state here. | ||
| let reallocation_fee_pool = REALLOCATION_FEE_POOL.load(deps.storage)?; | ||
| if !reallocation_fee_pool.is_zero() { | ||
| messages.push(CosmosMsg::Bank(BankMsg::Send { | ||
| to_address: game_config.fee_address.to_string(), | ||
| amount: coins(reallocation_fee_pool.into(), game_config.game_denom.clone()), | ||
| })) | ||
| } | ||
|
|
||
| // Deduct the total fee and add to messages | ||
| if !total_fee.is_zero() { | ||
| messages.push(CosmosMsg::Bank(BankMsg::Send { | ||
| to_address: game_config.fee_address.to_string(), | ||
| amount: vec![Coin { | ||
| denom: game_config.game_denom, | ||
| amount: total_fee, | ||
| }], | ||
| })); | ||
| } | ||
|
|
||
| Ok(messages) | ||
| } |
There was a problem hiding this comment.
Review the implementation of get_distribution_send_msgs for correctness and efficiency.
Review the get_distribution_send_msgs function to ensure it is implemented correctly and efficiently, particularly in how it distributes tokens to winning pots based on their contributions.
No description provided.