diff --git a/README.md b/README.md index f3a0cf9..28cebd9 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,18 @@ -# HatsWallet +# HatsAccount -HatsWallet is a smart contract account for every hat in [Hats Protocol](https://github.com/Hats-Protocol/hats-protocol). +HatsAccount is a smart contract account for every hat in [Hats Protocol](https://github.com/Hats-Protocol/hats-protocol). This repo contains three contracts: -1. [HatsWalletBase](#hatswalletbase), an abstract contract designed to be inherited by various flavors of HatsWallet -2. [HatsWallet1ofN](#hatswallet1ofn), a flavor of HatsWallet that mirrors the typical 1-of-n security model of hat-based role and permission management -3. [HatsWalletMofN](#hatswalletmofn), a flavor of HatsWallet that supports m-of-n security models, somewhat like a multisig of hat wearers +1. [HatsAccountBase](#HatsAccountbase), an abstract contract designed to be inherited by various flavors of HatsAccount +2. [HatsAccount1ofN](#HatsAccount1ofn), a flavor of HatsAccount that mirrors the typical 1-of-n security model of hat-based role and permission management +3. [HatsAccountMofN](#HatsAccountmofn), a flavor of HatsAccount that supports m-of-n security models, somewhat like a multisig of hat wearers ## Overview -HatsWallet gives every Hats Protocol hat a smart contract account. Each hat can have multiple flavors of HatsWallet, each following the ERC6551 standard and designed to be deployed via the ERC6551Registry factory. +HatsAccount gives every Hats Protocol hat a smart contract account. Each hat can have multiple flavors of HatsAccount, each following the ERC6551 standard and designed to be deployed via the ERC6551Registry factory. -HatsWallet gives every hat the ability to do the following: +HatsAccount gives every hat the ability to do the following: - Send ETH, ERC20, ERC721, and ERC1155 tokens - Sign ERC1271-compatible messages, e.g. as a signer on a multisig @@ -21,32 +21,32 @@ HatsWallet gives every hat the ability to do the following: - `Delegatecall` to other contracts, via [tokenbound](https://github.com/tokenbound/contracts)'s [sandbox](https://github.com/jaydenwindle/delegatecall-sandbox/) concept - Be assigned permissions in address-based onchain access control schemes -Apart from the first and last, all of these actions are performed by the hat's wearer(s), with the security model determined by the flavor of HatsWallet. +Apart from the first and last, all of these actions are performed by the hat's wearer(s), with the security model determined by the flavor of HatsAccount. -## HatsWalletBase +## HatsAccountBase -HatsWalletBase is an abstract contract built with [tokenbound's library](https://github.com/tokenbound/contracts) that provides the following common functionality for all other HatsWallet flavors: +HatsAccountBase is an abstract contract built with [tokenbound's library](https://github.com/tokenbound/contracts) that provides the following common functionality for all other HatsAccount flavors: - Ability to receive ETH (or other EVM chain-native tokens), ERC20, ERC721, and ERC1155 tokens - Implementation of the `IERC6551Account` interface, including / as well as getter functions for the account deployment parameters - `salt()` - `HATS()` — the address of the Hats Protocol contract, aka the `IERC6551Account.token.tokenContract` - - `hat()` — the id of the hat that this HatsWallet represents, aka the `IERC6551Account.token.tokenId` - - `IMPLEMENTATION()` — the address of the implementation contract for the inheriting flavor of HatsWallet + - `hat()` — the id of the hat that this HatsAccount represents, aka the `IERC6551Account.token.tokenId` + - `IMPLEMENTATION()` — the address of the implementation contract for the inheriting flavor of HatsAccount - Implementation of `IERC6551Account.isValidSigner` that sets wearers of the `hat()` as valid signers - Internal `_updateState` function adhering to the `IERC6551Account` standard -- EIP-721-compliant message-hashing function for use in signing and verifying messages by inheriting HatsWallet flavors -- [tokenbound](https://github.com/tokenbound/contracts)'s `BaseExecutor`, for use in executing transactions by inheriting HatsWallet flavors +- EIP-721-compliant message-hashing function for use in signing and verifying messages by inheriting HatsAccount flavors +- [tokenbound](https://github.com/tokenbound/contracts)'s `BaseExecutor`, for use in executing transactions by inheriting HatsAccount flavors ### Delegatecalls -For safety, HatsWalletBase constrains `delegatecall`s, only executing them from a special sandbox account coupled with the HatsWallet1ofN instance. This protects the HatsWallet from storage collision and self-destruct from malicious target contracts, with the tradeoff that the target contract must know — or be told about — the sandbox pattern in order for the `delegatecall` to succeed. +For safety, HatsAccountBase constrains `delegatecall`s, only executing them from a special sandbox account coupled with the HatsAccount1ofN instance. This protects the HatsAccount from storage collision and self-destruct from malicious target contracts, with the tradeoff that the target contract must know — or be told about — the sandbox pattern in order for the `delegatecall` to succeed. See the [delegatecall-sandbox docs](https://github.com/jaydenwindle/delegatecall-sandbox/) for more details. -## HatsWallet1ofN +## HatsAccount1ofN -HatsWallet1ofN is a flavor of HatsWallet that mirrors the typical 1-of-n security model of hat-based role and permission management. Any single wearer of a HatsWallet1ofN instance's hat has full control over that HatsWallet. If a hat has multiple wearers, they each individually have full control. +HatsAccount1ofN is a flavor of HatsAccount that mirrors the typical 1-of-n security model of hat-based role and permission management. Any single wearer of a HatsAccount1ofN instance's hat has full control over that HatsAccount. If a hat has multiple wearers, they each individually have full control. ### 1ofN: Executing Transactions @@ -59,13 +59,13 @@ Any single wearer of the hat can execute transactions under the hat's authority. For multiple transactions, this is done by calling the `executeBatch()` function, which takes as its sole argument an array of `Operations`. An `Operation` is a struct containing the same properties as the arguments of the `execute()` function above. -If execution succeeds, the HatsWallet's `state` is updated in compliance with the ERC6551 standard. +If execution succeeds, the HatsAccount's `state` is updated in compliance with the ERC6551 standard. ### 1ofN: Signing Messages Any single wearer of the hat can also sign messages on the hat's behalf. Other applications or contracts can verify that such signatures are valid by calling the `isValidSignature()` function, which takes the following arguments: -- `hash` — the keccak256 hash of the signed message, which can optionally be calculated with the `HatsWalletBase.getMessageHash()` function for compatibility with EIP-712. +- `hash` — the keccak256 hash of the signed message, which can optionally be calculated with the `HatsAccountBase.getMessageHash()` function for compatibility with EIP-712. - `signature` — the signature to verify The signature is considered valid as long as it is either... @@ -75,13 +75,13 @@ The signature is considered valid as long as it is either... This design follows [Gnosis Mech](https://github.com/gnosis/mech)'s approach, and creates flexibility for recursive validation of nested signatures. See [their docs for more details](https://github.com/gnosis/mech/tree/main#eip-1271-signatures). -## HatsWalletMofN +## HatsAccountMofN -HatsWalletMofN is a flavor of HatsWallet that supports m-of-n security models, somewhat like a multisig of hat wearers. To take any action with HatsWallet, m of the n present wearers of the hat must approve the action. +HatsAccountMofN is a flavor of HatsAccount that supports m-of-n security models, somewhat like a multisig of hat wearers. To take any action with HatsAccount, m of the n present wearers of the hat must approve the action. ### M of N Security Model -The specific security model of a given HatsWalletMofN instance is determined by a) the number of present wearers of the hat (ie the hat's supply), and b) the `THRESHOLD_RANGE` configured for that instance. As the supply of the hat changes, the required number of approvals to execute actions — given by `getThreshold()` — will move within the `THRESHOLD_RANGE`. +The specific security model of a given HatsAccountMofN instance is determined by a) the number of present wearers of the hat (ie the hat's supply), and b) the `THRESHOLD_RANGE` configured for that instance. As the supply of the hat changes, the required number of approvals to execute actions — given by `getThreshold()` — will move within the `THRESHOLD_RANGE`. These parameters are encoded at deployment time in the `salt` parameter of the `IERC6551Registry.deployAccount` function. @@ -107,7 +107,7 @@ M of the n wearers of the hat can execute transactions under the hat's authority Any wearer of the hat can propose a transaction by calling the `propose()` function, which takes the following arguments: -- `operations` — an array of `Operations`, the same struct used in [`HatsWallet1ofN.executeBatch()`](#1ofn-executing-transactions) +- `operations` — an array of `Operations`, the same struct used in [`HatsAccount1ofN.executeBatch()`](#1ofn-executing-transactions) - `expiration` — a uint32 timestamp after which the proposal will be not be executable even if it has enough approvals, similar to how [MolochV3](https://github.com/HausDAO/Baal/) handles proposal expiration. - `descriptionHash` — a bytes32 hash of the proposal description, similar to Governor's descriptionHash parameter. Beyond a commitment to a human-readable description, this can be used to make otherwise-identical proposals unique by including a small change in the description. @@ -115,7 +115,7 @@ Proposers also have the option to submit an approval vote with their proposal, b ##### Proposal Ids and Storage -Unlike some multisig and DAO contracts, HatsWalletMofN proposals can be executed in any order. There is no voting period, proposals can be executed as soon as they have enough approvals, and proposal ids are not sequential. +Unlike some multisig and DAO contracts, HatsAccountMofN proposals can be executed in any order. There is no voting period, proposals can be executed as soon as they have enough approvals, and proposal ids are not sequential. Proposal ids are bytes32 values derived from the proposal's `operations`, `expiration`, and `descriptionHash` parameters. @@ -167,7 +167,7 @@ Any account can execute any PENDING, non-expired proposal that has [enough appro - `operations` — an array of `Operations` - `expiration` — the expiration timestamp of the proposal - `descriptionHash` — the description hash of the proposal -- `voters` — an array of addresses that have voted to approve the proposal. The array must be strictly sorted in ascending order with no duplicates to ensure that votes are not double-counted. +- `voters` — an array of addresses that have voted to approve the proposal. The array must be strictly sorted in ascending order with no duplicates to ensure that votes are not double-counted. Its length must be greater than or equal to the threshold. The `execute()` function checks each of the voters in the `voters` array to ensure that they are wearers of the hat. If they are, and they voted to APPROVE for the proposalId derived from the other parameters, their vote is counted. If they are not, their vote is ignored. If the number of counted votes is greater than or equal to the [threshold](#m-of-n-security-model), the proposal is executed. @@ -175,14 +175,14 @@ Since `operations` is an array of `Operations`, each operation must succeed for #### Rejecting Proposals -For a proposal to be rejectable, it must receive enough REJECT votes such that the proposal could not be executed without a rejector changing their vote to APPROVE. This is different than other multisigs — which typically enforce the same threshold for approval and rejection — since HatsWalletMofN proposals can be executed in any order. +For a proposal to be rejectable, it must receive enough REJECT votes such that the proposal could not be executed without a rejector changing their vote to APPROVE. This is different than other multisigs — which typically enforce the same threshold for approval and rejection — since HatsAccountMofN proposals can be executed in any order. The primary reason to reject a proposal is to clear it from the list of PENDING proposals in front end applications. Note that rejecting a proposal does not prevent the same proposal from being re-proposed and executed. Any account can reject any PENDING, non-expired proposal that has enough rejections. This is done by calling the `reject()` function, which takes the following arguments: - `proposalId` -- `voters` — an array of addresses that have voted to reject the proposal. The array must be strictly sorted in ascending order with no duplicates to ensure that votes are not double-counted. +- `voters` — an array of addresses that have voted to reject the proposal. The array must be strictly sorted in ascending order with no duplicates to ensure that votes are not double-counted. Its length must be greater than or equal to the rejection threshold. The `reject()` function checks each of the voters in the `voters` array to ensure that they are wearers of the hat. If they are, and they voted to REJECT, their vote is counted. If they are not, their vote is ignored. If the number of counted votes is greater than or equal to the rejection threshold — given by `getRejectionThreshold()`, the proposal is rejected. @@ -200,17 +200,17 @@ The following view functions are provided to help front end applications manage ### MofN: Signing Messages -HatsWallet1ofN can produce valid EIP-1271 signatures on the hat's behalf, but the process is different from HatsWallet1ofN. Instead of a valid signer — i.e. hat-wearer — producing a cryptographic signature of the message, HatsWalletMofN marks a message as "signed" by adding the message's hash to a list of signed messages. +HatsAccount1ofN can produce valid EIP-1271 signatures on the hat's behalf, but the process is different from HatsAccount1ofN. Instead of a valid signer — i.e. hat-wearer — producing a cryptographic signature of the message, HatsAccountMofN marks a message as "signed" by adding the message's hash to a list of signed messages. -This can be done by executing a proposal which calls the `HatsWalletMofN.sign()` function. This function is only callable by the HatsWalletMofN instance itself, and takes a single argument: `message` — the bytes of the message itself, of arbitrary length. +This can be done by executing a proposal which calls the `HatsAccountMofN.sign()` function. This function is only callable by the HatsAccountMofN instance itself, and takes a single argument: `message` — the bytes of the message itself, of arbitrary length. When called, the `sign()` function gets the hash of the message and marks it as signed in the `signedMessages` mapping. #### Message Hashing -In HatsWallet, messages are hashed following the EIP-712 standard with a design similar to [Safe's message hashing scheme](https://github.com/safe-global/safe-contracts/blob/f03dfae65fd1d085224b00a10755c509a4eaacfe/contracts/libraries/SignMessageLib.sol#L33). +In HatsAccount, messages are hashed following the EIP-712 standard with a design similar to [Safe's message hashing scheme](https://github.com/safe-global/safe-contracts/blob/f03dfae65fd1d085224b00a10755c509a4eaacfe/contracts/libraries/SignMessageLib.sol#L33). -When signing a message, `sign()` takes care of the hashing. When verifying a signature via `IERC1271.isValidSignature()`, the message must be hashed by the caller. For convenience, HatsWalletMofN exposes the `getMessageHash()` function. +When signing a message, `sign()` takes care of the hashing. When verifying a signature via `IERC1271.isValidSignature()`, the message must be hashed by the caller. For convenience, HatsAccountMofN exposes the `getMessageHash()` function. ## Development and Testing @@ -220,4 +220,4 @@ This repo uses Foundry for development and testing. To build, test, and deploy: 2. Install dependencies: `forge install` 3. Build: `forge build` 4. Test: `forge test` -5. Deploy: see the [deployment script](./script/HatsWallet.s.sol) +5. Deploy: see the [deployment script](./script/HatsAccount.s.sol) diff --git a/foundry.toml b/foundry.toml index 911b844..06bc79f 100644 --- a/foundry.toml +++ b/foundry.toml @@ -3,7 +3,7 @@ src = 'src' out = 'out' libs = ['lib'] optimizer_runs = 1_000_000 -gas_reports = ["HatsWallet1ofN", "HatsWalletMofN"] +gas_reports = ["HatsAccount1ofN", "HatsAccountMofN"] auto_detect_solc = false solc = "0.8.23" bytecode_hash = "none" diff --git a/script/HatsWallet1ofN.s.sol b/script/HatsAccount1ofN.s.sol similarity index 84% rename from script/HatsWallet1ofN.s.sol rename to script/HatsAccount1ofN.s.sol index 4df86e4..9d87e74 100644 --- a/script/HatsWallet1ofN.s.sol +++ b/script/HatsAccount1ofN.s.sol @@ -2,12 +2,12 @@ pragma solidity ^0.8.18; import { Script, console2 } from "forge-std/Script.sol"; -import { HatsWallet1ofN } from "../src/HatsWallet1ofN.sol"; +import { HatsAccount1ofN } from "../src/HatsAccount1ofN.sol"; import { IHats } from "hats-protocol/Interfaces/IHats.sol"; import { IERC6551Registry } from "erc6551/interfaces/IERC6551Registry.sol"; contract DeployImplementation is Script { - HatsWallet1ofN public implementation; + HatsAccount1ofN public implementation; bool private _verbose = true; string private _version = "test1"; @@ -24,7 +24,7 @@ contract DeployImplementation is Script { vm.startBroadcast(deployer); // deploy the implementation - implementation = new HatsWallet1ofN{ salt: SALT }(_version); + implementation = new HatsAccount1ofN{ salt: SALT }(_version); vm.stopBroadcast(); @@ -32,13 +32,13 @@ contract DeployImplementation is Script { console2.log("implementation", address(implementation)); } } - // forge script script/HatsWallet1ofN.s.sol:DeployImplementation -f mainnet --broadcast --verify + // forge script script/HatsAccount1ofN.s.sol:DeployImplementation -f mainnet --broadcast --verify /* forge verify-contract --chain-id 5 --num-of-optimizations 1000000 --watch --constructor-args $(cast abi-encode \ "constructor(string)" "test1" ) \ --compiler-version v0.8.21 0x009702D64E366cA4E4b0c72a72c9d16cB7e2A728 \ - src/HatsWallet1ofN.sol:HatsWallet1ofN --etherscan-api-key $ETHERSCAN_KEY + src/HatsAccount1ofN.sol:HatsAccount1ofN --etherscan-api-key $ETHERSCAN_KEY */ } @@ -77,5 +77,5 @@ contract DeployWallet is Script { return wallet; } - // forge script script/HatsWallet.s.sol:DeployWallet -f goerli + // forge script script/HatsAccount.s.sol:DeployWallet -f goerli } diff --git a/script/HatsWalletMofN.s.sol b/script/HatsAccountMofN.s.sol similarity index 84% rename from script/HatsWalletMofN.s.sol rename to script/HatsAccountMofN.s.sol index 7c38a8a..fc22603 100644 --- a/script/HatsWalletMofN.s.sol +++ b/script/HatsAccountMofN.s.sol @@ -2,12 +2,12 @@ pragma solidity ^0.8.18; import { Script, console2 } from "forge-std/Script.sol"; -import { HatsWalletMofN } from "../src/HatsWalletMofN.sol"; +import { HatsAccountMofN } from "../src/HatsAccountMofN.sol"; import { IHats } from "hats-protocol/Interfaces/IHats.sol"; import { IERC6551Registry } from "erc6551/interfaces/IERC6551Registry.sol"; contract DeployImplementation is Script { - HatsWalletMofN public implementation; + HatsAccountMofN public implementation; bool private _verbose = true; string private _version = "test1"; @@ -24,20 +24,20 @@ contract DeployImplementation is Script { vm.startBroadcast(deployer); // deploy the implementation - implementation = new HatsWalletMofN{ salt: SALT }(_version); + implementation = new HatsAccountMofN{ salt: SALT }(_version); vm.stopBroadcast(); if (_verbose) { console2.log("implementation", address(implementation)); } } - // forge script script/HatsWallet.s.sol:DeployImplementation -f mainnet --broadcast --verify + // forge script script/HatsAccount.s.sol:DeployImplementation -f mainnet --broadcast --verify /* forge verify-contract --chain-id 5 --num-of-optimizations 1000000 --watch --constructor-args $(cast abi-encode \ "constructor(string)" "test1" ) \ --compiler-version v0.8.21 0xEA95A8Da1746897343c56f5468489a36BbC5e0Bc \ - src/HatsWalletMofN.sol:HatsWalletMofN --etherscan-api-key $ETHERSCAN_KEY + src/HatsAccountMofN.sol:HatsAccountMofN --etherscan-api-key $ETHERSCAN_KEY */ } @@ -76,5 +76,5 @@ contract DeployWallet is Script { return wallet; } - // forge script script/HatsWallet.s.sol:DeployWallet -f goerli + // forge script script/HatsAccount.s.sol:DeployWallet -f goerli } diff --git a/src/HatsWallet1ofN.sol b/src/HatsAccount1ofN.sol similarity index 80% rename from src/HatsWallet1ofN.sol rename to src/HatsAccount1ofN.sol index 710b703..1e5e86a 100644 --- a/src/HatsWallet1ofN.sol +++ b/src/HatsAccount1ofN.sol @@ -1,24 +1,23 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.19; -// import { console2, Test } from "forge-std/Test.sol"; // remove before deploy -import "./lib/HatsWalletErrors.sol"; -import { HatsWalletBase } from "./HatsWalletBase.sol"; -import { LibHatsWallet, Operation } from "./lib/LibHatsWallet.sol"; +// import { console2, Test } from "forge-std/Test.sol"; // comment out before deploy +import "./lib/HatsAccountErrors.sol"; +import { HatsAccountBase } from "./HatsAccountBase.sol"; +import { LibHatsAccount, Operation } from "./lib/LibHatsAccount.sol"; import { IERC1271 } from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import { ECDSA } from "solady/utils/ECDSA.sol"; -import { SignatureCheckerLib } from "solady/utils/SignatureCheckerLib.sol"; import { IERC6551Executable } from "erc6551/interfaces/IERC6551Executable.sol"; /** - * @title HatsWallet1ofN + * @title HatsAccount1ofN * @author Haberdasher Labs * @author spengrah - * @notice A HatsWallet implementation that requires a single signature from a valid signer — ie a single wearer of - * the hat — to execute a transaction. It supports execution of single as batch operations, as well as EIP-1271 + * @notice A HatsAccount implementation that requires a single signature from a valid signer — ie a single wearer of + * the hat — to execute a transaction. It supports execution of single operations, batch operations, and EIP-1271 * contract signatures. */ -contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { +contract HatsAccount1ofN is HatsAccountBase, IERC6551Executable { /*/////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ @@ -31,6 +30,7 @@ contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { //////////////////////////////////////////////////////////////*/ constructor(string memory version) { + // set the implementation version _version = version; } @@ -54,8 +54,9 @@ contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { _beforeExecute(); // execute the call, routing delegatecalls through the sandbox, and bubble up the result - result = LibHatsWallet._execute(_to, _value, _data, _operation); + result = LibHatsAccount._execute(_to, _value, _data, _operation); + // log the executor emit TxExecuted(msg.sender); } @@ -74,15 +75,14 @@ contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { uint256 length = operations.length; bytes[] memory results = new bytes[](length); - for (uint256 i; i < length;) { + for (uint256 i; i < length; ++i) { + /// @dev compile with solc ^0.8.23 to use unchecked incrementation // execute the call, routing delegatecalls through the sandbox, and bubble up the result results[i] = - LibHatsWallet._execute(operations[i].to, operations[i].value, operations[i].data, operations[i].operation); - unchecked { - ++i; - } + LibHatsAccount._execute(operations[i].to, operations[i].value, operations[i].data, operations[i].operation); } + // log the executor emit TxExecuted(msg.sender); return results; @@ -95,8 +95,8 @@ contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { /** * @notice Checks whether the signature provided is valid for the provided hash, complies with EIP-1271. A signature * is valid if either: - * - It's a valid ECDSA signature by a valid HatsWallet signer - * - It's a valid EIP-1271 signature by a valid HatsWallet signer + * - It's a valid ECDSA signature by a valid HatsAccount signer + * - It's a valid EIP-1271 signature by a valid HatsAccount signer * @dev Implementation borrowed from https://github.com/gnosis/mech/blob/main/contracts/base/Mech.sol * @param _hash Hash of the data (could be either a message hash or transaction hash) * @param _signature Signature to validate. Can be an EIP-1271 contract signature (identified by v=0) or an ECDSA @@ -107,7 +107,7 @@ contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { bytes32 r; bytes32 s; uint8 v; - (v, r, s) = LibHatsWallet._splitSignature(signature); + (v, r, s) = LibHatsAccount._splitSignature(signature); if (v == 0) { // This is an EIP-1271 contract signature @@ -141,7 +141,7 @@ contract HatsWallet1ofN is HatsWalletBase, IERC6551Executable { VIEW FUNCTIONS //////////////////////////////////////////////////////////////*/ - /// @inheritdoc HatsWalletBase + /// @inheritdoc HatsAccountBase function supportsInterface(bytes4 interfaceId) public view override returns (bool) { return ( interfaceId == type(IERC6551Executable).interfaceId || interfaceId == type(IERC1271).interfaceId diff --git a/src/HatsWalletBase.sol b/src/HatsAccountBase.sol similarity index 79% rename from src/HatsWalletBase.sol rename to src/HatsAccountBase.sol index 7d5d595..80bbada 100644 --- a/src/HatsWalletBase.sol +++ b/src/HatsAccountBase.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.19; // import { console2, Test } from "forge-std/Test.sol"; // remove before deploy -import "./lib/HatsWalletErrors.sol"; +import "./lib/HatsAccountErrors.sol"; import { IHats } from "hats-protocol/Interfaces/IHats.sol"; -import { LibHatsWallet, Operation } from "./lib/LibHatsWallet.sol"; +import { LibHatsAccount, Operation } from "./lib/LibHatsAccount.sol"; import { ERC6551Account, IERC165, IERC6551Account, ERC6551AccountLib } from "tokenbound/abstract/ERC6551Account.sol"; import { BaseExecutor } from "tokenbound/abstract/execution/BaseExecutor.sol"; import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; @@ -12,15 +12,15 @@ import { IERC721Receiver } from "@openzeppelin/contracts/interfaces/IERC721Recei import { IERC1155Receiver } from "@openzeppelin/contracts/interfaces/IERC1155Receiver.sol"; /** - * @title HatsWalletBase + * @title HatsAccountBase * @author Haberdasher Labs * @author spengrah - * @notice The base contract for all HatsWallet implementations. As an abstract contract, this contract will only work - * when inherited by a full implementation of HatsWallet. HatsWallet is a flavor of ERC6551-compatible token-bound + * @notice The base contract for all HatsAccount implementations. As an abstract contract, this contract will only work + * when inherited by a full implementation of HatsAccount. HatsAccount is a flavor of ERC6551-compatible token-bound * account for Hats Protocol hats. * @dev This contract implements ERC6551 with the use of the tokenbound library. */ -abstract contract HatsWalletBase is ERC6551Account, BaseExecutor, IERC721Receiver, IERC1155Receiver { +abstract contract HatsAccountBase is ERC6551Account, BaseExecutor, IERC721Receiver, IERC1155Receiver { /*////////////////////////////////////////////////////////////// CONSTANTS //////////////////////////////////////////////////////////////*/ @@ -30,14 +30,14 @@ abstract contract HatsWalletBase is ERC6551Account, BaseExecutor, IERC721Receive keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); /// @notice EIP-712 message typehash for this contract - bytes32 internal constant HATSWALLET_MSG_TYPEHASH = keccak256("HatsWallet(bytes message)"); + bytes32 internal constant HatsAccount_MSG_TYPEHASH = keccak256("HatsAccount(bytes message)"); - /// @notice The salt used to create this HatsWallet instance + /// @notice The salt used to create this HatsAccount instance function salt() public view returns (bytes32) { return ERC6551AccountLib.salt(); } - /// @notice The Hats Protocol hat whose wearer controls this HatsWallet + /// @notice The Hats Protocol hat whose wearer controls this HatsAccount function hat() public view returns (uint256) { bytes memory footer = new bytes(0x20); assembly { @@ -57,26 +57,28 @@ abstract contract HatsWalletBase is ERC6551Account, BaseExecutor, IERC721Receive return abi.decode(footer, (IHats)); } - /// @notice The address of this HatsWallet implementation + /// @notice The address of this HatsAccount implementation function IMPLEMENTATION() public view returns (address) { return ERC6551AccountLib.implementation(); } - /// @notice The version of the HatsWallet implementation contract - /// @dev Will return an empty string when called on a HatsWallet instance (clone) + /// @notice The version of the HatsAccount implementation contract + /// @dev Will return an empty string when called on a HatsAccount instance (clone) function version_() public view returns (string memory) { return _version; } - /// @notice The version of this HatsWallet instance (clone) + /// @notice The version of this HatsAccount instance (clone) function version() public view returns (string memory) { - return HatsWalletBase(payable(IMPLEMENTATION())).version_(); + return HatsAccountBase(payable(IMPLEMENTATION())).version_(); } /*////////////////////////////////////////////////////////////// NON-CONSTANT STORAGE //////////////////////////////////////////////////////////////*/ + /// @dev The version of this HatsAccount implementation contract. Must be set in the constructor of the inheriting + /// contract. Will be empty for HatsAccount instances (clones). string internal _version; /*////////////////////////////////////////////////////////////// @@ -94,7 +96,7 @@ abstract contract HatsWalletBase is ERC6551Account, BaseExecutor, IERC721Receive bytes1(0x19), bytes1(0x01), domainSeparator(), - keccak256(abi.encode(HATSWALLET_MSG_TYPEHASH, keccak256(_message))) // HatsWalletMessageHash + keccak256(abi.encode(HatsAccount_MSG_TYPEHASH, keccak256(_message))) // HatsAccountMessageHash ) ); } @@ -118,13 +120,14 @@ abstract contract HatsWalletBase is ERC6551Account, BaseExecutor, IERC721Receive //////////////////////////////////////////////////////////////*/ /// @inheritdoc ERC6551Account + /// @dev HatsAccount signer validation does not require additional context data function _isValidSigner(address _signer, bytes memory /* context */ ) internal view override returns (bool) { return _isValidSigner(_signer); } /** - * @dev Internal function to check if a given address is a valid signer for this HatsWallet. A signer is valid if they - * are wearing the `hat` of this HatsWallet. + * @dev Internal function to check if a given address is a valid signer for this HatsAccount. A signer is valid if and + * only if they are wearing the `hat` of this HatsAccount instance. * @param _signer The address to check */ function _isValidSigner(address _signer) internal view returns (bool) { diff --git a/src/HatsWalletMofN.sol b/src/HatsAccountMofN.sol similarity index 83% rename from src/HatsWalletMofN.sol rename to src/HatsAccountMofN.sol index 53a5d78..53f3132 100644 --- a/src/HatsWalletMofN.sol +++ b/src/HatsAccountMofN.sol @@ -1,25 +1,23 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.19; -import { console2, Test } from "forge-std/Test.sol"; // remove before deploy -import "./lib/HatsWalletErrors.sol"; -import { HatsWalletBase } from "./HatsWalletBase.sol"; -import { LibHatsWallet, Operation, ProposalStatus, Vote } from "./lib/LibHatsWallet.sol"; +// import { console2, Test } from "forge-std/Test.sol"; // comment out before deploy +import "./lib/HatsAccountErrors.sol"; +import { HatsAccountBase } from "./HatsAccountBase.sol"; +import { LibHatsAccount, Operation, ProposalStatus, Vote } from "./lib/LibHatsAccount.sol"; import { IERC1271 } from "@openzeppelin/contracts/interfaces/IERC1271.sol"; -import { ECDSA } from "solady/utils/ECDSA.sol"; -import { SignatureCheckerLib } from "solady/utils/SignatureCheckerLib.sol"; /** - * @title HatsWalletMofN + * @title HatsAccountMofN * @author Haberdasher Labs * @author spengrah - * @notice A HatsWallet implementation that requires m votes by valid signers — ie wearers of + * @notice A HatsAccount implementation that requires m votes by valid signers — ie wearers of * the hat — to execute a transaction. The threshold is derived dynamically as a factor of the wallet's configured * min- and max-threshold and the current supply of the hat. Transactions are queued via onchain proposal, and valid * signers vote onchain to approve or reject the proposal. Valid signers can also approve messages as "signed" by the * wallet, which can be used to create an EIP-1271 contract signature. */ -contract HatsWalletMofN is HatsWalletBase { +contract HatsAccountMofN is HatsAccountBase { /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ @@ -76,7 +74,7 @@ contract HatsWalletMofN is HatsWalletBase { /// @notice The votes on a proposal, indexed by its id and the voter's address mapping(bytes32 proposalId => mapping(address voter => Vote vote)) public votes; - /// @notice Messages approved as signed by this HatsWallet, eg for use as EIP12721 contract signatures + /// @notice Messages approved as signed by this HatsAccount, eg for use as EIP12721 contract signatures mapping(bytes32 messageHash => bool signed) public signedMessages; /*/////////////////////////////////////////////////////////////// @@ -92,10 +90,11 @@ contract HatsWalletMofN is HatsWalletBase { //////////////////////////////////////////////////////////////*/ /** - * @notice Propose a tx to be executed by this HatsWallet. The caller must be a valid signer for this HatsWallet. + * @notice Propose a tx to be executed by this HatsAccount. The caller must be a valid signer for this HatsAccount. * @dev Even though signer validity is also checked at execution time, we check it here to prevent spam and DoS * attacks. - * @param _operations Array of operations to be executed by this HatsWallet. Only call and delegatecall are supported. + * @param _operations Array of operations to be executed by this HatsAccount. Only call and delegatecall are + * supported. * Delegatecalls are routed through the sandbox. * @param _expiration The timestamp after which the proposal will be expired and no longer executable. If zero, the * proposal will never expire. @@ -112,10 +111,11 @@ contract HatsWalletMofN is HatsWalletBase { } /** - * @notice Propose a tx to be executed by this HatsWallet along with a vote to approve. + * @notice Propose a tx to be executed by this HatsAccount along with a vote to approve. * @dev Even though signer validity is also checked at execution time, we check it here to prevent spam and DoS * attacks. - * @param _operations Array of operations to be executed by this HatsWallet. Only call and delegatecall are supported. + * @param _operations Array of operations to be executed by this HatsAccount. Only call and delegatecall are + * supported. * Delegatecalls are routed through the sandbox. * @param _expiration The timestamp after which the proposal will be expired and no longer executable. If zero, the * proposal will never expire. @@ -153,11 +153,13 @@ contract HatsWalletMofN is HatsWalletBase { * @notice Execute a pending proposal. If enough valid signers have voted to approve the proposal, it will be * executed. * @dev Checks signer validity. - * @param _operations Array of operations to be executed by this HatsWallet. Only call and delegatecall are supported. + * @param _operations Array of operations to be executed by this HatsAccount. Only call and delegatecall are + * supported. * Delegatecalls are routed through the sandbox. * @param _expiration The timestamp after which the proposal will be expired and no longer executable. * @param _descriptionHash Hash of the description of the tx to be executed. - * @param _voters The addresses of the voters to check for approval votes + * @param _voters The addresses of the voters to check for approval votes. Must be sorted in ascending order, and have + * a length gte the threshold. * @return results The results of the operations */ function execute( @@ -182,9 +184,10 @@ contract HatsWalletMofN is HatsWalletBase { uint256 length = _operations.length; bytes[] memory results = new bytes[](length); - for (uint256 i = 0; i < length; i++) { + for (uint256 i; i < length; ++i) { + /// @dev compile with solc ^0.8.23 to use unchecked incrementation results[i] = - LibHatsWallet._execute(_operations[i].to, _operations[i].value, _operations[i].data, _operations[i].operation); + LibHatsAccount._execute(_operations[i].to, _operations[i].value, _operations[i].data, _operations[i].operation); } // log the proposal execution @@ -199,7 +202,8 @@ contract HatsWalletMofN is HatsWalletBase { * recorded. * @dev Checks signer validity. * @param _proposalId The unique id of the proposal - * @param _voters The addresses of the voters to check for rejection votes + * @param _voters The addresses of the voters to check for rejection votes. Must be sorted in ascending order, and + * have a length gte the rejection threshold. */ function reject(bytes32 _proposalId, address[] calldata _voters) external { // validate the voters and their rejections of this proposed tx @@ -214,13 +218,13 @@ contract HatsWalletMofN is HatsWalletBase { /** * @notice Mark a message as signed so that it can be used as an EIP-1271 contract signature - * @dev Only callable by this HatsWallet, i.e. as an operation executed by {execute} - * @param _message The message to mark as signed on behalf of this HatsWallet + * @dev Only callable by this HatsAccount, i.e. as an operation executed by {execute} + * @param _message The message to mark as signed on behalf of this HatsAccount * @return messageHash The hash of the message */ function sign(bytes calldata _message) external returns (bytes32 messageHash) { - // only callable by this HatsWallet - if (msg.sender != address(this)) revert NotHatsWallet(); + // only callable by this HatsAccount + if (msg.sender != address(this)) revert NotHatsAccount(); // hash the message messageHash = getMessageHash(_message); @@ -237,7 +241,7 @@ contract HatsWalletMofN is HatsWalletBase { /** * @notice Derive the proposal id as a hash of the operations and description hash - * @param operations Array of operations to be executed by this HatsWallet. + * @param operations Array of operations to be executed by this HatsAccount. * @param _expiration The timestamp after which the proposal will be expired and no longer executable. * @param _descriptionHash Hash of the description of the tx to be executed. * @return proposalId The unique id of the proposal @@ -259,7 +263,7 @@ contract HatsWalletMofN is HatsWalletBase { } /** - * @notice Derive the dynamic threshold, which is a function of the current hat supply and this HatsWallet's + * @notice Derive the dynamic threshold, which is a function of the current hat supply and this HatsAccount's * configured threshold range. * @return threshold The current threshold. */ @@ -278,12 +282,15 @@ contract HatsWalletMofN is HatsWalletBase { } /** - * @notice Derive the rejection threshold, which is the inverse of the current threshold. + * @notice Derive the rejection threshold, which is the inverse of the current threshold. If the hat supply is less + * than the current threshold, the rejection threshold is equivalent to the current threshold. * @return rejectionThreshold The current rejection threshold. */ function getRejectionThreshold() public view returns (uint256 rejectionThreshold) { uint256 hatSupply = HATS().hatSupply(hat()); - return hatSupply - _getThreshold(hatSupply) + 1; + uint256 threshold = _getThreshold(hatSupply); + + return (hatSupply < threshold) ? threshold : hatSupply - threshold + 1; } /** @@ -308,7 +315,8 @@ contract HatsWalletMofN is HatsWalletBase { * @return Whether the proposal is rejectable */ function isRejectableNow(bytes32 _proposalId, address[] calldata _voters) external view returns (bool) { - return _checkRejectableNow(_proposalId, _voters); + _checkRejectableNow(_proposalId, _voters); + return true; } /** @@ -323,7 +331,8 @@ contract HatsWalletMofN is HatsWalletBase { view returns (uint256 approvals, uint256 rejections) { - for (uint256 i; i < _voters.length;) { + for (uint256 i; i < _voters.length; ++i) { + /// @dev compile with solc ^0.8.23 to use unchecked incremenation unchecked { if (votes[_proposalId][_voters[i]] == Vote.APPROVE && _isValidSigner(_voters[i])) { // Should not overflow within the gas limit @@ -334,19 +343,16 @@ contract HatsWalletMofN is HatsWalletBase { // Should not overflow within the gas limit ++rejections; } - - // Should not overflow given the loop condition - ++i; } } } - /// @inheritdoc HatsWalletBase + /// @inheritdoc HatsAccountBase function domainSeparator() public view override returns (bytes32) { - return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, "HatsWalletMofN", version(), block.chainid, address(this))); + return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, "HatsAccountMofN", version(), block.chainid, address(this))); } - /// @inheritdoc HatsWalletBase + /// @inheritdoc HatsAccountBase function supportsInterface(bytes4 interfaceId) public view override returns (bool) { return (interfaceId == type(IERC1271).interfaceId || super.supportsInterface(interfaceId)); } @@ -359,7 +365,8 @@ contract HatsWalletMofN is HatsWalletBase { * @notice Set the status of a proposal to pending and log the proposal submission. * @dev Reverts if the proposal already exists or if the caller is not a valid signer. Even though signer validity is * also checked at execution time, we check it here to prevent spam and DoS attacks. - * @param _operations Array of operations to be executed by this HatsWallet. Only call and delegatecall are supported. + * @param _operations Array of operations to be executed by this HatsAccount. Only call and delegatecall are + * supported. * Delegatecalls are routed through the sandbox. * @param _expiration The timestamp after which the proposal will be expired and no longer executable. If zero, the * proposal will never expire. @@ -400,7 +407,7 @@ contract HatsWalletMofN is HatsWalletBase { } /** - * @dev The only way for a HatsWalletMofN to produce a valid signature is by marking a messageHash as signed, via + * @dev The only way for a HatsAccountMofN to produce a valid signature is by marking a messageHash as signed, via * {sign}. For this reason, an actual cryptographic signature is not required, so the second argument of the * {IERC1271.isValidSignature} function is not used. * @param _hash The hash of the message. @@ -429,9 +436,8 @@ contract HatsWalletMofN is HatsWalletBase { * 3. Has at least [threshold] approvals from valid signers * @param _proposalId The unique id of the proposal * @param _voters The addresses of the voters to check for approval votes - * @return executable Whether the proposal is executable now */ - function _checkExecutableNow(bytes32 _proposalId, address[] calldata _voters) internal view returns (bool) { + function _checkExecutableNow(bytes32 _proposalId, address[] calldata _voters) internal view { // proposal must not be expired. If the expiration is zero, the proposal has no expiration. uint256 expiration = getExpiration(_proposalId); if (expiration > 0 && expiration < block.timestamp) revert ProposalExpired(); @@ -443,8 +449,6 @@ contract HatsWalletMofN is HatsWalletBase { uint256 threshold = getThreshold(); _checkValidVotes(_proposalId, _voters, Vote.APPROVE, threshold); - - return true; } /** @@ -453,31 +457,39 @@ contract HatsWalletMofN is HatsWalletBase { * 2. Has at least [hatSupply - threshold] rejections from valid signers * @param _proposalId The unique id of the proposal * @param _voters The addresses of the voters to check for rejection votes - * @return rejectable Whether the proposal is rejectable now */ - function _checkRejectableNow(bytes32 _proposalId, address[] calldata _voters) internal view returns (bool) { + function _checkRejectableNow(bytes32 _proposalId, address[] calldata _voters) internal view { // proposal must be pending if (proposalStatus[_proposalId] != ProposalStatus.PENDING) revert ProposalNotPending(); - // proposal must not be expired - // the number of rejections required to reject the proposal is the inverse of the current threshold - // uint256 hatSupply = HATS().hatSupply(hat()); uint256 rejectionThreshold = getRejectionThreshold(); _checkValidVotes(_proposalId, _voters, Vote.REJECT, rejectionThreshold); - - return true; } + /** + * @dev Checks whether a proposal has enough valid votes to meet the required threshold, and reverts if not. + * Specifically, it reverts with... + * - VotersArrayTooShort if the voters array is shorter than the threshold + * - UnsortedVotersArray if the voters array is not sorted in ascending order by address + * - InsufficientValidVotes if there are fewer valid votes than the threshold + * @param _proposalId The unique id of the proposal + * @param _voters The addresses of the voters to check for votes + * @param _vote The type of vote to check for + * @param _threshold The threshold to check against + */ function _checkValidVotes(bytes32 _proposalId, address[] calldata _voters, Vote _vote, uint256 _threshold) internal view { - uint256 count; address currentVoter; address lastVoter; - for (uint256 i; i < _voters.length;) { + + if (_voters.length < _threshold) revert VotersArrayTooShort(); + + for (uint256 i; i < _threshold; ++i) { + /// @dev compile with solc ^0.8.23 to use unchecked incrementation // cache the current voter currentVoter = _voters[i]; @@ -489,23 +501,15 @@ contract HatsWalletMofN is HatsWalletBase { */ if (currentVoter <= lastVoter) revert UnsortedVotersArray(); - unchecked { - if (votes[_proposalId][currentVoter] == _vote) { - if (_isValidSigner(currentVoter)) { - ++count; // Should not overflow within the gas limit - } - } - - // once we have enough votes, we stop counting and return - if (count >= _threshold) return; - - // prepare for the next iteration - lastVoter = currentVoter; - ++i; // Should not overflow given the loop condition + // revert if the current voter has not recorded a correct vote or is not a valid signer + if (votes[_proposalId][currentVoter] == _vote) { + if (!_isValidSigner(currentVoter)) revert InvalidVote(currentVoter); + } else { + revert InvalidVote(currentVoter); } - } - // if we didn't get enough rejections, the proposal is not rejectable - revert InsufficientValidVotes(); + // prepare for the next iteration + lastVoter = currentVoter; + } } } diff --git a/src/lib/HatsAccountErrors.sol b/src/lib/HatsAccountErrors.sol new file mode 100644 index 0000000..9798ea3 --- /dev/null +++ b/src/lib/HatsAccountErrors.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import "tokenbound/utils/Errors.sol"; + +/// @notice Thrown when the caller is not wearing the `hat` +error InvalidSigner(); + +/// @notice Thrown when the caller is not this instance of HatsAccount +error NotHatsAccount(); + +/// @notice The same exact proposal cannot be submitted twice. Any differences in the proposal parameters — +/// including its operations, expiration, or descriptionHash — will produce a unique proposal. +error ProposalAlreadyExists(); + +/// @notice Proposals must be in the PENDING state to be voted on or processed (executed or rejected) +error ProposalNotPending(); + +/// @notice Proposals must not have expired to be executed +error ProposalExpired(); + +/// @notice Voters array must be at least as long as the required threshold +error VotersArrayTooShort(); + +/// @notice Voters must be sorted in ascending order by address +error UnsortedVotersArray(); + +/// @notice Thrown when attempting to process (execute or reject) a proposal with invalid votes +error InvalidVote(address voter); diff --git a/src/lib/HatsAccountStorage.sol b/src/lib/HatsAccountStorage.sol new file mode 100644 index 0000000..2a3eb0b --- /dev/null +++ b/src/lib/HatsAccountStorage.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import { ProposalStatus, Vote } from "./LibHatsAccount.sol"; + +contract HatsAccountStorage { + // HatsAccountBase + uint256 _state; + string public version_; + + // HatsAccount1ofN has no additional storage! + + // HatsAccountMofN + mapping(bytes32 proposalId => ProposalStatus) public proposalStatus; + mapping(bytes32 proposalId => mapping(address voter => Vote vote)) public votes; + mapping(bytes32 messageHash => bool signed) public signedMessages; +} diff --git a/src/lib/HatsWalletErrors.sol b/src/lib/HatsWalletErrors.sol deleted file mode 100644 index b55d3c3..0000000 --- a/src/lib/HatsWalletErrors.sol +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -import "tokenbound/utils/Errors.sol"; - -/// @notice Thrown when the caller is not wearing the `hat` -error InvalidSigner(); -error NotHatsWallet(); -error ProposalAlreadyExists(); -error ProposalNotPending(); -error ProposalExpired(); -error InsufficientValidVotes(); -error UnsortedVotersArray(); diff --git a/src/lib/HatsWalletStorage.sol b/src/lib/HatsWalletStorage.sol deleted file mode 100644 index 949a5d8..0000000 --- a/src/lib/HatsWalletStorage.sol +++ /dev/null @@ -1,9 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -contract HatsWalletStorage { - uint256 _state; - string public version_; - - // TODO add m-of-n storage -} diff --git a/src/lib/LibHatsWallet.sol b/src/lib/LibHatsAccount.sol similarity index 97% rename from src/lib/LibHatsWallet.sol rename to src/lib/LibHatsAccount.sol index a59d48f..645c1c4 100644 --- a/src/lib/LibHatsWallet.sol +++ b/src/lib/LibHatsAccount.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.19; -import "./HatsWalletErrors.sol"; +import "./HatsAccountErrors.sol"; import { LibExecutor, LibSandbox } from "tokenbound/lib/LibExecutor.sol"; -library LibHatsWallet { +library LibHatsAccount { function _execute(address _to, uint256 _value, bytes calldata _data, uint8 _operation) internal returns (bytes memory) diff --git a/test/HatsWallet1ofN.t.sol b/test/HatsAccount1ofN.t.sol similarity index 92% rename from test/HatsWallet1ofN.t.sol rename to test/HatsAccount1ofN.t.sol index ab27b76..c25455e 100644 --- a/test/HatsWallet1ofN.t.sol +++ b/test/HatsAccount1ofN.t.sol @@ -2,17 +2,17 @@ pragma solidity ^0.8.19; import { Test, console2, StdUtils } from "forge-std/Test.sol"; -import { HatsWalletBaseTest } from "./HatsWalletBase.t.sol"; -import { HatsWalletBase, HatsWallet1ofN } from "../src/HatsWallet1ofN.sol"; -import "../src/lib/HatsWalletErrors.sol"; -import { Operation } from "../src/lib/LibHatsWallet.sol"; +import { HatsAccountBaseTest } from "./HatsAccountBase.t.sol"; +import { HatsAccountBase, HatsAccount1ofN } from "../src/HatsAccount1ofN.sol"; +import "../src/lib/HatsAccountErrors.sol"; +import { Operation } from "../src/lib/LibHatsAccount.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ECDSA, SignerMock, MaliciousStateChanger } from "./utils/TestContracts.sol"; import { IMulticall3 } from "multicall/interfaces/IMulticall3.sol"; import { IERC1271 } from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import { IERC6551Executable } from "erc6551/interfaces/IERC6551Executable.sol"; -contract HatsWallet1ofNTest is HatsWalletBaseTest { +contract HatsAccount1ofNTest is HatsAccountBaseTest { event TxExecuted(address signer); function setUp() public virtual override { @@ -30,14 +30,14 @@ contract HatsWallet1ofNTest is HatsWalletBaseTest { } } -contract Constants is HatsWallet1ofNTest { +contract Constants is HatsAccount1ofNTest { function test_version() public { assertEq(implementation.version_(), version, "wrong implementation version"); assertEq(instance.version(), version, "wrong instance version"); } } -contract Execute is HatsWallet1ofNTest { +contract Execute is HatsAccount1ofNTest { bytes public data; IMulticall3 public multicall = IMulticall3(MULTICALL3_ADDRESS); uint256 state; @@ -102,8 +102,9 @@ contract Execute is HatsWallet1ofNTest { vm.prank(wearer1); instance.execute(target, 1 ether, EMPTY_BYTES, 0); - expState = - calculateNewState(state, abi.encodeWithSelector(HatsWallet1ofN.execute.selector, target, 1 ether, EMPTY_BYTES, 0)); + expState = calculateNewState( + state, abi.encodeWithSelector(HatsAccount1ofN.execute.selector, target, 1 ether, EMPTY_BYTES, 0) + ); assertEq(target.balance, 1 ether); assertEq(address(instance).balance, 99 ether); @@ -120,7 +121,7 @@ contract Execute is HatsWallet1ofNTest { instance.execute(address(DAI), 0, data, 0); expState = - calculateNewState(state, abi.encodeWithSelector(HatsWallet1ofN.execute.selector, address(DAI), 0, data, 0)); + calculateNewState(state, abi.encodeWithSelector(HatsAccount1ofN.execute.selector, address(DAI), 0, data, 0)); assertEq(DAI.balanceOf(target), 1 ether); assertEq(DAI.balanceOf(address(instance)), 99 ether); @@ -164,7 +165,7 @@ contract Execute is HatsWallet1ofNTest { instance.execute(address(multicall), 0, data, 1); expState = - calculateNewState(state, abi.encodeWithSelector(HatsWallet1ofN.execute.selector, address(multicall), 0, data, 1)); + calculateNewState(state, abi.encodeWithSelector(HatsAccount1ofN.execute.selector, address(multicall), 0, data, 1)); assertEq(DAI.balanceOf(target), 30 ether); assertEq(DAI.balanceOf(address(instance)), 70 ether); @@ -216,7 +217,7 @@ contract Execute is HatsWallet1ofNTest { } } -contract ExecuteBatch is HatsWallet1ofNTest { +contract ExecuteBatch is HatsAccount1ofNTest { uint256 state; uint256 expState; // one operation @@ -231,7 +232,7 @@ contract ExecuteBatch is HatsWallet1ofNTest { vm.prank(wearer1); instance.executeBatch(ops); - expState = calculateNewState(state, abi.encodeWithSelector(HatsWallet1ofN.executeBatch.selector, ops)); + expState = calculateNewState(state, abi.encodeWithSelector(HatsAccount1ofN.executeBatch.selector, ops)); // check that the operation effects were applied assertEq(target.balance, 1 ether); @@ -256,7 +257,7 @@ contract ExecuteBatch is HatsWallet1ofNTest { vm.prank(wearer1); instance.executeBatch(ops); - expState = calculateNewState(state, abi.encodeWithSelector(HatsWallet1ofN.executeBatch.selector, ops)); + expState = calculateNewState(state, abi.encodeWithSelector(HatsAccount1ofN.executeBatch.selector, ops)); // check that the operation effects were applied assertEq(target.balance, n * 1 ether); @@ -280,7 +281,7 @@ contract ExecuteBatch is HatsWallet1ofNTest { } } -contract IsValidSignature is HatsWallet1ofNTest { +contract IsValidSignature is HatsAccount1ofNTest { SignerMock public wearerContract; SignerMock public nonWearerContract; @@ -379,7 +380,7 @@ contract IsValidSignature is HatsWallet1ofNTest { } } -contract ERC165 is HatsWallet1ofNTest { +contract ERC165 is HatsAccount1ofNTest { function test_true_IERC6551Executable() public { assertTrue(instance.supportsInterface(type(IERC6551Executable).interfaceId)); assertTrue(instance.supportsInterface(0x51945447)); diff --git a/test/HatsWalletBase.t.sol b/test/HatsAccountBase.t.sol similarity index 86% rename from test/HatsWalletBase.t.sol rename to test/HatsAccountBase.t.sol index 0bcdcfe..46e75d5 100644 --- a/test/HatsWalletBase.t.sol +++ b/test/HatsAccountBase.t.sol @@ -3,20 +3,20 @@ pragma solidity ^0.8.19; import { Test, console2, StdUtils } from "forge-std/Test.sol"; import { WithForkTest } from "./Base.t.sol"; -import { HatsWalletBase, HatsWallet1ofN } from "../src/HatsWallet1ofN.sol"; -import "../src/lib/HatsWalletErrors.sol"; -import { DeployImplementation, DeployWallet } from "../script/HatsWallet1ofN.s.sol"; +import { HatsAccountBase, HatsAccount1ofN } from "../src/HatsAccount1ofN.sol"; +import "../src/lib/HatsAccountErrors.sol"; +import { DeployImplementation, DeployWallet } from "../script/HatsAccount1ofN.s.sol"; import { IERC721Receiver } from "@openzeppelin/contracts/interfaces/IERC721Receiver.sol"; import { IERC1155Receiver } from "@openzeppelin/contracts/interfaces/IERC1155Receiver.sol"; import { IERC6551Account } from "tokenbound/abstract/ERC6551Account.sol"; import { ERC721, ERC1155, MockERC721, MockERC1155 } from "./utils/TestContracts.sol"; -contract HatsWalletBaseTest is DeployImplementation, WithForkTest { +contract HatsAccountBaseTest is DeployImplementation, WithForkTest { // variables inhereted from DeployImplementation // bytes32 public constant SALT; - // HatsWallet1ofN public implementation; + // HatsAccount1ofN public implementation; - HatsWallet1ofN public instance; + HatsAccount1ofN public instance; DeployWallet public deployWallet; address public benefactor = makeAddr("benefactor"); @@ -35,11 +35,11 @@ contract HatsWalletBaseTest is DeployImplementation, WithForkTest { deployWallet = new DeployWallet(); deployWallet.prepare(false, address(implementation), hatWithWallet, SALT); // deploy wallet instance - instance = HatsWallet1ofN(payable(deployWallet.run())); + instance = HatsAccount1ofN(payable(deployWallet.run())); } } -contract Constants is HatsWalletBaseTest { +contract Constants is HatsAccountBaseTest { function test_hat() public { // console2.log("hat()", instance.hat()); assertEq(instance.hat(), hatWithWallet); @@ -66,7 +66,7 @@ contract Constants is HatsWalletBaseTest { } } -contract IsValidSigner is HatsWalletBaseTest { +contract IsValidSigner is HatsAccountBaseTest { function test_true_wearer() public { assertEq(instance.isValidSigner(wearer1, EMPTY_BYTES), ERC6551_MAGIC_NUMBER); } @@ -76,7 +76,7 @@ contract IsValidSigner is HatsWalletBaseTest { } } -contract Receive is HatsWalletBaseTest { +contract Receive is HatsAccountBaseTest { function test_receive_eth() public { // bankroll benefactor vm.deal(benefactor, 100 ether); @@ -137,7 +137,7 @@ contract Receive is HatsWalletBaseTest { } } -contract ERC165 is HatsWalletBaseTest { +contract ERC165 is HatsAccountBaseTest { function test_true_ERC721Receiver() public { assertTrue(instance.supportsInterface(type(IERC721Receiver).interfaceId)); } @@ -153,10 +153,10 @@ contract ERC165 is HatsWalletBaseTest { } /* - See HatsWallet1ofN.t.sol for coverage of the following HatsWalletBase internal functions: + See HatsAccount1ofN.t.sol for coverage of the following HatsAccountBase internal functions: - _beforeExecute - _updateState - See HatsWallet1ofN.t.sol for coverage of the following HatsWalletBase functions: + See HatsAccount1ofN.t.sol for coverage of the following HatsAccountBase functions: - getMessageHash */ diff --git a/test/HatsWalletMofN.t.sol b/test/HatsAccountMofN.t.sol similarity index 85% rename from test/HatsWalletMofN.t.sol rename to test/HatsAccountMofN.t.sol index 5cf3705..738de99 100644 --- a/test/HatsWalletMofN.t.sol +++ b/test/HatsAccountMofN.t.sol @@ -3,23 +3,23 @@ pragma solidity ^0.8.19; import { Test, console2, StdUtils } from "forge-std/Test.sol"; import { BaseTest, WithForkTest } from "./Base.t.sol"; -import { HatsWalletBase, HatsWalletMofN } from "../src/HatsWalletMofN.sol"; -import { Operation, ProposalStatus, Vote } from "../src/lib/LibHatsWallet.sol"; +import { HatsAccountBase, HatsAccountMofN } from "../src/HatsAccountMofN.sol"; +import { Operation, ProposalStatus, Vote } from "../src/lib/LibHatsAccount.sol"; import { ERC6551Account } from "tokenbound/abstract/ERC6551Account.sol"; -import "../src/lib/HatsWalletErrors.sol"; -import { DeployImplementation, DeployWallet } from "../script/HatsWalletMofN.s.sol"; +import "../src/lib/HatsAccountErrors.sol"; +import { DeployImplementation, DeployWallet } from "../script/HatsAccountMofN.s.sol"; import { IERC6551Registry } from "erc6551/interfaces/IERC6551Registry.sol"; import { IHats } from "hats-protocol/Interfaces/IHats.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { IERC1271 } from "@openzeppelin/contracts/interfaces/IERC1271.sol"; import { ECDSA, SignerMock, MaliciousStateChanger, MofNMock } from "./utils/TestContracts.sol"; -contract HatsWalletMofNTest is DeployImplementation, WithForkTest { +contract HatsAccountMofNTest is DeployImplementation, WithForkTest { // variables inhereted from DeployImplementation // bytes32 public constant SALT; - // HatsWalletMofN public implementation; + // HatsAccountMofN public implementation; - HatsWalletMofN public instance; + HatsAccountMofN public instance; DeployWallet public deployWallet; uint8 public minThreshold; @@ -108,13 +108,13 @@ contract HatsWalletMofNTest is DeployImplementation, WithForkTest { function deployWalletWithThresholds(uint256 _minThreshold, uint256 _maxThreshold) public - returns (HatsWalletMofN wallet) + returns (HatsAccountMofN wallet) { uint256 cap = nWearers - 2; uint8 min = uint8(bound(_minThreshold, 1, cap)); uint8 max = uint8(bound(_maxThreshold, minThreshold, cap)); deployWallet.prepare(false, address(implementation), hatWithWallet, _calculateSalt(min, max)); - wallet = HatsWalletMofN(payable(deployWallet.run())); + wallet = HatsAccountMofN(payable(deployWallet.run())); // bankroll the wallet with some ETH vm.deal(address(wallet), 10 ether); } @@ -300,18 +300,19 @@ contract HatsWalletMofNTest is DeployImplementation, WithForkTest { bytes32 DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); - return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, "HatsWalletMofN", version, block.chainid, address(instance))); + return + keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, "HatsAccountMofN", version, block.chainid, address(instance))); } function _getMessageHash(bytes memory message) internal view returns (bytes32) { - bytes32 HATSWALLET_MSG_TYPEHASH = keccak256("HatsWallet(bytes message)"); + bytes32 HatsAccount_MSG_TYPEHASH = keccak256("HatsAccount(bytes message)"); bytes32 domainSeparator = _domainSeparator(); - bytes32 hatsWalletMessageHash = keccak256(abi.encode(HATSWALLET_MSG_TYPEHASH, keccak256(message))); + bytes32 HatsAccountMessageHash = keccak256(abi.encode(HatsAccount_MSG_TYPEHASH, keccak256(message))); return keccak256( - abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator, hatsWalletMessageHash) // HatsWalletMessageHash + abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator, HatsAccountMessageHash) // HatsAccountMessageHash ); } @@ -327,7 +328,7 @@ contract HatsWalletMofNTest is DeployImplementation, WithForkTest { ) { // encode the sign call - bytes memory data = abi.encodeWithSelector(HatsWalletMofN.sign.selector, message); + bytes memory data = abi.encodeWithSelector(HatsAccountMofN.sign.selector, message); // create and submit the proposal description = bytes32("description"); expiration = 0; @@ -361,7 +362,7 @@ contract HatsWalletMofNTest is DeployImplementation, WithForkTest { } } -contract Constants is HatsWalletMofNTest { +contract Constants is HatsAccountMofNTest { function test_thresholdRange() public { (uint256 min, uint256 max) = instance.THRESHOLD_RANGE(); assertEq(min, minThreshold); @@ -382,7 +383,7 @@ contract Constants is HatsWalletMofNTest { } } -contract GetThreshold is HatsWalletMofNTest { +contract GetThresholds is HatsAccountMofNTest { uint256 public hatWithWallet2; function setUp() public virtual override { @@ -391,7 +392,7 @@ contract GetThreshold is HatsWalletMofNTest { // create a new hat with supply of 0; vm.prank(org); hatWithWallet2 = - HATS.createHat(tophat, "hatWithWallet2", 0, eligibility, toggle, true, "org.eth/hatWithWallet2.png"); + HATS.createHat(tophat, "hatWithWallet2", 1, eligibility, toggle, true, "org.eth/hatWithWallet2.png"); } function test_getThreshold(uint32 supply, uint8 min, uint8 max) public { @@ -401,7 +402,7 @@ contract GetThreshold is HatsWalletMofNTest { // deploy a new instance with the given min and max threshold deployWallet.prepare(false, address(implementation), hatWithWallet2, _calculateSalt(min, max)); - instance = HatsWalletMofN(payable(deployWallet.run())); + instance = HatsAccountMofN(payable(deployWallet.run())); vm.prank(org); HATS.changeHatMaxSupply(hatWithWallet2, supply); @@ -430,9 +431,45 @@ contract GetThreshold is HatsWalletMofNTest { // ensure threshold is correct assertEq(instance.getThreshold(), expectedThreshold); } + + function test_getRejectionThreshold(uint32 supply, uint8 min, uint8 max) public { + supply = uint32(bound(supply, 0, nWearers)); + min = uint8(bound(min, 1, nWearers - 1)); + max = uint8(bound(max, min + 1, nWearers)); + + // deploy a new instance with the given min and max threshold + deployWallet.prepare(false, address(implementation), hatWithWallet2, _calculateSalt(min, max)); + instance = HatsAccountMofN(payable(deployWallet.run())); + + vm.prank(org); + HATS.changeHatMaxSupply(hatWithWallet2, supply); + + // mint some hats to meet the supply + vm.startPrank(org); + for (uint256 i; i < supply; i++) { + // mint to random wearers + HATS.mintHat(hatWithWallet2, wearers[i]); + } + vm.stopPrank(); + + // ensure supply is correct + assertEq(HATS.hatSupply(hatWithWallet2), supply); + + // calculate expected rejection threshold + uint256 expectedRejectionThreshold; + uint256 threshold = instance.getThreshold(); + if (supply < threshold) { + expectedRejectionThreshold = threshold; + } else { + expectedRejectionThreshold = supply - threshold + 1; + } + + // ensure rejection threshold is correct + assertEq(instance.getRejectionThreshold(), expectedRejectionThreshold); + } } -contract getProposalId is HatsWalletMofNTest { +contract getProposalId is HatsAccountMofNTest { function test_getProposalId( uint256 actionCount, address toSeed, @@ -464,7 +501,7 @@ contract getProposalId is HatsWalletMofNTest { } } -contract MockMofNTest is HatsWalletMofNTest { +contract MockMofNTest is HatsAccountMofNTest { MofNMock mockImplementation; MofNMock mock; @@ -513,7 +550,7 @@ contract _UnsafeVoting is MockMofNTest { } } -contract _getExpiration is HatsWalletMofNTest { +contract _getExpiration is HatsAccountMofNTest { function test_happy(Operation[] memory ops, uint32 expiration, bytes32 description) public { // create a simple proposal id bytes32 proposalId = createProposalId(ops, expiration, description); @@ -523,7 +560,7 @@ contract _getExpiration is HatsWalletMofNTest { } } -contract Propose is HatsWalletMofNTest { +contract Propose is HatsAccountMofNTest { function test_happy( uint256 actionCount, address toSeed, @@ -588,7 +625,7 @@ contract Propose is HatsWalletMofNTest { } } -contract Voting is HatsWalletMofNTest { +contract Voting is HatsAccountMofNTest { address[] voters; function test_approve() public { @@ -773,7 +810,7 @@ contract _CheckValidVotes is MockMofNTest { assertTrue(mock.checkValidVotes(proposalId, voters, vote, threshold)); } - function test_revert_insufficientVotes(uint256 threshold, uint256 _vote) public { + function test_revert_shortVotersArray(uint256 threshold, uint256 _vote) public { vote = Vote(bound(_vote, 1, 2)); // only APPROVE or REJECT votes bytes32 proposalId = bytes32("proposalId"); threshold = bound(threshold, 1, nWearers - 2); @@ -788,25 +825,25 @@ contract _CheckValidVotes is MockMofNTest { } // assert that {_checkValidVotes} reverts - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(VotersArrayTooShort.selector); mock.checkValidVotes(proposalId, voters, vote, threshold); } - function test_revert_insufficientValidVotes(uint256 threshold, uint256 _vote, uint256 wrongVoterCount) public { + function test_revert_invalidVotes(uint256 threshold, uint256 _vote, uint256 invalidVoterIndex) public { vote = Vote(bound(_vote, 1, 2)); // only APPROVE or REJECT votes Vote wrongVote = (vote == Vote.APPROVE) ? Vote.REJECT : Vote.APPROVE; bytes32 proposalId = bytes32("proposalId"); threshold = bound(threshold, 1, nWearers - 2); - wrongVoterCount = bound(wrongVoterCount, 1, threshold); + invalidVoterIndex = bound(invalidVoterIndex, 0, threshold - 1); // create a sorted array of enough voters to meet the threshold voters = createSortedVoterArray(threshold); // use {unsafeVote} to have each voter cast a vote even though the proposal hasn't been created - // the first wrongVoterCount voters will cast the wrong vote, and the rest will cast the correct vote + // the invalid voter will cast the wrong vote for (uint256 i; i < voters.length; ++i) { vm.prank(voters[i]); - if (i < wrongVoterCount) { + if (i == invalidVoterIndex) { mock.unsafeVote(proposalId, wrongVote); } else { mock.unsafeVote(proposalId, vote); @@ -814,7 +851,7 @@ contract _CheckValidVotes is MockMofNTest { } // assert that {_checkValidVotes} reverts - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidVote.selector, voters[invalidVoterIndex])); mock.checkValidVotes(proposalId, voters, vote, threshold); } @@ -858,19 +895,19 @@ contract _CheckValidVotes is MockMofNTest { mock.checkValidVotes(proposalId, voters, vote, threshold); } - function test_revert_invalidVoter(uint256 threshold, uint256 _vote, uint256 invalidVoterCount) public { + function test_revert_invalidVoter(uint256 threshold, uint256 _vote, uint256 invalidVoterIndex) public { vote = Vote(bound(_vote, 1, 2)); // only APPROVE or REJECT votes bytes32 proposalId = bytes32("proposalId"); threshold = bound(threshold, 1, nWearers - 2); - invalidVoterCount = bound(invalidVoterCount, 1, threshold); + invalidVoterIndex = bound(invalidVoterIndex, 0, threshold - 1); // create a sorted array of enough voters to meet the threshold voters = createSortedVoterArray(threshold); // use {unsafeVote} to have each voter cast a vote even though the proposal hasn't been created - // the first invalidVoterCount voters will be invalidated by having their hat revoked, and the rest will be valid + // the invalidVoterIndex voter will have their hat revoked, making them invalid for (uint256 i; i < voters.length; ++i) { - if (i < invalidVoterCount) { + if (i == invalidVoterIndex) { // revoke the voter's hat to make them invalid vm.prank(eligibility); HATS.setHatWearerStatus(hatWithWallet, voters[i], false, true); @@ -881,12 +918,12 @@ contract _CheckValidVotes is MockMofNTest { } // assert that {_checkValidVotes} reverts - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidVote.selector, voters[invalidVoterIndex])); mock.checkValidVotes(proposalId, voters, vote, threshold); } } -contract ProposeWithApproval is HatsWalletMofNTest { +contract ProposeWithApproval is HatsAccountMofNTest { address proposer; function test_happy(uint256 proposerIndex) public { @@ -948,7 +985,7 @@ contract ProposeWithApproval is HatsWalletMofNTest { } } -contract IsExecutableNow is HatsWalletMofNTest { +contract IsExecutableNow is HatsAccountMofNTest { address[] voters; function test_true(uint256 _minThreshold, uint256 _maxThreshold) public { @@ -1017,7 +1054,7 @@ contract IsExecutableNow is HatsWalletMofNTest { } } - function test_revert_insufficientValidVotes(uint256 _minThreshold, uint256 _maxThreshold) public { + function test_revert_invalidVotes(uint256 _minThreshold, uint256 _maxThreshold) public { // deploy a new instance with bounded min and max threshold instance = deployWalletWithThresholds(_minThreshold, _maxThreshold); @@ -1030,14 +1067,14 @@ contract IsExecutableNow is HatsWalletMofNTest { // build the array of voters voters = createSortedVoterArray(threshold); - // threshold - 1 voters approve the proposal + // threshold - 1 vot ers approve the proposal for (uint256 i; i < threshold - 1; ++i) { vm.prank(voters[i]); instance.vote(proposalId, Vote.APPROVE); } // assert that the proposal is not executable - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidVote.selector, voters[threshold - 1])); instance.isExecutableNow(proposalId, voters); } @@ -1138,7 +1175,7 @@ contract IsExecutableNow is HatsWalletMofNTest { } } -contract Execute is HatsWalletMofNTest { +contract Execute is HatsAccountMofNTest { address[] voters; uint256 state; uint256 expState; @@ -1168,7 +1205,7 @@ contract Execute is HatsWalletMofNTest { // assertions expState = calculateNewState( - state, abi.encodeWithSelector(HatsWalletMofN.execute.selector, ops, expiration, description, voters) + state, abi.encodeWithSelector(HatsAccountMofN.execute.selector, ops, expiration, description, voters) ); assertEq(instance.state(), expState); @@ -1215,7 +1252,7 @@ contract Execute is HatsWalletMofNTest { // assertions expState = calculateNewState( - state, abi.encodeWithSelector(HatsWalletMofN.execute.selector, ops, expiration, description, voters) + state, abi.encodeWithSelector(HatsAccountMofN.execute.selector, ops, expiration, description, voters) ); assertEq(instance.state(), expState); @@ -1270,7 +1307,7 @@ contract Execute is HatsWalletMofNTest { // assertions expState = calculateNewState( - state, abi.encodeWithSelector(HatsWalletMofN.execute.selector, ops, expiration, description, voters) + state, abi.encodeWithSelector(HatsAccountMofN.execute.selector, ops, expiration, description, voters) ); assertEq(instance.state(), expState); @@ -1281,7 +1318,7 @@ contract Execute is HatsWalletMofNTest { assertEq(results.length, 3); } - function test_revert_insufficientValidVotes() public { + function test_revert_shortVotersArray() public { state = instance.state(); // submit a simple proposal (Operation[] memory ops, uint32 expiration, bytes32 proposalId, bytes32 description) = submitSimpleProposal(wearer1); @@ -1299,7 +1336,65 @@ contract Execute is HatsWalletMofNTest { } // execute the proposal, expecting a revert - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(VotersArrayTooShort.selector); + instance.execute(ops, expiration, description, voters); + + // assert that the proposal was not executed + assertEq(instance.proposalStatus(proposalId), ProposalStatus.PENDING); + assertEq(results.length, 0); + assertEq(target.balance, 0); + assertEq(address(instance).balance, 10 ether); + assertEq(instance.state(), state); + } + + function test_revert_invalidVotes() public { + state = instance.state(); + // submit a simple proposal + (Operation[] memory ops, uint32 expiration, bytes32 proposalId, bytes32 description) = submitSimpleProposal(wearer1); + + // get the current threshold + uint256 threshold = instance.getThreshold(); + + // build the array of voters + voters = createSortedVoterArray(threshold); + + // threshold - 1 number of voters approve the proposal + for (uint256 i; i < threshold - 1; ++i) { + vm.prank(voters[i]); + instance.vote(proposalId, Vote.APPROVE); + } + + // execute the proposal, expecting a revert + vm.expectRevert(abi.encodeWithSelector(InvalidVote.selector, voters[threshold - 1])); + instance.execute(ops, expiration, description, voters); + + // assert that the proposal was not executed + assertEq(instance.proposalStatus(proposalId), ProposalStatus.PENDING); + assertEq(results.length, 0); + assertEq(target.balance, 0); + assertEq(address(instance).balance, 10 ether); + assertEq(instance.state(), state); + } + + function test_revert_unsortedVotersArray() public { + state = instance.state(); + // submit a simple proposal + (Operation[] memory ops, uint32 expiration, bytes32 proposalId, bytes32 description) = submitSimpleProposal(wearer1); + + // get the current threshold + uint256 threshold = instance.getThreshold(); + + // build the array of voters, but unsorted + voters = createUnsortedVoterArray(threshold, 1); + + // threshold number of voters approve the proposal + for (uint256 i; i < threshold; ++i) { + vm.prank(voters[i]); + instance.vote(proposalId, Vote.APPROVE); + } + + // execute the proposal, expecting a revert + vm.expectRevert(UnsortedVotersArray.selector); instance.execute(ops, expiration, description, voters); // assert that the proposal was not executed @@ -1394,7 +1489,7 @@ contract Execute is HatsWalletMofNTest { instance.execute(ops, expiration, description, voters); expState = calculateNewState( - state, abi.encodeWithSelector(HatsWalletMofN.execute.selector, ops, expiration, description, voters) + state, abi.encodeWithSelector(HatsAccountMofN.execute.selector, ops, expiration, description, voters) ); // execute the proposal again, expecting a revert @@ -1442,7 +1537,7 @@ contract Execute is HatsWalletMofNTest { } } -contract IsRejectableNow is HatsWalletMofNTest { +contract IsRejectableNow is HatsAccountMofNTest { address[] voters; function test_true(uint256 _minThreshold, uint256 _maxThreshold) public { @@ -1468,7 +1563,7 @@ contract IsRejectableNow is HatsWalletMofNTest { assertTrue(instance.isRejectableNow(proposalId, voters)); } - function test_revert_insufficientValidVotes(uint256 _minThreshold, uint256 _maxThreshold) public { + function test_revert_shortVotersArray(uint256 _minThreshold, uint256 _maxThreshold) public { // deploy a new instance with bounded min and max threshold instance = deployWalletWithThresholds(_minThreshold, _maxThreshold); @@ -1478,7 +1573,7 @@ contract IsRejectableNow is HatsWalletMofNTest { // get the current rejection threshold uint256 rejectionThreshold = instance.getRejectionThreshold(); - // build the array of voters + // build the array of voters that is one less than the threshold voters = createSortedVoterArray(rejectionThreshold - 1); // threshold - 1 number of voters reject the proposal @@ -1488,7 +1583,31 @@ contract IsRejectableNow is HatsWalletMofNTest { } // assert that the proposal is not rejectable - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(VotersArrayTooShort.selector); + instance.isRejectableNow(proposalId, voters); + } + + function test_revert_invalidVotes() public { + // deploy a new instance with bounded min and max threshold + instance = deployWalletWithThresholds(2, 3); + + // submit a simple proposal + (,, bytes32 proposalId,) = submitSimpleProposal(wearer1); + + // get the current rejection threshold + uint256 rejectionThreshold = instance.getRejectionThreshold(); + + // build the array of voters + voters = createSortedVoterArray(rejectionThreshold); + + // threshold - 1 number of voters reject the proposal + for (uint256 i; i < rejectionThreshold - 1; ++i) { + vm.prank(voters[i]); + instance.vote(proposalId, Vote.REJECT); + } + + // assert that the proposal is not rejectable + vm.expectRevert(abi.encodeWithSelector(InvalidVote.selector, voters[rejectionThreshold - 1])); instance.isRejectableNow(proposalId, voters); } @@ -1589,7 +1708,7 @@ contract IsRejectableNow is HatsWalletMofNTest { } } -contract Reject is HatsWalletMofNTest { +contract Reject is HatsAccountMofNTest { address[] voters; function test_reject(uint256 _minThreshold, uint256 _maxThreshold) public { @@ -1620,7 +1739,7 @@ contract Reject is HatsWalletMofNTest { assertEq(instance.proposalStatus(proposalId), ProposalStatus.REJECTED); } - function test_revert_insufficientValidVotes(uint256 _minThreshold, uint256 _maxThreshold) public { + function test_revert_shortVotersArray(uint256 _minThreshold, uint256 _maxThreshold) public { // deploy a new instance with bounded min and max threshold instance = deployWalletWithThresholds(_minThreshold, _maxThreshold); uint256 rejectionThreshold = instance.getRejectionThreshold(); @@ -1628,7 +1747,7 @@ contract Reject is HatsWalletMofNTest { // submit a simple proposal (,, bytes32 proposalId,) = submitSimpleProposal(wearer1); - // build the array of voters + // build the array of voters that is one less than the threshold voters = createSortedVoterArray(rejectionThreshold - 1); // threshold - 1 number of voters reject the proposal @@ -1638,7 +1757,57 @@ contract Reject is HatsWalletMofNTest { } // assert that the proposal is not rejectable - vm.expectRevert(InsufficientValidVotes.selector); + vm.expectRevert(VotersArrayTooShort.selector); + instance.reject(proposalId, voters); + + // assert that the proposal was not rejected + assertEq(instance.proposalStatus(proposalId), ProposalStatus.PENDING); + } + + function test_revert_invalidVotes(uint256 _minThreshold, uint256 _maxThreshold) public { + // deploy a new instance with bounded min and max threshold + instance = deployWalletWithThresholds(_minThreshold, _maxThreshold); + uint256 rejectionThreshold = instance.getRejectionThreshold(); + + // submit a simple proposal + (,, bytes32 proposalId,) = submitSimpleProposal(wearer1); + + // build the array of voters + voters = createSortedVoterArray(rejectionThreshold); + + // threshold - 1 number of voters reject the proposal + for (uint256 i; i < rejectionThreshold - 1; ++i) { + vm.prank(voters[i]); + instance.vote(proposalId, Vote.REJECT); + } + + // assert that the proposal is not rejectable + vm.expectRevert(abi.encodeWithSelector(InvalidVote.selector, voters[rejectionThreshold - 1])); + instance.reject(proposalId, voters); + + // assert that the proposal was not rejected + assertEq(instance.proposalStatus(proposalId), ProposalStatus.PENDING); + } + + function test_revert_unsortedVotersArray(uint256 _minThreshold, uint256 _maxThreshold) public { + // deploy a new instance with bounded min and max threshold + instance = deployWalletWithThresholds(_minThreshold, _maxThreshold); + uint256 rejectionThreshold = instance.getRejectionThreshold(); + + // submit a simple proposal + (,, bytes32 proposalId,) = submitSimpleProposal(wearer1); + + // build the array of voters, but unsorted + voters = createUnsortedVoterArray(rejectionThreshold, 1); + + // threshold number of voters reject the proposal + for (uint256 i; i < rejectionThreshold; ++i) { + vm.prank(voters[i]); + instance.vote(proposalId, Vote.REJECT); + } + + // assert that the proposal is not rejectable + vm.expectRevert(UnsortedVotersArray.selector); instance.reject(proposalId, voters); // assert that the proposal was not rejected @@ -1646,7 +1815,7 @@ contract Reject is HatsWalletMofNTest { } } -contract ValidVoteCountsNow is HatsWalletMofNTest { +contract ValidVoteCountsNow is HatsAccountMofNTest { function test_validVoteCountsNow(uint256 approvals, uint256 rejections) public { uint256 cap = nWearers; approvals = bound(approvals, 0, cap); @@ -1674,7 +1843,7 @@ contract ValidVoteCountsNow is HatsWalletMofNTest { } } -contract MessageHashing is HatsWalletMofNTest { +contract MessageHashing is HatsAccountMofNTest { function test_domainSeparator() public { // assert that the domain separator is correct assertEq(instance.domainSeparator(), _domainSeparator()); @@ -1686,7 +1855,7 @@ contract MessageHashing is HatsWalletMofNTest { } } -contract Sign is HatsWalletMofNTest { +contract Sign is HatsAccountMofNTest { function test_happy(bytes memory message) public { // sign the message with an executed proposal (Operation[] memory ops, uint32 expiration, bytes32 description, address[] memory voters, bytes32 proposalId) = @@ -1708,11 +1877,11 @@ contract Sign is HatsWalletMofNTest { assertEq(instance.isValidSignature(messageHash, EMPTY_BYTES), ERC1271_MAGIC_VALUE); } - function test_revert_notHatsWallet() public { + function test_revert_notHatsAccount() public { bytes memory message = bytes("message"); // attempt to sign the message, expecting a revert - vm.expectRevert(NotHatsWallet.selector); + vm.expectRevert(NotHatsAccount.selector); instance.sign(message); bytes32 messageHash = instance.getMessageHash(message); @@ -1729,7 +1898,7 @@ contract Sign is HatsWalletMofNTest { } } -contract IsValidSignature is HatsWalletMofNTest { +contract IsValidSignature is HatsAccountMofNTest { function test_true(bytes memory message) public { // sign the message with an executed proposal (Operation[] memory ops, uint32 expiration, bytes32 description, address[] memory voters,) = @@ -1767,7 +1936,7 @@ contract IsValidSignature is HatsWalletMofNTest { } } -contract ERC165 is HatsWalletMofNTest { +contract ERC165 is HatsAccountMofNTest { function test_true_ERC1271() public { assertTrue(instance.supportsInterface(type(IERC1271).interfaceId)); } diff --git a/test/LibHatsWallet.t.sol b/test/LibHatsAccount.t.sol similarity index 99% rename from test/LibHatsWallet.t.sol rename to test/LibHatsAccount.t.sol index ce222ab..78778f5 100644 --- a/test/LibHatsWallet.t.sol +++ b/test/LibHatsAccount.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.19; import { Test, console2, StdUtils } from "forge-std/Test.sol"; import { WithForkTest } from "./Base.t.sol"; -import "../src/lib/HatsWalletErrors.sol"; +import "../src/lib/HatsAccountErrors.sol"; import { LibSandbox } from "tokenbound/lib/LibSandbox.sol"; import { MockHW } from "./utils/TestContracts.sol"; import { IMulticall3 } from "multicall/interfaces/IMulticall3.sol"; diff --git a/test/utils/TestContracts.sol b/test/utils/TestContracts.sol index cb94e31..ee36de7 100644 --- a/test/utils/TestContracts.sol +++ b/test/utils/TestContracts.sol @@ -2,11 +2,11 @@ pragma solidity ^0.8.19; import { Test, console2 } from "forge-std/Test.sol"; -import { HatsWallet1ofN } from "../../src/HatsWallet1ofN.sol"; -import { HatsWalletMofN } from "../../src/HatsWalletMofN.sol"; -import { LibHatsWallet, LibSandbox } from "../../src/lib/LibHatsWallet.sol"; -import { HatsWalletStorage } from "../../src/lib/HatsWalletStorage.sol"; -import { Operation, Vote } from "../../src/lib/LibHatsWallet.sol"; +import { HatsAccount1ofN } from "../../src/HatsAccount1ofN.sol"; +import { HatsAccountMofN } from "../../src/HatsAccountMofN.sol"; +import { LibHatsAccount, LibSandbox } from "../../src/lib/LibHatsAccount.sol"; +import { HatsAccountStorage } from "../../src/lib/HatsAccountStorage.sol"; +import { Operation, Vote } from "../../src/lib/LibHatsAccount.sol"; import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; import { ERC1155 } from "@openzeppelin/contracts/token/ERC1155/ERC1155.sol"; import { ECDSA } from "solady/utils/ECDSA.sol"; @@ -37,7 +37,7 @@ contract SignerMock { } } -contract MaliciousStateChanger is HatsWalletStorage { +contract MaliciousStateChanger is HatsAccountStorage { function decrementState() public { --_state; } @@ -56,16 +56,16 @@ contract MockERC1155 is ERC1155 { } } -contract MockHW is HatsWallet1ofN { - constructor(string memory _version) HatsWallet1ofN(_version) { } +contract MockHW is HatsAccount1ofN { + constructor(string memory _version) HatsAccount1ofN(_version) { } - /// @dev exposes the internal {LibHatsWallet._execute} function for testing + /// @dev exposes the internal {LibHatsAccount._execute} function for testing function execute_(address _to, uint256 _value, bytes calldata _data, uint8 _operation) external payable returns (bytes memory result) { - return LibHatsWallet._execute(_to, _value, _data, _operation); + return LibHatsAccount._execute(_to, _value, _data, _operation); } function getSandbox() public view returns (address) { @@ -77,8 +77,8 @@ contract MockHW is HatsWallet1ofN { } } -contract MofNMock is HatsWalletMofN { - constructor(string memory _version) HatsWalletMofN(_version) { } +contract MofNMock is HatsAccountMofN { + constructor(string memory _version) HatsAccountMofN(_version) { } /// @dev exposes the internal {_unsafeVote} function for testing function unsafeVote(bytes32 _proposalId, Vote _vote) public {