-
Notifications
You must be signed in to change notification settings - Fork 1
fix(vault): verify ERC-4626 operation state effects #102
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: master
Are you sure you want to change the base?
Changes from all commits
982c2fb
1593499
6c2ae88
13fe43f
2d4a1e6
6fd3939
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,12 +23,9 @@ import {ERC4626BaseAssertion} from "./ERC4626BaseAssertion.sol"; | |
| /// previewWithdraw rounds UP (returns more shares -> favors vault) | ||
| /// previewRedeem rounds DOWN (returns fewer assets -> favors vault) | ||
| /// | ||
| /// @dev Uses V2 `registerFnCallTrigger` + `ph.context()` for call-scoped triggers, | ||
| /// `ph.callinputAt()` to read call arguments, and `ph.callOutputAt()` to read the | ||
| /// actual return value — replacing the totalSupply/totalAssets delta inference from V1. | ||
| /// ERC-4626 specifies the conservative direction of previews, but it does not set a generic | ||
| /// maximum distance from the state-changing result. Concrete vaults may override | ||
| /// `_maxPreviewDeviation()` when their implementation proves a tighter bound. | ||
| /// @dev In addition to return values, every operation proves the corresponding receiver/owner | ||
| /// share delta, total-supply delta, and underlying-token movement. ERC-4626 does not define a | ||
| /// universal preview-distance bound, so every concrete supported adapter must provide one. | ||
| abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | ||
| /// @notice Register the default trigger set for preview-consistency invariants. | ||
| /// @dev Each ERC-4626 operation gets its own assertion function via registerFnCallTrigger. | ||
|
|
@@ -40,10 +37,15 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| } | ||
|
|
||
| /// @notice Maximum acceptable deviation between a preview result and the actual result. | ||
| /// @dev Defaults to no generic distance cap. Override only when the concrete vault's | ||
| /// implementation proves a smaller maximum. | ||
| function _maxPreviewDeviation() internal view virtual returns (uint256) { | ||
| return type(uint256).max; | ||
| /// @dev Must be derived from the concrete vault implementation; no generic default is sound. | ||
| function _maxPreviewDeviation() internal view virtual returns (uint256); | ||
|
|
||
| /// @notice Account whose underlying-token balance reflects ERC-4626 payments and payouts. | ||
| /// @dev Standard vaults custody assets themselves. Managed-custody adapters such as | ||
| /// LlamaLend must override this with the controller that actually receives and sends the | ||
| /// underlying token. | ||
| function _assetCustodyAccount() internal view virtual returns (address) { | ||
| return vault; | ||
| } | ||
|
|
||
| // --------------------------------------------------------------- | ||
|
|
@@ -55,20 +57,31 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| /// actualSharesMinted - previewDeposit(assets) <= maxDeviation | ||
| function assertDepositPreview() external { | ||
| PhEvm.TriggerContext memory ctx = ph.context(); | ||
| _requireVaultConfigurationAt(_preCall(ctx.callStart)); | ||
| PhEvm.ForkId memory pre = _preCall(ctx.callStart); | ||
| PhEvm.ForkId memory post = _postCall(ctx.callEnd); | ||
| _requireVaultConfigurationAt(pre); | ||
|
|
||
| bytes memory input = ph.callinputAt(ctx.callStart); | ||
| uint256 assets = _firstUint256Arg(input); | ||
| address receiver = _receiver(input, ctx); | ||
| address payer = _triggerCaller(ctx); | ||
|
|
||
| // Preview at pre-call state | ||
| uint256 previewShares = | ||
| _readUintAt(vault, abi.encodeCall(IERC4626.previewDeposit, (assets)), _preCall(ctx.callStart)); | ||
| uint256 previewShares = _readUintAt(vault, abi.encodeCall(IERC4626.previewDeposit, (assets)), pre); | ||
|
|
||
| // Actual return value: deposit returns shares minted | ||
| uint256 actualShares = abi.decode(ph.callOutputAt(ctx.callStart), (uint256)); | ||
|
|
||
| require(previewShares <= actualShares, "ERC4626: previewDeposit > actual shares"); | ||
| require(actualShares - previewShares <= _maxPreviewDeviation(), "ERC4626: deposit preview deviates from actual"); | ||
| _requireIncrease( | ||
| _shareBalanceAt(receiver, pre), | ||
| _shareBalanceAt(receiver, post), | ||
| actualShares, | ||
| "ERC4626: deposit receiver shares mismatch" | ||
| ); | ||
| _requireIncrease(_totalSupplyAt(pre), _totalSupplyAt(post), actualShares, "ERC4626: deposit supply mismatch"); | ||
| _requirePaymentEffects(payer, _assetCustodyAccount(), assets, pre, post, "deposit"); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------- | ||
|
|
@@ -80,19 +93,30 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| /// previewMint(shares) - actualAssetsCharged <= maxDeviation | ||
| function assertMintPreview() external { | ||
| PhEvm.TriggerContext memory ctx = ph.context(); | ||
| _requireVaultConfigurationAt(_preCall(ctx.callStart)); | ||
| PhEvm.ForkId memory pre = _preCall(ctx.callStart); | ||
| PhEvm.ForkId memory post = _postCall(ctx.callEnd); | ||
| _requireVaultConfigurationAt(pre); | ||
|
|
||
| bytes memory input = ph.callinputAt(ctx.callStart); | ||
| uint256 shares = _firstUint256Arg(input); | ||
| address receiver = _receiver(input, ctx); | ||
| address payer = _triggerCaller(ctx); | ||
|
|
||
| uint256 previewAssets = | ||
| _readUintAt(vault, abi.encodeCall(IERC4626.previewMint, (shares)), _preCall(ctx.callStart)); | ||
| uint256 previewAssets = _readUintAt(vault, abi.encodeCall(IERC4626.previewMint, (shares)), pre); | ||
|
|
||
| // Actual return value: mint returns assets charged | ||
| uint256 actualAssets = abi.decode(ph.callOutputAt(ctx.callStart), (uint256)); | ||
|
|
||
| require(previewAssets >= actualAssets, "ERC4626: previewMint < actual assets"); | ||
| require(previewAssets - actualAssets <= _maxPreviewDeviation(), "ERC4626: mint preview deviates from actual"); | ||
| _requireIncrease( | ||
| _shareBalanceAt(receiver, pre), | ||
| _shareBalanceAt(receiver, post), | ||
| shares, | ||
| "ERC4626: mint receiver shares mismatch" | ||
| ); | ||
| _requireIncrease(_totalSupplyAt(pre), _totalSupplyAt(post), shares, "ERC4626: mint supply mismatch"); | ||
| _requirePaymentEffects(payer, _assetCustodyAccount(), actualAssets, pre, post, "mint"); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------- | ||
|
|
@@ -104,13 +128,15 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| /// previewWithdraw(assets) - actualSharesBurned <= maxDeviation | ||
| function assertWithdrawPreview() external { | ||
| PhEvm.TriggerContext memory ctx = ph.context(); | ||
| _requireVaultConfigurationAt(_preCall(ctx.callStart)); | ||
| PhEvm.ForkId memory pre = _preCall(ctx.callStart); | ||
| PhEvm.ForkId memory post = _postCall(ctx.callEnd); | ||
| _requireVaultConfigurationAt(pre); | ||
|
|
||
| bytes memory input = ph.callinputAt(ctx.callStart); | ||
| uint256 assets = _firstUint256Arg(input); | ||
| (address receiver, address owner) = _withdrawAccounts(input, ctx); | ||
|
|
||
| uint256 previewShares = | ||
| _readUintAt(vault, abi.encodeCall(IERC4626.previewWithdraw, (assets)), _preCall(ctx.callStart)); | ||
| uint256 previewShares = _readUintAt(vault, abi.encodeCall(IERC4626.previewWithdraw, (assets)), pre); | ||
|
|
||
| // Actual return value: withdraw returns shares burned | ||
| uint256 actualShares = abi.decode(ph.callOutputAt(ctx.callStart), (uint256)); | ||
|
|
@@ -119,6 +145,14 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| require( | ||
| previewShares - actualShares <= _maxPreviewDeviation(), "ERC4626: withdraw preview deviates from actual" | ||
| ); | ||
| _requireDecrease( | ||
| _shareBalanceAt(owner, pre), | ||
| _shareBalanceAt(owner, post), | ||
| actualShares, | ||
| "ERC4626: withdraw owner shares mismatch" | ||
| ); | ||
| _requireDecrease(_totalSupplyAt(pre), _totalSupplyAt(post), actualShares, "ERC4626: withdraw supply mismatch"); | ||
| _requirePayoutEffects(_assetCustodyAccount(), receiver, assets, pre, post, "withdraw"); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------- | ||
|
|
@@ -130,19 +164,26 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| /// actualAssetsReturned - previewRedeem(shares) <= maxDeviation | ||
| function assertRedeemPreview() external { | ||
| PhEvm.TriggerContext memory ctx = ph.context(); | ||
| _requireVaultConfigurationAt(_preCall(ctx.callStart)); | ||
| PhEvm.ForkId memory pre = _preCall(ctx.callStart); | ||
| PhEvm.ForkId memory post = _postCall(ctx.callEnd); | ||
| _requireVaultConfigurationAt(pre); | ||
|
|
||
| bytes memory input = ph.callinputAt(ctx.callStart); | ||
| uint256 shares = _firstUint256Arg(input); | ||
| (address receiver, address owner) = _withdrawAccounts(input, ctx); | ||
|
|
||
| uint256 previewAssets = | ||
| _readUintAt(vault, abi.encodeCall(IERC4626.previewRedeem, (shares)), _preCall(ctx.callStart)); | ||
| uint256 previewAssets = _readUintAt(vault, abi.encodeCall(IERC4626.previewRedeem, (shares)), pre); | ||
|
|
||
| // Actual return value: redeem returns assets returned | ||
| uint256 actualAssets = abi.decode(ph.callOutputAt(ctx.callStart), (uint256)); | ||
|
|
||
| require(previewAssets <= actualAssets, "ERC4626: previewRedeem > actual assets"); | ||
| require(actualAssets - previewAssets <= _maxPreviewDeviation(), "ERC4626: redeem preview deviates from actual"); | ||
| _requireDecrease( | ||
| _shareBalanceAt(owner, pre), _shareBalanceAt(owner, post), shares, "ERC4626: redeem owner shares mismatch" | ||
| ); | ||
| _requireDecrease(_totalSupplyAt(pre), _totalSupplyAt(post), shares, "ERC4626: redeem supply mismatch"); | ||
| _requirePayoutEffects(_assetCustodyAccount(), receiver, actualAssets, pre, post, "redeem"); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------- | ||
|
|
@@ -167,4 +208,102 @@ abstract contract ERC4626PreviewAssertion is ERC4626BaseAssertion { | |
| value := mload(add(input, 36)) | ||
| } | ||
| } | ||
|
|
||
| function _receiver(bytes memory input, PhEvm.TriggerContext memory ctx) internal view returns (address) { | ||
| return input.length >= 68 ? _addressArg(input, 1) : _triggerCaller(ctx); | ||
| } | ||
|
|
||
| function _withdrawAccounts(bytes memory input, PhEvm.TriggerContext memory ctx) | ||
| internal | ||
| view | ||
| returns (address receiver, address owner) | ||
| { | ||
| if (input.length >= 100) return (_addressArg(input, 1), _addressArg(input, 2)); | ||
| address caller = _triggerCaller(ctx); | ||
|
makemake-kbo marked this conversation as resolved.
|
||
| receiver = input.length >= 68 ? _addressArg(input, 1) : caller; | ||
| owner = caller; | ||
| } | ||
|
|
||
| function _triggerCaller(PhEvm.TriggerContext memory ctx) internal view returns (address) { | ||
| PhEvm.CallFilter memory filter = PhEvm.CallFilter({ | ||
| callType: 0, minDepth: 0, maxDepth: type(uint32).max, topLevelOnly: false, successOnly: true | ||
| }); | ||
| PhEvm.TriggerCall[] memory calls = ph.matchingCalls(vault, ctx.selector, filter, 8); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Capping this lookup at eight makes the ninth valid deposit or mint in one transaction revert because its call id can never be found. That turns the performance bound into an undocumented protocol restriction. Could we either resolve the caller without a global prefix scan or explicitly enforce and document a supported call count at registration? |
||
| for (uint256 i; i < calls.length; ++i) { | ||
| if (calls[i].callId == ctx.callStart) return calls[i].caller; | ||
| } | ||
| revert("ERC4626: triggered call not found"); | ||
| } | ||
|
|
||
| function _requirePaymentEffects( | ||
| address payer, | ||
| address custody, | ||
| uint256 amount, | ||
| PhEvm.ForkId memory pre, | ||
| PhEvm.ForkId memory post, | ||
| string memory operation | ||
| ) internal view { | ||
| uint256 payerBefore = _assetBalanceAt(payer, pre); | ||
| uint256 payerAfter = _assetBalanceAt(payer, post); | ||
| if (payer == custody) { | ||
| require(payerAfter == payerBefore, string.concat("ERC4626: ", operation, " asset payment mismatch")); | ||
| return; | ||
| } | ||
| _requireIncrease( | ||
| _assetBalanceAt(custody, pre), | ||
| _assetBalanceAt(custody, post), | ||
| amount, | ||
| string.concat("ERC4626: ", operation, " asset payment mismatch") | ||
| ); | ||
| _requireDecrease( | ||
| payerBefore, payerAfter, amount, string.concat("ERC4626: ", operation, " payer assets mismatch") | ||
| ); | ||
| } | ||
|
|
||
| function _requirePayoutEffects( | ||
| address custody, | ||
| address receiver, | ||
| uint256 amount, | ||
| PhEvm.ForkId memory pre, | ||
| PhEvm.ForkId memory post, | ||
| string memory operation | ||
| ) internal view { | ||
| uint256 custodyBefore = _assetBalanceAt(custody, pre); | ||
| uint256 custodyAfter = _assetBalanceAt(custody, post); | ||
| if (custody == receiver) { | ||
| require(custodyAfter == custodyBefore, string.concat("ERC4626: ", operation, " asset payout mismatch")); | ||
| return; | ||
| } | ||
| _requireDecrease( | ||
| custodyBefore, custodyAfter, amount, string.concat("ERC4626: ", operation, " vault assets mismatch") | ||
| ); | ||
| _requireIncrease( | ||
| _assetBalanceAt(receiver, pre), | ||
| _assetBalanceAt(receiver, post), | ||
| amount, | ||
| string.concat("ERC4626: ", operation, " receiver assets mismatch") | ||
| ); | ||
| } | ||
|
|
||
| function _addressArg(bytes memory input, uint256 index) internal pure returns (address value) { | ||
| uint256 offset = 36 + index * 32; | ||
| require(input.length >= 4 + (index + 1) * 32, "ERC4626Preview: address arg missing"); | ||
| assembly ("memory-safe") { | ||
| value := mload(add(input, offset)) | ||
| } | ||
| } | ||
|
|
||
| function _requireIncrease(uint256 beforeValue, uint256 afterValue, uint256 amount, string memory reason) | ||
| internal | ||
| pure | ||
| { | ||
| require(afterValue >= beforeValue && afterValue - beforeValue == amount, reason); | ||
| } | ||
|
|
||
| function _requireDecrease(uint256 beforeValue, uint256 afterValue, uint256 amount, string memory reason) | ||
| internal | ||
| pure | ||
| { | ||
| require(beforeValue >= afterValue && beforeValue - afterValue == amount, reason); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.