[Vault] Simplify VaultCore. #2714

clement-ux · 2025-12-09T17:14:29Z

Description

This pull request introduces significant refactoring and cleanup to the Vault interface and related contracts, with a focus on removing price provider and swapper logic, simplifying asset and strategy management, and updating mock contracts to match the new interfaces. These changes streamline the codebase, deprecate unused or redundant functionality, and improve clarity and maintainability.

Vault Interface Refactoring

Removed all price provider and swapper-related events and functions from the IVault interface, including setPriceProvider, priceProvider, swap logic, and associated configuration and events. Deprecated several functions and added new ones for strategy management and asset handling. [1] [2] [3] [4] [5] [6] [7]
Replaced asset-specific strategy functions with simplified versions: setDefaultStrategy/defaultStrategy instead of per-asset strategies, and removed asset configuration queries. [1] [2]

Removal of Price Provider Logic

Commented out or removed all uses of priceProvider in AbstractHarvester, MockEvilReentrantContract, and BridgedWOETHStrategy, replacing them with direct oracle references or dummy values for test compatibility. [1] [2] [3] [4] [5] [6] [7] [8]

Asset and Strategy Handling Updates

Updated asset support and strategy functions in mock vaults and related contracts to align with the new interface, including changes to asset tracking and initialization. [1] [2] [3] [4]
Modified minting logic in OETHPlumeVaultCore and MockRebornMinter to use the new asset and amount parameters and removed unnecessary minimum checks. [1] [2] [3]

Deprecation and Cleanup

Removed the unused OETHVault.sol contract and other deprecated logic, further simplifying the codebase and removing references to outdated functionality.

Constructor and Initialization Updates

Updated constructors in mock contracts and strategies to match the new interface requirements, including passing assets and oracles directly. [1] [2] [3]

These changes collectively modernize the Vault architecture, improve testability, and remove legacy code paths.

Code Change Checklist

To be completed before internal review begins:

The contract code is complete
Executable deployment file
Fork tests that test after the deployment file runs
Unit tests *if needed
The owner has done a full checklist review of the code + tests

Internal review:

Two approvals by internal reviewers

Deploy checklist

Two reviewers complete the following checklist:

- [ ] All deployed contracts are listed in the deploy PR's description
- [ ] Deployed contract's verified code (and all dependencies) match the code in master
- [ ] Contract constructors have correct arguments
- [ ] The transactions that interacted with the newly deployed contract match the deploy script.
- [ ] Governance proposal matches the deploy script
- [ ] Smoke tests pass after fork test execution of the governance proposal

…emove WETH dependencies

…ialization

…nctionality

…implify total value calculations

… and clean up comments in VaultCore

…ctions, simplifying the codebase.

…implify the codebase.

…cess control

… improve clarity by removing deprecated functions and updating asset handling methods.

…itialization by using address(0) for backing asset.

…move deprecated asset handling

…rategy handling for backing asset

…address, simplifying price retrieval and enhancing clarity

…nstructors for improved clarity and functionality

…rror messages

…ved clarity and consistency

…y by removing unused variables and consolidating test cases

…nd consolidating test cases

…d consolidating test cases

…ables and consolidating test cases

…ate mint and redeem amounts for consistency

…ategy setting

…fy asset handling

…et address, update test fixtures to use USDC instead of USDS, and simplify Dripper and VaultValueChecker tests by removing unused variables and consolidating logic.

sparrowDom

Posting intermittent review comments

contracts/contracts/harvest/AbstractHarvester.sol

contracts/contracts/interfaces/IVault.sol

sparrowDom · 2025-12-29T13:17:50Z

contracts/contracts/vault/VaultAdmin.sol

        );
-        require(_assets.length == _amounts.length, "Parameter length mismatch");
+        require(
+            _assets.length == 1 &&


What is the reason that assets and amounts remain arrays?

I created this doc where I'm not sure if we should/could get rid of some part of the code: https://www.notion.so/originprotocol/Can-we-get-rid-of-it-2c484d46f53c8024a9eec3b6260ef8f4?source=copy_link. But I'm in favor of simplification here.

Linking here comment from Nick: link.
I think we will let things like this.

contracts/contracts/vault/OETHVaultCore.sol

contracts/contracts/vault/OSonicVaultAdmin.sol

contracts/contracts/vault/OUSDVaultAdmin.sol

contracts/contracts/vault/OUSDVaultCore.sol

…e contract

naddison36

The main thing that needs fixing is the units for the async withdrawals

contracts/contracts/vault/VaultInitializer.sol

contracts/contracts/harvest/AbstractHarvester.sol

contracts/contracts/interfaces/IVault.sol

contracts/contracts/strategies/BridgedWOETHStrategy.sol

contracts/contracts/interfaces/IVault.sol

contracts/contracts/vault/VaultCore.sol

contracts/contracts/vault/VaultAdmin.sol

naddison36 · 2026-01-07T06:53:13Z

contracts/contracts/vault/VaultCore.sol

+     * There is a minimum of 10 minutes before a request can be claimed. After that, the request just needs
+     * enough backingAsset liquidity in the Vault to satisfy all the outstanding requests to that point in the queue.
+     * OToken is converted to backingAsset at 1:1.
+     * @param _amount Amount of OToken to burn.


We have the units mixed up where. It didn't matter for OETH as the oToken and WETH were both 18 decimals. For OUSD with 18 decimals and USDC only being 6 decimals, we need to be very clear on the units. The Natspec states the amount is in OTokens but the function scales it up like it is only 6 decimals.
I think we should rename _amount to _oTokenAmount so it's clear

I misread the scaleBy function. I see the async queue amounts are being scaled down so this should work.
I'm now concerned we are losing precision by scaling oToken amounts down to asset amounts. But it might be ok. I'll think more about it.

naddison36 · 2026-01-07T06:54:06Z

contracts/contracts/vault/VaultCore.sol

+        requestId = withdrawalQueueMetadata.nextWithdrawalIndex;
+        queued =
+            withdrawalQueueMetadata.queued +
+            _amount.scaleBy(backingAssetDecimals, 18);


I think the _amount should be oTokens and not assets so this should not be scaled up

I see now, the withdrawalQueueMetadata quantaties have been scaled down.

…utput in VaultCore contract

…rivate

…rage constructor

…and redeem functions

…asset' in contracts and tests

…unction

contracts/contracts/vault/VaultCore.sol

…n values

…and redeem functions

* refactor: remove deprecated asset structure and related mappings from VaultStorage * refactor: remove deprecated mint function parameters and related tests * refactor: remove unused parameters from _mint and mint functions * refactor: remove unnecessary blank lines in test files * refactor: change internal mappings to private for better encapsulation

clement-ux added 30 commits December 9, 2025 16:38

Refactor OETHVaultCore and VaultCore to simplify asset handling and r…

cf1e4c6

…emove WETH dependencies

Add constructors to MockVault and MockVaultCoreInstantRebase for init…

cdf5ee8

…ialization

Refactor OETHVaultCore and VaultCore to implement async withdrawal fu…

17ef9fb

…nctionality

Refactor OETHVaultCore and VaultCore to remove unused functions and s…

7c38699

…implify total value calculations

Remove unused addWithdrawalQueueLiquidity function from OETHVaultCore…

be6a697

… and clean up comments in VaultCore

Refactor OETHVaultCore and VaultCore to remove unused imports and fun…

f49fa28

…ctions, simplifying the codebase.

Remove unused pricing functions and related imports in VaultCore to s…

9d58bde

…implify the codebase.

Update constructor parameter in OETHVaultCore to use _weth for clarity

d480ca9

Add OUSDVaultCore contract and implement redeem functionality with ac…

311fa8b

…cess control

Refactor OETHVaultAdmin and VaultAdmin contracts to simplify code and…

aedd14f

… improve clarity by removing deprecated functions and updating asset handling methods.

Refactor vault admin contracts to remove constructors and simplify in…

4a2a265

…itialization by using address(0) for backing asset.

Refactor vault contracts to enforce backing asset restrictions and re…

87bf396

…move deprecated asset handling

Refactor VaultAdmin and VaultStorage contracts to simplify default st…

2c41717

…rategy handling for backing asset

Refactor AbstractHarvester and related contracts to integrate oracle …

f520e69

…address, simplifying price retrieval and enhancing clarity

Revert changes on Harvester

3dd4788

remove old code

16ab03b

Refactor vault admin contracts to include backing asset address in co…

3833da5

…nstructors for improved clarity and functionality

Refactor OETH Vault tests to simplify strategy handling and improve e…

0cd238a

…rror messages

adjust decimal logic in VaultStorage

2fce2d5

wip fix test

0d931d4

Refactor scaling logic in VaultCore to use scaleBy function for impro…

3d864d1

…ved clarity and consistency

Refactor Compound strategy tests to simplify logic and improve clarit…

b7b2445

…y by removing unused variables and consolidating test cases

Refactor Vault tests to simplify logic by removing unused variables a…

9705708

…nd consolidating test cases

Refactor Vault tests to simplify code by removing unused variables an…

ceb6f82

…d consolidating test cases

Refactor Vault redeem tests to simplify logic by removing unused vari…

e2bb9e0

…ables and consolidating test cases

fix buyback fixture

c7ce0f2

Refactor Reborner contract and tests to use USDC instead of USDS, upd…

f9fbe28

…ate mint and redeem amounts for consistency

Refactor compoundingStakingSSVStrategyFixture to simplify default str…

377f32e

…ategy setting

Refactor Convex strategy tests to use USDC instead of USDS and simpli…

33c7e9a

…fy asset handling

Refactor MockVaultCoreInstantRebase constructor to accept backing ass…

e336f3e

…et address, update test fixtures to use USDC instead of USDS, and simplify Dripper and VaultValueChecker tests by removing unused variables and consolidating logic.

clement-ux added 2 commits December 19, 2025 09:17

Add lot of test for async withdraw

e72adab

adjust comment

38a00fa

clement-ux marked this pull request as ready for review December 19, 2025 10:56

clement-ux requested review from naddison36, shahthepro and sparrowDom as code owners December 19, 2025 10:56

sparrowDom reviewed Dec 29, 2025

View reviewed changes

clement-ux added 6 commits January 2, 2026 12:21

refactor: remove setDripper function from IVault interface

b61e10e

fix: update redeem function to use correct amount parameter

09131ae

feat: add deprecated weth asset index to OETHVaultCore contract

39bbbfb

Merge branch 'master' into clement/simplify-ousd

1b40aca

increase ousd vault upgrade number

ebe97cd

fix: add slither disable comment for constable-states in OETHVaultCor…

94d39ed

…e contract

naddison36 requested changes Jan 7, 2026

View reviewed changes

clement-ux added 9 commits January 7, 2026 09:48

fix: correct drip duration initialization in VaultInitializer contract

780d20b

feat: deprecate calculateRedeemOutputs and introduce calculateRedeemO…

48c661d

…utput in VaultCore contract

refactor: rename backingAsset to asset across contracts for consistency

c21040a

refactor: change visibility of deprecated asset-related mappings to p…

0c4325a

…rivate

fix: improve error message for unsupported asset decimals in VaultSto…

746f517

…rage constructor

fix: update error message for unsupported asset in minting process

a3da207

refactor: update comments to clarify AMO strategy references in mint …

d792893

…and redeem functions

refactor: standardize terminology by replacing 'backing asset' with '…

5de1870

…asset' in contracts and tests

refactor: simplify redeem output calculation by removing deprecated f…

ac929c6

…unction

naddison36 reviewed Jan 7, 2026

View reviewed changes

contracts/contracts/vault/VaultCore.sol Outdated Show resolved Hide resolved

clement-ux and others added 6 commits January 7, 2026 10:49

refactor: streamline redeem output calculation by consolidating retur…

b7e25e2

…n values

refactor: improve clarity in comments regarding AMO strategy in mint …

0db0931

…and redeem functions

refactor: mark isSupportedAsset function as deprecated

d6e5f7d

Merge branch 'master' into clement/simplify-ousd

cf0d427

Merge branch 'master' into clement/simplify-ousd

d936abd

[Vault] Simplify VaultCore. #2714

Are you sure you want to change the base?

[Vault] Simplify VaultCore. #2714

Conversation

clement-ux commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Vault Interface Refactoring

Removal of Price Provider Logic

Asset and Strategy Handling Updates

Deprecation and Cleanup

Constructor and Initialization Updates

Code Change Checklist

Deploy checklist

Uh oh!

sparrowDom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sparrowDom Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

clement-ux Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

clement-ux Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

naddison36 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

naddison36 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

naddison36 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

naddison36 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

naddison36 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

clement-ux commented Dec 9, 2025 •

edited

Loading