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

feat: support multiple coins for funding in stakers module #185

Merged
merged 6 commits into from
May 14, 2024

Conversation

troykessler
Copy link
Member

@troykessler troykessler commented May 7, 2024

This PR implements multiple coins for funding in the stakers module

Summary by CodeRabbit

  • New Features

    • Enhanced commission rewards system to support multiple denominations and amounts.
  • Bug Fixes

    • Improved accuracy and consistency in calculating and displaying commission rewards.
  • Refactor

    • Updated internal data structures to support new commission rewards format.
  • Tests

    • Modified test cases to validate new commission rewards format and calculations.

@troykessler troykessler requested a review from mbreithecker May 7, 2024 10:07
@troykessler troykessler self-assigned this May 7, 2024
Copy link

coderabbitai bot commented May 7, 2024

Walkthrough

The recent changes in the KYVE Network codebase involve updating the representation of commission_rewards to a more detailed structure using cosmos.base.v1beta1.Coin. This enhancement provides a comprehensive description of rewards by incorporating denomination and amount, ensuring consistency in handling and testing these new data structures.

Changes

File Path Change Summary
docs/static/openapi.yml Updated commission_rewards to an array of objects with denom and amount properties.
proto/kyve/query/v1beta1/query.proto
proto/kyve/stakers/v1beta1/events.proto
proto/kyve/stakers/v1beta1/stakers.proto
proto/kyve/stakers/v1beta1/tx.proto
Changed commission_rewards fields to repeated cosmos.base.v1beta1.Coin type.
testutil/integration/checks.go
x/bundles/keeper/keeper_suite_..._test.go
x/bundles/keeper/keeper_suite_zero_delegation_test.go
Updated handling and assertions for commission_rewards to align with the new data structure.

🐇

In the land of code, rewards now bloom,

Coins of many, in staker's room.

From single digits, they now expand,

Denom and amount, hand in hand.

Testing's thorough, assertions tight,

KYVE's future, shining bright. 🌟


<!-- walkthrough_end --><!-- This is an auto-generated comment: raw summary by coderabbit.ai -->

<!-------

docs/static/openapi.yml: ## Alterations to the declarations of exported or public entities

  • commission_rewards in StakerMetadata and CommissionChangeEntry:
    • Before:
      type: string
      format: uint64
    • After:
      type: array
      items:
        type: object
        properties:
          denom:
            type: string
          amount:
            type: string
        description: >-
          Coin defines a token with a denomination and an amount.
          
          NOTE: The amount field is an Int which implements the custom method
          signatures required by gogoproto.

proto/kyve/query/v1beta1/query.proto: ## Short summary

In the StakerMetadata message of query.proto, the commission_rewards field has been changed from a single uint64 representing rewards in $KYVE to a repeated field of type cosmos.base.v1beta1.Coin representing rewards through commission and storage cost.


proto/kyve/stakers/v1beta1/events.proto: ## Summary

In the events.proto file under kyve.stakers.v1beta1 package:

  • Added imports for amino/amino.proto and cosmos/base/v1beta1/coin.proto.
  • Modified the EventClaimCommissionRewards message by changing the type of amount to a repeated field of cosmos.base.v1beta1.Coin.

proto/kyve/stakers/v1beta1/stakers.proto: ## Short summary

In the stakers.proto file under kyve.stakers.v1beta1, the changes include adding imports for amino/amino.proto and cosmos/base/v1beta1/coin.proto. The commission_rewards field in the Staker message now uses a repeated type cosmos.base.v1beta1.Coin instead of uint64, with additional options for serialization.


proto/kyve/stakers/v1beta1/tx.proto: ## Short Summary

In the tx.proto file under kyve.stakers.v1beta1, the changes include adding imports for amino/amino.proto and cosmos/base/v1beta1/coin.proto. The amount field in MsgClaimCommissionRewards message now uses a repeated type cosmos.base.v1beta1.Coin with additional options.


testutil/integration/checks.go: ## Summary

In the checks.go file, the VerifyStakersModuleAssetsIntegrity function has been updated to use sdk.NewCoins() for expectedBalance instead of uint64(0). Additionally, the calculation of expectedBalance has been modified to use expectedBalance.Add(staker.CommissionRewards...). The retrieval of actualBalance has been changed to suite.App().BankKeeper.GetAllBalances(suite.Ctx(), moduleAcc), and the comparison between actualBalance and expectedBalance now compares their string representations.


x/bundles/keeper/keeper_suite_inflation_splitting_test.go: ### Alterations to the declarations of exported or public entities:

  • Expect(uploader.CommissionRewards).To(BeEmpty()) changed to Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(BeEmpty())
  • Expect(uploader.CommissionRewards).To(Equal(uploaderPayoutReward + storageReward) changed to Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(Equal(uploaderPayoutReward + storageReward))

x/bundles/keeper/keeper_suite_stakers_leave_test.go: ## Short summary

In the keeper_suite_stakers_leave_test.go file, the change involves modifying the assertion for uploader.CommissionRewards to use a method call AmountOf(globalTypes.Denom).Uint64() instead of a direct comparison with uploaderPayoutReward + storageReward.


x/bundles/keeper/keeper_suite_valid_bundles_test.go: ### Alterations to the declarations of exported or public entities:

  • Expect(uploader.CommissionRewards).To(Equal(uploaderPayoutReward + storageReward) changed to Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(Equal(uploaderPayoutReward + storageReward))

x/bundles/keeper/keeper_suite_zero_delegation_test.go: ## Short Summary
The change in functionality involves modifying the assertion for commission rewards in a test case. The expectation for commission rewards now includes accessing the amount of a specific denomination from a global type.

Alterations to the declarations of exported or public entities

  • Addition of globaltypes "github.com/KYVENetwork/chain/x/global/types" in imports in x/bundles/keeper/keeper_suite_zero_delegation_test.go
  • Modification in assertion:
    • Expect(uploader.CommissionRewards).To(Equal(totalUploaderReward + storageReward)) changed to Expect(uploader.CommissionRewards.AmountOf(globaltypes.Denom).Uint64()).To(Equal(totalUploaderReward + storageReward))

CHANGELOG.md: ## Short summary

The change introduces support for multiple coins in the stakers module, bundles module, and funding in the funders module. Additionally, an end-key is added to the pool in the bundles module.


x/bundles/keeper/logic_bundles.go: ## Short Summary

In the tallyBundleProposal function of logic_bundles.go, the calculation and assignment of uploaderReward have been simplified by directly assigning the result of adding UploaderCommission and UploaderStorageCost from `bundleReward without checking for a specific denomination. The comment regarding future reward payout has been removed.

Alterations to the declarations of exported or public entities

  • func (k Keeper) tallyBundleProposal(ctx sdk.Context, bundleProposal types.BundleProposal, bundleReward types.BundleReward) (types.TallyResult, error) in x/bundles/keeper/logic_bundles.go
    • Removed the explicit assignment of uploaderReward and simplified it to uploaderReward := bundleReward.UploaderCommission.Add(bundleReward.UploaderStorageCost...)

x/bundles/types/expected_keepers.go: ## Short Summary

In the StakerKeeper interface in x/bundles/types/expected_keepers.go, the IncreaseStakerCommissionRewards method's signature has been updated to take amount as sdk.Coins instead of uint64.

Alterations to the declarations of exported or public entities

  • IncreaseStakerCommissionRewards(ctx sdk.Context, address string, amount uint64) error in interface StakerKeeper in x/bundles/types/expected_keepers.go
    changed to
    IncreaseStakerCommissionRewards(ctx sdk.Context, address string, payerModuleName string, amount sdk.Coins) error

x/stakers/keeper/logic_stakers.go: ## Short Summary

The changes in logic_stakers.go involve refactoring the IncreaseStakerCommissionRewards function in the Keeper struct within the keeper package. The modifications include updating the parameter type from uint64 to sdk.Coins for the amount parameter and adjusting the corresponding logic for handling empty amounts.

Alterations to the declarations of exported or public entities

  • func (k Keeper) IncreaseStakerCommissionRewards(ctx sdk.Context, address string, amount uint64) error in package keeperfunc (k Keeper) IncreaseStakerCommissionRewards(ctx sdk.Context, address string, payerModuleName string, amount sdk.Coins) error

x/stakers/keeper/msg_server_claim_commission_rewards_test.go: ### Alterations to the declarations of exported or public entities

  • globalTypes in keeper_test package changed to globaltypes
  • Added import "cosmossdk.io/errors"
  • Added import "github.com/cosmos/cosmos-sdk/types/errors"

x/stakers/types/message_claim_commission_rewards.go: ## Update existing summary

For this task, the existing summary is still valid.

Short Summary

Added validation for the Amount field in the MsgClaimCommissionRewards struct's ValidateBasic function.

Alterations to the declarations of exported or public entities

  • func (msg *MsgClaimCommissionRewards) ValidateBasic() error in x/stakers/types/message_claim_commission_rewards.go:
    • Added validation for the Amount field in the MsgClaimCommissionRewards struct.

------->

<!-- end of auto-generated comment: raw summary by coderabbit.ai --><!-- This is an auto-generated comment: pr objectives by coderabbit.ai -->

<!-------


## PR Summary

**Title:** feat: support multiple coins for funding in stakers module  
**User:** troykessler  
**Number:** 185  

**Description:**  
This PR implements multiple coins for funding in the stakers module.

In this pull request, user troykessler has introduced support for multiple coins for funding within the stakers module. The specific details of how this implementation works or the rationale behind adding this feature are not provided in the description. The focus of this update is on enabling the stakers module to accept funding in various coins, expanding the flexibility and options available for users interacting with this module.


------->

<!-- end of auto-generated comment: pr objectives by coderabbit.ai --><!-- This is an auto-generated comment: shorter summary by coderabbit.ai -->

<!-------

### AI-generated Summary of Generated Summaries

The recent changes in the KYVE project involve transitioning the `commission_rewards` field from a single `uint64` to a repeated field of type `cosmos.base.v1beta1.Coin`. This alteration is reflected across various protobuf files like `query.proto`, `events.proto`, `stakers.proto`, and `tx.proto`, as well as in test files.

1. **Protobuf Files**:
   - **`query.proto`**: Updated `commission_rewards` in `StakerMetadata` to a repeated `cosmos.base.v1beta1.Coin`.
   - **`events.proto`**: Modified `amount` in `EventClaimCommissionRewards` to a repeated `cosmos.base.v1beta1.Coin`.
   - **`stakers.proto`**: Changed `commission_rewards` in `Staker` to a repeated `cosmos.base.v1beta1.Coin`.
   - **`tx.proto`**: Altered `amount` in `MsgClaimCommissionRewards` to a repeated `cosmos.base.v1beta1.Coin`.

2. **Test Files**:
   - **`checks.go`**: Updated `VerifyStakersModuleAssetsIntegrity` to use `sdk.NewCoins()` for `expectedBalance` and adjusted logic for balance calculations.
   - **Various Test Files**: Adjusted assertions for `uploader.CommissionRewards` to use `AmountOf(globalTypes.Denom).Uint64()` method.

These changes enhance the flexibility and accuracy of representing rewards by enabling the `commission_rewards` field to accommodate multiple denominations and amounts effectively.

### Alterations to the Declarations of Exported or Public Entities

#### `docs/static/openapi.yml`
- **Before**:
  ```yaml
  type: string
  format: uint64
  • After:
    type: array
    items:
      type: object
      properties:
        denom:
          type: string
        amount:
          type: string
      description: >-
        Coin defines a token with a denomination and an amount.
        
        NOTE: The amount field is an Int which implements the custom method
        signatures required by gogoproto.

proto/kyve/query/v1beta1/query.proto

  • Before:
    uint64 commission_rewards = 1;
  • After:
    repeated cosmos.base.v1beta1.Coin commission_rewards = 1;

proto/kyve/stakers/v1beta1/events.proto

  • Before:
    uint64 amount = 1;
  • After:
    repeated cosmos.base.v1beta1.Coin amount = 1;

proto/kyve/stakers/v1beta1/stakers.proto

  • Before:
    uint64 commission_rewards = 1;
  • After:
    repeated cosmos.base.v1beta1.Coin commission_rewards = 1;

proto/kyve/stakers/v1beta1/tx.proto

  • Before:
    uint64 amount = 1;
  • After:
    repeated cosmos.base.v1beta1.Coin amount = 1;

testutil/integration/checks.go

  • Before:
    expectedBalance := uint64(0)
    expectedBalance += staker.CommissionRewards
    actualBalance := suite.App().BankKeeper.GetBalance(suite.Ctx(), moduleAcc)
  • After:
    expectedBalance := sdk.NewCoins()
    expectedBalance = expectedBalance.Add(staker.CommissionRewards...)
    actualBalance := suite.App().BankKeeper.GetAllBalances(suite.Ctx(), moduleAcc)

x/bundles/keeper/keeper_suite_inflation_splitting_test.go

  • Before:
    Expect(uploader.CommissionRewards).To(BeZero())
    Expect(uploader.CommissionRewards).To(Equal(uploaderPayoutReward + storageReward))
  • After:
    Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(BeZero())
    Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(Equal(uploaderPayoutReward + storageReward))

x/bundles/keeper/keeper_suite_stakers_leave_test.go

  • Before:
    Expect(uploader.CommissionRewards).To(Equal(uploaderPayoutReward + storageReward))
  • After:
    Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(Equal(uploaderPayoutReward + storageReward))

x/bundles/keeper/keeper_suite_valid_bundles_test.go

  • Before:
    Expect(uploader.CommissionRewards).To(Equal(uploaderPayoutReward + storageReward))
  • After:
    Expect(uploader.CommissionRewards.AmountOf(globalTypes.Denom).Uint64()).To(Equal(uploaderPayoutReward + storageReward))

x/bundles/keeper/keeper_suite_zero_delegation_test.go

  • Before:
    Expect(uploader.CommissionRewards).To(Equal(totalUploaderReward + storageReward))
  • After:
    Expect(uploader.CommissionRewards.AmountOf(globaltypes.Denom).Uint64()).To(Equal(totalUploaderReward + storageReward))

"""

------->


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.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @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.

@troykessler troykessler force-pushed the troy/stakers-multiple-coins branch from 3c3776b to eb482c2 Compare May 14, 2024 08:37
@troykessler troykessler marked this pull request as ready for review May 14, 2024 08:39
@troykessler troykessler requested a review from shifty11 as a code owner May 14, 2024 08:39
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: 0

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

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 36-36: Add a comma after "poolId".

- For joining a pool the staker must provide the poolId and an address which is operated by the protocol node.
+ For joining a pool, the staker must provide the poolId and an address which is operated by the protocol node.

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

+ 
x/stakers/spec/07_exported.md (2)

Line range hint 16-16: Replace hard tabs with spaces.

-	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.

-	GetCommission(ctx sdk.Context, stakerAddress string) sdk.Dec
+    GetCommission(ctx sdk.Context, stakerAddress string) sdk.Dec
x/stakers/spec/05_events.md (1)

Line range hint 157-157: Add a newline character at the end of the file for consistency.

- }
+ }
+
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 50a1c3b and eb482c2.
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 (22)
  • 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)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go (12 hunks)
  • x/bundles/keeper/keeper_suite_stakers_leave_test.go (1 hunks)
  • x/bundles/keeper/keeper_suite_valid_bundles_test.go (7 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 skipped from review due to trivial changes (2)
  • x/bundles/keeper/logic_bundles.go
  • x/stakers/keeper/keeper_suite_test.go
Additional comments not posted (59)
x/stakers/client/cli/tx_claim_commission_rewards.go (2)

8-8: LGTM! The import of sdk is necessary for handling multiple coins.


23-23: LGTM! Parsing 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 correctly handles multiple coins.

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

24-25: LGTM! The check for sufficient rewards correctly handles multiple coins.


29-30: LGTM! The transfer of multiple coins is handled correctly.


35-35: LGTM! The update of commission rewards correctly handles multiple coins.

x/stakers/spec/03_messages.md (1)

31-32: LGTM! The documentation correctly reflects the support for multiple coins.

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

31-31: LGTM! The interface method correctly supports multiple coins.

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

71-76: LGTM! The protobuf message correctly supports multiple coins.

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

17-19: LGTM! The function correctly handles multiple coins.


29-29: LGTM! The transfer of multiple coins is handled correctly.

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

68-68: LGTM! The documentation correctly reflects the support for multiple coins.

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

31-36: The change to make commission_rewards a repeated field of type cosmos.base.v1beta1.Coin looks good and aligns with the objective of supporting multiple coins for commission rewards.

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

101-106: The change to make amount a repeated field of type cosmos.base.v1beta1.Coin in the EventClaimCommissionRewards message is appropriate and aligns with the objective of supporting multiple coins for commission rewards.

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

93-97: The change to make amount a repeated field of type cosmos.base.v1beta1.Coin in the MsgClaimCommissionRewards message is appropriate and aligns with the objective of supporting multiple coins for commission rewards.

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

132-137: The change to make commission_rewards a repeated field of type cosmos.base.v1beta1.Coin in the StakerMetadata message is appropriate and aligns with the objective of supporting multiple coins for commission rewards.

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

42-45: The update to handle sdk.Coins for the amount parameter in the updateStakerCommissionRewards function is appropriate and aligns with the objective of supporting multiple coins for commission rewards.

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

257-257: Update assertion to handle multiple denominations.

The change correctly updates the assertion to handle multiple denominations by using AmountOf(globalTypes.Denom).Uint64().

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

199-211: Ensure the calculation of commission rewards is accurate.

The calculation of commission rewards (10_000 - (10_000 * 0.01) - (100 * 0.5)) * 0.1 + (100 * 0.5) should be verified for accuracy. Ensure that the logic aligns with the expected reward distribution.


221-238: Good test coverage for non-staker account claiming rewards.

The test case correctly verifies that a non-staker account cannot claim rewards and ensures the commission rewards remain unchanged.


242-260: Good validation for claiming more rewards than available.

The test case correctly verifies that claiming more rewards than available results in an error and ensures the commission rewards remain unchanged.


264-279: Good validation for claiming zero rewards.

The test case correctly verifies that claiming zero rewards does not alter the commission rewards or the staker's balance.


281-296: Good validation for claiming partial rewards.

The test case correctly verifies that claiming partial rewards updates the commission rewards and the staker's balance as expected.


299-368: Good validation for claiming partial rewards twice.

The test case correctly verifies that claiming partial rewards twice updates the commission rewards and the staker's balance as expected.


370-385: Good validation for claiming all rewards.

The test case correctly verifies that claiming all rewards updates the commission rewards and the staker's balance as expected.


388-459: Good validation for claiming multiple coins.

The test case correctly verifies that claiming multiple coins updates the commission rewards and the staker's balance as expected.


461-535: Good validation for claiming one coin of multiple coins.

The test case correctly verifies that claiming one coin of multiple coins updates the commission rewards and the staker's balance as expected.


Line range hint 537-614: Good validation for claiming more rewards than available with multiple coins.

The test case correctly verifies that claiming more rewards than available with multiple coins results in an error and ensures the commission rewards remain unchanged.


616-689: Good validation for claiming a coin which does not exist.

The test case correctly verifies that claiming a coin which does not exist results in an error and ensures the commission rewards remain unchanged.

testutil/integration/checks.go (4)

169-169: Initialize expectedBalance as sdk.NewCoins().

The initialization of expectedBalance as sdk.NewCoins() is correct and aligns with the changes to handle multiple coins.


172-172: Update expectedBalance by adding staker.CommissionRewards.

The update to expectedBalance by adding staker.CommissionRewards using expectedBalance.Add(staker.CommissionRewards...) is correct and ensures that all commission rewards are accounted for.


176-176: Retrieve actualBalance using GetAllBalances.

The retrieval of actualBalance using suite.App().BankKeeper.GetAllBalances(suite.Ctx(), moduleAcc) is appropriate for handling multiple coins.


178-178: Compare actualBalance with expectedBalance.

The comparison of actualBalance with expectedBalance using Expect(actualBalance.String()).To(Equal(expectedBalance.String())) is correct and ensures the integrity of the stakers module assets.

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

476-476: LGTM! The change to access uploader.CommissionRewards using AmountOf(globaltypes.Denom).Uint64() aligns with the update to handle multiple coins for commission rewards.

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

184-184: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


277-277: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


370-370: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


476-476: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


583-583: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


690-690: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


795-795: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


902-902: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


1009-1009: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


1114-1114: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


1221-1221: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.


1328-1328: Update assertion to check specific denomination in uploader.CommissionRewards.

This change correctly updates the assertion to check the amount of a specific denomination in uploader.CommissionRewards.

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

248-248: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.


397-397: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.


409-409: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.


575-575: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.


754-754: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.


943-943: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.


1094-1094: Update assertion for CommissionRewards.

The change to use AmountOf(globalTypes.Denom).Uint64() for CommissionRewards is correct and consistent with the new structure.

docs/static/openapi.yml (6)

612-631: Changes to support multiple coins for commission_rewards look good.


3946-3965: Changes to support multiple coins for commission_rewards look good.


6323-6342: Changes to support multiple coins for commission_rewards look good.


6781-6800: Changes to support multiple coins for commission_rewards look good.


7345-7364: Changes to support multiple coins for commission_rewards look good.


7852-7871: Changes to support multiple coins for commission_rewards look good.

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: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between eb482c2 and 78cd57b.
Files selected for processing (1)
  • CHANGELOG.md (1 hunks)
Additional Context Used
LanguageTool (29)
CHANGELOG.md (29)

Near line 32: Unpaired symbol: ‘[’ seems to be missing
Context: ...ploader-role. ### Improvements - [#182](https://github.com/KYVENetwork/chain/pu...


Near line 43: Unpaired symbol: ‘[’ seems to be missing
Context: ... Add funders back to pool query. - [#163](https://github.com/KYVENetwork/chain/pu...


Near line 45: Unpaired symbol: ‘[’ seems to be missing
Context: ...YVE and Cosmos REST queries. ## [v1.4.0](https://github.com/KYVENetwork/chain/re...


Near line 50: Unpaired symbol: ‘[’ seems to be missing
Context: ...nhancing-kyves-funders-concept). - [#128](https://github.com/KYVENetwork/chain/pu...


Near line 59: Unpaired symbol: ‘[’ seems to be missing
Context: ...47.6-kyve-rc0)). ### Bug Fixes - [#149](https://github.com/KYVENetwork/chain/pu...


Near line 62: Unpaired symbol: ‘[’ seems to be missing
Context: ...gs amount for tx redelegate. ## [v1.3.2](https://github.com/KYVENetwork/chain/re...


Near line 73: Unpaired symbol: ‘[’ seems to be missing
Context: ...ap to genesis import/export. ## [v1.3.1](https://github.com/KYVENetwork/chain/re...


Near line 77: Unpaired symbol: ‘[’ seems to be missing
Context: ....1) - 2023-08-02 ### Bug Fixes - [#122](https://github.com/KYVENetwork/chain/pu...


Near line 79: Unpaired symbol: ‘[’ seems to be missing
Context: ...kefile go version parse cmd. ## [v1.3.0](https://github.com/KYVENetwork/chain/re...


Near line 96: Unpaired symbol: ‘[’ seems to be missing
Context: ...ds to be claimed. ### Bug Fixes - [#96](https://github.com/KYVENetwork/chain/pu...


Near line 100: Using many exclamation marks might seem excessive (in this case: 24 exclamation marks for a text that’s 11678 characters long)
Context: ...de auth module. ### Client Breaking - ! (x/stakers) [#46](https://github.com/...


Near line 108: Unpaired symbol: ‘[’ seems to be missing
Context: ...for finalized bundles query. ## [v1.2.3](https://github.com/KYVENetwork/chain/re...


Near line 115: Unpaired symbol: ‘[’ seems to be missing
Context: ...for finalized bundles query. ## [v1.2.2](https://github.com/KYVENetwork/chain/re...


Near line 121: Unpaired symbol: ‘[’ seems to be missing
Context: ...security-advisory-barberry). ## [v1.2.1](https://github.com/KYVENetwork/chain/re...


Near line 127: Unpaired symbol: ‘[’ seems to be missing
Context: ...urity-advisory-huckleberry). ## [v1.2.0](https://github.com/KYVENetwork/chain/re...


Near line 131: Unpaired symbol: ‘[’ seems to be missing
Context: ...2.0) - 2023-05-16 ### Bug Fixes - [#48](https://github.com/KYVENetwork/chain/pu...


Near line 134: Unpaired symbol: ‘[’ seems to be missing
Context: ...iple KYVE Core Team members. ## [v1.1.3](https://github.com/KYVENetwork/chain/re...


Near line 140: Unpaired symbol: ‘[’ seems to be missing
Context: ...urity-advisory-huckleberry). ## [v1.1.2](https://github.com/KYVENetwork/chain/re...


Near line 146: Unpaired symbol: ‘[’ seems to be missing
Context: ...t` when submitting a bundle. ## [v1.1.1](https://github.com/KYVENetwork/chain/re...


Near line 150: Unpaired symbol: ‘[’ seems to be missing
Context: ...) - 2023-05-08 ### Improvements - [#34](https://github.com/KYVENetwork/chain/pu...


Near line 152: Unpaired symbol: ‘[’ seems to be missing
Context: ...ve-ventures/interchaintest). ## [v1.1.0](https://github.com/KYVENetwork/chain/re...


Near line 156: Unpaired symbol: ‘[’ seems to be missing
Context: ...) - 2023-04-18 ### Improvements - [#22](https://github.com/KYVENetwork/chain/pu...


Near line 163: Unpaired symbol: ‘[’ seems to be missing
Context: ...received via IBC. ### Bug Fixes - [#20](https://github.com/KYVENetwork/chain/pu...


Near line 172: Unpaired symbol: ‘[’ seems to be missing
Context: ...tadata fields. ### API Breaking - [#22](https://github.com/KYVENetwork/chain/pu...


Near line 186: Unpaired symbol: ‘[’ seems to be missing
Context: ... fields, deprecating Logo. ## [v1.0.1](https://github.com/KYVENetwork/chain/re...


Near line 190: Unpaired symbol: ‘[’ seems to be missing
Context: ...) - 2023-05-08 ### Improvements - [#34](https://github.com/KYVENetwork/chain/pu...


Near line 192: Unpaired symbol: ‘[’ seems to be missing
Context: ...ve-ventures/interchaintest). ## [v1.0.0](https://github.com/KYVENetwork/chain/re...


Near line 196: Unpaired symbol: ‘[’ seems to be missing
Context: ...the KYVE network launch. ## [v1.0.0-rc1](https://github.com/KYVENetwork/chain/re...


Near line 211: Unpaired symbol: ‘[’ seems to be missing
Context: ...param to type sdk.Dec. ## [v1.0.0-rc0](https://github.com/KYVENetwork/chain/re...

Additional comments not posted (17)
CHANGELOG.md (17)

21-21: New feature entry for supporting multiple coins in the stakers module looks good.


22-22: New feature entry for supporting multiple coins in the bundles module looks good.


23-23: New feature entry for supporting multiple coins for funding looks good.


24-24: New feature entry for adding an end-key to the pool looks good.


Line range hint 25-25: New feature entry for tallying bundle proposals on timeout looks good.


Line range hint 26-26: New feature entry for supporting multiple denoms for delegation rewards looks good.


Line range hint 27-27: New feature entry for storage-cost per storage-provider looks good.


Line range hint 28-28: New feature entry for tallying bundle proposals on skip-uploader-role looks good.


Line range hint 30-30: Improvement entry for making release builds reproducible looks good.


Line range hint 31-31: Improvement entry for only charging coins which are whitelisted looks good.


Line range hint 32-32: Improvement entry for adding the mainnet KYVE image to interchain tests looks good.


Line range hint 33-33: Improvement entry for bumping the IBC version to v8.2.0 and removing an unnecessary dev-dependency looks good.


Line range hint 34-34: Improvement entry for migrating to Cosmos-SDK v0.50 looks good.


Line range hint 35-35: Improvement entry for bumping the IBC packet forward middleware looks good.


Line range hint 37-37: Bug fix entry for adding pool params back to the KYVE params query looks good.


Line range hint 38-38: Bug fix entry for adding funders back to the pool query looks good.


Line range hint 39-39: Bug fix entry for adding back the swagger UI for KYVE and Cosmos REST queries looks good.

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: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 78cd57b and f0696e9.
Files selected for processing (6)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go (12 hunks)
  • x/bundles/keeper/logic_bundles.go (1 hunks)
  • x/bundles/types/expected_keepers.go (1 hunks)
  • x/stakers/keeper/logic_stakers.go (3 hunks)
  • x/stakers/keeper/msg_server_claim_commission_rewards_test.go (6 hunks)
  • x/stakers/types/message_claim_commission_rewards.go (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • x/bundles/keeper/keeper_suite_inflation_splitting_test.go
  • x/bundles/keeper/logic_bundles.go
  • x/bundles/types/expected_keepers.go
  • x/stakers/keeper/logic_stakers.go
  • x/stakers/types/message_claim_commission_rewards.go
Additional comments not posted (11)
x/stakers/keeper/msg_server_claim_commission_rewards_test.go (11)

199-211: The test case "Produce a valid bundle and check commission rewards" correctly verifies the commission rewards for multiple coins. The changes look good.


221-238: The test case "Claim with non-staker account" correctly handles the scenario where a non-staker account attempts to claim rewards. The changes look good.


242-260: The test case "Claim more rewards than available" correctly handles the scenario where more rewards than available are claimed. The changes look good.


264-282: The test case "Claim zero rewards" correctly handles the scenario where zero rewards are claimed. The changes look good.


284-299: The test case "Claim partial rewards" correctly handles the scenario where partial rewards are claimed. The changes look good.


302-371: The test case "Claim partial rewards twice" correctly handles the scenario where partial rewards are claimed twice. The changes look good.


373-388: The test case "Claim all rewards" correctly handles the scenario where all rewards are claimed. The changes look good.


391-462: The test case "Claim multiple coins" correctly handles the scenario where multiple coins are claimed. The changes look good.


464-538: The test case "Claim one coin of multiple coins" correctly handles the scenario where one coin out of multiple coins is claimed. The changes look good.


Line range hint 540-617: The test case "Claim more rewards than available with multiple coins" correctly handles the scenario where more rewards than available with multiple coins are claimed. The changes look good.


619-692: The test case "Claim coin which does not exist" correctly handles the scenario where a coin that does not exist is claimed. The changes look good.

@troykessler troykessler merged commit 1303bc2 into main May 14, 2024
5 checks passed
@troykessler troykessler deleted the troy/stakers-multiple-coins branch May 14, 2024 12:00
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.

2 participants