Morpho erc4626 Swap and Sink connectors#126
Conversation
Kay-Zee
left a comment
There was a problem hiding this comment.
Where these fresh or copied over from another erc4626 connector? if it was copied then we can look at what the specific changes where for this morpho implementation
cadence/contracts/connectors/evm/morpho/MorphoERC4626SinkConnectors.cdc
Outdated
Show resolved
Hide resolved
cadence/contracts/connectors/evm/morpho/MorphoERC4626SinkConnectors.cdc
Outdated
Show resolved
Hide resolved
| /// Deposits up to the Sink's capacity from the provided Vault | ||
| access(all) fun depositCapacity(from: auth(FungibleToken.Withdraw) &{FungibleToken.Vault}) { | ||
| // check capacity & early return if none | ||
| let capacity = self.minimumCapacity() |
There was a problem hiding this comment.
should this just be a pre-condition?
There was a problem hiding this comment.
is it ok to abort/panic the deposit instead of doing nothing?
| // withdraw the appropriate amount from the referenced vault & deposit to the EVMTokenConnectors Sink | ||
| var amount = capacity <= from.balance ? capacity : from.balance | ||
|
|
||
| // TODO: pass from through and skip the intermediary withdrawal |
There was a problem hiding this comment.
can we explain why we're doing this intermediary withdrawl then?
There was a problem hiding this comment.
depositCapacity can deposit less than requested (capacity/fees/bridge constraints), and it doesn’t return "actualDeposited". Without the intermediary vault there's no way to safely compute the amount
| gasLimit: 1_000_000 | ||
| )! | ||
| if depositRes.status != EVM.Status.successful { | ||
| // TODO: Consider unwinding the deposit & returning to the from vault |
There was a problem hiding this comment.
what's the benefit, vs panic? since the funds still go back to the owner no?
There was a problem hiding this comment.
it was the original implementation from ERC4626 sink,
currently I also don't see any considerable benefits of unwinding apart from better error tracing/handing in case of deposit batching
cadence/contracts/connectors/evm/morpho/MorphoERC4626SinkConnectors.cdc
Outdated
Show resolved
Hide resolved
| /// Deposits up to the Sink's capacity from the provided Vault | ||
| access(all) fun depositCapacity(from: auth(FungibleToken.Withdraw) &{FungibleToken.Vault}) { | ||
| // check capacity & early return if none | ||
| let capacity = self.minimumCapacity() |
There was a problem hiding this comment.
same comment, can these be pre conditions?
| access(all) fun swapBack(quote: {DeFiActions.Quote}?, residual: @{FungibleToken.Vault}): @{FungibleToken.Vault} { | ||
| if residual.balance == 0.0 { | ||
| Burner.burn(<-residual) | ||
| return <- DeFiActionsUtils.getEmptyVault(self.assetType) |
There was a problem hiding this comment.
a little confused on the purpose of burning the empty vault then creating a new empty vault
There was a problem hiding this comment.
it burns an empty share vault and returns an empty asset vault
There was a problem hiding this comment.
ah, so the vault types are different, got it.
| let _quote = quote ?? self.quoteOut(forProvided: residual.balance, reverse: true) | ||
| let outAmount = _quote.outAmount | ||
|
|
||
| assert(_quote.inType == self.outType(), message: "Quote inType mismatch") |
There was a problem hiding this comment.
worth putting a comment here, as the asserts are on a first glance un-intuitive, even though this is the swap back function
Morpho ERC4626 sink and swap connectors
transactions and scripts for manual testing
basic setup for manual testing in emulator mainnet-fork
manual testing instruction:
flow deploy --network mainnet-fork --updateflow.json