From 3a5d34c605b068acba580c94e41e35f1e89fd773 Mon Sep 17 00:00:00 2001 From: highskore Date: Thu, 8 Aug 2024 20:43:38 +0200 Subject: [PATCH 01/14] feat(RhinestoneModuleKit): track installed modules --- src/test/ModuleKitHelpers.sol | 17 ++++- src/test/utils/ERC4337Helpers.sol | 13 +++- src/test/utils/Storage.sol | 115 ++++++++++++++++++++++++++++++ test/Diff.t.sol | 40 ++++++++++- 4 files changed, 182 insertions(+), 3 deletions(-) diff --git a/src/test/ModuleKitHelpers.sol b/src/test/ModuleKitHelpers.sol index 4a976e9f..21f84cec 100644 --- a/src/test/ModuleKitHelpers.sol +++ b/src/test/ModuleKitHelpers.sol @@ -23,7 +23,10 @@ import { writeAccountEnv, getFactory, getHelper as getHelperFromStorage, - getAccountEnv as getAccountEnvFromStorage + getAccountEnv as getAccountEnvFromStorage, + getInstalledModules as getInstalledModulesFromStorage, + writeInstalledModule as writeInstalledModuleToStorage, + InstalledModule } from "./utils/Storage.sol"; library ModuleKitHelpers { @@ -262,6 +265,18 @@ library ModuleKitHelpers { userOpData.entrypoint = instance.aux.entrypoint; } + function getInstalledModules(AccountInstance memory) + internal + view + returns (InstalledModule[] memory) + { + return getInstalledModulesFromStorage(); + } + + function writeInstalledModule(InstalledModule memory module) internal { + writeInstalledModuleToStorage(module); + } + /*////////////////////////////////////////////////////////////////////////// CONTROL FLOW //////////////////////////////////////////////////////////////////////////*/ diff --git a/src/test/utils/ERC4337Helpers.sol b/src/test/utils/ERC4337Helpers.sol index 46264bcf..1fbdf477 100644 --- a/src/test/utils/ERC4337Helpers.sol +++ b/src/test/utils/ERC4337Helpers.sol @@ -18,7 +18,10 @@ import { getExpectRevert, writeExpectRevert, getGasIdentifier, - writeGasIdentifier + writeGasIdentifier, + writeInstalledModule, + getInstalledModules, + InstalledModule } from "./Storage.sol"; library ERC4337Helpers { @@ -70,6 +73,14 @@ library ERC4337Helpers { } } } + // ModuleInstalled(uint256, address) + else if ( + logs[i].topics[0] + == 0xd21d0b289f126c4b473ea641963e766833c2f13866e4ff480abd787c100ef123 + ) { + (uint256 moduleType, address module) = abi.decode(logs[i].data, (uint256, address)); + writeInstalledModule(InstalledModule(moduleType, module)); + } } isExpectRevert = getExpectRevert(); if (isExpectRevert != 0) { diff --git a/src/test/utils/Storage.sol b/src/test/utils/Storage.sol index d16800a2..ca845e9c 100644 --- a/src/test/utils/Storage.sol +++ b/src/test/utils/Storage.sol @@ -133,6 +133,121 @@ function getHelper(string memory helperType) view returns (address helper) { } } +/*////////////////////////////////////////////////////////////// + INSTALLED MODULE +//////////////////////////////////////////////////////////////*/ + +struct InstalledModule { + uint256 moduleType; + address moduleAddress; +} + +// Adds new address to the installed module array +// The array stores structs with the module type and the module address +function writeInstalledModule(InstalledModule memory module) { + bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); + bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); + uint256 moduleType = module.moduleType; + address moduleAddress = module.moduleAddress; + assembly { + // Get the length of the array + let length := sload(lengthSlot) + // Update the length of the array + sstore(lengthSlot, add(length, 1)) + // Calculate the location of the new slot in the array (elementSlot + length * (0x20 * 2)) + let location := add(elementSlot, mul(length, 0x40)) + // Store the module type and address in the new slot + sstore(location, moduleType) + sstore(add(location, 0x20), moduleAddress) + } +} + +// Removes all installed modules +function clearInstalledModules() { + bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); + bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); + assembly { + sstore(lengthSlot, 0) + for { let i := 0 } lt(i, 100) { i := add(i, 1) } { + sstore(add(elementSlot, mul(i, 0x40)), 0) + sstore(add(add(elementSlot, mul(i, 0x40)), 0x20), 0) + } + } +} + +// Removes a specific installed module +function removeInstalledModule(uint256 index) { + bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); + bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); + assembly { + // Get the length of the array + let length := sload(lengthSlot) + // Ensure the index is within bounds + if gt(length, index) { + // Calculate the location of the slot to remove + let location := add(elementSlot, mul(index, 0x40)) + // Calculate the location of the last slot + let lastLocation := add(elementSlot, mul(sub(length, 1), 0x40)) + // Load the last slot + let lastModuleType := sload(lastLocation) + let lastModuleAddress := sload(add(lastLocation, 0x20)) + // Store the last slot in the location of the slot to remove + sstore(location, lastModuleType) + sstore(add(location, 0x20), lastModuleAddress) + // Clear the last slot + sstore(lastLocation, 0) + sstore(add(lastLocation, 0x20), 0) + // Update the length of the array + sstore(lengthSlot, sub(length, 1)) + } + } +} + +// Returns all installed modules as an array of InstalledModule structs +function getInstalledModules() view returns (InstalledModule[] memory modules) { + bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); + bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); + assembly { + // Get the length of the array from storage + let length := sload(lengthSlot) + + // Each struct is 64 bytes (32 bytes for moduleType and 32 bytes for moduleAddress) + let structSize := 0x40 // 64 bytes + let size := mul(length, structSize) // Total size for structs + let totalSize := add(add(size, 0x40), mul(0x20, length)) + + // Allocate memory for the array + let freeMemoryPtr := mload(0x40) + modules := freeMemoryPtr + + // Store the length of the array in the first 32 bytes of memory + mstore(modules, length) + + // Update the free memory pointer to the end of the allocated memory + mstore(0x40, add(freeMemoryPtr, totalSize)) + + // Copy the structs from storage to memory + for { let i := 0 } lt(i, length) { i := add(i, 1) } { + // Calculate memory location for this struct + let structLocation := + add(add(freeMemoryPtr, add(0x40, mul(i, structSize))), mul(0x20, length)) + let storageLocation := add(elementSlot, mul(i, structSize)) // Storage location for each + // struct + + // Load the moduleType and moduleAddress from storage + let moduleType := sload(storageLocation) + let moduleAddress := sload(add(storageLocation, 0x20)) + + // Store the structLocation into memory + mstore(add(freeMemoryPtr, add(0x20, mul(i, 0x20))), structLocation) + + // Store the moduleType and moduleAddress into memory + mstore(structLocation, moduleType) + mstore(add(structLocation, 0x20), moduleAddress) + } + } +} + /*////////////////////////////////////////////////////////////// STRING STORAGE //////////////////////////////////////////////////////////////*/ diff --git a/test/Diff.t.sol b/test/Diff.t.sol index ee85f2ae..2e035627 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -11,7 +11,7 @@ import { MODULE_TYPE_FALLBACK, CALLTYPE_SINGLE } from "src/external/ERC7579.sol"; -import { getAccountType } from "src/test/utils/Storage.sol"; +import { getAccountType, InstalledModule } from "src/test/utils/Storage.sol"; import { toString } from "src/test/utils/Vm.sol"; contract ERC7579DifferentialModuleKitLibTest is BaseTest { @@ -137,6 +137,44 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(validator1Enabled); } + function test_getInstalledModules() public { + address newValidator = address(new MockValidator()); + address newValidator1 = address(new MockValidator()); + vm.label(newValidator, "2nd validator"); + + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator, + data: "" + }); + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator1, + data: "" + }); + + InstalledModule[] memory modules = instance.getInstalledModules(); + assertTrue(modules.length == 2); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + ); + + address newExecutor = address(new MockExecutor()); + instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); + modules = instance.getInstalledModules(); + + assertTrue(modules.length == 3); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[2].moduleAddress == newExecutor + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + && modules[2].moduleType == MODULE_TYPE_EXECUTOR + ); + } + function testRemoveValidator() public { address newValidator = address(new MockValidator()); instance.installModule({ From 87bef549a991c99b7db1d381c9f813210776f67e Mon Sep 17 00:00:00 2001 From: highskore Date: Fri, 9 Aug 2024 14:36:10 +0200 Subject: [PATCH 02/14] feat(Storage): track modules for each account, remove module on uninstall --- src/test/ModuleKitHelpers.sol | 35 +++++++- src/test/utils/ERC4337Helpers.sol | 22 ++++- src/test/utils/Storage.sol | 29 ++++--- test/Diff.t.sol | 132 ++++++++++++++++++++++++++++++ 4 files changed, 202 insertions(+), 16 deletions(-) diff --git a/src/test/ModuleKitHelpers.sol b/src/test/ModuleKitHelpers.sol index 21f84cec..52b7b523 100644 --- a/src/test/ModuleKitHelpers.sol +++ b/src/test/ModuleKitHelpers.sol @@ -26,6 +26,7 @@ import { getAccountEnv as getAccountEnvFromStorage, getInstalledModules as getInstalledModulesFromStorage, writeInstalledModule as writeInstalledModuleToStorage, + removeInstalledModule as removeInstaleldModuleFromStorage, InstalledModule } from "./utils/Storage.sol"; @@ -265,18 +266,44 @@ library ModuleKitHelpers { userOpData.entrypoint = instance.aux.entrypoint; } - function getInstalledModules(AccountInstance memory) + function getInstalledModules(AccountInstance memory instance) internal view returns (InstalledModule[] memory) { - return getInstalledModulesFromStorage(); + return getInstalledModulesFromStorage(instance.account); } - function writeInstalledModule(InstalledModule memory module) internal { - writeInstalledModuleToStorage(module); + function writeInstalledModule( + AccountInstance memory instance, + InstalledModule memory module + ) + internal + { + writeInstalledModuleToStorage(module, instance.account); } + function removeInstalledModule( + AccountInstance memory instance, + uint256 moduleType, + address moduleAddress + ) + internal + { + // Get installed modules for account + InstalledModule[] memory installedModules = getInstalledModules(instance); + // Find module to remove (not super scalable at high module counts) + for (uint256 i; i < installedModules.length; i++) { + if ( + installedModules[i].moduleType == moduleType + && installedModules[i].moduleAddress == moduleAddress + ) { + // Remove module from storage + removeInstaleldModuleFromStorage(i, instance.account); + return; + } + } + } /*////////////////////////////////////////////////////////////////////////// CONTROL FLOW //////////////////////////////////////////////////////////////////////////*/ diff --git a/src/test/utils/ERC4337Helpers.sol b/src/test/utils/ERC4337Helpers.sol index 1fbdf477..1d58e9f7 100644 --- a/src/test/utils/ERC4337Helpers.sol +++ b/src/test/utils/ERC4337Helpers.sol @@ -21,6 +21,7 @@ import { writeGasIdentifier, writeInstalledModule, getInstalledModules, + removeInstalledModule, InstalledModule } from "./Storage.sol"; @@ -79,7 +80,26 @@ library ERC4337Helpers { == 0xd21d0b289f126c4b473ea641963e766833c2f13866e4ff480abd787c100ef123 ) { (uint256 moduleType, address module) = abi.decode(logs[i].data, (uint256, address)); - writeInstalledModule(InstalledModule(moduleType, module)); + writeInstalledModule(InstalledModule(moduleType, module), logs[i].emitter); + } + // ModuleUninstalled(uint256, address) + else if ( + logs[i].topics[0] + == 0x341347516a9de374859dfda710fa4828b2d48cb57d4fbe4c1149612b8e02276e + ) { + (uint256 moduleType, address module) = abi.decode(logs[i].data, (uint256, address)); + // Get all installed modules + InstalledModule[] memory installedModules = getInstalledModules(logs[i].emitter); + // Remove the uninstalled module from the list of installed modules + for (uint256 j; j < installedModules.length; j++) { + if ( + installedModules[j].moduleAddress == module + && installedModules[j].moduleType == moduleType + ) { + removeInstalledModule(j, logs[i].emitter); + break; + } + } } } isExpectRevert = getExpectRevert(); diff --git a/src/test/utils/Storage.sol b/src/test/utils/Storage.sol index ca845e9c..90603045 100644 --- a/src/test/utils/Storage.sol +++ b/src/test/utils/Storage.sol @@ -142,10 +142,11 @@ struct InstalledModule { address moduleAddress; } -// Adds new address to the installed module array -// The array stores structs with the module type and the module address -function writeInstalledModule(InstalledModule memory module) { - bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); +// Adds new address to the installed module array for the given account +function writeInstalledModule(InstalledModule memory module, address account) { + bytes32 lengthSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) + ); bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); uint256 moduleType = module.moduleType; address moduleAddress = module.moduleAddress; @@ -163,8 +164,10 @@ function writeInstalledModule(InstalledModule memory module) { } // Removes all installed modules -function clearInstalledModules() { - bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); +function clearInstalledModules(address account) { + bytes32 lengthSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) + ); bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); assembly { sstore(lengthSlot, 0) @@ -176,8 +179,10 @@ function clearInstalledModules() { } // Removes a specific installed module -function removeInstalledModule(uint256 index) { - bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); +function removeInstalledModule(uint256 index, address account) { + bytes32 lengthSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) + ); bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); assembly { // Get the length of the array @@ -203,9 +208,11 @@ function removeInstalledModule(uint256 index) { } } -// Returns all installed modules as an array of InstalledModule structs -function getInstalledModules() view returns (InstalledModule[] memory modules) { - bytes32 lengthSlot = keccak256(abi.encode("ModuleKit.InstalledModuleSlot")); +// Returns all installed modules for the given account +function getInstalledModules(address account) view returns (InstalledModule[] memory modules) { + bytes32 lengthSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) + ); bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); assembly { // Get the length of the array from storage diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 2e035627..ba443a78 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -175,6 +175,138 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } + function test_getInstalledModules_DifferentInstances() public { + address newValidator = address(new MockValidator()); + address newValidator1 = address(new MockValidator()); + vm.label(newValidator, "2nd validator"); + + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator, + data: "" + }); + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator1, + data: "" + }); + + InstalledModule[] memory modules = instance.getInstalledModules(); + assertTrue(modules.length == 2); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + ); + + address newExecutor = address(new MockExecutor()); + instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); + modules = instance.getInstalledModules(); + + assertTrue(modules.length == 3); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[2].moduleAddress == newExecutor + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + && modules[2].moduleType == MODULE_TYPE_EXECUTOR + ); + + // Deploy new instance using current env + AccountInstance memory newInstance = makeAccountInstance("newSalt"); + assertTrue(newInstance.account.code.length == 0); + newInstance.deployAccount(); + assertTrue(newInstance.account.code.length > 0); + + // Install modules on new instance + newInstance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator, + data: "" + }); + newInstance.installModule({ + moduleTypeId: MODULE_TYPE_EXECUTOR, + module: newExecutor, + data: "" + }); + + // Get installed modules on new instance + InstalledModule[] memory newModules = newInstance.getInstalledModules(); + assertTrue(newModules.length == 2); + assertTrue( + newModules[0].moduleAddress == newValidator + && newModules[1].moduleAddress == newExecutor + && newModules[0].moduleType == MODULE_TYPE_VALIDATOR + && newModules[1].moduleType == MODULE_TYPE_EXECUTOR + ); + + // Old instance modules should still be the same + modules = instance.getInstalledModules(); + assertTrue(modules.length == 3); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[2].moduleAddress == newExecutor + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + && modules[2].moduleType == MODULE_TYPE_EXECUTOR + ); + } + + function test_getInstalledModules_AfterUninstall() public { + address newValidator = address(new MockValidator()); + address newValidator1 = address(new MockValidator()); + vm.label(newValidator, "2nd validator"); + + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator, + data: "" + }); + instance.installModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator1, + data: "" + }); + + InstalledModule[] memory modules = instance.getInstalledModules(); + assertTrue(modules.length == 2); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + ); + + address newExecutor = address(new MockExecutor()); + instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); + modules = instance.getInstalledModules(); + + assertTrue(modules.length == 3); + assertTrue( + modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 + && modules[2].moduleAddress == newExecutor + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + && modules[2].moduleType == MODULE_TYPE_EXECUTOR + ); + + // Uninstall module + instance.uninstallModule({ + moduleTypeId: MODULE_TYPE_VALIDATOR, + module: newValidator, + data: "" + }); + + // Get installed modules + modules = instance.getInstalledModules(); + + assertTrue(modules.length == 2); + assertTrue( + modules[1].moduleAddress == newValidator1 && modules[0].moduleAddress == newExecutor + && modules[1].moduleType == MODULE_TYPE_VALIDATOR + && modules[0].moduleType == MODULE_TYPE_EXECUTOR + ); + } + function testRemoveValidator() public { address newValidator = address(new MockValidator()); instance.installModule({ From 635e49b0076308b65c60b70d8bb70985cd901dc2 Mon Sep 17 00:00:00 2001 From: highskore Date: Fri, 9 Aug 2024 17:39:32 +0200 Subject: [PATCH 03/14] feat(Storage): convert array to double linked list --- src/test/utils/Storage.sol | 140 ++++++++++++++++++++++++++----------- test/Diff.t.sol | 6 +- 2 files changed, 102 insertions(+), 44 deletions(-) diff --git a/src/test/utils/Storage.sol b/src/test/utils/Storage.sol index 90603045..474677d8 100644 --- a/src/test/utils/Storage.sol +++ b/src/test/utils/Storage.sol @@ -142,39 +142,71 @@ struct InstalledModule { address moduleAddress; } -// Adds new address to the installed module array for the given account +// Adds new address to the installed module linked list for the given account +// The list is stored in storage as a linked list in the following format: +// --------------------------------------------------------------------- +// | Slot | Value | +// |----------------------------------------------------------|--------| +// | keccak256(abi.encode("ModuleKit.InstalledModuleSlot.")); | length | +// | keccak256(abi.encode("ModuleKit.InstalledModuleHead.")); | head | +// | keccak256(abi.encode("ModuleKit.InstalledModuleTail.")); | tail | +// | keccak256(abi.encode(lengthSlot)) - initially X | element| +// --------------------------------------------------------------------- +// +// The elements are stored in the following way: +// -------------------------- +// | Slot | Value | +// |------------------------| +// | X | moduleType | +// | X + 0x20 | moduleAddr | +// | X + 0x40 | prev | +// | X + 0x60 | next | +// -------------------------- function writeInstalledModule(InstalledModule memory module, address account) { bytes32 lengthSlot = keccak256( abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) ); + bytes32 headSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleHead.", keccak256(abi.encodePacked(account))) + ); + bytes32 tailSlot = + keccak256(abi.encode("ModuleKit.InstalledModuleTail", keccak256(abi.encodePacked(account)))); bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); uint256 moduleType = module.moduleType; address moduleAddress = module.moduleAddress; assembly { // Get the length of the array let length := sload(lengthSlot) - // Update the length of the array + let nextSlot + let oldTail + switch iszero(length) + case 1 { + // If length is zero, set element slot to head and tail + sstore(headSlot, elementSlot) + sstore(tailSlot, elementSlot) + oldTail := elementSlot + nextSlot := elementSlot + } + default { + oldTail := sload(tailSlot) + // Set the new elemeont slot to the old tail + 0x80 + elementSlot := add(oldTail, 0x80) + // Set the old tail next slot to the new element slot + sstore(add(oldTail, 0x60), elementSlot) + // Update tailSlot to point to the new element slot + sstore(tailSlot, elementSlot) + // Set nextSlot to the head slot + nextSlot := sload(headSlot) + } + // Update the length of the list sstore(lengthSlot, add(length, 1)) - // Calculate the location of the new slot in the array (elementSlot + length * (0x20 * 2)) - let location := add(elementSlot, mul(length, 0x40)) // Store the module type and address in the new slot - sstore(location, moduleType) - sstore(add(location, 0x20), moduleAddress) - } -} - -// Removes all installed modules -function clearInstalledModules(address account) { - bytes32 lengthSlot = keccak256( - abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) - ); - bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); - assembly { - sstore(lengthSlot, 0) - for { let i := 0 } lt(i, 100) { i := add(i, 1) } { - sstore(add(elementSlot, mul(i, 0x40)), 0) - sstore(add(add(elementSlot, mul(i, 0x40)), 0x20), 0) - } + sstore(elementSlot, moduleType) + sstore(add(elementSlot, 0x20), moduleAddress) + // Store the old tail as the prev slot + sstore(add(elementSlot, 0x40), oldTail) + // Store the head as the next slot + sstore(add(elementSlot, 0x60), nextSlot) } } @@ -183,26 +215,46 @@ function removeInstalledModule(uint256 index, address account) { bytes32 lengthSlot = keccak256( abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) ); - bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); + bytes32 headSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleHead.", keccak256(abi.encodePacked(account))) + ); + bytes32 tailSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleTail.", keccak256(abi.encodePacked(account))) + ); assembly { - // Get the length of the array + // Get the length of the list let length := sload(lengthSlot) + // Get the initial element slot + let elementSlot := sload(headSlot) // Ensure the index is within bounds - if gt(length, index) { - // Calculate the location of the slot to remove - let location := add(elementSlot, mul(index, 0x40)) - // Calculate the location of the last slot - let lastLocation := add(elementSlot, mul(sub(length, 1), 0x40)) - // Load the last slot - let lastModuleType := sload(lastLocation) - let lastModuleAddress := sload(add(lastLocation, 0x20)) - // Store the last slot in the location of the slot to remove - sstore(location, lastModuleType) - sstore(add(location, 0x20), lastModuleAddress) - // Clear the last slot - sstore(lastLocation, 0) - sstore(add(lastLocation, 0x20), 0) - // Update the length of the array + if lt(index, length) { + // Traverse to the node to remove + for { let i := 0 } lt(i, index) { i := add(i, 1) } { + elementSlot := sload(add(elementSlot, 0x60)) + } + + // Get the previous and next slots + let prevSlot := sload(add(elementSlot, 0x40)) + let nextSlot := sload(add(elementSlot, 0x60)) + + // Update the previous slot's next pointer + sstore(add(prevSlot, 0x60), nextSlot) + // Update the next slot's previous pointer + sstore(add(nextSlot, 0x40), prevSlot) + + // Handle removing the head + if eq(elementSlot, sload(headSlot)) { sstore(headSlot, nextSlot) } + + // Handle removing the tail + if eq(elementSlot, sload(tailSlot)) { sstore(tailSlot, prevSlot) } + + // Clear the removed node + sstore(elementSlot, 0) + sstore(add(elementSlot, 0x20), 0) + sstore(add(elementSlot, 0x40), 0) + sstore(add(elementSlot, 0x60), 0) + + // Update the length of the list sstore(lengthSlot, sub(length, 1)) } } @@ -213,7 +265,9 @@ function getInstalledModules(address account) view returns (InstalledModule[] me bytes32 lengthSlot = keccak256( abi.encode("ModuleKit.InstalledModuleSlot.", keccak256(abi.encodePacked(account))) ); - bytes32 elementSlot = keccak256(abi.encode(lengthSlot)); + bytes32 headSlot = keccak256( + abi.encode("ModuleKit.InstalledModuleHead.", keccak256(abi.encodePacked(account))) + ); assembly { // Get the length of the array from storage let length := sload(lengthSlot) @@ -233,13 +287,14 @@ function getInstalledModules(address account) view returns (InstalledModule[] me // Update the free memory pointer to the end of the allocated memory mstore(0x40, add(freeMemoryPtr, totalSize)) + // Get the head of the linked list + let storageLocation := sload(headSlot) + // Copy the structs from storage to memory for { let i := 0 } lt(i, length) { i := add(i, 1) } { // Calculate memory location for this struct let structLocation := add(add(freeMemoryPtr, add(0x40, mul(i, structSize))), mul(0x20, length)) - let storageLocation := add(elementSlot, mul(i, structSize)) // Storage location for each - // struct // Load the moduleType and moduleAddress from storage let moduleType := sload(storageLocation) @@ -251,6 +306,9 @@ function getInstalledModules(address account) view returns (InstalledModule[] me // Store the moduleType and moduleAddress into memory mstore(structLocation, moduleType) mstore(add(structLocation, 0x20), moduleAddress) + + // Move to the next element in the linked list + storageLocation := sload(add(storageLocation, 0x60)) } } } diff --git a/test/Diff.t.sol b/test/Diff.t.sol index ba443a78..a8b2c60c 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -301,9 +301,9 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(modules.length == 2); assertTrue( - modules[1].moduleAddress == newValidator1 && modules[0].moduleAddress == newExecutor - && modules[1].moduleType == MODULE_TYPE_VALIDATOR - && modules[0].moduleType == MODULE_TYPE_EXECUTOR + modules[0].moduleAddress == newValidator1 && modules[1].moduleAddress == newExecutor + && modules[0].moduleType == MODULE_TYPE_VALIDATOR + && modules[1].moduleType == MODULE_TYPE_EXECUTOR ); } From 8d447d49a9e272a8e523e9d797ec9372a69918d4 Mon Sep 17 00:00:00 2001 From: highskore Date: Fri, 9 Aug 2024 18:09:27 +0200 Subject: [PATCH 04/14] fix(Diff): skip get modules tests if env is kernel or safe --- test/Diff.t.sol | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index f4f628ff..5da43e83 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -138,7 +138,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(validator1Enabled); } - function test_getInstalledModules() public { + function test_getInstalledModules() public whenEnvIsNotKernelOrSafe { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -176,7 +176,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } - function test_getInstalledModules_DifferentInstances() public { + function test_getInstalledModules_DifferentInstances() public whenEnvIsNotKernelOrSafe { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -253,7 +253,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } - function test_getInstalledModules_AfterUninstall() public { + function test_getInstalledModules_AfterUninstall() public whenEnvIsNotKernelOrSafe { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -536,4 +536,18 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(newInstance.account.code.length > 0); } + + /*////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////*/ + + // Used to skip tests when env is kernel or safe as sometimes they don't emit events on module + // installation + modifier whenEnvIsNotKernelOrSafe() { + AccountType env = ModuleKitHelpers.getAccountType(); + if (env == AccountType.KERNEL || env == AccountType.SAFE) { + return; + } + _; + } } From 558341434e80252fc27344e0629970a859aac535 Mon Sep 17 00:00:00 2001 From: highskore Date: Fri, 9 Aug 2024 18:15:52 +0200 Subject: [PATCH 05/14] chore: fix comment --- test/Diff.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 5da43e83..d3c44d59 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -541,11 +541,11 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { MODIFIERS //////////////////////////////////////////////////////////////*/ - // Used to skip tests when env is kernel or safe as sometimes they don't emit events on module - // installation + // Used to skip tests when env is kernel or safe as sometimes + // they don't emit events on module installation modifier whenEnvIsNotKernelOrSafe() { - AccountType env = ModuleKitHelpers.getAccountType(); - if (env == AccountType.KERNEL || env == AccountType.SAFE) { + AccountType _env = ModuleKitHelpers.getAccountType(); + if (_env == AccountType.KERNEL || _env == AccountType.SAFE) { return; } _; From 84c3682580f59f320e8693a9609f8378f0e7a892 Mon Sep 17 00:00:00 2001 From: highskore Date: Fri, 9 Aug 2024 18:22:23 +0200 Subject: [PATCH 06/14] fix: remove variables from modifier --- test/Diff.t.sol | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index d3c44d59..044eb455 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -138,7 +138,10 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(validator1Enabled); } - function test_getInstalledModules() public whenEnvIsNotKernelOrSafe { + function test_getInstalledModules() + public + whenEnvIsNotKernelOrSafe(ModuleKitHelpers.getAccountType()) + { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -176,7 +179,10 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } - function test_getInstalledModules_DifferentInstances() public whenEnvIsNotKernelOrSafe { + function test_getInstalledModules_DifferentInstances() + public + whenEnvIsNotKernelOrSafe(ModuleKitHelpers.getAccountType()) + { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -253,7 +259,10 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } - function test_getInstalledModules_AfterUninstall() public whenEnvIsNotKernelOrSafe { + function test_getInstalledModules_AfterUninstall() + public + whenEnvIsNotKernelOrSafe(ModuleKitHelpers.getAccountType()) + { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -543,8 +552,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Used to skip tests when env is kernel or safe as sometimes // they don't emit events on module installation - modifier whenEnvIsNotKernelOrSafe() { - AccountType _env = ModuleKitHelpers.getAccountType(); + modifier whenEnvIsNotKernelOrSafe(AccountType _env) { if (_env == AccountType.KERNEL || _env == AccountType.SAFE) { return; } From 2f3986d139df60d4e1f5b463033ed267bbaaa901 Mon Sep 17 00:00:00 2001 From: highskore Date: Sat, 10 Aug 2024 02:30:35 +0200 Subject: [PATCH 07/14] fix(Diff): expect different results for Safe installed modules --- test/Diff.t.sol | 116 +++++++++++++++++++++++++----------------------- 1 file changed, 60 insertions(+), 56 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 044eb455..49e4794a 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -138,10 +138,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(validator1Enabled); } - function test_getInstalledModules() - public - whenEnvIsNotKernelOrSafe(ModuleKitHelpers.getAccountType()) - { + function test_getInstalledModules() public whenEnvIsNotKernelOrSafe { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -158,31 +155,31 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { }); InstalledModule[] memory modules = instance.getInstalledModules(); - assertTrue(modules.length == 2); + AccountType env = ModuleKitHelpers.getAccountType(); + assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); + uint256 expectedIndex = env == AccountType.SAFE ? 1 : 0; assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR ); address newExecutor = address(new MockExecutor()); instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); modules = instance.getInstalledModules(); - - assertTrue(modules.length == 3); + assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[2].moduleAddress == newExecutor - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR - && modules[2].moduleType == MODULE_TYPE_EXECUTOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex + 2].moduleAddress == newExecutor + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR ); } - function test_getInstalledModules_DifferentInstances() - public - whenEnvIsNotKernelOrSafe(ModuleKitHelpers.getAccountType()) - { + function test_getInstalledModules_DifferentInstances() public whenEnvIsNotKernelOrSafe { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -199,24 +196,27 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { }); InstalledModule[] memory modules = instance.getInstalledModules(); - assertTrue(modules.length == 2); + AccountType env = ModuleKitHelpers.getAccountType(); + assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); + uint256 expectedIndex = env == AccountType.SAFE ? 1 : 0; assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR ); address newExecutor = address(new MockExecutor()); instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); modules = instance.getInstalledModules(); - - assertTrue(modules.length == 3); + assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[2].moduleAddress == newExecutor - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR - && modules[2].moduleType == MODULE_TYPE_EXECUTOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex + 2].moduleAddress == newExecutor + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR ); // Deploy new instance using current env @@ -249,20 +249,18 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Old instance modules should still be the same modules = instance.getInstalledModules(); - assertTrue(modules.length == 3); + assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[2].moduleAddress == newExecutor - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR - && modules[2].moduleType == MODULE_TYPE_EXECUTOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex + 2].moduleAddress == newExecutor + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR ); } - function test_getInstalledModules_AfterUninstall() - public - whenEnvIsNotKernelOrSafe(ModuleKitHelpers.getAccountType()) - { + function test_getInstalledModules_AfterUninstall() public whenEnvIsNotKernelOrSafe { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -279,24 +277,28 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { }); InstalledModule[] memory modules = instance.getInstalledModules(); - assertTrue(modules.length == 2); + AccountType env = ModuleKitHelpers.getAccountType(); + assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); + uint256 expectedIndex = env == AccountType.SAFE ? 1 : 0; assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR ); address newExecutor = address(new MockExecutor()); instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); modules = instance.getInstalledModules(); - assertTrue(modules.length == 3); + assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); assertTrue( - modules[0].moduleAddress == newValidator && modules[1].moduleAddress == newValidator1 - && modules[2].moduleAddress == newExecutor - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_VALIDATOR - && modules[2].moduleType == MODULE_TYPE_EXECUTOR + modules[expectedIndex].moduleAddress == newValidator + && modules[expectedIndex + 1].moduleAddress == newValidator1 + && modules[expectedIndex + 2].moduleAddress == newExecutor + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR ); // Uninstall module @@ -309,11 +311,12 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Get installed modules modules = instance.getInstalledModules(); - assertTrue(modules.length == 2); + assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); assertTrue( - modules[0].moduleAddress == newValidator1 && modules[1].moduleAddress == newExecutor - && modules[0].moduleType == MODULE_TYPE_VALIDATOR - && modules[1].moduleType == MODULE_TYPE_EXECUTOR + modules[expectedIndex].moduleAddress == newValidator1 + && modules[expectedIndex + 1].moduleAddress == newExecutor + && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && modules[expectedIndex + 1].moduleType == MODULE_TYPE_EXECUTOR ); } @@ -552,8 +555,9 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Used to skip tests when env is kernel or safe as sometimes // they don't emit events on module installation - modifier whenEnvIsNotKernelOrSafe(AccountType _env) { - if (_env == AccountType.KERNEL || _env == AccountType.SAFE) { + modifier whenEnvIsNotKernelOrSafe() { + AccountType env = ModuleKitHelpers.getAccountType(); + if (env == AccountType.KERNEL) { return; } _; From 1e1c857f00e6e4ed539e30e6f7e873ff92979f44 Mon Sep 17 00:00:00 2001 From: highskore Date: Sun, 11 Aug 2024 16:28:59 +0200 Subject: [PATCH 08/14] fix(Diff): dont deploy instance --- test/Diff.t.sol | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 49e4794a..2df0f42b 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -221,9 +221,6 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Deploy new instance using current env AccountInstance memory newInstance = makeAccountInstance("newSalt"); - assertTrue(newInstance.account.code.length == 0); - newInstance.deployAccount(); - assertTrue(newInstance.account.code.length > 0); // Install modules on new instance newInstance.installModule({ @@ -239,12 +236,12 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Get installed modules on new instance InstalledModule[] memory newModules = newInstance.getInstalledModules(); - assertTrue(newModules.length == 2); + assertTrue(newModules.length == (env == AccountType.SAFE ? 3 : 2)); assertTrue( - newModules[0].moduleAddress == newValidator - && newModules[1].moduleAddress == newExecutor - && newModules[0].moduleType == MODULE_TYPE_VALIDATOR - && newModules[1].moduleType == MODULE_TYPE_EXECUTOR + newModules[expectedIndex].moduleAddress == newValidator + && newModules[expectedIndex + 1].moduleAddress == newExecutor + && newModules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR + && newModules[expectedIndex + 1].moduleType == MODULE_TYPE_EXECUTOR ); // Old instance modules should still be the same From dc50fd2f22645fab175b52390888de7cff276961 Mon Sep 17 00:00:00 2001 From: highskore Date: Sun, 11 Aug 2024 17:11:39 +0200 Subject: [PATCH 09/14] feat(ModuleKitHelpers): record module install events on deployAccount --- src/test/ModuleKitHelpers.sol | 20 ++++++++++++++++++-- test/Diff.t.sol | 3 +++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/test/ModuleKitHelpers.sol b/src/test/ModuleKitHelpers.sol index 52b7b523..72c3115d 100644 --- a/src/test/ModuleKitHelpers.sol +++ b/src/test/ModuleKitHelpers.sol @@ -26,9 +26,10 @@ import { getAccountEnv as getAccountEnvFromStorage, getInstalledModules as getInstalledModulesFromStorage, writeInstalledModule as writeInstalledModuleToStorage, - removeInstalledModule as removeInstaleldModuleFromStorage, + removeInstalledModule as removeInstalledModuleFromStorage, InstalledModule } from "./utils/Storage.sol"; +import { recordLogs, VmSafe, getRecordedLogs } from "./utils/Vm.sol"; library ModuleKitHelpers { /*////////////////////////////////////////////////////////////////////////// @@ -299,7 +300,7 @@ library ModuleKitHelpers { && installedModules[i].moduleAddress == moduleAddress ) { // Remove module from storage - removeInstaleldModuleFromStorage(i, instance.account); + removeInstalledModuleFromStorage(i, instance.account); return; } } @@ -372,7 +373,22 @@ library ModuleKitHelpers { } function deployAccount(AccountInstance memory instance) internal { + // Record logs to track installed modules + recordLogs(); + // Deploy account HelperBase(instance.accountHelper).deployAccount(instance); + // Parse logs and determine if a module was installed + VmSafe.Log[] memory logs = getRecordedLogs(); + for (uint256 i; i < logs.length; i++) { + // ModuleInstalled(uint256, address) + if ( + logs[i].topics[0] + == 0xd21d0b289f126c4b473ea641963e766833c2f13866e4ff480abd787c100ef123 + ) { + (uint256 moduleType, address module) = abi.decode(logs[i].data, (uint256, address)); + writeInstalledModuleToStorage(InstalledModule(moduleType, module), logs[i].emitter); + } + } } function setAccountType(AccountInstance memory, AccountType env) internal { diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 2df0f42b..91c833cc 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -221,6 +221,9 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { // Deploy new instance using current env AccountInstance memory newInstance = makeAccountInstance("newSalt"); + assertTrue(newInstance.account.code.length == 0); + newInstance.deployAccount(); + assertTrue(newInstance.account.code.length > 0); // Install modules on new instance newInstance.installModule({ From e3afafa86ca99b45a0b94c8ad81f9a490bcec9f2 Mon Sep 17 00:00:00 2001 From: highskore Date: Sun, 11 Aug 2024 18:08:54 +0200 Subject: [PATCH 10/14] chore: import AccountType enum --- src/ModuleKit.sol | 7 ++++++- test/Diff.t.sol | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ModuleKit.sol b/src/ModuleKit.sol index d0b17dc8..35ebc6b5 100644 --- a/src/ModuleKit.sol +++ b/src/ModuleKit.sol @@ -2,7 +2,12 @@ pragma solidity ^0.8.23; /* solhint-disable no-unused-import */ -import { UserOpData, AccountInstance, RhinestoneModuleKit } from "./test/RhinestoneModuleKit.sol"; +import { + UserOpData, + AccountInstance, + RhinestoneModuleKit, + AccountType +} from "./test/RhinestoneModuleKit.sol"; import { ModuleKitHelpers } from "./test/ModuleKitHelpers.sol"; import { ModuleKitUserOp } from "./test/ModuleKitUserOp.sol"; import { PackedUserOperation } from "./external/ERC4337.sol"; diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 91c833cc..156045a5 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -13,7 +13,6 @@ import { } from "src/external/ERC7579.sol"; import { getAccountType, InstalledModule } from "src/test/utils/Storage.sol"; import { toString } from "src/test/utils/Vm.sol"; -import { AccountType } from "src/test/RhinestoneModuleKit.sol"; contract ERC7579DifferentialModuleKitLibTest is BaseTest { using ModuleKitHelpers for *; From ef88d359a1f2af75e781719faef7b5f816a57c8e Mon Sep 17 00:00:00 2001 From: highskore Date: Wed, 14 Aug 2024 17:47:39 +0200 Subject: [PATCH 11/14] chore(Diff): rename modifier --- test/Diff.t.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 156045a5..c6ed8f60 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -137,7 +137,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(validator1Enabled); } - function test_getInstalledModules() public whenEnvIsNotKernelOrSafe { + function test_getInstalledModules() public whenEnvIsNotKernel { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -178,7 +178,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } - function test_getInstalledModules_DifferentInstances() public whenEnvIsNotKernelOrSafe { + function test_getInstalledModules_DifferentInstances() public whenEnvIsNotKernel { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -259,7 +259,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { ); } - function test_getInstalledModules_AfterUninstall() public whenEnvIsNotKernelOrSafe { + function test_getInstalledModules_AfterUninstall() public whenEnvIsNotKernel { address newValidator = address(new MockValidator()); address newValidator1 = address(new MockValidator()); vm.label(newValidator, "2nd validator"); @@ -552,9 +552,8 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { MODIFIERS //////////////////////////////////////////////////////////////*/ - // Used to skip tests when env is kernel or safe as sometimes - // they don't emit events on module installation - modifier whenEnvIsNotKernelOrSafe() { + // Used to skip tests when env is kernel as they don't emit events on module installation + modifier whenEnvIsNotKernel() { AccountType env = ModuleKitHelpers.getAccountType(); if (env == AccountType.KERNEL) { return; From 67b49bd9856b65c97251695b57b9671fa0d00550 Mon Sep 17 00:00:00 2001 From: highskore Date: Thu, 15 Aug 2024 01:14:58 +0200 Subject: [PATCH 12/14] refactor(getModulesAndAssert): move logic to reusable function --- test/Diff.t.sol | 178 ++++++++++++++++++++++++++---------------------- 1 file changed, 97 insertions(+), 81 deletions(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index c6ed8f60..85140f60 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -13,6 +13,7 @@ import { } from "src/external/ERC7579.sol"; import { getAccountType, InstalledModule } from "src/test/utils/Storage.sol"; import { toString } from "src/test/utils/Vm.sol"; +import { console2 } from "forge-std/console2.sol"; contract ERC7579DifferentialModuleKitLibTest is BaseTest { using ModuleKitHelpers for *; @@ -153,28 +154,25 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { data: "" }); - InstalledModule[] memory modules = instance.getInstalledModules(); - AccountType env = ModuleKitHelpers.getAccountType(); - assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); - uint256 expectedIndex = env == AccountType.SAFE ? 1 : 0; - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 2, [newValidator, newValidator1], [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR] + ), + instance ); address newExecutor = address(new MockExecutor()); instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); - modules = instance.getInstalledModules(); - assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex + 2].moduleAddress == newExecutor - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR + + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 3, + [newValidator, newValidator1, newExecutor], + [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR] + ), + instance ); } @@ -194,28 +192,25 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { data: "" }); - InstalledModule[] memory modules = instance.getInstalledModules(); - AccountType env = ModuleKitHelpers.getAccountType(); - assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); - uint256 expectedIndex = env == AccountType.SAFE ? 1 : 0; - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 2, [newValidator, newValidator1], [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR] + ), + instance ); address newExecutor = address(new MockExecutor()); instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); - modules = instance.getInstalledModules(); - assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex + 2].moduleAddress == newExecutor - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR + + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 3, + [newValidator, newValidator1, newExecutor], + [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR] + ), + instance ); // Deploy new instance using current env @@ -236,26 +231,22 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { data: "" }); - // Get installed modules on new instance - InstalledModule[] memory newModules = newInstance.getInstalledModules(); - assertTrue(newModules.length == (env == AccountType.SAFE ? 3 : 2)); - assertTrue( - newModules[expectedIndex].moduleAddress == newValidator - && newModules[expectedIndex + 1].moduleAddress == newExecutor - && newModules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && newModules[expectedIndex + 1].moduleType == MODULE_TYPE_EXECUTOR + // Assert installed modules on new instance + this._getModulesAndAssert( + abi.encode( + 2, [newValidator, newExecutor], [MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR] + ), + newInstance ); // Old instance modules should still be the same - modules = instance.getInstalledModules(); - assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex + 2].moduleAddress == newExecutor - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR + this._getModulesAndAssert( + abi.encode( + 3, + [newValidator, newValidator1, newExecutor], + [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR] + ), + instance ); } @@ -275,29 +266,25 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { data: "" }); - InstalledModule[] memory modules = instance.getInstalledModules(); - AccountType env = ModuleKitHelpers.getAccountType(); - assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); - uint256 expectedIndex = env == AccountType.SAFE ? 1 : 0; - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 2, [newValidator, newValidator1], [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR] + ), + instance ); address newExecutor = address(new MockExecutor()); instance.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: newExecutor, data: "" }); - modules = instance.getInstalledModules(); - assertTrue(modules.length == (env == AccountType.SAFE ? 4 : 3)); - assertTrue( - modules[expectedIndex].moduleAddress == newValidator - && modules[expectedIndex + 1].moduleAddress == newValidator1 - && modules[expectedIndex + 2].moduleAddress == newExecutor - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 2].moduleType == MODULE_TYPE_EXECUTOR + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 3, // length + [newValidator, newValidator1, newExecutor], // expectedAddresses + [MODULE_TYPE_VALIDATOR, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR] // expectedTypes + ), + instance ); // Uninstall module @@ -307,15 +294,12 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { data: "" }); - // Get installed modules - modules = instance.getInstalledModules(); - - assertTrue(modules.length == (env == AccountType.SAFE ? 3 : 2)); - assertTrue( - modules[expectedIndex].moduleAddress == newValidator1 - && modules[expectedIndex + 1].moduleAddress == newExecutor - && modules[expectedIndex].moduleType == MODULE_TYPE_VALIDATOR - && modules[expectedIndex + 1].moduleType == MODULE_TYPE_EXECUTOR + // Assert installed modules + this._getModulesAndAssert( + abi.encode( + 2, [newValidator1, newExecutor], [MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR] + ), + instance ); } @@ -536,7 +520,7 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { } /*////////////////////////////////////////////////////////////// - INTERNAL + HELPERS //////////////////////////////////////////////////////////////*/ function _usingAccountEnv(string memory env) internal usingAccountEnv(env.toAccountType()) { @@ -548,6 +532,38 @@ contract ERC7579DifferentialModuleKitLibTest is BaseTest { assertTrue(newInstance.account.code.length > 0); } + function _getModulesAndAssert( + bytes calldata expectedResultBytes, + AccountInstance memory _instance + ) + public + view + { + InstalledModule[] memory modules = _instance.getInstalledModules(); + // Parse length + uint256 length = abi.decode(expectedResultBytes[0:32], (uint256)); + // Parse addresses and types + address[] memory expectedAddresses = new address[](length); + uint256[] memory expectedTypes = new uint256[](length); + for (uint256 i = 0; i < length; i++) { + expectedAddresses[i] = + abi.decode(expectedResultBytes[32 + i * 32:64 + i * 32], (address)); + expectedTypes[i] = abi.decode( + expectedResultBytes[32 + length * 32 + i * 32:64 + length * 32 + i * 32], (uint256) + ); + } + // Assert expected modules length + assertTrue( + modules.length == length + (instance.getAccountType() == AccountType.SAFE ? 1 : 0) + ); + // AccountType.SAFE has 1 extra module added during setup, skip it + uint256 index = instance.getAccountType() == AccountType.SAFE ? 1 : 0; + for (uint256 i = 0; i < length; i++) { + assertTrue(modules[index + i].moduleAddress == expectedAddresses[i]); + assertTrue(modules[index + i].moduleType == expectedTypes[i]); + } + } + /*////////////////////////////////////////////////////////////// MODIFIERS //////////////////////////////////////////////////////////////*/ From 32431c98743d6b70c2c172f1936ccf9fafc7aabc Mon Sep 17 00:00:00 2001 From: highskore Date: Thu, 15 Aug 2024 01:41:10 +0200 Subject: [PATCH 13/14] chore: remove unused import --- test/Diff.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Diff.t.sol b/test/Diff.t.sol index 85140f60..b0e72b85 100644 --- a/test/Diff.t.sol +++ b/test/Diff.t.sol @@ -13,7 +13,6 @@ import { } from "src/external/ERC7579.sol"; import { getAccountType, InstalledModule } from "src/test/utils/Storage.sol"; import { toString } from "src/test/utils/Vm.sol"; -import { console2 } from "forge-std/console2.sol"; contract ERC7579DifferentialModuleKitLibTest is BaseTest { using ModuleKitHelpers for *; From 779497b039a1450f072172d559e6e029ddc3f524 Mon Sep 17 00:00:00 2001 From: highskore Date: Thu, 15 Aug 2024 15:23:13 +0200 Subject: [PATCH 14/14] chore: forge fmt --- src/test/ModuleKitHelpers.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/ModuleKitHelpers.sol b/src/test/ModuleKitHelpers.sol index e63c50a3..886dd200 100644 --- a/src/test/ModuleKitHelpers.sol +++ b/src/test/ModuleKitHelpers.sol @@ -267,7 +267,9 @@ library ModuleKitHelpers { userOpData.entrypoint = instance.aux.entrypoint; } - function getInstalledModules(AccountInstance memory instance) + function getInstalledModules( + AccountInstance memory instance + ) internal view returns (InstalledModule[] memory)