-
Notifications
You must be signed in to change notification settings - Fork 387
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: users namespace #2471
feat: users namespace #2471
Conversation
Thanks! :-) Co-authored-by: Manfred Touron <[email protected]>
Co-authored-by: Manfred Touron <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2471 +/- ##
==========================================
- Coverage 54.95% 54.92% -0.04%
==========================================
Files 595 595
Lines 79568 79646 +78
==========================================
+ Hits 43727 43743 +16
- Misses 32533 32588 +55
- Partials 3308 3315 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
da67a87
to
4a3e5aa
Compare
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
2fb3331
to
497038b
Compare
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.
Mostly good.
For the record, I'm still thinking that this logic should live outside the VMKeeper.
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Co-authored-by: anarcher <[email protected]> Co-authored-by: Manfred Touron <[email protected]>
var reNamespace = regexp.MustCompile(`^gno.land/(?:r|p)/([\.~_a-zA-Z0-9]+)`) | ||
|
||
// checkNamespacePermission check if the user as given has correct permssion to on the given pkg path | ||
func (vm *VMKeeper) checkNamespacePermission(ctx sdk.Context, creator crypto.Address, pkgPath string) error { |
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.
For better security and trust of the chain, we should consider removing the admin from the system contract entirely. No one should be able to add a contract under /r/system unless it is voted on by the gov DAO or through a chain upgrade that involves consent from the validators.
Following and closing #384
This PR adds a user check when adding a new package. The check is based on the first part of the realm path:
gno.land/(r|p)/<user_namespace>
. It callsgno.land/r/demo/users.GetUserByName(<namespace>)
to ensure the user exists and has registered the user namespace.This PR also adds restricted name ability.
thanks @anarcher for the base
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description