fix(morpho-sdk): forward-accrue VaultV1 deposit for share-price slippage#797
fix(morpho-sdk): forward-accrue VaultV1 deposit for share-price slippage#797Foulks-Plb wants to merge 2 commits into
Conversation
MetaMorpho's deposit calls _accrueInterest(), so the execution-time share price is >= the build-time one. Deriving maxSharePrice from pre-accrue vault state can revert GeneralAdapter1's check with SlippageExceeded. Forward-accrue interest by 2h before computing shares, mirroring VaultV2 deposit and blue repay. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0e4aba637
ℹ️ 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".
| ) + Time.s.from.h(2n); | ||
| const accruedVault = vaultData.accrueInterest(accrualTimestamp); | ||
|
|
||
| const shares = accruedVault.toShares(totalAssets); |
There was a problem hiding this comment.
Round deposited shares down for the price bound
For deposits where the exact share amount is fractional, AccrualVault.toShares defaults to rounding up (packages/blue-sdk/src/vault/Vault.ts:190), while an ERC-4626 deposit mints the rounded-down share amount and the existing MetaMorpho operation path uses vault.toShares(assets, "Down") (packages/bundler-sdk-viem/src/actions.ts:841). This makes maxSharePrice use a larger denominator than the shares GA1 will actually observe, so with slippageTolerance: 0n or small/fractional deposits the transaction can still revert with the slippage check despite the forward accrual; pass "Down" here and keep the zero-share guard on that rounded-down value.
Useful? React with 👍 / 👎.
| const accruedVault = vaultData.accrueInterest(accrualTimestamp); | ||
|
|
||
| const shares = accruedVault.toShares(totalAssets); |
There was a problem hiding this comment.
Add regression coverage for VaultV1 forward accrual
Root AGENTS.md §2.10 says behavior-affecting package source changes must land with tests, but this commit changes the VaultV1 deposit maxSharePrice calculation and adds only source plus a changeset. Without an entity or fork regression test asserting the bound is computed from a forward-accrued VaultV1 state, the specific slippage regression this patch fixes can be reintroduced without any test failing.
Useful? React with 👍 / 👎.
Motivation
MorphoVaultV1.depositderivedmaxSharePricefrom pre-accrue vault state. But MetaMorpho's on-chaindepositcalls_accrueInterest(), so the execution-time share price is>=the build-time one — leaving the slippage bound too tight and able to revert GeneralAdapter1's check withSlippageExceededwhen interest accrues between build and execution. VaultV2 deposit, the V2 leg ofmigrateToV2, and blue repay already carry a 2h forward-accrual buffer; VaultV1 deposit was the one path missing it (it was overlooked in #681).Solution
Forward-accrue interest by 2h before computing
shares/maxSharePriceinMorphoVaultV1.deposit, mirroringMorphoVaultV2.deposit. Since an AccrualVault aggregates several markets with no singlelastUpdate, the accrual timestamp is clamped to the latest marketlastUpdateandnowbefore adding 2h (a precondition ofaccrueInterest). Adds a patch changeset for@morpho-org/morpho-sdk.