[#41] Royalty reduction, remove sunset, struct optimization#46
[#41] Royalty reduction, remove sunset, struct optimization#46realproject7 merged 1 commit intomainfrom
Conversation
1. Reduce mint/burn royalty: 500 (5%) → 100 (1%) basis points 2. Remove `sunset` bool from Storyline struct; derive via new `hasSunset()` view (hasDeadline && time > lastPlotTime + 168h) 3. Optimize struct from 5 slots to 2: plotCount uint256→uint24, lastPlotTime uint256→uint40, pack with token+hasDeadline in slot 2 4. Remove `require(!s.sunset)` from chainPlot — deadline check covers it 5. Update E2E scripts for new 5-field struct destructuring 6. Add tests: hasSunset (4 cases), royaltyConstants (1 case) All 32 tests pass. Fixes #41 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b Review — APPROVED
Solid contract change. Key checks:
Struct packing — uint24 plotCount (max 16.7M plots) and uint40 lastPlotTime (max year ~36812) are both safe for practical use. Writer (160b) fills slot 1; token (160b) + plotCount (24b) + lastPlotTime (40b) + hasDeadline (8b) = 232 bits in slot 2. Clean packing.
Sunset removal — sunset was never set by contract logic (only via vm.store in tests), so removing it and deriving via hasSunset() view is a safe refactor. The deadline check in chainPlot() already covers the behavior.
Arithmetic safety — uint256(s.lastPlotTime) + 168 hours correctly upcasts before addition. uint40(block.timestamp) is safe for centuries.
Test coverage — 4 hasSunset() tests cover all paths (no deadline, within, expired, reset after chainPlot). Royalty constant test confirms 100bp.
E2E scripts — All struct destructuring correctly updated to 5-field with matching types.
LGTM.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The PR implements the three coupled changes from issue #41 together: royalty reduction, sunset removal via derived view logic, and the struct packing update with the corresponding type/destructuring changes. The test updates cover the new hasSunset() behavior and the revised royalty constants, and GitHub checks are passing.
Findings
- None.
Decision
Approving because the change set is internally consistent, follows the issue scope, and the updated tests cover the behavior introduced here.
Summary
bool sunsetfrom Storyline struct; addedhasSunset()view that derives fromhasDeadline && block.timestamp > lastPlotTime + 168hplotCountuint256→uint24,lastPlotTimeuint256→uint40, packed into slot 2 withtokenandhasDeadlinerequire(!s.sunset, "Storyline sunset")fromchainPlot()— the existing deadline check covers thisTest plan
hasSunset()tests: no deadline, within deadline, expired, reset after chainPlotroyaltyConstants()test: asserts MINT_ROYALTY=100, BURN_ROYALTY=100forge fmt --checkcleanFixes #41
🤖 Generated with Claude Code