[Contract] StoryFactory Review Fixes — Royalty, Struct Optimization, CEI, Validations

[realproject7]

Summary

Contract review feedback from team + automated security audit findings. All points are valid and should be applied before next deployment. This requires a contract redeploy after changes.

---

Changes

1. Reduce Mint/Burn Royalty: 5% → 1%

// Before
uint16 public constant MINT_ROYALTY = 500;
uint16 public constant BURN_ROYALTY = 500;

// After
uint16 public constant MINT_ROYALTY = 100;
uint16 public constant BURN_ROYALTY = 100;

---

2. Make Bonding Curve Parameters Updatable

Add an owner (contract deployer) who can update stepRanges and stepPrices for future storyline creations. Existing storylines are unaffected (they already have their token on MCV2).

• Add address public owner (set in constructor to msg.sender)
• Add function updateCurve(uint128[] calldata newRanges, uint128[] calldata newPrices) external onlyOwner
• Validate arrays match in length and are non-empty (same as constructor)
• Emit a CurveUpdated event

---

3. Remove sunset Variable — Derive from lastPlotTime

Logic bug: sunset is never set to true anywhere in the contract. It's also redundant — sunset state can be computed from lastPlotTime + 168 hours < block.timestamp && hasDeadline.

• Remove bool sunset from the Storyline struct
• Remove require(!s.sunset, "Storyline sunset") from chainPlot()
• The existing deadline check already handles this: require(block.timestamp <= s.lastPlotTime + 168 hours, "Deadline passed")
• Add a view function for frontend convenience:

  function hasSunset(uint256 storylineId) external view returns (bool) {
      Storyline storage s = storylines[storylineId];
      return s.hasDeadline && block.timestamp > s.lastPlotTime + 168 hours;
  }

---

4. Struct Storage Optimization (5 slots → 2 slots, ~60% gas save)

Current struct uses 5 storage slots. Packing it into 2 slots:

struct Storyline {
    address writer;       // slot 1: 160 bits
    address token;        // slot 2: 160 bits
    uint24 plotCount;     // slot 2: +24 = 184 bits (max 16.7M plots)
    uint40 lastPlotTime;  // slot 2: +40 = 224 bits (max year 36,812)
    bool hasDeadline;     // slot 2: +8 = 232 bits
}

Update all references:

• plotCount type changes: uint256 → uint24
• lastPlotTime type changes: uint256 → uint40
• Cast block.timestamp to uint40 when assigning
• plotIndex in events can remain uint256 (events don't cost storage)

---

5. Add Title Validation to chainPlot()

createStoryline() validates bytes(title).length > 0 but chainPlot() doesn't. Add the same check:

require(bytes(title).length > 0, "Empty title");

---

6. Add Zero-Value Validation for Content Hashes

contentHash and openingHash are required fields but never validated against bytes32(0). Add:

// In createStoryline()
require(openingHash != bytes32(0), "Empty hash");

// In chainPlot()
require(contentHash != bytes32(0), "Empty hash");

---

7. CEI Pattern in createStoryline()

Currently external calls (BOND.createToken, BOND.updateBondCreator) happen before state storage. Reorder to follow Checks-Effects-Interactions:

function createStoryline(...) external payable returns (uint256 storylineId) {
    // CHECKS
    require(bytes(title).length > 0, "Empty title");
    require(bytes(openingCID).length >= 46 && bytes(openingCID).length <= 100, "Invalid CID");
    require(openingHash != bytes32(0), "Empty hash");

    // EFFECTS (state changes first)
    storylineId = ++storylineCount;
    storylines[storylineId] = Storyline({
        writer: msg.sender,
        token: address(0),  // placeholder, updated after interaction
        plotCount: 1,
        lastPlotTime: uint40(block.timestamp),
        hasDeadline: hasDeadline
    });

    // INTERACTIONS (external calls last)
    address tokenAddress = BOND.createToken{value: msg.value}(tp, bp);
    BOND.updateBondCreator(tokenAddress, msg.sender);

    // Update token address after interaction
    storylines[storylineId].token = tokenAddress;

    emit StorylineCreated(...);
    emit PlotChained(...);
}

---

8. Add Zero-Address Validation in Constructor (audit finding M-3)

No address(0) check on _bond or _plotToken in the constructor. Since the contract is immutable (no owner, no upgrade), deploying with zero addresses permanently bricks it.

require(_bond != address(0), "Zero bond address");
require(_plotToken != address(0), "Zero token address");

---

9. Cap stepRanges / stepPrices Array Length (audit finding M-4)

Arrays are stored without a length cap. Extremely large arrays could make createStoryline() hit block gas limits due to memory copy of the full arrays into BondParams.

require(_stepRanges.length <= 1000, "Too many steps");

---

10. Verify updateBondCreator Succeeded (audit finding M-5)

If BOND.updateBondCreator() silently fails, the writer never receives the royalty recipient role. Royalties would accrue to the factory contract with no way to recover them.

Options:

• Option A: Read back creator from Bond after the call and assert it matches msg.sender
• Option B: Document this as a trust assumption on MCV2_Bond (acceptable if Bond is a known, audited contract — which it is)

Recommended: Option B — MCV2_Bond is Mint Club's audited contract. Add a NatSpec comment documenting the trust assumption.

---

Impact

• Existing mainnet contract: Unaffected (immutable)
• Requires: Redeploy StoryFactory, update addresses in both repos
• E2E tests: Must be updated after changes (royalty amounts, struct fields, new validations)
• Web app: Update ABI if event signatures change, update hasSunset() calls

Acceptance Criteria

• All 10 changes implemented
• Existing unit tests updated to match new behavior
• New tests for: updateCurve(), hasSunset(), empty hash reverts, empty chainPlot title reverts, zero-address constructor reverts, array length cap
• forge test passes
• Gas comparison logged (before/after struct optimization)

Branch

task/{issue-number}-contract-review-fixes

[realproject7]

All 10 items implemented via sub-tickets #41 (PR #46), #42 (PR #47), #43 (PR #48), #44 (PR #49), #45 (PR #50). Contract redeployed as v3 at 0x337c5b96f03fB335b433291695A4171fd5dED8B0. E2E verified.