Bug: 5 RabbitTreasury tests failing on 254999d #15

Closed
opened 2026-04-04 01:33:40 +00:00 by Brouie · 7 comments
Brouie commented 2026-04-04 01:33:40 +00:00 (Migrated from gitlab.com)

forge test on 254999d shows 5 failing RabbitTreasury tests (146 passed, 5 failed). TimeCurve 58/58 green.

failing tests:

  1. test_deposit_withdraw_fuzz -- RT: zero payout
  2. test_protocolOwned_notExtracted_viaOrdinaryWithdraw -- ERC20InsufficientBalance
  3. test_repricingRaisesLiability_redemptionBelowNominal -- e should rise when coverage is strong
  4. test_stress_manyUsersExit_protocolBucketUntouched -- ERC20InsufficientBalance
  5. test_withdraw_basic -- assertion failed: 49499999999999999951 != 49500000000000000000

looks like the RabbitTreasury refactor (254 lines changed) introduced rounding or balance accounting issues. the withdraw_basic failure (off by 49 wei) suggests a precision/rounding change.

forge test on 254999d shows 5 failing RabbitTreasury tests (146 passed, 5 failed). TimeCurve 58/58 green. failing tests: 1. test_deposit_withdraw_fuzz -- RT: zero payout 2. test_protocolOwned_notExtracted_viaOrdinaryWithdraw -- ERC20InsufficientBalance 3. test_repricingRaisesLiability_redemptionBelowNominal -- e should rise when coverage is strong 4. test_stress_manyUsersExit_protocolBucketUntouched -- ERC20InsufficientBalance 5. test_withdraw_basic -- assertion failed: 49499999999999999951 != 49500000000000000000 looks like the RabbitTreasury refactor (254 lines changed) introduced rounding or balance accounting issues. the withdraw_basic failure (off by 49 wei) suggests a precision/rounding change.
PlasticDigits commented 2026-04-06 12:27:46 +00:00 (Migrated from gitlab.com)

Pushed fix in 91da459 on main.

Cause: Withdrawals use _previewWithdraw (pro-rata vs nominal cap, redemption-efficiency scaling, then mulDiv fee rounding). Several tests still assumed nominal 1:1 DOUB→reserve and gross * fee / WAD, which disagrees with on-chain math (e.g. ~49 wei on test_withdraw_basic) and could hit RT: zero payout or bad balances under default minRedemptionEfficiencyWad. The repricing test also needed stronger coverage (fee inflow) so e actually steps up.

Fix: Added previewWithdrawFor(user, amount) so previews match withdraw for a specific address (and documented that previewWithdraw uses msg.sender for cooldown). Updated tests to use that preview, tightened fuzz assumptions, and centralized setup via _setFullRedemptionEfficiency / _setDeterministicWithdrawNoFee. Added test_previewWithdrawFor_agrees_with_withdraw_fuzz to lock preview vs execution.

@brouie please run forge test in contracts/ and confirm this closes the failures you saw on 254999d; leaving the issue open until you verify.

Pushed fix in 91da459 on `main`. **Cause:** Withdrawals use `_previewWithdraw` (pro-rata vs nominal cap, redemption-efficiency scaling, then `mulDiv` fee rounding). Several tests still assumed nominal 1:1 DOUB→reserve and `gross * fee / WAD`, which disagrees with on-chain math (e.g. ~49 wei on `test_withdraw_basic`) and could hit `RT: zero payout` or bad balances under default `minRedemptionEfficiencyWad`. The repricing test also needed stronger coverage (fee inflow) so `e` actually steps up. **Fix:** Added `previewWithdrawFor(user, amount)` so previews match `withdraw` for a specific address (and documented that `previewWithdraw` uses `msg.sender` for cooldown). Updated tests to use that preview, tightened fuzz assumptions, and centralized setup via `_setFullRedemptionEfficiency` / `_setDeterministicWithdrawNoFee`. Added `test_previewWithdrawFor_agrees_with_withdraw_fuzz` to lock preview vs execution. @brouie please run `forge test` in `contracts/` and confirm this closes the failures you saw on 254999d; leaving the issue open until you verify.
PlasticDigits commented 2026-04-06 12:27:54 +00:00 (Migrated from gitlab.com)

mentioned in commit 43a8e0564d

mentioned in commit 43a8e0564d49f7aa7a4515400f56ef56bfc261b4
PlasticDigits commented 2026-04-06 12:28:02 +00:00 (Migrated from gitlab.com)

Update: after rebase onto latest main, the fix landed as commit 43a8e05 (same message as noted above).

Update: after rebase onto latest `main`, the fix landed as commit **43a8e05** (same message as noted above).
Brouie commented 2026-04-07 03:44:56 +00:00 (Migrated from gitlab.com)

Verified on 5d01bb4. All 5 RabbitTreasury failures now pass. 152/152 contracts, 0 failed (was 146 pass, 5 fail). Invariant fuzz tests also passing (256 runs each). Closing.

Verified on 5d01bb4. All 5 RabbitTreasury failures now pass. 152/152 contracts, 0 failed (was 146 pass, 5 fail). Invariant fuzz tests also passing (256 runs each). Closing.
Brouie (Migrated from gitlab.com) closed this issue 2026-04-07 03:45:22 +00:00
PlasticDigits commented 2026-06-25 20:07:36 +00:00 (Migrated from gitlab.com)

mentioned in issue #355

mentioned in issue #355
PlasticDigits commented 2026-06-25 20:07:51 +00:00 (Migrated from gitlab.com)

mentioned in issue #342

mentioned in issue #342
PlasticDigits commented 2026-06-28 02:44:49 +00:00 (Migrated from gitlab.com)

mentioned in merge request !528

mentioned in merge request !528
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
code/yieldomega#15
No description provided.