Skip to content

Commit

Permalink
feat: replace linked lists with enumerable sets (#250)
Browse files Browse the repository at this point in the history
* feat: repalce linked lists with enumerable sets

* test: add module install/uninstall tests
  • Loading branch information
ly0va authored Jan 10, 2025
1 parent 1ec7da0 commit 9034133
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 407 deletions.
4 changes: 3 additions & 1 deletion src/SsoAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { ERC1271Handler } from "./handlers/ERC1271Handler.sol";
import { BatchCaller } from "./batch/BatchCaller.sol";

import { ISsoAccount } from "./interfaces/ISsoAccount.sol";
import { IModuleValidator } from "./interfaces/IModuleValidator.sol";

/// @title SSO Account
/// @author Matter Labs
Expand Down Expand Up @@ -197,7 +198,8 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
// Extract the signature, validator address and hook data from the _transaction.signature
(bytes memory signature, address validator, ) = SignatureDecoder.decodeSignature(_transaction.signature);

bool validationSuccess = _handleValidation(validator, _signedHash, signature, _transaction);
bool validationSuccess = _isModuleValidator(validator) &&
IModuleValidator(validator).validateTransaction(_signedHash, signature, _transaction);
if (!validationSuccess) {
return bytes4(0);
}
Expand Down
7 changes: 4 additions & 3 deletions src/handlers/ERC1271Handler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,18 @@ pragma solidity ^0.8.24;
import { IERC1271Upgradeable } from "@openzeppelin/contracts-upgradeable/interfaces/IERC1271Upgradeable.sol";
import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import { Transaction } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/TransactionHelper.sol";
import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";

import { SignatureDecoder } from "../libraries/SignatureDecoder.sol";
import { IModuleValidator } from "../interfaces/IModuleValidator.sol";
import { ValidationHandler } from "./ValidationHandler.sol";
import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
import { OwnerManager } from "../managers/OwnerManager.sol";
import { ValidatorManager } from "../managers/ValidatorManager.sol";

/// @title ERC1271Handler
/// @author Matter Labs
/// @notice Contract which provides ERC1271 signature validation
/// @notice The implementation is inspired by Clave wallet.
abstract contract ERC1271Handler is IERC1271Upgradeable, EIP712("Sso1271", "1.0.0"), ValidationHandler {
abstract contract ERC1271Handler is IERC1271Upgradeable, EIP712("Sso1271", "1.0.0"), OwnerManager, ValidatorManager {
struct SsoMessage {
bytes32 signedHash;
}
Expand Down
29 changes: 0 additions & 29 deletions src/handlers/ValidationHandler.sol

This file was deleted.

155 changes: 0 additions & 155 deletions src/libraries/LinkedList.sol

This file was deleted.

11 changes: 6 additions & 5 deletions src/libraries/SsoStorage.sol
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.24;

import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

library SsoStorage {
bytes32 private constant SSO_STORAGE_SLOT = 0x3248da1aeae8bd923cbf26901dc4bfc6bb48bb0fbc5b6102f1151fe7012884f4;

struct Layout {
// ┌───────────────────┐
// │ Ownership Data │
mapping(address => address) k1Owners;
EnumerableSet.AddressSet k1Owners;
uint256[50] __gap_0;
// └───────────────────┘

// ┌───────────────────┐
// │ Validation │
mapping(address => address) moduleValidators;
EnumerableSet.AddressSet moduleValidators;
uint256[50] __gap_2;
// └───────────────────┘

// ┌───────────────────┐
// │ Hooks │
mapping(address => address) validationHooks;
mapping(address => address) executionHooks;
mapping(address => mapping(bytes32 => bytes)) __DEPRECATED_hookDataStore;
EnumerableSet.AddressSet validationHooks;
EnumerableSet.AddressSet executionHooks;
uint256[50] __gap_4;
// └───────────────────┘
}
Expand Down
37 changes: 18 additions & 19 deletions src/managers/HookManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
pragma solidity ^0.8.24;

import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { Transaction } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/TransactionHelper.sol";
import { ExcessivelySafeCall } from "@nomad-xyz/excessively-safe-call/src/ExcessivelySafeCall.sol";

import { Auth } from "../auth/Auth.sol";
import { SsoStorage } from "../libraries/SsoStorage.sol";
import { AddressLinkedList } from "../libraries/LinkedList.sol";
import { Errors } from "../libraries/Errors.sol";
import { IExecutionHook, IValidationHook } from "../interfaces/IHook.sol";
import { IInitable } from "../interfaces/IInitable.sol";
Expand All @@ -20,8 +20,7 @@ import { IHookManager } from "../interfaces/IHookManager.sol";
* @author https://getclave.io
*/
abstract contract HookManager is IHookManager, Auth {
// Helper library for address to address mappings
using AddressLinkedList for mapping(address => address);
using EnumerableSet for EnumerableSet.AddressSet;
// Interface helper library
using ERC165Checker for address;
// Low level calls helper library
Expand Down Expand Up @@ -50,19 +49,19 @@ abstract contract HookManager is IHookManager, Auth {
/// @inheritdoc IHookManager
function listHooks(bool isValidation) external view override returns (address[] memory hookList) {
if (isValidation) {
hookList = _validationHooksLinkedList().list();
hookList = _validationHooks().values();
} else {
hookList = _executionHooksLinkedList().list();
hookList = _executionHooks().values();
}
}

// Runs the validation hooks that are enabled by the account and returns true if none reverts
function runValidationHooks(bytes32 signedHash, Transaction calldata transaction) internal returns (bool) {
address[] memory hookList = _validationHooksLinkedList().list();
uint256 totalHooks = hookList.length;
EnumerableSet.AddressSet storage hookList = _validationHooks();
uint256 totalHooks = hookList.length();

for (uint256 i = 0; i < totalHooks; i++) {
bool success = _call(hookList[i], abi.encodeCall(IValidationHook.validationHook, (signedHash, transaction)));
bool success = _call(hookList.at(i), abi.encodeCall(IValidationHook.validationHook, (signedHash, transaction)));

if (!success) {
return false;
Expand All @@ -74,17 +73,17 @@ abstract contract HookManager is IHookManager, Auth {

// Runs the execution hooks that are enabled by the account before and after _executeTransaction
modifier runExecutionHooks(Transaction calldata transaction) {
address[] memory hookList = _executionHooksLinkedList().list();
uint256 totalHooks = hookList.length;
EnumerableSet.AddressSet storage hookList = _executionHooks();
uint256 totalHooks = hookList.length();

for (uint256 i = 0; i < totalHooks; i++) {
IExecutionHook(hookList[i]).preExecutionHook(transaction);
IExecutionHook(hookList.at(i)).preExecutionHook(transaction);
}

_;

for (uint256 i = 0; i < totalHooks; i++) {
IExecutionHook(hookList[i]).postExecutionHook();
IExecutionHook(hookList.at(i)).postExecutionHook();
}
}

Expand All @@ -106,9 +105,9 @@ abstract contract HookManager is IHookManager, Auth {
}

if (isValidation) {
_validationHooksLinkedList().add(hookAddress);
_validationHooks().add(hookAddress);
} else {
_executionHooksLinkedList().add(hookAddress);
_executionHooks().add(hookAddress);
}

IInitable(hookAddress).init(initData);
Expand All @@ -118,9 +117,9 @@ abstract contract HookManager is IHookManager, Auth {

function _removeHook(address hook, bool isValidation) private {
if (isValidation) {
_validationHooksLinkedList().remove(hook);
_validationHooks().remove(hook);
} else {
_executionHooksLinkedList().remove(hook);
_executionHooks().remove(hook);
}

hook.excessivelySafeCall(gasleft(), 0, abi.encodeWithSelector(IInitable.disable.selector));
Expand All @@ -129,7 +128,7 @@ abstract contract HookManager is IHookManager, Auth {
}

function _isHook(address addr) internal view override returns (bool) {
return _validationHooksLinkedList().exists(addr) || _executionHooksLinkedList().exists(addr);
return _validationHooks().contains(addr) || _executionHooks().contains(addr);
}

function _call(address target, bytes memory data) private returns (bool success) {
Expand All @@ -138,11 +137,11 @@ abstract contract HookManager is IHookManager, Auth {
}
}

function _validationHooksLinkedList() private view returns (mapping(address => address) storage validationHooks) {
function _validationHooks() private view returns (EnumerableSet.AddressSet storage validationHooks) {
validationHooks = SsoStorage.layout().validationHooks;
}

function _executionHooksLinkedList() private view returns (mapping(address => address) storage executionHooks) {
function _executionHooks() private view returns (EnumerableSet.AddressSet storage executionHooks) {
executionHooks = SsoStorage.layout().executionHooks;
}

Expand Down
Loading

0 comments on commit 9034133

Please sign in to comment.