[#45] Update E2E scripts for post-#38 contract changes

[realproject7]

Summary

• Update factory address to new deployment (0x27B4FCf333f29a3865b3B76ea00C955D7b64BD0F)
• Add 5 new test scenarios to revert script:
  • E10: empty title in chainPlot
  • E11: zero hash in createStoryline
  • E12: zero hash in chainPlot
  • E13: non-owner updateCurve
  • G1: hasSunset view on active storyline
• Revert script now covers 16 scenarios (was 11)

Test plan

• All 45 unit tests pass
• forge fmt --check clean
• E2E broadcast runs against new deployment
• E2E revert script passes all 16 scenarios

Fixes #45

🤖 Generated with Claude Code

[project7-interns]

T2b Review — APPROVED

• Factory address updated to new deployment in both scripts
• E10-E12: correctly test the new validations from #42 (empty title, zero hash) with proper writer context via vm.prank(deployer) where needed
• E13: non-owner updateCurve reverts on onlyOwner modifier before reaching validation — correct
• G1: hasSunset view tested against live mainnet state — correct since idA1 comes from e2e-results.json produced by the new deployment

All new scenarios follow the existing try/catch + reason-check pattern. LGTM.

[project7-interns]

Verdict: REQUEST CHANGES

Summary

The PR updates the deployed factory address and adds some new revert coverage, but it does not yet satisfy the post-#38 E2E scope from issue #45. Several required new contract behaviors are still unexercised in the scripts.

Findings

• [high] The new updateCurve() functionality is only covered for the non-owner revert path; the E2E scripts still do not test the owner success path or that a new storyline created after updateCurve() uses the updated curve.

  • File: script/E2ETestReverts.s.sol:178
  • Suggestion: add a happy-path owner updateCurve() scenario in the broadcastable script and then create a new storyline afterward to verify the updated curve is actually used, as required by issue #45.

• [high] hasSunset() coverage is incomplete. The PR only checks the false case on an active storyline, but issue #45 explicitly calls for active vs expired coverage.

  • File: script/E2ETestReverts.s.sol:198
  • Suggestion: add a second hasSunset() scenario for an expired deadline-enabled storyline and assert it returns true.

• [medium] The newly-added constructor validations from issue #42 are still not represented in the post-update E2E scripts.

  • File: script/E2ETestReverts.s.sol:149
  • Suggestion: add scenarios covering the zero-address constructor guards and the >1000 step-array cap, or document and implement an equivalent runtime path if constructor-level validation is being exercised another way. Right now the PR adds only title/hash/updateCurve-owner coverage.

Decision

Requesting changes because the PR only partially updates the E2E scripts for the post-#38 contract behavior and leaves multiple required scenarios uncovered.

[project7-interns]

Verdict: REQUEST CHANGES

Summary

The PR covers more of the post-#38 surface area now, but two acceptance-criteria items are still not actually exercised end-to-end. One of the new scenarios is unreachable on the current data, and the new curve scenario still does not verify that the updated curve was used.

Findings

• [high] The new hasSunset() expired-path check still never exercises an expired storyline, because it uses storylineA2, which the broadcast script creates with hasDeadline = false.

  • File: script/E2ETestReverts.s.sol:202
  • Suggestion: use a storyline that was actually created with a deadline enabled (for example storylineA1 after warping far enough) and assert hasSunset() becomes true. As written, G2 only re-tests the false case.

• [medium] F6 demonstrates that the owner can call updateCurve() and then create another storyline, but it still does not verify that the newly created storyline actually used the updated curve parameters.

  • File: script/E2ETest.s.sol:437
  • Suggestion: add an assertion tied to the new curve, such as reading back a downstream observable that depends on the updated step ranges/prices or otherwise proving the post-update create path consumed the new curve, since issue #45 explicitly calls for this scenario.

Decision

Requesting changes because the post-update E2E coverage is still missing a real expired-hasSunset() assertion and a proof that updateCurve() affects subsequent storyline creation.

[project7-interns]

Verdict: APPROVE

Summary

The follow-up completes the missing post-#38 E2E coverage: hasSunset() now exercises the expired path on a deadline-enabled storyline, and the updateCurve() happy path now proves the updated curve is actually applied by asserting the higher mint cost on the newly-created storyline. The additional constructor-validation scenarios are also present, and GitHub checks are passing.

Findings

• None.

Decision

Approving because the review blockers are resolved and the updated E2E scripts now cover the required post-update contract behavior.