Skip to content
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

Merged
merged 11 commits into from
Jan 8, 2025

Conversation

Unique-Divine
Copy link
Member

@Unique-Divine Unique-Divine commented Jan 7, 2025

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.

just gen-token-registry

You should get this message:

✅ Generation complete! See dist/assetlist.json

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

    • Introduced a strongly typed Nibiru Token Registry to manage offchain digital token metadata.
    • Added a generation command for creating Nibiru Token Registry files.
    • Enhanced capabilities for generating and tracking token assets with rich metadata.
  • Documentation

    • Updated README with details about the Token Registry functionality.
    • Enhanced changelog to reflect new token management capabilities.
  • Improvements

    • Implemented a comprehensive framework for managing token-related data, including metadata and cross-chain traceability.

@Unique-Divine Unique-Divine requested a review from a team as a code owner January 7, 2025 23:39
Copy link
Contributor

coderabbitai bot commented Jan 7, 2025

Walkthrough

This 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

File Change Summary
CHANGELOG.md Added entry for strongly typed Nibiru Token Registry implementation
justfile New recipe gen-token-registry to generate token registry files
token-registry/README.md Added section describing Nibiru Token Registry purpose
token-registry/assetlist.go Introduced NibiruAssetList() and TOKENS() functions for token management
token-registry/assetlist_test.go Added test suite to validate token image assets
token-registry/githubify.go Implemented methods to transform local image paths to GitHub URLs
token-registry/main/main.go Created main application for generating JSON asset list
token-registry/token.go Defined comprehensive token-related data structures and types

Possibly related PRs

Suggested reviewers

  • k-yang

Poem

🐰 Tokens dancing, metadata bright,
A registry born with GitHub's light,
From local paths to URLs so clear,
Nibiru's assets now sparkle here!
Code hops forward with playful might 🌟


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70df29b and 1584791.

📒 Files selected for processing (1)
  • token-registry/githubify.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • token-registry/githubify.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: integration-tests

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 configurable

The 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 logic

The 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 messages

Error 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 recipe

Consider 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
+  fi
token-registry/token.go (2)

8-10: Consider using generics constraint for numeric types.

The some helper function could benefit from a type constraint to restrict T 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

📥 Commits

Reviewing files that changed from the base of the PR and between de7239f and 89e2e10.

⛔ 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#./}"
done

Length 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 svg

Length 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.

token-registry/githubify.go Outdated Show resolved Hide resolved
token-registry/githubify.go Outdated Show resolved Hide resolved
token-registry/githubify.go Outdated Show resolved Hide resolved
token-registry/main/main.go Show resolved Hide resolved
token-registry/main/main.go Show resolved Hide resolved
token-registry/token.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 97.52066% with 6 lines in your changes missing coverage. Please review.

Project coverage is 65.13%. Comparing base (de7239f) to head (1584791).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
token-registry/githubify.go 93.18% 2 Missing and 1 partial ⚠️
token-registry/token.go 40.00% 3 Missing ⚠️
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     
Files with missing lines Coverage Δ
token-registry/assetlist.go 100.00% <100.00%> (ø)
token-registry/githubify.go 93.18% <93.18%> (ø)
token-registry/token.go 40.00% <40.00%> (ø)

... and 2 files with indirect coverage changes

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Add 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

📥 Commits

Reviewing files that changed from the base of the PR and between 89e2e10 and 70df29b.

📒 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.

@Unique-Divine Unique-Divine merged commit e5274c0 into main Jan 8, 2025
16 checks passed
@Unique-Divine Unique-Divine deleted the ud/token-registry branch January 8, 2025 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant