feat(EXSC-409): core ERC-4626 vault wrapper (S1)

[gvladika]

Re-opened for review. The original S1 PR (#1971) was merged into dev-vault-wrapper prematurely via auto-merge before SC-team review. That merge has been reverted (#1991), and this PR re-opens the identical S1 changes (same branch, tip 1d2392653) so it can be reviewed properly before landing. No code changes vs. #1971.

Which Linear task belongs to this PR?

EXSC-409 — S1: Core ERC-4626 + immutable args

Stacked PR. Base is feature/exsc-544-… (#1969), which is itself stacked on the S8 factory branch. Review the incremental diff against the base branch, and do not merge before its parents land.

Why did I implement it this way?

S1 is the base vault wrapper implementation that sits behind the beacon the factory (#1969's parent) already deploys.

• OpenZeppelin ERC4626Upgradeable base (v5.6.1), beacon implementation, initialize-based identity — not CWIA. Despite the ticket title, the merged factory deploys LibClone ERC-1967 beacon proxies and calls initialize write-once (see [CONV:VW-BEACON]). LiFiVaultWrapper is built on OZ's upgradeable ERC-4626 so the asset, decimals, and share metadata live in proxy storage (a beacon proxy cannot carry constructor-set immutables per instance), configured via the existing ILiFiVaultWrapper.initialize signature under OZ's initializer guard. CWIA bytecode-immutable identity stays deferred. OZ over Solady because a source-verified survey of major vault protocols found no production use of Solady's ERC-4626 and no documented standalone audit of it, while OZ's base has broad precedent (MetaMorpho, Gearbox, Ethena) — and Kiln DeFi, the closest comparable (integrator-facing beacon-proxy vaults with delegatecall connectors + per-vault fees), is built on ERC4626Upgradeable. The VaultWrapper subsystem runs on OZ v5.6.1 while the Diamond stays on the vendored v4.9.2 (see the dependency note below).
• Delegatecall yield adapter. IYieldAdapter is extended with deposit/withdraw (delegatecall-only, so they run in the wrapper's context and the wrapper holds the yield-source position) and totalAssets(underlying, holder) (plain view call). The call-type split is the security contract — delegatecalled methods must be stateless — and is documented in the interface NatSpec. Adapter routing lives in the _deposit/_withdraw overrides (OZ has no after/before hooks). This keeps "new yield source = new adapter, no wrapper/beacon change" ([CONV:VW-ADAPTERS]).
• Safety: core ReentrancyGuard on deposit/mint/withdraw/redeem (v5's guard is proxy-safe — namespaced storage, no initializer); OZ virtual-share offset for inflation-attack mitigation; adapter revert data bubbled verbatim.
• Factory reference captured write-once in initialize (factory = msg.sender) to unblock the S5 global circuit breaker read.
• Per-instance share metadata. name/symbol derive from the underlying asset symbol (LI.FI Earn USDC / lfUSDC) via Solady MetadataReaderLib (a utility — handles string/bytes32/missing), falling back to VW when the asset exposes no symbol, so instances are distinguishable in wallets and explorers.
• Mock removed. The temporary MockVaultWrapper is deleted; the factory, beacon, and timelock tests now exercise the real LiFiVaultWrapper, and only the underlying yield source is mocked (Solmate MockERC4626).
• Fees/access/pause are scaffolded as no-op seams (S2/S4/S5). _beforeOperation() (pause → access → fee accrual) is wired into the four entrypoints, the preview* functions route through _entryFee/_exitFee, and the deposit/withdraw hooks skim/route fees via _routeFee. Every stub is a no-op today (fees return 0, guards pass), so behaviour is unchanged; the bodies land in the fee/access/pause tickets. initData and the fee config remain stored opaquely.
• Hardened against direct/invalid initialization. The implementation is locked with _disableInitializers() (only beacon proxies initialize); initialize rejects zero asset/underlying/adapter/admin, integratorShareBps > 10000, and an adapter that resolves the underlying to a different asset than the one passed. These are defense-in-depth — redundant for the trusted factory flow, but cheap insurance on a custody contract, with testRevert_ coverage for each path.

Deferred CodeRabbit finding (final local re-run is fully clean): earlier /pr-ready runs flagged that the wrapper stores/exposes fee config and initData while fee, pause, and access enforcement are still no-op seams — so a vault could appear fee-enabled/permissioned without runtime enforcement. Deferred intentionally: storing config ahead of enforcement is the merged factory's deliberate staged design (the factory validates fees against bounds/caps and passes them through), so rejecting it in the wrapper would break the factory's plumbing and its fee-enabled deploy tests. The subsystem is unreleased and will not be production-deployed before the enforcement modules (S2 fees / S4 access / S5 pause) land — which is when these guards come off the seams.

New dependencies (OZ v5 for the VaultWrapper only): the subsystem builds on OpenZeppelin v5.6.1 while the Diamond stays on the vendored v4.9.2 core. Because v5's -upgradeable package imports the non-upgradeable core as a peer, this vendors a second core submodule lib/openzeppelin-contracts-v5 (v5.6.1) and bumps lib/openzeppelin-contracts-upgradeable to v5.6.1. Version routing is by path-scoped remappings: @openzeppelin/contracts/ resolves to v5 for src/VaultWrapper/, test/solidity/VaultWrapper/, script/deploy/vaultWrapper/, and the upgradeable lib itself; the global stays v4.9.2, so the Diamond is unaffected ([CONV:VW-OZ-VERSION]). v5 specifics absorbed: core ReentrancyGuard/IERC20/SafeERC20/Initializable, two-arg UpgradeableBeacon(impl, owner), custom-error reverts, and _initErc4626Metadata extracted to avoid a v5 stack-too-deep (no repo-wide via_ir).

Pending security triage (gates Ready/merge): Olympix flags 4 design-inherent alerts on LiFiVaultWrapper.sol — reentrancy-events ×2 (events after the guarded adapter interaction) and arbitrary-delegatecall + call-without-gas-budget on the delegatecall adapter route. These are the wrapper's intended design (governance-approved adapter, nonReentrant + CEI) and predate the v5 change; they must be dismissed-with-justification (or addressed) in code scanning before the securityAlertsReview gate will let this PR stay Ready for Review.

Divergences from the wrapper-research flow doc (which predates the merged factory) that are intentional: delegatecall adapter vs the doc's call-based/no-adapter; namespace+adapter salt vs integrator+chainLockId (chainLockId dropped); single integratorShareBps vs per-fee-type splits; opaque initData vs explicit init fields.

Checklist before requesting a review

• I have performed a self-review of my code
• This pull request is as small as possible and only tackles one problem
• I have run /pr-ready (local CodeRabbit) on this branch and resolved (or explicitly documented) all findings — see .agents/commands/pr-ready.md
• I have added tests that cover the functionality / test the bug
• For new facets: I have checked all points from this list: https://www.notion.so/lifi/New-Facet-Contract-Checklist-157f0ff14ac78095a2b8f999d655622e
• I have updated any required documentation

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

• I have checked that any arbitrary calls to external contracts are validated and or restricted
• I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
• I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 97021d60-f350-47cb-8ec7-ef51c7866d89

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/exsc-409-s1-core-erc-4626-immutable-args-cwia

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lifi-action-bot]

Test Coverage Report

Line Coverage: 90.20% (3602 / 3993 lines)
Function Coverage: 93.63% ( 574 / 613 functions)
Branch Coverage: 73.80% ( 665 / 901 branches)
Test coverage (90.20%) is above min threshold (89%). Check passed.

[mirooon]

left some comments

[lifi-qa-agent]

🔍 QA Review — EXSC-409 · S1 — Core ERC-4626 + immutable args (CWIA)

⚠️ Active code review in progress — an unresolved reviewer thread exists from the last 24h (mirooon, 2026-07-01T10:33:01Z). This QA review is advisory until those threads are resolved.

PR: #1992 · feature/exsc-409-s1-core-erc-4626-immutable-args-cwia → dev-vault-wrapper
Ticket: EXSC-409
Reviewer: lifi-qa-agent[bot] · Date: 2026-07-01

---

What this PR does

Introduces LiFiVaultWrapper.sol (422 lines) — the core ERC-4626 vault wrapper built on OpenZeppelin v5 (ERC4626Upgradeable + Ownable2StepUpgradeable + ReentrancyGuard). Deploys as a beacon proxy; configured once via initialize(). Asset pass-through to the underlying yield source is routed via IYieldAdapter (delegatecall). Fee/access/pause extension hooks are present as no-op seams for follow-up tickets. Adds two new OZ v5 git submodules with path-scoped remappings that isolate VaultWrapper from the Diamond's OZ v4.9.2. Removes MockVaultWrapper.sol placeholder. Adds a 575-line comprehensive test suite (LiFiVaultWrapper.t.sol) and updates supporting tests and deploy script.

---

Ticket quality notes

The title references "CWIA" (Clones-with-immutable-args), and the AC states "Clone immutable args via clones-with-immutable-args." The implementation uses OZ BeaconProxy + write-once initialize() storage — no bytecode-encoded immutable args, no chainLockId field. The convention file (108-vault-wrapper.md) documents CONV:VW-BEACON as the authoritative locked choice. The ticket AC was not updated to reflect this design pivot (see M-2).

---

Acceptance Criteria check

#	AC Item	Status
AC-01	ERC-4626 deposit/mint/withdraw/redeem	✅
AC-02	convert*/preview* share accounting	✅
AC-03	Pass-through to underlying via IYieldAdapter	✅
AC-04	Clone via clones-with-immutable-args	⚠️ Design pivot to BeaconProxy — M-2
AC-05	chainLockId optional arg	❌ Absent — M-2
AC-06	asset() / underlyingVault() getters	✅
AC-07	Fee/access/pause hook points (no-op seams)	✅
AC-08	Unit tests: round-trip, preview/convert correctness	✅
AC-09	Unit tests: immutable-args decoding	⚠️ BeaconProxy storage args; no bytecode-immutable test
AC-10	Mock underlying fixture	✅ (solmate MockERC4626)

---

Version / Audit check

• LiFiVaultWrapper.sol — new at @custom:version 1.0.0. No audit log entry yet; expected for pre-main merge.
• ERC4626Adapter.sol — modified at @custom:version 1.0.0. Same.
• IYieldAdapter.sol — at @custom:version 2.0.0 — incorrect for a brand-new never-deployed interface (see M-3).
• versionControlAndAuditCheck only runs on PRs targeting main. This PR targets dev-vault-wrapper; audit gating is deferred.
• requires-types label present; no TypeScript task files in diff. Must be resolved before main merge.

---

Findings

C-1 — CI Failure: enforceLibAssetRouting.yml allowlist missing VaultWrapper files [Critical / Blocks Merge]

Also flagged by reviewer mirooon (inline comment, LiFiVaultWrapper.sol line 341).

src/VaultWrapper/LiFiVaultWrapper.sol and src/VaultWrapper/adapters/ERC4626Adapter.sol both use SafeERC20.safeTransfer() / safeTransferFrom() / forceApprove() directly. The enforceLibAssetRouting.yml CI check scans all of src/ for direct ERC-20 transfer calls and fails unless the file is on its allowlist. Neither file is on the allowlist.

This is architecturally intentional (CONV:VW-OZ-VERSION explicitly sanctions OZ v5 SafeERC20 for the subsystem — it is standalone, not part of the Diamond, so LibAsset does not apply). The CI check is currently red.

Fix: Add both files to the allowlist in .github/workflows/enforceLibAssetRouting.yml with a comment:

# VaultWrapper subsystem uses OZ v5 SafeERC20 directly (CONV:VW-OZ-VERSION);
# standalone product, not Diamond — LibAsset routing does not apply.
"src/VaultWrapper/LiFiVaultWrapper.sol"
"src/VaultWrapper/adapters/ERC4626Adapter.sol"

---

M-2 — AC / Ticket deviation: CWIA claim not reflected in implementation; chainLockId absent [Medium]

The ticket title says "immutable args (CWIA)" and the AC explicitly states "Clone immutable args (underlying asset, underlying-vault target, optional chainLockId) via clones-with-immutable-args."

The implementation uses OZ BeaconProxy + write-once initialize() storage. No bytecode-encoded immutable args exist. No chainLockId field anywhere in the contract or storage layout.

CONV:VW-BEACON in 108-vault-wrapper.md documents the BeaconProxy choice as locked (CWIA is incompatible with beacon-proxied upgrades). The ticket AC was never updated to reflect this design pivot.

Required action: Update the EXSC-409 ticket AC (and title) to reflect the BeaconProxy/initialize approach and note that chainLockId is either deferred to initData or a later ticket, OR add a team-visible comment on the PR/ticket accepting the deviation with rationale. This is required before QA can mark the ticket as validated against its stated ACs.

---

M-3 — IYieldAdapter.sol versioned at 2.0.0 with no deployed 1.0.0 predecessor [Medium]

Also flagged by reviewer mirooon (inline comment, IYieldAdapter.sol line 22).

@custom:version 2.0.0 on a brand-new, never-deployed interface. Semantic versioning designates a MAJOR bump to signal a breaking change against a previously released version. There is no deployed IYieldAdapter v1.0.0 to break from. Every other new file in this subsystem starts at 1.0.0.

Fix: Change @custom:version 2.0.0 to @custom:version 1.0.0 in src/VaultWrapper/interfaces/IYieldAdapter.sol. If there is a history behind 2.0.0 (discarded spike branch), add a comment explaining it.

---

L-4 — NatSpec gap on _decimalsOffset() == 0 and inflation-attack posture [Low]

Also noted by reviewer mirooon (inline comment, LiFiVaultWrapper.sol line 31).

The class-level NatSpec states "Inflation-attack protection relies on the ERC-4626 virtual-share offset." However, _decimalsOffset() is not overridden and defaults to 0 — no virtual-share multiplier. Because totalAssets() reads a live balance from the underlying vault (not a stored accounting variable), a large direct transfer to the underlying vault would inflate totalAssets() and disadvantage a first depositor.

This is a known trade-off of the live-balance design, not a bug introduced by this PR — but the NatSpec is misleading.

Fix: Add a NatSpec note near the _decimalsOffset() non-override explaining: (a) why offset=0 is acceptable, (b) that totalAssets() reads live balance intentionally, and (c) what factory-side or operational mitigation applies (e.g., minimum first deposit seeded by deployer, or underlying vault's own inflation protection).

---

L-5 — Missing tests for initialize() zero-address guards on _underlying and _adapter [Low]

File: test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol

testRevert_InitializeRejectsZeroAddress only tests the _vaultWrapperAdmin == address(0) branch. The guard also covers _underlying == address(0) and _adapter == address(0), leaving two of three conditions untested.

Fix:

• test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol (existing):
  • Missing: testRevert_InitializeRejectsZeroUnderlying
  • Missing: testRevert_InitializeRejectsZeroAdapter

---

L-6 — Missing test: allowance-based withdraw/redeem (caller ≠ owner) [Low]

File: test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol

All withdraw and redeem tests have owner == caller. The ERC-4626 _spendAllowance path (third-party call with prior approval) is entirely untested.

Fix:

• test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol (existing):
  • Missing: test_WithdrawByApprovedCaller — Alice deposits, approves Bob's share allowance, Bob calls withdraw(assets, alice, alice), verify shares burned from Alice and assets reach Alice.
  • Missing: test_RedeemByApprovedCaller — symmetric for redeem.

---

L-7 — previewMint/previewRedeem fee-basis mismatch — forward risk for S2 [Low]

Also noted by reviewer mirooon (inline comment, LiFiVaultWrapper.sol line 274).

previewMint applies _entryFee on the net amount; _deposit applies _entryFee on the gross amount. When S2 implements non-zero entry fees, quoted cost and actual deduction will diverge systematically on the mint path. Same asymmetry exists for previewRedeem / _transferOut on the exit-fee side.

Not a bug for S1 (all fees are zero). Risk is that S2 will implement fee logic without noticing the mismatched bases.

Fix: Add a // NOTE(S2): _entryFee here is applied to NET amount; _deposit applies it to GROSS — align before enabling non-zero entry fees comment in previewMint. Same for previewRedeem / exit fee.

---

L-8 — Initialized event name collides with OZ Initializable's event [Low]

Also noted by reviewer mirooon (inline comment, LiFiVaultWrapper.sol line 84).

LiFiVaultWrapper declares event Initialized(address indexed asset, ...). OZ Initializable (v5) emits event Initialized(uint64 version). Different signatures → different topic hashes (no on-chain confusion), but two Initialized event definitions appear in the ABI and both fire during initialize(), creating noise for indexers and SDK consumers that match by name.

Fix: Rename to event WrapperInitialized(...).

---

Security assessment

Area	Result
Delegatecall target (adapter)	Write-once at init, no setter. ERC4626Adapter is stateless. ✅
Initializer guard	_disableInitializers() in constructor; initializer on initialize(). ✅
Reentrancy	nonReentrant on all four entrypoints; OZ v5 ERC-7201 slot (collision-free). ✅
CEI in _deposit	super._deposit() (pull + mint) before adapter delegation; revert unwinds atomically. ✅
CEI in _transferOut	Burn happens inside OZ _withdraw before _transferOut; shortfall check fires before receiver transfer. ✅
Admin role	Two-step ownership; renounceOwnership disabled. Appropriate for a custody contract. ✅
forceApprove in adapter	Under delegatecall approves yield source for wrapper's asset. Target is governance-approved. ✅
Adapter shortfall guards	AdapterDepositShortfall + AdapterWithdrawShortfall tested with LossyVault. ✅
Inflation attack	_decimalsOffset() == 0 + live totalAssets() read. Not a bug; NatSpec should be more explicit (L-4).
Beacon upgrade path	Owner = 48h timelock; upgradeTo gated. No bypass in this PR. ✅
SafeERC20 direct use	CI check failing — C-1. Architecturally intentional; needs allowlist carve-out.

---

Test coverage assessment

Scenario	Covered
Round-trip deposit/withdraw/mint/redeem	✅
Preview correctness vs actual	✅
Yield accrual share math	✅
Two-depositor proportional shares	✅
Adapter revert bubbles up	✅
Reentrancy blocked	✅
Deposit/withdraw shortfall from adapter	✅
Two-step ownership transfer	✅
renounceOwnership disabled	✅
initialize() rejects zero admin	✅
Double-init blocked	✅
initialize() rejects zero underlying	❌ L-5
initialize() rejects zero adapter	❌ L-5
Allowance-based withdraw (caller ≠ owner)	❌ L-6
Fees / access / pause (S2–S5)	Out of S1 scope — deferred

Coverage report: Line 89.83% / Function 93.27% / Branch 72.83% (uncovered branches are primarily no-op stubs for S2–S5 seams — expected for S1 scope).

---

Ticket coverage rating: Substantial, with one blocking CI issue and two AC discrepancies requiring explicit acceptance

---

Downstream impact

• EXSC-410 (S2 — Fee engine), EXSC-412 (S4 — Access), EXSC-413 (S5 — Pause), EXSC-414 (S6 — MEV/slippage), EXSC-415 (S7 — Metadata), EXSC-419 (S13 — Composer) all block on this PR. The previewMint fee-basis mismatch (L-7) is a direct risk for S2.
• C-1 (CI allowlist) must be resolved to unblock PR merge; it does not block downstream development on stacked PRs.

---

✅ Verdict: Needs Work

Severity	Count	Items
🔴 Critical	1	C-1 — CI enforceLibAssetRouting allowlist missing VaultWrapper files (red CI)
🟠 Medium	2	M-2 — CWIA/chainLockId AC deviation; M-3 — IYieldAdapter v2.0.0 versioning
🟡 Low	5	L-4 — NatSpec inflation posture; L-5 — init zero-addr tests; L-6 — allowance withdraw tests; L-7 — previewMint fee basis; L-8 — Initialized event name
ℹ️ Informational	2	I-9 — impl initialized() returns true (expected); I-10 — OZ override param naming (acceptable)

---

Posted by lifi-qa-agent[bot] · EXSC-409 · QA Agent SDK

[lifi-qa-agent]

Requesting changes on 8 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	🔴 Critical	Code	enforceLibAssetRouting CI check failing — VaultWrapper files not in allowlist
2	🟠 Medium	Documentation	AC / ticket title claims CWIA; implementation uses BeaconProxy; chainLockId absent
3	🟠 Medium	Convention	IYieldAdapter.sol versioned at 2.0.0 with no deployed 1.0.0 predecessor
4	🟡 Low	Documentation	NatSpec missing _decimalsOffset() == 0 risk acceptance and inflation posture
5	🟡 Low	Test gap	test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol (existing)
6	🟡 Low	Test gap	test/solidity/VaultWrapper/LiFiVaultWrapper.t.sol (existing)
7	🟡 Low	Documentation	previewMint/previewRedeem fee-basis mismatch creates forward risk for S2
8	🟡 Low	Convention	Initialized event name collides with OZ Initializable's event in the ABI

1. [Critical] enforceLibAssetRouting CI check is red — VaultWrapper files missing from allowlist

src/VaultWrapper/LiFiVaultWrapper.sol and src/VaultWrapper/adapters/ERC4626Adapter.sol both use SafeERC20.safeTransfer() / forceApprove() directly. The enforceLibAssetRouting.yml workflow scans all of src/ and fails unless the file is on its allowlist. Neither is. The check is currently red.

The use is architecturally correct (CONV:VW-OZ-VERSION sanctions OZ v5 SafeERC20 for the VaultWrapper subsystem — it is not a Diamond contract, so LibAsset routing does not apply). The CI allowlist needs a carve-out:

# VaultWrapper subsystem uses OZ v5 SafeERC20 directly (CONV:VW-OZ-VERSION);
# standalone product, not Diamond — LibAsset routing does not apply.
"src/VaultWrapper/LiFiVaultWrapper.sol"
"src/VaultWrapper/adapters/ERC4626Adapter.sol"

2. [Medium] Ticket AC deviation — CWIA / chainLockId

The ticket title ("immutable args (CWIA)") and AC ("Clone immutable args … via clones-with-immutable-args") conflict with the implementation (OZ BeaconProxy + write-once initialize() storage, no bytecode-immutable args, no chainLockId). The CONV:VW-BEACON rule documents the BeaconProxy choice as locked and correct.

Required: Update EXSC-409's AC and title to reflect the BeaconProxy/initialize design, and note chainLockId status (deferred to initData or a later ticket). Alternatively, add a team comment on the PR explicitly accepting the deviation with rationale. This is required before QA can verify the ticket against its stated ACs.

3. [Medium] IYieldAdapter.sol @custom:version 2.0.0 — incorrect semver for an undeployed interface

A MAJOR semver bump signals a breaking change against an already-released version. There is no deployed IYieldAdapter v1.0.0 to break from — this is the initial ship. Every other new file in the subsystem starts at 1.0.0. Change @custom:version 2.0.0 to @custom:version 1.0.0.

4. [Low] NatSpec gap — _decimalsOffset() == 0 inflation posture

@custom:notice says "Inflation-attack protection relies on the ERC-4626 virtual-share offset." But _decimalsOffset() is not overridden and defaults to 0. Because totalAssets() reads a live balance from the underlying vault, a large direct transfer inflates totalAssets() and can disadvantage a first depositor.

Add a NatSpec note explaining: (a) why offset=0 is acceptable here, (b) that totalAssets() reads live balance intentionally, (c) what factory-side or operational mitigation applies (e.g., minimum first deposit, or yield source's own inflation protection).

5. [Low] Test gap — LiFiVaultWrapper.t.sol

• Missing: testRevert_InitializeRejectsZeroUnderlying — verify _underlying == address(0) triggers ZeroAddress()
• Missing: testRevert_InitializeRejectsZeroAdapter — verify _adapter == address(0) triggers ZeroAddress()

6. [Low] Test gap — LiFiVaultWrapper.t.sol

• Missing: test_WithdrawByApprovedCaller — Alice deposits, approves Bob's share allowance, Bob calls withdraw(assets, alice, alice), verify shares burned from Alice and assets reach Alice
• Missing: test_RedeemByApprovedCaller — symmetric for redeem

7. [Low] previewMint/previewRedeem fee-basis mismatch — forward S2 risk

previewMint applies _entryFee on the net amount; _deposit applies it on the gross amount. When S2 enables non-zero entry fees, the quoted cost and the actual deduction will drift on the mint path. Same asymmetry on previewRedeem / _transferOut for exit fees.

Not a bug today (fees are zero). Add a // NOTE(S2): _entryFee here is applied to NET; _deposit applies to GROSS — align before enabling non-zero entry fees comment in previewMint and a corresponding note in previewRedeem.

8. [Low] Initialized event name collides with OZ Initializable's event

LiFiVaultWrapper declares event Initialized(address indexed asset, ...). OZ Initializable v5 emits event Initialized(uint64 version). Different signatures → different topic hashes, but both events appear in the ABI and both fire during initialize(), creating noise for indexers and SDK consumers that match by name.

Rename to event WrapperInitialized(...).

---

💡 Once you've addressed the items above, re-apply the "Agent Review Request" label to trigger an automated re-review.