Skip to content

fix(tokens): l-03 tokens uris presist accross burn and remint#436

Merged
0xisk merged 1 commit intomainfrom
fix/l-03
Apr 10, 2026
Merged

fix(tokens): l-03 tokens uris presist accross burn and remint#436
0xisk merged 1 commit intomainfrom
fix/l-03

Conversation

@0xisk
Copy link
Copy Markdown
Member

@0xisk 0xisk commented Apr 9, 2026

Types of changes

What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Fixes #427

PR Checklist

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Summary by CodeRabbit

  • Bug Fixes

    • Token burning now properly removes associated metadata, ensuring complete cleanup when tokens are destroyed.
  • Tests

    • Added test validation confirming that stored metadata is correctly cleared when tokens are burned and reminted.

@0xisk 0xisk requested review from a team as code owners April 9, 2026 13:51
@0xisk 0xisk self-assigned this Apr 9, 2026
@0xisk 0xisk requested a review from andrew-fleming April 9, 2026 13:51
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7f296440-318e-445e-9f88-47c9ff90fed7

📥 Commits

Reviewing files that changed from the base of the PR and between 25aa666 and dc3c679.

📒 Files selected for processing (2)
  • contracts/src/token/NonFungibleToken.compact
  • contracts/src/token/test/nonFungibleToken.test.ts

Walkthrough

The pull request fixes a token URI persistence issue by updating the _burn() function to delete stored token URIs when tokens are burned, preventing stale metadata from being inherited by reminted tokens. A corresponding test verifies this behavior.

Changes

Cohort / File(s) Summary
NFT Burn Logic
contracts/src/token/NonFungibleToken.compact
Added deletion of _tokenURIs entry in _burn() to prevent URI persistence after token burn.
Burn Test Coverage
contracts/src/token/test/nonFungibleToken.test.ts
Added test case verifying that burning a token clears its stored URI, with token remint confirming the URI is empty.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A token burns, its memory too—
No stale URIs haunting what's new,
Fresh mint, clean slate, the old is gone,
In Compact's flame, we move along! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: clearing token URIs on burn to prevent persistence across burn and remint cycles.
Linked Issues check ✅ Passed The PR successfully implements the core requirement from issue #427: clearing the stored token URI when a token is burned, and includes a test verifying the fix.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #427; the contract modification clears URIs on burn and the test validates this behavior without extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/l-03

Comment @coderabbitai help to get the list of available commands and usage tips.

@0xisk 0xisk added the audit Issues reported by an audit label Apr 9, 2026
@0xisk 0xisk moved this to Needs Review in OZ Development for Midnight Apr 9, 2026
Copy link
Copy Markdown
Contributor

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

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

LGTM!

@0xisk 0xisk merged commit e8fec17 into main Apr 10, 2026
10 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Complete/Stable Release in OZ Development for Midnight Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit Issues reported by an audit

Projects

Status: Complete/Stable Release

Development

Successfully merging this pull request may close these issues.

L-03: Token URIs Persist Across Burn and Remint

2 participants