Skip to content

Sync dev → main (wholesale): YS invariant fuzz guard - #773

Merged
alcueca merged 2 commits into
mainfrom
dev
Aug 4, 2026
Merged

Sync dev → main (wholesale): YS invariant fuzz guard#773
alcueca merged 2 commits into
mainfrom
dev

Conversation

@alcueca

@alcueca alcueca commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What

Wholesale sync of dev into main. main was last synced from dev on 2026-07-31 (PR #765), so the only delta since is:

Content diff is exactly those 4 test-harness files (+52 / −150); no source changes, no conflicts.

Why

The nightly-fuzz workflow runs a main + dev matrix on the same commit. dev is now green on both nightlies; main is still red on the nightly-fuzz YS invariant (InvariantTestYS Rule 14/15) because it lacks #771. Merging brings the fix to main so the main leg clears.

Nightly status (2026-08-03 runs)

Workflow dev leg main leg
nightly-fuzz ✅ success ❌ failure (missing #771)
nightly-certora ✅ A–E ✅ A–E

Scope of #771 (recap)

Test-only. The withdraw fuzz handlers now refuse to drain the YS vault to zero while a vest is active (an unreachable-in-production state), replacing a flaky best-effort repair. The underlying source Finding 1 remains intentionally WONTFIX. Verified at the failing nightly seed: FOUNDRY_PROFILE=nightly forge test --match-path test/fuzzing/InvariantTestYS.sol --fuzz-seed 0x2 → 51 passed / 0 failed.

After merge, the next nightly-fuzz main leg is expected green.

🤖 Generated with Claude Code

alcueca and others added 2 commits August 1, 2026 06:29
…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>
…guard

test(fuzzing): guard YS invariant against unreachable empty-while-vesting state
@alcueca
alcueca requested a review from a team as a code owner August 4, 2026 04:46
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

The PR replaces a best-effort post-deposit repair with a preventive YS withdrawal guard intended to keep the vault nonempty during active vesting.

  • Removes fixVestingStateAfterFirstDeposit from both Accountant fuzz handlers.
  • Applies a shared withdrawal clamp to Foundry and Medusa YS withdrawal paths.
  • Updates the fuzzing documentation to describe the modeled production precondition and acknowledged source issue.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking fuzz-coverage gap for final withdrawals after vesting has fully accrued.

The production contracts are unchanged and the new guard prevents the targeted invalid state, but checking stored gains before exchange-rate realization can cause both fuzz withdrawal handlers to skip the only operation capable of draining a one-share vault after vesting ends.

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

Important Files Changed

Filename Overview
test/fuzzing/handlers/TellerHandler.sol Adds the shared YS withdrawal clamp, but its raw vestingGains check can prevent the last post-vest share from reaching the Teller's realization path.
test/fuzzing/handlers/AccountantHandler.sol Cleanly removes the unreliable Foundry-side post-deposit vesting repair.
test/fuzzing/medusa/handlers/AccountantHandler.sol Removes the equivalent Medusa repair while relying on the shared guarded Teller handler.
test/fuzzing/README.md Documents the new prevention model, although the stated post-accrual draining behavior is not guaranteed when only one share remains.

Reviews (1): Last reviewed commit: "Merge pull request #771 from Veda-Labs/t..." | Re-trigger Greptile

Comment on lines +923 to +927
(, uint128 vestingGains, , , ) = accountantHandler.accountantYS().vestingState();
if (vestingGains == 0) return shareAmount;
uint256 supply = vaultYS.totalSupply();
if (shareAmount < supply) return shareAmount; // other holders keep the vault non-empty
return supply > 1 ? supply - 1 : 0; // leave 1 wei of shares as dust, or skip entirely

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 Post-vest withdrawals remain clamped

After the vesting end time, vestingGains remains nonzero until an exchange-rate update, but this guard runs before the Teller performs that update. A vault with one remaining share therefore returns early without realizing the completed vest, preventing both fuzz handlers from covering the documented post-vest full-drain path.

Suggested change
(, uint128 vestingGains, , , ) = accountantHandler.accountantYS().vestingState();
if (vestingGains == 0) return shareAmount;
uint256 supply = vaultYS.totalSupply();
if (shareAmount < supply) return shareAmount; // other holders keep the vault non-empty
return supply > 1 ? supply - 1 : 0; // leave 1 wei of shares as dust, or skip entirely
(, uint128 vestingGains, , , uint64 endVestingTime) = accountantHandler.accountantYS().vestingState();
if (vestingGains == 0 || block.timestamp >= endVestingTime) return shareAmount;
uint256 supply = vaultYS.totalSupply();
if (shareAmount < supply) return shareAmount; // other holders keep the vault non-empty
return supply > 1 ? supply - 1 : 0; // leave 1 wei of shares as dust, or skip entirely

Knowledge Base Used: CI, Testing, and Fuzzing

@alcueca
alcueca requested review from bxmmm1 and imrtlfarm August 4, 2026 05:11
@alcueca
alcueca merged commit 39f9d31 into main Aug 4, 2026
1 check passed
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