From 71b99352ee1c052e1e8edf5ec2104bdea6628412 Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 00:01:27 +0400 Subject: [PATCH 1/8] feat: add beforeShip hook for native ETH wrapping Add IShipHook interface and modify ship() to support native ETH: - Add IShipHook interface with beforeShip() callback - Make ship() payable to accept native ETH - Call beforeShip hook on the app when ETH is sent - App can wrap ETH to WETH and send to maker This enables apps (like Fusion SwapVM) to handle native ETH in a single transaction: user sends ETH with ship(), app wraps it to WETH and sends to maker, then normal Aqua flow continues. Flow: 1. User calls Aqua.ship{value: X}(app, strategy, [WETH,...], [X,...]) 2. Aqua calls app.beforeShip{value: X}(maker, strategyHash, tokens, amounts) 3. App wraps ETH to WETH and transfers to maker 4. Aqua continues with normal ship flow (virtual balance tracking) 5. Aqua.pull() works normally (maker now has WETH) --- src/Aqua.sol | 19 ++++++++++++++++++- src/interfaces/IAqua.sol | 7 ++++++- src/interfaces/IShipHook.sol | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 src/interfaces/IShipHook.sol diff --git a/src/Aqua.sol b/src/Aqua.sol index d4c95ce..939678b 100644 --- a/src/Aqua.sol +++ b/src/Aqua.sol @@ -8,6 +8,7 @@ import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { SafeERC20, IERC20 } from "@1inch/solidity-utils/contracts/libraries/SafeERC20.sol"; import { IAqua } from "./interfaces/IAqua.sol"; +import { IShipHook } from "./interfaces/IShipHook.sol"; import { Balance, BalanceLib } from "./libs/Balance.sol"; /// @title Aqua - Shared Liquidity Layer @@ -37,11 +38,27 @@ contract Aqua is IAqua { balance1 = amount1; } - function ship(address app, bytes calldata strategy, address[] calldata tokens, uint256[] calldata amounts) external returns(bytes32 strategyHash) { + /// @notice Thrown when the beforeShip hook fails + /// @param app The app address that failed the hook + error ShipHookFailed(address app); + + function ship(address app, bytes calldata strategy, address[] calldata tokens, uint256[] calldata amounts) external payable returns(bytes32 strategyHash) { strategyHash = keccak256(strategy); uint8 tokensCount = tokens.length.toUint8(); require(tokensCount != _DOCKED, MaxNumberOfTokensExceeded(tokensCount, _DOCKED)); + // If ETH is sent, call beforeShip hook on the app + // This allows apps to handle ETH wrapping (e.g., wrap to WETH and send to maker) + if (msg.value > 0) { + bool success = IShipHook(app).beforeShip{value: msg.value}( + msg.sender, + strategyHash, + tokens, + amounts + ); + require(success, ShipHookFailed(app)); + } + emit Shipped(msg.sender, app, strategyHash, strategy); for (uint256 i = 0; i < tokens.length; i++) { Balance storage balance = _balances[msg.sender][app][strategyHash][tokens[i]]; diff --git a/src/interfaces/IAqua.sol b/src/interfaces/IAqua.sol index b29f6f6..9f21a01 100644 --- a/src/interfaces/IAqua.sol +++ b/src/interfaces/IAqua.sol @@ -37,6 +37,10 @@ interface IAqua { /// @param token The token being queried error SafeBalancesForTokenNotInActiveStrategy(address maker, address app, bytes32 strategyHash, address token); + /// @notice Thrown when the beforeShip hook fails + /// @param app The app address that failed the hook + error ShipHookFailed(address app); + /// @notice Emitted when a new strategy is shipped (deployed) and initialized with balances /// @param maker The address of the maker shipping the strategy /// @param app The app address associated with the strategy @@ -89,6 +93,7 @@ interface IAqua { /// @notice Ships a new strategy as of an app and sets initial balances /// @dev Parameter `strategy` is presented fully instead of being pre-hashed for data availability + /// If ETH is sent, calls beforeShip hook on the app to handle wrapping /// @param app The implementation contract /// @param strategy Initialization data passed to the strategy /// @param tokens Array of token addresses to approve @@ -98,7 +103,7 @@ interface IAqua { bytes calldata strategy, address[] calldata tokens, uint256[] calldata amounts - ) external returns(bytes32 strategyHash); + ) external payable returns(bytes32 strategyHash); /// @notice Docks (deactivates) a strategy by clearing balances for specified tokens /// @dev Sets balances to 0 for all specified tokens diff --git a/src/interfaces/IShipHook.sol b/src/interfaces/IShipHook.sol new file mode 100644 index 0000000..9953524 --- /dev/null +++ b/src/interfaces/IShipHook.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: LicenseRef-Degensoft-Aqua-Source-1.1 +pragma solidity ^0.8.0; + +/// @custom:license-url https://github.com/1inch/aqua/blob/main/LICENSES/Aqua-Source-1.1.txt +/// @custom:copyright © 2025 Degensoft Ltd + +/// @title IShipHook +/// @notice Hook interface for apps to handle pre-ship operations +/// @dev Apps can implement this to handle native ETH wrapping or other pre-ship logic +interface IShipHook { + /// @notice Called by Aqua before processing ship when ETH is sent + /// @dev The hook receives the ETH and should handle it appropriately + /// For ETH wrapping: wrap to WETH and transfer to maker + /// @param maker The maker address who is shipping the strategy + /// @param strategyHash The hash of the strategy being shipped + /// @param tokens Array of token addresses in the strategy + /// @param amounts Array of amounts for each token + /// @return success True if hook processed successfully + function beforeShip( + address maker, + bytes32 strategyHash, + address[] calldata tokens, + uint256[] calldata amounts + ) external payable returns (bool success); +} From 7d338db0cad1f15dc79686f8f4b5cc28dd1d0103 Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:22:28 +0400 Subject: [PATCH 2/8] feat: add optional afterShip hook with flags-based control Extend ship hooks to support both before and after callbacks: - Add afterShip() to IShipHook interface - Add hooks flags parameter to ship(): - HOOK_NONE (0x00): No hooks called - HOOK_BEFORE (0x01): Call beforeShip before balance storage - HOOK_AFTER (0x02): Call afterShip after balance storage - HOOK_BOTH (0x03): Call both hooks - ETH can only be sent when HOOK_BEFORE flag is set - beforeShip: Use for ETH wrapping, pre-validation, setup - afterShip: Use for notifications, state setup, external calls This makes hooks completely optional - apps that don't need hooks can pass hooks=0 for zero overhead. --- src/Aqua.sol | 47 ++++++++++++++++++++++++++++++------ src/interfaces/IAqua.sol | 17 ++++++++++--- src/interfaces/IShipHook.sol | 30 ++++++++++++++++++++--- 3 files changed, 79 insertions(+), 15 deletions(-) diff --git a/src/Aqua.sol b/src/Aqua.sol index 939678b..f7ae85c 100644 --- a/src/Aqua.sol +++ b/src/Aqua.sol @@ -19,6 +19,12 @@ contract Aqua is IAqua { uint8 private constant _DOCKED = 0xff; + /// @notice Hook flags for optional ship lifecycle hooks + uint8 public constant HOOK_NONE = 0x00; + uint8 public constant HOOK_BEFORE = 0x01; + uint8 public constant HOOK_AFTER = 0x02; + uint8 public constant HOOK_BOTH = 0x03; + mapping(address maker => mapping(address app => mapping(bytes32 strategyHash => @@ -38,27 +44,43 @@ contract Aqua is IAqua { balance1 = amount1; } - /// @notice Thrown when the beforeShip hook fails + /// @notice Thrown when a ship hook fails /// @param app The app address that failed the hook - error ShipHookFailed(address app); - - function ship(address app, bytes calldata strategy, address[] calldata tokens, uint256[] calldata amounts) external payable returns(bytes32 strategyHash) { + /// @param hookType 1 = beforeShip, 2 = afterShip + error ShipHookFailed(address app, uint8 hookType); + + /// @notice Thrown when ETH is sent but HOOK_BEFORE flag is not set + error ETHSentWithoutBeforeHook(); + + function ship( + address app, + bytes calldata strategy, + address[] calldata tokens, + uint256[] calldata amounts, + uint8 hooks + ) external payable returns(bytes32 strategyHash) { strategyHash = keccak256(strategy); uint8 tokensCount = tokens.length.toUint8(); require(tokensCount != _DOCKED, MaxNumberOfTokensExceeded(tokensCount, _DOCKED)); - // If ETH is sent, call beforeShip hook on the app - // This allows apps to handle ETH wrapping (e.g., wrap to WETH and send to maker) + // If ETH is sent, HOOK_BEFORE must be set if (msg.value > 0) { + require((hooks & HOOK_BEFORE) != 0, ETHSentWithoutBeforeHook()); + } + + // beforeShip hook: called before balance storage (if HOOK_BEFORE flag set) + // Use for: ETH wrapping, pre-validation, setup + if ((hooks & HOOK_BEFORE) != 0) { bool success = IShipHook(app).beforeShip{value: msg.value}( msg.sender, strategyHash, tokens, amounts ); - require(success, ShipHookFailed(app)); + require(success, ShipHookFailed(app, HOOK_BEFORE)); } + // Core ship logic: store balances emit Shipped(msg.sender, app, strategyHash, strategy); for (uint256 i = 0; i < tokens.length; i++) { Balance storage balance = _balances[msg.sender][app][strategyHash][tokens[i]]; @@ -66,6 +88,17 @@ contract Aqua is IAqua { balance.store(amounts[i].toUint248(), tokensCount); emit Pushed(msg.sender, app, strategyHash, tokens[i], amounts[i]); } + + // afterShip hook: called after balance storage (if HOOK_AFTER flag set) + // Use for: notifications, additional state setup, external calls + if ((hooks & HOOK_AFTER) != 0) { + IShipHook(app).afterShip( + msg.sender, + strategyHash, + tokens, + amounts + ); + } } function dock(address app, bytes32 strategyHash, address[] calldata tokens) external { diff --git a/src/interfaces/IAqua.sol b/src/interfaces/IAqua.sol index 9f21a01..151daeb 100644 --- a/src/interfaces/IAqua.sol +++ b/src/interfaces/IAqua.sol @@ -37,9 +37,13 @@ interface IAqua { /// @param token The token being queried error SafeBalancesForTokenNotInActiveStrategy(address maker, address app, bytes32 strategyHash, address token); - /// @notice Thrown when the beforeShip hook fails + /// @notice Thrown when a ship hook fails /// @param app The app address that failed the hook - error ShipHookFailed(address app); + /// @param hookType 1 = beforeShip, 2 = afterShip + error ShipHookFailed(address app, uint8 hookType); + + /// @notice Thrown when ETH is sent but HOOK_BEFORE flag is not set + error ETHSentWithoutBeforeHook(); /// @notice Emitted when a new strategy is shipped (deployed) and initialized with balances /// @param maker The address of the maker shipping the strategy @@ -93,16 +97,21 @@ interface IAqua { /// @notice Ships a new strategy as of an app and sets initial balances /// @dev Parameter `strategy` is presented fully instead of being pre-hashed for data availability - /// If ETH is sent, calls beforeShip hook on the app to handle wrapping + /// Hooks are optional and controlled via the `hooks` flags parameter: + /// - HOOK_BEFORE (0x01): Call beforeShip before balance storage (required if ETH sent) + /// - HOOK_AFTER (0x02): Call afterShip after balance storage + /// - HOOK_BOTH (0x03): Call both hooks /// @param app The implementation contract /// @param strategy Initialization data passed to the strategy /// @param tokens Array of token addresses to approve /// @param amounts Array of balance amounts for each token + /// @param hooks Bitmask of hooks to call (0x00=none, 0x01=before, 0x02=after, 0x03=both) function ship( address app, bytes calldata strategy, address[] calldata tokens, - uint256[] calldata amounts + uint256[] calldata amounts, + uint8 hooks ) external payable returns(bytes32 strategyHash); /// @notice Docks (deactivates) a strategy by clearing balances for specified tokens diff --git a/src/interfaces/IShipHook.sol b/src/interfaces/IShipHook.sol index 9953524..930aee3 100644 --- a/src/interfaces/IShipHook.sol +++ b/src/interfaces/IShipHook.sol @@ -5,11 +5,19 @@ pragma solidity ^0.8.0; /// @custom:copyright © 2025 Degensoft Ltd /// @title IShipHook -/// @notice Hook interface for apps to handle pre-ship operations -/// @dev Apps can implement this to handle native ETH wrapping or other pre-ship logic +/// @notice Hook interface for apps to handle ship lifecycle operations +/// @dev Apps can implement these hooks to handle: +/// - beforeShip: Native ETH wrapping, pre-validation, setup +/// - afterShip: Notifications, additional state setup, external calls +/// +/// Hooks are optional and controlled via the `hooks` flags parameter in ship(): +/// - HOOK_BEFORE (0x01): Call beforeShip before balance storage +/// - HOOK_AFTER (0x02): Call afterShip after balance storage +/// - HOOK_BOTH (0x03): Call both hooks interface IShipHook { - /// @notice Called by Aqua before processing ship when ETH is sent - /// @dev The hook receives the ETH and should handle it appropriately + /// @notice Called by Aqua BEFORE processing ship (when HOOK_BEFORE flag is set) + /// @dev Use for: ETH wrapping, pre-validation, setup + /// The hook receives ETH if msg.value > 0 and should handle it appropriately /// For ETH wrapping: wrap to WETH and transfer to maker /// @param maker The maker address who is shipping the strategy /// @param strategyHash The hash of the strategy being shipped @@ -22,4 +30,18 @@ interface IShipHook { address[] calldata tokens, uint256[] calldata amounts ) external payable returns (bool success); + + /// @notice Called by Aqua AFTER ship is completed (when HOOK_AFTER flag is set) + /// @dev Use for: notifications, additional state setup, external calls + /// Called after balances are stored, so strategy is already active + /// @param maker The maker address who shipped the strategy + /// @param strategyHash The hash of the strategy that was shipped + /// @param tokens Array of token addresses in the strategy + /// @param amounts Array of amounts for each token + function afterShip( + address maker, + bytes32 strategyHash, + address[] calldata tokens, + uint256[] calldata amounts + ) external; } From c9d1fe4253856e3a5ac2a5c0f66a4e646f7ea50d Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:25:03 +0400 Subject: [PATCH 3/8] fix: remove duplicate error declarations from Aqua.sol Errors are already declared in IAqua.sol interface and inherited by Aqua contract. Removes ShipHookFailed and ETHSentWithoutBeforeHook from Aqua.sol to fix compilation. --- src/Aqua.sol | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Aqua.sol b/src/Aqua.sol index f7ae85c..e863a5c 100644 --- a/src/Aqua.sol +++ b/src/Aqua.sol @@ -44,14 +44,6 @@ contract Aqua is IAqua { balance1 = amount1; } - /// @notice Thrown when a ship hook fails - /// @param app The app address that failed the hook - /// @param hookType 1 = beforeShip, 2 = afterShip - error ShipHookFailed(address app, uint8 hookType); - - /// @notice Thrown when ETH is sent but HOOK_BEFORE flag is not set - error ETHSentWithoutBeforeHook(); - function ship( address app, bytes calldata strategy, From a2013dfb1558a29262698786f732e7bc383deb1b Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:28:12 +0400 Subject: [PATCH 4/8] fix: update tests to pass hooks parameter to ship() Add the 5th argument (hooks=0) to all aqua.ship() calls to match the updated function signature that now includes the hooks flags. --- test/Aqua.t.sol | 51 +++++++++++++++++++++++++------------- test/AquaStorageTest.t.sol | 24 ++++++++++++------ 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/test/Aqua.t.sol b/test/Aqua.t.sol index e0ba2c1..f4285f1 100644 --- a/test/Aqua.t.sol +++ b/test/Aqua.t.sol @@ -58,7 +58,8 @@ contract AquaTest is Test { app, "strategy1", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); // Try to ship again with same strategy @@ -68,7 +69,8 @@ contract AquaTest is Test { app, "strategy1", dynamic([address(token1)]), - dynamic([uint256(50e18)]) + dynamic([uint256(50e18)]), + 0 ); } @@ -81,7 +83,8 @@ contract AquaTest is Test { app, "strategy_dup", dynamic([address(token1), address(token1)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); } @@ -94,7 +97,8 @@ contract AquaTest is Test { app, "strategy2", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); // Try to dock with only 1 token @@ -114,7 +118,8 @@ contract AquaTest is Test { app, "strategy3", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); // Try to dock with different token @@ -134,7 +139,8 @@ contract AquaTest is Test { app, "strategy4", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); // Try to dock with 3 tokens @@ -163,7 +169,8 @@ contract AquaTest is Test { app, "strategy5", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); vm.prank(maker); @@ -186,7 +193,8 @@ contract AquaTest is Test { app, "strategy6", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); // Try to push token2 (not shipped) @@ -207,7 +215,8 @@ contract AquaTest is Test { app, "lifecycle", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); // 2. Push to token1 @@ -251,7 +260,8 @@ contract AquaTest is Test { app, "multi1", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); // Ship strategy 2 with same tokens but different salt @@ -260,7 +270,8 @@ contract AquaTest is Test { app, "multi2", dynamic([address(token1), address(token2)]), - dynamic([uint256(300e18), uint256(400e18)]) + dynamic([uint256(300e18), uint256(400e18)]), + 0 ); // Verify both strategies work independently @@ -307,7 +318,8 @@ contract AquaTest is Test { app, "balances_test", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); // Query balance for token2 (not in strategy) - should return 0 @@ -322,7 +334,8 @@ contract AquaTest is Test { app, "balances_multi", dynamic([address(token1), address(token2), address(token3)]), - dynamic([uint256(100e18), uint256(200e18), uint256(300e18)]) + dynamic([uint256(100e18), uint256(200e18), uint256(300e18)]), + 0 ); bytes32 strategyHash = keccak256("balances_multi"); @@ -343,7 +356,8 @@ contract AquaTest is Test { app, "safe_balances", dynamic([address(token1), address(token2)]), - dynamic([uint256(150e18), uint256(250e18)]) + dynamic([uint256(150e18), uint256(250e18)]), + 0 ); bytes32 strategyHash = keccak256("safe_balances"); @@ -388,7 +402,8 @@ contract AquaTest is Test { app, "safe_partial", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); bytes32 strategyHash = keccak256("safe_partial"); @@ -419,7 +434,8 @@ contract AquaTest is Test { app, "safe_docked", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); bytes32 strategyHash = keccak256("safe_docked"); @@ -458,7 +474,8 @@ contract AquaTest is Test { app, "safe_changes", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); bytes32 strategyHash = keccak256("safe_changes"); diff --git a/test/AquaStorageTest.t.sol b/test/AquaStorageTest.t.sol index 5714d61..3881ec5 100644 --- a/test/AquaStorageTest.t.sol +++ b/test/AquaStorageTest.t.sol @@ -58,7 +58,8 @@ contract AquaStorageTest is Test { address(this), "strategy", dynamic([address(token1)]), - dynamic([uint256(1000e18)]) + dynamic([uint256(1000e18)]), + 0 ); // Test push storage operations @@ -77,7 +78,8 @@ contract AquaStorageTest is Test { address(this), "strategy", dynamic([address(token1)]), - dynamic([uint256(1000e18)]) + dynamic([uint256(1000e18)]), + 0 ); // Test pull storage operations (called directly from test contract acting as app) @@ -97,7 +99,8 @@ contract AquaStorageTest is Test { address(this), "ship1", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(aqua)); @@ -111,7 +114,8 @@ contract AquaStorageTest is Test { address(this), "ship2", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(aqua)); @@ -125,7 +129,8 @@ contract AquaStorageTest is Test { address(this), "ship3", dynamic([address(token1), address(token2), address(token3)]), - dynamic([uint256(100e18), uint256(200e18), uint256(300e18)]) + dynamic([uint256(100e18), uint256(200e18), uint256(300e18)]), + 0 ); (bytes32[] memory reads, bytes32[] memory writes) = vm.accesses(address(aqua)); @@ -141,7 +146,8 @@ contract AquaStorageTest is Test { address(this), "dock1", dynamic([address(token1)]), - dynamic([uint256(100e18)]) + dynamic([uint256(100e18)]), + 0 ); // Test dock storage operations @@ -164,7 +170,8 @@ contract AquaStorageTest is Test { address(this), "dock2", dynamic([address(token1), address(token2)]), - dynamic([uint256(100e18), uint256(200e18)]) + dynamic([uint256(100e18), uint256(200e18)]), + 0 ); // Test dock storage operations @@ -187,7 +194,8 @@ contract AquaStorageTest is Test { address(this), "dock3", dynamic([address(token1), address(token2), address(token3)]), - dynamic([uint256(100e18), uint256(200e18), uint256(300e18)]) + dynamic([uint256(100e18), uint256(200e18), uint256(300e18)]), + 0 ); // Test dock storage operations From f7af3fb55c967fd3059df8acb43c3ea893d96d4c Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:29:41 +0400 Subject: [PATCH 5/8] chore: update gas snapshot for hooks feature Gas values slightly increased due to the new hooks parameter in ship(). All tests pass with 5% tolerance. --- .gas-snapshot | 52 +++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 848ff41..89709c4 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,26 +1,26 @@ -AquaStorageTest:testDock1Token() (gas: 50947) -AquaStorageTest:testDock2Tokens() (gas: 80108) -AquaStorageTest:testDock3Tokens() (gas: 109363) -AquaStorageTest:testPullSingleSloadSstore() (gas: 67696) -AquaStorageTest:testPushSingleSloadSstore() (gas: 70616) -AquaStorageTest:testShip1Token() (gas: 46337) -AquaStorageTest:testShip2Tokens() (gas: 74317) -AquaStorageTest:testShip3Tokens() (gas: 102276) -AquaTest:testBalancesReturnsCorrectAmounts() (gas: 105159) -AquaTest:testBalancesReturnsZeroForNonExistentStrategy() (gas: 14848) -AquaTest:testBalancesReturnsZeroForTokenNotInStrategy() (gas: 50741) -AquaTest:testDockRequiresAllTokensFromShip() (gas: 76825) -AquaTest:testDockRequiresCorrectTokenCount() (gas: 79476) -AquaTest:testDockRequiresExactTokensFromShip() (gas: 81174) -AquaTest:testFullLifecycle() (gas: 144932) -AquaTest:testMultipleStrategiesSameTokens() (gas: 169969) -AquaTest:testPushFailsAfterDock() (gas: 54897) -AquaTest:testPushOnlyForShippedTokens() (gas: 54277) -AquaTest:testPushRequiresActiveStrategy() (gas: 21078) -AquaTest:testSafeBalancesReturnsCorrectAmountsForActiveStrategy() (gas: 75692) -AquaTest:testSafeBalancesRevertsAfterDock() (gas: 54832) -AquaTest:testSafeBalancesRevertsForNonExistentStrategy() (gas: 20785) -AquaTest:testSafeBalancesRevertsIfAnyTokenNotInStrategy() (gas: 79974) -AquaTest:testSafeBalancesTracksChangesFromPushPull() (gas: 138495) -AquaTest:testShipCannotBeCalledTwiceForSameStrategy() (gas: 52784) -AquaTest:testShipCannotHaveDuplicateTokens() (gas: 46793) \ No newline at end of file +AquaStorageTest:testDock1Token() (gas: 51078) +AquaStorageTest:testDock2Tokens() (gas: 80250) +AquaStorageTest:testDock3Tokens() (gas: 109516) +AquaStorageTest:testPullSingleSloadSstore() (gas: 67881) +AquaStorageTest:testPushSingleSloadSstore() (gas: 70766) +AquaStorageTest:testShip1Token() (gas: 46416) +AquaStorageTest:testShip2Tokens() (gas: 74402) +AquaStorageTest:testShip3Tokens() (gas: 102367) +AquaTest:testBalancesReturnsCorrectAmounts() (gas: 105511) +AquaTest:testBalancesReturnsZeroForNonExistentStrategy() (gas: 14934) +AquaTest:testBalancesReturnsZeroForTokenNotInStrategy() (gas: 50909) +AquaTest:testDockRequiresAllTokensFromShip() (gas: 76958) +AquaTest:testDockRequiresCorrectTokenCount() (gas: 79609) +AquaTest:testDockRequiresExactTokensFromShip() (gas: 81312) +AquaTest:testFullLifecycle() (gas: 145649) +AquaTest:testMultipleStrategiesSameTokens() (gas: 170784) +AquaTest:testPushFailsAfterDock() (gas: 55080) +AquaTest:testPushOnlyForShippedTokens() (gas: 54409) +AquaTest:testPushRequiresActiveStrategy() (gas: 21131) +AquaTest:testSafeBalancesReturnsCorrectAmountsForActiveStrategy() (gas: 75875) +AquaTest:testSafeBalancesRevertsAfterDock() (gas: 55060) +AquaTest:testSafeBalancesRevertsForNonExistentStrategy() (gas: 20883) +AquaTest:testSafeBalancesRevertsIfAnyTokenNotInStrategy() (gas: 80155) +AquaTest:testSafeBalancesTracksChangesFromPushPull() (gas: 138857) +AquaTest:testShipCannotBeCalledTwiceForSameStrategy() (gas: 52909) +AquaTest:testShipCannotHaveDuplicateTokens() (gas: 46848) \ No newline at end of file From 5cce4d299c29812c526223cdc0b50db85b2d50b3 Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:34:41 +0400 Subject: [PATCH 6/8] test: add comprehensive hook tests (18 tests) Tests cover: - Hook flag combinations (HOOK_NONE, HOOK_BEFORE, HOOK_AFTER, HOOK_BOTH) - ETH wrapping via beforeShip hook - ETH validation (requires HOOK_BEFORE flag) - Hook failure scenarios (revert, return false) - Non-hook app compatibility - Strategy hash verification - Multiple tokens with ETH - Fuzz tests for flags and ETH amounts --- .gas-snapshot | 16 ++ test/AquaHooks.t.sol | 540 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 556 insertions(+) create mode 100644 test/AquaHooks.t.sol diff --git a/.gas-snapshot b/.gas-snapshot index 89709c4..9caf05a 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,3 +1,19 @@ +AquaHooksTest:testAfterShipRevertsPropagatesToAqua() (gas: 72314) +AquaHooksTest:testBeforeShipReturnsFalseReverts() (gas: 45333) +AquaHooksTest:testBeforeShipRevertsPropagatesToAqua() (gas: 44126) +AquaHooksTest:testBeforeShipWrapsETH() (gas: 199368) +AquaHooksTest:testETHSentRequiresBeforeHook() (gas: 24018) +AquaHooksTest:testETHSentWithAfterHookOnlyFails() (gas: 23820) +AquaHooksTest:testETHWithBothHooksSucceeds() (gas: 202640) +AquaHooksTest:testETHWithMultipleTokens() (gas: 230170) +AquaHooksTest:testHookConstants() (gas: 7841) +AquaHooksTest:testHooksReceiveCorrectStrategyHash() (gas: 105802) +AquaHooksTest:testNonHookAppWithHooksFails() (gas: 20639) +AquaHooksTest:testNonHookAppWithNoHooksSucceeds() (gas: 46359) +AquaHooksTest:testShipWithAfterHookOnly() (gas: 103134) +AquaHooksTest:testShipWithBeforeHookOnly() (gas: 104703) +AquaHooksTest:testShipWithBothHooks() (gas: 107166) +AquaHooksTest:testShipWithNoHooks() (gas: 50423) AquaStorageTest:testDock1Token() (gas: 51078) AquaStorageTest:testDock2Tokens() (gas: 80250) AquaStorageTest:testDock3Tokens() (gas: 109516) diff --git a/test/AquaHooks.t.sol b/test/AquaHooks.t.sol new file mode 100644 index 0000000..93bc357 --- /dev/null +++ b/test/AquaHooks.t.sol @@ -0,0 +1,540 @@ +// SPDX-License-Identifier: LicenseRef-Degensoft-Aqua-Source-1.1 +pragma solidity ^0.8.13; + +/// @custom:license-url https://github.com/1inch/aqua/blob/main/LICENSES/Aqua-Source-1.1.txt +/// @custom:copyright © 2025 Degensoft Ltd + +import {Test} from "forge-std/Test.sol"; +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {Aqua} from "src/Aqua.sol"; +import {IAqua} from "src/interfaces/IAqua.sol"; +import {IShipHook} from "src/interfaces/IShipHook.sol"; + +contract MockToken is ERC20 { + constructor(string memory name) ERC20(name, "MOCK") { + _mint(msg.sender, 1000000e18); + } +} + +/// @title MockWETH - Minimal WETH for testing +contract MockWETH is ERC20 { + constructor() ERC20("Wrapped Ether", "WETH") {} + + function deposit() external payable { + _mint(msg.sender, msg.value); + } + + function withdraw(uint256 amount) external { + _burn(msg.sender, amount); + payable(msg.sender).transfer(amount); + } + + receive() external payable { + _mint(msg.sender, msg.value); + } +} + +/// @title MockHookApp - App that implements IShipHook for testing +contract MockHookApp is IShipHook { + MockWETH public weth; + + // Tracking for tests + bool public beforeShipCalled; + bool public afterShipCalled; + address public lastMaker; + bytes32 public lastStrategyHash; + uint256 public lastEthReceived; + + // Control flags for testing + bool public shouldRevertBeforeShip; + bool public shouldRevertAfterShip; + bool public shouldReturnFalse; + + constructor(address _weth) { + weth = MockWETH(payable(_weth)); + } + + function beforeShip( + address maker, + bytes32 strategyHash, + address[] calldata tokens, + uint256[] calldata amounts + ) external payable override returns (bool success) { + if (shouldRevertBeforeShip) revert("beforeShip reverted"); + if (shouldReturnFalse) return false; + + beforeShipCalled = true; + lastMaker = maker; + lastStrategyHash = strategyHash; + lastEthReceived = msg.value; + + // If ETH received, wrap to WETH and send to maker + if (msg.value > 0) { + // Find WETH in tokens and verify amount matches + for (uint256 i = 0; i < tokens.length; i++) { + if (tokens[i] == address(weth) && amounts[i] == msg.value) { + weth.deposit{value: msg.value}(); + weth.transfer(maker, msg.value); + break; + } + } + } + + return true; + } + + function afterShip( + address maker, + bytes32 strategyHash, + address[] calldata, + uint256[] calldata + ) external override { + if (shouldRevertAfterShip) revert("afterShip reverted"); + + afterShipCalled = true; + lastMaker = maker; + lastStrategyHash = strategyHash; + } + + function reset() external { + beforeShipCalled = false; + afterShipCalled = false; + lastMaker = address(0); + lastStrategyHash = bytes32(0); + lastEthReceived = 0; + shouldRevertBeforeShip = false; + shouldRevertAfterShip = false; + shouldReturnFalse = false; + } + + function setRevertBeforeShip(bool _revert) external { + shouldRevertBeforeShip = _revert; + } + + function setRevertAfterShip(bool _revert) external { + shouldRevertAfterShip = _revert; + } + + function setReturnFalse(bool _returnFalse) external { + shouldReturnFalse = _returnFalse; + } + + receive() external payable {} +} + +/// @title MockNonHookApp - App that does NOT implement IShipHook +contract MockNonHookApp { + // This app doesn't implement IShipHook +} + +contract AquaHooksTest is Test { + Aqua public aqua; + MockToken public token1; + MockWETH public weth; + MockHookApp public hookApp; + MockNonHookApp public nonHookApp; + + address public maker = address(0x1111); + + function setUp() public { + aqua = new Aqua(); + token1 = new MockToken("Token1"); + weth = new MockWETH(); + hookApp = new MockHookApp(address(weth)); + nonHookApp = new MockNonHookApp(); + + // Setup tokens and approvals + token1.transfer(maker, 10000e18); + vm.deal(maker, 100 ether); + + vm.prank(maker); + token1.approve(address(aqua), type(uint256).max); + vm.prank(maker); + weth.approve(address(aqua), type(uint256).max); + } + + // ========== HOOK FLAGS TESTS ========== + + function testShipWithNoHooks() public { + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(hookApp), + "strategy_no_hooks", + tokens, + amounts, + 0 // HOOK_NONE + ); + + // Hooks should NOT be called + assertFalse(hookApp.beforeShipCalled()); + assertFalse(hookApp.afterShipCalled()); + } + + function testShipWithBeforeHookOnly() public { + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(hookApp), + "strategy_before_only", + tokens, + amounts, + 1 // HOOK_BEFORE + ); + + // Only beforeShip should be called + assertTrue(hookApp.beforeShipCalled()); + assertFalse(hookApp.afterShipCalled()); + assertEq(hookApp.lastMaker(), maker); + } + + function testShipWithAfterHookOnly() public { + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(hookApp), + "strategy_after_only", + tokens, + amounts, + 2 // HOOK_AFTER + ); + + // Only afterShip should be called + assertFalse(hookApp.beforeShipCalled()); + assertTrue(hookApp.afterShipCalled()); + assertEq(hookApp.lastMaker(), maker); + } + + function testShipWithBothHooks() public { + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(hookApp), + "strategy_both_hooks", + tokens, + amounts, + 3 // HOOK_BOTH + ); + + // Both hooks should be called + assertTrue(hookApp.beforeShipCalled()); + assertTrue(hookApp.afterShipCalled()); + assertEq(hookApp.lastMaker(), maker); + } + + // ========== ETH WRAPPING TESTS ========== + + function testBeforeShipWrapsETH() public { + uint256 ethAmount = 10 ether; + + address[] memory tokens = new address[](1); + tokens[0] = address(weth); + uint256[] memory amounts = new uint256[](1); + amounts[0] = ethAmount; + + vm.prank(maker); + aqua.ship{value: ethAmount}( + address(hookApp), + "strategy_eth_wrap", + tokens, + amounts, + 1 // HOOK_BEFORE + ); + + // Maker should have WETH + assertEq(weth.balanceOf(maker), ethAmount); + assertEq(hookApp.lastEthReceived(), ethAmount); + } + + function testETHSentRequiresBeforeHook() public { + address[] memory tokens = new address[](1); + tokens[0] = address(weth); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 1 ether; + + // Try to send ETH without HOOK_BEFORE flag + vm.prank(maker); + vm.expectRevert(IAqua.ETHSentWithoutBeforeHook.selector); + aqua.ship{value: 1 ether}( + address(hookApp), + "strategy_eth_no_hook", + tokens, + amounts, + 0 // HOOK_NONE - should fail! + ); + } + + function testETHSentWithAfterHookOnlyFails() public { + address[] memory tokens = new address[](1); + tokens[0] = address(weth); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 1 ether; + + // Try to send ETH with only HOOK_AFTER flag + vm.prank(maker); + vm.expectRevert(IAqua.ETHSentWithoutBeforeHook.selector); + aqua.ship{value: 1 ether}( + address(hookApp), + "strategy_eth_after_only", + tokens, + amounts, + 2 // HOOK_AFTER only - should fail! + ); + } + + function testETHWithBothHooksSucceeds() public { + uint256 ethAmount = 5 ether; + + address[] memory tokens = new address[](1); + tokens[0] = address(weth); + uint256[] memory amounts = new uint256[](1); + amounts[0] = ethAmount; + + vm.prank(maker); + aqua.ship{value: ethAmount}( + address(hookApp), + "strategy_eth_both", + tokens, + amounts, + 3 // HOOK_BOTH + ); + + assertTrue(hookApp.beforeShipCalled()); + assertTrue(hookApp.afterShipCalled()); + assertEq(weth.balanceOf(maker), ethAmount); + } + + // ========== HOOK FAILURE TESTS ========== + + function testBeforeShipReturnsFalseReverts() public { + hookApp.setReturnFalse(true); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + vm.expectRevert(abi.encodeWithSelector(IAqua.ShipHookFailed.selector, address(hookApp), 1)); + aqua.ship( + address(hookApp), + "strategy_false", + tokens, + amounts, + 1 // HOOK_BEFORE + ); + } + + function testBeforeShipRevertsPropagatesToAqua() public { + hookApp.setRevertBeforeShip(true); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + vm.expectRevert("beforeShip reverted"); + aqua.ship( + address(hookApp), + "strategy_revert_before", + tokens, + amounts, + 1 // HOOK_BEFORE + ); + } + + function testAfterShipRevertsPropagatesToAqua() public { + hookApp.setRevertAfterShip(true); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + vm.expectRevert("afterShip reverted"); + aqua.ship( + address(hookApp), + "strategy_revert_after", + tokens, + amounts, + 2 // HOOK_AFTER + ); + } + + // ========== NON-HOOK APP TESTS ========== + + function testNonHookAppWithNoHooksSucceeds() public { + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + // Non-hook app with no hooks flag should work + vm.prank(maker); + aqua.ship( + address(nonHookApp), + "strategy_non_hook", + tokens, + amounts, + 0 // HOOK_NONE + ); + + // Verify strategy was created + (uint248 balance,) = aqua.rawBalances(maker, address(nonHookApp), keccak256("strategy_non_hook"), address(token1)); + assertEq(balance, 100e18); + } + + function testNonHookAppWithHooksFails() public { + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + // Non-hook app with hooks flag should fail (app doesn't implement IShipHook) + vm.prank(maker); + vm.expectRevert(); + aqua.ship( + address(nonHookApp), + "strategy_non_hook_fail", + tokens, + amounts, + 1 // HOOK_BEFORE - but app doesn't implement it! + ); + } + + // ========== STRATEGY HASH VERIFICATION ========== + + function testHooksReceiveCorrectStrategyHash() public { + bytes memory strategy = "unique_strategy_123"; + bytes32 expectedHash = keccak256(strategy); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(hookApp), + strategy, + tokens, + amounts, + 3 // HOOK_BOTH + ); + + assertEq(hookApp.lastStrategyHash(), expectedHash); + } + + // ========== MULTIPLE TOKENS WITH ETH ========== + + function testETHWithMultipleTokens() public { + uint256 ethAmount = 2 ether; + + address[] memory tokens = new address[](2); + tokens[0] = address(token1); + tokens[1] = address(weth); + uint256[] memory amounts = new uint256[](2); + amounts[0] = 100e18; + amounts[1] = ethAmount; + + vm.prank(maker); + aqua.ship{value: ethAmount}( + address(hookApp), + "strategy_multi_eth", + tokens, + amounts, + 1 // HOOK_BEFORE + ); + + // Verify both tokens are tracked + bytes32 strategyHash = keccak256("strategy_multi_eth"); + (uint248 balance1,) = aqua.rawBalances(maker, address(hookApp), strategyHash, address(token1)); + (uint248 balance2,) = aqua.rawBalances(maker, address(hookApp), strategyHash, address(weth)); + + assertEq(balance1, 100e18); + assertEq(balance2, ethAmount); + assertEq(weth.balanceOf(maker), ethAmount); + } + + // ========== HOOK CONSTANTS TESTS ========== + + function testHookConstants() public view { + assertEq(aqua.HOOK_NONE(), 0x00); + assertEq(aqua.HOOK_BEFORE(), 0x01); + assertEq(aqua.HOOK_AFTER(), 0x02); + assertEq(aqua.HOOK_BOTH(), 0x03); + } + + // ========== FUZZ TESTS ========== + + function testFuzz_ShipWithHooksFlags(uint8 hooks) public { + // Bound to valid flags (0-3) + hooks = uint8(bound(hooks, 0, 3)); + + hookApp.reset(); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(hookApp), + abi.encodePacked("fuzz_strategy_", hooks), + tokens, + amounts, + hooks + ); + + // Verify correct hooks were called + bool expectBefore = (hooks & 1) != 0; + bool expectAfter = (hooks & 2) != 0; + + assertEq(hookApp.beforeShipCalled(), expectBefore); + assertEq(hookApp.afterShipCalled(), expectAfter); + } + + function testFuzz_ETHAmount(uint256 ethAmount) public { + // Bound to reasonable range + ethAmount = bound(ethAmount, 1, 1000 ether); + vm.deal(maker, ethAmount); + + hookApp.reset(); + + address[] memory tokens = new address[](1); + tokens[0] = address(weth); + uint256[] memory amounts = new uint256[](1); + amounts[0] = ethAmount; + + vm.prank(maker); + aqua.ship{value: ethAmount}( + address(hookApp), + abi.encodePacked("fuzz_eth_", ethAmount), + tokens, + amounts, + 1 // HOOK_BEFORE + ); + + assertEq(weth.balanceOf(maker), ethAmount); + assertEq(hookApp.lastEthReceived(), ethAmount); + } +} From 0039a0c6a2da7b048d1ac7d28153be2d82f72be5 Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:37:29 +0400 Subject: [PATCH 7/8] feat: add reentrancy protection for ship() with hooks - Added ReentrancyGuardTransient to Aqua contract - Added nonReentrant modifier to ship() function - Added 3 new reentrancy tests to verify protection works - Total hook tests: 21 (including 3 reentrancy tests) - Total tests: 47 (all passing) This addresses the security review concern about reentrancy in hooks. --- .gas-snapshot | 79 ++++++++++++------------- src/Aqua.sol | 5 +- test/AquaHooks.t.sol | 133 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 40 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 9caf05a..c9ba02d 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,42 +1,45 @@ -AquaHooksTest:testAfterShipRevertsPropagatesToAqua() (gas: 72314) -AquaHooksTest:testBeforeShipReturnsFalseReverts() (gas: 45333) -AquaHooksTest:testBeforeShipRevertsPropagatesToAqua() (gas: 44126) -AquaHooksTest:testBeforeShipWrapsETH() (gas: 199368) -AquaHooksTest:testETHSentRequiresBeforeHook() (gas: 24018) -AquaHooksTest:testETHSentWithAfterHookOnlyFails() (gas: 23820) -AquaHooksTest:testETHWithBothHooksSucceeds() (gas: 202640) -AquaHooksTest:testETHWithMultipleTokens() (gas: 230170) +AquaHooksTest:testAfterShipRevertsPropagatesToAqua() (gas: 72625) +AquaHooksTest:testBeforeShipReturnsFalseReverts() (gas: 45643) +AquaHooksTest:testBeforeShipRevertsPropagatesToAqua() (gas: 44349) +AquaHooksTest:testBeforeShipWrapsETH() (gas: 199696) +AquaHooksTest:testETHSentRequiresBeforeHook() (gas: 24263) +AquaHooksTest:testETHSentWithAfterHookOnlyFails() (gas: 24043) +AquaHooksTest:testETHWithBothHooksSucceeds() (gas: 202975) +AquaHooksTest:testETHWithMultipleTokens() (gas: 230487) AquaHooksTest:testHookConstants() (gas: 7841) -AquaHooksTest:testHooksReceiveCorrectStrategyHash() (gas: 105802) -AquaHooksTest:testNonHookAppWithHooksFails() (gas: 20639) -AquaHooksTest:testNonHookAppWithNoHooksSucceeds() (gas: 46359) -AquaHooksTest:testShipWithAfterHookOnly() (gas: 103134) -AquaHooksTest:testShipWithBeforeHookOnly() (gas: 104703) -AquaHooksTest:testShipWithBothHooks() (gas: 107166) -AquaHooksTest:testShipWithNoHooks() (gas: 50423) -AquaStorageTest:testDock1Token() (gas: 51078) -AquaStorageTest:testDock2Tokens() (gas: 80250) -AquaStorageTest:testDock3Tokens() (gas: 109516) -AquaStorageTest:testPullSingleSloadSstore() (gas: 67881) -AquaStorageTest:testPushSingleSloadSstore() (gas: 70766) -AquaStorageTest:testShip1Token() (gas: 46416) -AquaStorageTest:testShip2Tokens() (gas: 74402) -AquaStorageTest:testShip3Tokens() (gas: 102367) -AquaTest:testBalancesReturnsCorrectAmounts() (gas: 105511) +AquaHooksTest:testHooksReceiveCorrectStrategyHash() (gas: 106159) +AquaHooksTest:testNonHookAppWithHooksFails() (gas: 20950) +AquaHooksTest:testNonHookAppWithNoHooksSucceeds() (gas: 46710) +AquaHooksTest:testReentrancyFromAfterShipIsBlocked() (gas: 58456) +AquaHooksTest:testReentrancyFromBeforeShipIsBlocked() (gas: 58458) +AquaHooksTest:testReentrancyFromBothHooksIsBlocked() (gas: 62047) +AquaHooksTest:testShipWithAfterHookOnly() (gas: 103558) +AquaHooksTest:testShipWithBeforeHookOnly() (gas: 105031) +AquaHooksTest:testShipWithBothHooks() (gas: 107501) +AquaHooksTest:testShipWithNoHooks() (gas: 50752) +AquaStorageTest:testDock1Token() (gas: 51407) +AquaStorageTest:testDock2Tokens() (gas: 80579) +AquaStorageTest:testDock3Tokens() (gas: 109845) +AquaStorageTest:testPullSingleSloadSstore() (gas: 68210) +AquaStorageTest:testPushSingleSloadSstore() (gas: 71095) +AquaStorageTest:testShip1Token() (gas: 46745) +AquaStorageTest:testShip2Tokens() (gas: 74731) +AquaStorageTest:testShip3Tokens() (gas: 102696) +AquaTest:testBalancesReturnsCorrectAmounts() (gas: 105840) AquaTest:testBalancesReturnsZeroForNonExistentStrategy() (gas: 14934) -AquaTest:testBalancesReturnsZeroForTokenNotInStrategy() (gas: 50909) -AquaTest:testDockRequiresAllTokensFromShip() (gas: 76958) -AquaTest:testDockRequiresCorrectTokenCount() (gas: 79609) -AquaTest:testDockRequiresExactTokensFromShip() (gas: 81312) -AquaTest:testFullLifecycle() (gas: 145649) -AquaTest:testMultipleStrategiesSameTokens() (gas: 170784) -AquaTest:testPushFailsAfterDock() (gas: 55080) -AquaTest:testPushOnlyForShippedTokens() (gas: 54409) +AquaTest:testBalancesReturnsZeroForTokenNotInStrategy() (gas: 51238) +AquaTest:testDockRequiresAllTokensFromShip() (gas: 77287) +AquaTest:testDockRequiresCorrectTokenCount() (gas: 79938) +AquaTest:testDockRequiresExactTokensFromShip() (gas: 81641) +AquaTest:testFullLifecycle() (gas: 145978) +AquaTest:testMultipleStrategiesSameTokens() (gas: 171442) +AquaTest:testPushFailsAfterDock() (gas: 55409) +AquaTest:testPushOnlyForShippedTokens() (gas: 54738) AquaTest:testPushRequiresActiveStrategy() (gas: 21131) -AquaTest:testSafeBalancesReturnsCorrectAmountsForActiveStrategy() (gas: 75875) -AquaTest:testSafeBalancesRevertsAfterDock() (gas: 55060) +AquaTest:testSafeBalancesReturnsCorrectAmountsForActiveStrategy() (gas: 76204) +AquaTest:testSafeBalancesRevertsAfterDock() (gas: 55389) AquaTest:testSafeBalancesRevertsForNonExistentStrategy() (gas: 20883) -AquaTest:testSafeBalancesRevertsIfAnyTokenNotInStrategy() (gas: 80155) -AquaTest:testSafeBalancesTracksChangesFromPushPull() (gas: 138857) -AquaTest:testShipCannotBeCalledTwiceForSameStrategy() (gas: 52909) -AquaTest:testShipCannotHaveDuplicateTokens() (gas: 46848) \ No newline at end of file +AquaTest:testSafeBalancesRevertsIfAnyTokenNotInStrategy() (gas: 80484) +AquaTest:testSafeBalancesTracksChangesFromPushPull() (gas: 139186) +AquaTest:testShipCannotBeCalledTwiceForSameStrategy() (gas: 53461) +AquaTest:testShipCannotHaveDuplicateTokens() (gas: 47071) \ No newline at end of file diff --git a/src/Aqua.sol b/src/Aqua.sol index e863a5c..c804329 100644 --- a/src/Aqua.sol +++ b/src/Aqua.sol @@ -6,13 +6,14 @@ pragma solidity 0.8.30; import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { SafeERC20, IERC20 } from "@1inch/solidity-utils/contracts/libraries/SafeERC20.sol"; +import { ReentrancyGuardTransient } from "@openzeppelin/contracts/utils/ReentrancyGuardTransient.sol"; import { IAqua } from "./interfaces/IAqua.sol"; import { IShipHook } from "./interfaces/IShipHook.sol"; import { Balance, BalanceLib } from "./libs/Balance.sol"; /// @title Aqua - Shared Liquidity Layer -contract Aqua is IAqua { +contract Aqua is IAqua, ReentrancyGuardTransient { using SafeERC20 for IERC20; using SafeCast for uint256; using BalanceLib for Balance; @@ -50,7 +51,7 @@ contract Aqua is IAqua { address[] calldata tokens, uint256[] calldata amounts, uint8 hooks - ) external payable returns(bytes32 strategyHash) { + ) external payable nonReentrant returns(bytes32 strategyHash) { strategyHash = keccak256(strategy); uint8 tokensCount = tokens.length.toUint8(); require(tokensCount != _DOCKED, MaxNumberOfTokensExceeded(tokensCount, _DOCKED)); diff --git a/test/AquaHooks.t.sol b/test/AquaHooks.t.sol index 93bc357..df10063 100644 --- a/test/AquaHooks.t.sol +++ b/test/AquaHooks.t.sol @@ -127,12 +127,76 @@ contract MockNonHookApp { // This app doesn't implement IShipHook } +/// @title ReentrantHookApp - App that tries to re-enter ship() from hooks +contract ReentrantHookApp is IShipHook { + Aqua public aqua; + bool public attemptReentry; + bool public reentrancyAttempted; + bool public reentrancySucceeded; + + constructor(address _aqua) { + aqua = Aqua(_aqua); + } + + function setAttemptReentry(bool _attempt) external { + attemptReentry = _attempt; + } + + function beforeShip( + address, + bytes32, + address[] calldata tokens, + uint256[] calldata amounts + ) external payable override returns (bool success) { + if (attemptReentry) { + reentrancyAttempted = true; + // Try to re-enter ship + try aqua.ship( + address(this), + "reentrant_strategy", + tokens, + amounts, + 0 + ) { + reentrancySucceeded = true; + } catch { + reentrancySucceeded = false; + } + } + return true; + } + + function afterShip( + address, + bytes32, + address[] calldata tokens, + uint256[] calldata amounts + ) external override { + if (attemptReentry) { + reentrancyAttempted = true; + // Try to re-enter ship + try aqua.ship( + address(this), + "reentrant_strategy_after", + tokens, + amounts, + 0 + ) { + reentrancySucceeded = true; + } catch { + reentrancySucceeded = false; + } + } + } +} + contract AquaHooksTest is Test { Aqua public aqua; MockToken public token1; MockWETH public weth; MockHookApp public hookApp; MockNonHookApp public nonHookApp; + ReentrantHookApp public reentrantApp; address public maker = address(0x1111); @@ -142,6 +206,7 @@ contract AquaHooksTest is Test { weth = new MockWETH(); hookApp = new MockHookApp(address(weth)); nonHookApp = new MockNonHookApp(); + reentrantApp = new ReentrantHookApp(address(aqua)); // Setup tokens and approvals token1.transfer(maker, 10000e18); @@ -537,4 +602,72 @@ contract AquaHooksTest is Test { assertEq(weth.balanceOf(maker), ethAmount); assertEq(hookApp.lastEthReceived(), ethAmount); } + + // ========== REENTRANCY PROTECTION TESTS ========== + + function testReentrancyFromBeforeShipIsBlocked() public { + reentrantApp.setAttemptReentry(true); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(reentrantApp), + "strategy_reentry_before", + tokens, + amounts, + 1 // HOOK_BEFORE + ); + + // Reentrancy was attempted but blocked + assertTrue(reentrantApp.reentrancyAttempted()); + assertFalse(reentrantApp.reentrancySucceeded()); + } + + function testReentrancyFromAfterShipIsBlocked() public { + reentrantApp.setAttemptReentry(true); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(reentrantApp), + "strategy_reentry_after", + tokens, + amounts, + 2 // HOOK_AFTER + ); + + // Reentrancy was attempted but blocked + assertTrue(reentrantApp.reentrancyAttempted()); + assertFalse(reentrantApp.reentrancySucceeded()); + } + + function testReentrancyFromBothHooksIsBlocked() public { + reentrantApp.setAttemptReentry(true); + + address[] memory tokens = new address[](1); + tokens[0] = address(token1); + uint256[] memory amounts = new uint256[](1); + amounts[0] = 100e18; + + vm.prank(maker); + aqua.ship( + address(reentrantApp), + "strategy_reentry_both", + tokens, + amounts, + 3 // HOOK_BOTH + ); + + // Reentrancy was attempted but blocked + assertTrue(reentrantApp.reentrancyAttempted()); + assertFalse(reentrantApp.reentrancySucceeded()); + } } From 7bc588a6edb525636b0c92316d7a73a4f5705282 Mon Sep 17 00:00:00 2001 From: Sergej Kunz Date: Wed, 14 Jan 2026 06:44:20 +0400 Subject: [PATCH 8/8] docs: add comprehensive documentation for hook interface and error handling Addresses code review feedback: 1. Interface Validation: Documented that Aqua intentionally does NOT verify apps implement IShipHook (saves ~2600 gas). Apps should only set hook flags if they implement the corresponding hook methods. 2. Inconsistent Error Handling: Documented the intentional difference: - beforeShip returns bool for graceful failure signaling - afterShip has no return (follows callback pattern, reverts on failure) This is by design, not an oversight. 3. Hook Failure Semantics: Added clear guidance: - Return false: Expected failures with ShipHookFailed error - Revert: Unexpected failures with detailed error info Both cause ship() to revert. --- src/Aqua.sol | 13 ++++++-- src/interfaces/IShipHook.sol | 61 ++++++++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/Aqua.sol b/src/Aqua.sol index c804329..9f1faa9 100644 --- a/src/Aqua.sol +++ b/src/Aqua.sol @@ -61,8 +61,11 @@ contract Aqua is IAqua, ReentrancyGuardTransient { require((hooks & HOOK_BEFORE) != 0, ETHSentWithoutBeforeHook()); } - // beforeShip hook: called before balance storage (if HOOK_BEFORE flag set) + // beforeShip hook: called BEFORE balance storage (if HOOK_BEFORE flag set) // Use for: ETH wrapping, pre-validation, setup + // Note: If app doesn't implement IShipHook, this will revert. This is intentional - + // apps should only set HOOK_BEFORE if they implement the hook. No ERC-165 check + // is performed to save gas (~2600 gas saved per call). if ((hooks & HOOK_BEFORE) != 0) { bool success = IShipHook(app).beforeShip{value: msg.value}( msg.sender, @@ -70,6 +73,8 @@ contract Aqua is IAqua, ReentrancyGuardTransient { tokens, amounts ); + // beforeShip returns bool to allow graceful failure signaling. + // Returning false triggers ShipHookFailed; reverting propagates the error. require(success, ShipHookFailed(app, HOOK_BEFORE)); } @@ -82,8 +87,12 @@ contract Aqua is IAqua, ReentrancyGuardTransient { emit Pushed(msg.sender, app, strategyHash, tokens[i], amounts[i]); } - // afterShip hook: called after balance storage (if HOOK_AFTER flag set) + // afterShip hook: called AFTER balance storage (if HOOK_AFTER flag set) // Use for: notifications, additional state setup, external calls + // Note: Unlike beforeShip, afterShip has no return value. This is intentional: + // - afterShip is for side effects, not validation + // - If it fails, it should revert (consistent with callback patterns) + // - Any revert here will roll back the entire transaction including balance storage if ((hooks & HOOK_AFTER) != 0) { IShipHook(app).afterShip( msg.sender, diff --git a/src/interfaces/IShipHook.sol b/src/interfaces/IShipHook.sol index 930aee3..a5192d7 100644 --- a/src/interfaces/IShipHook.sol +++ b/src/interfaces/IShipHook.sol @@ -10,20 +10,54 @@ pragma solidity ^0.8.0; /// - beforeShip: Native ETH wrapping, pre-validation, setup /// - afterShip: Notifications, additional state setup, external calls /// +/// ## Hook Flags /// Hooks are optional and controlled via the `hooks` flags parameter in ship(): +/// - HOOK_NONE (0x00): No hooks called /// - HOOK_BEFORE (0x01): Call beforeShip before balance storage -/// - HOOK_AFTER (0x02): Call afterShip after balance storage +/// - HOOK_AFTER (0x02): Call afterShip after balance storage /// - HOOK_BOTH (0x03): Call both hooks +/// +/// ## Interface Validation +/// Aqua does NOT verify that the app implements IShipHook before calling hooks. +/// If an app address is used with hooks enabled but doesn't implement IShipHook, +/// the call will revert. This is intentional to avoid gas overhead of ERC-165 checks. +/// Apps should only set hook flags if they implement the corresponding hook methods. +/// +/// ## Error Handling +/// - beforeShip: MUST return true on success. Returning false or reverting will +/// cause the entire ship() transaction to revert. Use this for critical validation. +/// - afterShip: Any revert will propagate and cause ship() to fail. Since afterShip +/// is called AFTER balances are stored, apps should handle errors gracefully +/// if the hook failure shouldn't block the ship operation. +/// +/// ## When to Return False vs Revert in beforeShip +/// - Return false: For expected failures that should block ship with a clear error +/// - Revert with custom error: For unexpected failures with detailed error info +/// Both will cause ship() to revert, but returning false uses ShipHookFailed error. interface IShipHook { /// @notice Called by Aqua BEFORE processing ship (when HOOK_BEFORE flag is set) /// @dev Use for: ETH wrapping, pre-validation, setup - /// The hook receives ETH if msg.value > 0 and should handle it appropriately - /// For ETH wrapping: wrap to WETH and transfer to maker + /// + /// ## ETH Handling + /// The hook receives ETH via msg.value if sent with ship(). For ETH wrapping: + /// 1. Wrap ETH to WETH: WETH.deposit{value: msg.value}() + /// 2. Transfer WETH to maker: WETH.transfer(maker, msg.value) + /// This allows the maker to have WETH for the strategy without pre-wrapping. + /// + /// ## Return Value + /// - Return true: Hook succeeded, continue with ship + /// - Return false: Hook failed, revert ship with ShipHookFailed error + /// - Revert: Propagates to ship() caller + /// + /// ## Security + /// This hook is called BEFORE balances are stored. Any state changes made here + /// will be reverted if the ship fails. The hook is protected by reentrancy guard. + /// /// @param maker The maker address who is shipping the strategy - /// @param strategyHash The hash of the strategy being shipped + /// @param strategyHash The hash of the strategy being shipped (keccak256 of strategy bytes) /// @param tokens Array of token addresses in the strategy /// @param amounts Array of amounts for each token - /// @return success True if hook processed successfully + /// @return success True if hook processed successfully, false to revert ship function beforeShip( address maker, bytes32 strategyHash, @@ -33,7 +67,22 @@ interface IShipHook { /// @notice Called by Aqua AFTER ship is completed (when HOOK_AFTER flag is set) /// @dev Use for: notifications, additional state setup, external calls - /// Called after balances are stored, so strategy is already active + /// + /// ## Execution Context + /// Called AFTER balances are stored, so strategy is already active in Aqua. + /// If this hook reverts, the entire ship() transaction reverts, including + /// the balance storage. Apps should handle non-critical failures gracefully. + /// + /// ## No Return Value + /// Unlike beforeShip, afterShip has no return value. This is intentional: + /// - afterShip is for side effects (notifications, external calls) + /// - If it fails, it should revert (no silent failures) + /// - The pattern matches common callback patterns (no boolean dance) + /// + /// ## Security + /// This hook is called AFTER state changes but still within the reentrancy guard. + /// Reentering ship() from this hook will revert. + /// /// @param maker The maker address who shipped the strategy /// @param strategyHash The hash of the strategy that was shipped /// @param tokens Array of token addresses in the strategy