-
Notifications
You must be signed in to change notification settings - Fork 0
close position from strategies #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 19 commits
44854d6
d4c07bb
87f1fe4
94e6013
79f79e4
1e6acc3
21c6c5a
190e1c3
8e3c733
341461d
d154cea
226c86a
6a6b85c
934319a
1c61628
41b24de
c26627c
a92ce68
884e899
3452d17
a763eac
c5d2c55
cbfbd04
4d7b359
e2787e0
ca91632
9b105b1
27edd22
268bbaa
2d8788b
6a2915e
9cf0944
49b0a17
f88fff4
f74da70
068457d
bfc1fc0
2dddb68
8ea43dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ on: | |
| pull_request: | ||
| branches: | ||
| - main | ||
| - nialexsan/pre-refactor | ||
|
|
||
| jobs: | ||
| tests: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ on: | |
| pull_request: | ||
| branches: | ||
| - main | ||
| - nialexsan/pre-refactor | ||
|
|
||
| jobs: | ||
| e2e-tests: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -42,7 +42,7 @@ access(all) contract FlowYieldVaultsStrategiesV2 { | |||||
| access(all) let univ3RouterEVMAddress: EVM.EVMAddress | ||||||
| access(all) let univ3QuoterEVMAddress: EVM.EVMAddress | ||||||
|
|
||||||
| access(all) let config: {String: AnyStruct} | ||||||
| access(all) let config: {String: AnyStruct} | ||||||
nialexsan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| /// Canonical StoragePath where the StrategyComposerIssuer should be stored | ||||||
| access(all) let IssuerStoragePath: StoragePath | ||||||
|
|
@@ -81,7 +81,15 @@ access(all) contract FlowYieldVaultsStrategiesV2 { | |||||
| access(self) var sink: {DeFiActions.Sink} | ||||||
| access(self) var source: {DeFiActions.Source} | ||||||
|
|
||||||
| init(id: DeFiActions.UniqueIdentifier, collateralType: Type, position: @FlowALPv0.Position) { | ||||||
| /// @TODO on the next iteration store yieldToMoetSwapper in the resource | ||||||
| /// Swapper used to convert yield tokens back to MOET for debt repayment | ||||||
| //access(self) let yieldToMoetSwapper: {DeFiActions.Swapper} | ||||||
|
|
||||||
| init( | ||||||
| id: DeFiActions.UniqueIdentifier, | ||||||
| collateralType: Type, | ||||||
| position: @FlowALPv0.Position | ||||||
| ) { | ||||||
| self.uniqueID = id | ||||||
| self.sink = position.createSink(type: collateralType) | ||||||
| self.source = position.createSourceWithOptions(type: collateralType, pullFromTopUpSource: true) | ||||||
|
|
@@ -110,6 +118,104 @@ access(all) contract FlowYieldVaultsStrategiesV2 { | |||||
| } | ||||||
| return <- self.source.withdrawAvailable(maxAmount: maxAmount) | ||||||
| } | ||||||
| /// Closes the underlying FlowALP position by preparing repayment funds and closing with them. | ||||||
| /// | ||||||
| /// This method: | ||||||
| /// 1. Calculates debt amount from position | ||||||
| /// 2. Creates external yield token source from AutoBalancer | ||||||
| /// 3. Swaps yield tokens → MOET via stored swapper | ||||||
| /// 4. Closes position with prepared MOET vault | ||||||
| /// | ||||||
| /// This approach eliminates circular dependencies by preparing all funds externally | ||||||
| /// before calling the position's close method. | ||||||
| /// | ||||||
| access(FungibleToken.Withdraw) fun closePosition(collateralType: Type): @{FungibleToken.Vault} { | ||||||
| pre { | ||||||
| self.isSupportedCollateralType(collateralType): | ||||||
| "Unsupported collateral type \(collateralType.identifier)" | ||||||
| } | ||||||
|
|
||||||
| // Step 1: Get debt amount from position using helper | ||||||
| let debtInfos = self.position.getTotalDebt() | ||||||
|
|
||||||
| // Step 2: Calculate total debt amount across all debt types | ||||||
| var totalDebtAmount: UFix64 = 0.0 | ||||||
| for debtInfo in debtInfos { | ||||||
| totalDebtAmount = totalDebtAmount + debtInfo.amount | ||||||
| } | ||||||
|
|
||||||
| // Add a tiny buffer to ensure we overpay slightly and flip from Debit to Credit | ||||||
| // This works around FlowALPv0's recordDeposit logic where exact repayment keeps direction as Debit | ||||||
| let repaymentBuffer: UFix64 = 0.00000001 // 1e-8 | ||||||
| totalDebtAmount = totalDebtAmount + repaymentBuffer | ||||||
|
|
||||||
| // Step 3: If no debt, pass empty vault array | ||||||
| if totalDebtAmount == 0.0 { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the
Suggested change
or create a new variable, and then we can check if the original debt amount without buffer is 0
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the close position logic should always overpay debt slightly
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. Look at this logic,
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems to be an artifact from one of the previous versions, there is no repayment buffer any more |
||||||
| let emptyVaults: @[{FungibleToken.Vault}] <- [] | ||||||
| let resultVaults <- self.position.closePosition( | ||||||
| repaymentVaults: <-emptyVaults | ||||||
| ) | ||||||
| // Extract the first vault (should be collateral) | ||||||
| assert(resultVaults.length > 0, message: "No vaults returned from closePosition") | ||||||
|
||||||
| let collateralVault <- resultVaults.removeFirst() | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we ever have more than one collateral type in the future, we will destroy it here. I'd add an assertion that we got exactly one collateral vault back (if we're certain that is the only allowed initial state), or handle multiple collateral vaults being returned.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this strategy will work only with one collateral and one debt type at a time, I added assertions |
||||||
| destroy resultVaults | ||||||
| return <- collateralVault | ||||||
| } | ||||||
|
|
||||||
| // Step 4: Create external yield token source from AutoBalancer | ||||||
| let yieldTokenSource = FlowYieldVaultsAutoBalancers.createExternalSource(id: self.id()!) | ||||||
| ?? panic("Could not create external source from AutoBalancer") | ||||||
|
|
||||||
| // Step 5: Retrieve yield→MOET swapper from contract config | ||||||
| let swapperKey = "yieldToMoetSwapper_".concat(self.id()!.toString()) | ||||||
|
||||||
| let yieldToMoetSwapper = FlowYieldVaultsStrategiesV2.config[swapperKey] as! {DeFiActions.Swapper}? | ||||||
| ?? panic("No yield→MOET swapper found for strategy \(self.id()!)") | ||||||
|
|
||||||
| // Step 6: Use quoteIn to calculate exact yield token input needed for desired MOET output | ||||||
| // This bypasses SwapSource's branch selection issue where minimumAvailable | ||||||
| // underestimates due to RoundDown in quoteOut, causing insufficient output | ||||||
| // quoteIn rounds UP the input to guarantee exact output delivery | ||||||
| let quote = yieldToMoetSwapper.quoteIn(forDesired: totalDebtAmount, reverse: false) | ||||||
|
|
||||||
| // Step 7: Withdraw the calculated yield token amount | ||||||
| let yieldTokenVault <- yieldTokenSource.withdrawAvailable(maxAmount: quote.inAmount) | ||||||
|
|
||||||
| // Step 8: Swap with quote to get exact MOET output | ||||||
| // Swap honors the quote and delivers exactly totalDebtAmount | ||||||
| let moetVault <- yieldToMoetSwapper.swap(quote: quote, inVault: <-yieldTokenVault) | ||||||
|
|
||||||
| // Step 9: Close position with prepared MOET vault | ||||||
| let repaymentVaults: @[{FungibleToken.Vault}] <- [<-moetVault] | ||||||
| let resultVaults <- self.position.closePosition( | ||||||
| repaymentVaults: <-repaymentVaults | ||||||
| ) | ||||||
|
|
||||||
| // Extract all returned vaults | ||||||
| assert(resultVaults.length > 0, message: "No vaults returned from closePosition") | ||||||
|
|
||||||
| // First vault should be collateral | ||||||
| var collateralVault <- resultVaults.removeFirst() | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better double check the collateral type?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. put a post check |
||||||
|
|
||||||
| // Handle any overpayment dust (MOET) by swapping back to collateral | ||||||
| while resultVaults.length > 0 { | ||||||
| let dustVault <- resultVaults.removeFirst() | ||||||
| if dustVault.balance > 0.0 && dustVault.getType() != collateralType { | ||||||
| // Swap overpayment back to collateral using configured swapper | ||||||
| let dustToCollateralSwapper = FlowYieldVaultsStrategiesV2.config["moetToCollateralSwapper_".concat(self.id()!.toString())] as! {DeFiActions.Swapper}? | ||||||
|
||||||
| ?? panic("No MOET→collateral swapper found for strategy \(self.id()!)") | ||||||
| let swappedCollateral <- dustToCollateralSwapper.swap( | ||||||
| quote: nil, | ||||||
| inVault: <-dustVault | ||||||
| ) | ||||||
| collateralVault.deposit(from: <-swappedCollateral) | ||||||
| } else { | ||||||
| destroy dustVault | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note we will destroy a non-empty collateral vault regardless the amount. This could is techinically possible since the returned vaults can have pending deposits, and overpay vaults. I think we should do: |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| destroy resultVaults | ||||||
| return <- collateralVault | ||||||
| } | ||||||
| /// Executed when a Strategy is burned, cleaning up the Strategy's stored AutoBalancer | ||||||
| access(contract) fun burnCallback() { | ||||||
| FlowYieldVaultsAutoBalancers._cleanupAutoBalancer(id: self.id()!) | ||||||
|
|
@@ -301,9 +407,43 @@ access(all) contract FlowYieldVaultsStrategiesV2 { | |||||
| uniqueID: uniqueID | ||||||
| ) | ||||||
|
|
||||||
| // Create Position source with CONSERVATIVE settings | ||||||
|
||||||
| // pullFromTopUpSource: false ensures Position maintains health buffer | ||||||
| // This prevents Position from being pushed to minHealth (1.1) limit | ||||||
| let positionSource = position.createSourceWithOptions( | ||||||
| type: collateralType, | ||||||
| pullFromTopUpSource: false // ← CONSERVATIVE: maintain safety buffer | ||||||
| ) | ||||||
|
|
||||||
| // Create Collateral -> Yield swapper (reverse of yieldToCollateralSwapper) | ||||||
| // Allows AutoBalancer to pull collateral, swap to yield token | ||||||
| let collateralToYieldSwapper = self._createCollateralToYieldSwapper( | ||||||
| collateralConfig: collateralConfig, | ||||||
| yieldTokenEVMAddress: tokens.yieldTokenEVMAddress, | ||||||
| yieldTokenType: tokens.yieldTokenType, | ||||||
| collateralType: collateralType, | ||||||
| uniqueID: uniqueID | ||||||
| ) | ||||||
|
|
||||||
| // Create Position swap source for AutoBalancer deficit recovery | ||||||
| // When AutoBalancer value drops below deposits, pulls collateral from Position | ||||||
| let positionSwapSource = SwapConnectors.SwapSource( | ||||||
| swapper: collateralToYieldSwapper, | ||||||
| source: positionSource, | ||||||
| uniqueID: uniqueID | ||||||
| ) | ||||||
|
|
||||||
| // Set AutoBalancer sink for overflow -> recollateralize | ||||||
| balancerIO.autoBalancer.setSink(positionSwapSink, updateSinkID: true) | ||||||
|
|
||||||
| // Set AutoBalancer source for deficit recovery -> pull from Position | ||||||
| // CONSERVATIVE: pullFromTopUpSource=false means Position maintains health buffer | ||||||
|
||||||
| // CONSERVATIVE: pullFromTopUpSource=false means Position maintains health buffer |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here - reasoning is it's better to have only one place (single source of truth) where we define this logic (correspondence between ID and swapper key), to help avoid typos or inconsistent usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this reverse computation happen a lot? Does it make sense to store the reversed path within the CollateralConfig?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it's calculated only once during position creation, storing it separately would be redundant and it will increase risk of misconfiguration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll need to store another path though: moet to collateral
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the builtin reverse function here? https://cadence-lang.org/docs/language/values-and-types/arrays#array-fields-and-functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the builtin reverse function here? https://cadence-lang.org/docs/language/values-and-types/arrays#array-fields-and-functions
Uh oh!
There was an error while loading. Please reload this page.