MarketV1AdapterV2 (1 adapter per market) #790
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
adhusson
force-pushed
the
feat/one-market-per-adapter
branch
from
514886a to
a7c04f7
Compare
adhusson
force-pushed
the
feat/one-market-per-adapter
branch
from
a7c04f7 to
8862319
Compare
|
@codex please review again (wrong branch was pushed before) |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-authored-by: Quentin Garchery <garchery.quentin@gmail.com> Signed-off-by: Adrien Husson <adhusson@gmail.com>
This reverts commit 92c42a6.
style: update shares variable names
|
In the current adapter code, we have two distinct concept of ids
don't think it's a huge issue but it might be worth a dev comment to prevent misunderstandings (or we could use the market id as the vault v2 cap id but it would break the current cap id computation standard) |
…o chore/merge-main
|
we never define the id, I think it's fine |
|
@codex review again please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (assets > 0) { | ||
| (, burnedShares) = IMorpho(morpho).withdraw(marketParams(), assets, 0, address(this), address(this)); | ||
| // Safe cast because Market V1 bounds the total shares to uint128.max. | ||
| supplyShares -= uint128(burnedShares); | ||
| } |
There was a problem hiding this comment.
The adapter now derives its position from the locally tracked supplyShares and subtracts the shares burned on withdraw (supplyShares -= uint128(burnedShares)). Because supplyShares is only updated inside allocate/deallocate, an external user can call IMorpho.supply(..., onBehalf = address(this)) to mint supply shares for the adapter without touching this counter. When the vault later deallocates, burnedShares reflects the real Morpho balance (including the donated shares) but the subtraction uses the stale supplyShares, causing an underflow revert and blocking withdrawals; the extra assets are also invisible to realAssets because newAllocation multiplies the tracked supplyShares only. This makes the adapter DOS‑able by a benign “donation” to its market.
Useful? React with 👍 / 👎.
[force remove] merge with main
| MarketParams({loanToken: loanToken, collateralToken: collateralToken, oracle: oracle, irm: irm, lltv: lltv}); | ||
| } | ||
|
|
||
| function newAllocation() public view returns (uint256) { |
There was a problem hiding this comment.
The name is strange for me. It's also an external function so it's even more strange.
expectedSupplyAssetsaccrueAllocationViewcurrentSupplyPosition
|
replaced by #789 |
net gas cost could be ok here we never read the entire market from storage
gas cost test file