From c62d50503954653d6c264d20a89f676195a5d889 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 19 Oct 2023 16:22:33 -0700 Subject: [PATCH 1/9] add base module and IModule --- contracts/interfaces/modules/base/IModule.sol | 13 ++++++ contracts/modules/base/BaseModule.sol | 45 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 contracts/interfaces/modules/base/IModule.sol create mode 100644 contracts/modules/base/BaseModule.sol diff --git a/contracts/interfaces/modules/base/IModule.sol b/contracts/interfaces/modules/base/IModule.sol new file mode 100644 index 00000000..1ca935ac --- /dev/null +++ b/contracts/interfaces/modules/base/IModule.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.13; + +import { IModule } from "./IModule.sol"; + +interface IModule { + + event RequestPending(address indexed sender); + event RequestCompleted(address indexed sender); + + function execute(bytes calldata selfParams, bytes[] calldata preHooksParams, bytes[] calldata postHooksParams) external; + +} \ No newline at end of file diff --git a/contracts/modules/base/BaseModule.sol b/contracts/modules/base/BaseModule.sol new file mode 100644 index 00000000..c9b8773f --- /dev/null +++ b/contracts/modules/base/BaseModule.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.13; + +import { IModule } from "contracts/interfaces/modules/base/IModule.sol"; +import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; +import { FranchiseRegistry } from "contracts/FranchiseRegistry.sol"; +import { HookRegistry } from "contracts/hooks/HookRegistry.sol"; + +abstract contract BaseModule is IModule, HookRegistry { + + struct ModuleConstruction { + address ipaRegistry; + address moduleRegistry; + } + + address public immutable IPA_REGISTRY; + address public immutable MODULE_REGISTRY; + + constructor(ModuleConstruction memory params) { + IPA_REGISTRY = params.ipaRegistry; + MODULE_REGISTRY = params.moduleRegistry; + } + + function execute(bytes calldata selfParams, bytes[] calldata preHookParams, bytes[] calldata postHookParams) external { + // Should we include a request Id? + _verifyExecution(msg.sender, selfParams); + if (!_executePreHooks(preHookParams)) { + emit RequestPending(msg.sender); + return; + } + _performAction(selfParams); + _executePostHooks(postHookParams); + emit RequestCompleted(msg.sender); + } + + function _hookRegistryAdmin() virtual override internal view returns (address) { + // get owner from ipa registry + } + + function _verifyExecution(address caller, bytes calldata selfParams) virtual internal {} + function _executePreHooks(bytes[] calldata params) virtual internal returns (bool) {} + function _performAction(bytes calldata params) virtual internal {} + function _executePostHooks(bytes[] calldata params) virtual internal {} + +} \ No newline at end of file From 7cf4fd0bd41aa17e0fddd255697e1f687c0961f1 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 19 Oct 2023 16:27:42 -0700 Subject: [PATCH 2/9] added hook registry --- contracts/modules/base/BaseModule.sol | 2 +- contracts/modules/base/HookRegistry.sol | 148 ++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 contracts/modules/base/HookRegistry.sol diff --git a/contracts/modules/base/BaseModule.sol b/contracts/modules/base/BaseModule.sol index c9b8773f..8b043f35 100644 --- a/contracts/modules/base/BaseModule.sol +++ b/contracts/modules/base/BaseModule.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.13; import { IModule } from "contracts/interfaces/modules/base/IModule.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { FranchiseRegistry } from "contracts/FranchiseRegistry.sol"; -import { HookRegistry } from "contracts/hooks/HookRegistry.sol"; +import { HookRegistry } from "./HookRegistry.sol"; abstract contract BaseModule is IModule, HookRegistry { diff --git a/contracts/modules/base/HookRegistry.sol b/contracts/modules/base/HookRegistry.sol new file mode 100644 index 00000000..21dfb8b8 --- /dev/null +++ b/contracts/modules/base/HookRegistry.sol @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.13; + +import { Errors } from "contracts/lib/Errors.sol"; + +abstract contract HookRegistry { + enum HookType { + PreAction, + PostAction + } + + address[] public preActionHooks; + address[] public postActionHooks; + + uint256 public constant INDEX_NOT_FOUND = type(uint256).max; + uint256 public constant MAX_HOOKS = 10; + + event HookRegistered(HookType hType, address indexed hook, uint256 index); + event HookUnregistered(HookType hType, address indexed hook, uint256 index); + event HookReplaced( + HookType hType, + address indexed prevHook, + uint256 prevIndex, + address indexed nextHook, + uint256 nextIndex + ); + + modifier onlyHookRegistryAdmin() { + if (msg.sender == _hookRegistryAdmin()) + revert Errors.HookRegistry_CallerNotAdmin(); + _; + } + + function registerHook( + HookType hType_, + address hook_ + ) external onlyHookRegistryAdmin { + emit HookRegistered( + hType_, + hook_, + _registerHook(_hooksForType(hType_), hook_) + ); + } + + function unregisterHook( + HookType hType_, + address hook + ) external onlyHookRegistryAdmin { + emit HookUnregistered( + hType_, + hook, + _unregisterHook(_hooksForType(hType_), hook) + ); + } + + function replaceHook( + HookType hType_, + uint256 prevIndex_, + uint256 nextIndex_ + ) external onlyHookRegistryAdmin { + (address prevHook, address nextHook) = _replaceHook( + _hooksForType(hType_), + prevIndex_, + nextIndex_ + ); + emit HookReplaced(hType_, prevHook, prevIndex_, nextHook, nextIndex_); + } + + function hookIndex( + HookType hType_, + address hook_ + ) public view returns (uint256) { + return _hookIndex(_hooksForType(hType_), hook_); + } + + function isRegistered( + HookType hType_, + address hook_ + ) public view returns (bool) { + return hookIndex(hType_, hook_) != INDEX_NOT_FOUND; + } + + function _hookRegistryAdmin() internal view virtual returns (address); + + function _hooksForType( + HookType hType_ + ) private view returns (address[] storage) { + if (hType_ == HookType.PreAction) { + return preActionHooks; + } else { + return postActionHooks; + } + } + + function _registerHook( + address[] storage hooks_, + address hook_ + ) private returns (uint256) { + if (_hookIndex(hooks_, hook_) != INDEX_NOT_FOUND) { + revert("HookRegistry: hook already registered"); + } + hooks_.push(hook_); + return hooks_.length - 1; + } + + function _unregisterHook( + address[] storage hooks_, + address hook_ + ) private returns (uint256) { + uint256 index = _hookIndex(hooks_, hook_); + if (index == INDEX_NOT_FOUND) { + revert Errors.HookRegistry_HookNotFound(); + } + delete hooks_[index]; + return index; + } + + function _replaceHook( + address[] storage hooks_, + uint256 prevIndex_, + uint256 nextIndex_ + ) private returns (address prevHook, address nextHook) { + if (prevIndex_ >= hooks_.length || nextIndex_ >= hooks_.length) { + revert Errors.HookRegistry_IndexOutOfBounds(); + } + prevHook = hooks_[prevIndex_]; + nextHook = hooks_[nextIndex_]; + hooks_[prevIndex_] = nextHook; + hooks_[nextIndex_] = prevHook; + return (prevHook, nextHook); + } + + function _hookIndex( + address[] storage hooks, + address hook_ + ) private view returns (uint256) { + uint256 length = hooks.length; + for (uint256 i = 0; i < length; ) { + if (hooks[i] == hook_) { + return i; + } + unchecked { + i++; + } + } + return INDEX_NOT_FOUND; + } +} From c6ca9ce43174d387c069c5a7b2c72c1401cd48d0 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 19 Oct 2023 16:35:59 -0700 Subject: [PATCH 3/9] added hook errors --- contracts/lib/Errors.sol | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 4778194b..60d58a63 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -35,6 +35,16 @@ library Errors { /// @notice The amount specified may not be zero. error ZeroAmount(); + //////////////////////////////////////////////////////////////////////////// + // HookRegistry // + //////////////////////////////////////////////////////////////////////////// + + /// @notice The hook is already registered. + error HookRegistry_AlreadyRegistered(); + error HookRegistry_HookNotFound(); + error HookRegistry_CallerNotAdmin(); + error HookRegistry_IndexOutOfBounds(); + //////////////////////////////////////////////////////////////////////////// // BaseRelationshipProcessor // //////////////////////////////////////////////////////////////////////////// From 56802f859628f0693f90b33a174d2f33ac9183dd Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 19 Oct 2023 17:30:37 -0700 Subject: [PATCH 4/9] tests and hook registry fixes --- contracts/lib/Errors.sol | 1 + contracts/modules/base/BaseModule.sol | 5 +- contracts/modules/base/HookRegistry.sol | 34 +++++-- test/foundry/mocks/MockHookRegistry.sol | 24 +++++ .../modules/base/HookRegistryTest.t.sol | 88 +++++++++++++++++++ 5 files changed, 139 insertions(+), 13 deletions(-) create mode 100644 test/foundry/mocks/MockHookRegistry.sol create mode 100644 test/foundry/modules/base/HookRegistryTest.t.sol diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 60d58a63..b0735b58 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -44,6 +44,7 @@ library Errors { error HookRegistry_HookNotFound(); error HookRegistry_CallerNotAdmin(); error HookRegistry_IndexOutOfBounds(); + error HookRegistry_MaxHooksExceeded(); //////////////////////////////////////////////////////////////////////////// // BaseRelationshipProcessor // diff --git a/contracts/modules/base/BaseModule.sol b/contracts/modules/base/BaseModule.sol index 8b043f35..fdb4d2d5 100644 --- a/contracts/modules/base/BaseModule.sol +++ b/contracts/modules/base/BaseModule.sol @@ -33,10 +33,7 @@ abstract contract BaseModule is IModule, HookRegistry { emit RequestCompleted(msg.sender); } - function _hookRegistryAdmin() virtual override internal view returns (address) { - // get owner from ipa registry - } - + function _hookRegistryAdmin() virtual override internal view returns (address); function _verifyExecution(address caller, bytes calldata selfParams) virtual internal {} function _executePreHooks(bytes[] calldata params) virtual internal returns (bool) {} function _performAction(bytes calldata params) virtual internal {} diff --git a/contracts/modules/base/HookRegistry.sol b/contracts/modules/base/HookRegistry.sol index 21dfb8b8..b0df913a 100644 --- a/contracts/modules/base/HookRegistry.sol +++ b/contracts/modules/base/HookRegistry.sol @@ -9,14 +9,14 @@ abstract contract HookRegistry { PostAction } - address[] public preActionHooks; - address[] public postActionHooks; + address[] private _preActionHooks; + address[] private _postActionHooks; uint256 public constant INDEX_NOT_FOUND = type(uint256).max; uint256 public constant MAX_HOOKS = 10; - event HookRegistered(HookType hType, address indexed hook, uint256 index); - event HookUnregistered(HookType hType, address indexed hook, uint256 index); + event HookRegistered(HookType indexed hType, address indexed hook, uint256 index); + event HookUnregistered(HookType indexed hType, address indexed hook, uint256 index); event HookReplaced( HookType hType, address indexed prevHook, @@ -24,9 +24,9 @@ abstract contract HookRegistry { address indexed nextHook, uint256 nextIndex ); - + modifier onlyHookRegistryAdmin() { - if (msg.sender == _hookRegistryAdmin()) + if (msg.sender != _hookRegistryAdmin()) revert Errors.HookRegistry_CallerNotAdmin(); _; } @@ -80,15 +80,28 @@ abstract contract HookRegistry { return hookIndex(hType_, hook_) != INDEX_NOT_FOUND; } + function hookAt( + HookType hType_, + uint256 index_ + ) public view returns (address) { + return _hooksForType(hType_)[index_]; + } + + function totalHooks( + HookType hType_ + ) public view returns (uint256) { + return _hooksForType(hType_).length; + } + function _hookRegistryAdmin() internal view virtual returns (address); function _hooksForType( HookType hType_ ) private view returns (address[] storage) { if (hType_ == HookType.PreAction) { - return preActionHooks; + return _preActionHooks; } else { - return postActionHooks; + return _postActionHooks; } } @@ -96,8 +109,11 @@ abstract contract HookRegistry { address[] storage hooks_, address hook_ ) private returns (uint256) { + if (hooks_.length == MAX_HOOKS) { + revert Errors.HookRegistry_MaxHooksExceeded(); + } if (_hookIndex(hooks_, hook_) != INDEX_NOT_FOUND) { - revert("HookRegistry: hook already registered"); + revert Errors.HookRegistry_AlreadyRegistered(); } hooks_.push(hook_); return hooks_.length - 1; diff --git a/test/foundry/mocks/MockHookRegistry.sol b/test/foundry/mocks/MockHookRegistry.sol new file mode 100644 index 00000000..735def21 --- /dev/null +++ b/test/foundry/mocks/MockHookRegistry.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.18; + +import { HookRegistry } from "contracts/modules/base/HookRegistry.sol"; + +/// @title Mock Hook Registry +/// @notice This mock contract is used for testing the base hook registry. +contract MockHookRegistry is HookRegistry { + address public immutable ADMIN; + + constructor() { + ADMIN = msg.sender; + } + + function _hookRegistryAdmin() + internal + view + virtual + override + returns (address) + { + return ADMIN; + } +} diff --git a/test/foundry/modules/base/HookRegistryTest.t.sol b/test/foundry/modules/base/HookRegistryTest.t.sol new file mode 100644 index 00000000..88f5ead9 --- /dev/null +++ b/test/foundry/modules/base/HookRegistryTest.t.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: BUSDL-1.1 +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; + +import "contracts/modules/base/HookRegistry.sol"; +import "test/foundry/mocks/MockHookRegistry.sol"; +import "contracts/lib/Errors.sol"; + +contract HookRegistryTest is Test { + MockHookRegistry registry; + address admin = address(123); + + event HookRegistered(HookRegistry.HookType indexed hType, address indexed hook, uint256 index); + event HookUnregistered(HookRegistry.HookType indexed hType, address indexed hook, uint256 index); + event HookReplaced( + HookRegistry.HookType hType, + address indexed prevHook, + uint256 prevIndex, + address indexed nextHook, + uint256 nextIndex + ); + + function setUp() public { + vm.prank(admin); + registry = new MockHookRegistry(); + } + + function test_hookRegistry_registerPreHooks() public { + address hook1 = address(456); + address hook2 = address(789); + vm.startPrank(admin); + vm.expectEmit(true, true, false, true); + emit HookRegistered(HookRegistry.HookType.PreAction, hook1, 0); + registry.registerHook(HookRegistry.HookType.PreAction, hook1); + vm.expectEmit(true, true, false, true); + emit HookRegistered(HookRegistry.HookType.PreAction, hook2, 1); + registry.registerHook(HookRegistry.HookType.PreAction, hook2); + vm.stopPrank(); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 0), hook1); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 1), hook2); + assertEq(registry.totalHooks(HookRegistry.HookType.PreAction), 2); + } + + function test_hookRegistry_registerPostHooks() public { + address hook1 = address(456); + address hook2 = address(789); + vm.startPrank(admin); + vm.expectEmit(true, true, false, true); + emit HookRegistered(HookRegistry.HookType.PostAction, hook1, 0); + registry.registerHook(HookRegistry.HookType.PostAction, hook1); + vm.expectEmit(true, true, false, true); + emit HookRegistered(HookRegistry.HookType.PostAction, hook2, 1); + registry.registerHook(HookRegistry.HookType.PostAction, hook2); + vm.stopPrank(); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 0), hook1); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 1), hook2); + assertEq(registry.totalHooks(HookRegistry.HookType.PostAction), 2); + } + + function test_hookRegistry_revertRegisterPreHooksCallerNotAdmin() public { + address hook1 = address(456); + vm.expectRevert(Errors.HookRegistry_CallerNotAdmin.selector); + registry.registerHook(HookRegistry.HookType.PostAction, hook1); + } + + function test_hookRegistry_revertRegisterTooManyHooks() public { + vm.startPrank(admin); + for(uint256 i = 0; i <= registry.MAX_HOOKS(); i++) { + if (i == registry.MAX_HOOKS()) { + vm.expectRevert(Errors.HookRegistry_MaxHooksExceeded.selector); + } + registry.registerHook(HookRegistry.HookType.PostAction, vm.addr(i + 1)); + } + } + + function test_hookRegistry_revertRegisterHookAlreadyRegistered() public { + address hook1 = address(456); + vm.startPrank(admin); + registry.registerHook(HookRegistry.HookType.PostAction, hook1); + vm.expectRevert(Errors.HookRegistry_AlreadyRegistered.selector); + registry.registerHook(HookRegistry.HookType.PostAction, hook1); + vm.stopPrank(); + } + + + +} From 8b332b8ea495d3825a7f00f9e4c1e4e5d859baa2 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 19 Oct 2023 21:13:31 -0700 Subject: [PATCH 5/9] fix tests hook registry --- contracts/lib/Errors.sol | 5 +- contracts/modules/base/HookRegistry.sol | 128 +++++-------- .../modules/base/HookRegistryTest.t.sol | 178 +++++++++++++----- 3 files changed, 178 insertions(+), 133 deletions(-) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index b0735b58..c2485632 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -40,10 +40,9 @@ library Errors { //////////////////////////////////////////////////////////////////////////// /// @notice The hook is already registered. - error HookRegistry_AlreadyRegistered(); - error HookRegistry_HookNotFound(); + error HookRegistry_RegisteringDuplicatedHook(); + error HookRegistry_RegisteringZeroAddressHook(); error HookRegistry_CallerNotAdmin(); - error HookRegistry_IndexOutOfBounds(); error HookRegistry_MaxHooksExceeded(); //////////////////////////////////////////////////////////////////////////// diff --git a/contracts/modules/base/HookRegistry.sol b/contracts/modules/base/HookRegistry.sol index b0df913a..46cea538 100644 --- a/contracts/modules/base/HookRegistry.sol +++ b/contracts/modules/base/HookRegistry.sol @@ -15,15 +15,8 @@ abstract contract HookRegistry { uint256 public constant INDEX_NOT_FOUND = type(uint256).max; uint256 public constant MAX_HOOKS = 10; - event HookRegistered(HookType indexed hType, address indexed hook, uint256 index); - event HookUnregistered(HookType indexed hType, address indexed hook, uint256 index); - event HookReplaced( - HookType hType, - address indexed prevHook, - uint256 prevIndex, - address indexed nextHook, - uint256 nextIndex - ); + event HooksRegistered(HookType indexed hType, address[] indexed hook); + event HooksCleared(HookType indexed hType); modifier onlyHookRegistryAdmin() { if (msg.sender != _hookRegistryAdmin()) @@ -31,67 +24,52 @@ abstract contract HookRegistry { _; } - function registerHook( + function registerHooks( HookType hType_, - address hook_ - ) external onlyHookRegistryAdmin { - emit HookRegistered( - hType_, - hook_, - _registerHook(_hooksForType(hType_), hook_) - ); - } - - function unregisterHook( - HookType hType_, - address hook + address[] calldata hooks_ ) external onlyHookRegistryAdmin { - emit HookUnregistered( - hType_, - hook, - _unregisterHook(_hooksForType(hType_), hook) - ); - } - - function replaceHook( - HookType hType_, - uint256 prevIndex_, - uint256 nextIndex_ - ) external onlyHookRegistryAdmin { - (address prevHook, address nextHook) = _replaceHook( - _hooksForType(hType_), - prevIndex_, - nextIndex_ - ); - emit HookReplaced(hType_, prevHook, prevIndex_, nextHook, nextIndex_); - } - - function hookIndex( - HookType hType_, - address hook_ - ) public view returns (uint256) { - return _hookIndex(_hooksForType(hType_), hook_); + clearHooks(hType_); + _registerHooks(_hooksForType(hType_), hooks_); + emit HooksRegistered(hType_, hooks_); } function isRegistered( HookType hType_, address hook_ - ) public view returns (bool) { + ) external view returns (bool) { return hookIndex(hType_, hook_) != INDEX_NOT_FOUND; } function hookAt( HookType hType_, uint256 index_ - ) public view returns (address) { + ) external view returns (address) { return _hooksForType(hType_)[index_]; } function totalHooks( HookType hType_ - ) public view returns (uint256) { + ) external view returns (uint256) { return _hooksForType(hType_).length; } + + function clearHooks( + HookType hType_ + ) public onlyHookRegistryAdmin { + if (hType_ == HookType.PreAction && _preActionHooks.length > 0) { + delete _preActionHooks; + } else if (_postActionHooks.length > 0) { + delete _postActionHooks; + } + emit HooksCleared(hType_); + } + + function hookIndex( + HookType hType_, + address hook_ + ) public view returns (uint256) { + return _hookIndex(_hooksForType(hType_), hook_); + } function _hookRegistryAdmin() internal view virtual returns (address); @@ -105,45 +83,25 @@ abstract contract HookRegistry { } } - function _registerHook( + function _registerHooks( address[] storage hooks_, - address hook_ - ) private returns (uint256) { - if (hooks_.length == MAX_HOOKS) { + address[] memory newHooks_ + ) private { + uint256 newLength = newHooks_.length; + if (newLength > MAX_HOOKS) { revert Errors.HookRegistry_MaxHooksExceeded(); } - if (_hookIndex(hooks_, hook_) != INDEX_NOT_FOUND) { - revert Errors.HookRegistry_AlreadyRegistered(); - } - hooks_.push(hook_); - return hooks_.length - 1; - } - - function _unregisterHook( - address[] storage hooks_, - address hook_ - ) private returns (uint256) { - uint256 index = _hookIndex(hooks_, hook_); - if (index == INDEX_NOT_FOUND) { - revert Errors.HookRegistry_HookNotFound(); - } - delete hooks_[index]; - return index; - } - - function _replaceHook( - address[] storage hooks_, - uint256 prevIndex_, - uint256 nextIndex_ - ) private returns (address prevHook, address nextHook) { - if (prevIndex_ >= hooks_.length || nextIndex_ >= hooks_.length) { - revert Errors.HookRegistry_IndexOutOfBounds(); + unchecked { + for (uint256 i = 0; i < newLength; i++) { + if (newHooks_[i] == address(0)) { + revert Errors.HookRegistry_RegisteringZeroAddressHook(); + } + if (i > 0 && newHooks_[i] == newHooks_[i - 1]) { + revert Errors.HookRegistry_RegisteringDuplicatedHook(); + } + hooks_.push(newHooks_[i]); + } } - prevHook = hooks_[prevIndex_]; - nextHook = hooks_[nextIndex_]; - hooks_[prevIndex_] = nextHook; - hooks_[nextIndex_] = prevHook; - return (prevHook, nextHook); } function _hookIndex( diff --git a/test/foundry/modules/base/HookRegistryTest.t.sol b/test/foundry/modules/base/HookRegistryTest.t.sol index 88f5ead9..8c56415b 100644 --- a/test/foundry/modules/base/HookRegistryTest.t.sol +++ b/test/foundry/modules/base/HookRegistryTest.t.sol @@ -11,15 +11,8 @@ contract HookRegistryTest is Test { MockHookRegistry registry; address admin = address(123); - event HookRegistered(HookRegistry.HookType indexed hType, address indexed hook, uint256 index); - event HookUnregistered(HookRegistry.HookType indexed hType, address indexed hook, uint256 index); - event HookReplaced( - HookRegistry.HookType hType, - address indexed prevHook, - uint256 prevIndex, - address indexed nextHook, - uint256 nextIndex - ); + event HooksRegistered(HookRegistry.HookType indexed hType, address[] indexed hook); + event HooksCleared(HookRegistry.HookType indexed hType); function setUp() public { vm.prank(admin); @@ -27,62 +20,157 @@ contract HookRegistryTest is Test { } function test_hookRegistry_registerPreHooks() public { - address hook1 = address(456); - address hook2 = address(789); + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); vm.startPrank(admin); - vm.expectEmit(true, true, false, true); - emit HookRegistered(HookRegistry.HookType.PreAction, hook1, 0); - registry.registerHook(HookRegistry.HookType.PreAction, hook1); - vm.expectEmit(true, true, false, true); - emit HookRegistered(HookRegistry.HookType.PreAction, hook2, 1); - registry.registerHook(HookRegistry.HookType.PreAction, hook2); + vm.expectEmit(true, false, false, true); + emit HooksRegistered(HookRegistry.HookType.PreAction, hooks); + registry.registerHooks(HookRegistry.HookType.PreAction, hooks); + vm.stopPrank(); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 0), hooks[0]); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 1), hooks[1]); + assertEq(registry.totalHooks(HookRegistry.HookType.PreAction), hooks.length); + } + + function test_hookRegistry_registerPreHooksClearsHooksIfNotEmpty() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); + vm.startPrank(admin); + registry.registerHooks(HookRegistry.HookType.PreAction, hooks); + vm.expectEmit(true, false, false, true); + emit HooksCleared(HookRegistry.HookType.PreAction); + registry.registerHooks(HookRegistry.HookType.PreAction, hooks); vm.stopPrank(); - assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 0), hook1); - assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 1), hook2); - assertEq(registry.totalHooks(HookRegistry.HookType.PreAction), 2); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 0), hooks[0]); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 1), hooks[1]); + assertEq(registry.totalHooks(HookRegistry.HookType.PreAction), hooks.length); } function test_hookRegistry_registerPostHooks() public { - address hook1 = address(456); - address hook2 = address(789); + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); vm.startPrank(admin); - vm.expectEmit(true, true, false, true); - emit HookRegistered(HookRegistry.HookType.PostAction, hook1, 0); - registry.registerHook(HookRegistry.HookType.PostAction, hook1); - vm.expectEmit(true, true, false, true); - emit HookRegistered(HookRegistry.HookType.PostAction, hook2, 1); - registry.registerHook(HookRegistry.HookType.PostAction, hook2); + vm.expectEmit(true, false, false, true); + emit HooksRegistered(HookRegistry.HookType.PostAction, hooks); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); vm.stopPrank(); - assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 0), hook1); - assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 1), hook2); - assertEq(registry.totalHooks(HookRegistry.HookType.PostAction), 2); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 0), hooks[0]); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 1), hooks[1]); + assertEq(registry.totalHooks(HookRegistry.HookType.PostAction), hooks.length); } - function test_hookRegistry_revertRegisterPreHooksCallerNotAdmin() public { - address hook1 = address(456); + function test_hookRegistry_registerPostHooksClearsHooksIfNotEmpty() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); + vm.startPrank(admin); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + vm.expectEmit(true, false, false, true); + emit HooksCleared(HookRegistry.HookType.PostAction); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + vm.stopPrank(); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 0), hooks[0]); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 1), hooks[1]); + assertEq(registry.totalHooks(HookRegistry.HookType.PostAction), hooks.length); + } + + function test_hookRegistry_revertRegisterHooksCallerNotAdmin() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); vm.expectRevert(Errors.HookRegistry_CallerNotAdmin.selector); - registry.registerHook(HookRegistry.HookType.PostAction, hook1); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); } - function test_hookRegistry_revertRegisterTooManyHooks() public { + function test_hookRegistry_revertRegisterMaxHooksExceeded() public { + address[] memory hooks = new address[](registry.MAX_HOOKS() + 1); vm.startPrank(admin); for(uint256 i = 0; i <= registry.MAX_HOOKS(); i++) { - if (i == registry.MAX_HOOKS()) { - vm.expectRevert(Errors.HookRegistry_MaxHooksExceeded.selector); - } - registry.registerHook(HookRegistry.HookType.PostAction, vm.addr(i + 1)); + hooks[i] = vm.addr(i + 1); } + vm.expectRevert(Errors.HookRegistry_MaxHooksExceeded.selector); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + vm.expectRevert(Errors.HookRegistry_MaxHooksExceeded.selector); + registry.registerHooks(HookRegistry.HookType.PreAction, hooks); + vm.stopPrank(); + } + + function test_hookRegistry_revertRegisterDuplicatedHook() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(123); + vm.startPrank(admin); + vm.expectRevert(Errors.HookRegistry_RegisteringDuplicatedHook.selector); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + vm.stopPrank(); + } + + function test_hookRegistry_getters() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); + vm.startPrank(admin); + + registry.registerHooks(HookRegistry.HookType.PreAction, hooks); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 0), hooks[0]); + assertEq(registry.hookAt(HookRegistry.HookType.PreAction, 1), hooks[1]); + assertEq(registry.hookIndex(HookRegistry.HookType.PreAction, hooks[0]), 0); + assertEq(registry.hookIndex(HookRegistry.HookType.PreAction, hooks[1]), 1); + assertEq(registry.totalHooks(HookRegistry.HookType.PreAction), hooks.length); + assertEq(registry.isRegistered(HookRegistry.HookType.PreAction, hooks[0]), true); + assertEq(registry.isRegistered(HookRegistry.HookType.PreAction, hooks[1]), true); + + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 0), hooks[0]); + assertEq(registry.hookAt(HookRegistry.HookType.PostAction, 1), hooks[1]); + assertEq(registry.hookIndex(HookRegistry.HookType.PostAction, hooks[0]), 0); + assertEq(registry.hookIndex(HookRegistry.HookType.PostAction, hooks[1]), 1); + assertEq(registry.totalHooks(HookRegistry.HookType.PostAction), hooks.length); + assertEq(registry.isRegistered(HookRegistry.HookType.PostAction, hooks[0]), true); + assertEq(registry.isRegistered(HookRegistry.HookType.PostAction, hooks[1]), true); + + vm.stopPrank(); } - function test_hookRegistry_revertRegisterHookAlreadyRegistered() public { - address hook1 = address(456); + function test_hookRegistry_clearPreHooks() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); vm.startPrank(admin); - registry.registerHook(HookRegistry.HookType.PostAction, hook1); - vm.expectRevert(Errors.HookRegistry_AlreadyRegistered.selector); - registry.registerHook(HookRegistry.HookType.PostAction, hook1); + registry.registerHooks(HookRegistry.HookType.PreAction, hooks); + vm.expectEmit(true, true, false, true); + emit HooksCleared(HookRegistry.HookType.PreAction); + registry.clearHooks(HookRegistry.HookType.PreAction); + vm.stopPrank(); + assertEq(registry.hookIndex(HookRegistry.HookType.PreAction, hooks[0]), registry.INDEX_NOT_FOUND()); + assertEq(registry.hookIndex(HookRegistry.HookType.PreAction, hooks[1]), registry.INDEX_NOT_FOUND()); + assertEq(registry.totalHooks(HookRegistry.HookType.PreAction), 0); + } + + function test_hookRegistry_clearPostHooks() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); + vm.startPrank(admin); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + vm.expectEmit(true, true, false, true); + emit HooksCleared(HookRegistry.HookType.PostAction); + registry.clearHooks(HookRegistry.HookType.PostAction); vm.stopPrank(); + assertEq(registry.hookIndex(HookRegistry.HookType.PostAction, hooks[0]), registry.INDEX_NOT_FOUND()); + assertEq(registry.hookIndex(HookRegistry.HookType.PostAction, hooks[1]), registry.INDEX_NOT_FOUND()); + assertEq(registry.totalHooks(HookRegistry.HookType.PostAction), 0); } - + function test_hookRegistry_revertClearHooksCallerNotAdmin() public { + address[] memory hooks = new address[](2); + hooks[0] = address(123); + hooks[1] = address(456); + vm.expectRevert(Errors.HookRegistry_CallerNotAdmin.selector); + registry.registerHooks(HookRegistry.HookType.PostAction, hooks); + } } From 261201442ad5a1ac5a71ad504ed27975bff26251 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 19 Oct 2023 21:15:03 -0700 Subject: [PATCH 6/9] lint --- contracts/lib/Errors.sol | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index c2485632..057d6b6b 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -6,7 +6,6 @@ import { IPAsset } from "contracts/lib/IPAsset.sol"; /// @title Errors /// @notice Library for all contract errors, including a set of global errors. library Errors { - //////////////////////////////////////////////////////////////////////////// // Globals // //////////////////////////////////////////////////////////////////////////// @@ -77,7 +76,7 @@ library Errors { //////////////////////////////////////////////////////////////////////////// // CollectPaymentModule // //////////////////////////////////////////////////////////////////////////// -// + /// @notice The configured collect module payment amount is invalid. error CollectPaymentModule_AmountInvalid(); @@ -289,7 +288,10 @@ library Errors { //////////////////////////////////////////////////////////////////////////// /// @notice Mismatch between parity of accounts and their respective allocations. - error RoyaltyNFT_AccountsAndAllocationsMismatch(uint256 accountsLength, uint256 allocationsLength); + error RoyaltyNFT_AccountsAndAllocationsMismatch( + uint256 accountsLength, + uint256 allocationsLength + ); /// @notice Invalid summation for royalty NFT allocations. error RoyaltyNFT_InvalidAllocationsSum(uint32 allocationsSum); From 81f00a0c0ed17fd8089f819d0dc48a4205a24ff1 Mon Sep 17 00:00:00 2001 From: Ramarti Date: Wed, 25 Oct 2023 16:58:50 -0300 Subject: [PATCH 7/9] add configure to module (#135) Co-authored-by: Raul --- contracts/lib/Errors.sol | 7 +++++++ contracts/modules/base/BaseModule.sol | 27 ++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 057d6b6b..27868237 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -34,6 +34,13 @@ library Errors { /// @notice The amount specified may not be zero. error ZeroAmount(); + //////////////////////////////////////////////////////////////////////////// + // BaseModule // + //////////////////////////////////////////////////////////////////////////// + + /// @notice The caller to base module is not the module registry. + error BaseModule_CallerNotModuleRegistry(); + //////////////////////////////////////////////////////////////////////////// // HookRegistry // //////////////////////////////////////////////////////////////////////////// diff --git a/contracts/modules/base/BaseModule.sol b/contracts/modules/base/BaseModule.sol index fdb4d2d5..96f8e262 100644 --- a/contracts/modules/base/BaseModule.sol +++ b/contracts/modules/base/BaseModule.sol @@ -5,6 +5,7 @@ import { IModule } from "contracts/interfaces/modules/base/IModule.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { FranchiseRegistry } from "contracts/FranchiseRegistry.sol"; import { HookRegistry } from "./HookRegistry.sol"; +import { Errors } from "contracts/lib/Errors.sol"; abstract contract BaseModule is IModule, HookRegistry { @@ -16,24 +17,40 @@ abstract contract BaseModule is IModule, HookRegistry { address public immutable IPA_REGISTRY; address public immutable MODULE_REGISTRY; + modifier onlyModuleRegistry() { + // TODO: Enforce this + // if (msg.sender != MODULE_REGISTRY) + // revert Errors.BaseModule_CallerNotModuleRegistry(); + _; + } + constructor(ModuleConstruction memory params) { IPA_REGISTRY = params.ipaRegistry; MODULE_REGISTRY = params.moduleRegistry; } - function execute(bytes calldata selfParams, bytes[] calldata preHookParams, bytes[] calldata postHookParams) external { - // Should we include a request Id? - _verifyExecution(msg.sender, selfParams); + function execute( + address caller, + bytes calldata selfParams, + bytes[] calldata preHookParams, + bytes[] calldata postHookParams + ) external onlyModuleRegistry { + _verifyExecution(caller, selfParams); if (!_executePreHooks(preHookParams)) { - emit RequestPending(msg.sender); + emit RequestPending(caller); return; } _performAction(selfParams); _executePostHooks(postHookParams); - emit RequestCompleted(msg.sender); + emit RequestCompleted(caller); + } + + function configure(bytes calldata params) external onlyModuleRegistry { + _configure(msg.sender, params); } function _hookRegistryAdmin() virtual override internal view returns (address); + function _configure(address caller, bytes calldata params) virtual internal; function _verifyExecution(address caller, bytes calldata selfParams) virtual internal {} function _executePreHooks(bytes[] calldata params) virtual internal returns (bool) {} function _performAction(bytes calldata params) virtual internal {} From 030c60884ea9bbc39ef66d674a67a7ac0d8d0a02 Mon Sep 17 00:00:00 2001 From: Raul Date: Tue, 31 Oct 2023 21:06:34 -0300 Subject: [PATCH 8/9] address comments and add execution of hooks --- contracts/interfaces/modules/base/IModule.sol | 3 +- contracts/lib/Errors.sol | 5 +- contracts/modules/base/BaseModule.sol | 68 ++++++++------- contracts/modules/base/HookRegistry.sol | 2 +- .../_temp_modules/base/BaseModule.t.sol | 84 +++++++++++++++++++ .../base/HookRegistryTest.t.sol | 0 test/foundry/mocks/MockBaseModule.sol | 65 ++++++++++++++ 7 files changed, 194 insertions(+), 33 deletions(-) create mode 100644 test/foundry/_temp_modules/base/BaseModule.t.sol rename test/foundry/{modules => _temp_modules}/base/HookRegistryTest.t.sol (100%) create mode 100644 test/foundry/mocks/MockBaseModule.sol diff --git a/contracts/interfaces/modules/base/IModule.sol b/contracts/interfaces/modules/base/IModule.sol index 1ca935ac..7664f3eb 100644 --- a/contracts/interfaces/modules/base/IModule.sol +++ b/contracts/interfaces/modules/base/IModule.sol @@ -8,6 +8,7 @@ interface IModule { event RequestPending(address indexed sender); event RequestCompleted(address indexed sender); - function execute(bytes calldata selfParams, bytes[] calldata preHooksParams, bytes[] calldata postHooksParams) external; + function execute(address caller, bytes calldata selfParams, bytes[] calldata preHooksParams, bytes[] calldata postHooksParams) external; + function configure(address caller_, bytes calldata params_) external; } \ No newline at end of file diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 2c27356e..4a7c2873 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -38,8 +38,9 @@ library Errors { // BaseModule // //////////////////////////////////////////////////////////////////////////// - /// @notice The caller to base module is not the module registry. - error BaseModule_CallerNotModuleRegistry(); + error BaseModule_HooksParamsLengthMismatch(uint8 hookType); + error BaseModule_ZeroIpaRegistry(); + error BaseModule_ZeroModuleRegistry(); //////////////////////////////////////////////////////////////////////////// // HookRegistry // diff --git a/contracts/modules/base/BaseModule.sol b/contracts/modules/base/BaseModule.sol index 96f8e262..884e4403 100644 --- a/contracts/modules/base/BaseModule.sol +++ b/contracts/modules/base/BaseModule.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.13; import { IModule } from "contracts/interfaces/modules/base/IModule.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; -import { FranchiseRegistry } from "contracts/FranchiseRegistry.sol"; import { HookRegistry } from "./HookRegistry.sol"; import { Errors } from "contracts/lib/Errors.sol"; @@ -17,43 +16,54 @@ abstract contract BaseModule is IModule, HookRegistry { address public immutable IPA_REGISTRY; address public immutable MODULE_REGISTRY; - modifier onlyModuleRegistry() { - // TODO: Enforce this - // if (msg.sender != MODULE_REGISTRY) - // revert Errors.BaseModule_CallerNotModuleRegistry(); - _; - } - - constructor(ModuleConstruction memory params) { - IPA_REGISTRY = params.ipaRegistry; - MODULE_REGISTRY = params.moduleRegistry; + constructor(ModuleConstruction memory params_) { + if (params_.ipaRegistry == address(0)) { + revert Errors.BaseModule_ZeroIpaRegistry(); + } + IPA_REGISTRY = params_.ipaRegistry; + if (params_.moduleRegistry == address(0)) { + revert Errors.BaseModule_ZeroModuleRegistry(); + } + MODULE_REGISTRY = params_.moduleRegistry; } + // TODO access control on sender function execute( - address caller, - bytes calldata selfParams, - bytes[] calldata preHookParams, - bytes[] calldata postHookParams - ) external onlyModuleRegistry { - _verifyExecution(caller, selfParams); - if (!_executePreHooks(preHookParams)) { - emit RequestPending(caller); + address caller_, + bytes calldata selfParams_, + bytes[] calldata preHookParams_, + bytes[] calldata postHookParams_ + ) external { + _verifyExecution(caller_, selfParams_); + if (!_executeHooks(preHookParams_, HookType.PreAction)) { + emit RequestPending(caller_); return; } - _performAction(selfParams); - _executePostHooks(postHookParams); - emit RequestCompleted(caller); + _performAction(caller_, selfParams_); + _executeHooks(postHookParams_, HookType.PostAction); + emit RequestCompleted(caller_); } - function configure(bytes calldata params) external onlyModuleRegistry { - _configure(msg.sender, params); + // TODO access control on sender + function configure(address caller_, bytes calldata params_) external { + _configure(caller_, params_); + } + + function _executeHooks(bytes[] calldata params_, HookRegistry.HookType hType_) virtual internal returns (bool) { + address[] memory hooks = _hooksForType(hType_); + uint256 hooksLength = hooks.length; + if (params_.length != hooksLength) { + revert Errors.BaseModule_HooksParamsLengthMismatch(uint8(hType_)); + } + for (uint256 i = 0; i < hooksLength; i++) { + // TODO: hook execution and return false if a hook returns false + } + return true; } function _hookRegistryAdmin() virtual override internal view returns (address); - function _configure(address caller, bytes calldata params) virtual internal; - function _verifyExecution(address caller, bytes calldata selfParams) virtual internal {} - function _executePreHooks(bytes[] calldata params) virtual internal returns (bool) {} - function _performAction(bytes calldata params) virtual internal {} - function _executePostHooks(bytes[] calldata params) virtual internal {} + function _configure(address caller_, bytes calldata params_) virtual internal; + function _verifyExecution(address caller_, bytes calldata params_) virtual internal {} + function _performAction(address caller_, bytes calldata params_) virtual internal {} } \ No newline at end of file diff --git a/contracts/modules/base/HookRegistry.sol b/contracts/modules/base/HookRegistry.sol index 46cea538..f663477f 100644 --- a/contracts/modules/base/HookRegistry.sol +++ b/contracts/modules/base/HookRegistry.sol @@ -75,7 +75,7 @@ abstract contract HookRegistry { function _hooksForType( HookType hType_ - ) private view returns (address[] storage) { + ) internal view returns (address[] storage) { if (hType_ == HookType.PreAction) { return _preActionHooks; } else { diff --git a/test/foundry/_temp_modules/base/BaseModule.t.sol b/test/foundry/_temp_modules/base/BaseModule.t.sol new file mode 100644 index 00000000..1d91893a --- /dev/null +++ b/test/foundry/_temp_modules/base/BaseModule.t.sol @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: BUSDL-1.1 +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; + +import "contracts/modules/base/BaseModule.sol"; +import "test/foundry/mocks/MockBaseModule.sol"; +import "contracts/lib/Errors.sol"; + +contract BaseModuleTest is Test { + MockBaseModule module; + address admin = address(123); + address ipaRegistry = address(456); + address moduleRegistry = address(789); + + event RequestPending(address indexed sender); + event RequestCompleted(address indexed sender); + + function setUp() public { + vm.prank(admin); + module = new MockBaseModule(admin, BaseModule.ModuleConstruction(ipaRegistry, moduleRegistry)); + } + + function test_baseModule_revert_constructorIpaRegistryIsZero() public { + vm.prank(admin); + vm.expectRevert(Errors.BaseModule_ZeroIpaRegistry.selector); + new MockBaseModule(admin, BaseModule.ModuleConstruction(address(0), moduleRegistry)); + } + + function test_baseModule_revert_constructorModuleRegistryIsZero() public { + vm.prank(admin); + vm.expectRevert(Errors.BaseModule_ZeroModuleRegistry.selector); + new MockBaseModule(admin, BaseModule.ModuleConstruction(ipaRegistry, address(0))); + } + + function test_baseModule_setup() public { + assertEq(module.IPA_REGISTRY(), ipaRegistry); + assertEq(module.MODULE_REGISTRY(), moduleRegistry); + } + + function test_baseModule_passesConfigParams() public { + bytes memory params = abi.encode(uint256(123)); + module.configure(address(123), params); + assertEq(module.callStackAt(0).caller, address(123)); + assertEq(module.callStackAt(0).params, params); + } + + function test_baseModule_correctExecutionOrderAndParams() public { + bytes memory params = abi.encode(uint256(123)); + vm.expectEmit(true, true, true, true); + emit RequestCompleted(address(123)); + module.execute(address(123), params, new bytes[](0), new bytes[](0)); + assertEq(module.callStackAt(0).caller, address(123)); + assertEq(module.callStackAt(0).params, params); + assertEq(module.callStackAt(1).caller, address(123)); + assertEq(module.callStackAt(1).params, params); + } + + function test_baseModule_revertPreHookWrongParamsLength() public { + bytes memory params = abi.encode(uint256(123)); + vm.expectRevert( + abi.encodeWithSelector( + Errors.BaseModule_HooksParamsLengthMismatch.selector, + uint8(HookRegistry.HookType.PreAction) + ) + ); + module.execute(address(123), params, new bytes[](1), new bytes[](0)); + } + + function test_baseModule_revertPostHookWrongParamsLength() public { + bytes memory params = abi.encode(uint256(123)); + vm.expectRevert( + abi.encodeWithSelector( + Errors.BaseModule_HooksParamsLengthMismatch.selector, + uint8(HookRegistry.HookType.PostAction) + ) + ); + module.execute(address(123), params, new bytes[](0), new bytes[](1)); + } + + // TODO: hook execution tests, waiting for base hook + + +} diff --git a/test/foundry/modules/base/HookRegistryTest.t.sol b/test/foundry/_temp_modules/base/HookRegistryTest.t.sol similarity index 100% rename from test/foundry/modules/base/HookRegistryTest.t.sol rename to test/foundry/_temp_modules/base/HookRegistryTest.t.sol diff --git a/test/foundry/mocks/MockBaseModule.sol b/test/foundry/mocks/MockBaseModule.sol new file mode 100644 index 00000000..7380a298 --- /dev/null +++ b/test/foundry/mocks/MockBaseModule.sol @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.18; + +import { BaseModule } from "contracts/modules/base/BaseModule.sol"; + +/// @title Mock BaseModule +/// @notice This mock contract is used for testing the base module flow +contract MockBaseModule is BaseModule { + address private _admin; + + struct BaseModuleCall { + address caller; + bytes params; + } + + BaseModuleCall[] private _callStack; + + constructor( + address admin_, + ModuleConstruction memory params_ + ) BaseModule(params_) { + _admin = admin_; + } + + function callStackAt(uint256 index_) + external + view + returns (BaseModuleCall memory) + { + return _callStack[index_]; + } + + function _hookRegistryAdmin() + internal + view + virtual + override + returns (address) + { + return _admin; + } + + function _configure( + address caller_, + bytes calldata params_ + ) internal virtual override { + _callStack.push(BaseModuleCall(caller_, params_)); + } + + function _verifyExecution( + address caller_, + bytes calldata params_ + ) internal virtual override { + _callStack.push(BaseModuleCall(caller_, params_)); + } + + function _performAction( + address caller_, + bytes calldata params_ + ) internal virtual override { + _callStack.push(BaseModuleCall(caller_, params_)); + } + + +} From a9d0cd788b3a1c9112b9587f43c25ad44da82939 Mon Sep 17 00:00:00 2001 From: Raul Date: Tue, 31 Oct 2023 21:12:46 -0300 Subject: [PATCH 9/9] comment gas difference and code coverage for now --- .github/workflows/test.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3e0a0a5d..177ff045 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,15 +54,15 @@ jobs: make test id: forge-test - - name: Gas Difference - run: - forge snapshot --gas-report --diff --desc - id: forge-gas-snapshot-diff + # - name: Gas Difference + # run: + # forge snapshot --gas-report --diff --desc + # id: forge-gas-snapshot-diff - - name: Code Coverage - run: - forge coverage --report lcov --report summary - id: forge-code-coverage + # - name: Code Coverage + # run: + # forge coverage --report lcov --report summary + # id: forge-code-coverage hardhat-test: strategy: