-
Notifications
You must be signed in to change notification settings - Fork 193
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(token-registry): Implement strongly typed Nibiru Token Registry and generation command #2144
Conversation
WalkthroughThis pull request introduces a new feature to the Nibiru Token Registry system, enhancing the management and documentation of digital token metadata within the Nibiru blockchain ecosystem. The changes encompass the addition of a strongly typed token registry, a generation command, and comprehensive data structures for token assets. It includes updates to the changelog, a new recipe in the build system, and detailed documentation, while also ensuring appropriate testing for the new functionalities. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🧹 Nitpick comments (6)
token-registry/githubify.go (1)
19-22
: Consider making the GitHub URL configurableThe GitHub repository URL is hardcoded in
localImageToGitHub
. Consider making it configurable through environment variables or configuration to support different repository paths or hosting platforms.+const defaultGitHubBaseURL = "https://raw.githubusercontent.com/NibiruChain/nibiru/main/token-registry/img/" + func localImageToGitHub(local string) string { trimmed := strings.TrimPrefix(local, "./img/") - return "https://raw.githubusercontent.com/NibiruChain/nibiru/main/token-registry/img/" + trimmed + baseURL := os.Getenv("GITHUB_BASE_URL") + if baseURL == "" { + baseURL = defaultGitHubBaseURL + } + return baseURL + trimmed }token-registry/assetlist_test.go (1)
21-54
: Refactor duplicate image validation logicThe test has duplicate code for checking PNG and SVG files. Consider extracting this into a helper method.
+func (s *Suite) validateImage(path *string) { + if path != nil && strings.Contains(*path, "token-registry/img/") { + _, err := os.Stat(*path) + s.NoError(err) + } +} + func (s *Suite) TestImagesPresent() { assetList := tokenregistry.NibiruAssetList() for _, token := range assetList.Assets { s.Run(token.Name, func() { token = token.GitHubify() if token.LogoURIs != nil { - png, svg := token.LogoURIs.Png, token.LogoURIs.Svg - if png != nil && strings.Contains(*png, "token-registry/img/") { - _, err := os.Stat(*png) - s.NoError(err) - } - if svg != nil && strings.Contains(*svg, "token-registry/img/") { - _, err := os.Stat(*svg) - s.NoError(err) - } + s.validateImage(token.LogoURIs.Png) + s.validateImage(token.LogoURIs.Svg) } for _, img := range token.Images { - png, svg := img.Png, img.Svg - if png != nil && strings.Contains(*png, "token-registry/img/") { - _, err := os.Stat(*png) - s.NoError(err) - } - if svg != nil && strings.Contains(*svg, "token-registry/img/") { - _, err := os.Stat(*svg) - s.NoError(err) - } + s.validateImage(img.Png) + s.validateImage(img.Svg) } }) } }token-registry/main/main.go (1)
34-38
: Enhance error messagesError messages could be more descriptive to help with debugging.
prettyBz, err := json.MarshalIndent(assetList, "", " ") if err != nil { - log.Error().Msg(err.Error()) + log.Error(). + Err(err). + Str("context", "JSON marshaling"). + Msg("Failed to marshal asset list") return }justfile (1)
44-47
: Enhance gen-token-registry recipeConsider adding prerequisites and error handling to the recipe.
# Generate the Nibiru Token Registry files -gen-token-registry: +gen-token-registry: install + #!/usr/bin/env bash + source contrib/bashlib.sh + log_info "Generating token registry..." go run token-registry/main/main.go + if [ $? -eq 0 ]; then + log_success "Token registry generated successfully" + else + log_error "Failed to generate token registry" + exit 1 + fitoken-registry/token.go (2)
8-10
: Consider using generics constraint for numeric types.The
some
helper function could benefit from a type constraint to restrictT
to numeric types if that's the intended usage.-func some[T any](v T) *T { +func some[T constraints.Ordered](v T) *T {
47-51
: Handle potential JSON marshaling error.The String() method ignores potential errors from json.MarshalIndent. Consider handling the error or using a panic-safe alternative.
func (a AssetList) String() string { - jsonBz, _ := json.MarshalIndent(a, "", " ") + jsonBz, err := json.MarshalIndent(a, "", " ") + if err != nil { + return fmt.Sprintf("failed to marshal AssetList: %v", err) + } return string(jsonBz) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
token-registry/img/0000_nibiru.png
is excluded by!**/*.png
token-registry/img/0000_nibiru.svg
is excluded by!**/*.svg
token-registry/img/0001_stnibi-500x500.png
is excluded by!**/*.png
token-registry/img/0001_stnibi-500x500.svg
is excluded by!**/*.svg
token-registry/img/0002_usdc.png
is excluded by!**/*.png
token-registry/img/0002_usdc.svg
is excluded by!**/*.svg
token-registry/img/0003_astrovault-axv.png
is excluded by!**/*.png
token-registry/img/0003_astrovault-axv.svg
is excluded by!**/*.svg
📒 Files selected for processing (8)
CHANGELOG.md
(1 hunks)justfile
(1 hunks)token-registry/README.md
(1 hunks)token-registry/assetlist.go
(1 hunks)token-registry/assetlist_test.go
(1 hunks)token-registry/githubify.go
(1 hunks)token-registry/main/main.go
(1 hunks)token-registry/token.go
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- token-registry/README.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-tests
🔇 Additional comments (4)
token-registry/token.go (1)
12-39
: LGTM! Well-structured token metadata.The Token struct is well-documented with clear field descriptions and appropriate JSON tags. The optional fields are correctly marked with omitempty.
token-registry/assetlist.go (2)
3-14
: Consider adding validation for schema path.The schema path is hardcoded and might not exist. Consider adding validation or making it configurable.
16-199
: LGTM! Comprehensive token definitions.The token definitions are well-structured with appropriate metadata, including descriptions, social links, denominations, and images. The implementation follows consistent patterns across all tokens.
However, consider adding validation for image paths.
✅ Verification successful
All referenced image files exist and are properly organized
The image paths in the token definitions are valid and all referenced files are present in the
token-registry/img/
directory. The relative paths are correctly specified and follow a consistent naming pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all referenced image files exist # Test: Extract and check all image paths from the code # Find all image paths in the code and verify they exist rg -o './img/[^"]+' | while read -r img_path; do fd -t f "${img_path#./}" doneLength of output: 1360
Script:
#!/bin/bash # Check if img directory exists and list its contents echo "=== Checking img directory ===" fd -t d "img$" echo -e "\n=== Listing contents of img directory (if exists) ===" fd -t f . img/ 2>/dev/null || echo "No img directory found" echo -e "\n=== Searching for image files in the repository ===" fd -e png -e svgLength of output: 748
CHANGELOG.md (1)
61-61
: LGTM! Changelog entry follows conventions.The new entry correctly documents the token registry feature addition and follows the established format.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2144 +/- ##
==========================================
+ Coverage 64.69% 65.13% +0.44%
==========================================
Files 274 277 +3
Lines 21925 22167 +242
==========================================
+ Hits 14184 14439 +255
+ Misses 6754 6738 -16
- Partials 987 990 +3
|
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
token-registry/githubify.go (1)
28-30
:⚠️ Potential issueAdd validation for nil pointer.
The
isLocalImage
function dereferences the pointer without checking if it's nil, which could cause a panic.func isLocalImage(maybeLocal *string) bool { + if maybeLocal == nil { + return false + } return strings.HasPrefix(*maybeLocal, "./img") }
🧹 Nitpick comments (3)
token-registry/githubify.go (1)
21-24
: Consider making the GitHub base URL configurable.The hardcoded GitHub URL makes the function less flexible for testing and different environments (e.g., different branches or forks).
Consider extracting the base URL to a package-level variable or configuration:
+var githubBaseURL = "https://raw.githubusercontent.com/NibiruChain/nibiru/main/token-registry/img/" + func localImageToGitHub(local string) string { trimmed := strings.TrimPrefix(local, "./img/") - return "https://raw.githubusercontent.com/NibiruChain/nibiru/main/token-registry/img/" + trimmed + return githubBaseURL + trimmed }token-registry/assetlist_test.go (2)
21-58
: Reduce code duplication in image file checks.The PNG and SVG verification logic is duplicated multiple times. Consider extracting the common logic into a helper function.
+func (s *Suite) verifyLocalImage(path *string) { + if path != nil && strings.Contains(*path, "token-registry/img/") { + localImgPath := "./img/" + strings.Split(*path, "/img/")[1] + _, err := os.Stat(localImgPath) + s.NoError(err) + } +} + func (s *Suite) TestImagesPresent() { assetList := tokenregistry.NibiruAssetList() for _, token := range assetList.Assets { s.Run(token.Name, func() { token = token.GitHubify() if token.LogoURIs != nil { - png, svg := token.LogoURIs.Png, token.LogoURIs.Svg - if png != nil && strings.Contains(*png, "token-registry/img/") { - localImgPath := "./img/" + strings.Split(*png, "/img/")[1] - _, err := os.Stat(localImgPath) - s.NoError(err) - } - if svg != nil && strings.Contains(*svg, "token-registry/img/") { - localImgPath := "./img/" + strings.Split(*svg, "/img/")[1] - _, err := os.Stat(localImgPath) - s.NoError(err) - } + s.verifyLocalImage(token.LogoURIs.Png) + s.verifyLocalImage(token.LogoURIs.Svg) } for _, img := range token.Images { - png, svg := img.Png, img.Svg - if png != nil && strings.Contains(*png, "token-registry/img/") { - localImgPath := "./img/" + strings.Split(*png, "/img/")[1] - _, err := os.Stat(localImgPath) - s.NoError(err) - } - if svg != nil && strings.Contains(*svg, "token-registry/img/") { - localImgPath := "./img/" + strings.Split(*svg, "/img/")[1] - _, err := os.Stat(localImgPath) - s.NoError(err) - } + s.verifyLocalImage(img.Png) + s.verifyLocalImage(img.Svg) } }) } }
124-182
: Add test cases for Theme and ImageSync fields.The test suite should include cases that verify the preservation of Theme and ImageSync fields during transformation.
Add these test cases:
{ name: "Both Png and Svg are nil", input: tokenregistry.AssetImage{ Png: nil, Svg: nil, }, expected: tokenregistry.AssetImage{ Png: nil, Svg: nil, }, }, + { + name: "Preserves Theme and ImageSync fields", + input: tokenregistry.AssetImage{ + Png: some("./img/asset.png"), + Theme: some("dark"), + ImageSync: some("2024-01-01"), + }, + expected: tokenregistry.AssetImage{ + Png: some("https://raw.githubusercontent.com/NibiruChain/nibiru/main/token-registry/img/asset.png"), + Theme: some("dark"), + ImageSync: some("2024-01-01"), + }, + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
token-registry/assetlist_test.go
(1 hunks)token-registry/githubify.go
(1 hunks)token-registry/token.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- token-registry/token.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration-tests
🔇 Additional comments (5)
token-registry/githubify.go (3)
5-17
: LGTM! Well-structured implementation.The method safely handles nil checks and properly transforms both LogoURIs and Images fields while maintaining immutability by creating a copy.
32-42
: LGTM! Previous feedback addressed.The implementation now correctly preserves non-local URLs by copying them before transformation.
44-60
: LGTM! Consistent implementation.The method follows the same pattern as LogoURIs.GitHubify and correctly handles all fields, including Theme and ImageSync.
token-registry/assetlist_test.go (2)
60-62
: LGTM! Clear and focused helper function.
64-122
: LGTM! Comprehensive test coverage.Well-structured table-driven tests covering all important scenarios: local paths, external URLs, and nil values.
Purpose / Abstract
Described in prior engineering call. This change implements a typed registry for
ERC20, bank coins, CW20, and ICS20 tokens on Nibiru.
For now, it generates the JSON file for the Cosmos/chain-registry.
Run with the following
just
command.You should get this message:
Background and Related Links
Asset Registry | Fungible Tokens on Nibiru (Notion): Internal doc describing a future-proof process on how to maintain the registry
Nibiru | Fungible Token Registration (Form): Google Form for people to request new tokens to be added to the registry
Summary by CodeRabbit
Release Notes
New Features
Documentation
Improvements