Add collection freeze manifests and guards#84
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughStreamCore records a deterministic collection freeze manifest hash, emits ChangesCollection Freeze Manifest & Immutability Enforcement
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 |
Contributor
Author
|
@coderabbitai review Validation already run locally:
Claude review intentionally skipped per maintainer instruction for this PR. |
|
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (1)
test/StreamMetadataFreeze.t.sol (1)
133-168: ⚡ Quick winUse explicit revert assertions for all freeze-guard checks in this test.
The generic
vm.expectRevert()calls at Line 133, Line 136, Line 141, Line 157, and Line 167 can pass on unrelated failures. Asserting the specific revert strings/errors will make these regressions precise.Example tightening
- vm.expectRevert(); + vm.expectRevert(bytes("err/freezed")); deployed.core.setCollectionData(COLLECTION_ID, address(0xA11CE), 5, 10, 1 days); - vm.expectRevert(); + vm.expectRevert(bytes("Not allowed")); deployed.core.changeMetadataView(COLLECTION_ID, true); - vm.expectRevert(); + vm.expectRevert(bytes("Not allowed")); deployed.core .updateCollectionInfo( COLLECTION_ID, "Genesis", "6529", "Description", "https://6529.io", "CC0", "ipfs://new-base/", "https://cdn.example/script.js", bytes32(0), 999999, scripts ); - vm.expectRevert(); + vm.expectRevert(bytes("Data frozen")); deployed.core.changeTokenData(TOKEN_ID, "4,5,6"); - vm.expectRevert(); + vm.expectRevert(bytes("Data frozen")); deployed.core.updateImagesAndAttributes(tokenIds, images, attributes);🤖 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 `@test/StreamMetadataFreeze.t.sol` around lines 133 - 168, Replace the five generic vm.expectRevert() calls with explicit revert assertions that match the freeze-guard revert reason emitted by the core contract: for setCollectionData, changeMetadataView, updateCollectionInfo, changeTokenData, and updateImagesAndAttributes call vm.expectRevert(abi.encodePacked("<EXACT_REVERT_STRING>")) (or vm.expectRevert(<CustomError>.selector) if the contract uses custom errors) using the exact revert message or custom error used in the freeze guard, then perform the same call; update each vm.expectRevert() at the locations around deployed.core.setCollectionData(...), deployed.core.changeMetadataView(...), deployed.core.updateCollectionInfo(...), deployed.core.changeTokenData(...), and deployed.core.updateImagesAndAttributes(...) so the test asserts the specific error rather than any revert.
🤖 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 `@ops/AUTONOMOUS_RUN.md`:
- Line 40: Update the run-state in ops/AUTONOMOUS_RUN.md to reflect the open PR
`#84` by changing the status from "Status: In progress." to "Status: Open", update
the PR URL/reference to point to PR `#84`, and set the top-level timestamp (the
`Last updated` field) to the current UTC time for this change; also make the
same corrections where the same pre-open text appears (the other occurrences
noted around the same section, previously referenced as lines 3556-3558) so the
durable state is consistent across the file.
In `@smart-contracts/StreamCore.sol`:
- Around line 904-914: The current full-range scans in
_requireLiveTokenMetadataFinal (iterating mintedCount from
collectionAdditionalData.collectionCirculationSupply and checking _exists +
tokenToHash) cause freezeCollection and the preview path to be uncallable for
large collections; instead add and maintain a per-collection pending-metadata
counter (e.g., collectionAdditionalData.pendingMetadataCount) or a
per-collection set/bitmap of pending token ids that gets updated when tokens are
minted and when tokenToHash is set, then replace the loop in
_requireLiveTokenMetadataFinal with a constant-time check against that counter
(revert CollectionHasPendingTokenMetadata if pendingMetadataCount > 0) and
update mint, setTokenHash (or equivalent metadata assignment functions) to
increment/decrement the counter atomically so freezeCollection and preview (the
code around lines ~1008-1022) no longer scan every token id.
---
Nitpick comments:
In `@test/StreamMetadataFreeze.t.sol`:
- Around line 133-168: Replace the five generic vm.expectRevert() calls with
explicit revert assertions that match the freeze-guard revert reason emitted by
the core contract: for setCollectionData, changeMetadataView,
updateCollectionInfo, changeTokenData, and updateImagesAndAttributes call
vm.expectRevert(abi.encodePacked("<EXACT_REVERT_STRING>")) (or
vm.expectRevert(<CustomError>.selector) if the contract uses custom errors)
using the exact revert message or custom error used in the freeze guard, then
perform the same call; update each vm.expectRevert() at the locations around
deployed.core.setCollectionData(...), deployed.core.changeMetadataView(...),
deployed.core.updateCollectionInfo(...), deployed.core.changeTokenData(...), and
deployed.core.updateImagesAndAttributes(...) so the test asserts the specific
error rather than any revert.
🪄 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: fc144288-75ad-4ee3-a347-c6e7950debac
📒 Files selected for processing (9)
docs/known-blockers.mddocs/metadata.mddocs/status.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdsmart-contracts/IStreamCore.solsmart-contracts/StreamCore.soltest/README.mdtest/StreamMetadataFreeze.t.sol
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
smart-contracts/StreamCore.sol (2)
321-333:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPost-freeze burns still mutate the freeze manifest surface.
burn()updates bothburnAmountand the live-token accumulator without a frozen check. Those values feed_freezeSupplyStateHash()and_liveTokenMetadataHash(), so a burn afterfreezeCollection()makespreviewCollectionFreezeManifestHash()drift away from the storedcollectionFreezeManifestHash(). If the freeze boundary is meant to remain stable, burns need the same guard or the preview path needs to return the stored hash once frozen.🤖 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 `@smart-contracts/StreamCore.sol` around lines 321 - 333, The burn() function is mutating freeze-related state (calling _removeLiveTokenMetadataRecord and incrementing burnAmount) after a collection has been frozen, which desynchronizes _freezeSupplyStateHash() and _liveTokenMetadataHash() used by previewCollectionFreezeManifestHash() and the stored collectionFreezeManifestHash(); add a frozen-check guard in burn() that prevents any state changes when the target collection is frozen (check the collection's frozen flag set by freezeCollection()), or alternatively short-circuit previewCollectionFreezeManifestHash() to return the stored collectionFreezeManifestHash() when frozen; ensure you reference and protect data mutated in burn() (burnAmount, _removeLiveTokenMetadataRecord, _burn) so frozen collections remain immutable.
471-488:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate pre-mint hashes against the collection’s token range.
The new
liveTokenbranch only enforces_collectionIDfor already-minted tokens. For pre-mint callbacks, a randomizer assigned to collection A can still writetokenToHashfor an ID in collection B’s reserved range; when B later mints it,_addLiveTokenMetadataRecord()treats that token as already final, and Line 476 prevents B’s own randomizer from correcting it. Please reject_mintIndexvalues outside_collectionID’s reserved range before accepting any pre-mint hash.Suggested fix
function setTokenHash(uint256 _collectionID, uint256 _mintIndex, bytes32 _hash) external { require(msg.sender == collectionAdditionalData[_collectionID].randomizerContract); _requireCollectionNotFrozen(_collectionID); require(_hash != bytes32(0), "Zero token hash"); + uint256 minTokenId = collectionAdditionalData[_collectionID].reservedMinTokensIndex; + uint256 maxTokenId = collectionAdditionalData[_collectionID].reservedMaxTokensIndex; + require(_mintIndex >= minTokenId && _mintIndex <= maxTokenId, "Wrong collection"); require( tokenToHash[_mintIndex] == 0x0000000000000000000000000000000000000000000000000000000000000000 ); bool liveToken = _exists(_mintIndex);🤖 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 `@smart-contracts/StreamCore.sol` around lines 471 - 488, The setTokenHash path allows pre-mint writes to tokenToHash for IDs outside the target collection; add a check that _mintIndex falls inside _collectionID’s reserved token ID range before accepting any hash. Use the collection metadata in collectionAdditionalData[_collectionID] (e.g., its start token id and size/last id fields) to compute the valid range and add a require(...) that _mintIndex is >= start and <= end; keep this validation alongside the existing _requireCollectionNotFrozen and before writing tokenToHash so both pre-mint and live-token flows are guarded.
🤖 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.
Outside diff comments:
In `@smart-contracts/StreamCore.sol`:
- Around line 321-333: The burn() function is mutating freeze-related state
(calling _removeLiveTokenMetadataRecord and incrementing burnAmount) after a
collection has been frozen, which desynchronizes _freezeSupplyStateHash() and
_liveTokenMetadataHash() used by previewCollectionFreezeManifestHash() and the
stored collectionFreezeManifestHash(); add a frozen-check guard in burn() that
prevents any state changes when the target collection is frozen (check the
collection's frozen flag set by freezeCollection()), or alternatively
short-circuit previewCollectionFreezeManifestHash() to return the stored
collectionFreezeManifestHash() when frozen; ensure you reference and protect
data mutated in burn() (burnAmount, _removeLiveTokenMetadataRecord, _burn) so
frozen collections remain immutable.
- Around line 471-488: The setTokenHash path allows pre-mint writes to
tokenToHash for IDs outside the target collection; add a check that _mintIndex
falls inside _collectionID’s reserved token ID range before accepting any hash.
Use the collection metadata in collectionAdditionalData[_collectionID] (e.g.,
its start token id and size/last id fields) to compute the valid range and add a
require(...) that _mintIndex is >= start and <= end; keep this validation
alongside the existing _requireCollectionNotFrozen and before writing
tokenToHash so both pre-mint and live-token flows are guarded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52aa3f07-00b0-40fa-881d-73cff602c48d
📒 Files selected for processing (4)
docs/metadata.mdops/AUTONOMOUS_RUN.mdsmart-contracts/StreamCore.soltest/StreamMetadataFreeze.t.sol
✅ Files skipped from review due to trivial changes (1)
- docs/metadata.md
🚧 Files skipped from review as they are similar to previous changes (2)
- ops/AUTONOMOUS_RUN.md
- test/StreamMetadataFreeze.t.sol
This was referenced Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs #47.
This PR implements the first
P1-META-002collection freeze-boundary slice forStreamCore.It adds:
CollectionFrozen(collectionId, manifestHash, schemaVersion, admin)StreamCoremetadata-significant pathsStreamMetadataFreeze.t.solcoverageIssue #47 remains open because dependency content immutability is broader than this slice. Immutable dependency version records and registry provenance remain
P1-META-003/ #48.Validation
forge test --match-contract StreamMetadataFreezeTest -vvvmake checkpowershell -ExecutionPolicy Bypass -File scripts\check.ps1forge fmt --check smart-contracts\StreamCore.sol smart-contracts\IStreamCore.sol test\StreamMetadataFreeze.t.solgit diff --checkHigh=4,Medium=19unchanged; total findings718Review Notes
Summary by CodeRabbit
New Features
Documentation
Tests