fix(x/gov): deposits in simulations #18336
Conversation
WalkthroughThe changes made to the codebase primarily revolve around enhancing the functionality of the Changes
Poem
TipsChat with CodeRabbit Bot (
|
|
@facundomedica your pull request is missing a changelog! |
github-prbot
requested review from
a team,
alexanderbez and
samricotta
and removed request for
a team
facundomedica
added
the
backport/v0.50.x
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| return simtypes.NoOpMsg(types.ModuleName, TypeMsgDeposit, "unable to generate proposalID"), nil, nil | ||
| } | ||
|
|
||
| deposit, skip, err := randomDeposit(r, ctx, ak, bk, k, simAccount.Address, false, false) | ||
| p, err := k.Proposals.Get(ctx, proposalID) | ||
| if err != nil { | ||
| return simtypes.NoOpMsg(types.ModuleName, TypeMsgDeposit, "unable to get proposal"), nil, err | ||
| } | ||
|
|
||
| deposit, skip, err := randomDeposit(r, ctx, ak, bk, k, simAccount.Address, false, p.Expedited) | ||
| switch { | ||
| case skip: | ||
| return simtypes.NoOpMsg(types.ModuleName, TypeMsgDeposit, "skip deposit"), nil, nil |
There was a problem hiding this comment.
The error handling here is good, it checks for errors and handles them appropriately. However, the error messages could be more descriptive to provide more context about the error. For example, instead of "unable to generate proposalID", you could say "unable to generate proposalID in SimulateMsgDeposit function". This would make it easier to debug if an error occurs.
| return nil, false, err | ||
| } | ||
| amount = amount.Add(minAmount) | ||
| amount = amount.MulRaw(3) // 3x what's required // TODO: this is a hack, we need to be able to calculate the correct amount using params | ||
|
|
||
| if amount.GT(spendableBalance) || amount.LT(threshold) { | ||
| return nil, true, nil |
There was a problem hiding this comment.
The error handling here is good, it checks for errors and handles them appropriately. However, the error messages could be more descriptive to provide more context about the error. For example, instead of "unable to generate deposit", you could say "unable to generate deposit in randomDeposit function". This would make it easier to debug if an error occurs.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- x/gov/simulation/operations_test.go (3 hunks)
Additional comments: 3
x/gov/simulation/operations_test.go (3)
155-155: The change in the initial deposit amount in the
TestSimulateMsgSubmitProposalfunction from "143523282stake" to "47841094stake" is noted. Ensure that this new value is appropriate for the test scenario.188-188: The change in the initial deposit amount in the
TestSimulateMsgSubmitLegacyProposalfunction from "75498768stake" to "25166256stake" is noted. Ensure that this new value is appropriate for the test scenario.279-279: The change in the deposit amount in the
TestSimulateMsgDepositfunction from "1682907stake" to "560969stake" is noted. Ensure that this new value is appropriate for the test scenario.
julienrbrt
removed
the
backport/v0.50.x
|
Removing the backport tag, we can cherry-pick in #18333 |
Description
Closes: #XXXX
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
New Features:
k.Proposals.Get, improving the system's efficiency.randomDepositfunction to include thep.Expeditedflag, providing more flexibility in function calls.Please note that these changes are aimed at improving the overall performance and flexibility of the system, which should result in a smoother user experience.