-
Notifications
You must be signed in to change notification settings - Fork 31
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
feat: introduce censorship into x/foundation #912
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #912 +/- ##
==========================================
+ Coverage 62.34% 62.36% +0.01%
==========================================
Files 653 655 +2
Lines 79485 79759 +274
==========================================
+ Hits 49558 49741 +183
- Misses 27248 27334 +86
- Partials 2679 2684 +5
|
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.
LGTM
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.
LGTM
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.
How can we move the auth of CreateValidatorAuthorization
from x/foundation
to x/gov
?
Please refer to the corresponding item in the description of this PR:
You can change the authority into |
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.
Do you mean we need to use MsgUpdateCencorship
if you want to move the auth.
But the autht to want to change should be lower the enum number. Is it right?
authorityAddrs := map[foundation.CensorshipAuthority]string{ | ||
foundation.CensorshipAuthorityGovernance: authtypes.NewModuleAddress(govtypes.ModuleName).String(), | ||
foundation.CensorshipAuthorityFoundation: k.authority, | ||
} | ||
if expected := authorityAddrs[censorship.Authority]; authority != expected { | ||
return sdkerrors.ErrUnauthorized.Wrapf("invalid authority; expected %s, got %s", expected, authority) | ||
} |
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 think it's good to use if-else
, how about this?
* Add censorship into proto * Implement censorship * Add FoundationExecProposal to proto * Implement FoundationExecProposal * Update documentation * Update CHANGELOG.md * Rename files * Lint * Add amino test on MsgUpdateCensorship * Add pagination on cli * Update x/foundation/README.md * Update proposal cli description * Chore * Apply feedbacks (cherry picked from commit d9428ec)
Description
This patch would:
Censorship
, which has its target message type url and authority.CENSORSHIP_AUTHORITY_GOVERNANCE
CENSORSHIP_AUTHORITY_FOUNDATION
/lbm.foundation.v1.MsgUpdateCensorship
/lbm.foundation.v1.MsgGrant
/lbm.foundation.v1.MsgRevoke
/lbm.foundation.v1.MsgUpdateCensorship
, which updates censorship information.CENSORSHIP_AUTHORITY_UNSPECIFIED
.FoundationExecProposal
which allows x/gov's authority to trigger x/foundation's messages./lbm.foundation.v1.MsgUpdateCensorship
/lbm.foundation.v1.MsgGrant
/lbm.foundation.v1.MsgRevoke
closes: #910
Checklist:
CHANGELOG.md
client/docs/swagger-ui/swagger.yaml