Add burn metadata semantics#86
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@coderabbitai review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR implements burned-token audit retention and audit-only post-burn randomness recording: contracts persist burn audit state and expose ChangesBurn metadata semantics implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@smart-contracts/StreamCore.sol`:
- Around line 372-382: The burnedTokenAuditRecords.burned flag is left set when
a token ID is burned but the contract allows that same tokenId to be reminted,
causing isTokenBurned to remain true for a live token; update the minting paths
(e.g., mint, any wrapper that calls _mint or _safeMint) to check
burnedTokenAuditRecords[_tokenId].burned and revert if true
(require(!burnedTokenAuditRecords[_tokenId].burned, "token previously burned")),
or alternatively, if reminting is intended, reset the BurnedTokenAudit fields
for _tokenId on mint; ensure consistency with isTokenBurned and any burn-related
events by referencing burnedTokenAuditRecords, the BurnedTokenAudit struct, and
the mint/_mint/_safeMint 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21ee5310-ac4e-47aa-99ec-a918cff30d4d
📒 Files selected for processing (15)
docs/adr/0005-randomness.mddocs/adr/0006-metadata-freeze.mddocs/known-blockers.mddocs/metadata.mddocs/status.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdsmart-contracts/IStreamCore.solsmart-contracts/RandomizerRNG.solsmart-contracts/RandomizerVRF.solsmart-contracts/StreamCore.solsmart-contracts/StreamRandomizerLifecycle.soltest/README.mdtest/StreamCoreBurn.t.soltest/mocks/MockRandomizerCore.sol
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
TokenBurnedevent while preserving ERC-721 post-burn ownership/tokenURI semanticsCloses #50.
Validation
forge test --match-contract StreamCoreBurnTest -vvv(4 passed)forge test --match-contract "Stream(RandomizerLifecycle|RandomizerRetry|MetadataEvents|MetadataFreeze|MetadataGolden|CoreBurn)Test" -vvv(55 passed)make checkpowershell -ExecutionPolicy Bypass -File scripts\check.ps1forge fmt --check smart-contracts\StreamCore.sol smart-contracts\IStreamCore.sol smart-contracts\StreamRandomizerLifecycle.sol smart-contracts\RandomizerVRF.sol smart-contracts\RandomizerRNG.sol test\mocks\MockRandomizerCore.sol test\StreamCoreBurn.t.solgit diff --check718total findings, high/medium unchanged at4/19Summary by CodeRabbit
New Features
Documentation
Tests