Dev

[matteoettam09]

Summary by Sourcery

Add comprehensive accounting, slippage, decommissioning, and manager lifecycle coverage through new tests and minor contract/interface adjustments.

New Features:

• Introduce an OrionVault accounting test suite covering fee calculations, hurdle benchmarks, high water marks, and point-in-time asset conversions.
• Add a test-only ERC4626 vault with mutable decimals to validate adapter behavior when asset metadata changes.

Bug Fixes:

• Ensure liquidity orchestration sell and buy legs correctly treat successful adapter calls and only short-circuit on failures.
• Verify decommissioned vault behavior for ERC4626 redeem edge cases, including exceeding balance and missing allowance.
• Validate that removing a whitelisted manager updates the manager registry and that repeated asset decommissioning calls do not emit duplicate events.
• Enforce slippage limits in ERC4626 execution adapter buy/sell flows through regression tests and behavior verification.

Enhancements:

• Standardize terminology from 'vault owner' to 'manager' across access control and removal tests for clarity.
• Extend config tests to assert whitelisted asset names remain consistent during decommissioning and that manager lists are updated on removal.
• Clarify protocol events and interfaces by documenting return values and marking additional event fields as indexed for improved observability.

Tests:

• Expand adapter, removal, access control, fee cooldown, and config test suites with additional edge cases around slippage, decommissioning state, management permissions, and event emission.
• Add an accounting-focused test suite that exercises vault fee computation paths and share-to-asset conversion logic under various fee models and market conditions.

Summary by CodeRabbit

• New Features

  • Added new verifier interface for enhanced proof verification.

• Tests

  • Added comprehensive test suites for vault accounting, adapter validation, and decommissioning scenarios.
  • Enhanced existing tests with improved state validation and edge case coverage.

[sourcery-ai]

Reviewer's Guide

Adds comprehensive accounting, slippage, decommissioning, and access-control tests; introduces a mutable-decimals ERC4626 mock; tightens liquidity orchestration error handling and protocol events/interfaces; and standardizes terminology from "vault owner" to "manager" across tests and config APIs.

Sequence diagram for updated liquidity orchestration sell/buy execution error handling

sequenceDiagram
    actor Keeper
    participant LiquidityOrchestrator
    participant Token

    Keeper->>LiquidityOrchestrator: performLiquidityUpkeep()
    loop For_each_selling_token
        LiquidityOrchestrator->>LiquidityOrchestrator: _executeSell(token, amount, estUnderlying)
        alt _executeSell_success
            LiquidityOrchestrator-->>LiquidityOrchestrator: continue to next token
        else _executeSell_reverts
            LiquidityOrchestrator->>LiquidityOrchestrator: currentPhase = StateCommitment
            LiquidityOrchestrator->>LiquidityOrchestrator: _failedEpochTokens.push(token)
            LiquidityOrchestrator-->>Keeper: return
        end
    end

    loop For_each_buying_token
        LiquidityOrchestrator->>LiquidityOrchestrator: _executeBuy(token, amount, estUnderlying)
        alt _executeBuy_success
            LiquidityOrchestrator-->>LiquidityOrchestrator: continue to next token
        else _executeBuy_reverts
            LiquidityOrchestrator->>LiquidityOrchestrator: currentPhase = StateCommitment
            LiquidityOrchestrator->>LiquidityOrchestrator: _failedEpochTokens.push(token)
            LiquidityOrchestrator-->>Keeper: return
        end
    end

Loading

Class diagram for updated verifier and vault interfaces and mock ERC4626

classDiagram
    class ISP1Verifier {
        <<interface>>
        +verifyProof(programVKey bytes32, publicValues bytes, proofBytes bytes) view
    }

    class ISP1VerifierWithHash {
        <<interface>>
        +VERIFIER_HASH() bytes32
    }

    ISP1VerifierWithHash --|> ISP1Verifier

    class IERC4626 {
        <<interface>>
        +asset() address
        +totalAssets() uint256
        +convertToShares(assets uint256) uint256
        +convertToAssets(shares uint256) uint256
        +deposit(assets uint256, receiver address) uint256
        +mint(shares uint256, receiver address) uint256
        +withdraw(assets uint256, receiver address, owner address) uint256
        +redeem(shares uint256, receiver address, owner address) uint256
    }

    class IOrionVault {
        <<interface>>
        +VaultFeesClaimed(manager address, feeAmount uint256) event
    }

    IOrionVault --|> IERC4626

    class IOrionConfig {
        <<interface>>
        +isWhitelisted(asset address) bool view
        +decommissioningAssets() address[] view
    }

    class ERC4626 {
        +asset() address
        +totalAssets() uint256
        +convertToShares(assets uint256) uint256
        +convertToAssets(shares uint256) uint256
        +deposit(assets uint256, receiver address) uint256
        +mint(shares uint256, receiver address) uint256
        +withdraw(assets uint256, receiver address, owner address) uint256
        +redeem(shares uint256, receiver address, owner address) uint256
    }

    class MockERC4626WithSettableDecimals {
        +setDecimals(newDecimals uint8)
        +decimals() uint8
    }

    MockERC4626WithSettableDecimals --|> ERC4626

Loading

Class diagram for updated protocol events in EventsLib

classDiagram
    class EventsLib {
        <<library>>
        +WhitelistedAssetRemoved(asset address) event
        +AssetDecommissioningInitiated(asset address) event
        +LiquidityDeposited(depositor address, amount uint256) event
        +LiquidityWithdrawn(withdrawer address, amount uint256) event
    }

Loading

File-Level Changes

Change	Details	Files
Extend ERC4626 execution adapter and vault removal test coverage for edge cases around decimals changes, slippage, and decommissioned vault behavior.	Add test ensuring validateExecutionAdapter reverts with InvalidAdapter if a whitelisted ERC4626 asset later changes its decimals. Add sell-path test asserting SlippageExceeded behavior when redeem returns less than the max underlying amount after simulated losses. Add buy-path test asserting SlippageExceeded behavior when previewMint requires more underlying than max allowed after simulated gains. Add decommissioned-vault redeem tests for exceeding owner balance and missing allowance, and explicit assertions around decommissioned vault bookkeeping and behavior of requestDeposit/requestRedeem when decommissioning.	test/Adapters.test.ts test/Removal.test.ts
Standardize terminology from "vault owner" to "manager" and strengthen manager removal semantics and tests.	Rename VaultOwnerRemoval tests/file to ManagerRemoval and update test descriptions and comments to use "manager" terminology. Ensure removeWhitelistedManager decommissions only vaults belonging to the removed manager and leaves others active. Add tests verifying only owner can remove managers, correct handling when system not idle or manager not whitelisted, and that getAllOrionManagers reflects removals.	test/ManagerRemoval.test.ts test/OrionConfigVault.test.ts test/AccessControl.test.ts test/FeeCooldown.test.ts
Enhance OrionConfig asset and manager bookkeeping, including events and whitelists.	Add test that AssetDecommissioningInitiated is emitted only on the first removeWhitelistedAsset call for a given asset. Add tests asserting asset names remain present in getAllWhitelistedAssetNames while assets are decommissioning. Add tests verifying getAllOrionManagers includes a manager before and excludes it after removeWhitelistedManager, and that add/remove manager tests use updated terminology.	test/OrionConfigVault.test.ts
Refine LiquidityOrchestrator error handling around adapter execution and improve event/interface metadata.	Wrap _executeSell and _executeBuy calls in try/catch with explicit empty-success branches and failure path that switches to StateCommitment and records failed tokens. Add NatSpec @param documentation for AssetDecommissioningInitiated and @return docs for decommissioningAssets. Index the amount field on liquidity buffer and vault fee events to aid querying (LiquidityDeposited, LiquidityWithdrawn, VaultFeesClaimed). Document ISP1VerifierWithHash interface purpose and relationship to ISP1Verifier.	contracts/LiquidityOrchestrator.sol contracts/libraries/EventsLib.sol contracts/interfaces/IOrionVault.sol contracts/interfaces/IOrionConfig.sol contracts/interfaces/ISP1Verifier.sol
Introduce a dedicated accounting test suite for OrionVault fee logic and point-in-time conversions.	Add Accounting.test.ts covering convertToAssetsWithPITTotalAssets behavior with floor/ceil rounding, virtual supply, and integration in fulfillRedeem using snapshot totalSupply. Add tests for management-fee calculation proportional to assets, epoch duration, and fee bps, including zero-fee cases. Add tests for performance-fee behavior under ABSOLUTE, HIGH_WATER_MARK, SOFT_HURDLE, HARD_HURDLE, and HURDLE_HWM modes, including interaction with protocol risk-free rate and hurdle price. Verify vaultFee returns management then performance fee and that the performance component is computed on post-management-fee assets.	test/Accounting.test.ts
Add a test-only ERC4626 implementation with mutable decimals to support adapter validation tests.	Implement MockERC4626WithSettableDecimals that extends MockERC4626Asset, overrides decimals(), and exposes setDecimals(uint8) to change the reported decimals after deployment. Use the new mock in adapter tests to simulate assets whose decimals change post-whitelisting and ensure adapter validation detects mismatches.	contracts/test/MockERC4626WithSettableDecimals.sol test/Adapters.test.ts

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes introduce event signature modifications (indexed parameters for fees and liquidity amounts), a new SP1 verifier interface with hash accessor, new test contract for decimal overriding, comprehensive accounting and adapter test suites, and terminology updates renaming "vault owner" to "manager" across multiple test files.

Changes

Cohort / File(s)	Summary
Event Signature Updates contracts/interfaces/IOrionVault.sol, contracts/libraries/EventsLib.sol	Modified VaultFeesClaimed, LiquidityDeposited, and LiquidityWithdrawn events to index feeAmount and amount parameters respectively, enabling filtering by these values on-chain.
Interface Additions contracts/interfaces/ISP1Verifier.sol, contracts/interfaces/IOrionConfig.sol	Added new ISP1VerifierWithHash interface extending ISP1Verifier with VERIFIER_HASH() function; added documentation for decommissioningAssets() getter in IOrionConfig.
LiquidityOrchestrator Inline Documentation contracts/LiquidityOrchestrator.sol	Added inline comments in catch blocks of minibatch handlers to annotate successful execution continuation; no functional or control-flow changes.
Test Infrastructure contracts/test/MockERC4626WithSettableDecimals.sol	New test contract extending MockERC4626Asset with runtime-settable ERC4626 decimals via setDecimals() function and _overrideDecimals override mechanism.
Test Terminology Updates test/AccessControl.test.ts, test/FeeCooldown.test.ts, test/ManagerRemoval.test.ts, test/OrionConfigVault.test.ts	Replaced "vault owner" terminology with "manager" across test descriptions, variable names, and authorization checks without altering test logic or assertions.
Comprehensive Test Coverage test/Accounting.test.ts, test/Adapters.test.ts, test/Removal.test.ts	Added extensive test suites for vault accounting (fee models: ABSOLUTE, HIGH_WATER_MARK, SOFT_HURDLE, HARD_HURDLE, HURDLE_HWM), adapter validation with decimals changes, slippage scenarios, and decommissioning edge cases (ERC4626ExceededMaxRedeem, ERC20InsufficientAllowance).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Dev #134 — Modifies the same EventsLib.sol event signatures (LiquidityDeposited and LiquidityWithdrawn amount indexing) as this PR.
• Dev #91 — Changes LiquidityOrchestrator.sol minibatch handler logic in the same catch block areas touched by inline comment additions in this PR.
• Dev #73 — Performs broader refactoring of LiquidityOrchestrator.sol, overlapping with the same file modified in this PR.

Poem

🐰 Events now indexed bright,
Managers dance in tests' light,
Accounting's maze laid bare,
Adapters validated with care,
Comments guide the catch-block's flight! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The pull request title 'Dev' is vague and generic, providing no meaningful information about the changeset's purpose or scope.	Use a descriptive title that summarizes the main changes, such as 'Add event indexing, test utilities, and test coverage for vault accounting' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dev

---

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 and usage tips.}

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In the new slippage tests for sell and buy you describe SlippageExceeded in the it string but only assert .to.be.reverted; consider asserting the specific custom error (or updating the description) so the tests clearly validate the intended failure mode.
• The new decommissioning-related tests in Removal.test.ts re-implement the same vault-decommissioning setup multiple times; consider extracting a shared helper to prepare a decommissioned vault to reduce duplication and make the scenarios easier to maintain.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In the new slippage tests for `sell` and `buy` you describe `SlippageExceeded` in the `it` string but only assert `.to.be.reverted`; consider asserting the specific custom error (or updating the description) so the tests clearly validate the intended failure mode.
- The new decommissioning-related tests in `Removal.test.ts` re-implement the same vault-decommissioning setup multiple times; consider extracting a shared helper to prepare a decommissioned vault to reduce duplication and make the scenarios easier to maintain.

## Individual Comments

### Comment 1
<location> `contracts/LiquidityOrchestrator.sol:694-696` </location>
<code_context>
             if (token == address(underlyingAsset)) continue;
             uint256 amount = sellLeg.sellingAmounts[i];
-            try this._executeSell(token, amount, sellLeg.sellingEstimatedUnderlyingAmounts[i]) {} catch {
+            try this._executeSell(token, amount, sellLeg.sellingEstimatedUnderlyingAmounts[i]) {
+                // successful execution, continue.
+            } catch {
                 currentPhase = LiquidityUpkeepPhase.StateCommitment;
                 _failedEpochTokens.push(token);
</code_context>

<issue_to_address>
**suggestion:** The generic catch block swallows error details, which can hinder debugging of failed token operations.

Because the untyped `catch` hides why `_executeSell` / `_executeBuy` reverted, diagnosing issues with specific tokens becomes harder, especially given this code drives phase transitions and failed-token tracking. Consider using a typed catch (e.g., `catch Error(string memory reason)` or `catch (bytes memory lowLevelData)`) and emitting an event with the token and error details to retain observability without changing the control flow.

Suggested implementation:

```
            address token = sellLeg.sellingTokens[i];
            if (token == address(underlyingAsset)) continue;
            uint256 amount = sellLeg.sellingAmounts[i];
            try this._executeSell(token, amount, sellLeg.sellingEstimatedUnderlyingAmounts[i]) {
                // successful execution, continue.
            } catch (bytes memory lowLevelData) {
                emit TokenOperationFailed(token, true, lowLevelData);
                currentPhase = LiquidityUpkeepPhase.StateCommitment;
                _failedEpochTokens.push(token);
                return;
            address token = buyLeg.buyingTokens[i];
            if (token == address(underlyingAsset)) continue;
            uint256 amount = buyLeg.buyingAmounts[i];
            try this._executeBuy(token, amount, buyLeg.buyingEstimatedUnderlyingAmounts[i]) {
                // successful execution, continue.
            } catch (bytes memory lowLevelData) {
                emit TokenOperationFailed(token, false, lowLevelData);
                currentPhase = LiquidityUpkeepPhase.StateCommitment;
                _failedEpochTokens.push(token);

```

1. Add an event declaration inside the `LiquidityOrchestrator` contract (near the top of the contract body is typical):
   ```solidity
   event TokenOperationFailed(address indexed token, bool isSell, bytes lowLevelData);
   ```
   - `isSell == true` will denote `_executeSell` failures; `false` will denote `_executeBuy` failures.
2. If your project uses an existing logging/telemetry pattern (e.g., a more specific event naming convention), adjust the event name and parameters accordingly and update the `emit` calls to match.
3. Ensure the control-flow braces around these snippets are correctly balanced; the provided context is partial, so you may need to re-run `solc` / your linter to fix any structural issues around the `return;` and closing braces.
</issue_to_address>

### Comment 2
<location> `test/Accounting.test.ts:133-137` </location>
<code_context>
+    await underlyingAsset.connect(user).approve(await vault.getAddress(), parseUnderlying("1000000"));
+  });
+
+  describe("convertToAssetsWithPITTotalAssets", function () {
+    it("returns assets using current totalSupply and point-in-time total assets (Floor)", async function () {
+      const depositAssets = parseUnderlying("100000");
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding edge-case tests for `convertToAssetsWithPITTotalAssets` with zero shares / zero PIT total assets.

Please also add tests for edge inputs:

- `shares = 0` and non-zero `pitTotalAssets` → expect 0 assets.
- Non-zero `shares` and `pitTotalAssets = 0` → verify behavior (likely 0 assets) and that it does not revert.
- Both `shares = 0` and `pitTotalAssets = 0`.

These will harden coverage around division/rounding corner cases and empty-position scenarios.

```suggestion
    await underlyingAsset.mint(user.address, parseUnderlying("1000000"));
    await underlyingAsset.connect(user).approve(await vault.getAddress(), parseUnderlying("1000000"));
  });

  describe("convertToAssetsWithPITTotalAssets edge cases", function () {
    it("returns 0 assets when shares is 0 and pitTotalAssets is non-zero", async function () {
      const depositAssets = parseUnderlying("100000");
      await vault.connect(user).requestDeposit(depositAssets);
      await setVaultStateWithFulfilledDeposit(vault, depositAssets, depositAssets);

      const pitTotalAssets = depositAssets;
      const zeroShares = 0n;

      const assets = await vault.convertToAssetsWithPITTotalAssets(
        zeroShares,
        pitTotalAssets,
        Rounding.Floor,
      );
      expect(assets).to.equal(0n);
    });

    it("returns 0 assets and does not revert when pitTotalAssets is 0 and shares are non-zero", async function () {
      const depositAssets = parseUnderlying("100000");
      await vault.connect(user).requestDeposit(depositAssets);
      await setVaultStateWithFulfilledDeposit(vault, depositAssets, depositAssets);

      const supply = await vault.totalSupply();
      expect(supply).to.be.gt(0);

      const nonZeroShares = 10n ** BigInt(SHARE_DECIMALS);
      const pitTotalAssets = 0n;

      const assets = await vault.convertToAssetsWithPITTotalAssets(
        nonZeroShares,
        pitTotalAssets,
        Rounding.Floor,
      );
      expect(assets).to.equal(0n);
    });

    it("returns 0 assets when both shares and pitTotalAssets are 0", async function () {
      const zeroShares = 0n;
      const pitTotalAssets = 0n;

      const assets = await vault.convertToAssetsWithPITTotalAssets(
        zeroShares,
        pitTotalAssets,
        Rounding.Floor,
      );
      expect(assets).to.equal(0n);
    });
  });

  describe("convertToAssetsWithPITTotalAssets", function () {
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion: The generic catch block swallows error details, which can hinder debugging of failed token operations.

Because the untyped catch hides why _executeSell / _executeBuy reverted, diagnosing issues with specific tokens becomes harder, especially given this code drives phase transitions and failed-token tracking. Consider using a typed catch (e.g., catch Error(string memory reason) or catch (bytes memory lowLevelData)) and emitting an event with the token and error details to retain observability without changing the control flow.

Suggested implementation:

            address token = sellLeg.sellingTokens[i];
            if (token == address(underlyingAsset)) continue;
            uint256 amount = sellLeg.sellingAmounts[i];
            try this._executeSell(token, amount, sellLeg.sellingEstimatedUnderlyingAmounts[i]) {
                // successful execution, continue.
            } catch (bytes memory lowLevelData) {
                emit TokenOperationFailed(token, true, lowLevelData);
                currentPhase = LiquidityUpkeepPhase.StateCommitment;
                _failedEpochTokens.push(token);
                return;
            address token = buyLeg.buyingTokens[i];
            if (token == address(underlyingAsset)) continue;
            uint256 amount = buyLeg.buyingAmounts[i];
            try this._executeBuy(token, amount, buyLeg.buyingEstimatedUnderlyingAmounts[i]) {
                // successful execution, continue.
            } catch (bytes memory lowLevelData) {
                emit TokenOperationFailed(token, false, lowLevelData);
                currentPhase = LiquidityUpkeepPhase.StateCommitment;
                _failedEpochTokens.push(token);

1. Add an event declaration inside the LiquidityOrchestrator contract (near the top of the contract body is typical):

   event TokenOperationFailed(address indexed token, bool isSell, bytes lowLevelData);

   • isSell == true will denote _executeSell failures; false will denote _executeBuy failures.
2. If your project uses an existing logging/telemetry pattern (e.g., a more specific event naming convention), adjust the event name and parameters accordingly and update the emit calls to match.
3. Ensure the control-flow braces around these snippets are correctly balanced; the provided context is partial, so you may need to re-run solc / your linter to fix any structural issues around the return; and closing braces.

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@contracts/libraries/EventsLib.sol`:
- Around line 21-23: The LiquidityDeposited and LiquidityWithdrawn event
definitions were changed to mark the amount parameter as indexed, which alters
the ABI/topic layout; revert the change by removing the indexed modifier from
the amount parameter in the LiquidityDeposited and LiquidityWithdrawn event
declarations so the event signature and topics match the previous ABI (leave any
legitimately indexed address parameters as-is) and run a quick compile to ensure
the original event signatures are restored for off-chain consumers.

In `@contracts/test/MockERC4626WithSettableDecimals.sol`:
- Around line 26-28: The setDecimals(uint8 d) function on
MockERC4626WithSettableDecimals currently allows anyone to change
_overrideDecimals; update the contract by adding a brief inline comment above
setDecimals stating this is intentionally unrestricted for testing convenience
(or, if you prefer stricter access, restrict it to onlyOwner by adding an access
check) so readers understand the choice; reference the function name setDecimals
and the variable _overrideDecimals when adding the explanatory note or when
applying the access modifier.

In `@test/Accounting.test.ts`:
- Around line 66-76: The test currently uses a non-null assertion on `log` and
then calls `transparentVaultFactory.interface.parseLog(log!)?.args`, which can
throw or produce unclear failures if the event isn't found; update the helper to
explicitly check whether `log` is null/undefined and throw a clear error (e.g.
"OrionVaultCreated event not found in receipt") before calling
`transparentVaultFactory.interface.parseLog`, and also validate that `args` and
`vaultAddress` are defined before calling
`ethers.getContractAt("OrionTransparentVault", vaultAddress)` (or throw another
descriptive error) so failures point to the missing event/arg rather than a
cryptic runtime exception.

In `@test/Adapters.test.ts`:
- Around line 457-461: The test currently uses a generic `.to.be.reverted` when
calling erc4626ExecutionAdapter.connect(loSigner).sell(...), which can mask
wrong failures; update the assertion to expect the specific custom error
`SlippageExceeded` using the Chai/Waffle custom error matcher (e.g.,
`.to.be.revertedWithCustomError(erc4626ExecutionAdapter, 'SlippageExceeded')`)
so the test asserts the adapter's `sell` call fails for the intended slippage
condition; locate the call to `erc4626ExecutionAdapter.connect(loSigner).sell`
and replace the generic revert assertion with the custom error assertion
referencing `erc4626ExecutionAdapter` and `SlippageExceeded`.
- Around line 490-494: Replace the generic `.to.be.reverted` assertion with a
custom-error assertion for SlippageExceeded: call the same promise (the result
of `erc4626ExecutionAdapter.connect(loSigner).buy(await
erc4626Vault.getAddress(), sharesAmount, estimatedUnderlying)`) but assert
`.to.be.revertedWithCustomError(erc4626ExecutionAdapter, 'SlippageExceeded')`
(or the appropriate contract instance exposing the error) so the test
specifically checks for the SlippageExceeded custom error.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!