Skip to content
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

Feat/aduit part7 #195

Merged
merged 4 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
178162
178172
Original file line number Diff line number Diff line change
@@ -1 +1 @@
188425
188435
Original file line number Diff line number Diff line change
@@ -1 +1 @@
184048
184058
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
311216
311226
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
189420
189430
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerBytecodeSize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23951
24175
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasDonate.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118763
118513
Original file line number Diff line number Diff line change
@@ -1 +1 @@
968494
968244
Original file line number Diff line number Diff line change
@@ -1 +1 @@
327806
327556
Original file line number Diff line number Diff line change
@@ -1 +1 @@
337530
337280
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140081
139831
Original file line number Diff line number Diff line change
@@ -1 +1 @@
173046
172918
Original file line number Diff line number Diff line change
@@ -1 +1 @@
179074
178946
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133077
132949
Original file line number Diff line number Diff line change
@@ -1 +1 @@
304488
304363
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerBytecodeSize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20902
21103
Original file line number Diff line number Diff line change
@@ -1 +1 @@
347609
347359
Original file line number Diff line number Diff line change
@@ -1 +1 @@
163070
162820
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#donateBothTokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
163350
163100
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
108297
108172
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130864
130739
Original file line number Diff line number Diff line change
@@ -1 +1 @@
163225
163100
Original file line number Diff line number Diff line change
@@ -1 +1 @@
148644
148519
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_withHooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
87398
87408
2 changes: 1 addition & 1 deletion .forge-snapshots/VaultBytecodeSize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7651
7792
6 changes: 4 additions & 2 deletions src/Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/IVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
51 changes: 37 additions & 14 deletions src/libraries/CustomRevert.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}
9 changes: 3 additions & 6 deletions src/libraries/Hooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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") {
Expand Down
21 changes: 10 additions & 11 deletions src/types/Currency.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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.
Expand Down Expand Up @@ -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
);
}
}
}

Expand Down
17 changes: 14 additions & 3 deletions test/libraries/Currency.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
18 changes: 15 additions & 3 deletions test/pool-bin/BinHookRevertWithReason.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading