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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
| /// Used by testing code to configure the DEX with swappers. | ||
| /// Overwrites existing swapper with same types, if any. | ||
| access(all) fun _addSwapper(swapper: Swapper) { | ||
| if self.swappers[swapper.inType()] == nil { | ||
| self.swappers[swapper.inType()] = { swapper.outType(): swapper } | ||
| } else { | ||
| let swappersForInType = self.swappers[swapper.inType()]! | ||
| swappersForInType[swapper.outType()] = swapper | ||
| self.swappers[swapper.inType()] = swappersForInType | ||
| } | ||
| } |
There was a problem hiding this comment.
I want a better distinction between adding a new mock swapper and updating the price by overwriting an existing mock swapper. The tests currently call addMockDexSwapper to update the DEX price, after which the original "1:1" price mock swapper is no longer available - I think this should be made more clear.
To that end: Can we split this into two functions - one for exclusively adding a swapper and one for updating it? (Both with appropriate preconditions.)
If we want to keep the existing function, I think we should update the name and documentation to clarify that this overwrites existing swapper for the same input/output token pair, e.g. rename to "_addOrUpdateSwapper".
There was a problem hiding this comment.
Updated naming and documentation to clarify this in 5fc56c6. The function used within tests is now called setMockDexPriceForPair.
| access(all) | ||
| fun addMockDexSwapper( | ||
| signer: Test.TestAccount, | ||
| inVaultIdentifier: String, | ||
| outVaultIdentifier: String, | ||
| vaultSourceStoragePath: StoragePath, | ||
| priceRatio: UFix64 | ||
| ) { | ||
| let addRes = _executeTransaction( | ||
| "./transactions/mock-dex-swapper/add_swapper.cdc", | ||
| [inVaultIdentifier, outVaultIdentifier, vaultSourceStoragePath, priceRatio], | ||
| signer | ||
| ) | ||
| Test.expect(addRes, Test.beSucceeded()) | ||
| } |
There was a problem hiding this comment.
Same concern here, I want a better distinction between adding a new mock swapper and updating the price by overwriting an existing mock swapper, or at least documentation that either can happen
joshuahannan
left a comment
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).
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 <[email protected]>
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 <[email protected]>
| 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 <[email protected]>
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.