feat(EXSC-411): S3 — fee receivers & sweep distribution#1983
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds configurable integrator receiver fan-out to LiFiVaultWrapper, extends initialization and factory deployment to accept receiver sets, adds permissionless sweep distribution with failure redirection, and updates the interface and tests to match the new receiver-based fee flow. ChangesIntegrator receivers and sweep distribution
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b0e15d6 to
f83fbb7
Compare
…rawal (S2) Implement the management (dilution), deposit, and withdrawal fees on top of the S1 no-op seams. Management fee accrues pro-rata on AUM and crystallizes as dilution shares minted to the wrapper on every deposit/withdraw; deposit and withdrawal fees are skimmed asset-side at transaction time and held idle so the share price is unaffected. Both sides accrue in-contract (totals only); payout and the LI.FI/integrator split are follow-up tickets. - LibVaultWrapperFees: pure fee math (OZ Convention B raw/total, management pro-rata, dilution-share formula) as the audit/fuzz surface. - Effective-supply override on _convertToShares/_convertToAssets so previews reflect pending management dilution: preview equals execution to the wei. - setFeeRate (vaultWrapperAdmin-gated) reads factory bounds live; rate 0 disables; accrues at the old rate before applying a change. Performance fee is rejected here (separate ticket EXSC-556). - Events: FeeConfigUpdated, DilutionFeeAccrued, AssetFeeCharged. - Storage appended (accruedFeeShares, accruedFeeAssets, lastMgmtAccrual); __gap reduced 49->46. Version bumped 1.0.0 -> 1.1.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tWrapper.sol:555) CodeRabbit (major): _accrueFees only advanced lastMgmtAccrual on a mint, so dormant periods (fee disabled, empty vault) and an uninitialized baseline (==0, e.g. an instance upgraded into this version) would later be charged at the current rate on current AUM. Reset the baseline to now in those cases and return; mirror the ==0 guard in _pendingManagementFee so preview==execution still holds. Sub-threshold dust handling (Q3) is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nce (LiFiVaultWrapperFees.t.sol:187) CodeRabbit (minor): the event-matching loop set a bool on first match, so a duplicate or stray emission would pass. Count matches and assert == 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rFees.t.sol:464) CodeRabbit (minor): the test ran with bounds 0..1000, so setFeeRate(...,0) would have passed even if bounds were enforced. Set bounds to 100..1000 so zero is out of range, making the skip-bounds path the only reason it succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rebuild S3 on top of S2's accrual engine (dev-vault-wrapper <- S1 <- S2 <- S3). S2 owns accrual into undivided totals (accruedFeeAssets, accruedFeeShares); S3 adds the receiver config + a permissionless sweep that applies the integratorShareBps split at payout time over both reservoirs. - Receiver config (1..5 wallets, bps summing to 100%) set at initialize via a typed IntegratorReceivers param and mutable by the vaultWrapperAdmin; never emptiable, so an unconfigured-receivers state is impossible. - sweep() accrues pending management fees first, then distributes the idle-asset reservoir and the dilution-share reservoir in their native denominations (no adapter touch). Each split by integratorShareBps; integrator portion fanned across wallets (last absorbs the remainder); LI.FI paid to the factory's live lifiFeeRecipient. - A failing integrator transfer (e.g. a blacklisted wallet) is caught via a self-only trustedTransfer and redirected to LI.FI, so a single hostile wallet can never block the sweep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… math lib, naming - Move all wrapper events + errors into ILiFiVaultWrapper (consistent with the factory interface); update test error references to the interface. - Rename LibVaultWrapperFees -> LibVaultWrapperMath and move the fee-adjusted share/asset conversions into it, so all wrapper arithmetic lives in one lib. - Drop feeAssets from DilutionFeeAccrued (shares is the natural unit; the event no longer requires recomputing the asset value). - NatSpec: remove ticket/stage references and the opaque "Convention B" label. - Keep @Custom:version at 1.0.0; set __gap to 50; use named args at lib call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(LibVaultWrapperMath.t.sol:477) CodeRabbit (minor): testFuzz_Convert_PendingSharesDilute hardcoded _decimalsOffset 0, so it never exercised non-zero offsets. Add a bounded fuzzed offset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeRabbit: a node_modules symlink with an absolute local path was accidentally staged via git add -A (it escaped the node_modules/ dir-pattern gitignore as a file). Untrack it; the on-disk symlink stays for local hooks.
…s.sol:48) CodeRabbit: 'bps summing to exactly 100%' reads ambiguously as [60,40]; clarify the denominator is 10_000.
…LiFiVaultWrapper.sol:193) CodeRabbit: the check rejects a 100% integrator share (>= 10000) so LI.FI always keeps a non-zero cut, but the doc said 'exceeds 100%'. Reword to match.
…rapperDistribution.t.sol:36,149) CodeRabbit: add a direct trustedTransfer external-caller revert (OnlySelf) test — the self-call guard is the main protection against arbitrary payout routing — and assert ReceiverBpsSumNot100 on the setIntegratorReceivers update path, not just at initialize.
9ef7a07 to
b9afb41
Compare
…into feature/exsc-410-s2-fee-engine-4-types-accrual-caps-per-fee-split # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol # test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol
…-fee-split' into feature/exsc-411-s3-rebased-on-s2 # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol # src/VaultWrapper/interfaces/ILiFiVaultWrapper.sol # test/solidity/VaultWrapper/LiFiVaultWrapperFees.t.sol
The feeShares==0 early return preserved lastMgmtAccrual so sub-threshold elapsed time would be charged later. But carried time is re-priced against whatever totalAssets is THEN: a dormant dust vault could charge a new depositor management fees for up to a year they were not invested (worst case near-confiscation via the totalAssets-1 clamp), and setFeeRate's accrue-at-old-rate-first guarantee was void whenever the old-rate accrual floored to zero. The baseline now always advances; sub-threshold time is dropped (holder-favouring, bounded by ~one share's worth per accrual). Found by workflow-backed review (finding 1, CONFIRMED). Replaces the test that asserted the carry behavior as intended with regression tests proving baseline advancement; all three fail against the old ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_transferOut delegatecalled the adapter with owed == 0, so on yield sources that reject zero-amount withdrawals a dust redeem whose previewRedeem is 0 (or a bare withdraw(0)) reverted — breaking the preview==execution invariant and trapping dust-share holders. Mirrors the existing zero short-circuit in _deposit. Also removes the stale S1 'fees unimplemented' sentence from _transferOut's NatSpec and fixes the dangling @notice on the fee-config-getters section divider. Regression test uses a strict ERC-4626 mock that rejects zero-amount withdrawals; verified it fails without the short-circuit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every setFeeRate test targeted FeeType.Management, so a mis-indexed rate write or a wrong feeBounds key would have passed the suite. Adds live-effect tests (rate change observed on the next deposit/withdraw, sibling slots unchanged) and a bounds test where the deposit minimum disagrees with the management bounds, so reading the wrong key fails. Removes testFuzz_WithdrawRedeemInverse, a byte-identical copy of testFuzz_DepositMintInverse — both sides compose feeOnRaw/feeOnTotal identically, so one fuzz covers the identity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion Each deposit/withdraw performed up to 4 adapter.totalAssets() calls (two external hops each): the accrual's pending computation, the conversion override, and the pending computation again inside the conversion. Now _pendingManagementFee is parameterized on (supply, assets) so the conversion overrides pass the values they already read, a cheap-guard no-arg entry skips the external read entirely when the type is disabled or no time has elapsed, and the elapsed==0 short-circuit makes in-transaction conversions (post- accrual) free of a second pending computation. Pure caller plumbing; the fee math is unchanged. Measured on the fee suite (test totals, MockERC4626 underlying): deposit-with-pending-mgmt flows drop ~13-26k gas (e.g. 951,492 -> 925,106; 821,065 -> 807,634); deeper adapters save more per hop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-fee-split' into feature/exsc-411-s3-fee-receivers-sweep-distribution
…into feature/exsc-410-s2-fee-engine-4-types-accrual-caps-per-fee-split # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol
…into feature/exsc-410-s2-fee-engine-4-types-accrual-caps-per-fee-split
…-fee-split' into feature/exsc-411-s3-rebased-on-s2 # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol
…exsc-410-s2-fee-engine-4-types-accrual-caps-per-fee-split # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol # src/VaultWrapper/interfaces/ILiFiVaultWrapperFactory.sol
…t accrual Per-fee-type LI.FI/integrator split: DeployParams/initialize take uint16[4] integratorShareBps (per-element sentinel, <100% validation, self-serve above-default guard per element). The split is applied the moment a fee accrues, into packed uint128 per-recipient counters (lifi/integrator x shares/assets); integrator part floors, dust to LI.FI. Performance fee: PPS high-water mark (1e18-scaled, packed with lastMgmtAccrual), anchored at initialize, charged in _accrueFees after management dilution on gains above the watermark, watermark ratchets to the post-crystallization PPS only when shares were minted. Previews include pending perf shares via _pendingFeeShares. setFeeRate now accepts Performance (accrue-at-old-rate first; enable from disabled re-anchors the watermark); FeeTypeNotConfigurable removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aultWrapperFactory.sol:244) CR: '_resolveSplits rejects share >= 10000, so the onboarding manager can set at most 9999 bps, not 100%.' Comment-only wording fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eflow NatSpec Address review feedback on PR #1993: - Replace hardcoded uint16[4] array sizes and fee-type loop bounds with a single FEE_TYPE_COUNT constant in LiFiVaultWrapperTypes. - Rename USE_DEFAULT_SPLIT to DEFAULT_SPLIT_SENTINEL to describe what it is. - Reflow an over-length withdrawal NatSpec line to the 100-char cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (LiFiVaultWrapper.sol:452) CR/reviewer readability: "overage" (a correct but uncommon accounting term) reads as a possible typo; "excess" is unambiguous. Comment-only, no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…411) A bare `Receiver` collides with ERC-4626's `receiver` param; the struct ties a wallet to a fee bps, so `FeeReceiver` reads clearer. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…EXSC-411) Replace the parallel `address[] wallets` / `uint16[] bps` boundary (the `IntegratorReceivers` struct) with a single `FeeReceiver[]` across initialize, setIntegratorReceivers, DeployParams, and the ReceiversSet event. This removes the ReceiversLengthMismatch error since the arrays can no longer diverge. Reorder initialize to persist calldata before resolving the asset, keeping its stack within the limit (no via_ir) now that the receiver param is a two-slot calldata array. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(EXSC-411) Rename the storage array and its mutators to integratorFeeReceivers / setIntegratorFeeReceivers / _setIntegratorFeeReceivers so "receiver" no longer collides with the ERC-4626 `receiver` param used elsewhere in the contract. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… feePool (EXSC-411) `sweep` becomes `distributeFees` (it pays out both LI.FI's and the integrator's accrued entitlements, not a one-sided sweep), and the bespoke "reservoir" term becomes "fee pool" throughout — the per-token pools the call distributes (`_distributeFeePool`, `FeePoolDistributed`). No behavior change. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🔍 QA Review — EXSC-411 / PR #1983Ticket: EXSC-411 — S3 Fee Receivers & Sweep Distribution
New commits reviewed (post last QA comment 2026-07-10T16:58:47Z)
Analysis of new commitsMerge commits (97481f, 59ba1c): Two sequential base-branch merges resolved conflicts in FIND-01 commit (00d234): FIND-02 commit (0ad36e): FIND-03 commit (6888fc): Updated status of all prior findings
Detail on remaining open itemsNEW-02 — Redirect of failed ERC-20 transfers to
|
There was a problem hiding this comment.
Requesting changes on 5 items — each requires either a code fix or an explicit acceptance comment with justification before this review is considered complete.
| # | Severity | Type | Issue / File |
|---|---|---|---|
| 1 | 🔴 High | Design | MAX_FEE_RECEIVERS = 50 product decision must be formally documented (not just Slack) |
| 2 | 🟠 Medium | Compliance | Legal/compliance sign-off required for redirect of failed ERC-20 transfers to lifiFeeRecipient |
| 3 | 🟠 Medium | Test gap | test/solidity/VaultWrapper/VaultWrapperDistribution.t.sol (NatSpec + test for receiver-mutation semantics) |
| 4 | 🟢 Low | Test gap | test/solidity/VaultWrapper/VaultWrapperDistribution.t.sol (50-wallet boundary acceptance test) |
| 5 | 🟢 Low | Test gap | test/solidity/VaultWrapper/VaultWrapperDistribution.t.sol (zero-share receiver skip test) |
1. [High] Formally document MAX_FEE_RECEIVERS = 50 product decision
The ticket specifies "1..5 receiver wallets" and a developer inline comment in the code itself asks "shouldn't this be 5 according to our requirements?". The developer confirmed via Slack that Alessandro approved N wallets — this verbal sign-off needs to be recorded formally. Required: (a) a comment on the Linear ticket from the ticket owner confirming 50 is the intended product limit, and (b) an inline @dev comment in the contract explaining the design rationale (e.g., "50 is a gas-safety cap; the product-level default is enforced by integrator onboarding, not the protocol").
2. [Medium] Legal/compliance sign-off for LI.FI receiving redirected failed transfers
When trySafeTransfer fails for an integrator wallet (e.g., USDC blacklist, OFAC-sanctioned address), the failed amount is redirected to lifiFeeRecipient — LI.FI's own fee recipient. Depending on jurisdiction, LI.FI receiving funds earmarked for a sanctioned wallet may create reporting obligations or compliance exposure. The developer has acknowledged this and is raising it with Alessandro. Before merge: legal sign-off must be documented in the PR or Linear ticket, OR the design must change (escrow contract or silent skip). No code merge without this sign-off.
**3. [Medium] Test gap — VaultWrapperDistribution.t.sol (NatSpec + test for receiver-mutation semantics)
- Missing
@devNatSpec onsetIntegratorFeeReceiversstating: "Accrued but not yet distributed fees will be paid to the new receiver set. CalldistributeFees()before calling this function if you want the outgoing receivers to receive already-earned fees." - Missing:
test_SetReceiversThenDistributeUsesNewReceivers— exercise the scenario: accrue fees, callsetIntegratorFeeReceivers, calldistributeFees(), assert fees go to the new receivers.
**4. [Low] Test gap — VaultWrapperDistribution.t.sol (50-wallet boundary acceptance)
- Missing:
test_SetIntegratorFeeReceiversAccepts50Wallets— construct exactly 50FeeReceiverentries summing to 10,000 bps and assert the call succeeds. The 51-wallet rejection is already tested; the 50-wallet acceptance must also be explicit to guard against off-by-one regression.
**5. [Low] Test gap — VaultWrapperDistribution.t.sol (zero-share receiver skip)
- Missing:
test_DistributeFeesSkipsZeroShareReceiver— configure a 1-bps receiver with a fee amount so small thatshare == 0, confirm the receiver gets nothing, and confirm total distributed remains correct (no stranded funds).
💡 Once you've addressed the items above, re-apply the "Agent Review Request" label to trigger an automated re-review.
…-types-accrual-caps-per-fee-split' into feature/exsc-411-s3-fee-receivers-sweep-distribution # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol # src/VaultWrapper/interfaces/ILiFiVaultWrapper.sol
…exsc-411-s3-fee-receivers-sweep-distribution # Conflicts: # src/VaultWrapper/LiFiVaultWrapper.sol # src/VaultWrapper/interfaces/ILiFiVaultWrapper.sol # src/VaultWrapper/interfaces/ILiFiVaultWrapperFactory.sol # test/solidity/VaultWrapper/BeaconUpgrade.t.sol # test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol # test/solidity/VaultWrapper/LiFiVaultWrapperFactory.t.sol # test/solidity/VaultWrapper/VaultWrapperFeeTestBase.sol # test/solidity/VaultWrapper/VaultWrapperPause.t.sol
Accrued-but-undistributed integrator fees are held as a single total with no per-wallet bookkeeping, so rotating receivers before distributeFees pays the already-earned fees to the new set. Document this on setIntegratorFeeReceivers and cover it with test_SetReceiversThenDistributeUsesNewReceivers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…SC-411) The 51-receiver rejection was already covered; add the mirroring acceptance case so an off-by-one in the count guard would be caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…XSC-411) A receiver whose bps share of a tiny fee pool floors to 0 is skipped and the last receiver absorbs the whole integrator total. Exercise the `share == 0` branch so a regression that stranded funds would be caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — responses to the cycle-3 review, item by item. 1. [High] 2. [Medium] Redirect of failed transfers to 3. [Medium] Receiver-rotation semantics — done. 4. [Low] 50-wallet acceptance test — done. 5. [Low] Zero-share skip test — done. FIND-04 (requires-types) and FIND-06 (dedup Full VaultWrapper suite: 232 passing. |
There was a problem hiding this comment.
Requesting changes on 4 items — each requires either a code fix or an explicit acceptance comment with justification before this review is considered complete.
| # | Severity | Type | Issue / File |
|---|---|---|---|
| 1 | 🟠 Medium | Compliance | Legal/compliance sign-off required for redirect of failed ERC-20 transfers to lifiFeeRecipient (NEW-02) |
| 2 | 🟢 Low | Process | Confirm requires-types label tracks a type-package update — link PR or document no-op (FIND-04) |
| 3 | 🟢 Low | Process | Update Linear ticket EXSC-411 description to reflect MAX_FEE_RECEIVERS = 50 cap (not "1..5 wallets") (NEW-01) |
| 4 | 🟢 Low | Process | Dismiss reopened Olympix alert on LiFiVaultWrapper.sol:123 (integratorFeeReceivers) with beacon proxy rationale (FIND-05) |
1. [Medium] Legal/compliance sign-off for redirect-to-LI.FI of failed integrator transfers
When trySafeTransfer fails for an integrator wallet (blacklisted/sanctioned address), the failed share is redirected to lifiFeeRecipient — LI.FI's own fee recipient. Depending on jurisdiction, LI.FI receiving funds earmarked for a sanctioned wallet may trigger OFAC or similar reporting obligations. The developer confirmed on 2026-07-10 this is pending explicit sign-off with Alessandro. Required: document legal/compliance sign-off in the PR or Linear ticket before merge. If sign-off is not forthcoming, options are: (a) redirect to a dedicated escrow contract instead of lifiFeeRecipient, or (b) silent skip with no redirect.
2. [Low] Confirm requires-types triggers type-package update
The requires-types label remains on the PR. The VaultWrapperConfigured event signature changed (removed factory and integratorShareBps fields). Any off-chain consumer built against the previous signature will fail to decode events from S3-deployed wrappers. Post a comment confirming the type-package PR is tracked (or that no consumer currently indexes this event, making this a no-op).
3. [Low] Update Linear ticket EXSC-411 description
The ticket description states "1..5 receiver wallets" — the code implements MAX_FEE_RECEIVERS = 50 with inline NatSpec clarifying it is a gas-safety cap, not a product limit. Update the ticket description or post a comment from the ticket owner formally recording that 50 is the protocol-level cap and 1–5 is an integrator-onboarding convention. This prevents future auditor/reviewer confusion when comparing ticket to code.
4. [Low] Dismiss reopened Olympix alert at LiFiVaultWrapper.sol:123
The merge commits triggered a new Olympix scanner alert on the integratorFeeReceivers dynamic array (line 123), flagged as "uninitialized state variable." The beacon proxy justification from the previously dismissed alerts applies: the variable is initialized write-once in initialize(), the factory deploys and initializes atomically, and _disableInitializers() in the constructor prevents the implementation itself from being left uninitialized. Dismiss with the standard CWIA/beacon proxy rationale.
💡 Once you've addressed the items above, re-apply the "Agent Review Request" label to trigger an automated re-review.
…e (S4 alternative) (#2032) * feat(EXSC-410): vault wrapper fee engine — management, deposit, withdrawal (S2) Implement the management (dilution), deposit, and withdrawal fees on top of the S1 no-op seams. Management fee accrues pro-rata on AUM and crystallizes as dilution shares minted to the wrapper on every deposit/withdraw; deposit and withdrawal fees are skimmed asset-side at transaction time and held idle so the share price is unaffected. Both sides accrue in-contract (totals only); payout and the LI.FI/integrator split are follow-up tickets. - LibVaultWrapperFees: pure fee math (OZ Convention B raw/total, management pro-rata, dilution-share formula) as the audit/fuzz surface. - Effective-supply override on _convertToShares/_convertToAssets so previews reflect pending management dilution: preview equals execution to the wei. - setFeeRate (vaultWrapperAdmin-gated) reads factory bounds live; rate 0 disables; accrues at the old rate before applying a change. Performance fee is rejected here (separate ticket EXSC-556). - Events: FeeConfigUpdated, DilutionFeeAccrued, AssetFeeCharged. - Storage appended (accruedFeeShares, accruedFeeAssets, lastMgmtAccrual); __gap reduced 49->46. Version bumped 1.0.0 -> 1.1.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: reset management-fee baseline when no fee accrues (LiFiVaultWrapper.sol:555) CodeRabbit (major): _accrueFees only advanced lastMgmtAccrual on a mint, so dormant periods (fee disabled, empty vault) and an uninitialized baseline (==0, e.g. an instance upgraded into this version) would later be charged at the current rate on current AUM. Reset the baseline to now in those cases and return; mirror the ==0 guard in _pendingManagementFee so preview==execution still holds. Sub-threshold dust handling (Q3) is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: assert exactly one DilutionFeeAccrued event, not just presence (LiFiVaultWrapperFees.t.sol:187) CodeRabbit (minor): the event-matching loop set a bool on first match, so a duplicate or stray emission would pass. Count matches and assert == 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: prove zero-rate bypasses bounds with min>0 (LiFiVaultWrapperFees.t.sol:464) CodeRabbit (minor): the test ran with bounds 0..1000, so setFeeRate(...,0) would have passed even if bounds were enforced. Set bounds to 100..1000 so zero is out of range, making the skip-bounds path the only reason it succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(EXSC-411): S3 fee receivers & sweep distribution on S2 Rebuild S3 on top of S2's accrual engine (dev-vault-wrapper <- S1 <- S2 <- S3). S2 owns accrual into undivided totals (accruedFeeAssets, accruedFeeShares); S3 adds the receiver config + a permissionless sweep that applies the integratorShareBps split at payout time over both reservoirs. - Receiver config (1..5 wallets, bps summing to 100%) set at initialize via a typed IntegratorReceivers param and mutable by the vaultWrapperAdmin; never emptiable, so an unconfigured-receivers state is impossible. - sweep() accrues pending management fees first, then distributes the idle-asset reservoir and the dilution-share reservoir in their native denominations (no adapter touch). Each split by integratorShareBps; integrator portion fanned across wallets (last absorbs the remainder); LI.FI paid to the factory's live lifiFeeRecipient. - A failing integrator transfer (e.g. a blacklisted wallet) is caught via a self-only trustedTransfer and redirected to LI.FI, so a single hostile wallet can never block the sweep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(EXSC-410): address PR review — interface-host events/errors, math lib, naming - Move all wrapper events + errors into ILiFiVaultWrapper (consistent with the factory interface); update test error references to the interface. - Rename LibVaultWrapperFees -> LibVaultWrapperMath and move the fee-adjusted share/asset conversions into it, so all wrapper arithmetic lives in one lib. - Drop feeAssets from DilutionFeeAccrued (shares is the natural unit; the event no longer requires recomputing the asset value). - NatSpec: remove ticket/stage references and the opaque "Convention B" label. - Keep @Custom:version at 1.0.0; set __gap to 50; use named args at lib call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: fuzz the decimals offset in the dilution-monotonicity test (LibVaultWrapperMath.t.sol:477) CodeRabbit (minor): testFuzz_Convert_PendingSharesDilute hardcoded _decimalsOffset 0, so it never exercised non-zero offsets. Add a bounded fuzzed offset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: drop stray node_modules symlink from tracking (node_modules:1) CodeRabbit: a node_modules symlink with an absolute local path was accidentally staged via git add -A (it escaped the node_modules/ dir-pattern gitignore as a file). Untrack it; the on-disk symlink stays for local hooks. * pr-ready: spell out 10_000 bps in receiver docs (LiFiVaultWrapperTypes.sol:48) CodeRabbit: 'bps summing to exactly 100%' reads ambiguously as [60,40]; clarify the denominator is 10_000. * pr-ready: align InvalidIntegratorShareBps NatSpec with the >= check (LiFiVaultWrapper.sol:193) CodeRabbit: the check rejects a 100% integrator share (>= 10000) so LI.FI always keeps a non-zero cut, but the doc said 'exceeds 100%'. Reword to match. * pr-ready: cover trustedTransfer self-guard and setter bad-sum (VaultWrapperDistribution.t.sol:36,149) CodeRabbit: add a direct trustedTransfer external-caller revert (OnlySelf) test — the self-call guard is the main protection against arbitrary payout routing — and assert ReceiverBpsSumNot100 on the setIntegratorReceivers update path, not just at initialize. * fix(EXSC-410): always advance mgmt-fee baseline in _accrueFees The feeShares==0 early return preserved lastMgmtAccrual so sub-threshold elapsed time would be charged later. But carried time is re-priced against whatever totalAssets is THEN: a dormant dust vault could charge a new depositor management fees for up to a year they were not invested (worst case near-confiscation via the totalAssets-1 clamp), and setFeeRate's accrue-at-old-rate-first guarantee was void whenever the old-rate accrual floored to zero. The baseline now always advances; sub-threshold time is dropped (holder-favouring, bounded by ~one share's worth per accrual). Found by workflow-backed review (finding 1, CONFIRMED). Replaces the test that asserted the carry behavior as intended with regression tests proving baseline advancement; all three fail against the old ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(EXSC-410): short-circuit zero withdrawals before the adapter call _transferOut delegatecalled the adapter with owed == 0, so on yield sources that reject zero-amount withdrawals a dust redeem whose previewRedeem is 0 (or a bare withdraw(0)) reverted — breaking the preview==execution invariant and trapping dust-share holders. Mirrors the existing zero short-circuit in _deposit. Also removes the stale S1 'fees unimplemented' sentence from _transferOut's NatSpec and fixes the dangling @notice on the fee-config-getters section divider. Regression test uses a strict ERC-4626 mock that rejects zero-amount withdrawals; verified it fails without the short-circuit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(EXSC-410): cover setFeeRate deposit/withdrawal paths, drop dup fuzz Every setFeeRate test targeted FeeType.Management, so a mis-indexed rate write or a wrong feeBounds key would have passed the suite. Adds live-effect tests (rate change observed on the next deposit/withdraw, sibling slots unchanged) and a bounds test where the deposit minimum disagrees with the management bounds, so reading the wrong key fails. Removes testFuzz_WithdrawRedeemInverse, a byte-identical copy of testFuzz_DepositMintInverse — both sides compose feeOnRaw/feeOnTotal identically, so one fuzz covers the identity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(EXSC-410): drop redundant external totalAssets() reads per operation Each deposit/withdraw performed up to 4 adapter.totalAssets() calls (two external hops each): the accrual's pending computation, the conversion override, and the pending computation again inside the conversion. Now _pendingManagementFee is parameterized on (supply, assets) so the conversion overrides pass the values they already read, a cheap-guard no-arg entry skips the external read entirely when the type is disabled or no time has elapsed, and the elapsed==0 short-circuit makes in-transaction conversions (post- accrual) free of a second pending computation. Pure caller plumbing; the fee math is unchanged. Measured on the fee suite (test totals, MockERC4626 underlying): deposit-with-pending-mgmt flows drop ~13-26k gas (e.g. 951,492 -> 925,106; 821,065 -> 807,634); deeper adapters save more per hop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(EXSC-556): performance fee (HWM dilution) + per-fee-type split at accrual Per-fee-type LI.FI/integrator split: DeployParams/initialize take uint16[4] integratorShareBps (per-element sentinel, <100% validation, self-serve above-default guard per element). The split is applied the moment a fee accrues, into packed uint128 per-recipient counters (lifi/integrator x shares/assets); integrator part floors, dust to LI.FI. Performance fee: PPS high-water mark (1e18-scaled, packed with lastMgmtAccrual), anchored at initialize, charged in _accrueFees after management dilution on gains above the watermark, watermark ratchets to the post-crystallization PPS only when shares were minted. Previews include pending perf shares via _pendingFeeShares. setFeeRate now accepts Performance (accrue-at-old-rate first; enable from disabled re-anchors the watermark); FeeTypeNotConfigurable removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * pr-ready: deploy NatSpec says shares cap below 100%, not at it (LiFiVaultWrapperFactory.sol:244) CR: '_resolveSplits rejects share >= 10000, so the onboarding manager can set at most 9999 bps, not 100%.' Comment-only wording fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(EXSC-410, EXSC-556): harden fee engine per code review - setFeeRate: enabling the performance fee from disabled re-anchors the high-water mark up-only, so toggling the fee off/on at a trough cannot reset the watermark and re-charge the recovery to the old peak - fee counters: saturate at uint128 max (with a 512-bit mulDiv split) instead of reverting, so the accrual path can never brick exits or the fee-disable escape hatch - _transferOut: book adapter withdrawal overage with the fee so idle assets stay fully attributed instead of stranding as untracked dust - dedupe the LI.FI/integrator split into _splitFee, shared by the share-side and asset-side counters Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(EXSC-410, EXSC-556): close review test gaps, extract shared base - extract VaultWrapperFeeTestBase (beacon-proxy setup, factory stack, deposit/yield/loss helpers, crystallize dust) shared by the fees and performance suites, removing ~150 duplicated lines - regression tests for the review fixes: HWM toggle at trough keeps the watermark, counter saturation never bricks exits, adapter withdrawal overage is booked not stranded - close execution-coverage gaps: mint() with the deposit fee enabled, full exit via withdraw(maxWithdraw) with the withdrawal fee enabled - fix the vacuous performance-fee monotonicity fuzz: it now compares two above-watermark fee levels instead of asserting fee >= 0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(EXSC-411): assert sweep accrues and pays out while deposits are paused The S2 base now includes the S5 pause module, so the ticket's 'sweep works while paused' behaviour is assertable — closes the deferral documented in the PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-410): address PR review — enum docs, inline _beforeOperation, fee var names, view grouping - Document each FeeType member's charging semantics in the enum - Remove the _beforeOperation indirection; entrypoints call _checkAccess + _accrueFees directly - Rename generic 'fee' locals to depositFee/withdrawalFee and feeAssets to mgmtFeeAssets/perfFeeAssets - Group maxDeposit/maxMint with the preview/limit views instead of the state-changing entrypoints Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-410): drop redundant FeeConfig.enabled flags, rate 0 is disabled A fee type's enabled state was stored twice: setFeeRate maintained enabled[i] == (rateBps[i] != 0) strictly, every consumer read the flag only as a proxy for a non-zero rate, and the factory needed a dedicated error just to police consistency between the two arrays. The rate is now the single source: 0 = disabled. - FeeConfig collapses to uint16[4] rateBps - feeEnabled() derives from the rate; FeeConfigUpdated drops its derived bool arg - Factory _validateFees skips zero rates; DisabledFeeMustBeZero error and its now-unrepresentable test removed - Enabling the performance fee from a zero rate now always re-anchors the watermark (previously skipped for vaults initialized enabled-at-0) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(EXSC-410): compare-before-add in _saturatingAddUint128 dilutionShares can return values far above 2^128 (only the _mint supply check bounds it), so the previous uint256(_accrued) + _delta could itself overflow uint256 and revert the accrual path the saturating add exists to keep alive. Comparing against the headroom first is revert-free for all inputs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-411): address S3 review feedback - pack integrator receivers into a single Receiver[] slot (address+uint16), dropping one state var so the contract clears solhint max-states-count without a disable - restore __gap to uint256[50] (fixed pre-release tail buffer) - move initialize's _initData to the last param - trim VaultWrapperConfigured to 4 fields; inline the emit and remove the _emitInitialized helper - replace the trustedTransfer self-call + try/catch with OZ v5 SafeERC20.trySafeTransfer; remove the OnlySelf error - rename sweep recipient -> lifiRecipient; de-ternary the payout share calc - set @Custom:version to 1.0.0 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(EXSC-411): raise integrator receiver cap to 50 The 5-wallet cap was more restrictive than any real fee split needs. Raise MAX_FEE_RECEIVERS to 50, which keeps the permissionless sweep() loop gas-bounded (so an integrator cannot lock fee distribution via an oversized receiver array) while removing the practical restriction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(EXSC-411): expose integratorReceivers as a public array Replace the two hand-written split getters with the compiler-generated getter on a public `Receiver[] integratorReceivers` (indexed access returns the packed wallet + bps). Also note on MAX_FEE_RECEIVERS that the cap is only a sanity bound on the permissionless sweep fan-out to prevent griefing, not a product limit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add some comments * test(EXSC-411): dedup receiver helper and cover partial fan-out redirect Address CodeRabbit nitpicks on the S3 distribution PR: - Hoist the duplicated single-wallet `_defaultReceivers()` construction into a shared free function (VaultWrapperTestHelpers.sol) and use it from the fee base, LiFiVaultWrapper, Pause, BeaconUpgrade, and Factory suites (which extend Test directly, so a base-class hoist wouldn't reach them). - Add test_SweepRedirectsOnlyFailedWalletWithinFanOut: one blocked wallet inside a 3-way fan-out redirects only its own share to LI.FI while the other wallets are paid in full and the sweep does not revert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(EXSC-412): vault wrapper access control via single pluggable gate (S4) Replace the _checkAccess seam and the opaque initData blob with one IAccessGate per instance (address(0) = fully permissionless): - entry: isAllowed(receiver) on deposit/mint (receiver, never msg.sender) - share moves: isTransferable(from, to); mints, burns, and fee payouts from the wrapper are structurally exempt so accrual/sweep never depend on gate behavior - exit: isSanctioned on share owner AND asset receiver (sanctions-only, so any non-sanctioned holder can always exit) - fail-closed everywhere, no try/catch: a misbehaving gate blocks the guarded operation and its error bubbles verbatim; recovery is the owner's instant setAccessGate - max* limit views mirror the gate; previews stay gate-blind (EIP-4626) - DeployParams.initData (bytes) becomes accessGate (address) end-to-end Alternative to the S4 module in #2026; stacked on S3 (#1983). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-410): FEE_TYPE_COUNT constant, rename split sentinel, reflow NatSpec Address review feedback on PR #1993: - Replace hardcoded uint16[4] array sizes and fee-type loop bounds with a single FEE_TYPE_COUNT constant in LiFiVaultWrapperTypes. - Rename USE_DEFAULT_SPLIT to DEFAULT_SPLIT_SENTINEL to describe what it is. - Reflow an over-length withdrawal NatSpec line to the 100-char cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: reword "overage" to "excess" in withdrawal NatSpec/comments (LiFiVaultWrapper.sol:452) CR/reviewer readability: "overage" (a correct but uncommon accounting term) reads as a possible typo; "excess" is unambiguous. Comment-only, no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(vault-wrapper): rename Receiver struct to FeeReceiver (EXSC-411) A bare `Receiver` collides with ERC-4626's `receiver` param; the struct ties a wallet to a fee bps, so `FeeReceiver` reads clearer. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(vault-wrapper): pass integrator receivers as FeeReceiver[] (EXSC-411) Replace the parallel `address[] wallets` / `uint16[] bps` boundary (the `IntegratorReceivers` struct) with a single `FeeReceiver[]` across initialize, setIntegratorReceivers, DeployParams, and the ReceiversSet event. This removes the ReceiversLengthMismatch error since the arrays can no longer diverge. Reorder initialize to persist calldata before resolving the asset, keeping its stack within the limit (no via_ir) now that the receiver param is a two-slot calldata array. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(vault-wrapper): prefix integrator receiver members with Fee (EXSC-411) Rename the storage array and its mutators to integratorFeeReceivers / setIntegratorFeeReceivers / _setIntegratorFeeReceivers so "receiver" no longer collides with the ERC-4626 `receiver` param used elsewhere in the contract. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(vault-wrapper): rename sweep to distributeFees, reservoir to feePool (EXSC-411) `sweep` becomes `distributeFees` (it pays out both LI.FI's and the integrator's accrued entitlements, not a one-sided sweep), and the bespoke "reservoir" term becomes "fee pool" throughout — the per-token pools the call distributes (`_distributeFeePool`, `FeePoolDistributed`). No behavior change. Addresses review feedback on PR #1983. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(vaultWrapper): document receiver-rotation fee semantics (EXSC-411) Accrued-but-undistributed integrator fees are held as a single total with no per-wallet bookkeeping, so rotating receivers before distributeFees pays the already-earned fees to the new set. Document this on setIntegratorFeeReceivers and cover it with test_SetReceiversThenDistributeUsesNewReceivers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(vaultWrapper): assert 50-receiver upper boundary is accepted (EXSC-411) The 51-receiver rejection was already covered; add the mirroring acceptance case so an off-by-one in the count guard would be caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(vaultWrapper): cover zero-share receiver skip in distribution (EXSC-411) A receiver whose bps share of a tiny fee pool floors to 0 is skipped and the last receiver absorbs the whole integrator total. Exercise the `share == 0` branch so a regression that stranded funds would be caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lation protection (S6) (#2036) * feat(EXSC-410): vault wrapper fee engine — management, deposit, withdrawal (S2) Implement the management (dilution), deposit, and withdrawal fees on top of the S1 no-op seams. Management fee accrues pro-rata on AUM and crystallizes as dilution shares minted to the wrapper on every deposit/withdraw; deposit and withdrawal fees are skimmed asset-side at transaction time and held idle so the share price is unaffected. Both sides accrue in-contract (totals only); payout and the LI.FI/integrator split are follow-up tickets. - LibVaultWrapperFees: pure fee math (OZ Convention B raw/total, management pro-rata, dilution-share formula) as the audit/fuzz surface. - Effective-supply override on _convertToShares/_convertToAssets so previews reflect pending management dilution: preview equals execution to the wei. - setFeeRate (vaultWrapperAdmin-gated) reads factory bounds live; rate 0 disables; accrues at the old rate before applying a change. Performance fee is rejected here (separate ticket EXSC-556). - Events: FeeConfigUpdated, DilutionFeeAccrued, AssetFeeCharged. - Storage appended (accruedFeeShares, accruedFeeAssets, lastMgmtAccrual); __gap reduced 49->46. Version bumped 1.0.0 -> 1.1.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: reset management-fee baseline when no fee accrues (LiFiVaultWrapper.sol:555) CodeRabbit (major): _accrueFees only advanced lastMgmtAccrual on a mint, so dormant periods (fee disabled, empty vault) and an uninitialized baseline (==0, e.g. an instance upgraded into this version) would later be charged at the current rate on current AUM. Reset the baseline to now in those cases and return; mirror the ==0 guard in _pendingManagementFee so preview==execution still holds. Sub-threshold dust handling (Q3) is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: assert exactly one DilutionFeeAccrued event, not just presence (LiFiVaultWrapperFees.t.sol:187) CodeRabbit (minor): the event-matching loop set a bool on first match, so a duplicate or stray emission would pass. Count matches and assert == 1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: prove zero-rate bypasses bounds with min>0 (LiFiVaultWrapperFees.t.sol:464) CodeRabbit (minor): the test ran with bounds 0..1000, so setFeeRate(...,0) would have passed even if bounds were enforced. Set bounds to 100..1000 so zero is out of range, making the skip-bounds path the only reason it succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(EXSC-411): S3 fee receivers & sweep distribution on S2 Rebuild S3 on top of S2's accrual engine (dev-vault-wrapper <- S1 <- S2 <- S3). S2 owns accrual into undivided totals (accruedFeeAssets, accruedFeeShares); S3 adds the receiver config + a permissionless sweep that applies the integratorShareBps split at payout time over both reservoirs. - Receiver config (1..5 wallets, bps summing to 100%) set at initialize via a typed IntegratorReceivers param and mutable by the vaultWrapperAdmin; never emptiable, so an unconfigured-receivers state is impossible. - sweep() accrues pending management fees first, then distributes the idle-asset reservoir and the dilution-share reservoir in their native denominations (no adapter touch). Each split by integratorShareBps; integrator portion fanned across wallets (last absorbs the remainder); LI.FI paid to the factory's live lifiFeeRecipient. - A failing integrator transfer (e.g. a blacklisted wallet) is caught via a self-only trustedTransfer and redirected to LI.FI, so a single hostile wallet can never block the sweep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(EXSC-410): address PR review — interface-host events/errors, math lib, naming - Move all wrapper events + errors into ILiFiVaultWrapper (consistent with the factory interface); update test error references to the interface. - Rename LibVaultWrapperFees -> LibVaultWrapperMath and move the fee-adjusted share/asset conversions into it, so all wrapper arithmetic lives in one lib. - Drop feeAssets from DilutionFeeAccrued (shares is the natural unit; the event no longer requires recomputing the asset value). - NatSpec: remove ticket/stage references and the opaque "Convention B" label. - Keep @Custom:version at 1.0.0; set __gap to 50; use named args at lib call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: fuzz the decimals offset in the dilution-monotonicity test (LibVaultWrapperMath.t.sol:477) CodeRabbit (minor): testFuzz_Convert_PendingSharesDilute hardcoded _decimalsOffset 0, so it never exercised non-zero offsets. Add a bounded fuzzed offset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pr-ready: drop stray node_modules symlink from tracking (node_modules:1) CodeRabbit: a node_modules symlink with an absolute local path was accidentally staged via git add -A (it escaped the node_modules/ dir-pattern gitignore as a file). Untrack it; the on-disk symlink stays for local hooks. * pr-ready: spell out 10_000 bps in receiver docs (LiFiVaultWrapperTypes.sol:48) CodeRabbit: 'bps summing to exactly 100%' reads ambiguously as [60,40]; clarify the denominator is 10_000. * pr-ready: align InvalidIntegratorShareBps NatSpec with the >= check (LiFiVaultWrapper.sol:193) CodeRabbit: the check rejects a 100% integrator share (>= 10000) so LI.FI always keeps a non-zero cut, but the doc said 'exceeds 100%'. Reword to match. * pr-ready: cover trustedTransfer self-guard and setter bad-sum (VaultWrapperDistribution.t.sol:36,149) CodeRabbit: add a direct trustedTransfer external-caller revert (OnlySelf) test — the self-call guard is the main protection against arbitrary payout routing — and assert ReceiverBpsSumNot100 on the setIntegratorReceivers update path, not just at initialize. * fix(EXSC-410): always advance mgmt-fee baseline in _accrueFees The feeShares==0 early return preserved lastMgmtAccrual so sub-threshold elapsed time would be charged later. But carried time is re-priced against whatever totalAssets is THEN: a dormant dust vault could charge a new depositor management fees for up to a year they were not invested (worst case near-confiscation via the totalAssets-1 clamp), and setFeeRate's accrue-at-old-rate-first guarantee was void whenever the old-rate accrual floored to zero. The baseline now always advances; sub-threshold time is dropped (holder-favouring, bounded by ~one share's worth per accrual). Found by workflow-backed review (finding 1, CONFIRMED). Replaces the test that asserted the carry behavior as intended with regression tests proving baseline advancement; all three fail against the old ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(EXSC-410): short-circuit zero withdrawals before the adapter call _transferOut delegatecalled the adapter with owed == 0, so on yield sources that reject zero-amount withdrawals a dust redeem whose previewRedeem is 0 (or a bare withdraw(0)) reverted — breaking the preview==execution invariant and trapping dust-share holders. Mirrors the existing zero short-circuit in _deposit. Also removes the stale S1 'fees unimplemented' sentence from _transferOut's NatSpec and fixes the dangling @notice on the fee-config-getters section divider. Regression test uses a strict ERC-4626 mock that rejects zero-amount withdrawals; verified it fails without the short-circuit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(EXSC-410): cover setFeeRate deposit/withdrawal paths, drop dup fuzz Every setFeeRate test targeted FeeType.Management, so a mis-indexed rate write or a wrong feeBounds key would have passed the suite. Adds live-effect tests (rate change observed on the next deposit/withdraw, sibling slots unchanged) and a bounds test where the deposit minimum disagrees with the management bounds, so reading the wrong key fails. Removes testFuzz_WithdrawRedeemInverse, a byte-identical copy of testFuzz_DepositMintInverse — both sides compose feeOnRaw/feeOnTotal identically, so one fuzz covers the identity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(EXSC-410): drop redundant external totalAssets() reads per operation Each deposit/withdraw performed up to 4 adapter.totalAssets() calls (two external hops each): the accrual's pending computation, the conversion override, and the pending computation again inside the conversion. Now _pendingManagementFee is parameterized on (supply, assets) so the conversion overrides pass the values they already read, a cheap-guard no-arg entry skips the external read entirely when the type is disabled or no time has elapsed, and the elapsed==0 short-circuit makes in-transaction conversions (post- accrual) free of a second pending computation. Pure caller plumbing; the fee math is unchanged. Measured on the fee suite (test totals, MockERC4626 underlying): deposit-with-pending-mgmt flows drop ~13-26k gas (e.g. 951,492 -> 925,106; 821,065 -> 807,634); deeper adapters save more per hop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(EXSC-556): performance fee (HWM dilution) + per-fee-type split at accrual Per-fee-type LI.FI/integrator split: DeployParams/initialize take uint16[4] integratorShareBps (per-element sentinel, <100% validation, self-serve above-default guard per element). The split is applied the moment a fee accrues, into packed uint128 per-recipient counters (lifi/integrator x shares/assets); integrator part floors, dust to LI.FI. Performance fee: PPS high-water mark (1e18-scaled, packed with lastMgmtAccrual), anchored at initialize, charged in _accrueFees after management dilution on gains above the watermark, watermark ratchets to the post-crystallization PPS only when shares were minted. Previews include pending perf shares via _pendingFeeShares. setFeeRate now accepts Performance (accrue-at-old-rate first; enable from disabled re-anchors the watermark); FeeTypeNotConfigurable removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * pr-ready: deploy NatSpec says shares cap below 100%, not at it (LiFiVaultWrapperFactory.sol:244) CR: '_resolveSplits rejects share >= 10000, so the onboarding manager can set at most 9999 bps, not 100%.' Comment-only wording fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(EXSC-410, EXSC-556): harden fee engine per code review - setFeeRate: enabling the performance fee from disabled re-anchors the high-water mark up-only, so toggling the fee off/on at a trough cannot reset the watermark and re-charge the recovery to the old peak - fee counters: saturate at uint128 max (with a 512-bit mulDiv split) instead of reverting, so the accrual path can never brick exits or the fee-disable escape hatch - _transferOut: book adapter withdrawal overage with the fee so idle assets stay fully attributed instead of stranding as untracked dust - dedupe the LI.FI/integrator split into _splitFee, shared by the share-side and asset-side counters Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(EXSC-410, EXSC-556): close review test gaps, extract shared base - extract VaultWrapperFeeTestBase (beacon-proxy setup, factory stack, deposit/yield/loss helpers, crystallize dust) shared by the fees and performance suites, removing ~150 duplicated lines - regression tests for the review fixes: HWM toggle at trough keeps the watermark, counter saturation never bricks exits, adapter withdrawal overage is booked not stranded - close execution-coverage gaps: mint() with the deposit fee enabled, full exit via withdraw(maxWithdraw) with the withdrawal fee enabled - fix the vacuous performance-fee monotonicity fuzz: it now compares two above-watermark fee levels instead of asserting fee >= 0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(EXSC-411): assert sweep accrues and pays out while deposits are paused The S2 base now includes the S5 pause module, so the ticket's 'sweep works while paused' behaviour is assertable — closes the deferral documented in the PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-410): address PR review — enum docs, inline _beforeOperation, fee var names, view grouping - Document each FeeType member's charging semantics in the enum - Remove the _beforeOperation indirection; entrypoints call _checkAccess + _accrueFees directly - Rename generic 'fee' locals to depositFee/withdrawalFee and feeAssets to mgmtFeeAssets/perfFeeAssets - Group maxDeposit/maxMint with the preview/limit views instead of the state-changing entrypoints Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-410): drop redundant FeeConfig.enabled flags, rate 0 is disabled A fee type's enabled state was stored twice: setFeeRate maintained enabled[i] == (rateBps[i] != 0) strictly, every consumer read the flag only as a proxy for a non-zero rate, and the factory needed a dedicated error just to police consistency between the two arrays. The rate is now the single source: 0 = disabled. - FeeConfig collapses to uint16[4] rateBps - feeEnabled() derives from the rate; FeeConfigUpdated drops its derived bool arg - Factory _validateFees skips zero rates; DisabledFeeMustBeZero error and its now-unrepresentable test removed - Enabling the performance fee from a zero rate now always re-anchors the watermark (previously skipped for vaults initialized enabled-at-0) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(EXSC-410): compare-before-add in _saturatingAddUint128 dilutionShares can return values far above 2^128 (only the _mint supply check bounds it), so the previous uint256(_accrued) + _delta could itself overflow uint256 and revert the accrual path the saturating add exists to keep alive. Comparing against the headroom first is revert-free for all inputs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(EXSC-411): address S3 review feedback - pack integrator receivers into a single Receiver[] slot (address+uint16), dropping one state var so the contract clears solhint max-states-count without a disable - restore __gap to uint256[50] (fixed pre-release tail buffer) - move initialize's _initData to the last param - trim VaultWrapperConfigured to 4 fields; inline the emit and remove the _emitInitialized helper - replace the trustedTransfer self-call + try/catch with OZ v5 SafeERC20.trySafeTransfer; remove the OnlySelf error - rename sweep recipient -> lifiRecipient; de-ternary the payout share calc - set @Custom:version to 1.0.0 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(EXSC-411): raise integrator receiver cap to 50 The 5-wallet cap was more restrictive than any real fee split needs. Raise MAX_FEE_RECEIVERS to 50, which keeps the permissionless sweep() loop gas-bounded (so an integrator cannot lock fee distribution via an oversized receiver array) while removing the practical restriction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(EXSC-411): expose integratorReceivers as a public array Replace the two hand-written split getters with the compiler-generated getter on a public `Receiver[] integratorReceivers` (indexed access returns the packed wallet + bps). Also note on MAX_FEE_RECEIVERS that the cap is only a sanity bound on the permissionless sweep fan-out to prevent griefing, not a product limit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add some comments * test(EXSC-411): dedup receiver helper and cover partial fan-out redirect Address CodeRabbit nitpicks on the S3 distribution PR: - Hoist the duplicated single-wallet `_defaultReceivers()` construction into a shared free function (VaultWrapperTestHelpers.sol) and use it from the fee base, LiFiVaultWrapper, Pause, BeaconUpgrade, and Factory suites (which extend Test directly, so a base-class hoist wouldn't reach them). - Add test_SweepRedirectsOnlyFailedWalletWithinFanOut: one blocked wallet inside a 3-way fan-out redirects only its own share to LI.FI while the other wallets are paid in full and the sweep does not revert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(EXSC-412): vault wrapper access control via single pluggable gate (S4) Replace the _checkAccess seam and the opaque initData blob with one IAccessGate per instance (address(0) = fully permissionless): - entry: isAllowed(receiver) on deposit/mint (receiver, never msg.sender) - share moves: isTransferable(from, to); mints, burns, and fee payouts from the wrapper are structurally exempt so accrual/sweep never depend on gate behavior - exit: isSanctioned on share owner AND asset receiver (sanctions-only, so any non-sanctioned holder can always exit) - fail-closed everywhere, no try/catch: a misbehaving gate blocks the guarded operation and its error bubbles verbatim; recovery is the owner's instant setAccessGate - max* limit views mirror the gate; previews stay gate-blind (EIP-4626) - DeployParams.initData (bytes) becomes accessGate (address) end-to-end Alternative to the S4 module in #2026; stacked on S3 (#1983). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(EXSC-414): vault wrapper slippage guards and inflation protection (S6) - EIP-5143 slippage overloads of deposit/mint/withdraw/redeem, reverting SlippageExceeded when the realized amount crosses the caller's bound - asset-derived virtual-share decimals offset (18 - assetDecimals, floored at 0) normalizing shares to 18 decimals, written once at initialize - MIN_SHARE_SUPPLY post-operation floor: total supply must be 0 or >= 1e6 shares, closing the dust-supply regime the inflation attack needs - same-block deposit-then-withdraw lock descoped (see Linear comment) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * pr-ready: exit-side supply floor could block last exit on fee-share dust (src/VaultWrapper/LiFiVaultWrapper.sol:381) CR finding: _accrueFees() mints fee shares to the wrapper before the exit burn; if the last holder redeems all owned shares and only a sub-floor fee-share residue remains, _enforceSupplyFloor() reverts despite the documented full-exit behavior. Fix (approved): enforce the floor on deposit/mint only. Exits are exempt so they always work; depositors entering a stranded sub-floor state are still protected because their deposit must restore the floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(LiFiVaultWrapper): reject zero-share deposits into donated empty vault The deposit-side supply floor exempted supply == 0, but that state is only reachable after a deposit that mints zero shares (assets rounded away against a donation-inflated, zero-supply vault). Such a deposit forwarded the caller's assets into the yield source for no shares — a 100% loss. Drop the exemption so any deposit/mint leaving supply below MIN_SHARE_SUPPLY reverts; exits are unaffected (they never call the floor). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(LiFiVaultWrapper): floor virtual-share offset to bound donation griefing The derived decimals offset was 0 for assets with >= 18 decimals, leaving the MIN_SHARE_SUPPLY floor as the only inflation guard. Because the floor is an absolute share count while a donation inflates the share price, an attacker could donate ~1 token to a fresh 18-decimal vault and force every sub-1M-token first deposit to mint below the floor and revert (a launch-time deposit DoS; the attacker forfeits the donation, so it is griefing, not theft). Floor the offset at MIN_DECIMALS_OFFSET = 6 (= log10(MIN_SHARE_SUPPLY)) so the donation an attacker must burn to block a deposit is at least as large as the deposit itself, making the grief self-defeating. Consequence: assets with more than 12 decimals no longer normalize to exactly 18-decimal shares (an 18-decimal asset now yields 24-decimal shares). Tests updated to derive expected share/PPS/fee values from the live offset rather than a hardcoded 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(LiFiVaultWrapper): reject assets with unreadable decimals at init OZ v5's ERC-4626 initializer silently substitutes 18 when the asset's decimals() staticcall fails or is malformed. Sizing the virtual-share offset off that fallback would quietly weaken first-depositor inflation protection for a low-decimal asset with no on-chain signal. initialize now reads the asset's decimals via an explicit staticcall and reverts AssetDecimalsUnavailable() if it is unreadable or out of uint8 range, so the offset is never derived from a fabricated value. The factory/beacon wiring tests were passing only because their codeless mock asset triggered that same silent fallback; they now use a real MockERC20. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(LiFiVaultWrapper): correct MIN_SHARE_SUPPLY floor NatSpec The offset floor (MIN_DECIMALS_OFFSET) makes the derived offset never zero, so the supply floor is no longer "only load-bearing when the offset is 0". Any nonzero first deposit now mints >= MIN_SHARE_SUPPLY shares, making the floor a backstop for the donation-inflated zero-share deposit and the sub-floor dust an exit can strand. Also corrects the share-decimals reference (shares are now >= 18 decimals, 24 for an 18-decimal asset). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(LiFiVaultWrapper): make deposit(0)/mint(0) non-reverting no-ops A zero-amount deposit or mint moves no assets and mints no shares, but the post-operation supply floor still reverted it in an empty or exit-stranded sub-floor vault. Skip the floor check when the operation is a no-op (assets == 0 for deposit, shares == 0 for mint); it still runs for every value-moving deposit, so the donation-inflated zero-share deposit (a 100% loss) stays rejected. Also documents the now-narrowed max* deviation: with the offset floored, an ordinary deposit into a clean vault always clears the floor, so the only amounts <= maxDeposit that still revert are non-zero deposits into a donation-inflated empty or exit-stranded sub-floor vault. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(LibVaultWrapperMath): note performance-fee PPS granularity The virtual-share offset makes the price-per-share step ~10^-assetDecimals of value, so a performance gain smaller than one step accrues nothing that round (the gain stays in AUM and is charged once cumulative gains cross a step). Negligible for mainstream assets (>= 6 decimals) but coarse for very-low-decimal assets (<= 3), which onboarding excludes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(LiFiVaultWrapper): enforce supply floor in shared _deposit seam Move the post-operation supply-floor check out of the deposit() and mint() entrypoints into the shared _deposit override, gated on `assets != 0`. Behavior is identical (both entrypoints route through _deposit; the gate keeps the zero-amount no-op exemption and still reverts a nonzero deposit that mints zero shares), but the guard now lives at one call site and covers every inflow entrypoint by construction. Exits go through _withdraw and are exempt by structure rather than by omission. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(VaultWrapper): fairness at minimum offset; share floor constant; dedup init encoding - Add test_InflationAttackIsUnprofitableAtMinimumOffset: exercises the donation-fairness invariant at the weakest config (18-decimal asset, offset 6, 1e6 virtual shares) — victim keeps >=99.9%, attacker takes a loss. The prior inflation test only covered the offset-12 (6-decimal) case. - Hoist MIN_SHARE_SUPPLY into VaultWrapperFeeTestBase so the suites read one shared copy instead of redeclaring it. - Centralize the initialize encoding behind _initFor/_newWrapperFor; the 6-decimal and malformed-asset helpers now delegate, so a signature change lands in one place. - Drop the unnecessary crystallizeDust=1e6 override: the 1-wei default already mints 1e6 shares and clears the floor on an empty 18-decimal vault. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(ILiFiVaultWrapper): expose EIP-5143 slippage overloads on the interface The interface declared the SlippageExceeded error but not the four guarded entrypoint signatures, so interface-typed integrations could not call them. Add the deposit/mint/withdraw/redeem overload declarations (with NatSpec mirroring the implementation) and mark the four implementations `override`. No behavior change; the concrete entrypoints are unchanged. Addresses a CodeRabbit review finding on PR #2036. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Which Linear task belongs to this PR?
EXSC-411 — S3: Fee receivers & sweep distribution
Stacked on S2. The base of this PR is the S2 branch (#1993, the fee accrual engine — now including S2b's performance fee and per-fee-type split, and the S5 pause module via the merged lineage):
dev-vault-wrapper ← S1 ← S2 ← S3. The diff is only the S3 distribution layer; retarget todev-vault-wrapperonce S2 lands.Why did I implement it this way?
S2 splits every fee between LI.FI and the integrator the moment it accrues, into four per-recipient counters:
lifiFeeAssets/integratorFeeAssets(idle asset, held in the wrapper, excluded fromtotalAssets()) andlifiFeeShares/integratorFeeShares(dilution shares minted toaddress(this)). S3 is the payout layer: it folds receiver configuration and a permissionlessdistributeFees()directly into the wrapper (S2 keeps all fee logic there, so no separate mixin), paying out exactly those tracked entitlements — nothing is re-split at distribution, so a later default-split change can never retroactively re-split already-earned fees.distributeFees()first calls_accrueFees()(so pending management/performance fees crystallize even while deposits are paused — tested), then pays out each fee pool in its own token: the idle asset as the asset, the dilution shares as wrapper shares. No adapter touch, so the distribution can never be blocked by underlying illiquidity. LI.FI's booked parts go to the factory's livelifiFeeRecipient()(an integrator can never redirect it); the integrator's booked parts are fanned across its 1..N wallets (sanity-capped at 50) by their bps, with the last wallet absorbing the integer-division remainder so the pool zeroes exactly.distributeFees(), never blockable. Each integrator transfer uses OZ's non-revertingtrySafeTransfer; a reverting wallet (e.g. a USDC-blacklisted address) has its share redirected to LI.FI in the same distribution instead of reverting. There is no separate LI.FI-only escape hatch — redirect makes one unnecessary. LI.FI's own transfer usessafeTransferand is not caught (a blacklisted LI.FI recipient is LI.FI's own, factory-governed, concern).initializevia a typedFeeReceiver[]deploy param (1..N non-zero wallets, sanity-capped at 50, bps summing to exactly 100%); thesetIntegratorFeeReceiversmutator (gated onvaultWrapperAdmin) re-validates, so an empty/unconfigured receiver set is structurally impossible — no griefing window where a permissionless distribution could divert unconfigured integrator fees.__gap(uint256[46]); the subsystem is pre-release and the layout freezes before first deployment.distributeFees()zeroes all four counters before any transfer (CEI) and isnonReentrant. If a counter ever saturated (S2's uint128 clamp), the distribution pays the tracked amount — the excess stays in the wrapper rather than reverting anything.Tests drive real accrual (deposit/withdraw + time warp through S2's engine), not seeded pools: payout equals the at-accrual tracked parts across both fee pools, 1..N-wallet fan-out with last-receiver remainder, the blacklist→redirect-to-LI.FI path, permissionless distribution, live-recipient reads, distribution-crystallizes-pending-management, distribution accrues and pays out while deposits are paused (the ticket behaviour previously deferred to S5, now in the base), zero-split books everything to LI.FI, and the init/mutator validation reverts.
/pr-ready (local CodeRabbit) findings
max-states-count— scoped disable with justification (single beacon-proxy storage host; mixins deliberately rejected).Checklist before requesting a review
/pr-ready(local CodeRabbit) on this branch and resolved (or explicitly documented) all findings — see.agents/commands/pr-ready.mdChecklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)