diff --git a/contracts/extensions/account/Account.sol b/contracts/extensions/account/Account.sol index 2d6d908a..ba07feab 100644 --- a/contracts/extensions/account/Account.sol +++ b/contracts/extensions/account/Account.sol @@ -17,8 +17,26 @@ import {ERC721Holder} from "@openzeppelin/contracts/token/ERC721/utils/ERC721Hol import {ERC1155Receiver} from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Receiver.sol"; import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol"; +library PermissionsHelper { + function equals(AccountManagerPermissions memory permissions, AccountManagerPermissions memory otherPermissions) + internal + pure + returns (bool) + { + return permissions.canExecuteTransactions == otherPermissions.canExecuteTransactions + && permissions.canTransferTokens == otherPermissions.canTransferTokens + && permissions.canTransferNative == otherPermissions.canTransferNative + && permissions.canSetMetadataURI == otherPermissions.canSetMetadataURI; + } + + function isAccountManager(AccountManagerPermissions memory managerPermissions) internal pure returns (bool) { + return managerPermissions.canExecuteTransactions || managerPermissions.canSetMetadataURI; + } +} + contract Account is IAccount, Initializable, Ownable, ExtraStorageBased, MetadataBased, ERC1155Holder, ERC721Holder { using CallLib for address; + using PermissionsHelper for AccountManagerPermissions; // In a future Account version/upgrade this could be configurable by the owner. uint256 constant SPENDING_TIMELOCK = 1 hours; @@ -103,14 +121,25 @@ contract Account is IAccount, Initializable, Ownable, ExtraStorageBased, Metadat emit Lens_Account_AllowNonOwnerSpending(allow, allow ? block.timestamp : 0); } + function _isAccountManager(address accountManager) internal view returns (bool) { + return $storage().accountManagerPermissions[accountManager].isAccountManager(); + } + + function _validateAccountManagerPermissions(AccountManagerPermissions calldata permissions) internal pure { + if (permissions.canTransferNative || permissions.canTransferTokens) { + require(permissions.canExecuteTransactions, Errors.InvalidParameter()); + } else { + require(permissions.isAccountManager(), Errors.InvalidParameter()); + } + } + function addAccountManager(address accountManager, AccountManagerPermissions calldata accountManagerPermissions) external override onlyOwner { - require( - !$storage().accountManagerPermissions[accountManager].canExecuteTransactions, Errors.RedundantStateChange() - ); + require(!_isAccountManager(accountManager), Errors.RedundantStateChange()); + _validateAccountManagerPermissions(accountManagerPermissions); require(accountManager != owner(), Errors.InvalidParameter()); require(accountManager != address(0), Errors.InvalidParameter()); $storage().accountManagerPermissions[accountManager] = accountManagerPermissions; @@ -118,9 +147,7 @@ contract Account is IAccount, Initializable, Ownable, ExtraStorageBased, Metadat } function removeAccountManager(address accountManager) external override onlyOwner { - require( - $storage().accountManagerPermissions[accountManager].canExecuteTransactions, Errors.RedundantStateChange() - ); + require(_isAccountManager(accountManager), Errors.RedundantStateChange()); delete $storage().accountManagerPermissions[accountManager]; emit Lens_Account_AccountManagerRemoved(accountManager); } @@ -129,8 +156,12 @@ contract Account is IAccount, Initializable, Ownable, ExtraStorageBased, Metadat address accountManager, AccountManagerPermissions calldata accountManagerPermissions ) external override onlyOwner { - require($storage().accountManagerPermissions[accountManager].canExecuteTransactions, Errors.InvalidParameter()); - require(accountManagerPermissions.canExecuteTransactions, Errors.InvalidParameter()); + require(_isAccountManager(accountManager), Errors.InvalidParameter()); + _validateAccountManagerPermissions(accountManagerPermissions); + require( + !$storage().accountManagerPermissions[accountManager].equals(accountManagerPermissions), + Errors.RedundantStateChange() + ); $storage().accountManagerPermissions[accountManager] = accountManagerPermissions; emit Lens_Account_AccountManagerUpdated(accountManager, accountManagerPermissions); } @@ -202,6 +233,14 @@ contract Account is IAccount, Initializable, Ownable, ExtraStorageBased, Metadat return $storage().accountManagerPermissions[executor].canExecuteTransactions || executor == owner(); } + function isAccountManager(address accountManager) external view override returns (bool) { + return _isAccountManager(accountManager); + } + + function canSetMetadataURI(address accountManager) external view override returns (bool) { + return $storage().accountManagerPermissions[accountManager].canSetMetadataURI; + } + function getAccountManagerPermissions(address accountManager) external view diff --git a/contracts/extensions/account/IAccount.sol b/contracts/extensions/account/IAccount.sol index 280c8813..75493f1b 100644 --- a/contracts/extensions/account/IAccount.sol +++ b/contracts/extensions/account/IAccount.sol @@ -54,6 +54,12 @@ interface IAccount is IMetadataBased, IERC1155Receiver, IERC721Receiver { function executeTransactions(Transaction[] calldata transactions) external payable returns (bytes[] memory); + function isAccountManager(address accountManager) external view returns (bool); + + function canExecuteTransactions(address executor) external view returns (bool); + + function canSetMetadataURI(address accountManager) external view returns (bool); + function getAccountManagerPermissions(address accountManager) external view @@ -61,7 +67,5 @@ interface IAccount is IMetadataBased, IERC1155Receiver, IERC721Receiver { function getExtraData(bytes32 key) external view returns (bytes memory); - function canExecuteTransactions(address executor) external view returns (bool); - receive() external payable; } diff --git a/test/Account.t.sol b/test/Account.t.sol index cd7d2b1b..d2d890dd 100644 --- a/test/Account.t.sol +++ b/test/Account.t.sol @@ -162,6 +162,9 @@ contract AccountTest is Test, BaseDeployments { vm.assume(accountManager != address(0)); vm.assume(accountManager != owner); vm.assume(accountManager != manager); + vm.assume(canTransferTokensBefore != canTransferTokensAfter); + vm.assume(canTransferNativeBefore != canTransferNativeAfter); + vm.assume(canSetMetadataURIBefore != canSetMetadataURIAfter); vm.prank(owner); account.addAccountManager(