Integrate DEX into manual liquidation#112
Conversation
(no errors in IDE, but tests failing (can't find FCM in this scope) on this commit)
state is stored in smart contract, struct with reference to contract acts as mock object from FCM's perspective
Un-comment the DEX price validation logic to ensure liquidators provide competitive prices and prevent oracle manipulation. Add test infrastructure with MockDexSwapper transaction and helpers to configure DEX prices that stay synchronized with oracle prices during liquidation tests. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implement 5 test functions covering DEX pricing validation logic: - testManualLiquidation_dexOraclePriceDivergence: Tests price divergence thresholds with 3 sub-cases (within/exceeds/above oracle) - testManualLiquidation_increaseHealthBelowTarget: Tests partial liquidation improving health without reaching target - testManualLiquidation_liquidateToTarget: Tests liquidation bringing health to exactly 1.05 target - testManualLiquidation_liquidatorOfferWorseThanDex: Tests rejection when liquidator offers worse price than DEX - testManualLiquidation_combinedEdgeCase: Tests divergence check precedence over competitive offers All 18 tests in liquidation_phase1_test.cdc now pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Refactor testManualLiquidation_dexOraclePriceDivergence into: - testManualLiquidation_dexOraclePriceDivergence_withinThreshold: Tests successful liquidation when divergence is within 3% threshold - testManualLiquidation_dexOraclePriceDivergence_exceedsThreshold: Tests two failure cases when divergence exceeds threshold (DEX below and above oracle) This improves test clarity by separating success and failure scenarios into distinct test functions. All 19 tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Refactor testManualLiquidation_dexOraclePriceDivergence_exceedsThreshold into: - testManualLiquidation_dexOraclePriceDivergence_dexBelowOracle: Tests failure when DEX price is below oracle (6.06% divergence) - testManualLiquidation_dexOraclePriceDivergence_dexAboveOracle: Tests failure when DEX price is above oracle (5.71% divergence) This provides better test isolation with one assertion per test function. All 20 tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
joshuahannan
left a comment
There was a problem hiding this comment.
I'm a little confused about the upgrade plans here
| /// | ||
| /// FCM does not attempt to construct multi-part paths (using multiple Swappers) or compare prices across Swappers. | ||
| /// It relies directly on the Swapper's returned by the configured SwapperProvider. | ||
| access(self) let dex: {DeFiActions.SwapperProvider} |
There was a problem hiding this comment.
We can't add this in an upgrade, right? Or are we redeploying?
There was a problem hiding this comment.
You're right -- we will need to redeploy to add this field.
I've been operating under the assumption that we may not redeploy, but obviously this change will make redeployment necessary (we also separately decided this morning to proceed under the assumption that we will do a redeployment). I think we can leave this field as is, and I will be going through the fields marked deprecated in previous PRs to remove them prior to redeployment (since we can safely do that now).
There was a problem hiding this comment.
yep, we'll be redeploying
there are already non-upgradable changes in this contract
Renamed functions and updated documentation to make it clear that setting a mock DEX swapper overwrites any existing swapper for the same token pair. This is primarily used in tests to set or update prices. Changes: - Renamed MockDexSwapper._addSwapper() to setMockDEXSwapperForPair() - Renamed test helper addMockDexSwapper() to setMockDexPriceForPair() - Renamed transaction file to set_mock_dex_price_for_pair.cdc - Updated all test usages in liquidation_phase1_test.cdc - Added documentation explaining overwrite behavior and intended use case Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Resolved merge conflicts in liquidation_phase1_test.cdc by accepting DEX integration changes and updating identifier variables to use global CAPS constants (FLOW_TOKEN_IDENTIFIER, MOET_TOKEN_IDENTIFIER, PROTOCOL_ACCOUNT). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
| let swappersForInType = self.swappers[inType]! | ||
| swappersForInType[outType] = swapper | ||
| self.swappers[inType] = swappersForInType |
There was a problem hiding this comment.
This is just a mock contract so it doesn't matter, but in production code we should consider optimizing such code, i.e. avoiding copying and updating existing (potentially large) datasets by using in-place mutation (get a reference and insert)
Co-authored-by: Bastian Müller <bastian@turbolent.com>
| /// @param swapper: The swapper to set for the token pair | ||
| access(all) fun setMockDEXSwapperForPair(swapper: Swapper) { |
There was a problem hiding this comment.
do we also want to implement deleting a swapper in case we want to remove unwanted swappers?
| /// TODO: unused! To remove, must re-deploy existing contracts | ||
| access(self) var allowedSwapperTypes: {Type: Bool} |
There was a problem hiding this comment.
it's safe to remove it, we'll be renaming the contract and deploying it with a new name
| /// TODO: unused! To remove, must re-deploy existing contracts | ||
| access(self) var allowedSwapperTypes: {Type: Bool} | ||
|
|
||
| /// A trusted DEX (or set of DEXes) used by FCM as a pricing oracle and trading counterparty for liquidations. |
There was a problem hiding this comment.
nit: use full contract name (FlowCreditMarket) instead of FCM
it'll be easier to find and replace it later
| /// TODO: unused! To remove, must re-deploy existing contracts | ||
| access(self) var dexMaxSlippageBps: UInt64 | ||
|
|
||
| /// Max route hops allowed for DEX liquidations | ||
| // TODO(jord): unused | ||
| /// TODO: unused! To remove, must re-deploy existing contracts | ||
| access(self) var dexMaxRouteHops: UInt64 |
There was a problem hiding this comment.
There are a few other unused fields to remove too and non-trivial downstream changes from removing them -- going to handle that separately in #137
This reverts commit f8f69ba.
Closes: #94
Extends #77 (review that PR first)
Description
This PR implements integration with a generic DEX (
SwapperProvider), as a source of pricing comparison, for the purposes of manual liquidation. The DEX price is used in two ways:Policy Decisions
SwapperProvider(DEX) must always be provided at construction time of thePool(field is non-optional)SwapperProvidermust provide aSwapperfor all possible supported token pairs. If it does not, liquidation will fail.