[KLC-2332] Override KLV and KFI asset metadata in GetAsset response#40
[KLC-2332] Override KLV and KFI asset metadata in GetAsset response#40Beroni wants to merge 4 commits into
Conversation
WalkthroughGetAsset in network/api/asset/routes.go now applies a post-fetch mutation via a new replaceKDAInfo helper: when asset.ID equals KLV or KFI it overwrites asset.Logo and replaces asset.URIs with fixed external links; nil guards and early returns preserve non-matching assets. ChangesAsset response + tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the node REST API GET /asset/:id response to override stale on-chain metadata (logo + URIs) for the two native assets KLV and KFI, ensuring clients receive current canonical values without requiring a hard fork.
Changes:
- Added
KLVAssetID/KFIAssetIDconstants in the asset API routes. - Added
replaceKDAInfohelper to overrideLogoandURIsforKLVandKFI. - Updated
GetAssetto callreplaceKDAInfobefore serializing the response.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@network/api/asset/routes.go`:
- Around line 113-140: Add regression tests for the replaceKDAInfo helper to
ensure the KLV/KFI override path never regresses: create unit tests that call
replaceKDAInfo with a kapps.KDAData instance having ID == KLVAssetID and another
with ID == KFIAssetID and assert their Logo and URIs are replaced to the
expected values (KLV -> klv-logo.png, KFI -> kfi-logo.png and the full URIs
map), and include a third test using a non-matching ID to assert the Logo and
URIs remain unchanged; locate replaceKDAInfo and the constants
KLVAssetID/KFIAssetID to build the inputs and assertions.
- Around line 21-22: The KLVAssetID and KFIAssetID constants are duplicated
here; remove the local declarations for KLVAssetID and KFIAssetID in routes.go
and instead use the canonical constants from the existing package that defines
them (import that package and replace local uses with package.KLVAssetID and
package.KFIAssetID), or if they truly belong to a shared domain, move the
canonical definitions into a shared package and update references (including
functions/handlers that reference KLVAssetID/KFIAssetID) to import and reference
the single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 122f211d-a9f6-485f-9ba0-ac64b892929f
📒 Files selected for processing (1)
network/api/asset/routes.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Analyze (go)
- GitHub Check: setup-and-lint / setup-and-lint
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: Verify that any new or modified concurrent code (goroutines, channels, mutexes, sync primitives) is free of race conditions. Check for: proper lock/unlock pairing, no goroutine leaks, correct channel lifecycle management, and proper context cancellation propagation.
Verify that errors are not silently discarded. Check for: unchecked error returns, error wrapping with context, proper error propagation up the call chain, and no bare panic() calls outside of init() functions.
Files:
network/api/asset/routes.go
network/**
⚙️ CodeRabbit configuration file
network/**: Peer-to-peer networking layer. - Check for proper input validation on all received messages - Verify rate limiting and DoS protection mechanisms - Ensure connection handling is goroutine-safe - Look for potential message amplification attacks - Verify TLS/authentication on peer connections
Files:
network/api/asset/routes.go
🧠 Learnings (1)
📚 Learning: 2026-04-21T20:12:22.959Z
Learnt from: phcarneirobc
Repo: klever-io/klever-go PR: 38
File: indexer/eventsProcessor.go:188-211
Timestamp: 2026-04-21T20:12:22.959Z
Learning: In Go structs that are JSON-marshaled, if a field is a `bool` and has the `json:"...,omitempty"` tag, then leaving that field at its zero value (`false`) is functionally equivalent (in the resulting JSON) to explicitly setting `Foundation: false`. Reviewers should not flag struct literals that omit such `bool` fields as an inconsistency; they will serialize identically because `omitempty` suppresses `false` values.
Applied to files:
network/api/asset/routes.go
🔇 Additional comments (1)
network/api/asset/routes.go (1)
100-140: No change needed.The
GetAssetmethod allocates a fresh&kapps.KDAData{}struct on each call and deserializes into it before returning. There is no pointer caching or reuse across requests. Mutations inreplaceKDAInfoaffect only the caller's response object, not any shared state. The concern about branding leaking into later requests does not apply here.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@network/api/asset/routes_test.go`:
- Around line 175-176: The calls to wrapper.NewRouterWrapper are ignoring
returned errors (e.g., the call that assigns assetRoute and the other
occurrence), which can hide config problems; change both sites to capture the
error (e.g., assetRoute, err := wrapper.NewRouterWrapper(...)) and fail the test
immediately with a clear message and wrapped error (e.g., using t.Fatalf or
require.NoError) so route setup errors propagate instead of being discarded
before calling asset.Routes or other route setup functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 95e1154d-6cf8-4e33-88fe-d9e3e19d4008
📒 Files selected for processing (1)
network/api/asset/routes_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: Verify that any new or modified concurrent code (goroutines, channels, mutexes, sync primitives) is free of race conditions. Check for: proper lock/unlock pairing, no goroutine leaks, correct channel lifecycle management, and proper context cancellation propagation.
Verify that errors are not silently discarded. Check for: unchecked error returns, error wrapping with context, proper error propagation up the call chain, and no bare panic() calls outside of init() functions.
Files:
network/api/asset/routes_test.go
network/**
⚙️ CodeRabbit configuration file
network/**: Peer-to-peer networking layer. - Check for proper input validation on all received messages - Verify rate limiting and DoS protection mechanisms - Ensure connection handling is goroutine-safe - Look for potential message amplification attacks - Verify TLS/authentication on peer connections
Files:
network/api/asset/routes_test.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Test files. Review for: - Adequate coverage of edge cases and error paths - Proper use of test helpers and assertions - Race condition coverage (tests should use -race flag patterns) - No hardcoded sleep for synchronization (use channels or sync primitives) - Test isolation (no shared mutable state between tests)
Files:
network/api/asset/routes_test.go
🧠 Learnings (1)
📚 Learning: 2026-04-21T20:12:22.959Z
Learnt from: phcarneirobc
Repo: klever-io/klever-go PR: 38
File: indexer/eventsProcessor.go:188-211
Timestamp: 2026-04-21T20:12:22.959Z
Learning: In Go structs that are JSON-marshaled, if a field is a `bool` and has the `json:"...,omitempty"` tag, then leaving that field at its zero value (`false`) is functionally equivalent (in the resulting JSON) to explicitly setting `Foundation: false`. Reviewers should not flag struct literals that omit such `bool` fields as an inconsistency; they will serialize identically because `omitempty` suppresses `false` values.
Applied to files:
network/api/asset/routes_test.go
🔇 Additional comments (1)
network/api/asset/routes_test.go (1)
37-163: Good coverage for override and passthrough behavior.This table-driven test exercises the key success and failure paths for
GetAsset(non-native passthrough, KLV/KFI overrides, facade error) and validates response envelope fields.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
network/api/asset/routes.go (1)
21-23: 🛠️ Refactor suggestion | 🟠 MajorUse canonical KDA identifiers instead of local string IDs
Line 21-22 and Line 121-123 introduce a second source of truth for KLV/KFI IDs and rely on
string(asset.ID)matching. Preferkdautils.KLVIdentifier/kdautils.KFIIdentifierwithbytes.Equalto avoid drift and keep ID handling consistent with the rest of the codebase.Proposed refactor
import ( + "bytes" "fmt" "net/http" "github.com/gin-gonic/gin" kdafeespool "github.com/klever-io/klever-go/core/kapp/kdaFeesPool" + "github.com/klever-io/klever-go/core/process/kda/kdautils" "github.com/klever-io/klever-go/kapps" "github.com/klever-io/klever-go/network/api/errors" "github.com/klever-io/klever-go/network/api/shared" "github.com/klever-io/klever-go/network/api/wrapper" ) @@ - klvAssetID = "KLV" - kfiAssetID = "KFI" ) @@ - id := string(asset.ID) - if id != klvAssetID && id != kfiAssetID { + isKLV := bytes.Equal(asset.ID, kdautils.KLVIdentifier) + isKFI := bytes.Equal(asset.ID, kdautils.KFIIdentifier) + if !isKLV && !isKFI { return } asset.Logo = "https://kleverscan.org/assets/klv-logo.png" - if id == kfiAssetID { + if isKFI { asset.Logo = "https://kleverscan.org/assets/kfi-logo.png" }Also applies to: 121-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@network/api/asset/routes.go` around lines 21 - 23, Replace the local string constants klvAssetID/kfiAssetID and any use of string(asset.ID) comparisons with the canonical byte identifiers kdautils.KLVIdentifier and kdautils.KFIIdentifier and compare using bytes.Equal; specifically, remove or stop relying on klvAssetID/kfiAssetID, update places that do string(asset.ID) == klvAssetID / == kfiAssetID (e.g., the comparison logic around asset.ID) to use bytes.Equal(asset.ID, kdautils.KLVIdentifier) and bytes.Equal(asset.ID, kdautils.KFIIdentifier) so ID handling matches the rest of the codebase and avoids drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@network/api/asset/routes_test.go`:
- Around line 129-130: The tests currently discard errors returned by
http.NewRequest (calls creating req at the GET /asset tests around the req
creation lines); change both occurrences to capture the error and fail the test
on error (e.g., err := http.NewRequest(...); if err != nil { t.Fatalf("failed to
create request: %v", err) }) or use require.NoError(t, err) if you prefer
testify — update the creation of req in the test function(s) that use tc.assetID
and the similar request at line ~180 to handle the error explicitly.
---
Duplicate comments:
In `@network/api/asset/routes.go`:
- Around line 21-23: Replace the local string constants klvAssetID/kfiAssetID
and any use of string(asset.ID) comparisons with the canonical byte identifiers
kdautils.KLVIdentifier and kdautils.KFIIdentifier and compare using bytes.Equal;
specifically, remove or stop relying on klvAssetID/kfiAssetID, update places
that do string(asset.ID) == klvAssetID / == kfiAssetID (e.g., the comparison
logic around asset.ID) to use bytes.Equal(asset.ID, kdautils.KLVIdentifier) and
bytes.Equal(asset.ID, kdautils.KFIIdentifier) so ID handling matches the rest of
the codebase and avoids drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0bed3b72-21a9-477b-8df0-7977600d0b2b
📒 Files selected for processing (2)
network/api/asset/routes.gonetwork/api/asset/routes_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup-and-lint / setup-and-lint
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: Verify that any new or modified concurrent code (goroutines, channels, mutexes, sync primitives) is free of race conditions. Check for: proper lock/unlock pairing, no goroutine leaks, correct channel lifecycle management, and proper context cancellation propagation.
Verify that errors are not silently discarded. Check for: unchecked error returns, error wrapping with context, proper error propagation up the call chain, and no bare panic() calls outside of init() functions.
Files:
network/api/asset/routes.gonetwork/api/asset/routes_test.go
network/**
⚙️ CodeRabbit configuration file
network/**: Peer-to-peer networking layer. - Check for proper input validation on all received messages - Verify rate limiting and DoS protection mechanisms - Ensure connection handling is goroutine-safe - Look for potential message amplification attacks - Verify TLS/authentication on peer connections
Files:
network/api/asset/routes.gonetwork/api/asset/routes_test.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Test files. Review for: - Adequate coverage of edge cases and error paths - Proper use of test helpers and assertions - Race condition coverage (tests should use -race flag patterns) - No hardcoded sleep for synchronization (use channels or sync primitives) - Test isolation (no shared mutable state between tests)
Files:
network/api/asset/routes_test.go
🧠 Learnings (1)
📚 Learning: 2026-04-21T20:12:22.959Z
Learnt from: phcarneirobc
Repo: klever-io/klever-go PR: 38
File: indexer/eventsProcessor.go:188-211
Timestamp: 2026-04-21T20:12:22.959Z
Learning: In Go structs that are JSON-marshaled, if a field is a `bool` and has the `json:"...,omitempty"` tag, then leaving that field at its zero value (`false`) is functionally equivalent (in the resulting JSON) to explicitly setting `Foundation: false`. Reviewers should not flag struct literals that omit such `bool` fields as an inconsistency; they will serialize identically because `omitempty` suppresses `false` values.
Applied to files:
network/api/asset/routes.gonetwork/api/asset/routes_test.go
🔇 Additional comments (1)
network/api/asset/routes_test.go (1)
38-164: Good regression coverage for KLV/KFI override behaviorThis table-driven suite correctly pins the KLV/KFI rewrite path and the non-native pass-through path, including response payload validation for
LogoandURIs.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
network/api/asset/routes_test.go (1)
32-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle request-construction errors in this test path.
Line 32 discards the
http.NewRequesterror, which can mask setup failures and produce misleading assertions.Suggested fix
- req, _ := http.NewRequest("GET", "/asset", nil) + req, err := http.NewRequest("GET", "/asset", nil) + require.NoError(t, err)As per coding guidelines, "
**/*.go: Verify that errors are not silently discarded. Check for: unchecked error returns..."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@network/api/asset/routes_test.go` at line 32, The test currently ignores the error returned by http.NewRequest (req, _ := http.NewRequest(...)); update the test in routes_test.go to check the returned error from http.NewRequest and fail the test on error (e.g., if err != nil { t.Fatalf("failed to create request: %v", err) } or use your test helper like require.NoError(t, err)), ensuring the request construction error is not silently discarded and the rest of the test uses the validated req variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@network/api/asset/routes_test.go`:
- Around line 56-58: The tests share the staleURIs map between the KLV and KFI
table cases which breaks test isolation; update the test table to create a fresh
map instance for each case (e.g. construct staleURIs inside each test case entry
or in the per-case setup) instead of reusing the top-level staleURIs variable so
KLV and KFI get independent maps; this keeps behavior correct even if
replaceKDAInfo() changes from reassigning the URIs field to mutating it in place
and prevents cross-case flakiness.
---
Duplicate comments:
In `@network/api/asset/routes_test.go`:
- Line 32: The test currently ignores the error returned by http.NewRequest
(req, _ := http.NewRequest(...)); update the test in routes_test.go to check the
returned error from http.NewRequest and fail the test on error (e.g., if err !=
nil { t.Fatalf("failed to create request: %v", err) } or use your test helper
like require.NoError(t, err)), ensuring the request construction error is not
silently discarded and the rest of the test uses the validated req variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a6931ff-8453-42d2-8383-fa5ab419c7b4
📒 Files selected for processing (1)
network/api/asset/routes_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup-and-lint / setup-and-lint
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: Verify that any new or modified concurrent code (goroutines, channels, mutexes, sync primitives) is free of race conditions. Check for: proper lock/unlock pairing, no goroutine leaks, correct channel lifecycle management, and proper context cancellation propagation.
Verify that errors are not silently discarded. Check for: unchecked error returns, error wrapping with context, proper error propagation up the call chain, and no bare panic() calls outside of init() functions.
Files:
network/api/asset/routes_test.go
network/**
⚙️ CodeRabbit configuration file
network/**: Peer-to-peer networking layer. - Check for proper input validation on all received messages - Verify rate limiting and DoS protection mechanisms - Ensure connection handling is goroutine-safe - Look for potential message amplification attacks - Verify TLS/authentication on peer connections
Files:
network/api/asset/routes_test.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Test files. Review for: - Adequate coverage of edge cases and error paths - Proper use of test helpers and assertions - Race condition coverage (tests should use -race flag patterns) - No hardcoded sleep for synchronization (use channels or sync primitives) - Test isolation (no shared mutable state between tests)
Files:
network/api/asset/routes_test.go
🧠 Learnings (1)
📚 Learning: 2026-04-21T20:12:22.959Z
Learnt from: phcarneirobc
Repo: klever-io/klever-go PR: 38
File: indexer/eventsProcessor.go:188-211
Timestamp: 2026-04-21T20:12:22.959Z
Learning: In Go structs that are JSON-marshaled, if a field is a `bool` and has the `json:"...,omitempty"` tag, then leaving that field at its zero value (`false`) is functionally equivalent (in the resulting JSON) to explicitly setting `Foundation: false`. Reviewers should not flag struct literals that omit such `bool` fields as an inconsistency; they will serialize identically because `omitempty` suppresses `false` values.
Applied to files:
network/api/asset/routes_test.go
🔇 Additional comments (1)
network/api/asset/routes_test.go (1)
202-204: Good fail-fast setup for router wrapper initialization.Using
require.NoErrorhere is the right pattern to surface route setup failures immediately.
|
deferred, old finance domain have been redirected via NS |
Summary
Override the KLV and KFI asset metadata in the
GetAssetnodeAPI response so clients receive up-to-date logo and URI values
instead of the stale ones registered at genesis.
Problem
The KLV and KFI assets had their
LogoandURIsregistered atgenesis and have since become outdated. Updating those fields
on-chain would require a hard fork, so the
GET /asset/:idendpoint kept returning incorrect metadata for the two native
assets.
Solution
Patch the response in the API layer: after fetching the asset,
if the ID is
KLVorKFI, replace theLogoandURIswiththe current canonical values before serializing the response.
All other assets are returned untouched.
Key Changes
replaceKDAInfohelper innetwork/api/asset/routes.gothat overrides
LogoandURIsforKLVandKFI.KLVAssetIDandKFIAssetIDconstants innetwork/api/asset/routes.go.GetAssetinnetwork/api/asset/routes.gonow callsreplaceKDAInfobefore returning the response.Testing
make tests)GET /asset/KLVandGET /asset/KFIagainst a localnode and confirmed the logo URL and URIs match the expected
values.
GET /asset/<other-asset>and confirmed the response isunchanged.
Configuration Changes
None.
Breaking Changes
None. The override is response-only and applies to two
well-known asset IDs; the data shape is unchanged.
Related Issues
Fixes # KLC-2332
Related to #
Checklist
make goimports)Commits
Scope and Impact Assessment
This change is non-consensus and strictly API-layer. It patches HTTP responses for two well-known native asset IDs (KLV, KFI) by replacing Logo and URIs after the asset is fetched and before JSON serialization. It does not modify on-chain state, transaction processing, consensus, KVM, or peer networking. Node stability and data integrity are not impacted because the mutation is local to the request-scoped response object.
Changes
Implementation details relevant to safety and cross-cutting concerns
Blockchain-critical impact
Testing
Configuration / Breaking changes
Related issue