feat(x/protocolpool,x/distribution)!: remove dependency + fix continuous fund bug #20790
Conversation
WalkthroughWalkthroughThe recent updates involve significant modifications to the distribution handling in 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 Configuration File (
|
github-actions
bot
added
C:x/distribution
facundomedica
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…/removepooldep-distr
…-sdk into facu/removepooldep-distr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (2)
simapp/v2/app_config.go (1)
80-80: Add a comment to explain the purpose ofProtocolPoolDistrAccount.While the addition of
pooltypes.ProtocolPoolDistrAccountis clear, adding a comment explaining its role would improve code readability and maintainability.{Account: pooltypes.StreamAccount}, + // Account for protocol pool distribution {Account: pooltypes.ProtocolPoolDistrAccount},simapp/app_config.go (1)
82-82: Add a comment to explain the purpose ofProtocolPoolDistrAccount.While the addition of
pooltypes.ProtocolPoolDistrAccountis clear, adding a comment explaining its role would improve code readability and maintainability.{Account: pooltypes.StreamAccount}, + // Account for protocol pool distribution {Account: pooltypes.ProtocolPoolDistrAccount},
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
x/distribution/go.sumis excluded by!**/*.sum
Files selected for processing (39)
- scripts/mockgen.sh (1 hunks)
- simapp/app.go (4 hunks)
- simapp/app_config.go (2 hunks)
- simapp/v2/app_config.go (2 hunks)
- tests/integration/distribution/keeper/msg_server_test.go (2 hunks)
- tests/integration/gov/keeper/keeper_test.go (1 hunks)
- tests/integration/protocolpool/module_test.go (2 hunks)
- testutil/configurator/configurator.go (2 hunks)
- x/distribution/CHANGELOG.md (2 hunks)
- x/distribution/depinject.go (2 hunks)
- x/distribution/go.mod (1 hunks)
- x/distribution/keeper/allocation.go (2 hunks)
- x/distribution/keeper/allocation_test.go (8 hunks)
- x/distribution/keeper/delegation_test.go (18 hunks)
- x/distribution/keeper/grpc_query.go (1 hunks)
- x/distribution/keeper/grpc_query_test.go (2 hunks)
- x/distribution/keeper/keeper.go (3 hunks)
- x/distribution/keeper/keeper_test.go (4 hunks)
- x/distribution/keeper/migrations.go (1 hunks)
- x/distribution/keeper/msg_server.go (2 hunks)
- x/distribution/keeper/msg_server_test.go (2 hunks)
- x/distribution/migrations/v4/migrate_funds_test.go (2 hunks)
- x/distribution/module.go (1 hunks)
- x/distribution/testutil/expected_keepers_mocks.go (1 hunks)
- x/distribution/types/expected_keepers.go (1 hunks)
- x/distribution/types/keys.go (1 hunks)
- x/gov/testutil/expected_keepers.go (1 hunks)
- x/gov/testutil/expected_keepers_mocks.go (1 hunks)
- x/gov/types/expected_keepers.go (1 hunks)
- x/protocolpool/CHANGELOG.md (1 hunks)
- x/protocolpool/keeper/genesis.go (2 hunks)
- x/protocolpool/keeper/genesis_test.go (1 hunks)
- x/protocolpool/keeper/keeper.go (7 hunks)
- x/protocolpool/keeper/keeper_test.go (5 hunks)
- x/protocolpool/keeper/msg_server.go (3 hunks)
- x/protocolpool/keeper/msg_server_test.go (12 hunks)
- x/protocolpool/module.go (2 hunks)
- x/protocolpool/types/errors.go (1 hunks)
- x/protocolpool/types/keys.go (2 hunks)
Files skipped from review due to trivial changes (8)
- scripts/mockgen.sh
- tests/integration/protocolpool/module_test.go
- x/distribution/go.mod
- x/distribution/keeper/delegation_test.go
- x/distribution/migrations/v4/migrate_funds_test.go
- x/distribution/module.go
- x/distribution/testutil/expected_keepers_mocks.go
- x/protocolpool/types/errors.go
Additional context used
Path-based instructions (31)
x/protocolpool/CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"x/protocolpool/types/keys.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/protocolpool/keeper/genesis_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/distribution/keeper/migrations.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/testutil/expected_keepers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/types/expected_keepers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/gov/types/expected_keepers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/protocolpool/keeper/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/protocolpool/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/types/keys.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/keeper/allocation.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/keeper/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/distribution/keeper/grpc_query_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/distribution/CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"tests/integration/gov/keeper/keeper_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/protocolpool/keeper/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/distribution/keeper/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/protocolpool/keeper/msg_server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/v2/app_config.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/app_config.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/keeper/msg_server_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/distribution/keeper/grpc_query.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.testutil/configurator/configurator.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/distribution/keeper/allocation_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/gov/testutil/expected_keepers_mocks.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/protocolpool/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/integration/distribution/keeper/msg_server_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/protocolpool/keeper/msg_server_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Learnings (3)
x/protocolpool/keeper/genesis_test.go (1)
Learnt from: likhita-809 PR: cosmos/cosmos-sdk#18471 File: x/protocolpool/keeper/genesis.go:12-51 Timestamp: 2023-11-22T12:32:39.368Z Learning: - The user `likhita-809` has confirmed the changes suggested in the previous interaction. - The file in question is `x/protocolpool/keeper/genesis.go` from a Cosmos SDK module. - The changes involve optimizing the `InitGenesis` function by removing redundant code and ensuring proper handling of start times for budget proposals.x/protocolpool/keeper/genesis.go (1)
Learnt from: likhita-809 PR: cosmos/cosmos-sdk#18471 File: x/protocolpool/keeper/genesis.go:12-51 Timestamp: 2023-11-22T12:32:39.368Z Learning: - The user `likhita-809` has confirmed the changes suggested in the previous interaction. - The file in question is `x/protocolpool/keeper/genesis.go` from a Cosmos SDK module. - The changes involve optimizing the `InitGenesis` function by removing redundant code and ensuring proper handling of start times for budget proposals.x/distribution/keeper/allocation.go (1)
Learnt from: julienrbrt PR: cosmos/cosmos-sdk#18539 File: x/distribution/keeper/allocation.go:26-42 Timestamp: 2023-11-22T14:50:39.724Z Learning: User clarified that the addition of fees to the DecimalPool is handled in a different part of the code when `totalPreviousPower` is zero.
GitHub Check: CodeQL
x/protocolpool/module.go
[warning] 118-118: Panic in BeginBock or EndBlock consensus methods
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic callx/protocolpool/keeper/keeper.go
[warning] 481-481: Panic in BeginBock or EndBlock consensus methods
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
Additional comments not posted (68)
x/protocolpool/CHANGELOG.md (1)
28-30: Changelog entry is clear and follows the guidelines.The entry correctly mentions the improvement and provides a reference to the GitHub pull request.
x/protocolpool/types/keys.go (1)
32-33: New constants are correctly defined.The new constants
DistributionsKeyandLastBalanceKeyfollow the naming conventions and are properly defined.x/protocolpool/keeper/genesis_test.go (1)
34-35: Mocks are correctly set up for testing.The mocks for
GetModuleAccountandGetAllBalancesare correctly set up to simulate the expected behavior.x/distribution/keeper/migrations.go (1)
45-45: Ensure proper error handling during fund migration.The function correctly retrieves the module account and migrates the funds, but ensure that error handling is thorough and covers all edge cases.
x/distribution/depinject.go (1)
77-77: Refactored to remove PoolKeeper dependency.The
ProvideModulefunction has been correctly refactored to remove thePoolKeeperdependency and update theNewAppModulecall.x/gov/testutil/expected_keepers.go (1)
46-46: Removed PoolKeeper interface.The
PoolKeeperinterface has been correctly removed in alignment with the PR objectives.x/gov/types/expected_keepers.go (1)
54-54: Removed PoolKeeper interface.The
PoolKeeperinterface has been correctly removed in alignment with the PR objectives.x/protocolpool/keeper/genesis.go (2)
52-52: LGTM! Code changes approved.The addition of setting distributions in the genesis state is correct.
99-108: LGTM! Code changes approved.The addition of exporting balances and setting the amount to distribute is correct.
x/protocolpool/module.go (2)
Line range hint
87-95: LGTM! Code changes approved.The
InitGenesismethod correctly initializes the genesis state using the keeper.
116-119: Verify potential panic calls inBeginBlocker.Static analysis tools indicate potential panic calls within the
BeginBlockermethod. Ensure that theBeginBlockermethod handles errors gracefully and does not cause a panic.Verification successful
No panic calls found in
BeginBlockermethods.The
BeginBlockermethods inx/slashing/abci.goandx/authz/module/abci.godo not contain anypaniccalls, indicating that they handle errors gracefully.
x/slashing/abci.gox/authz/module/abci.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify potential panic calls within the BeginBlocker method. # Test: Search for panic calls within BeginBlocker. Expect: No panic calls. rg --type go -A 5 $'func BeginBlocker'Length of output: 907
Script:
#!/bin/bash # Description: Verify potential panic calls within the full body of the BeginBlocker methods. # Search for panic calls within the BeginBlocker method in x/slashing/abci.go rg --type go -A 50 'func BeginBlocker' x/slashing/abci.go | rg 'panic' # Search for panic calls within the BeginBlocker method in x/authz/module/abci.go rg --type go -A 50 'func BeginBlocker' x/authz/module/abci.go | rg 'panic'Length of output: 145
Tools
GitHub Check: CodeQL
[warning] 118-118: Panic in BeginBock or EndBlock consensus methods
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic callx/distribution/types/keys.go (2)
27-30: LGTM! Code changes approved.The addition of the
ProtocolPoolDistrAccountconstant and its documentation is correct.
31-32: LGTM! Code changes approved.The addition of the
ProtocolPoolModuleNameconstant and its documentation is correct.x/distribution/keeper/allocation.go (2)
82-82: LGTM! Code changes approved.The addition of sending tokens to the
ProtocolPoolDistrAccountis correct.
168-168: LGTM! Code changes approved.The addition of sending tokens to the
ProtocolPoolDistrAccountis correct.x/distribution/keeper/grpc_query_test.go (1)
151-155: LGTM!The changes correctly mock the necessary methods to test the community pool query.
x/distribution/CHANGELOG.md (2)
34-35: LGTM!The improvement is correctly documented.
Line range hint
38-59:
LGTM!The breaking changes are correctly documented.
Tools
LanguageTool
[style] ~39-~39: Consider a shorter alternative to avoid wordiness.
Context: ...x/distributionnow takes cometService in order to get consensus related information. * [#...(IN_ORDER_TO_PREMIUM)
Markdownlint
41-41: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
tests/integration/gov/keeper/keeper_test.go (1)
70-76: LGTM!The changes correctly add the new account to the permissions map.
x/protocolpool/keeper/keeper_test.go (4)
30-32: Correct initialization ofProtocolPoolDistrAccount.The initialization of
ProtocolPoolDistrAccountas a module account looks correct.
61-61: Correct setup of mock expectations forProtocolPoolDistrAccount.The mock expectations for
ProtocolPoolDistrAccountare correctly added.
107-114: Correct setup of mock expectations inmockStreamFunds.The mock expectations for
ProtocolPoolDistrAccountin themockStreamFundsfunction are correctly added.
125-129: Correct setup of mock expectations inTestIterateAndUpdateFundsDistribution.The mock expectations for
ProtocolPoolDistrAccountin theTestIterateAndUpdateFundsDistributionfunction are correctly added.x/distribution/keeper/msg_server.go (2)
117-117: Correct update ofFundCommunityPoolmethod call.The method call in the
FundCommunityPoolfunction is correctly updated to useProtocolPoolModuleName.
161-161: Correct update ofCommunityPoolSpendmethod call.The method call in the
CommunityPoolSpendfunction is correctly updated to useProtocolPoolModuleName.x/protocolpool/keeper/msg_server.go (3)
142-146: Correct distribution of funds before creating continuous funds.The code correctly distributes funds before creating continuous funds to avoid giving the new fund more than it should get.
169-178: Correct distribution of funds before withdrawing continuous funds.The code correctly distributes funds before withdrawing continuous funds to ensure accurate fund allocation.
201-202: Correct distribution of funds before canceling continuous funds.The code correctly distributes funds before canceling continuous funds to ensure accurate fund allocation.
simapp/v2/app_config.go (1)
119-119: EnsureBeginBlockersare ordered correctly.The addition of
pooltypes.ModuleNameto theBeginBlockerslist is correct. Ensure the order aligns with the intended execution sequence.simapp/app_config.go (1)
121-121: EnsureBeginBlockersare ordered correctly.The addition of
pooltypes.ModuleNameto theBeginBlockerslist is correct. Ensure the order aligns with the intended execution sequence.x/distribution/keeper/msg_server_test.go (2)
184-185: Verify the correctness of theSendCoinsFromAccountToModulecall.The expectation for
SendCoinsFromAccountToModulealigns with the new interaction model. Ensure that the method call and parameters are correct.
292-293: Verify the correctness of theSendCoinsFromModuleToAccountcall.The expectation for
SendCoinsFromModuleToAccountaligns with the new interaction model. Ensure that the method call and parameters are correct.x/distribution/keeper/grpc_query.go (1)
376-382: Ensure theProtocolPoolModuleNameis correctly defined and used.The changes correctly replace
poolKeeperwithauthKeeperandbankKeeper. Verify thattypes.ProtocolPoolModuleNameis defined and used consistently.testutil/configurator/configurator.go (2)
53-53: Addition ofProtocolPoolModuleNameto BeginBlockersOrderThe addition of
testutil.ProtocolPoolModuleNameto theBeginBlockersOrderensures that theProtocolPoolmodule'sBeginBlockoperations are executed. This aligns with the PR objective of making thex/protocolpoolmodule independent and capable of handling its own fund distribution.
168-168: Addition ofprotocolpool_distrmodule accountThe addition of the
protocolpool_distrmodule account is necessary for the new intermediary account setup described in the PR objectives. This change supports the new fund distribution mechanism.x/distribution/keeper/allocation_test.go (2)
199-199: Update inTestAllocateTokensToManyValidatorsto useProtocolPoolDistrAccountThe change ensures that the distribution logic now correctly sends community pool coins to the new intermediary account (
disttypes.ProtocolPoolDistrAccount). This update is in line with the PR objective to decouplex/distributionfromx/protocolpool.However, ensure that the new intermediary account logic is thoroughly tested.
Verification successful
We need to verify the context in which
ProtocolPoolDistrAccountis being tested in the relevant test files. Specifically, let's look at the tests inx/protocolpool/keeper/keeper_test.goandx/distribution/keeper/allocation_test.go.
Verification Successful: Thorough Testing of
ProtocolPoolDistrAccountThe intermediary account logic for
ProtocolPoolDistrAccountis thoroughly tested in the codebase. The extracted test contexts fromx/protocolpool/keeper/keeper_test.goandx/distribution/keeper/allocation_test.goconfirm that various interactions involvingProtocolPoolDistrAccountare covered.
x/protocolpool/keeper/keeper_test.go: Tests interactions with account keeper and bank keeper.x/distribution/keeper/allocation_test.go: Tests token allocation involving this account.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the intermediary account logic is thoroughly tested. # Test: Search for test cases using `disttypes.ProtocolPoolDistrAccount`. rg --type go 'ProtocolPoolDistrAccount'Length of output: 3504
Script:
#!/bin/bash # Extract lines around the references to ProtocolPoolDistrAccount in test files for deeper context. # Extracting from keeper_test.go rg --type go --context 10 'ProtocolPoolDistrAccount' x/protocolpool/keeper/keeper_test.go # Extracting from allocation_test.go rg --type go --context 10 'ProtocolPoolDistrAccount' x/distribution/keeper/allocation_test.goLength of output: 6391
350-350: Update inTestAllocateTokensTruncationto useProtocolPoolDistrAccountThis line ensures that the allocation logic for truncation tests now correctly sends coins to the new intermediary account (
disttypes.ProtocolPoolDistrAccount). This update aligns with the decoupling ofx/distributionfromx/protocolpool.Ensure that the truncation logic is thoroughly tested with the new intermediary account.
Verification successful
Verification Successful: Truncation Logic Tested with ProtocolPoolDistrAccount
The test case
TestAllocateTokensTruncationinx/distribution/keeper/allocation_test.gocorrectly includes the usage ofProtocolPoolDistrAccount, ensuring that the truncation logic is thoroughly tested with the new intermediary account.
x/distribution/keeper/allocation_test.go: TestAllocateTokensTruncationScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the truncation logic with the intermediary account is thoroughly tested. # Test: Search for test cases using `disttypes.ProtocolPoolDistrAccount`. rg --type go 'ProtocolPoolDistrAccount'Length of output: 3504
Script:
#!/bin/bash # Description: Verify the truncation logic in TestAllocateTokensTruncation with the intermediary account. # Test: Search for the specific test case `TestAllocateTokensTruncation` and inspect its usage of `ProtocolPoolDistrAccount`. rg --type go 'TestAllocateTokensTruncation' -A 20Length of output: 2078
Script:
#!/bin/bash # Description: Verify the truncation logic in TestAllocateTokensTruncation with the intermediary account. # Test: Search for the specific test case `TestAllocateTokensTruncation` and inspect its usage of `ProtocolPoolDistrAccount` with more context. rg --type go 'TestAllocateTokensTruncation' -A 100Length of output: 8709
Script:
#!/bin/bash # Description: Verify the truncation logic in TestAllocateTokensTruncation with the intermediary account. # Test: Search for the specific test case `TestAllocateTokensTruncation` and inspect its usage of `ProtocolPoolDistrAccount` with a larger context. rg --type go 'TestAllocateTokensTruncation' -A 200Length of output: 11162
x/gov/testutil/expected_keepers_mocks.go (1)
293-293: Removal ofFundCommunityPoolmethod fromMockPoolKeeperThe removal of the
FundCommunityPoolmethod from theMockPoolKeeperaligns with the PR's objective to decouplex/distributionfromx/protocolpool. This change ensures that the distribution module no longer relies on the pool keeper for community pool funding.x/protocolpool/keeper/keeper.go (6)
38-39: Addition ofDistributionsandLastBalancecollectionsThe addition of these collections is essential for tracking distributions and the last balance in the protocol pool module. This change supports the new fund distribution mechanism described in the PR objectives.
52-55: Ensure module accounts are setThe additional checks ensure that the module accounts for
types.ModuleName,types.StreamAccount, andtypes.ProtocolPoolDistrAccountare set. This is crucial for the proper functioning of the protocol pool module.
69-70: Initialization ofDistributionsandLastBalancecollectionsThe initialization of these collections ensures they are properly set up in the keeper. This is necessary for the new distribution logic to function correctly.
143-181: Implementation ofSetToDistributemethodThe
SetToDistributemethod calculates the amount to be distributed among recipients and updates the last balance. This method is crucial for the new fund distribution mechanism described in the PR objectives.
185-300: Implementation ofIterateAndUpdateFundsDistributionmethodThe
IterateAndUpdateFundsDistributionmethod iterates over continuous funds and distributions, calculates each recipient's share, and updates the recipient fund distribution. This method is essential for the new continuous fund distribution logic described in the PR objectives.
480-482: Implementation ofBeginBlockermethodThe
BeginBlockermethod callsSetToDistributeto set the amount to be distributed at the beginning of each block. This aligns with the new fund distribution mechanism described in the PR objectives.However, ensure that the
BeginBlockermethod does not lead to any panics, as indicated by the CodeQL warning.Tools
GitHub Check: CodeQL
[warning] 481-481: Panic in BeginBock or EndBlock consensus methods
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic callsimapp/app.go (4)
126-135: Addition ofprotocolpoolrelated accounts inmaccPermsThese lines add the
ProtocolPoolDistrAccountalong with other necessary accounts to themaccPermsmap. This ensures that the module accounts are correctly set up with the required permissions.
357-357: UpdateDistrKeeperinitializationThis line removes the
PoolKeeperfrom theDistrKeeperinitialization, reflecting the decoupling of thex/distributionmodule from thex/protocolpoolmodule. The change appears correct and aligns with the PR objective.
450-450: Update module manager withDistrKeeperThis line updates the module manager to reflect the new
DistrKeeperinitialization without thePoolKeeper. This change aligns with the decoupling objective.
476-476: UpdateSetOrderBeginBlockerswithpooltypes.ModuleNameThis line ensures that the
BeginBlockerfor theprotocolpoolmodule is correctly set in the module manager. This is necessary for theprotocolpoolmodule to function independently.tests/integration/distribution/keeper/msg_server_test.go (3)
89-94: Addition ofprotocolpoolrelated accounts inmaccPermsThese lines add
ProtocolPoolDistrAccountalong with other necessary accounts to themaccPermsmap in the test setup. This ensures that the module accounts are correctly set up with the required permissions for testing.
141-141: UpdatedistrKeeperinitializationThis line removes the
PoolKeeperfrom thedistrKeeperinitialization in the test setup, reflecting the decoupling of thex/distributionmodule from thex/protocolpoolmodule. The change appears correct and aligns with the PR objective.
147-147: Update module manager withDistrKeeperThis line updates the module manager to reflect the new
distrKeeperinitialization without thePoolKeeper. This change aligns with the decoupling objective and ensures the tests are set up correctly.x/protocolpool/keeper/msg_server_test.go (17)
6-7: LGTM! Ensure test cases are comprehensive.The new imports are valid and necessary for the added functionality.
408-408: LGTM! Ensure test cases are comprehensive.The test case for "recipient with no continuous fund" is valid.
446-446: LGTM! Ensure test cases are comprehensive.The test case for setting distributions is valid.
453-453: LGTM! Ensure test cases are comprehensive.The test case for expired funds with no funds left to withdraw is valid.
488-490: LGTM! Ensure test cases are comprehensive.The test case for valid withdrawal with zero distribution amount is valid.
510-511: LGTM! Ensure test cases are comprehensive.The test case for valid withdrawal with non-zero distribution amount is valid.
535-536: LGTM! Ensure test cases are comprehensive.The test case for valid withdrawal with multiple continuous funds is valid.
592-593: LGTM! Ensure test cases are comprehensive.The test case for setting distributions with multiple continuous funds is valid.
851-852: LGTM! Ensure test cases are comprehensive.The test case for setting distributions is valid.
957-958: LGTM! Ensure test cases are comprehensive.The test case for streaming funds is valid.
969-973: LGTM! Ensure test cases are comprehensive.The test case for repeated withdrawal attempts after expiration is valid.
978-979: LGTM! Ensure test cases are comprehensive.The test case for setting distributions after multiple withdrawals is valid.
987-992: LGTM! Ensure test cases are comprehensive.The test case for canceling a continuous fund is valid.
994-1000: LGTM! Ensure test cases are comprehensive.The test case for canceling an expired continuous fund is valid.
1001-1008: LGTM! Ensure test cases are comprehensive.The test case for repeated cancellation attempts of the same continuous fund is valid.
1010-1024: LGTM! Ensure test case is comprehensive.The test case for funding the community pool is valid.
1026-1040: LGTM! Ensure test case is comprehensive.The test case for spending from the community pool is valid.
| @@ -78,7 +75,7 @@ func initFixture(t *testing.T) (sdk.Context, []sdk.AccAddress, keeper.Keeper, de | |||
| params := types.DefaultParams() | |||
| require.NoError(t, distrKeeper.Params.Set(ctx, params)) | |||
|
|
|||
| return ctx, addrs, distrKeeper, dep{bankKeeper, stakingKeeper, accountKeeper, poolKeeper} | |||
| return ctx, addrs, distrKeeper, dep{bankKeeper, stakingKeeper, accountKeeper} | |||
There was a problem hiding this comment.
Missing distrAcc dependency.
The distrAcc variable is not defined, causing the GetModuleAddress call to fail.
- accountKeeper.EXPECT().GetModuleAddress("distribution").Return(distrAcc.GetAddress())
+ distrAcc := authtypes.NewEmptyModuleAccount("distribution")
+ accountKeeper.EXPECT().GetModuleAddress("distribution").Return(distrAcc.GetAddress())Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ctx, addrs, distrKeeper, dep{bankKeeper, stakingKeeper, accountKeeper} | |
| distrAcc := authtypes.NewEmptyModuleAccount("distribution") | |
| accountKeeper.EXPECT().GetModuleAddress("distribution").Return(distrAcc.GetAddress()) | |
| return ctx, addrs, distrKeeper, dep{bankKeeper, stakingKeeper, accountKeeper} |
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "sort" |
There was a problem hiding this comment.
Ensure proper error wrapping
The errors package is imported but not used in the provided code. Ensure that all errors are wrapped correctly using errorsmod.Wrapf or similar functions for consistent error handling.
- import (
- "errors"
- )Committable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "sort" | |
| "sort" |
x/protocolpool/keeper/genesis.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| genState.ToDistribute = balances.AmountOf(bondDenom) | ||
|
|
||
| // TODO: fix export by adding all the funds to be distributed |
There was a problem hiding this comment.
will this be completed in a follow up pr?
There was a problem hiding this comment.
Just added it here: 4dc77b1#diff-443d2bd72ab97253021289109c54b93f9e87f182481df4a6cdfa5a18fff20ae0
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
api/cosmos/staking/v1beta1/staking.pulsar.go (2)
15064-15064: Deprecated function needs documentation on alternatives.The function
GetHistoricalEntriesis marked as deprecated. It would be beneficial to provide information on what should be used instead, if applicable.// Deprecated: Do not use. Use [alternative method] instead. func (x *Params) GetHistoricalEntries() uint32 { if x != nil { return x.HistoricalEntries } }
15011-15012: Clarify the deprecation ofHistoricalEntriesand provide migration guidance.The
HistoricalEntriesfield is marked as deprecated, but there is no documentation or comments regarding the migration plans or alternatives. It is essential to provide clear guidance to developers on how to handle this deprecation to avoid potential issues.
- File:
x/staking/types/staking.pb.go- Line: 4
- Code:
HistoricalEntries uint32protobuf:"varint,4,opt,name=historical_entries,json=historicalEntries,proto3" json:"historical_entries,omitempty"// Deprecated: Do not use.Please add documentation or comments to clarify the deprecation and provide migration guidance.
Analysis chain
Clarify the use of the
HistoricalEntriesfield.The
HistoricalEntriesfield is marked as deprecated but still present in the protobuf definition. It's important to ensure that deprecating this does not affect existing systems that might rely on this field.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of `HistoricalEntries` to ensure it's not actively used in critical paths. rg --type go "HistoricalEntries"Length of output: 4405
Script:
#!/bin/bash # Description: Search for any comments or documentation regarding the deprecation and migration plans for `HistoricalEntries`. rg --type go "HistoricalEntries" -A 5 | grep -i "deprecat\|migrat\|alternat"Length of output: 275
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
x/protocolpool/types/genesis.pb.gois excluded by!**/*.pb.gox/staking/types/staking.pb.gois excluded by!**/*.pb.go
Files selected for processing (7)
- api/cosmos/protocolpool/v1/genesis.pulsar.go (15 hunks)
- api/cosmos/staking/v1beta1/staking.pulsar.go (4 hunks)
- x/protocolpool/keeper/genesis.go (3 hunks)
- x/protocolpool/keeper/genesis_test.go (1 hunks)
- x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto (2 hunks)
- x/protocolpool/types/genesis.go (1 hunks)
- x/staking/proto/cosmos/staking/v1beta1/staking.proto (2 hunks)
Files not summarized due to errors (2)
- api/cosmos/protocolpool/v1/genesis.pulsar.go: Error: Message exceeds token limit
- api/cosmos/staking/v1beta1/staking.pulsar.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- x/staking/proto/cosmos/staking/v1beta1/staking.proto
Additional context used
Path-based instructions (5)
x/protocolpool/keeper/genesis_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/protocolpool/types/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/protocolpool/keeper/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/staking/v1beta1/staking.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Learnings (2)
x/protocolpool/keeper/genesis_test.go (1)
Learnt from: likhita-809 PR: cosmos/cosmos-sdk#18471 File: x/protocolpool/keeper/genesis.go:12-51 Timestamp: 2023-11-22T12:32:39.368Z Learning: - The user `likhita-809` has confirmed the changes suggested in the previous interaction. - The file in question is `x/protocolpool/keeper/genesis.go` from a Cosmos SDK module. - The changes involve optimizing the `InitGenesis` function by removing redundant code and ensuring proper handling of start times for budget proposals.x/protocolpool/keeper/genesis.go (1)
Learnt from: likhita-809 PR: cosmos/cosmos-sdk#18471 File: x/protocolpool/keeper/genesis.go:12-51 Timestamp: 2023-11-22T12:32:39.368Z Learning: - The user `likhita-809` has confirmed the changes suggested in the previous interaction. - The file in question is `x/protocolpool/keeper/genesis.go` from a Cosmos SDK module. - The changes involve optimizing the `InitGenesis` function by removing redundant code and ensuring proper handling of start times for budget proposals.
Additional comments not posted (16)
x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto (2)
9-9: Approved addition of import.The import of
google/protobuf/timestamp.protois necessary for using theTimestamptype in the newDistributionmessage. This is a standard practice when dealing with time-related data in protobuf.
32-40: Approved new message typeDistribution.The new message type
Distributionis correctly defined with appropriate fields and protobuf options. This change aligns with the PR's objective to handle distributions more explicitly.x/protocolpool/keeper/genesis_test.go (3)
34-37: Approved addition of distribution setting in tests.The addition of distribution setting in the
TestInitGenesisfunction is correct and necessary to test the new functionality introduced in the genesis state handling.
40-41: Error handling check approved.The error handling for the condition "total to be distributed is greater than the last balance" is crucial and properly implemented in the test. This ensures that the system behaves as expected under erroneous conditions.
52-52: Approved verification of exported genesis state values.Verifying the exported genesis state values in the test is essential for ensuring that the state is correctly managed and exported. This change is well-implemented.
x/protocolpool/types/genesis.go (1)
16-17: Approved initialization of new fields inNewGenesisState.The initialization of
LastBalanceto zero andDistributionsto an empty slice is appropriate and aligns with the changes in the genesis structure. This ensures that the genesis state starts with a consistent and expected state.x/protocolpool/keeper/genesis.go (2)
54-68: Approved changes in distribution setting and error handling inInitGenesis.The modifications to set distributions and the added error handling for distribution totals exceeding the last balance are correctly implemented. This ensures that the system maintains integrity and prevents incorrect state mutations.
114-126: Approved changes inExportGenesisfunction.The changes to calculate the
ToDistributestate based on account balances and to walk through distributions are correct. This ensures that the exported genesis state is accurate and reflects the current state of the system.api/cosmos/protocolpool/v1/genesis.pulsar.go (6)
265-270: Confirm handling ofLastBalancefield.The handling of the
LastBalancefield should be checked to ensure it correctly reflects the state of funds yet to be distributed. This is crucial for the correct operation of the module following the PR changes.Verification successful
Handling of
LastBalancefield is correct.The
LastBalancefield is consistently checked, set, retrieved, encoded, and decoded throughout the code. It correctly reflects the state of undistributed funds as specified in the struct definition.
- The field is checked for non-empty values before processing.
- It is properly set and retrieved using appropriate methods.
- Encoding and decoding logic is correctly implemented.
- The struct definition clearly states its purpose.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check handling of LastBalance field to ensure it correctly reflects the state of undistributed funds. rg --type go 'LastBalance' -A 5 -B 5 api/cosmos/protocolpool/v1/genesis.pulsar.goLength of output: 4996
271-273: Review usage ofDistributionslist inRangemethod.The
Rangemethod's handling of theDistributionslist should be scrutinized to ensure it correctly processes each distribution, particularly given the new structure of the list.Verification successful
The
Rangemethod correctly processes theDistributionslist.The method checks if the
Distributionslist is non-empty and processes it appropriately, similar to other fields.
- The
Distributionslist is handled correctly within theRangemethod.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Review the Range method's handling of the Distributions list to ensure correct processing. rg --type go 'func \(x \*fastReflection_GenesisState\) Range' -A 10 -B 10 api/cosmos/protocolpool/v1/genesis.pulsar.goLength of output: 1000
Script:
#!/bin/bash # Review the complete implementation of the Range method in the genesis.pulsar.go file. rg --type go 'func \(x \*fastReflection_GenesisState\) Range' -A 50 -B 10 api/cosmos/protocolpool/v1/genesis.pulsar.goLength of output: 2523
320-323: Ensure proper clearing ofdistributionsfield.The
Clearmethod must properly handle thedistributionsfield to ensure that it can be reset correctly without leaving residual data that could affect application state.Verification successful
The
Clearmethod properly handles thedistributionsfield.The
Clearmethod includes a case for "cosmos.protocolpool.v1.GenesisState.distributions" where it setsx.Distributionstonil, ensuring the field is correctly reset.
api/cosmos/protocolpool/v1/genesis.pulsar.go, lines 320-323Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure the Clear method properly handles the distributions field. rg --type go 'func \(x \*fastReflection_GenesisState\) Clear' -A 5 -B 5 api/cosmos/protocolpool/v1/genesis.pulsar.goLength of output: 561
Script:
#!/bin/bash # Ensure the Clear method properly handles the distributions field by retrieving the complete method implementation. rg --type go 'func \(x \*fastReflection_GenesisState\) Clear' -A 20 -B 5 api/cosmos/protocolpool/v1/genesis.pulsar.goLength of output: 1095
175-185: Check field descriptors for newdistributionsfield.The field descriptor setup for the new
distributionsfield appears correct. Ensure that these descriptors are consistently used across the application to maintain data integrity and avoid serialization issues.
120-165: Ensure proper initialization and validation of newDistributionlist.The introduction of the
_GenesisState_4_listto handleDistributionsis aligned with the PR's objectives. However, ensure that the list is properly initialized and validated in all relevant methods to prevent runtime issues.
852-863: Validate new message descriptor forDistribution.The setup for the
Distributionmessage descriptor is crucial for the correct serialization and deserialization of this type. Ensure that this setup is robust and error-free.Verification successful
The setup for the
Distributionmessage descriptor is robust and error-free.
- The
file_cosmos_protocolpool_v1_genesis_proto_initfunction initializes the proto file correctly.- The
md_Distributionmessage descriptor is correctly set usingFile_cosmos_protocolpool_v1_genesis_proto.Messages().ByName("Distribution").- The field descriptors
fd_Distribution_amountandfd_Distribution_timeare correctly assigned frommd_Distribution.Fields().ByName("amount")andmd_Distribution.Fields().ByName("time").Scripts executed
The following scripts were executed for the analysis:
Script:
#!/binash # Validate the setup for the Distribution message descriptor. rg --type go 'md_Distribution' -A 3 -B 3 api/cosmos/protocolpool/v1/genesis.pulsar.goLength of output: 1082
Script:
#!/bin/bash # Search for the definition of file_cosmos_protocolpool_v1_genesis_proto_init rg --type go 'func file_cosmos_protocolpool_v1_genesis_proto_init' -A 10 # Verify the message descriptor File_cosmos_protocolpool_v1_genesis_proto.Messages().ByName("Distribution") rg --type go 'File_cosmos_protocolpool_v1_genesis_proto.Messages().ByName("Distribution")' -A 3 -B 3 # Verify the field descriptors fd_Distribution_amount and fd_Distribution_time rg --type go 'fd_Distribution_amount' -A 3 -B 3 rg --type go 'fd_Distribution_time' -A 3 -B 3Length of output: 4423
api/cosmos/staking/v1beta1/staking.pulsar.go (2)
15704-15704: Review serialized protobuf data.This hunk seems to represent serialized protobuf data. It's crucial to verify that these changes are consistent with the protobuf schema and that they don't introduce any serialization/deserialization issues.
15714-15714: Check for consistency in protobuf field definitions.The protobuf field definitions should be checked for consistency and alignment with the overall schema, especially given the presence of deprecated fields.
| totalToBeDistributed := math.ZeroInt() | ||
| for _, distribution := range data.Distributions { | ||
| totalToBeDistributed = totalToBeDistributed.Add(distribution.Amount) | ||
| if err := k.Distributions.Set(ctx, *distribution.Time, distribution.Amount); err != nil { |
There was a problem hiding this comment.
why do we need to keep track of when the distribution is done?
There was a problem hiding this comment.
Because a continuous fund could expire in between, so we keep track of distributions received at t, t+1, t+n..., so when we distribute to the continuous funds, a fund expiring at t+2 will receive the distributions made until that point, and not more
There was a problem hiding this comment.
facundomedica
added
the
backport/v0.52.x
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- simapp/app.go (5 hunks)
- simapp/app_config.go (2 hunks)
Additional context used
Path-based instructions (2)
simapp/app_config.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.simapp/app.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (4)
simapp/app_config.go (2)
123-123: Verify the correct initialization and usage ofpooltypes.ModuleNameinBeginBlockers.Ensure that the
BeginBlockerfor theprotocolpoolmodule is correctly initialized and used.Verification successful
Verify the correct initialization and usage of
pooltypes.ModuleNameinBeginBlockers.
- Ensure that the
BeginBlockerfor theprotocolpoolmodule is correctly implemented inx/protocolpool/module.goandx/protocolpool/keeper/keeper.go.- Verify that
pooltypes.ModuleNameis correctly added to theBeginBlockerslist in the application configuration files (simapp/app_config.goandsimapp/app.go).Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of `pooltypes.ModuleName` in `BeginBlockers`. # Test: Search for the initialization and usage of the `BeginBlocker`. Expect: Correct initialization and usage. rg --type go 'BeginBlocker' -A 5Length of output: 52085
84-84: Verify the correct usage ofpooltypes.ProtocolPoolDistrAccount.Ensure that the new account is correctly integrated and used in the application.
simapp/app.go (2)
440-461: LGTM! The changes improve readability and maintainability.The modification to use a variadic function call for
ModuleManagerinstantiation is approved.
130-130: Verify the correct usage ofpooltypes.ProtocolPoolDistrAccountinmaccPerms.Ensure that the new account is correctly integrated and used in the application.
Verification successful
The new account
pooltypes.ProtocolPoolDistrAccountis correctly integrated and used in the application.The integration is consistent across test files and configuration files:
tests/integration/distribution/keeper/msg_server_test.gotests/integration/gov/keeper/keeper_test.gosimapp/app_config.gosimapp/v2/app_config.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `pooltypes.ProtocolPoolDistrAccount` in `maccPerms`. # Test: Search for the usage of the new account. Expect: Correct integration and usage. rg --type go 'pooltypes.ProtocolPoolDistrAccount'Length of output: 434
Script:
#!/bin/bash # Description: Fetch the relevant lines of code around the usage of `pooltypes.ProtocolPoolDistrAccount` in the identified files to verify correct integration and usage. # Fetching lines around the usage in test files and configuration files rg --type go --context 5 'pooltypes.ProtocolPoolDistrAccount' tests/integration/distribution/keeper/msg_server_test.go rg --type go --context 5 'pooltypes.ProtocolPoolDistrAccount' tests/integration/gov/keeper/keeper_test.go rg --type go --context 5 'pooltypes.ProtocolPoolDistrAccount' simapp/app_config.go rg --type go --context 5 'pooltypes.ProtocolPoolDistrAccount' simapp/v2/app_config.goLength of output: 2627
Description
This PR is a proposal to remove the dependency that exists between x/distribution and x/protocolpool.
Main changes:
x/distribution does not require x/protocolpool anymore, it sends a simple bank send to a predefined x/protocolpool account. This is an "intermediary" account, that its only purpose is to hold this tokens until they get distributed by x/protocolpool (otherwise it's difficult to know how many tokens were added to be distributed).
x/protocolpool now has its own BeginBlocker where it takes the funds from the intermediary account and sends them to community pool and the stream account.
x/distribution uses the bank module to interact with x/protocolpool (there are a couple of instances in which it does a send "impersonating" x/protocolpool, but those are deprecated endpoints).
fixes bug that was causing expired continuous funds from withdrawing if they didn't withdraw at every height.
Before: x/distr would make a bank send to the community pool and also call SetToDistribute (because we needed to know how many tokens there were to distribute).
graph TD A[ABCI BeginBlock] --> B[x/distr Module] B --> C[x/bank Send to x/protocolpool] C --> D[x/protocolpool SetToDistribute] D --> E[End]After this PR: x/distribution does a simple bank send to an intermediary account held by protocol pool (pool_distr), which then in its own BeginBlock x/protocolpool will distribute accordingly. x/distribution knows nothing other than the name of the intermediary account.
graph TD A[ABCI BeginBlock] --> B[x/distribution's BeginBlock] B --> C[Bank Send to <b>pool_distr</b> Intermediary Account] A --> D[x/protocolpool's BeginBlock] D --> E[Distribute Funds from <b>pool_distr</b>] E --> G[Community Pool] E --> H[Stream Account]Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Distributiondata structure to manage distributions with timestamp and amount.ProtocolPoolDistrAccountinto the application.Bug Fixes
Documentation
UnbondingDelegationandRedelegationmessages for better readability.Refactor
LastBalanceandDistributions.