feat(examples): awesome gno #3963
Conversation
github-actions
bot
added
the
🧾 package/realm
🛠 PR Checks Summary🔴 Pending initial approval by a review team member, or review from tech-staff Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
matijamarjanovic
changed the title
matijamarjanovic
changed the title
Gno2D2
added
the
review/triage-pending
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
|
Problem here with "not enough arguments in call to page.Picker", same as #3793 (comment) :-) |
Addressed in 567df81. I've also added hof registration (if you remember #3479 it caused lint to break in init) which suprisingly causes no trouble here.. |
| admins.Set("g1ej0qca5ptsw9kfr64ey8jvfy9eacga6mpj2z0y", true) | ||
| admins.Set(std.CurrentRealm().Address().String(), true) | ||
| admins.Set(std.PreviousRealm().Address().String(), true) | ||
|
|
There was a problem hiding this comment.
you can use p/moul/addrset for this
There was a problem hiding this comment.
Hey I figured since authz is already managing admin set so I've just implemented that instead in 048d3ef
| func AddAdmin(addr std.Address) error { | ||
| caller := std.PreviousRealm().Address() | ||
|
|
||
| if !isAdmin(caller) { | ||
| return ufmt.Errorf("not authorized: only admins can add new admins") | ||
| } | ||
| if !addr.IsValid() { | ||
| return ufmt.Errorf("invalid address") | ||
| } | ||
|
|
||
| admins.Set(addr.String(), true) | ||
| std.Emit("AdminAdded", "address", addr.String()) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Should be handled with authorizable or with p/moul/authz
There was a problem hiding this comment.
applied in 048d3ef
| func RemoveAdmin(addr std.Address) error { | ||
| caller := std.PreviousRealm().Address() | ||
|
|
||
| if !isAdmin(caller) { | ||
| return ufmt.Errorf("not authorized: only admins can remove admins") | ||
| } | ||
| if !addr.IsValid() { | ||
| return ufmt.Errorf("invalid address") | ||
| } | ||
| if admins.Size() <= 1 { | ||
| return ufmt.Errorf("cannot remove the last admin") | ||
| } | ||
|
|
||
| admins.Remove(addr.String()) | ||
| std.Emit("AdminRemoved", "address", addr.String()) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Same as above, use a package for privileged actions. I suggest you dig into p/moul/authz; we are slowly moving away from ownable and authorizable
There was a problem hiding this comment.
applied in 048d3ef
| categories.Iterate("", "", func(key string, value interface{}) bool { | ||
| cat := value.(Category) | ||
| if cat.Name == name { | ||
| exists = true | ||
| return true | ||
| } | ||
| return false | ||
| }) |
There was a problem hiding this comment.
Definitely not like this; you should use the fact that AVL tree has a lookup of O(1) - just use the category name as the key. Make sure that you sanitize the key properly (ie no spaces,etc)
There was a problem hiding this comment.
Also you can use any instead of interface{}
There was a problem hiding this comment.
yeah, makes sense, forgot about avl.Has().. implemented in 4f6f4fa
| DappIDs: []uint64{}, | ||
| } | ||
|
|
||
| categories.Set(strconv.FormatUint(nextCategoryID, 10), category) |
There was a problem hiding this comment.
Does this mean I should generally use FormatUint ?
| nextDappID uint64 = 1 | ||
| nextProposalID uint64 = 1 |
There was a problem hiding this comment.
seqid is more practical when working with AVL trees
There was a problem hiding this comment.
Ok implemented in 9887784
| func ListCategories() []Category { | ||
| result := []Category{} | ||
|
|
||
| categories.Iterate("", "", func(key string, value interface{}) bool { | ||
| result = append(result, value.(Category)) | ||
| return false | ||
| }) | ||
|
|
||
| return result | ||
| } |
There was a problem hiding this comment.
If you need this to be a getter for someone outside of this realm, check out avl/rotree, which exposes a tree without setter functions
There was a problem hiding this comment.
Yes, that's what I was going for (but also for render), applied in 6fe1d53
| return result | ||
| } | ||
|
|
||
| func ProposeNewDapp(title, description, url string, categoryNamesStr string) (uint64, error) { |
There was a problem hiding this comment.
For proposals, let's try using some DAO code that already exists. @jeronimoalbi maybe we can integrate with gno.me as we talked, but maybe we should just do commondao+datasource. wdyt?
There was a problem hiding this comment.
Using gno.me seems to be the right approach with the caveat that before mainnet will require some small changes to your custom proposal definition methods, there are a couple of changes to be done for security that are still pending. But this way would be interesting so you can have a SubDAO within the gno.me DAO tree.
Using commondao is also valid, as a package or directly though the commondao realm. The package would require a bit more effort. If commondao is used we can also integrate with gnome space if it makes sense and list content there if you support the datasource interface from gno.land/p/jeronimoalbi/datasource.
There was a problem hiding this comment.
Hey, so in 381c72d I've added datasource support for Proposals, Dapps and Categories (with tests) as we talked today @leohhhn, I'd love some feedback on that as it is pretty long. Looking forward, I should wait until commonDao package/realm is merged and use that here if I understood our conversation today correctly?
| var categoryNames []string | ||
| if categoryNamesStr != "" { | ||
| for _, name := range strings.Split(categoryNamesStr, ",") { | ||
| trimmedName := strings.TrimSpace(name) | ||
| if trimmedName != "" { | ||
| categoryNames = append(categoryNames, trimmedName) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(categoryNames) > 0 { | ||
| for _, catName := range categoryNames { | ||
| categoryExists := false | ||
| categories.Iterate("", "", func(key string, value interface{}) bool { | ||
| cat := value.(Category) | ||
| if cat.Name == catName { | ||
| categoryExists = true | ||
| return true | ||
| } | ||
| return false | ||
| }) | ||
| if !categoryExists { | ||
| return 0, ufmt.Errorf("category '%s' does not exist", catName) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Too much nesting, can be simplified
There was a problem hiding this comment.
done in 29f82c1
| caller := std.PreviousRealm().Address() | ||
|
|
||
| if !isAdmin(caller) { | ||
| return ufmt.Errorf("not authorized: only admins can update dApp categories") |
There was a problem hiding this comment.
this error repeats, it should be declared somewhere. speciifcally for this, either use the p/ errors for control access (ownable/authz), or declare it at top level of this package.
There was a problem hiding this comment.
this commit defines errors in types.gno and uses those
| for _, catName := range strings.Split(newCategories, ",") { | ||
| _, found := GetCategory(catName) | ||
| if !found { | ||
| return ufmt.Errorf("category not found: %s", catName) | ||
| } | ||
| } | ||
|
|
||
| for _, oldCat := range dapp.Categories { | ||
| found := false | ||
| for _, newCat := range strings.Split(newCategories, ",") { | ||
| if oldCat == newCat { | ||
| found = true | ||
| break | ||
| } | ||
| } | ||
| if !found { | ||
| err := RemoveDappFromCategory(dappID, oldCat) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for _, newCat := range strings.Split(newCategories, ",") { | ||
| found := false | ||
| for _, oldCat := range dapp.Categories { | ||
| if newCat == oldCat { | ||
| found = true | ||
| break | ||
| } | ||
| } | ||
| if !found { | ||
| err := addDappToCategory(dappID, newCat) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Definitely too much nesting.
If you need to use double for loops, you're probably doing something wrong - these could be avoided with the proper use of maps/avl trees
There was a problem hiding this comment.
yes i see what you mean, hopefully this makes more sense : 27dc704
| caller := std.PreviousRealm().Address() | ||
|
|
||
| if !isAdmin(caller) { | ||
| return ufmt.Errorf("not authorized: only admins can delete dApps") |
There was a problem hiding this comment.
same comment as above for errors
There was a problem hiding this comment.
4cd9637 this and all other non specific errors
implementation of #3928
This is an on chain representation of the Awesome Gno repo which serves a purpose of showcasing dApp built in the community. The way this implementation works is:
UI (mock data):