feat(x/protocolpool): Stream funds from community pool #18103
Conversation
|
@hxrts can you please have a look at this initial implementation ? |
|
so this handles funds that are flowing from the distribution module to the community pool. I think we can add a second step, with the exact same logic, except we start with to total funds in the community pool and then distribute those funds to a second set of weighted addresses. This flow would look something like the following, with two splits A (funds coming from distribution for a given epoch) and B (total community pool balance). All of this logic can be in the community pool module. I called this a "middleware" above just to illustrate the logical point at which the split occurs. Not suggesting any middleware abstraction. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- x/protocolpool/keeper/grpc_query_test.go (1 hunks)
- x/protocolpool/keeper/keeper.go (4 hunks)
- x/protocolpool/keeper/msg_server_test.go (1 hunks)
- x/protocolpool/types/msg.go (2 hunks)
Additional comments: 13
x/protocolpool/keeper/grpc_query_test.go (1)
- 1-105: The test cases are well-structured and cover a variety of scenarios. The use of a
preRunfunction in the test cases is a good practice as it allows for setup steps specific to each test case. The error messages are checked for specific content, which is a good practice for ensuring the correct error is being returned. The test function also checks the returned unclaimed funds against the expected value, which is a good practice for ensuring the function is working as expected.x/protocolpool/keeper/msg_server_test.go (3)
1-17: The import statements and global variables are correctly defined. The test suite setup looks good.
19-143: The
TestMsgSubmitBudgetProposalfunction tests various scenarios for submitting a budget proposal. The test cases are well defined and cover a wide range of input combinations and expected error messages. The assertions are correctly checking for expected errors and error messages.145-339: The
TestMsgClaimBudgetfunction tests various scenarios for claiming a budget. The test cases are well defined and cover a wide range of input combinations and expected error messages. The assertions are correctly checking for expected errors, error messages, and claimable funds.x/protocolpool/types/msg.go (2)
1-8: The import statements and package declaration are standard and correct. No issues found.
29-41: The
NewBudgetProposalfunction is correctly defined and returns a newBudgetobject. However, it's important to ensure that therecipientstring is a valid address. If therecipientstring is not validated before this function is called, it could lead to issues. Consider adding a validation check for therecipientstring within this function or ensure it's validated before the function is called.func NewBudgetProposal(recipient string, totalBudget sdk.Coin, startTime *time.Time, tranches uint64, period *time.Duration) *Budget { if _, err := sdk.AccAddressFromBech32(recipient); err != nil { panic(err) } return &Budget{ RecipientAddress: recipient, TotalBudget: &totalBudget, StartTime: startTime, Tranches: tranches, Period: period, } }x/protocolpool/keeper/keeper.go (7)
2-15: The import statements are well organized and necessary for the code. Ensure that all the imported packages are used in the code.
23-33: The
Keeperstruct has been updated with new fields. Ensure that these fields are used appropriately in the code.39-62: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [35-60]
The
NewKeeperfunction has been updated to initialize the new fields and build the schema. Ensure that the schema is built correctly and that the new fields are initialized as expected.
93-107: The
claimFundsfunction claims funds from the fee pool and distributes them to a recipient. Ensure that the function handles errors correctly and that the funds are distributed as expected.109-152: The
getClaimableFundsfunction calculates the claimable funds for a recipient based on a budget. Ensure that the function handles errors correctly and that the claimable funds are calculated as expected.154-189: The
calculateClaimableFundsfunction calculates the claimable funds for a recipient based on a budget and the current time. Ensure that the function handles errors correctly and that the claimable funds are calculated as expected.191-219: The
validateAndUpdateBudgetProposalfunction validates and updates a budget proposal. Ensure that the function handles errors correctly and that the budget proposal is validated and updated as expected.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- x/protocolpool/keeper/grpc_query.go (2 hunks)
- x/protocolpool/keeper/grpc_query_test.go (1 hunks)
Additional comments: 12
x/protocolpool/keeper/grpc_query_test.go (1)
- 1-125: The test cases are well-structured and cover a variety of scenarios. The use of a table-driven test structure is a good practice in Go, as it allows for easy addition of new test cases and makes the test function more readable. The use of a
preRunfunction in the test cases is also a good practice, as it allows for setup code to be encapsulated within the test case itself. The error checking and response comparison are thorough. Overall, the test function appears to be well-written and comprehensive.x/protocolpool/keeper/grpc_query.go (11)
2-14: The import section looks fine. All the necessary packages are imported for the functionality of the code.
33-34: The conversion of
amounttoDecCoinsand the return statement are correct.37-81: The
UnclaimedBudgetfunction is well implemented. It checks for an empty request, validates the recipient address, retrieves the budget proposal, calculates the unclaimed budget, and updates the budget's claimed amount, next claim from time, and tranches left. It then returns the relevant information in the response. However, it's important to ensure that theKeeperstruct and its methods are thread-safe if they are accessed from multiple goroutines.39-41: Good error handling for an empty request.
42-45: Good error handling for invalid recipient addresses.
46-52: Good error handling for retrieving the budget proposal.
53-58: The calculation of the unclaimed budget is correct.
60-63: Good handling of a nil
ClaimedAmount.65-67: Good handling of a nil
NextClaimFrom.69-71: Good handling of zero
TranchesLeft.73-80: The response is correctly formed and returned.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- x/protocolpool/go.mod
Files selected for processing (1)
- simapp/app.go (1 hunks)
Files skipped from review due to trivial changes (1)
- simapp/app.go
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- x/protocolpool/go.mod
Files selected for processing (4)
- x/protocolpool/README.md (5 hunks)
- x/protocolpool/keeper/grpc_query.go (2 hunks)
- x/protocolpool/keeper/keeper.go (4 hunks)
- x/protocolpool/keeper/msg_server.go (1 hunks)
Additional comments: 21
x/protocolpool/keeper/keeper.go (6)
23-33: The Keeper struct has been updated with new fields. Ensure that these fields are properly initialized and used throughout the codebase.
39-60: The
NewKeeperfunction has been updated to initialize the new fields and build the schema. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.93-107: The
claimFundsfunction claims funds from the distribution info and distributes them from the fee pool. Ensure that error handling is properly done and that the function is called in the correct context.109-152: The
getClaimableFundsfunction retrieves the claimable funds for a recipient. Ensure that error handling is properly done and that the function is called in the correct context.154-189: The
calculateClaimableFundsfunction calculates the claimable funds based on the budget and current time. Ensure that error handling is properly done and that the function is called in the correct context.191-228: The
validateAndUpdateBudgetProposalfunction validates and updates a budget proposal. Ensure that error handling is properly done and that the function is called in the correct context.x/protocolpool/keeper/msg_server.go (2)
25-37: The
ClaimBudgetfunction seems to be correctly implemented. It validates the recipient address, claims the funds, and returns the claimed amount. However, it's important to ensure that theclaimFundsfunction is correctly implemented and handles all possible error scenarios.39-60: The
SubmitBudgetProposalfunction validates the authority, validates and updates the budget proposal, and sets the budget proposal in the state. It's important to ensure that thevalidateAndUpdateBudgetProposalfunction is correctly implemented and handles all possible error scenarios.x/protocolpool/README.md (6)
8-15: The introduction of the
protocolpoolmodule and its purpose is well explained. The lazy "claim-based" system is a good design choice for flexibility and control over fund distribution.34-35: The
CommunityPoolSpendmethod is now explained in detail, addressing the previous comment.37-46: The
SubmitBudgetProposalmethod is well explained. It's clear that it's used to propose a budget allocation for a specific recipient, with funds distributed periodically over a specified time frame.48-55: The
ClaimBudgetmethod is well explained. It's clear that it's used to claim funds from a previously submitted budget proposal.99-115: The conditions under which the
MsgSubmitBudgetProposalmessage will fail are clearly outlined. This is helpful for users to understand the constraints and requirements of submitting a budget proposal.134-148: The conditions under which the
MsgClaimBudgetmessage will fail are clearly outlined. This is helpful for users to understand the constraints and requirements of claiming a budget.x/protocolpool/keeper/grpc_query.go (7)
39-41: Good practice to check for nil requests and return an error if the request is nil.
42-45: Good practice to validate the recipient address and return an error if it's invalid.
46-52: Good error handling for retrieving the budget proposal. It checks for both not found errors and other errors.
54-60: The code checks if
ClaimedAmountis nil and if so, it setsunclaimedBudgettoTotalBudgetandClaimedAmountto zero. This is a good practice to ensure thatClaimedAmountis never nil.62-64: The code checks if
NextClaimFromis nil and if so, it setsNextClaimFromtoStartTime. This is a good practice to ensure thatNextClaimFromis never nil.66-68: The code checks if
TranchesLeftis zero and if so, it setsTranchesLefttoTranches. This is a good practice to ensure thatTranchesLeftis never zero.70-77: The response includes all the necessary information about the budget proposal. Good practice.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- x/protocolpool/go.mod
Files selected for processing (2)
- x/protocolpool/README.md (5 hunks)
- x/protocolpool/keeper/msg_server.go (1 hunks)
Additional comments: 9
x/protocolpool/keeper/msg_server.go (3)
25-37: The
ClaimBudgetfunction looks good. It correctly handles the conversion of the recipient address from a string to bytes and handles any errors that may occur. It also correctly handles the claiming of funds and any errors that may occur during that process.39-61: The
SubmitBudgetProposalfunction also looks good. It correctly validates the authority, converts the recipient address from a string to bytes, validates and updates the budget proposal, and sets the budget proposal in the state. All potential errors are handled appropriately.56-56: As per the previous comments, if two budgets to the same address are created, the budget would be updated with the new budget. This behavior should be documented to avoid confusion.
x/protocolpool/README.md (6)
8-15: The introduction provides a clear overview of the
protocolpoolmodule and its purpose. It also explains the "claim-based" system for fund distribution.34-35: The
CommunityPoolSpendmethod is explained in more detail in this version. It's clear now that it can be called by the module authority or any account with authorization to spend funds from theprotocolpoolmodule account to a receiver address.37-46: The
SubmitBudgetProposalmessage is introduced and explained. It's used to propose a budget allocation for a specific recipient, with funds distributed periodically over a specified time frame. Users are responsible for actively claiming their allocated funds.48-55: The
ClaimBudgetmessage is introduced and explained. It's used to claim funds from a previously submitted budget proposal. Recipients can claim their share of the budget at the appropriate time.98-115: The
MsgSubmitBudgetProposalmessage is explained in more detail, including the conditions under which the message will fail. It's also noted that if two budgets to the same address are created, the budget would be updated with the new budget.134-148: The
MsgClaimBudgetmessage is explained in more detail, including the conditions under which the message will fail.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- x/protocolpool/README.md (5 hunks)
Additional comments: 6
x/protocolpool/README.md (6)
8-15: The introduction and explanation of the module's purpose and functionality are clear and concise. The lazy "claim-based" system is well explained.
34-35: The
CommunityPoolSpendmethod is now explained in detail, addressing the previous comment. The explanation is clear and provides a good understanding of its purpose and usage.37-46: The
SubmitBudgetProposalmethod is well explained. It's clear that it's used to propose a budget allocation for a specific recipient and that users are responsible for claiming their allocated funds.48-55: The
ClaimBudgetmethod is well explained. It's clear that it's used to claim funds from a previously submitted budget proposal and that funds are distributed in tranches over specific periods.99-115: The
MsgSubmitBudgetProposalmessage is well explained, including the conditions under which the message will fail. The warning about budgets to the same address being updated with the new budget is a useful piece of information.123-137: The
MsgClaimBudgetmessage is well explained, including the conditions under which the message will fail. It's clear that it's used to claim funds from a previously submitted budget proposal.
|
Post merge ACK. I was litterally clicking "Approve" 😆 |
Description
Closes: #17267
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...
!to the type prefix if API or client breaking changeCHANGELOG.mdmake lintandmake testReviewers 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.
I have...
!in the type prefix if API or client breaking changeSummary by CodeRabbit
Release Notes
New Features:
Improvements:
protocolpoolmodule with new messagesSubmitBudgetProposalandClaimBudgetfor proposing and claiming budget allocations.Keeperstruct with new fields and functions to handle fund claiming, calculate claimable funds, and validate budget proposals.Tests:
protocolpoolmodule.Documentation:
protocolpoolmodule.