From a431f2d9b8d10a945c648701979975d99a7fce98 Mon Sep 17 00:00:00 2001 From: chef-burger <137024020+chefburger@users.noreply.github.com> Date: Fri, 1 Nov 2024 11:40:59 +0800 Subject: [PATCH] Feat/aduit part7 (#195) * forge install: pancake-create3-factory * optimization: [internal-s46] remove the restriction that sync must be called inside lock callback * optimization: [internal-s47] adjust bubble up revert error to conform with ERC7751 --------- Co-authored-by: ChefMist <133624774+ChefMist@users.noreply.github.com> --- .../BinHookTest#testBurnSucceedsWithHook.snap | 2 +- ...inHookTest#testDonateSucceedsWithHook.snap | 2 +- ...okTest#testInitializeSucceedsWithHook.snap | 2 +- .../BinHookTest#testMintSucceedsWithHook.snap | 2 +- .../BinHookTest#testSwapSucceedsWithHook.snap | 2 +- .../BinPoolManagerBytecodeSize.snap | 2 +- .../BinPoolManagerTest#testGasDonate.snap | 2 +- ...nPoolManagerTest#testGasMintNneBins-1.snap | 2 +- ...nPoolManagerTest#testGasMintNneBins-2.snap | 2 +- ...inPoolManagerTest#testGasMintOneBin-1.snap | 2 +- ...inPoolManagerTest#testGasMintOneBin-2.snap | 2 +- ...olManagerTest#testGasSwapMultipleBins.snap | 2 +- ...nagerTest#testGasSwapOverBigBinIdGate.snap | 2 +- ...nPoolManagerTest#testGasSwapSingleBin.snap | 2 +- ...oolManagerTest#testMintNativeCurrency.snap | 2 +- .../CLPoolManagerBytecodeSize.snap | 2 +- ...oolManagerTest#addLiquidity_fromEmpty.snap | 2 +- ...ManagerTest#addLiquidity_fromNonEmpty.snap | 2 +- .../CLPoolManagerTest#donateBothTokens.snap | 2 +- .../CLPoolManagerTest#gasDonateOneToken.snap | 2 +- ...PoolManagerTest#swap_againstLiquidity.snap | 2 +- ...gerTest#swap_leaveSurplusTokenInVault.snap | 2 +- ...oolManagerTest#swap_runOutOfLiquidity.snap | 2 +- .../CLPoolManagerTest#swap_withHooks.snap | 2 +- .forge-snapshots/VaultBytecodeSize.snap | 2 +- src/Vault.sol | 6 ++- src/interfaces/IVault.sol | 4 +- src/libraries/CustomRevert.sol | 51 ++++++++++++++----- src/libraries/Hooks.sol | 9 ++-- src/types/Currency.sol | 21 ++++---- test/libraries/Currency.t.sol | 17 +++++-- test/pool-bin/BinHookRevertWithReason.t.sol | 18 +++++-- test/pool-bin/libraries/BinPoolFee.t.sol | 14 +++-- test/pool-cl/CLHookRevertWithReason.t.sol | 18 +++++-- test/pool-cl/CLPoolManager.t.sol | 8 ++- test/vault/Vault.t.sol | 35 +++++++++++-- test/vault/VaultSync.t.sol | 4 +- 37 files changed, 173 insertions(+), 82 deletions(-) diff --git a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap index fbe64408..2b568eb5 100644 --- a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap @@ -1 +1 @@ -178162 \ No newline at end of file +178172 \ No newline at end of file diff --git a/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap index 9a6770a8..ebc63de1 100644 --- a/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap @@ -1 +1 @@ -188425 \ No newline at end of file +188435 \ No newline at end of file diff --git a/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap index 250531ae..678f7241 100644 --- a/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap @@ -1 +1 @@ -184048 \ No newline at end of file +184058 \ No newline at end of file diff --git a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap index cbed68c2..c3de81a3 100644 --- a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap @@ -1 +1 @@ -311216 \ No newline at end of file +311226 \ No newline at end of file diff --git a/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap index 93bf46aa..f263fba9 100644 --- a/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap @@ -1 +1 @@ -189420 \ No newline at end of file +189430 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerBytecodeSize.snap b/.forge-snapshots/BinPoolManagerBytecodeSize.snap index b301e2a9..0bbd03da 100644 --- a/.forge-snapshots/BinPoolManagerBytecodeSize.snap +++ b/.forge-snapshots/BinPoolManagerBytecodeSize.snap @@ -1 +1 @@ -23951 \ No newline at end of file +24175 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap b/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap index 83155a78..c263e782 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap @@ -1 +1 @@ -118763 \ No newline at end of file +118513 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap index 9cd1996a..82fe3898 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap @@ -1 +1 @@ -968494 \ No newline at end of file +968244 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap index be9547b5..d9bb8fa3 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap @@ -1 +1 @@ -327806 \ No newline at end of file +327556 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap index 8c1e4929..4b29f52e 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap @@ -1 +1 @@ -337530 \ No newline at end of file +337280 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap index 2fd7a38b..78acc12b 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap @@ -1 +1 @@ -140081 \ No newline at end of file +139831 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap index 8ca7bd7f..64d97ebe 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap @@ -1 +1 @@ -173046 \ No newline at end of file +172918 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap index c7a37b8d..c290063a 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap @@ -1 +1 @@ -179074 \ No newline at end of file +178946 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap index 39786e8d..94bbfdd8 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap @@ -1 +1 @@ -133077 \ No newline at end of file +132949 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap index 03f02199..2a930d48 100644 --- a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap +++ b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap @@ -1 +1 @@ -304488 \ No newline at end of file +304363 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerBytecodeSize.snap b/.forge-snapshots/CLPoolManagerBytecodeSize.snap index 83b597c0..1931df6c 100644 --- a/.forge-snapshots/CLPoolManagerBytecodeSize.snap +++ b/.forge-snapshots/CLPoolManagerBytecodeSize.snap @@ -1 +1 @@ -20902 \ No newline at end of file +21103 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap index f0e0e21d..40f0c110 100644 --- a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap +++ b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap @@ -1 +1 @@ -347609 \ No newline at end of file +347359 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap index e1f09317..3966f9c9 100644 --- a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap +++ b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap @@ -1 +1 @@ -163070 \ No newline at end of file +162820 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap b/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap index c9edc385..f46f6004 100644 --- a/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap +++ b/.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap @@ -1 +1 @@ -163350 \ No newline at end of file +163100 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap b/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap index d7be5186..2927ac50 100644 --- a/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap +++ b/.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap @@ -1 +1 @@ -108297 \ No newline at end of file +108172 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap index 4b031271..42fb48cf 100644 --- a/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap +++ b/.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap @@ -1 +1 @@ -130864 \ No newline at end of file +130739 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap b/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap index 365335b6..f46f6004 100644 --- a/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap +++ b/.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap @@ -1 +1 @@ -163225 \ No newline at end of file +163100 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap index 8657088b..df9ce9e6 100644 --- a/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap +++ b/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap @@ -1 +1 @@ -148644 \ No newline at end of file +148519 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap b/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap index 50386289..a0fa5b33 100644 --- a/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap +++ b/.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap @@ -1 +1 @@ -87398 \ No newline at end of file +87408 \ No newline at end of file diff --git a/.forge-snapshots/VaultBytecodeSize.snap b/.forge-snapshots/VaultBytecodeSize.snap index 008a307e..d25ee827 100644 --- a/.forge-snapshots/VaultBytecodeSize.snap +++ b/.forge-snapshots/VaultBytecodeSize.snap @@ -1 +1 @@ -7651 \ No newline at end of file +7792 \ No newline at end of file diff --git a/src/Vault.sol b/src/Vault.sol index 853fedd4..d3c0502c 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -118,7 +118,7 @@ contract Vault is IVault, VaultToken, Ownable { } } - function sync(Currency currency) public override isLocked { + function sync(Currency currency) public override { if (currency.isNative()) { VaultReserve.setVaultReserve(CurrencyLibrary.NATIVE, 0); } else { @@ -156,7 +156,9 @@ contract Vault is IVault, VaultToken, Ownable { /// @inheritdoc IVault function collectFee(Currency currency, uint256 amount, address recipient) external onlyRegisteredApp { - if (SettlementGuard.getLocker() != address(0)) revert LockHeld(); + // prevent transfer between the sync and settle balanceOfs (native settle uses msg.value) + (Currency syncedCurrency,) = VaultReserve.getVaultReserve(); + if (!currency.isNative() && syncedCurrency == currency) revert FeeCurrencySynced(); reservesOfApp[msg.sender][currency] -= amount; currency.transfer(recipient, amount); } diff --git a/src/interfaces/IVault.sol b/src/interfaces/IVault.sol index 98e4abca..58c7de13 100644 --- a/src/interfaces/IVault.sol +++ b/src/interfaces/IVault.sol @@ -27,8 +27,8 @@ interface IVault is IVaultToken { /// @notice Thrown when there is no locker error NoLocker(); - /// @notice Thrown when lock is held by someone - error LockHeld(); + /// @notice Thrown when collectFee is attempted on a token that is synced. + error FeeCurrencySynced(); function isAppRegistered(address app) external returns (bool); diff --git a/src/libraries/CustomRevert.sol b/src/libraries/CustomRevert.sol index fca6a395..02678cac 100644 --- a/src/libraries/CustomRevert.sol +++ b/src/libraries/CustomRevert.sol @@ -5,23 +5,46 @@ pragma solidity ^0.8.0; /// @notice Contains functions for reverting with custom errors with different argument types efficiently /// @dev The functions may tamper with the free memory pointer but it is fine since the call context is exited immediately library CustomRevert { - /// @notice bubble up the revert message returned by a call and revert with the selector provided - /// @dev this function should only be used with custom errors of the type `CustomError(address target, bytes revertReason)` - function bubbleUpAndRevertWith(bytes4 selector, address addr) internal pure { + /// @dev ERC-7751 error for wrapping bubbled up reverts + error WrappedError(address target, bytes4 selector, bytes reason, bytes details); + + /// @notice bubble up the revert message returned by a call and revert with a wrapped ERC-7751 error + /// @dev this method can be vulnerable to revert data bombs + function bubbleUpAndRevertWith( + address revertingContract, + bytes4 revertingFunctionSelector, + bytes4 additionalContext + ) internal pure { + bytes4 wrappedErrorSelector = WrappedError.selector; assembly ("memory-safe") { - let size := returndatasize() - let fmp := mload(0x40) + // Ensure the size of the revert data is a multiple of 32 bytes + let encodedDataSize := mul(div(add(returndatasize(), 31), 32), 32) - // Encode selector, address, offset, size, data - mstore(fmp, selector) - mstore(add(fmp, 0x04), addr) - mstore(add(fmp, 0x24), 0x40) - mstore(add(fmp, 0x44), size) - returndatacopy(add(fmp, 0x64), 0, size) + let fmp := mload(0x40) - // Ensure the size is a multiple of 32 bytes - let encodedSize := add(0x64, mul(div(add(size, 31), 32), 32)) - revert(fmp, encodedSize) + // Encode wrapped error selector, address, function selector, offset, additional context, size, revert reason + mstore(fmp, wrappedErrorSelector) + mstore(add(fmp, 0x04), and(revertingContract, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore( + add(fmp, 0x24), + and(revertingFunctionSelector, 0xffffffff00000000000000000000000000000000000000000000000000000000) + ) + // offset revert reason + mstore(add(fmp, 0x44), 0x80) + // offset additional context + mstore(add(fmp, 0x64), add(0xa0, encodedDataSize)) + // size revert reason + mstore(add(fmp, 0x84), returndatasize()) + // revert reason + returndatacopy(add(fmp, 0xa4), 0, returndatasize()) + // size additional context + mstore(add(fmp, add(0xa4, encodedDataSize)), 0x04) + // additional context + mstore( + add(fmp, add(0xc4, encodedDataSize)), + and(additionalContext, 0xffffffff00000000000000000000000000000000000000000000000000000000) + ) + revert(fmp, add(0xe4, encodedDataSize)) } } } diff --git a/src/libraries/Hooks.sol b/src/libraries/Hooks.sol index 465b2a0a..98595d40 100644 --- a/src/libraries/Hooks.sol +++ b/src/libraries/Hooks.sol @@ -15,12 +15,9 @@ library Hooks { using ParametersHelper for bytes32; using LPFeeLibrary for uint24; using ParseBytes for bytes; - using CustomRevert for bytes4; - /// @notice thrown when a hook call fails - /// @param hook the hook address - /// @param revertReason bubbled up revert reason - error Wrap__FailedHookCall(address hook, bytes revertReason); + /// @notice Additional context for ERC-7751 wrapped error when a hook call fails + error HookCallFailed(); /// @notice Hook permissions contain conflict /// 1. enabled beforeSwapReturnsDelta, but lacking beforeSwap call @@ -79,7 +76,7 @@ library Hooks { success := call(gas(), self, 0, add(data, 0x20), mload(data), 0, 0) } // Revert with FailedHookCall, containing any error message to bubble up - if (!success) Wrap__FailedHookCall.selector.bubbleUpAndRevertWith(address(self)); + if (!success) CustomRevert.bubbleUpAndRevertWith(address(self), bytes4(data), HookCallFailed.selector); // The call was successful, fetch the returned data assembly ("memory-safe") { diff --git a/src/types/Currency.sol b/src/types/Currency.sol index 31aae375..4c6757e8 100644 --- a/src/types/Currency.sol +++ b/src/types/Currency.sol @@ -29,17 +29,12 @@ function greaterThanOrEqualTo(Currency currency, Currency other) pure returns (b /// @dev This library allows for transferring and holding native tokens and ERC20 tokens library CurrencyLibrary { using CurrencyLibrary for Currency; - using CustomRevert for bytes4; - /// @notice Thrown when a native transfer fails - /// @param recipient address that the transfer failed to - /// @param revertReason bubbled up revert reason - error Wrap__NativeTransferFailed(address recipient, bytes revertReason); + /// @notice Additional context for ERC-7751 wrapped error when a native transfer fails + error NativeTransferFailed(); - /// @notice Thrown when an ERC20 transfer fails - /// @param token address of the ERC20 token - /// @param revertReason bubbled up revert reason - error Wrap__ERC20TransferFailed(address token, bytes revertReason); + /// @notice Additional context for ERC-7751 wrapped error when an ERC20 transfer fails + error ERC20TransferFailed(); /// @notice A constant to represent the native currency Currency public constant NATIVE = Currency.wrap(address(0)); @@ -55,7 +50,7 @@ library CurrencyLibrary { success := call(gas(), to, amount, 0, 0, 0, 0) } // revert with NativeTransferFailed, containing the bubbled up error as an argument - if (!success) Wrap__NativeTransferFailed.selector.bubbleUpAndRevertWith(to); + if (!success) CustomRevert.bubbleUpAndRevertWith(to, bytes4(0), NativeTransferFailed.selector); } else { assembly ("memory-safe") { // Get a pointer to some free memory. @@ -84,7 +79,11 @@ library CurrencyLibrary { mstore(add(fmp, 0x40), 0) // 4 bytes of `amount` were stored here } // revert with ERC20TransferFailed, containing the bubbled up error as an argument - if (!success) Wrap__ERC20TransferFailed.selector.bubbleUpAndRevertWith(Currency.unwrap(currency)); + if (!success) { + CustomRevert.bubbleUpAndRevertWith( + Currency.unwrap(currency), IERC20Minimal.transfer.selector, ERC20TransferFailed.selector + ); + } } } diff --git a/test/libraries/Currency.t.sol b/test/libraries/Currency.t.sol index 904d3532..7e8fa89b 100644 --- a/test/libraries/Currency.t.sol +++ b/test/libraries/Currency.t.sol @@ -2,11 +2,12 @@ pragma solidity ^0.8.24; import {Test} from "forge-std/Test.sol"; -import {MockERC20} from "solmate/src/test/utils/mocks/MockERC20.sol"; +import {MockERC20, ERC20} from "solmate/src/test/utils/mocks/MockERC20.sol"; import {Currency, CurrencyLibrary} from "../../src/types/Currency.sol"; import {TokenRejecter} from "../helpers/TokenRejecter.sol"; import {TokenSender} from "../helpers/TokenSender.sol"; import {stdError} from "forge-std/StdError.sol"; +import {CustomRevert} from "../../src/libraries/CustomRevert.sol"; contract TestCurrency is Test { using CurrencyLibrary for uint256; @@ -110,7 +111,13 @@ contract TestCurrency is Test { deal(address(sender), 10 ether); vm.expectRevert( - abi.encodeWithSelector(CurrencyLibrary.Wrap__NativeTransferFailed.selector, otherAddress, new bytes(0)) + abi.encodeWithSelector( + CustomRevert.WrappedError.selector, + otherAddress, + bytes4(0), + new bytes(0), + abi.encodeWithSelector(CurrencyLibrary.NativeTransferFailed.selector) + ) ); sender.send(nativeCurrency, otherAddress, 10 ether + 1); } @@ -121,7 +128,11 @@ contract TestCurrency is Test { vm.expectRevert( abi.encodeWithSelector( - CurrencyLibrary.Wrap__ERC20TransferFailed.selector, erc20Currency, stdError.arithmeticError + CustomRevert.WrappedError.selector, + erc20Currency, + ERC20.transfer.selector, + stdError.arithmeticError, + abi.encodeWithSelector(CurrencyLibrary.ERC20TransferFailed.selector) ) ); sender.send(erc20Currency, otherAddress, 101); diff --git a/test/pool-bin/BinHookRevertWithReason.t.sol b/test/pool-bin/BinHookRevertWithReason.t.sol index c33c1f79..b973cdbc 100644 --- a/test/pool-bin/BinHookRevertWithReason.t.sol +++ b/test/pool-bin/BinHookRevertWithReason.t.sol @@ -11,6 +11,8 @@ import {BinPoolParametersHelper} from "../../src/pool-bin/libraries/BinPoolParam import {Hooks} from "../../src/libraries/Hooks.sol"; import {BinRevertHook} from "./helpers/BinRevertHook.sol"; import {BaseBinTestHook} from "./helpers/BaseBinTestHook.sol"; +import {IBinHooks} from "../../src/pool-bin/interfaces/IBinHooks.sol"; +import {CustomRevert} from "../../src/libraries/CustomRevert.sol"; /// @dev make sure the revert reason is bubbled up contract BinHookRevertWithReasonTest is Test { @@ -52,7 +54,15 @@ contract BinHookRevertWithReasonTest is Test { } function testRevertWithNoReason() public { - vm.expectRevert(abi.encodeWithSelector(Hooks.Wrap__FailedHookCall.selector, hook, new bytes(0))); + vm.expectRevert( + abi.encodeWithSelector( + CustomRevert.WrappedError.selector, + hook, + IBinHooks.afterInitialize.selector, + new bytes(0), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) + ) + ); poolManager.initialize(key, activeId); } @@ -60,9 +70,11 @@ contract BinHookRevertWithReasonTest is Test { hook.setRevertWithHookNotImplemented(true); vm.expectRevert( abi.encodeWithSelector( - Hooks.Wrap__FailedHookCall.selector, + CustomRevert.WrappedError.selector, hook, - abi.encodeWithSelector(BaseBinTestHook.HookNotImplemented.selector) + IBinHooks.afterInitialize.selector, + abi.encodeWithSelector(BaseBinTestHook.HookNotImplemented.selector), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) ) ); poolManager.initialize(key, activeId); diff --git a/test/pool-bin/libraries/BinPoolFee.t.sol b/test/pool-bin/libraries/BinPoolFee.t.sol index 186c120a..4e75903e 100644 --- a/test/pool-bin/libraries/BinPoolFee.t.sol +++ b/test/pool-bin/libraries/BinPoolFee.t.sol @@ -28,6 +28,8 @@ import {BinFeeManagerHook} from "../helpers/BinFeeManagerHook.sol"; import {HOOKS_AFTER_INITIALIZE_OFFSET, HOOKS_BEFORE_MINT_OFFSET} from "../../../src/pool-bin/interfaces/IBinHooks.sol"; import {Hooks} from "../../../src/libraries/Hooks.sol"; import {SortTokens} from "../../helpers/SortTokens.sol"; +import {CustomRevert} from "../../../src/libraries/CustomRevert.sol"; +import {IBinHooks} from "../../../src/pool-bin/interfaces/IBinHooks.sol"; /** * @dev tests around fee for mint(), swap() and burn() @@ -172,9 +174,11 @@ contract BinPoolFeeTest is BinTestHelper { bytes memory data = abi.encode(true, uint24(swapFee)); vm.expectRevert( abi.encodeWithSelector( - Hooks.Wrap__FailedHookCall.selector, + CustomRevert.WrappedError.selector, binFeeManagerHook, - abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, uint24(swapFee)) + IBinHooks.beforeMint.selector, + abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, uint24(swapFee)), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) ) ); addLiquidityToBin(key, poolManager, bob, activeId, 10_000 ether, 10_000 ether, 1e18, 1e18, data); @@ -416,9 +420,11 @@ contract BinPoolFeeTest is BinTestHelper { bytes memory data = abi.encode(true, uint24(swapFee)); vm.expectRevert( abi.encodeWithSelector( - Hooks.Wrap__FailedHookCall.selector, + CustomRevert.WrappedError.selector, binFeeManagerHook, - abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, uint24(swapFee)) + IBinHooks.beforeSwap.selector, + abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, uint24(swapFee)), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) ) ); poolManager.swap(key, true, 1e18, data); diff --git a/test/pool-cl/CLHookRevertWithReason.t.sol b/test/pool-cl/CLHookRevertWithReason.t.sol index 001c5c0e..f2ef59f2 100644 --- a/test/pool-cl/CLHookRevertWithReason.t.sol +++ b/test/pool-cl/CLHookRevertWithReason.t.sol @@ -19,6 +19,8 @@ import {TokenFixture} from "../helpers/TokenFixture.sol"; import {CLRevertHook} from "./helpers/CLRevertHook.sol"; import {CLPoolParametersHelper} from "../../src/pool-cl/libraries/CLPoolParametersHelper.sol"; import {BaseCLTestHook} from "./helpers/BaseCLTestHook.sol"; +import {CustomRevert} from "../../src/libraries/CustomRevert.sol"; +import {ICLHooks} from "../../src/pool-cl/interfaces/ICLHooks.sol"; /// @dev make sure the revert reason is bubbled up contract CLHookRevertWithReasonTest is Test, Deployers, TokenFixture { @@ -45,7 +47,15 @@ contract CLHookRevertWithReasonTest is Test, Deployers, TokenFixture { } function testRevertWithNoReason() public { - vm.expectRevert(abi.encodeWithSelector(Hooks.Wrap__FailedHookCall.selector, hook, new bytes(0))); + vm.expectRevert( + abi.encodeWithSelector( + CustomRevert.WrappedError.selector, + hook, + ICLHooks.afterInitialize.selector, + new bytes(0), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) + ) + ); poolManager.initialize(key, SQRT_RATIO_1_1); } @@ -53,9 +63,11 @@ contract CLHookRevertWithReasonTest is Test, Deployers, TokenFixture { hook.setRevertWithHookNotImplemented(true); vm.expectRevert( abi.encodeWithSelector( - Hooks.Wrap__FailedHookCall.selector, + CustomRevert.WrappedError.selector, hook, - abi.encodeWithSelector(BaseCLTestHook.HookNotImplemented.selector) + ICLHooks.afterInitialize.selector, + abi.encodeWithSelector(BaseCLTestHook.HookNotImplemented.selector), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) ) ); poolManager.initialize(key, SQRT_RATIO_1_1); diff --git a/test/pool-cl/CLPoolManager.t.sol b/test/pool-cl/CLPoolManager.t.sol index 28843454..7c3b856a 100644 --- a/test/pool-cl/CLPoolManager.t.sol +++ b/test/pool-cl/CLPoolManager.t.sol @@ -39,6 +39,8 @@ import {NoIsolate} from "../helpers/NoIsolate.sol"; import {Pausable} from "@openzeppelin/contracts/utils/Pausable.sol"; import {CLPoolGetter} from "./helpers/CLPoolGetter.sol"; import {CLSlot0} from "../../src/pool-cl/types/CLSlot0.sol"; +import {CustomRevert} from "../../src/libraries/CustomRevert.sol"; +import {ICLHooks} from "../../src/pool-cl/interfaces/ICLHooks.sol"; contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnapshot { using CLPoolParametersHelper for bytes32; @@ -731,9 +733,11 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps clFeeManagerHook.setFee(dynamicSwapFee); vm.expectRevert( abi.encodeWithSelector( - Hooks.Wrap__FailedHookCall.selector, + CustomRevert.WrappedError.selector, clFeeManagerHook, - abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, dynamicSwapFee) + ICLHooks.afterInitialize.selector, + abi.encodeWithSelector(LPFeeLibrary.LPFeeTooLarge.selector, dynamicSwapFee), + abi.encodeWithSelector(Hooks.HookCallFailed.selector) ) ); poolManager.initialize(key, SQRT_RATIO_1_1); diff --git a/test/vault/Vault.t.sol b/test/vault/Vault.t.sol index 5fb8bcf9..6a573de2 100644 --- a/test/vault/Vault.t.sol +++ b/test/vault/Vault.t.sol @@ -467,7 +467,7 @@ contract VaultTest is Test, NoIsolate, GasSnapshot, TokenFixture { function test_CollectFee() public noIsolate { vault.lock(abi.encodeCall(VaultTest._test_CollectFee, ())); - // collectFee must be called when vault is unlocked + // collectFee can be called no matter vault is locked or not vm.prank(address(poolManager1)); vault.collectFee(currency0, 10 ether, address(poolManager1)); @@ -488,6 +488,30 @@ contract VaultTest is Test, NoIsolate, GasSnapshot, TokenFixture { assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(poolManager1)), 0 ether); } + function test_CollectFee_WhenVaultLocked() public noIsolate { + vault.lock(abi.encodeCall(VaultTest._test_CollectFee_WhenVaultLocked, ())); + } + + function _test_CollectFee_WhenVaultLocked() external { + currency0.settle(vault, address(this), 10 ether, false); + currency1.settle(vault, address(this), 10 ether, false); + poolManager1.mockAccounting(poolKey1, -10 ether, -10 ether); + + // before collectFee assert + assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(vault)), 10 ether); + assertEq(vault.reservesOfApp(address(poolKey1.poolManager), currency0), 10 ether); + assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(poolManager1)), 0 ether); + + // collectFee can be called no matter vault is locked or not + vm.prank(address(poolManager1)); + vault.collectFee(currency0, 10 ether, address(poolManager1)); + + // after collectFee assert + assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(vault)), 0 ether); + assertEq(vault.reservesOfApp(address(poolKey1.poolManager), currency0), 0 ether); + assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(poolManager1)), 10 ether); + } + function test_CollectFeeFromRandomUser() public { address bob = makeAddr("bob"); vm.startPrank(bob); @@ -496,12 +520,13 @@ contract VaultTest is Test, NoIsolate, GasSnapshot, TokenFixture { vault.collectFee(currency0, 10 ether, bob); } - function test_CollectFeeWhenVaultIsLocked() public noIsolate { - vm.expectRevert(IVault.LockHeld.selector); - vault.lock(abi.encodeCall(VaultTest._test_CollectFeeWhenVaultIsLocked, ())); + function test_CollectFeeWhenCurrencyIsSynced() public noIsolate { + vm.expectRevert(IVault.FeeCurrencySynced.selector); + vault.lock(abi.encodeCall(VaultTest._test_CollectFeeWhenCurrencyIsSynced, ())); } - function _test_CollectFeeWhenVaultIsLocked() external { + function _test_CollectFeeWhenCurrencyIsSynced() external { + vault.sync(currency0); vm.prank(address(poolManager1)); vault.collectFee(currency0, 10 ether, address(poolManager1)); } diff --git a/test/vault/VaultSync.t.sol b/test/vault/VaultSync.t.sol index 2c5aef1e..216eb3f9 100644 --- a/test/vault/VaultSync.t.sol +++ b/test/vault/VaultSync.t.sol @@ -124,8 +124,8 @@ contract VaultSyncTest is Test, TokenFixture, GasSnapshot, NoIsolate { assertEq(amount, 10 ether); } - function test_sync_NoLock() public { - vm.expectRevert(abi.encodeWithSelector(IVault.NoLocker.selector)); + function test_sync() public { + // it's ok to sync without lock vault.sync(currency0); }