Skip to content

fix(morpho-sdk): forward-accrue VaultV1 deposit for share-price slippage#797

Open
Foulks-Plb wants to merge 2 commits into
mainfrom
vault-v2-deposit-interest-accrual
Open

fix(morpho-sdk): forward-accrue VaultV1 deposit for share-price slippage#797
Foulks-Plb wants to merge 2 commits into
mainfrom
vault-v2-deposit-interest-accrual

Conversation

@Foulks-Plb

Copy link
Copy Markdown
Contributor

Motivation

MorphoVaultV1.deposit derived maxSharePrice from pre-accrue vault state. But MetaMorpho's on-chain deposit calls _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 with SlippageExceeded when interest accrues between build and execution. VaultV2 deposit, the V2 leg of migrateToV2, 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/maxSharePrice in MorphoVaultV1.deposit, mirroring MorphoVaultV2.deposit. Since an AccrualVault aggregates several markets with no single lastUpdate, the accrual timestamp is clamped to the latest market lastUpdate and now before adding 2h (a precondition of accrueInterest). Adds a patch changeset for @morpho-org/morpho-sdk.

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>
@Foulks-Plb Foulks-Plb added the bug Something isn't working label Jun 16, 2026
@Foulks-Plb Foulks-Plb self-assigned this Jun 16, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Foulks-Plb Foulks-Plb requested a review from Rubilmax June 16, 2026 13:26
@Foulks-Plb Foulks-Plb marked this pull request as ready for review June 16, 2026 13:26
@Foulks-Plb Foulks-Plb requested a review from a team as a code owner June 16, 2026 13:26

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +227 to +229
const accruedVault = vaultData.accrueInterest(accrualTimestamp);

const shares = accruedVault.toShares(totalAssets);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant