Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2800 +/- ##
==========================================
- Coverage 45.06% 44.92% -0.15%
==========================================
Files 105 107 +2
Lines 4500 4523 +23
Branches 1228 1235 +7
==========================================
+ Hits 2028 2032 +4
- Misses 2469 2488 +19
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Sonic SwapXAMO strategy into a generalized Algebra-based AMO strategy and introduces a new Supernova AMO strategy for OETH on mainnet. The refactoring enables code reuse across different Algebra-style DEX pools while making the strategy token-order agnostic.
Changes:
- Extracted a generalized
StableSwapAMMStrategybase contract that works with any Algebra-style stable swap pool - Refactored
SonicSwapXAMOStrategyto inherit from the base contract, removing ~850 lines of duplicated code - Added new
OETHSupernovaAMOStrategyfor OETH/WETH pool on mainnet - Created comprehensive behavior-driven tests that can be reused across different Algebra AMO implementations
- Updated deployment scripts and fixtures to support the new strategy architecture
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
contracts/contracts/strategies/algebra/StableSwapAMMStrategy.sol |
New generalized base strategy for Algebra stable pools with token-order-agnostic handling |
contracts/contracts/strategies/sonic/SonicSwapXAMOStrategy.sol |
Refactored to inherit from base, reducing from ~850 to ~5 lines |
contracts/contracts/strategies/algebra/OETHSupernovaAMOStrategy.sol |
New OETH Supernova AMO strategy implementation |
contracts/test/behaviour/algebraAmoStrategy.js |
Comprehensive reusable behavior tests for Algebra AMO strategies |
contracts/test/strategies/sonic/swapx-amo.sonic.fork-test.js |
Refactored to use behavior tests |
contracts/test/strategies/oeth-supernova-amo.mainnet.fork-test.js |
New tests for OETH Supernova strategy |
contracts/deploy/mainnet/178_supernova_AMO.js |
Deployment script for mainnet Supernova AMO |
contracts/deploy/deployActions.js |
Helper functions for pool/gauge creation and strategy deployment |
contracts/test/_fixture.js |
New fixture for OETH Supernova AMO testing |
contracts/test/_fixture-sonic.js |
Enhanced fixture with auto-allocate handling |
contracts/contracts/interfaces/algebra/* |
New interfaces for Algebra pair and gauge contracts |
contracts/utils/addresses.js |
Added Supernova protocol addresses |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@naddison36 I've opened a new pull request, #2810, to work on those changes. Once the pull request is ready, I'll request review from you. |
naddison36
left a comment
There was a problem hiding this comment.
I have a separate PR with some changes #2811
contracts/contracts/strategies/algebra/StableSwapAMMStrategy.sol
Outdated
Show resolved
Hide resolved
* Removed asset and oToken from StableSwapAMMStrategy constructor Added oToken to IVault which is used by StableSwapAMMStrategy constructor Removed calculateRedeemOutput and calculateRedeemOutputs from IVault * Added contract diagrams for OETHSupernovaAMOStrategy * Fix SwapX AMO fork tests * Fix SwapX AMO fork tests * More rounding issues with Algebra fork tests
|
@copilot can you do another pass? |
naddison36
left a comment
There was a problem hiding this comment.
We need to decide if we combined the 172_oeth_vault_upgrade and 179_supernova_AMO deploy scripts.
OETHSupernovaAMOStrategy calls oToken and asset on the OETHVault so I'm happy to combine these into the one governance proposal.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
contracts/contracts/strategies/algebra/StableSwapAMMStrategy.sol
Outdated
Show resolved
Hide resolved
|
Due to Supernova requiring the upgraded vault to be deployed, for this deploy WETH and OETH values have been hardcoded: 6bde7d3 |
contracts/test/_fixture.js
Outdated
| return fixture; | ||
| } | ||
|
|
||
| async function supernovaOETHAMOFixure( |
There was a problem hiding this comment.
typo: supernovaOETHAMOFixture
Supernova AMO
TODO:
StableSwapAMMStrategy.solafter the deployWhat was done
SwapXAMOstrategy into a generalized Algebra-based strategy.Test fixes and updates
Why this change
SwapXAMOimplementation was too specialized and was expecting ws / OS tokens in specific order. Also the strategy was named SwapX even though the pool engine underneath was AlgebraWhat isn't done
Code review suggestions
A couple of files have been moved still they should be compared with their old versions:
StableSwapAMMStrategy.solwith theSonicSwapXAMOStrategy.solexample code-diff as of 03e81d9 commit : https://www.diffchecker.com/MdOvAfVf/algebraAmoStrategy.jswith theswapx-amo.sonic.fork-test.jsonline diff tools do a terrible job heresol2uml diff 0x6509f770B83856Ac51613AEe73D1E7bFaD032784 -bn sonic 0xcfE67b6c7B65c8d038e666b3241a161888B7f2b0