-
Notifications
You must be signed in to change notification settings - Fork 22
test(e2e): dynamic quorum e2e tests #154
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
base: main
Are you sure you want to change the base?
Conversation
Since #152 has been merged, can you rebase on main so we have a better diff ? |
23a262b
to
d456ae5
Compare
quorumMin := sdk.MustNewDecFromStr(quorumRange.Min) | ||
quorumMax := sdk.MustNewDecFromStr(quorumRange.Max) | ||
currentQuorum := sdk.MustNewDecFromStr(quorums.GetQuorum()) | ||
quorumPEma := (currentQuorum.Sub(quorumMin)).Quo(quorumMax.Sub(quorumMin)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this code makes me realize that maybe the participationEMAs should be queryable. Maybe we could return the different participations in the Quorums
query response ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use a separate, dedicated endpoint
paramsFeemarket := s.queryFeemarketParams(chainAAPIEndpoint) | ||
oldAlpha := paramsFeemarket.Params.Alpha | ||
paramsFeemarket.Params.Alpha = oldAlpha.Add(sdk.NewDec(1)) | ||
s.writeFeemarketParamChangeProposal(s.chainA, paramsFeemarket.Params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather use a less specific proposal for this test, it's confusing to imply the feemarket in it. We should use a simple text proposal, but there is currently no test for a text proposal submission, right ? Maybe it's time to add one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, also this will make life easier for the renaming of the module, so we don't have leftover "feemarket" references
Description
E2E tests for the dynamic quorum feature
Closes: #145
(with PR #146)
Based on PR #152