Skip to content

test: refactor unit tests to focus on the main ARM#233

Open
clement-ux wants to merge 28 commits into
nicka/withdraw-on-swapfrom
clement/add-tests
Open

test: refactor unit tests to focus on the main ARM#233
clement-ux wants to merge 28 commits into
nicka/withdraw-on-swapfrom
clement/add-tests

Conversation

@clement-ux
Copy link
Copy Markdown
Contributor

@clement-ux clement-ux commented May 20, 2026

Summary

  • Refactored the unit-test suite to give us better coverage and a cleaner structure.
  • Focused the work on the main ARM (LidoARM) for unit testing, dropping the older OriginARM / SiloMarket / shared unit suites which weren't pulling their weight.
  • New layout under `test/unit/LidoARM/`: `Base`, `Shared`, `concrete/` (admin, accounting, swap, deposit, redeem, allocate, pause, market mgmt, adapters), `fuzz/` (swap, deposit, redeem, adapter splits), `mocks/` (LidoARM-local mocks).
  • `test/unit/mocks/` is preserved because invariants and fork OriginARM tests still import `MockVault` / `MockERC4626Market` from there.

Test plan

  • `forge build` — succeeds.
  • `make test-unit` — 234/234 passing across 19 LidoARM suites (concrete + fuzz + adapters).

clement-ux and others added 28 commits May 16, 2026 15:17
- Add Swap.fuzz.t.sol with 8 fuzz tests covering both
  swapExactTokensForTokens and swapTokensForExactTokens, fuzzing
  amounts and prices on both buy and sell sides
- Add aliceFirstDeposit(uint256) overload in Shared.t.sol
- Group imports and add section separators in concrete swap tests
Replaces the bare `lidoWithdrawalQueue` placeholder address with a working
mock so unit tests can exercise the adapter's external redeem path. Exposes
`mock_setFinalized` / `mock_setClaimed` / `mock_setOwner` knobs to drive the
adapter's edge-case branches.
…tests

Covers the ARM-side flow only (accounting, access control, funds movement
between ARM/adapter/withdrawal queue); the adapter's internal queue logic
has its own suite. Also fixes Shared.t.sol to run the adapter's
`initialize()` through the proxy so the lido queue gets its stETH allowance.
Covers `addMarkets`, `removeMarket`, `setActiveMarket`, `setARMBuffer`, and
`allocate()`. Includes the maxRedeem-below-threshold branch in `_allocate`
that the legacy suite kept commented out — seeded via a dummy depositor that
mints the ARM exactly `minSharesToRedeem` market shares. Adds a second mock
ERC4626 market and three governor-pranking helpers to Shared.t.sol.
Covers totalAssets, convertToAssets/convertToShares, claimable, getReserves
and the previewDeposit/previewRedeem wrappers across the four scenario axes
(market, fees, LP queue, yield/loss). Includes the MIN_TOTAL_SUPPLY clamp
path and the market-liquidity-constrained claimable case (via vm.mockCall
on maxWithdraw).
Direct unit coverage for the abstract adapter exercised through both
concrete implementations. StETHAssetAdapter.t.sol covers initialize,
modifiers, _splitAmounts chunking, redeem branch matrix (including
unfinalized/claimed/owner-changed mock paths), claimableRedeem, and
getters. WstETHAssetAdapter.t.sol covers the wrap/unwrap path and the
non-1:1 _splitShares arithmetic where the last chunk absorbs the
rounding remainder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pter

The two safety branches inside _splitShares — capping splitShares to
remainingShares and the proportional fallback when splitShares is zero —
are unreachable through StETHAssetAdapter (1:1) and WstETHAssetAdapter
(rate >= 1) under self-consistent unwrap math. Drive them with a
test-only adapter that injects _pullSharesAndConvertToSteth and
_assetsToShares values, bringing AbstractLidoAssetAdapter to 100%
line and branch coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Property-based fuzz tests for the two chunking helpers in
AbstractLidoAssetAdapter. An inline ExposedLidoAdapter exposes both as
external functions and drives _assetsToShares via a settable mock rate
so a single contract covers the full rate spectrum — happy path,
remainingShares cap, and zero-fallback branches.

Asserted invariants:
- _splitAmounts: chunk count = ceil(amount / MAX), sum == amount,
  non-final chunks == MAX, final chunk in (0, MAX].
- _splitShares: sum(splits) == totalShares, no non-final split exceeds
  remaining shares at its iteration, and under self-consistent rates
  non-final splits equal _assetsToShares(MAX).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… case

Two depth-targeted additions to the adapter unit suite:

- test_StateMachine_MultiRequestMixedFinalization: walks a single
  adapter through 2 requests / 3 queue ids with a (finalized,
  un-finalized, finalized) shape — exercises the strict-FIFO claim
  rule (id 2 unreachable until id 1 catches up), incremental
  nextPendingIndex advancement across three sequential redeems, and
  the empty-queue revert after full drain. Catches integration-level
  regressions that isolated branch tests miss.

- test_RequestRedeem_ZeroAssetsOut_IsSilentNoOp: pins the documented
  edge case where _pullSharesAndConvertToSteth returns 0 — the
  adapter still returns sharesRequested = shares (non-zero) but
  queues nothing. Future fix should turn this test into an
  expectRevert.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the three remaining `a * b / c` patterns in the Split fuzz file
with explicit Math.mulDiv(.., Math.Rounding.Floor). Removes any
intermediate-overflow risk if bounds widen later and aligns with the
rounding direction used by the contract's _assetsToShares conversion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single contract covering every owner/operator setter on LidoARM with
both happy paths and the full revert matrix:

- addBaseAsset: 12 tests — happy path with event/storage/approval, plus
  reverts for non-owner, zero asset/adapter, already-supported,
  non-18-decimal asset, adapter asset mismatch, cross price out of
  range, and the three _validatePrices guards.
- setPrices: 7 tests — owner + operator happy paths, all reverts.
- setCrossPrice: 9 tests — raise/lower, unsupported asset, range
  guards, sell/buy price collision, and TooManyBaseAssets when lowering
  with non-trivial base-asset exposure.
- setFee: 4 tests — including the side-effect flush of accrued fees
  through collectFees (feesAccrued seeded via vm.store on slot 62).
- setFeeCollector: 3 tests.
- collectFees: 3 tests — zero-accrued early return, non-zero transfer,
  insufficient liquidity revert.
- setCapManager: 3 tests including zero-address disable.
- setARMBuffer: 5 tests — owner + operator + boundary at 1e18 + reverts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the storage poking that seeded feesAccrued in the setFee and
collectFees tests with a real stETH->WETH swap, so the tests exercise
the same accrual path production callers hit. The InsufficientLiquidity
revert is now driven by reserving most of the ARM's WETH via an LP
requestRedeem, leaving on-hand balance below fees + reservedWithdrawLiquidity.
Covers access control (operator can pause but only owner can unpause), the
`paused` state flip, and the Paused/Unpaused events. Downstream
whenNotPaused reverts are already covered in the per-function suites.
Unused since the file does not reference the IERC20 type directly.
…plit

Drop legacy OriginARM/SiloMarket/shared unit suites and replace with a
LidoARM-focused tree under test/unit/LidoARM/ (Base, Shared, concrete/,
fuzz/, mocks/). Keeps test/unit/mocks/ since invariants and fork
OriginARM tests still import MockVault and MockERC4626Market from there.
@clement-ux clement-ux changed the base branch from main to nicka/withdraw-on-swap May 20, 2026 14:26
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