-
Notifications
You must be signed in to change notification settings - Fork 21
V2 Adapter #786
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
base: main
Are you sure you want to change the base?
V2 Adapter #786
Conversation
fa42572 to
eaab822
Compare
peyha
left a comment
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.
Nice 🔥 🔥 using a linked list to manage maturities is smart
If I understand correctly, the allocation workflow should be
An allocator creates a make offer for the vault
A taker takes this offer onchain
| morphoV2 calls onBuy on the adapter
| | the adapter updates its state
| | the adapter calls allocate on its parent vault
| | | the vault V2 calls allocate on the market V2 adapter
| | | | the adapter returns the corresponding ids and allocated assets
| _positions[obligationId].growth += gainedGrowth; | ||
| _maturities[obligation.maturity].growthLostAtMaturity += gainedGrowth; | ||
| currentGrowth += gainedGrowth; | ||
| } else { |
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.
what is this usecase for ? buying after maturity ? the vault is likely to get liquidated so not sure there is a huge usecase
| /// @dev Losses are immdiately accounted minus a discount applied to the remaining interest to be earned, in proportion | ||
| /// to the relative sizes of the loss and the adapter's position in the obligation hit by the loss. |
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.
not very clear
| /* ACCOUNTING */ | ||
|
|
||
| uint256 public _totalAssets; | ||
| uint48 public lastUpdate; |
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 guess uint48 is for packing, so why not uint64? It is much more likely to be handled by all EVM tools such as certora
|
|
||
| // Do not cleanup the linked list if we end up at 0 growth | ||
| function withdraw(Obligation memory obligation, uint256 obligationUnits, uint256 shares) external { | ||
| require(IVaultV2(parentVault).isAllocator(msg.sender), NotAuthorized()); |
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.
shouldn't lenders also be able to trigger a withdraw on morpho V2 ?
| accrueInterest(); | ||
| if (obligation.maturity > block.timestamp) { | ||
| uint128 timeToMaturity = uint128(obligation.maturity - block.timestamp); | ||
| uint128 gainedGrowth = ((obligationUnits - buyerAssets) / timeToMaturity).toUint128(); |
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.
there could be some roundings by defining growth with this, not sure it's an issue though
| nextMaturity = firstMaturity; | ||
| } else { | ||
| nextMaturity = _maturities[prevMaturity].nextMaturity; | ||
| require(nextMaturity != 0, IncorrectHint()); |
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.
not sure we should revert on this, a vault v2 could set multiple offers (with both sell and buy) and this could create some path dependency on the execution of these offers (what if the sell offer removes the prevMaturity for some buy)
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.
in this case, you could just use nextMaturity=firstMaturity
|
|
||
| uint256 _totalAssetsBefore = _totalAssets; | ||
| removeUnits(obligation, obligationUnits); | ||
| require(vaultBuffer >= _totalAssetsBefore.zeroFloorSub(_totalAssets), BufferTooLow()); |
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.
as discussed during the bootcamp, using this specific buffer is the second idea but it lacks the ability to sell at a loss in a controlled way
No description provided.