-
Notifications
You must be signed in to change notification settings - Fork 7
feat: add Trust Factor app #17
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
leohhhn
left a comment
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.
Nice start! I'm leaving comments on how you can improve your code below. Please check them out
| name := userData.Name() | ||
| if name != "" { | ||
| return name | ||
| } |
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.
if userData exists, name cannot be empty. Always check the contract that you're using - at one point you will use something and make a mistake that could cost a lot if you're not sure what you're using
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.
done
| var trustScores map[address]TrustScore // Main storage for user trust scores | ||
| var userVotes map[string]bool // Tracks votes in "voter_target" format | ||
| var voteHistory map[address][]VoteHistory // Historical vote records per user | ||
| var userComments map[address]*avl.Tree // User comments storage using AVL tree | ||
| var owner address // Contract owner address |
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.
this can be a single var block
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.
done
| var sortedUsers []address // Cached sorted user list | ||
| var lastSortTime int64 // Last cache update timestamp | ||
| var totalUsers int // Total registered users count |
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.
same here
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.
done
| const MAX_HISTORY_SIZE = 50 // Maximum vote history entries per user | ||
| const SORT_CACHE_TTL = 300 // Cache time-to-live in seconds |
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.
same here for the block
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.
done
| trustScores = make(map[address]TrustScore) | ||
| userVotes = make(map[string]bool) | ||
| voteHistory = make(map[address][]VoteHistory) | ||
| userComments = make(map[address]*avl.Tree) |
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.
You should use the avl.Tree instead of map.
| func onlyOwner() { | ||
| caller := runtime.OriginCaller() | ||
| if caller != owner { | ||
| panic("only owner can call this function") | ||
| } | ||
| } |
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.
Check out p/nt/ownable for easy admin management
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.
done
|
|
||
| // extractSearchParam parses and extracts the "search" query parameter from a URL path. | ||
| // Returns an empty string if no search parameter is found or if URL parsing fails. | ||
| func extractSearchParam(path string) string { |
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.
Generally good to move helpers down to the bottom of the file. If I see a render.gno file, I expect to see the Render() function close to the top
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.
done
| func renderSearchResults(query string) string { | ||
| if query == "" { | ||
| return "Please enter an address to search." | ||
| } | ||
|
|
||
| addr := address(query) | ||
| if _, exists := GetAllUsers()[addr]; exists { | ||
| return renderUserProfile(string(addr)) | ||
| } | ||
|
|
||
| result := md.H1("🔍 Search results for: " + md.InlineCode(query)) | ||
|
|
||
| var matches []address | ||
| for addr := range GetAllUsers() { | ||
| addrStr := string(addr) | ||
| if len(query) >= 4 && strings.Contains(addrStr, query) { | ||
| matches = append(matches, addr) | ||
| } | ||
| } | ||
|
|
||
| if len(matches) == 0 { | ||
| result += "❌ " + md.Bold("No users found") + "\n\n" | ||
| result += md.Bold("Suggestions:") + "\n" | ||
| result += md.BulletList([]string{ | ||
| "Check the address spelling", | ||
| "Try with at least 4 characters", | ||
| "User must be registered in the system", | ||
| }) + "\n" | ||
| } else if len(matches) == 1 { | ||
| return renderUserProfile(string(matches[0])) | ||
| } else { |
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.
Try using packages for this. p/nt/mux, net/url, etc
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.
done
| } else if len(matches) == 1 { | ||
| return renderUserProfile(string(matches[0])) | ||
| } else { | ||
| result += "✅ " + md.Bold(strconv.Itoa(len(matches))+" user(s) found:") + "\n\n" |
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.
Since you're using len(matches) so many times, save it out in a variable. this saves compute power
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.
done
| result += md.Bold("Votes: ") + strconv.Itoa(trust.TotalVotes) + " total\n\n" | ||
|
|
||
| detailURL := "/r/greg007/trustfactor:" + string(addr) | ||
| upvoteURL := "https://gno.land/r/greg007/trustfactor$help&func=Upvote&target=" + string(addr) |
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.
This will not work on every network as the domain is different for each one. These links will link to the Staging network, but not others. Rendered links should work for whatever testnet the user is viewing your app on.
Use the p/moul/txlink library for generating links like these - check out r/docs/ for a tutorial
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.
done
- Convert Register, Upvote, Downvote, and AddComment URLs to txlink.Call() - Use builder pattern for AddComment to allow user-editable text field - Cache match count to reduce redundant len() calls
Thank you for all your comments. I have corrected and tested everything. Let me know if you have anything else to add ! |
leohhhn
left a comment
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.
leaving some comments, check them out
| // RemoveUser removes a malicious user from the system (emergency function) | ||
| func RemoveUser(target address) { | ||
| owner.AssertOwnedByCurrent() | ||
|
|
||
| trustScores.Remove(string(target)) | ||
| voteHistory.Remove(string(target)) | ||
|
|
||
| // Clean up all votes involving the target user | ||
| keysToRemove := []string{} | ||
| userVotes.Iterate("", "", func(key string, value interface{}) bool { | ||
| if strings.Contains(key, string(target)) { | ||
| keysToRemove = append(keysToRemove, key) | ||
| } | ||
| return false | ||
| }) | ||
| for _, key := range keysToRemove { | ||
| userVotes.Remove(key) | ||
| } | ||
|
|
||
| totalUsers-- | ||
| invalidateCache() | ||
| } |
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.
this will not work when called from outside as it's not crossing and it's modifying state in the realm. please check out other example realms and see how crossing works
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.
Yes, I hadn't thought about that, but I've corrected it now.
| // TransferOwnership transfers contract ownership to a new address | ||
| func TransferOwnership(newOwner address) error { | ||
| return owner.TransferOwnership(newOwner) | ||
| } | ||
|
|
||
| // DropOwnership removes the owner, effectively disabling all admin functions | ||
| func DropOwnership() error { | ||
| return owner.DropOwnershipByCurrent() | ||
| } | ||
|
|
||
| // Owner returns the current owner address | ||
| func Owner() address { | ||
| return owner.Owner() | ||
| } |
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.
Check out r/docs/safeobjects - no need to manually write these functions out; you can just expose the ownable object itself
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.
done ✅
| // ============================================================================= | ||
|
|
||
| // FilterUsers returns paginated and filtered user results | ||
| func FilterUsers(searchTerm string, limit int, offset int) ([]address, int) { |
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.
check if you can use p/nt/avl/pager for paging in this case instead of writing your own pager
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 can't, because I'm currently using an array and not a tree. If I switch to a tree, I think it will unnecessarily complicate the code.
| } | ||
|
|
||
| // containsSubstring checks if a string contains a substring (case-sensitive) | ||
| func containsSubstring(str, substr string) bool { |
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.
use strings.Index
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 replaced it, it's more appropriate.
|
|
||
| import ( | ||
| "time" | ||
|
|
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.
random newline
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.
removed ✅
| tree := treeValue.(*avl.Tree) | ||
|
|
||
| comments := make([]Comment, 0) | ||
| tree.ReverseIterate("", "", func(key string, value interface{}) bool { |
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.
youn can use any instead of interface{}
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.
replaced !
| // GetAllUsers returns the complete trust scores map (used internally) | ||
| func GetAllUsers() map[address]TrustScore { | ||
| result := make(map[address]TrustScore) | ||
| trustScores.Iterate("", "", func(key string, value interface{}) bool { |
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.
| trustScores.Iterate("", "", func(key string, value interface{}) bool { | |
| trustScores.Iterate("", "", func(key string, value any) bool { |
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.
replaced !
| // ============================================================================= | ||
|
|
||
| // GetAllUsers returns the complete trust scores map (used internally) | ||
| func GetAllUsers() map[address]TrustScore { |
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.
why return map when you can do avl.Tree
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.
of course, I must been tired at that moment.😅
TrustFactor is a realm where you can upvote or downvote a user to give them a score out of 10. This score can be used by a platform to restrict users with a trust score below a certain threshold. In this system, I also added an SVG widget that can be imported to a user's homepage, for example, to display their trust score.