-
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 25 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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||
|
|
@@ -80,11 +80,24 @@ access(all) contract FlowYieldVaultsStrategiesV2 { | |||||
| access(self) let position: @FlowALPv0.Position | ||||||
| access(self) var sink: {DeFiActions.Sink} | ||||||
| access(self) var source: {DeFiActions.Source} | ||||||
| /// Tracks whether the underlying FlowALP position has been closed. Once true, | ||||||
| /// availableBalance() returns 0.0 to avoid panicking when the pool no longer | ||||||
| /// holds the position (e.g. during YieldVault burnCallback after close). | ||||||
| access(self) var positionClosed: Bool | ||||||
|
|
||||||
| 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) | ||||||
| self.positionClosed = false | ||||||
| self.position <-position | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -96,6 +109,7 @@ access(all) contract FlowYieldVaultsStrategiesV2 { | |||||
| } | ||||||
| /// Returns the amount available for withdrawal via the inner Source | ||||||
| access(all) fun availableBalance(ofToken: Type): UFix64 { | ||||||
| if self.positionClosed { return 0.0 } | ||||||
| return ofToken == self.source.getSourceType() ? self.source.minimumAvailable() : 0.0 | ||||||
| } | ||||||
| /// Deposits up to the inner Sink's capacity from the provided authorized Vault reference | ||||||
|
|
@@ -110,6 +124,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)" | ||||||
| } | ||||||
| post { | ||||||
| result.getType() == collateralType: "Withdraw Vault (\(result.getType().identifier)) is not of a requested collateral type (\(collateralType.identifier))" | ||||||
| } | ||||||
|
|
||||||
| // Step 1: Get debt amounts - returns {Type: UFix64} dictionary | ||||||
| let debtsByType = self.position.getTotalDebt() | ||||||
|
|
||||||
| // Step 2: Calculate total debt amount across all debt types | ||||||
| var totalDebtAmount: UFix64 = 0.0 | ||||||
| for debtAmount in debtsByType.values { | ||||||
| totalDebtAmount = totalDebtAmount + debtAmount | ||||||
| } | ||||||
|
||||||
|
|
||||||
| // Step 3: If no debt, close with empty sources 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 resultVaults <- self.position.closePosition( | ||||||
| repaymentSources: [] | ||||||
| ) | ||||||
| // 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 | ||||||
| self.positionClosed = true | ||||||
| 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 = FlowYieldVaultsStrategiesV2.getYieldToMoetSwapperConfigKey(self.uniqueID)! | ||||||
| let yieldToMoetSwapper = FlowYieldVaultsStrategiesV2.config[swapperKey] as! {DeFiActions.Swapper}? | ||||||
| ?? panic("No yield→MOET swapper found for strategy \(self.id()!)") | ||||||
|
|
||||||
| // Step 6: Create a SwapSource that converts yield tokens to MOET when pulled by closePosition. | ||||||
| // The pool will call source.withdrawAvailable(maxAmount: debtAmount) which internally uses | ||||||
| // quoteIn(forDesired: debtAmount) to compute the exact yield token input needed. | ||||||
| let moetSource = SwapConnectors.SwapSource( | ||||||
| swapper: yieldToMoetSwapper, | ||||||
| source: yieldTokenSource, | ||||||
| uniqueID: self.copyID() | ||||||
| ) | ||||||
|
|
||||||
| // Step 7: Close position - pool pulls exactly the debt amount from moetSource | ||||||
| let resultVaults <- self.position.closePosition(repaymentSources: [moetSource]) | ||||||
|
|
||||||
| // 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 { | ||||||
|
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. The current logic is to destroy any funds we get back that aren't the collateral type without checking their type or balance, which seems dangerous to me. On line 215 above, we assert that there are either 1 or 2 result vaults, so this while loop is operating on at most one vault in If it is still possible to have overpayment dust, when let dustVault <- resultVaults.removeFirst()
assert(resultVaults.length == 0)
destroy <-resultVaults
// We may receive a small amount of MOET in addition to collateral, which represents dust overpayment from repaying the position's debt.
// Because this is a trivial amount of funds, we do not attempt to recover it.
assert(dustVault.getType() == Type<MOET>)
assert(dustVault.balance <= maximumPossibleDustBalance)
destroy <-dustVaultIf it isn't possible to have overpayment dust, then we can change line 215 to assert
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. I implemented dust handling in #183, so the strategy will swap overpayment dust back to collateral
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. OK, I would still avoid destroying vaults without checking them. The current code handles the first collateral vault (👍) but can destroy subsequent vaults without checking them. I'd just add assertions prior to destroying those vaults, to make sure they are the expected dust (expected type and expected small value). |
||||||
| if dustVault.getType() == collateralType { | ||||||
| collateralVault.deposit(from: <-dustVault) | ||||||
| } else { | ||||||
| // @TODO implement swapping moet to collateral | ||||||
|
|
||||||
| // // Swap overpayment back to collateral using configured swapper | ||||||
| // let moetToCollateralSwapperKey = FlowYieldVaultsStrategiesV2.getMoetToCollateralSwapperConfigKey(self.id()!) | ||||||
| // let dustToCollateralSwapper = FlowYieldVaultsStrategiesV2.config[moetToCollateralSwapperKey] 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) | ||||||
| destroy dustVault | ||||||
| } | ||||||
| } 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 | ||||||
| self.positionClosed = true | ||||||
| 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 +413,48 @@ 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 |
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.
This writes per-vault runtime state into a contract-global dictionary, and nothing removes it on close/burn, so every opened+closed vault leaks a yieldToMoetSwapper_<id> entry.
I opened a follow-up fix in #201
That PR moves the swapper onto FUSDEVStrategy itself and removes the global runtime config/key path.
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.
I'll fix that, I missed this one during config partitioning
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.
that's right, this contract is already deployed, and this was the original implementation
the top level config with nested maps is a workaround to keep the contract updatable
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.
I've addressed this issue in #183
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.