Dev - #77
Merged
Merged
Conversation
Reviewer's GuideThis PR fully removes encrypted-vault (FHE) support across the protocol, simplifying the internal state orchestrator to handle only transparent vaults, updating configuration and interfaces accordingly, and cleaning up related utilities, tests, CI workflow and artifacts. ER diagram for updated vault relationships (only transparent vaults)erDiagram
ORION_CONFIG ||--o| TRANSPARENT_VAULT_FACTORY : manages
TRANSPARENT_VAULT_FACTORY ||--o| ORION_TRANSPARENT_VAULT : creates
ORION_CONFIG ||--o| ORION_TRANSPARENT_VAULT : tracks
ORION_CONFIG {
address transparentVaultFactory
}
TRANSPARENT_VAULT_FACTORY {
dummy_attribute string
}
ORION_TRANSPARENT_VAULT {
dummy_attribute string
}
Class diagram for updated InternalStatesOrchestrator (removal of encrypted vault support)classDiagram
class InternalStatesOrchestrator {
+address automationRegistry
+ILiquidityOrchestrator liquidityOrchestrator
+IOrionConfig config
+IPriceAdapterRegistry registry
+uint8 transparentMinibatchSize
+InternalUpkeepPhase currentPhase
+uint8 currentMinibatchIndex
+address[] transparentVaultsEpoch
+uint256 bufferAmount
+function updateMinibatchSize(uint8)
+function checkUpkeep(bytes)
+function performUpkeep(bytes)
+function getVaultTotalAssetsForFulfillDeposit(address)
+function getVaultTotalAssetsForFulfillRedeem(address)
}
class InternalUpkeepPhase {
Idle
PreprocessingTransparentVaults
Buffering
PostprocessingTransparentVaults
BuildingOrders
}
InternalStatesOrchestrator --> InternalUpkeepPhase
InternalStatesOrchestrator --> ILiquidityOrchestrator
InternalStatesOrchestrator --> IOrionConfig
InternalStatesOrchestrator --> IPriceAdapterRegistry
Class diagram for updated OrionConfig (removal of encrypted vault factory)classDiagram
class OrionConfig {
+address liquidityOrchestrator
+address transparentVaultFactory
+address priceAdapterRegistry
+function setVaultFactory(address)
+function getAllOrionVaults(VaultType)
}
class VaultType {
Transparent
}
OrionConfig --> VaultType
Class diagram for updated IInternalStateOrchestrator interface (removal of encrypted vault methods)classDiagram
class IInternalStateOrchestrator {
+updateMinibatchSize(uint8)
+getVaultTotalAssetsForFulfillDeposit(address)
+getVaultTotalAssetsForFulfillRedeem(address)
}
class InternalUpkeepPhase {
Idle
PreprocessingTransparentVaults
Buffering
PostprocessingTransparentVaults
BuildingOrders
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis PR removes encrypted-vault functionality and related FHE components across contracts, tests, and artifacts; it simplifies config from two vault factories to one; it updates orchestrators to remove encrypted phases and callbacks; and it adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as UpkeepScheduler
participant Orchestrator as InternalStatesOrchestrator
participant VaultT as TransparentVault
participant Liquidity as LiquidityOrchestrator
note over Scheduler,Orchestrator: Old flow (with encrypted vaults)
Scheduler->>Orchestrator: trigger checkUpkeep()
Orchestrator->>VaultT: PreprocessingTransparentVaults
Orchestrator->>VaultT: PreprocessingEncryptedVaults
Orchestrator->>Orchestrator: Buffering
Orchestrator->>Orchestrator: ProcessingDecryptedValues
Orchestrator->>Orchestrator: PostprocessingEncryptedVaults
Orchestrator->>Liquidity: BuildingOrders
note right of Orchestrator: multiple encrypted-related phases & callbacks
sequenceDiagram
participant Scheduler as UpkeepScheduler
participant Orchestrator as InternalStatesOrchestrator
participant VaultT as TransparentVault
participant Liquidity as LiquidityOrchestrator
note over Scheduler,Orchestrator: New flow (encrypted vaults removed)
Scheduler->>Orchestrator: trigger checkUpkeep()
Orchestrator->>VaultT: PreprocessingTransparentVaults
Orchestrator->>Orchestrator: Buffering
Orchestrator->>Orchestrator: BuildingOrders
Orchestrator->>Liquidity: fulfill / publish orders
note right of Orchestrator: No encrypted phases or decryption callbacks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
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 |
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Shrinking the InternalUpkeepPhase enum by removing encrypted phases reindexes all downstream phase values—consider reserving those slots or explicitly versioning the contract to avoid breaking existing automation scripts that rely on the previous enum ordering.
- You’ve removed the encrypted‐vault loop in LiquidityOrchestrator but still reference EventsLib.VaultType.Encrypted in config.getAllOrionVaults tests—make sure no residual encrypted vault logic or enum references remain to prevent dead code or unexpected behavior.
- After switching to a single vault factory in OrionConfig, confirm that the onlyFactories modifier and any on-chain permission checks no longer expect an encryptedVaultFactory address so you don’t unintentionally lock out valid calls.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Shrinking the InternalUpkeepPhase enum by removing encrypted phases reindexes all downstream phase values—consider reserving those slots or explicitly versioning the contract to avoid breaking existing automation scripts that rely on the previous enum ordering.
- You’ve removed the encrypted‐vault loop in LiquidityOrchestrator but still reference EventsLib.VaultType.Encrypted in config.getAllOrionVaults tests—make sure no residual encrypted vault logic or enum references remain to prevent dead code or unexpected behavior.
- After switching to a single vault factory in OrionConfig, confirm that the onlyFactories modifier and any on-chain permission checks no longer expect an encryptedVaultFactory address so you don’t unintentionally lock out valid calls.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/OrchestratorsZeroState.test.ts (1)
98-105: First upkeep shouldn’t leave you in IdleWith a vault registered, the first
performUpkeepcall transitions the orchestrator fromIdletoPreprocessingTransparentVaults(phase1). ExpectingcurrentPhase()to remain0makes this test fail against the actual state machine. Update the assertion toexpect(...).to.equal(1)(and adjust comments if needed).
🧹 Nitpick comments (1)
contracts/OrionConfig.sol (1)
199-226: Consider removing encrypted vault handling.The
addOrionVaultandremoveOrionVaultfunctions still contain branching logic forVaultType.Encrypted, even though theonlyFactoriesmodifier (line 62) now only authorizestransparentVaultFactory. This encrypted vault handling appears to be unreachable code.If encrypted vault support is being removed permanently, consider simplifying these functions to only handle transparent vaults. If this is intentional for future extensibility or backward compatibility with already-registered vaults, please document this decision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.github/workflows/ci.yml(1 hunks)artifacts/contracts/OrionConfig.sol/OrionConfig.json(2 hunks)artifacts/contracts/execution/OrionAssetERC4626ExecutionAdapter.sol/OrionAssetERC4626ExecutionAdapter.json(1 hunks)artifacts/contracts/factories/EncryptedVaultFactory.sol/EncryptedVaultFactory.json(0 hunks)artifacts/contracts/interfaces/IInternalStateOrchestrator.sol/IInternalStateOrchestrator.json(1 hunks)artifacts/contracts/interfaces/IOrionConfig.sol/IOrionConfig.json(1 hunks)artifacts/contracts/interfaces/IOrionEncryptedVault.sol/IOrionEncryptedVault.json(0 hunks)artifacts/contracts/libraries/UtilitiesLib.sol/UtilitiesLib.json(1 hunks)artifacts/contracts/orchestrators/LiquidityOrchestrator.sol/LiquidityOrchestrator.json(1 hunks)artifacts/contracts/price/OrionAssetERC4626PriceAdapter.sol/OrionAssetERC4626PriceAdapter.json(1 hunks)artifacts/contracts/price/PriceAdapterRegistry.sol/PriceAdapterRegistry.json(1 hunks)artifacts/contracts/strategies/KBestTvlWeightedAverage.sol/KBestTvlWeightedAverage.json(1 hunks)contracts/OrionConfig.sol(2 hunks)contracts/factories/EncryptedVaultFactory.sol(0 hunks)contracts/interfaces/IInternalStateOrchestrator.sol(1 hunks)contracts/interfaces/IOrionConfig.sol(1 hunks)contracts/interfaces/IOrionEncryptedVault.sol(0 hunks)contracts/libraries/UtilitiesLib.sol(0 hunks)contracts/orchestrators/InternalStatesOrchestrator.sol(6 hunks)contracts/orchestrators/LiquidityOrchestrator.sol(0 hunks)contracts/vaults/OrionEncryptedVault.sol(0 hunks)test/EncryptedVault.test.ts(0 hunks)test/Orchestrators.test.ts(11 hunks)test/OrchestratorsZeroState.test.ts(1 hunks)test/OrionConfigVault.test.ts(10 hunks)test/PassiveCuratorStrategy.test.ts(2 hunks)test/TransparentVault.test.ts(1 hunks)
💤 Files with no reviewable changes (8)
- contracts/libraries/UtilitiesLib.sol
- contracts/orchestrators/LiquidityOrchestrator.sol
- contracts/interfaces/IOrionEncryptedVault.sol
- artifacts/contracts/factories/EncryptedVaultFactory.sol/EncryptedVaultFactory.json
- contracts/factories/EncryptedVaultFactory.sol
- artifacts/contracts/interfaces/IOrionEncryptedVault.sol/IOrionEncryptedVault.json
- contracts/vaults/OrionEncryptedVault.sol
- test/EncryptedVault.test.ts
🧰 Additional context used
🧬 Code graph analysis (6)
artifacts/contracts/OrionConfig.sol/OrionConfig.json (1)
test/EncryptedVault.test.ts (3)
tx(538-562)tx(162-201)beforeEach(410-468)
test/PassiveCuratorStrategy.test.ts (1)
test/EncryptedVault.test.ts (4)
tx(538-562)tx(130-160)tx(162-201)it(129-202)
contracts/OrionConfig.sol (1)
test/EncryptedVault.test.ts (2)
tx(538-562)tx(162-201)
test/OrchestratorsZeroState.test.ts (1)
test/EncryptedVault.test.ts (3)
tx(538-562)tx(162-201)tx(471-534)
artifacts/contracts/price/OrionAssetERC4626PriceAdapter.sol/OrionAssetERC4626PriceAdapter.json (1)
test/Adapters.test.ts (2)
orionConfig(7-53)owner(15-42)
test/Orchestrators.test.ts (1)
test/EncryptedVault.test.ts (3)
tx(471-534)tx(538-562)it(470-535)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Lint and Test
- GitHub Check: Sourcery review
🔇 Additional comments (9)
.github/workflows/ci.yml (1)
5-7: LGTM!The addition of the
fhevmbranch to CI triggers is appropriate for testing the simplified single-vault architecture across both main and fhevm development branches.artifacts/contracts/strategies/KBestTvlWeightedAverage.sol/KBestTvlWeightedAverage.json (1)
196-197: LGTM!Bytecode update reflects the underlying contract refactoring. The ABI remains unchanged, ensuring no breaking changes to the public interface.
artifacts/contracts/price/PriceAdapterRegistry.sol/PriceAdapterRegistry.json (1)
226-227: LGTM!Bytecode update is consistent with the broader refactoring. The unchanged ABI confirms no breaking changes to the public interface.
artifacts/contracts/OrionConfig.sol/OrionConfig.json (1)
523-523: LGTM!The artifact correctly reflects the API change from
setVaultFactoriestosetVaultFactory, simplifying the vault factory configuration to a single transparent factory. Bytecode updates are consistent with this refactoring.Also applies to: 600-601
contracts/interfaces/IOrionConfig.sol (1)
55-58: LGTM!The interface correctly reflects the simplified vault factory API, removing the encrypted factory parameter. Documentation clearly describes the single transparent factory parameter.
contracts/OrionConfig.sol (2)
62-62: LGTM!The
onlyFactoriesmodifier correctly restricts access to only the transparent vault factory, aligning with the removal of encrypted vault support.
106-111: LGTM!The simplified
setVaultFactoryimplementation correctly handles the single transparent factory configuration. The validation logic appropriately prevents zero addresses and duplicate registrations.artifacts/contracts/orchestrators/LiquidityOrchestrator.sol/LiquidityOrchestrator.json (1)
672-673: LGTM!Bytecode update reflects the removal of encrypted vault processing from the orchestrator flows. The unchanged ABI maintains interface stability.
test/TransparentVault.test.ts (1)
112-112: LGTM!Test correctly updated to use the new single-parameter
setVaultFactoryAPI, properly configuring the transparent vault factory for test scenarios.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Nov 16, 2025
Merged
Merged
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Remove encrypted vault support and FHE-based logic across the protocol, simplifying orchestrator, config, and liquidity components to operate exclusively with transparent vaults
Enhancements:
Build:
Tests:
Summary by CodeRabbit
Chores
Bug Fixes / Removals