Add schema-v1 metadata state outputs#83
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR exposes metadata schema-v1/state in StreamCore, refactors tokenURI to produce base64 JSON data URIs with ChangesMetadata Schema-v1 and State Exposure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
|
@coderabbitai review Please review latest head Local validation on this branch: |
|
✅ Action performedFull review 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: 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 539-540: The invariant was changed so bytes32(0) now means
"pending", but setTokenHash still allows a zero hash; update the setter to
enforce finality by rejecting zero values: in setTokenHash add a guard that
requires _hash != bytes32(0) (or alternatively add a separate boolean/enum to
track finality and set that when writing any hash) so that
_isTokenMetadataFinal, tokenMetadataState, and tokenURI will not treat a
completed token as pending when a randomizer callback writes the hash.
🪄 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: e26c6c29-13c8-468b-bb5c-7c8f65d46b66
📒 Files selected for processing (12)
docs/known-blockers.mddocs/metadata.mddocs/status.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdsmart-contracts/StreamCore.soltest/README.mdtest/StreamMetadataGolden.t.soltest/fixtures/metadata/current-onchain-final-token-uri.txttest/fixtures/metadata/current-onchain-pending-token-uri.txttest/fixtures/metadata/onchain-final-schema-v1-token-uri.txttest/fixtures/metadata/onchain-pending-schema-v1-token-uri.txt
💤 Files with no reviewable changes (2)
- test/fixtures/metadata/current-onchain-pending-token-uri.txt
- test/fixtures/metadata/current-onchain-final-token-uri.txt
|
@coderabbitai review Please review latest head Follow-up change:
Local validation after the fix: |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect setTokenHash for bytes32(0) guard
rg -n "setTokenHash" smart-contracts/StreamCore.sol -A 10
echo "---"
# Look for the zero-hash revert in the contract
rg -n "bytes32(0)\|zero\|ZeroHash\|ZERO_HASH\|zero.hash\|revert.*[Hh]ash\|require.*hash" smart-contracts/StreamCore.sol -A 2
echo "---"
# Check the new test
rg -n "zero\|revert\|ZeroHash" test/StreamMetadataGolden.t.sol -A 5 -B 2
echo "---"
# Show AUTONOMOUS_RUN additions
tail -60 ops/AUTONOMOUS_RUN.mdLength of output: 13907 🏁 Script executed: #!/bin/bash
echo "=== setTokenHash full context ==="
rg -n "setTokenHash" smart-contracts/StreamCore.sol -A 15 -B 2
echo ""
echo "=== New regression test in StreamMetadataGolden.t.sol ==="
cat test/StreamMetadataGolden.t.sol
echo ""
echo "=== AUTONOMOUS_RUN PR `#83` section ==="
rg -n "PR `#83`\|3dc56d6\|PRRT_kwDOM7REis6IpsTo\|zero.hash\|zero.*hash\|hash.*zero\|ZeroHash" ops/AUTONOMOUS_RUN.md -A 3 -B 1
echo ""
echo "=== Check error message consistency - all revert strings around hash ==="
rg -n "require\|revert\|error" smart-contracts/StreamCore.sol | grep -i "hash\|zero"
echo ""
echo "=== Check NoopRandomizer / MockRandomizer for setTokenHash call ==="
rg -n "setTokenHash\|NoopRandomizer\|MockRandomizer\|ConfiguredRandomizer" test/ -A 5 -B 2
echo ""
echo "=== StreamCore.sol full setTokenHash + _isTokenMetadataFinal area ==="
rg -n "_isTokenMetadataFinal\|tokenToHash\|METADATA_STATE" smart-contracts/StreamCore.sol -A 3 -B 1Length of output: 8972
require(_hash != bytes32(0), "Zero token hash");Correctly placed — before the slot-check and the assignment. The caller check → zero-hash check → existing-slot check → write ordering is all correct. One minor style note: the existing slot-check at lines 409–412 uses the full New regression test
One nit on the assertion label: deployed.core.retrieveTokenHash(TOKEN_ID).assertEq(bytes32(0), "zero hash stored");This reads ambiguously — "zero hash stored" sounds like it describes a successful zero-hash write rather than "hash should still be zero/unset after rejected write." Something like Overall the fix is correct and the test coverage is comprehensive. Triggering the full review now. ✅ Action performedFull review finished. |
|
@coderabbitai review Please review final head Delta since
Validation after this final cleanup: |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== Verify assertion message fix in regression test ==="
rg -n "zero hash\|unset\|rejection\|pending.*reject\|token hash should" test/StreamMetadataGolden.t.sol -A 2 -B 2
echo ""
echo "=== Full regression test context ==="
rg -n "testSetTokenHashRejectsZeroHash\|ZeroHash\|expectRevert\|retrieveTokenHash\|tokenMetadataState" test/StreamMetadataGolden.t.sol -A 3 -B 1
echo ""
echo "=== AUTONOMOUS_RUN.md tail for final state ==="
tail -40 ops/AUTONOMOUS_RUN.md
echo ""
echo "=== Confirm setTokenHash guard still in place ==="
rg -n "setTokenHash\|Zero token hash\|bytes32(0)" smart-contracts/StreamCore.sol -A 5 -B 2Length of output: 8990 🏁 Script executed: #!/bin/bash
echo "=== test/StreamMetadataGolden.t.sol full content ==="
cat test/StreamMetadataGolden.t.sol
echo ""
echo "=== setTokenHash and zero guard in StreamCore.sol ==="
grep -n "setTokenHash\|Zero token hash\|bytes32(0)\|_isTokenMetadataFinal\|tokenToHash" smart-contracts/StreamCore.sol | head -40
echo ""
echo "=== Last 80 lines of ops/AUTONOMOUS_RUN.md ==="
tail -80 ops/AUTONOMOUS_RUN.mdLength of output: 25284
Assertion label fix (the raised nit) — confirmed in // before (3dc56d6)
.assertEq(bytes32(0), "zero hash stored");
// after (78664b0)
.assertEq(bytes32(0), "token hash should remain unset after zero-hash rejection");Both post-revert assertions are now unambiguous — the hash check and the Everything else (guard ordering at line 408, ✅ Action performedReview finished.
|
|
Merge readiness for PR #83:
|
1 participant
Summary
Adds the next P1-META-001 metadata-schema slice: schema-versioned on-chain metadata output with explicit pending/final state.
This PR:
METADATA_SCHEMA_VERSION = "6529stream-v1"metadataSchemaVersion()andtokenMetadataState(tokenId)viewstokenURIJSON from raw UTF-8 data URIs to base64 JSON data URIsmetadata_schema_versionandmetadata_statefields to on-chain JSONRefs #46.
Roadmap
Security And Maturity
Review Routing
Validation
Local commands run:
make checkor platform equivalent passed.Impact
StreamMetadataGolden.t.soland metadata fixtures for schema-v1 pending/final on-chain output.Reviewer Notes
CodeRabbit-only review for this PR. Claude is intentionally skipped per maintainer instruction.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests