diff --git a/docs/specs.md b/docs/specs.md index 41ef650d..3a820310 100644 --- a/docs/specs.md +++ b/docs/specs.md @@ -17,7 +17,7 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S |CER-36 |Account and Vault Status Checks immediate |If the Execution Context's `checksDeferred` flag is not set, required Account and Vault Status Checks MUST be performed immediately | |CER-48 |Account and Vault Status Checks not performed |Forgiven Account and Vault Status Checks will not be performed when the Execution Context's `checksDeferred` flag is cleared, unless they are required again after the forgiveness | |CER-35 |Account and Vault Status Checks storage sets restored |Both Account- and Vault-Status-Checks-related storage sets MUST return to their default state when the Execution Context's `checksDeferred` flag is cleared and the checks are performed | -|CER-25 |Call/Batch EVC self-call |In Call and Batch, if the target is the EVC, the account specified MUST be `address(0)` for the sake of consistency. In that case, the EVC MUST be `delegatecall`ed to preserve the `msg.sender` and, depending on a function, a function-specific authentication is performed | +|CER-25 |Call/Batch EVC self-call |In Call and Batch, for the sake of consistency, if the target is the EVC, the account specified MUST be address(0) and the value specified MUST be 0. In that case, the EVC MUST be `delegatecall`ed to preserve the `msg.sender` and, depending on a function, a function-specific authentication is performed | |CER-23 |Call/Batch callback |In Call and Batch, if the target is `msg.sender`, the caller MAY specify any Account address to be set in the Execution Context's on behalf of Account address. In that case, the authentication is not performed | |CER-24 |Call/Batch external call |In Call and Batch, if the target is not `msg.sender`, only the Owner or an Operator of the specified Account MUST be allowed to perform Call and individual Batch operations on behalf of the Account | |CER-20 |Checks-deferrable calls | | diff --git a/docs/whitepaper.md b/docs/whitepaper.md index 0126512c..c3ba8648 100644 --- a/docs/whitepaper.md +++ b/docs/whitepaper.md @@ -235,7 +235,7 @@ Sub-accounts allow users access to multiple (up to 256) virtual accounts that ar Since an account can only have one controller at a time (except for mid-transaction), sub-accounts are also the only way an Ethereum account can hold multiple Vault borrows concurrently. -The EVC also maintains a look-up mapping `ownerLookup` so sub-accounts can be easily resolved to owner addresses, on- or off-chain. This mapping is populated when an address interacts with the EVC for the first time. In order to resolve a sub-account, the `getAccountOwner` function should be called with a sub-account address. It will either return the account's primary address, or revert with an error if the account has not yet interacted with the EVC. +The EVC also maintains a look-up mapping `ownerLookup` so sub-accounts can be easily resolved to owner addresses, on- or off-chain. This mapping is populated when an address interacts with the EVC for the first time. In order to resolve a sub-account, the `getAccountOwner` function should be called with a sub-account address. It will either return the account's primary address, or `address(0)` if the account has not yet interacted with the EVC. #### Operators @@ -270,9 +270,7 @@ The previous value of `onBehalfOfAccount` is stored in a local "cache" variable #### checksInProgress -The EVC invokes the `checkAccountStatus` and `checkVaultStatus` callbacks using low-level `call` instead of `staticcall` so that controllers can checkpoint state during these operations. However, because of this there is a danger that the EVC could be re-entered during these operations, either directly by a controller, or indirectly by a contract it invokes. - -Because of this, the EVC's execution context maintains a `checksInProgress` mutex that is acquired before unwinding the sets of accounts and vaults that need checking. This mutex is also checked during operations that alter these sets. If it did not do this, then information cached by the higher-level unwinding function (such as the sizes of the sets) could become inconsistent with the underlying storage, which could be used to bypass these critical checks. +Because the EVC invokes the `checkAccountStatus` and `checkVaultStatus` callbacks that could re-enter the EVC, either directly or by a contract they invoke, the EVC's execution context maintains a `checksInProgress` mutex that is acquired before unwinding the sets of accounts and vaults that need checking. This mutex is also checked during operations that alter these sets. If it did not do this, then information cached by the higher-level unwinding function (such as the sizes of the sets) could become inconsistent with the underlying storage, which could be used to bypass these critical checks. #### controlCollateralInProgress diff --git a/src/EthereumVaultConnector.sol b/src/EthereumVaultConnector.sol index 725a2308..95009da8 100644 --- a/src/EthereumVaultConnector.sol +++ b/src/EthereumVaultConnector.sol @@ -26,15 +26,11 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { /// @notice Name of the Ethereum Vault Connector. string public constant name = "Ethereum Vault Connector"; - /// @notice Version of the Ethereum Vault Connector. - string public constant version = "1"; - uint160 internal constant ACCOUNT_ID_OFFSET = 8; bytes32 internal constant HASHED_NAME = keccak256(bytes(name)); - bytes32 internal constant HASHED_VERSION = keccak256(bytes(version)); bytes32 internal constant TYPE_HASH = - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); bytes32 internal constant PERMIT_TYPEHASH = keccak256( "Permit(address signer,address sender,uint256 nonceNamespace,uint256 nonce,uint256 deadline,uint256 value,bytes data)" @@ -260,7 +256,8 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { /// @inheritdoc IEVC function getAccountOwner(address account) external view returns (address) { - return getAccountOwnerInternal(account); + bytes19 addressPrefix = getAddressPrefixInternal(account); + return ownerLookup[addressPrefix].owner; } /// @inheritdoc IEVC @@ -364,12 +361,13 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { /// caller address at once. function setAccountOperator(address account, address operator, bool authorized) public payable virtual { address msgSender = authenticateCaller({account: account, allowOperator: true, checkLockdownMode: false}); + bytes19 addressPrefix = getAddressPrefixInternal(account); // if the account and the caller have a common owner, the caller must be the owner. if the account and the // caller don't have a common owner, the caller must be an operator and the owner address is taken from the // storage. the caller authentication above guarantees that the account owner is already registered hence // non-zero - address owner = haveCommonOwnerInternal(account, msgSender) ? msgSender : getAccountOwnerInternal(account); + address owner = haveCommonOwnerInternal(account, msgSender) ? msgSender : ownerLookup[addressPrefix].owner; // if it's an operator calling, it can only act for itself and must not be able to change other operators status if (owner != msgSender && operator != msgSender) { @@ -381,8 +379,6 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { revert EVC_InvalidAddress(); } - bytes19 addressPrefix = getAddressPrefixInternal(account); - // The bitMask defines which accounts the operator is authorized for. The bitMask is created from the account // number which is a number up to 2^8 in binary, or 256. 1 << (uint160(owner) ^ uint160(account)) transforms // that number in an 256-position binary array like 0...010...0, marking the account positionally in a uint256. @@ -495,7 +491,8 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { bytes calldata signature ) public payable virtual nonReentrantChecksAndControlCollateral { // cannot be called within the self-call of the permit function; can occur for nested calls. - // the permit function can be called only by the specified sender + // the permit function can be called only by the specified sender, unless address zero is specified in which + // case anyone can call it if (inPermitSelfCall() || (sender != address(0) && sender != msg.sender)) { revert EVC_NotAuthorized(); } @@ -771,7 +768,7 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { if (haveCommonOwnerInternal(account, msgSender)) { // if the owner is not registered, register it if (owner == address(0)) { - ownerLookup[addressPrefix].owner = msgSender; + ownerLookup[addressPrefix].owner = owner = msgSender; emit OwnerRegistered(addressPrefix, msgSender); authenticated = true; } else if (owner == msgSender) { @@ -784,6 +781,11 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { authenticated = true; } + // if the authenticated account is non-owner, prevent its account from being a smart contract + if (authenticated && owner != account && account.code.length != 0) { + authenticated = false; + } + // must revert if neither the owner nor the operator were authenticated if (!authenticated) { revert EVC_NotAuthorized(); @@ -798,8 +800,9 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { } /// @notice Authenticates the caller of a function. - /// @dev This function converts a bytes19 address prefix into a phantom account address which is an account address - /// that belongs to the owner of the address prefix. + /// @dev This function either passes the address prefix owner address, if the address prefix owner is already + /// registered, or converts the bytes19 address prefix into an account address which will belong to the owner when + /// it's finally registered. /// @param addressPrefix The bytes19 address prefix to authenticate the caller against. /// @param allowOperator A boolean indicating if operators are allowed to authenticate as the caller. /// @param checkLockdownMode A boolean indicating if the function should check for lockdown mode on the account. @@ -809,10 +812,10 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { bool allowOperator, bool checkLockdownMode ) internal virtual returns (address) { - address phantomAccount = address(uint160(uint152(addressPrefix)) << ACCOUNT_ID_OFFSET); + address owner = ownerLookup[addressPrefix].owner; return authenticateCaller({ - account: phantomAccount, + account: owner == address(0) ? address(uint160(uint152(addressPrefix)) << ACCOUNT_ID_OFFSET) : owner, allowOperator: allowOperator, checkLockdownMode: checkLockdownMode }); @@ -937,8 +940,9 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { else if (numOfControllers > 1) return (false, abi.encodeWithSelector(EVC_ControllerViolation.selector)); bool success; - (success, result) = - controller.call(abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get()))); + (success, result) = controller.staticcall( + abi.encodeCall(IVault.checkAccountStatus, (account, accountCollaterals[account].get())) + ); isValid = success && result.length == 32 && abi.decode(result, (bytes32)) == bytes32(IVault.checkAccountStatus.selector); @@ -1148,7 +1152,7 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { /// @notice Calculates the EIP-712 domain separator for the contract. /// @return The calculated EIP-712 domain separator as a bytes32 value. function calculateDomainSeparator() internal view returns (bytes32) { - return keccak256(abi.encode(TYPE_HASH, HASHED_NAME, HASHED_VERSION, block.chainid, address(this))); + return keccak256(abi.encode(TYPE_HASH, HASHED_NAME, block.chainid, address(this))); } // Auxiliary functions @@ -1187,14 +1191,6 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { return bytes19(uint152(uint160(account) >> ACCOUNT_ID_OFFSET)); } - /// @notice Retrieves the owner of a given account by its address prefix. - /// @param account The account address to retrieve the owner for. - /// @return The address of the account owner. - function getAccountOwnerInternal(address account) internal view returns (address) { - bytes19 addressPrefix = getAddressPrefixInternal(account); - return ownerLookup[addressPrefix].owner; - } - /// @notice Checks if an operator is authorized for a specific account. /// @dev Determines operator authorization by checking if the operator's bit is set in the operator's bit field for /// the account's address prefix. If the owner is not registered (address(0)), it implies the operator cannot be @@ -1207,13 +1203,12 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC { address account, address operator ) internal view returns (bool isAuthorized) { - address owner = getAccountOwnerInternal(account); + bytes19 addressPrefix = getAddressPrefixInternal(account); + address owner = ownerLookup[addressPrefix].owner; // if the owner is not registered yet, it means that the operator couldn't have been authorized if (owner == address(0)) return false; - bytes19 addressPrefix = getAddressPrefixInternal(account); - // The bitMask defines which accounts the operator is authorized for. The bitMask is created from the account // number which is a number up to 2^8 in binary, or 256. 1 << (uint160(owner) ^ uint160(account)) transforms // that number in an 256-position binary array like 0...010...0, marking the account positionally in a uint256. diff --git a/src/Set.sol b/src/Set.sol index b0426e95..a254b1ec 100644 --- a/src/Set.sol +++ b/src/Set.sol @@ -102,7 +102,9 @@ library Set { } /// @notice Removes an element and returns information whether the element was removed or not. - /// @dev This operation may affect the order of elements in the array of elements obtained using get() function. + /// @dev This operation may affect the order of elements in the array of elements obtained using get() function. This + /// function does not modify the metadata of the set, even if it becomes empty as a result of invoking this + /// function. /// @param setStorage The set storage from which the element will be removed. /// @param element The element to be removed. /// @return A boolean value that indicates whether the element was removed or not. If the element was not in the set @@ -245,7 +247,8 @@ library Set { /// @notice Iterates over each element in the set and applies the callback function to it. /// @dev The set is cleared as a result of this call. Considering that this function does not follow the - /// Checks-Effects-Interactions pattern, the function using it must prevent re-entrancy. + /// Checks-Effects-Interactions pattern, the function using it must prevent re-entrancy. This function does not + /// modify the metadata of the set. /// @param setStorage The set storage to be processed. /// @param callback The function to be applied to each element. function forEachAndClear(SetStorage storage setStorage, function(address) callback) internal { @@ -273,7 +276,8 @@ library Set { /// @notice Iterates over each element in the set and applies the callback function to it, returning the array of /// callback results. /// @dev The set is cleared as a result of this call. Considering that this function does not follow the - /// Checks-Effects-Interactions pattern, the function using it must prevent re-entrancy. + /// Checks-Effects-Interactions pattern, the function using it must prevent re-entrancy. This function does not + /// modify the metadata of the set. /// @param setStorage The set storage to be processed. /// @param callback The function to be applied to each element. /// @return result An array of encoded bytes that are the addresses passed to the callback function and results of diff --git a/src/TransientStorage.sol b/src/TransientStorage.sol index 7a6c06bc..1e1f90dd 100644 --- a/src/TransientStorage.sol +++ b/src/TransientStorage.sol @@ -10,7 +10,7 @@ import {Set, SetStorage} from "./Set.sol"; /// @author Euler Labs (https://www.eulerlabs.com/) /// @notice This contract provides transient storage for the Ethereum Vault Connector. /// @dev All the variables in this contract are considered transient meaning that their state does not change between -/// transactions. +/// invocations. abstract contract TransientStorage { using ExecutionContext for EC; using Set for SetStorage; diff --git a/src/interfaces/IVault.sol b/src/interfaces/IVault.sol index c668000e..c1392a63 100644 --- a/src/interfaces/IVault.sol +++ b/src/interfaces/IVault.sol @@ -22,7 +22,10 @@ interface IVault { /// @param collaterals The array of enabled collateral addresses to be considered for the account status check. /// @return magicValue Must return the bytes4 magic value 0xb168c58f (which is a selector of this function) when /// account status is valid, or revert otherwise. - function checkAccountStatus(address account, address[] calldata collaterals) external returns (bytes4 magicValue); + function checkAccountStatus( + address account, + address[] calldata collaterals + ) external view returns (bytes4 magicValue); /// @notice Checks the status of the vault. /// @dev This function must only deliberately revert if the vault status is invalid. If this function reverts due to diff --git a/test/unit/EthereumVaultConnector/Call.t.sol b/test/unit/EthereumVaultConnector/Call.t.sol index 3b33b2ef..48ec7d2b 100644 --- a/test/unit/EthereumVaultConnector/Call.t.sol +++ b/test/unit/EthereumVaultConnector/Call.t.sol @@ -216,6 +216,105 @@ contract CallTest is Test { } } + function test_RevertIfOwnerAndAccountAreSmartContracts_Call(address alice, uint8 id) public { + vm.assume(uint160(alice) > 255 && alice != address(evc)); + vm.assume(id != 0); + + address alicesSubAccount = address(uint160(alice) ^ id); + + // both alice and her sub-account are contracts + vm.etch(alice, "ff"); + vm.etch(alicesSubAccount, "ff"); + + address targetContract = address(new Target()); + vm.assume(targetContract != alice && targetContract != address(evc)); + + bytes memory data1 = abi.encodeWithSelector( + Target(targetContract).callTest.selector, address(evc), address(evc), 0, alicesSubAccount, false + ); + + bytes memory data2 = abi.encodeWithSelector( + Target(targetContract).callTest.selector, address(evc), address(evc), 0, alice, false + ); + + // authentication is unsuccessfull because both the caller and the account are smart contracts at the same time + vm.prank(alice); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.call(targetContract, alicesSubAccount, 0, data1); + + // authentication is unsuccessfull because both the caller and the account are smart contracts at the same time + vm.prank(alicesSubAccount); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.call(targetContract, alice, 0, data2); + + // alice successfully registers as an owner + vm.prank(alice); + evc.call(targetContract, alice, 0, data2); + assertEq(evc.getAccountOwner(alice), alice); + + // authentication is unsuccessfull because both the caller and the account are smart contracts at the same time + vm.prank(alice); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.call(targetContract, alicesSubAccount, 0, data1); + + // authentication is unsuccessfull because both the caller and the account are smart contracts at the same time + vm.prank(alicesSubAccount); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.call(targetContract, alice, 0, data2); + } + + function test_RevertIfSubAccountOfRegisteredOwnerIsSmartContract_Call( + address alice, + uint8 id, + address operator, + address targetContract, + bytes memory data + ) public { + vm.assume(uint160(alice) > 255 && alice != address(evc)); + vm.assume(targetContract != operator && !evc.haveCommonOwner(alice, operator)); + vm.assume( + uint160(targetContract) > 255 && targetContract != address(evc) + && !evc.haveCommonOwner(alice, targetContract) + ); + vm.assume(id != 0); + vm.assume(data.length != 0); + + address alicesSubAccount = address(uint160(alice) ^ id); + + // alice's sub-account is contract + vm.etch(alicesSubAccount, "ff"); + + // authentication is unsuccessfull because alice's sub-account is a smart contract + vm.prank(alice); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.setAccountOperator(alicesSubAccount, operator, true); + + // alice's sub-account is no longer a contract + vm.etch(alicesSubAccount, ""); + + // this time authentication is successful; register alice as an owner by registering an operator for her + // sub-account + vm.prank(alice); + evc.setAccountOperator(alicesSubAccount, operator, true); + + // authentication is successfull as long as alice's sub-account is not a smart contract + vm.prank(alice); + evc.call(targetContract, alicesSubAccount, 0, data); + vm.prank(operator); + evc.call(targetContract, alicesSubAccount, 0, data); + + // alice's sub-account is a contract once again + vm.etch(alicesSubAccount, "ff"); + + // authentication is unsuccessfull because alice's sub-account is a smart contract + vm.prank(alice); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.call(targetContract, alicesSubAccount, 0, data); + vm.prank(operator); + vm.expectRevert(Errors.EVC_NotAuthorized.selector); + evc.call(targetContract, alicesSubAccount, 0, data); + } + function test_RevertIfChecksReentrancy_Call(address alice, uint256 seed) public { vm.assume(alice != address(evc)); diff --git a/test/unit/EthereumVaultConnector/ControllersManagement.t.sol b/test/unit/EthereumVaultConnector/ControllersManagement.t.sol index 88b3c7b2..c8f8e271 100644 --- a/test/unit/EthereumVaultConnector/ControllersManagement.t.sol +++ b/test/unit/EthereumVaultConnector/ControllersManagement.t.sol @@ -50,6 +50,7 @@ contract ControllersManagementTest is Test { vm.assume(seed > 1000); address account = address(uint160(uint160(alice) ^ subAccountId)); + vm.assume(account.code.length == 0); // test controllers management with use of an operator address msgSender = alice; diff --git a/test/unit/EthereumVaultConnector/Permit.t.sol b/test/unit/EthereumVaultConnector/Permit.t.sol index d97022e4..a9ad8136 100644 --- a/test/unit/EthereumVaultConnector/Permit.t.sol +++ b/test/unit/EthereumVaultConnector/Permit.t.sol @@ -12,33 +12,26 @@ abstract contract EIP712 { using ShortStrings for *; bytes32 internal constant _TYPE_HASH = - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); bytes32 internal immutable _hashedName; - bytes32 internal immutable _hashedVersion; - ShortString private immutable _name; - ShortString private immutable _version; string private _nameFallback; - string private _versionFallback; /** * @dev Initializes the domain separator. * - * The meaning of `name` and `version` is specified in + * The meaning of `name` is specified in * https://eips.ethereum.org/EIPS/eip-712#definition-of-domainseparator[EIP 712]: * * - `name`: the user readable name of the signing domain, i.e. the name of the DApp or the protocol. - * - `version`: the current major version of the signing domain. * * NOTE: These parameters cannot be changed except through a xref:learn::upgrading-smart-contracts.adoc[smart * contract upgrade]. */ - constructor(string memory name, string memory version) { + constructor(string memory name) { _name = name.toShortStringWithFallback(_nameFallback); - _version = version.toShortStringWithFallback(_versionFallback); _hashedName = keccak256(bytes(name)); - _hashedVersion = keccak256(bytes(version)); } /** @@ -80,7 +73,7 @@ contract SignerECDSA is EIP712, Test { "Permit(address signer,address sender,uint256 nonceNamespace,uint256 nonce,uint256 deadline,uint256 value,bytes data)" ); - constructor(EthereumVaultConnector _evc) EIP712(_evc.name(), _evc.version()) { + constructor(EthereumVaultConnector _evc) EIP712(_evc.name()) { evc = _evc; } @@ -89,7 +82,7 @@ contract SignerECDSA is EIP712, Test { } function _buildDomainSeparator() internal view override returns (bytes32) { - return keccak256(abi.encode(_TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(evc))); + return keccak256(abi.encode(_TYPE_HASH, _hashedName, block.chainid, address(evc))); } function signPermit( @@ -118,12 +111,12 @@ contract SignerERC1271 is EIP712, IERC1271 { "Permit(address signer,address sender,uint256 nonceNamespace,uint256 nonce,uint256 deadline,uint256 value,bytes data)" ); - constructor(EthereumVaultConnector _evc) EIP712(_evc.name(), _evc.version()) { + constructor(EthereumVaultConnector _evc) EIP712(_evc.name()) { evc = _evc; } function _buildDomainSeparator() internal view override returns (bytes32) { - return keccak256(abi.encode(_TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(evc))); + return keccak256(abi.encode(_TYPE_HASH, _hashedName, block.chainid, address(evc))); } function setSignatureHash(bytes calldata signature) external { diff --git a/test/unit/EthereumVaultConnector/SetOperator.t.sol b/test/unit/EthereumVaultConnector/SetOperator.t.sol index ea0c1e9f..d32671ad 100644 --- a/test/unit/EthereumVaultConnector/SetOperator.t.sol +++ b/test/unit/EthereumVaultConnector/SetOperator.t.sol @@ -39,6 +39,8 @@ contract SetOperatorTest is Test { bool isAlreadyAuthorized = operatorBitField & (1 << i) != 0; assertEq(evc.isAccountOperatorAuthorized(account, operator), isAlreadyAuthorized); + if (account.code.length != 0) continue; + // authorize the operator if (!isAlreadyAuthorized) { vm.expectEmit(true, true, false, true, address(evc)); @@ -81,6 +83,8 @@ contract SetOperatorTest is Test { bytes19 addressPrefix = evc.getAddressPrefix(account); assertEq(evc.isAccountOperatorAuthorized(account, operator), false); + vm.assume(account.code.length == 0); + if (i == 0) { assertEq(evc.getAccountOwner(account), address(0)); } else { @@ -144,6 +148,8 @@ contract SetOperatorTest is Test { address account = address(uint160(uint160(alice) ^ i)); bool isAlreadyAuthorized = operatorBitField & (1 << i) != 0; + if (account.code.length != 0) continue; + // revert when trying to set the same operator status vm.prank(alice); vm.expectRevert(Errors.EVC_InvalidOperatorStatus.selector); diff --git a/test/utils/mocks/Vault.sol b/test/utils/mocks/Vault.sol index 199e8afe..97de25e2 100644 --- a/test/utils/mocks/Vault.sol +++ b/test/utils/mocks/Vault.sol @@ -91,7 +91,7 @@ contract Vault is IVault, Target { } } - function checkAccountStatus(address, address[] memory) external virtual override returns (bytes4 magicValue) { + function checkAccountStatus(address, address[] memory) external view virtual override returns (bytes4 magicValue) { try evc.getCurrentOnBehalfOfAccount(address(0)) { revert("cas/on-behalf-of-account"); } catch (bytes memory reason) { @@ -176,7 +176,7 @@ contract VaultMalicious is Vault { } (bool success, bytes memory result) = - address(evc).call(abi.encodeWithSelector(evc.batch.selector, new IEVC.BatchItem[](0))); + address(evc).staticcall(abi.encodeWithSelector(evc.getLastAccountStatusCheckTimestamp.selector, address(0))); if (success || bytes4(result) != expectedErrorSelector) { return this.checkVaultStatus.selector; @@ -185,7 +185,7 @@ contract VaultMalicious is Vault { revert("malicious vault"); } - function checkAccountStatus(address, address[] memory) external override returns (bytes4) { + function checkAccountStatus(address, address[] memory) external view override returns (bytes4) { try evc.getCurrentOnBehalfOfAccount(address(0)) { revert("cas/on-behalf-of-account"); } catch (bytes memory reason) { @@ -207,7 +207,7 @@ contract VaultMalicious is Vault { } (bool success, bytes memory result) = - address(evc).call(abi.encodeWithSelector(evc.batch.selector, new IEVC.BatchItem[](0))); + address(evc).staticcall(abi.encodeWithSelector(evc.getLastAccountStatusCheckTimestamp.selector, address(0))); if (success || bytes4(result) != expectedErrorSelector) { return this.checkAccountStatus.selector;