Skip to content

Conversation

@whankinsiv
Copy link
Collaborator

Description

This PR refactors assets_state to index CIP25 and CIP68 metadata separately instead of overwriting CIP25 metadata with CIP68 metadata. This fixes incorrect behavior in the /addresses/{address}/extended endpoint, where the has_nft_onchain_metadata flag failed to reflect cases where an asset carries both CIP25 and CIP68 metadata. The refactor also simplifies how metadata is processed and resolved.

Related Issue(s)

#335

How was this tested?

  • Added cip25_existence_overrides_decimals() and cip68_decimals_are_extracted_when_no_cip25() unit tests in rest_blockfrost/handlers/addresses.rs.
  • Added get_asset_info_resolves_user_token_metadata_from_reference_nft() and handle_cip68_version_detection() unit tests in assets_state/src/state.rs.
  • Compared responses with Blockfrost to confirm unchanged behavior for /assets/{asset} and correct behavior for /addresses/{address}/extended outside of off-chain metadata differences.
    Note: decimals currently differs from Blockfrost due to their off-chain registry dependency. Local indexing of the registry will be implemented in a future PR.

Checklist

  • My code builds and passes local tests
  • I added/updated tests for my changes, where applicable
  • I updated documentation (if applicable)
  • CI is green for this PR

Impact / Side effects

CIP25 metadata is now stored independently rather than being overwritten by CIP68 metadata.

Reviewer notes / Areas to focus

Please focus review on resolve_cip68_metadata and handle_cip68_metadata in assets_state/src/state.rs.

Signed-off-by: William Hankins <[email protected]>

#[test]
fn resolve_cip68_metadata_overwrites_cip25_user_token_metadata() {
fn get_asset_info_resolves_user_token_metadata_from_reference_nft() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice tests 👍

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.

3 participants