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 <[email protected]>

.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap

@@ -1 +1 @@
-178162
+178172

.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap

@@ -1 +1 @@
-188425
+188435

.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap

@@ -1 +1 @@
-184048
+184058

.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap

@@ -1 +1 @@
-311216
+311226

.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap

@@ -1 +1 @@
-189420
+189430

.forge-snapshots/BinPoolManagerBytecodeSize.snap

@@ -1 +1 @@
-23951
+24175

.forge-snapshots/BinPoolManagerTest#testGasDonate.snap

@@ -1 +1 @@
-118763
+118513

.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap

@@ -1 +1 @@
-968494
+968244

.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap

@@ -1 +1 @@
-327806
+327556

.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap

@@ -1 +1 @@
-337530
+337280

.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap

@@ -1 +1 @@
-140081
+139831

.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap

@@ -1 +1 @@
-173046
+172918

.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap

@@ -1 +1 @@
-179074
+178946

.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap

@@ -1 +1 @@
-133077
+132949

.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap

@@ -1 +1 @@
-304488
+304363

.forge-snapshots/CLPoolManagerBytecodeSize.snap

@@ -1 +1 @@
-20902
+21103

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap

@@ -1 +1 @@
-347609
+347359

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap

@@ -1 +1 @@
-163070
+162820

.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap

@@ -1 +1 @@
-163350
+163100

.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap

@@ -1 +1 @@
-108297
+108172

.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap

@@ -1 +1 @@
-130864
+130739

.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap

@@ -1 +1 @@
-163225
+163100

.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap

@@ -1 +1 @@
-148644
+148519

.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap

@@ -1 +1 @@
-87398
+87408

.forge-snapshots/VaultBytecodeSize.snap

@@ -1 +1 @@
-7651
+7792

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

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

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))
         }
     }
 }

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") {

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
+                );
+            }
         }
     }
 

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

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,17 +54,27 @@ 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);
     }
 
     function testRevertWithHookNotImplemented() public {
         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);