-
Notifications
You must be signed in to change notification settings - Fork 33
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
fix: Global min Self Delegation #134
Conversation
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.
Would it be able to add tests for this change?(genesis_test, params_test in sim etc)
Also, how does this param enforce validators to have min_self_delegation
?
…east the provided MinSelfDelegation (#132) * add check to make sure validator object is only created if min delegation threshold is met * fix check location and use standard error type * fix logic for check * add test for new check * update test comments for better context
@AlpinYukseloglu you have to update the GetParams queries for the new field in the param struct before merge. To test locally, inside of |
Interesting... local tests seem to be passing ( |
FYI: The Rosetta test was broken in the main sdk branch as well |
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.
Actually, it seems that something in this PR does seem to break the rosetta-test that needs further investigation on fixes. Also, by a brief look, it seems that the "build" action might be failing due to changes in go.sum
, can you revert go.sum into the original state?
TODO: Ask in cosmos SDK discord how rosetta tests work / how to debug this failure. If theres a genesis file somewhere for Rosetta, we did break it / would have to update it for this PR |
Frojdi told me to look at this function:
I think we need to log |
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
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.
🚀 🚀 🚀
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Closes: #113
What is the purpose of the change
Our current implementation of Superfluid Staking exposes the chain to a specific vulnerability relating to the validator creation process. This PR adds the necessary guardrails to make sure that an attacker cannot halt the chain by generating an arbitrary number of empty validators.
This was not an issue prior to Superfluid Staking because the full validator set was not used in the ante anywhere, but our specific implementation of Superfluid introduced a linear-time search of the validator set at epoch end that made the issue fixed in this PR a vulnerability.