Skip to content

SIG-747: remove historical and version reference comments#73

Merged
worjs merged 2 commits into
mainfrom
refactor/SIG-747-remove-version-comments
May 9, 2026
Merged

SIG-747: remove historical and version reference comments#73
worjs merged 2 commits into
mainfrom
refactor/SIG-747-remove-version-comments

Conversation

@worjs
Copy link
Copy Markdown
Contributor

@worjs worjs commented May 9, 2026

Context

SIG-747 (Epic: SIG-742, audit-prep). Contract comments carried historical metadata that is meaningless to an audit reader looking at v1 in isolation: Unlike v0's _forcePushPendingFactor, from older deployments, Per WP v2:, (WP v2 state machine), (WP v2 Eq. 3.11). v0 lives in a separate repository and is not in scope for the v1 audit, so v0 references invite questions about code that is not present. Whitepaper version prefixes and equation numbers add a versioning dimension that drifts independently of the contract.

Decisions

  • Comment surgery only. No signatures, function bodies, storage layout, control flow, or imports were touched.
  • For LazyMulSegmentTree, safety/invariant wording was kept where it explains current v1 behavior (e.g., why leaf pending is dead data, why l == r skips flush). Only the v0 comparison clauses were removed.
  • For MarketLifecycleModule equation comments, the formulas (Payout_t := Q_{t,τ_t}, L_t := ΔC_t - Payout_t) were preserved as audit anchors. Only the WP v2 Eq. X.YY numbering was dropped.
  • verification/foundry/src/LazyMulSegmentTreeNoLink.sol mirrors the contracts/lib/LazyMulSegmentTree.sol edits one-for-one because it is a no-link copy of the same tree.
  • ClmsrMathHarness.exposedSafeExp notice was changed from "parity tests against v0" to "unit, fuzz, and parity tests" rather than a generic "tests" label, reflecting actual call sites.
  • forge fmt --check was intentionally left untouched. Repo-wide formatter drift exists across files outside this scope and rewriting them would violate the comment-only boundary.

Risk Areas

  • LazyMulSegmentTree push-down comments (_pushPendingFactor, _applyFactorToChildWithFlush): the comment edits are adjacent to leaf-pending overflow safety logic. Reviewer should confirm the remaining wording still describes current v1 behavior accurately.
  • MarketLifecycleModule equation comments at _computeReserveDelta: payout/loss formulas are still load-bearing audit annotations. Reviewer should confirm formula text matches the code.
  • verification/foundry/src/LazyMulSegmentTreeNoLink.sol parity with contracts/lib/LazyMulSegmentTree.sol: three comment edits applied symmetrically. Reviewer should diff the two files to confirm no drift.

Findings

  • forge fmt --check does not pass at repo scope or in verification/foundry. The drift predates this branch and was left in place.

worjs added 2 commits May 9, 2026 21:53
The prod fork tests used `vm.createSelectFork(envName)` without a block
number, causing baseline runs to drift as live prod state evolves. With
the same code that passed at SIG-626 merge time (4/14 09:31 UTC), the
`AdminLifecycleFork.test_fee_waterfall_processing_*` assertion changed
result on the latest block: realized batch processing now produces a
zero backstop NAV delta, so the strict `assertGt` fails.

Pin prod fork to block 6029250 (≈ 2026-04-14 09:31 UTC, last green
Fork Tests Baseline on main) so the suite is reproducible. Allow
`FORK_BLOCK` env override so the pin can be advanced deliberately.
Dev fork keeps using latest because the dev chain has no equivalent
baseline mapping.

Verified locally: full fork suite 22/22 pass on the pinned block;
unit suite 1081/1081 still green.
Copy link
Copy Markdown

@signals-reviewer signals-reviewer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Comment-only refactoring that removes v0 comparison references and WP v2 version prefixes from contract comments, preparing the codebase for v1 audit readability. A second commit pins prod fork tests to a deterministic block (6029250) to fix baseline drift.

Issues

No issues found.

Suggestions

No suggestions.

Verdict

APPROVE: Comment surgery is clean and symmetric. The three LazyMulSegmentTree edits are mirrored correctly in the verification copy. Remaining comments still accurately describe v1 behavior (leaf-pending skip, l==r flush safety, NAV floor invariant). Equation formulas preserved as audit anchors with only version numbering removed. ForkBaseTest block pinning is sound with env override escape hatch. All tests pass per the implementation report (1081 unit, 22 fork, 3 verification). forge fmt pre-existing drift is correctly out of scope.

Copy link
Copy Markdown

@signals-reviewer signals-reviewer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Comment-only refactoring that removes v0 comparison references and WP v2 version prefixes from contract comments, preparing the codebase for v1 audit readability. A second commit pins prod fork tests to a deterministic block (6029250) to fix baseline drift.

Issues

No issues found.

Suggestions

No suggestions.

Verdict

APPROVE: Comment surgery is clean and symmetric. The three LazyMulSegmentTree edits are mirrored correctly in the verification copy. Remaining comments still accurately describe v1 behavior (leaf-pending skip, l==r flush safety, NAV floor invariant). Equation formulas preserved as audit anchors with only version numbering removed. ForkBaseTest block pinning is sound with env override escape hatch. All tests pass per the implementation report (1081 unit, 22 fork, 3 verification). forge fmt pre-existing drift is correctly out of scope.

Copy link
Copy Markdown

@signals-reviewer signals-reviewer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Comment-only refactoring that removes v0 comparison references and WP v2 version prefixes from contract comments, preparing the codebase for v1 audit readability. A second commit pins prod fork tests to a deterministic block (6029250) to fix baseline drift.

Issues

No issues found.

Suggestions

No suggestions.

Verdict

APPROVE: Comment surgery is clean and symmetric. The three LazyMulSegmentTree edits are mirrored correctly in the verification copy. Remaining comments still accurately describe v1 behavior (leaf-pending skip, l==r flush safety, NAV floor invariant). Equation formulas preserved as audit anchors with only version numbering removed. ForkBaseTest block pinning is sound with env override escape hatch. All tests pass per the implementation report (1081 unit, 22 fork, 3 verification). forge fmt pre-existing drift is correctly out of scope.

@worjs worjs merged commit 419f3e0 into main May 9, 2026
16 checks passed
@worjs worjs deleted the refactor/SIG-747-remove-version-comments branch May 9, 2026 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant