Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: multiple coins final tests #186

Merged
merged 29 commits into from
Jun 6, 2024

Conversation

troykessler
Copy link
Member

@troykessler troykessler commented May 15, 2024

Implemented final unit tests to test the multiple coin feature with all relevant modules together

Summary by CodeRabbit

  • New Features

    • Added support for multiple coins in the stakers, bundles, and funders modules.
    • Enhanced commission rewards to support multiple coins.
  • Bug Fixes

    • Improved the accuracy of commission rewards by using arrays for multiple denominations.
  • Refactor

    • Updated various functions and messages to handle sdk.Coins instead of uint64.
  • Documentation

    • Clarified that commission rewards can be multiple coins in the MsgClaimCommission message.

Copy link

coderabbitai bot commented May 15, 2024

Walkthrough

The recent updates introduce support for handling multiple coins across various modules (stakers, bundles, funders). These changes involve modifying data structures and functions to accommodate and manage multiple coin types instead of a single coin type. This includes updates to protobuf definitions, OpenAPI specifications, and several parts of the codebase to ensure compatibility and correct functionality with the new multi-coin support.

Changes

Files/Modules Change Summary
CHANGELOG.md Documented changes involving multiple coin support in stakers, bundles, funders modules.
docs/static/openapi.yml Updated commission_rewards in StakerMetadata to support multiple coins.
proto/kyve/query/v1beta1/query.proto Changed commission_rewards to a repeated field of cosmos.base.v1beta1.Coin.
proto/kyve/stakers/v1beta1/events.proto Updated EventClaimCommissionRewards to use repeated cosmos.base.v1beta1.Coin with additional annotations.
proto/kyve/stakers/v1beta1/stakers.proto Changed commission_rewards to repeated cosmos.base.v1beta1.Coin and added necessary imports.
proto/kyve/stakers/v1beta1/tx.proto Changed amount in MsgClaimCommissionRewards to repeated cosmos.base.v1beta1.Coin and added necessary imports.
testutil/integration/checks.go Updated balance checks to use sdk.NewCoins() and GetAllBalances for multi-coin support.
testutil/integration/helpers.go Added GetCoinsFromCommunityPool function to retrieve coins from the community pool.
x/bundles/keeper/keeper_suite_funding_bundles_test.go Added necessary imports for handling multiple coins.
x/bundles/keeper/keeper_suite_stakers_leave_test.go Modified assertion for commission rewards to handle multiple coins.
x/bundles/keeper/keeper_suite_zero_delegation_test.go Modified assertion for commission rewards to handle multiple coins.
x/bundles/keeper/logic_bundles.go Simplified logic for calculating and assigning uploader rewards.
x/bundles/types/expected_keepers.go Updated IncreaseStakerCommissionRewards to take sdk.Coins instead of uint64.
x/stakers/client/cli/tx_claim_commission_rewards.go Replaced cast.ToUint64E with sdk.ParseCoinsNormalized for processing arguments.
x/stakers/keeper/getters_staker.go Updated updateStakerCommissionRewards to use sdk.Coins for the amount parameter.
x/stakers/keeper/keeper_suite_test.go Renamed test function from TestBundlesKeeper to TestStakersKeeper.
x/stakers/keeper/logic_stakers.go Updated IncreaseStakerCommissionRewards to use sdk.Coins instead of uint64 and changed fund transfer method to SendCoinsFromModuleToModule.
x/stakers/keeper/msg_server_claim_commission_rewards.go Refactored logic for claiming commission rewards, including updating method calls and modifying reward calculations.
x/stakers/keeper/msg_server_claim_commission_rewards_test.go Added necessary imports for handling errors and types.
x/stakers/spec/03_messages.md Clarified that commission rewards can be multiple coins when claimed using MsgClaimCommission.
x/stakers/spec/05_events.md Modified amount field in EventClaimCommissionRewards to be a repeated field of cosmos.base.v1beta1.Coin with specific annotations.
x/stakers/spec/07_exported.md Updated IncreaseStakerCommissionRewards in StakersKeeper interface to take sdk.Coins instead of uint64.
x/stakers/types/message_claim_commission_rewards.go Added validation check for Amount in MsgClaimCommissionRewards.

Poem

In code's vast realm, where changes bloom,
Multi-coins now dance, dispelling gloom.
Stakers, funders, bundles unite,
Embracing coins of varied might.
With protobufs and tests aligned,
Our blockchain's future, redefined.
🌟✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Out of diff range and nitpick comments (11)
x/stakers/spec/03_messages.md (4)

Line range hint 10-10: Add a comma after "address".

- for each address. The sender can specify an amount which in turn is a direct
+ for each address. The sender can specify an amount, which in turn is a direct

Line range hint 25-25: Add a comma after "passed".

- After the `CommissionChangeTime` has passed the new commission is applied.
+ After the `CommissionChangeTime` has passed, the new commission is applied.

Line range hint 50-50: Add a comma after "passed".

- After the `LeavePoolTime` has passed the valaccount is deleted and the staker can shut down the protocol node.
+ After the `LeavePoolTime` has passed, the valaccount is deleted and the staker can shut down the protocol node.

Line range hint 51-51: Add a newline at the end of the file.

}
+
proto/kyve/stakers/v1beta1/events.proto (1)

5-6: Ensure that the new imports are necessary and used within the file. If they are not used, consider removing them to keep the code clean.

x/stakers/keeper/logic_stakers.go (1)

19-19: Consider adding a log message here to indicate that the amount is empty and no action will be taken. This can help with debugging and understanding the flow of the code.

if amount.Empty() {
    ctx.Logger().Info("No commission rewards to increase, amount is empty.")
    return nil
}
x/stakers/spec/07_exported.md (2)

Line range hint 16-16: Replace hard tabs with spaces for consistency and to avoid formatting issues.

-	LeavePool(ctx sdk.Context, staker string, poolId uint64)
+    LeavePool(ctx sdk.Context, staker string, poolId uint64)

Line range hint 24-24: Replace hard tabs with spaces for consistency and to avoid formatting issues.

-	GetCommission(ctx sdk.Context, stakerAddress string) sdk.Dec
+    GetCommission(ctx sdk.Context, stakerAddress string) sdk.Dec
proto/kyve/stakers/v1beta1/stakers.proto (1)

5-6: Ensure that the new imports are necessary and used within the file. If they are not used, consider removing them to keep the code clean.

x/stakers/spec/05_events.md (1)

Line range hint 157-157: Files should end with a single newline character to avoid potential issues with version control systems and to follow best practices.

-}
\ No newline at end of file
+}
proto/kyve/stakers/v1beta1/tx.proto (1)

5-6: Ensure that the new imports are necessary and used within the file. If they are not used, consider removing them to keep the code clean.

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 1303bc2 and 02350af.
Files ignored due to path filters (4)
  • x/query/types/query.pb.go is excluded by !**/*.pb.go
  • x/stakers/types/events.pb.go is excluded by !**/*.pb.go
  • x/stakers/types/stakers.pb.go is excluded by !**/*.pb.go
  • x/stakers/types/tx.pb.go is excluded by !**/*.pb.go
Files selected for processing (25)
  • CHANGELOG.md (1 hunks)
  • docs/static/openapi.yml (6 hunks)
  • proto/kyve/query/v1beta1/query.proto (1 hunks)
  • proto/kyve/stakers/v1beta1/events.proto (2 hunks)
  • proto/kyve/stakers/v1beta1/stakers.proto (2 hunks)
  • proto/kyve/stakers/v1beta1/tx.proto (2 hunks)
  • testutil/integration/checks.go (1 hunks)
  • testutil/integration/helpers.go (1 hunks)
  • x/bundles/keeper/keeper_suite_funding_bundles_test.go (13 hunks)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go (28 hunks)
  • x/bundles/keeper/keeper_suite_stakers_leave_test.go (1 hunks)
  • x/bundles/keeper/keeper_suite_valid_bundles_test.go (18 hunks)
  • x/bundles/keeper/keeper_suite_zero_delegation_test.go (2 hunks)
  • x/bundles/keeper/logic_bundles.go (1 hunks)
  • x/bundles/types/expected_keepers.go (1 hunks)
  • x/stakers/client/cli/tx_claim_commission_rewards.go (2 hunks)
  • x/stakers/keeper/getters_staker.go (1 hunks)
  • x/stakers/keeper/keeper_suite_test.go (1 hunks)
  • x/stakers/keeper/logic_stakers.go (3 hunks)
  • x/stakers/keeper/msg_server_claim_commission_rewards.go (2 hunks)
  • x/stakers/keeper/msg_server_claim_commission_rewards_test.go (6 hunks)
  • x/stakers/spec/03_messages.md (1 hunks)
  • x/stakers/spec/05_events.md (1 hunks)
  • x/stakers/spec/07_exported.md (1 hunks)
  • x/stakers/types/message_claim_commission_rewards.go (1 hunks)
Files not summarized due to errors (2)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go: Error: Message exceeds token limit
  • x/bundles/keeper/keeper_suite_valid_bundles_test.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
  • x/stakers/keeper/keeper_suite_test.go
Additional comments not posted (125)
x/stakers/client/cli/tx_claim_commission_rewards.go (1)

8-8: LGTM! The change to parse the amount as sdk.Coins is appropriate for handling multiple coins.

x/stakers/types/message_claim_commission_rewards.go (1)

42-45: LGTM! The validation logic for sdk.Coins is correctly implemented.

x/stakers/keeper/msg_server_claim_commission_rewards.go (3)

24-25: LGTM! The logic for checking if the amount can be claimed is correctly implemented.


29-30: LGTM! The logic for transferring sdk.Coins is correctly implemented.


35-35: LGTM! The logic for updating the staker's commission rewards is correctly implemented.

testutil/integration/helpers.go (1)

8-16: LGTM! The function correctly retrieves and returns the coins from the community pool.

x/bundles/types/expected_keepers.go (1)

31-31: LGTM! The change to accept sdk.Coins is appropriate for handling multiple coins.

proto/kyve/stakers/v1beta1/events.proto (1)

71-76: The amount field in EventClaimCommissionRewards has been correctly updated to support multiple coins. The annotations ensure proper handling of repeated fields.

x/stakers/keeper/logic_stakers.go (2)

17-17: The method signature for IncreaseStakerCommissionRewards has been correctly updated to accept sdk.Coins instead of uint64. This change aligns with the new requirement to handle multiple coins.


29-29: The method for transferring funds has been correctly updated to use SendCoinsFromModuleToModule to handle multiple coins. Ensure that this method is properly tested to verify the transfer of multiple coins.

x/stakers/spec/07_exported.md (1)

68-68: The documentation for the IncreaseStakerCommissionRewards function has been correctly updated to reflect the new signature that accepts sdk.Coins instead of uint64. Ensure that all references to this function in the documentation are consistent with this change.

proto/kyve/stakers/v1beta1/stakers.proto (1)

31-36: The commission_rewards field in the Staker message has been correctly updated to support multiple coins. The annotations ensure proper handling of repeated fields.

x/stakers/spec/05_events.md (1)

101-106: The documentation for the EventClaimCommissionRewards event has been correctly updated to reflect the new amount field that supports multiple coins. Ensure that all references to this event in the documentation are consistent with this change.

proto/kyve/stakers/v1beta1/tx.proto (1)

93-97: The amount field in the MsgClaimCommissionRewards message has been correctly updated to support multiple coins. The annotations ensure proper handling of repeated fields.

proto/kyve/query/v1beta1/query.proto (1)

132-137: Update commission_rewards field to support multiple coins.

The changes to the commission_rewards field in the StakerMetadata message correctly update it to support multiple coins using repeated cosmos.base.v1beta1.Coin. This is consistent with the overall goal of supporting multiple coins.

x/stakers/keeper/getters_staker.go (1)

42-45: Update updateStakerCommissionRewards to handle sdk.Coins.

The changes to the updateStakerCommissionRewards function correctly update it to handle sdk.Coins instead of uint64. This aligns with the goal of supporting multiple coins.

CHANGELOG.md (3)

21-21: Add changelog entry for multiple coins support in stakers module.

The changelog entry correctly describes the support for multiple coins in the stakers module.


22-22: Add changelog entry for multiple coins support in bundles module.

The changelog entry correctly describes the support for multiple coins in the bundles module.


23-23: Add changelog entry for multiple coins support in funders module.

The changelog entry correctly describes the support for multiple coins in the funders module.

x/bundles/keeper/keeper_suite_stakers_leave_test.go (1)

257-257: Update assertion to handle multiple coins correctly.

The assertion for uploader.CommissionRewards has been updated to check the amount of a specific denomination. This change ensures that the test correctly handles multiple coins.

x/stakers/keeper/msg_server_claim_commission_rewards_test.go (11)

199-211: The test case "Produce a valid bundle and check commission rewards" is well-structured and covers the necessary assertions for verifying the commission rewards. Good job!


221-238: The test case "Claim with non-staker account" correctly verifies that an error is returned when a non-staker account attempts to claim rewards. Well done!


242-260: The test case "Claim more rewards than available" correctly verifies that an error is returned when attempting to claim more rewards than available. Good job!


264-279: The test case "Claim zero rewards" correctly verifies that claiming zero rewards does not alter the commission rewards. Well done!


281-296: The test case "Claim partial rewards" correctly verifies that claiming partial rewards correctly updates the commission rewards. Good job!


299-368: The test case "Claim partial rewards twice" correctly verifies that claiming partial rewards twice correctly updates the commission rewards. Well done!


370-385: The test case "Claim all rewards" correctly verifies that claiming all rewards correctly updates the commission rewards. Good job!


388-459: The test case "Claim multiple coins" correctly verifies that claiming multiple coins correctly updates the commission rewards. Well done!


461-535: The test case "Claim one coin of multiple coins" correctly verifies that claiming one coin of multiple coins correctly updates the commission rewards. Good job!


Line range hint 537-614: The test case "Claim more rewards than available with multiple coins" correctly verifies that an error is returned when attempting to claim more rewards than available with multiple coins. Well done!


616-689: The test case "Claim coin which does not exist" correctly verifies that an error is returned when attempting to claim a coin that does not exist. Good job!

testutil/integration/checks.go (2)

169-172: Correctly updated to handle sdk.Coins for expected balance.


176-178: Correctly updated to handle sdk.Coins for actual balance.

x/bundles/keeper/logic_bundles.go (15)

588-588: Ensure correct handling of multiple coins in uploader rewards.

The change to handle multiple coins in uploader rewards looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for uploader rewards.


Line range hint 288-288: Ensure correct handling of multiple coins in storage cost calculation.

The change to handle multiple coins in storage cost calculation looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for storage costs.


Line range hint 278-278: Ensure correct handling of multiple coins in treasury payouts.

The change to handle multiple coins in treasury payouts looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for treasury payouts.


Line range hint 298-298: Ensure correct handling of multiple coins in delegation rewards.

The change to handle multiple coins in delegation rewards looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for delegation rewards.


Line range hint 308-308: Ensure correct handling of multiple coins in uploader commission.

The change to handle multiple coins in uploader commission looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for uploader commission.


Line range hint 578-578: Ensure correct handling of multiple coins in treasury rewards.

The change to handle multiple coins in treasury rewards looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for treasury rewards.


588-588: Ensure correct handling of multiple coins in uploader rewards.

The change to handle multiple coins in uploader rewards looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for uploader rewards.


Line range hint 598-598: Ensure correct handling of multiple coins in delegator rewards.

The change to handle multiple coins in delegator rewards looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for delegator rewards.


Line range hint 608-608: Ensure correct handling of multiple coins in staker slashing.

The change to handle multiple coins in staker slashing looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for staker slashing.


Line range hint 618-618: Ensure correct handling of multiple coins in tally result.

The change to handle multiple coins in tally result looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for tally result.


Line range hint 268-268: Ensure correct handling of multiple coins in total payout calculation.

The change to handle multiple coins in total payout calculation looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for total payout calculation.


Line range hint 298-298: Ensure correct handling of multiple coins in commission rewards calculation.

The change to handle multiple coins in commission rewards calculation looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for commission rewards calculation.


Line range hint 288-288: Ensure correct handling of multiple coins in uploader storage cost calculation.

The change to handle multiple coins in uploader storage cost calculation looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for uploader storage cost calculation.


Line range hint 278-278: Ensure correct handling of multiple coins in treasury payout calculation.

The change to handle multiple coins in treasury payout calculation looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for treasury payout calculation.


Line range hint 268-268: Ensure correct handling of multiple coins in total payout calculation.

The change to handle multiple coins in total payout calculation looks correct. However, please verify that all relevant parts of the codebase correctly handle sdk.Coins instead of uint64 for total payout calculation.

x/bundles/keeper/keeper_suite_zero_delegation_test.go (2)

8-8: Import alias globaltypes is used correctly.


476-476: Ensure the correct denomination is used in the assertion.

Please verify that globaltypes.Denom is the correct denomination for the assertion.

x/bundles/keeper/keeper_suite_funding_bundles_test.go (3)

773-845: The test case "Produce a valid bundle with only one funder and multiple coins" is well-structured and covers the necessary assertions. Good job!


847-932: The test case "Produce a valid bundle with multiple funders and multiple coins" is well-structured and covers the necessary assertions. Good job!


934-1019: The test case "Produce a valid bundle although the only funder can not pay for the full bundle reward" is well-structured and covers the necessary assertions. Good job!

x/bundles/keeper/keeper_suite_inflation_splitting_test.go (37)

11-11: Importing sdk as an alias is a good practice for readability and consistency.


36-39: The added test cases for multiple coins funded with different inflation splitting percentages are well-documented and cover various scenarios. This enhances the test coverage significantly.


45-46: Defining amountPerBundle as a variable at the beginning of the test suite improves readability and maintainability.


71-75: Setting the storage cost parameter ensures that the tests are run with the correct configuration. This is a good practice to ensure consistency across test runs.


76-102: The addition of multiple coin entries in the funders' parameters is well-structured. It ensures that the tests cover scenarios with different coin denominations and weights.


186-186: The addition of GetCoinsFromCommunityPool is a useful helper function to retrieve the state of the community pool, which is essential for validating the test outcomes.


219-219: The assertion for empty commission rewards is correct and ensures that the expected state is validated.


225-228: The assertion for the treasury payout when inflation is zero is correctly implemented. It ensures that no unexpected payouts occur.


298-298: The calculation and assertion of the inflation payout are correctly implemented. This ensures that the payout logic is validated accurately.


305-310: The detailed comments explaining the calculation of commission and delegation rewards are very helpful for understanding the logic. The assertions are correctly implemented.


383-383: The calculation and assertion of the inflation payout for 100% inflation splitting are correctly implemented. This ensures that the payout logic is validated accurately.


390-395: The detailed comments explaining the calculation of commission and delegation rewards for 100% inflation splitting are very helpful for understanding the logic. The assertions are correctly implemented.


420-427: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with sufficient amounts for the test scenarios.


455-455: The addition of GetCoinsFromCommunityPool is a useful helper function to retrieve the state of the community pool, which is essential for validating the test outcomes.


484-493: The detailed comments explaining the calculation of commission and delegation rewards are very helpful for understanding the logic. The assertions are correctly implemented.


497-503: The assertion for the treasury payout when inflation is zero is correctly implemented. It ensures that no unexpected payouts occur.


525-532: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with sufficient amounts for the test scenarios.


587-599: The detailed comments explaining the calculation of commission and delegation rewards for 10% inflation splitting are very helpful for understanding the logic. The assertions are correctly implemented.


723-730: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with insufficient amounts for the test scenarios.


758-758: The addition of GetCoinsFromCommunityPool is a useful helper function to retrieve the state of the community pool, which is essential for validating the test outcomes.


790-795: The detailed comments explaining the calculation of commission and delegation rewards for insufficient funders are very helpful for understanding the logic. The assertions are correctly implemented.


799-805: The assertion for the treasury payout when inflation is zero is correctly implemented. It ensures that no unexpected payouts occur.


827-834: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with insufficient amounts for the test scenarios.


889-901: The detailed comments explaining the calculation of commission and delegation rewards for 30% inflation splitting are very helpful for understanding the logic. The assertions are correctly implemented.


926-933: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with insufficient amounts for the test scenarios.


988-1000: The detailed comments explaining the calculation of commission and delegation rewards for 10% inflation splitting are very helpful for understanding the logic. The assertions are correctly implemented.


1025-1032: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with some insufficient amounts for the test scenarios.


1060-1060: The addition of GetCoinsFromCommunityPool is a useful helper function to retrieve the state of the community pool, which is essential for validating the test outcomes.


1092-1097: The detailed comments explaining the calculation of commission and delegation rewards for some insufficient funders are very helpful for understanding the logic. The assertions are correctly implemented.


1101-1107: The assertion for the treasury payout when inflation is zero is correctly implemented. It ensures that no unexpected payouts occur.


1129-1136: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with some insufficient amounts for the test scenarios.


1191-1203: The detailed comments explaining the calculation of commission and delegation rewards for 30% inflation splitting are very helpful for understanding the logic. The assertions are correctly implemented.


1228-1235: The funding transactions for Alice and Bob are correctly implemented, ensuring that the pool is funded with some insufficient amounts for the test scenarios.


1290-1302: The detailed comments explaining the calculation of commission and delegation rewards for 100% inflation splitting are very helpful for understanding the logic. The assertions are correctly implemented.


1311-1413: The test case for multiple coins funded with 0% inflation splitting is comprehensive and correctly implemented. It ensures that the logic for handling multiple coins is validated.


1415-1523: The test case for multiple coins funded with 10% inflation splitting is comprehensive and correctly implemented. It ensures that the logic for handling multiple coins is validated.


1525-1632: The test case for multiple coins funded with 100% inflation splitting is comprehensive and correctly implemented. It ensures that the logic for handling multiple coins is validated.

x/bundles/keeper/keeper_suite_valid_bundles_test.go (29)

28-30: Good addition of test cases for multiple validators and multiple coins scenarios. This ensures comprehensive coverage for the new feature.


36-51: The initialization of balances for multiple stakers and validators is correctly done. This setup is crucial for the subsequent assertions.


79-82: Setting the storage cost to 0.5 is a good step to ensure the test case covers the scenario where storage costs are involved.


84-110: The funders' parameters are set correctly, including multiple coins with different weights. This aligns with the PR objective of testing multiple coins.


146-150: Re-initializing the balances after the setup is a good practice to ensure the initial state is correctly captured for assertions.


183-186: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


255-279: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


283-288: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


328-331: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


400-427: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


434-439: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


496-499: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


568-595: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


602-607: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


672-675: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


744-771: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


778-783: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


848-851: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


Line range hint 920-957: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


964-969: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


1007-1010: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


1079-1103: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


1107-1113: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


1150-1153: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


1219-1243: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


1247-1256: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.


1484-1487: Capturing the initial state of the community pool is essential for later assertions on treasury payouts.


1553-1577: The assertions for uploader and voter balances, rewards, and commission rewards are thorough and correctly implemented. This ensures the integrity of the bundle processing logic.


1581-1588: The assertion for the treasury payout is correctly implemented, ensuring that the community pool's state is validated.

docs/static/openapi.yml (6)

612-631: The changes to the commission_rewards field to support multiple coins look good. The description and properties are accurate.


3946-3965: The changes to the commission_rewards field to support multiple coins are consistent and accurate.


6323-6342: The changes to the commission_rewards field to support multiple coins are consistent and accurate.


6781-6800: The changes to the commission_rewards field to support multiple coins are consistent and accurate.


7345-7364: The changes to the commission_rewards field to support multiple coins are consistent and accurate.


7852-7871: The changes to the commission_rewards field to support multiple coins are consistent and accurate.

x/stakers/spec/03_messages.md Outdated Show resolved Hide resolved
# Conflicts:
#	x/bundles/keeper/keeper_suite_inflation_splitting_test.go
#	x/bundles/keeper/keeper_suite_valid_bundles_test.go
shifty11
shifty11 previously approved these changes Jun 5, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Outside diff range and nitpick comments (4)
x/stakers/spec/03_messages.md (1)

Line range hint 51-51: Please ensure the file ends with a single newline character.

x/bundles/keeper/keeper_suite_valid_bundles_test.go (1)

7-7: Ensure consistent import aliasing for SDK types.

It's good practice to consistently alias imported packages, especially common ones like sdk. This helps maintain readability and prevents conflicts.

x/bundles/keeper/keeper_suite_inflation_splitting_test.go (2)

12-12: Ensure consistent import aliasing for SDK types.

It's a good practice to consistently use or avoid aliasing imports unless necessary to resolve conflicts. Here, sdk is aliased, which is common, but ensure it's consistently used across your project for better readability.


434-434: Ensure consistency in funding amounts per bundle across different funders.

It's noted that the AmountsPerBundle for both funders are set to the same value. If this is intentional and required by the business logic, consider adding a comment to clarify why this consistency is necessary. If not, this might be an area to allow variability based on funder preferences or strategies.

Also applies to: 441-441

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 02350af and 4fb1df1.

Files selected for processing (4)
  • x/bundles/keeper/keeper_suite_funding_bundles_test.go (13 hunks)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go (38 hunks)
  • x/bundles/keeper/keeper_suite_valid_bundles_test.go (18 hunks)
  • x/stakers/spec/03_messages.md (1 hunks)
Files not summarized due to errors (2)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go: Error: Message exceeds token limit
  • x/bundles/keeper/keeper_suite_valid_bundles_test.go: Error: Message exceeds token limit
Additional context used
LanguageTool
x/stakers/spec/03_messages.md

[uncategorized] ~25-~25: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: .... After the CommissionChangeTime has passed the new commission is applied. ## `Msg...


[uncategorized] ~50-~50: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...en pool. After the LeavePoolTime has passed the valaccount is deleted and the stake...

Markdownlint
x/stakers/spec/03_messages.md

51-51: null (MD047, single-trailing-newline)
Files should end with a single newline character

Additional comments not posted (12)
x/stakers/spec/03_messages.md (2)

36-36: Clarification on multiple coins in commission rewards is clear and aligns with system updates.


Line range hint 25-25: The suggested commas by the static analysis tool are not necessary in this context.

Also applies to: 50-50

x/bundles/keeper/keeper_suite_funding_bundles_test.go (5)

8-11: Imports are correctly organized and relevant to the test cases.


36-37: Initialization of initialBalanceAlice and initialBalanceBob is clear and concise.


70-95: The setup for the whitelist parameters is comprehensive and covers multiple scenarios with different coin denominations and weights.


200-211: The assertions for balances and funded amounts are correctly implemented and check the expected outcomes based on the test setup.

Also applies to: 281-295, 368-382, 455-473, 543-547, 622-626, 703-707, 833-844, 914-931, 994-1005


145-146: Ensure consistency in the representation of coin amounts across all test cases.

Also applies to: 219-220, 226-227, 306-307, 313-314, 393-394, 400-401, 555-556, 562-563, 634-635, 641-642, 776-779, 850-853, 857-860

Verification successful

The representation of coin amounts using i.ACoins is consistent across all test cases in the specified file. The values differ as expected for different test scenarios, but the format and usage remain uniform.

  • Lines 145-146: i.ACoins(100 * i.T_KYVE) and i.ACoins(10 * i.T_KYVE)
  • Lines 219-220, 226-227, 306-307, 313-314, 393-394, 400-401, 555-556, 562-563, 634-635, 641-642, 776-779, 850-853, 857-860: All instances follow the same format.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify consistent representation of coin amounts across test cases.

# Test: Search for inconsistent coin amount representations. Expect: Consistent usage.
rg --type go $'i.ACoins' x/bundles/keeper/keeper_suite_funding_bundles_test.go

Length of output: 3009

x/bundles/keeper/keeper_suite_valid_bundles_test.go (2)

186-186: Verify the correctness of GetCoinsFromCommunityPool method.

Verification successful

The implementation of the GetCoinsFromCommunityPool method is as follows:

func (suite *KeeperTestSuite) GetCoinsFromCommunityPool() sdk.Coins {
	pool, err := suite.App().DistributionKeeper.FeePool.Get(suite.Ctx())
	if (err != nil) {
		return sdk.NewCoins()
	}
	coins, _ := pool.CommunityPool.TruncateDecimal()
	return coins
}

This method retrieves the community pool from the distribution keeper, handles any errors by returning an empty set of coins, and truncates the decimal values of the coins in the community pool before returning them.

The method appears to be correctly implemented and used consistently across the test files.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correctness of the GetCoinsFromCommunityPool method by checking its implementation and usage across the test suite.

# Test: Search for the function usage and implementation. Expect: Correct implementation and consistent usage.
rg --type go $'GetCoinsFromCommunityPool'

Length of output: 3106



Script:

#!/bin/bash
# Extract the implementation of the GetCoinsFromCommunityPool method from the helpers.go file.
ast-grep --lang go --pattern $'func (suite *KeeperTestSuite) GetCoinsFromCommunityPool() $_' testutil/integration/helpers.go

Length of output: 689


255-265: Ensure consistent handling of balances and rewards calculations.

x/bundles/keeper/keeper_suite_inflation_splitting_test.go (3)

37-39: Add test cases for multiple coins with inflation splitting.

These new test cases are crucial for ensuring that the system behaves correctly when handling multiple types of coins with different inflation splitting settings. Good addition for comprehensive testing.


197-197: Validate changes to community pool coin handling.

The modifications to handle multiple types of coins in the community pool are well-implemented. This is crucial for systems that deal with various coin types and ensures that the logic for handling pool balances is robust.

Also applies to: 237-239


81-107: Ensure dynamic handling of multiple coin types in funders parameters.

The handling of multiple coin types with different weights and minimum funding amounts is a complex feature. Verify that the logic correctly handles edge cases, such as extremely high or low values, and interacts correctly with other parts of the system.

@troykessler troykessler merged commit 7683992 into main Jun 6, 2024
5 checks passed
@troykessler troykessler deleted the troy/multiple-coins-integration-tests branch June 6, 2024 06:23
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.

3 participants