Skip to content

test(fuzzing): guard YS invariant against unreachable empty-while-vesting state - #771

Merged
imrtlfarm merged 1 commit into
devfrom
test/ys-invariant-empty-vault-guard
Aug 3, 2026
Merged

test(fuzzing): guard YS invariant against unreachable empty-while-vesting state#771
imrtlfarm merged 1 commit into
devfrom
test/ys-invariant-empty-vault-guard

Conversation

@alcueca

@alcueca alcueca commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

What

Fixes the recurring nightly-fuzz failure in test/fuzzing/InvariantTestYS.sol — Rule 14/15 (startVestingTime <= endVestingTime), which began failing once main advanced to include the yield-streaming system.

Root cause (not a regression)

The acknowledged Finding 1 (setFirstDepositTimestamp bug): setFirstDepositTimestamp() sets startVestingTime without clearing a stale endVestingTime, combined with _updateExchangeRate() not clearing vestingGains at totalSupply == 0. The bad state (start > end with vestingGains > 0) is reachable only when a vault empties to totalSupply == 0 while a vest is still active and is then re-entered.

That state is unreachable in production (deprecated vaults never fully drain, let alone mid-vest), so the source is intentionally WONTFIX. The nightly went red because the prior handler workaround let the bad state happen and then tried to repair it with a best-effort vestYield — which reverts under the contract's own guards, silently leaving the invalid state for the invariant to catch (a flaky failure that surfaced once the seedless nightly campaign found the sequence).

Change (test-only: prevention over repair)

  • Withdraw handlers (withdrawYS, bulkWithdrawYS) now refuse to drain the YS vault to zero while vestingGains > 0 — leaving dust or skipping. vestYield already refuses to vest into an empty vault, so this makes totalSupply == 0 always imply vestingGains == 0, and Rule 14/15 can no longer be violated without weakening what they check for real regressions.
  • Removed the dead best-effort repair (fixVestingStateAfterFirstDeposit) from both the Foundry and Medusa handlers.
  • Rewrote README Finding 1 to document the rationale (prevention, WONTFIX in source).

Covers both the Foundry and Medusa YS suites (shared TellerHandler). Net −150 / +52 lines.

Verification

FOUNDRY_PROFILE=nightly forge test --match-path test/fuzzing/InvariantTestYS.sol --fuzz-seed 0x2
→ 51 passed; 0 failed   (previously 2 failed on the same seed and depth)
  invariant_startVestingTimeLEendVestingTime  PASS (256 runs, 128000 calls)
  invariant_vestingGainsIntegrity             PASS (256 runs, 128000 calls)

Same seed (0x2) and depth (256×500) as the failing nightly run #66.

🤖 Generated with Claude Code

…ting state

InvariantTestYS Rule 14/15 (startVestingTime <= endVestingTime) started failing
in nightly-fuzz once main advanced to include the YS system. Root cause is the
acknowledged Finding 1: setFirstDepositTimestamp() moves startVestingTime without
clearing a stale endVestingTime, reachable only when a vault hits totalSupply == 0
while a vest is still active and is then re-entered.

That state is unreachable in production (deprecated vaults never fully drain, let
alone mid-vest) and the source is intentionally WONTFIX. The prior handler
workaround let the bad state happen then repaired it with a best-effort vestYield
that reverts under the contract's own guards, so it silently left the invalid
state and the invariant caught it — a flaky failure.

Replace repair with prevention: the YS withdraw handlers now refuse to drain the
vault to zero while vestingGains > 0 (vestYield already refuses to vest into an
empty vault), so totalSupply == 0 always implies vestingGains == 0 and Rule 14/15
cannot be violated without weakening what they check. Remove the now-dead repair
function from both the Foundry and Medusa handlers and document the rationale in
the fuzzing README.

Verified: FOUNDRY_PROFILE=nightly forge test --match-path
test/fuzzing/InvariantTestYS.sol --fuzz-seed 0x2 -> 51 passed, 0 failed
(previously 2 failed on the same seed and depth).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alcueca
alcueca requested a review from a team as a code owner August 1, 2026 05:29
@alcueca
alcueca requested review from RadioJulius and bxmmm1 August 1, 2026 05:30
@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces a best-effort vesting-state repair in the YS fuzz handlers with a withdrawal clamp intended to prevent an acknowledged, production-unreachable empty-vault state.

  • Prevents Foundry and Medusa YS withdrawal handlers from draining the vault while stored vesting gains are nonzero.
  • Removes the obsolete repair function and its deposit call sites from both handler implementations.
  • Documents the excluded state and WONTFIX rationale in the fuzzing README.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking fuzz-coverage gap around full withdrawals whose requested time advance completes the active vest.

The new guard stabilizes the targeted invariant and is shared by both fuzzers, but its pre-warp state check can reduce or skip a withdrawal that would be valid when the actual call executes after vest completion.

Files Needing Attention: test/fuzzing/handlers/TellerHandler.sol

Important Files Changed

Filename Overview
test/fuzzing/handlers/TellerHandler.sol Replaces post-deposit repair with a shared YS withdrawal clamp, but evaluates the vest before the handler's time warp and therefore suppresses valid completed-vest withdrawals.
test/fuzzing/handlers/AccountantHandler.sol Removes the Foundry-only best-effort vesting repair without leaving a dangling caller.
test/fuzzing/medusa/handlers/AccountantHandler.sol Mirrors removal of the obsolete repair in the Medusa accountant handler.
test/fuzzing/README.md Documents the acknowledged source behavior and the new prevention-based fuzz-model constraint.

Sequence Diagram

sequenceDiagram
  participant F as Fuzzer
  participant H as TellerHandler
  participant A as YS Accountant
  participant T as YS Teller
  F->>H: withdrawYS(full shares, timeDelta)
  H->>A: read vestingState()
  A-->>H: "vestingGains > 0"
  H->>H: clamp shares or skip
  H->>H: warp by timeDelta
  Note over H,A: Vest may now be complete
  H->>A: sync vested assets
  H->>T: withdraw(clamped shares)
  T->>A: updateExchangeRate()
  A-->>T: realize completed vest
Loading

Reviews (1): Last reviewed commit: "test(fuzzing): guard YS invariant agains..." | Re-trigger Greptile


uint256 actorShares = vaultYS.balanceOf(actor);
shareAmount = bound(shareAmount, 1, actorShares);
shareAmount = _clampYSWithdrawForActiveVest(shareAmount);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Clamp uses pre-warp vest state

withdrawYS clamps the amount before applying timeDelta, so a vest that completes during the requested warp is still treated as active. The handler consequently leaves dust—or skips a one-share withdrawal entirely—instead of exercising the valid completed-vest full-withdrawal path.

Knowledge Base Used: CI, Testing, and Fuzzing

@alcueca
alcueca requested review from mel0ndev and removed request for bxmmm1 August 1, 2026 05:32
@imrtlfarm
imrtlfarm merged commit 1e44cbb into dev Aug 3, 2026
1 check passed
@imrtlfarm
imrtlfarm deleted the test/ys-invariant-empty-vault-guard branch August 3, 2026 16:49
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.

3 participants