diff --git a/contracts/extension/interface/IAccountPermissions.sol b/contracts/extension/interface/IAccountPermissions.sol index fd1f33db3..6c0a670db 100644 --- a/contracts/extension/interface/IAccountPermissions.sol +++ b/contracts/extension/interface/IAccountPermissions.sol @@ -19,9 +19,11 @@ interface IAccountPermissions { * @param reqValidityStartTimestamp The UNIX timestamp at and after which a signature is valid. * @param reqValidityEndTimestamp The UNIX timestamp at and after which a signature is invalid/expired. * @param uid A unique non-repeatable ID for the payload. + * @param isAdmin Whether the signer should be an admin. */ struct SignerPermissionRequest { address signer; + uint8 isAdmin; address[] approvedTargets; uint256 nativeTokenLimitPerTransaction; uint128 permissionStartTimestamp; @@ -107,9 +109,6 @@ interface IAccountPermissions { External functions //////////////////////////////////////////////////////////////*/ - /// @notice Adds / removes an account as an admin. - function setAdmin(address account, bool isAdmin) external; - /// @notice Sets the permissions for a given signer. function setPermissionsForSigner(SignerPermissionRequest calldata req, bytes calldata signature) external; } diff --git a/contracts/extension/upgradeable/AccountPermissions.sol b/contracts/extension/upgradeable/AccountPermissions.sol index 547970442..bc9d9918a 100644 --- a/contracts/extension/upgradeable/AccountPermissions.sol +++ b/contracts/extension/upgradeable/AccountPermissions.sol @@ -42,39 +42,45 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 { bytes32 private constant TYPEHASH = keccak256( - "SignerPermissionRequest(address signer,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" + "SignerPermissionRequest(address signer,uint8 isAdmin,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" ); - modifier onlyAdmin() virtual { - require(isAdmin(msg.sender), "caller is not an admin"); - _; + function _onlyAdmin() internal virtual { + require(isAdmin(msg.sender), "!admin"); } /*/////////////////////////////////////////////////////////////// External functions //////////////////////////////////////////////////////////////*/ - /// @notice Adds / removes an account as an admin. - function setAdmin(address _account, bool _isAdmin) external virtual onlyAdmin { - _setAdmin(_account, _isAdmin); - } - /// @notice Sets the permissions for a given signer. function setPermissionsForSigner(SignerPermissionRequest calldata _req, bytes calldata _signature) external { address targetSigner = _req.signer; - require(!isAdmin(targetSigner), "signer is already an admin"); require( _req.reqValidityStartTimestamp <= block.timestamp && block.timestamp < _req.reqValidityEndTimestamp, - "invalid request validity period" + "!period" ); (bool success, address signer) = verifySignerPermissionRequest(_req, _signature); - require(success, "invalid signature"); + require(success, "!sig"); - _accountPermissionsStorage().allSigners.add(targetSigner); _accountPermissionsStorage().executed[_req.uid] = true; + //isAdmin > 0, set admin or remove admin + if (_req.isAdmin > 0) { + //isAdmin = 1, set admin + //isAdmin > 1, remove admin + bool _isAdmin = _req.isAdmin == 1; + + _setAdmin(targetSigner, _isAdmin); + return; + } + + require(!isAdmin(targetSigner), "admin"); + + _accountPermissionsStorage().allSigners.add(targetSigner); + _accountPermissionsStorage().signerPermissions[targetSigner] = SignerPermissionsStatic( _req.nativeTokenLimitPerTransaction, _req.permissionStartTimestamp, @@ -82,13 +88,13 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 { ); address[] memory currentTargets = _accountPermissionsStorage().approvedTargets[targetSigner].values(); - uint256 currentLen = currentTargets.length; + uint256 len = currentTargets.length; - for (uint256 i = 0; i < currentLen; i += 1) { + for (uint256 i = 0; i < len; i += 1) { _accountPermissionsStorage().approvedTargets[targetSigner].remove(currentTargets[i]); } - uint256 len = _req.approvedTargets.length; + len = _req.approvedTargets.length; for (uint256 i = 0; i < len; i += 1) { _accountPermissionsStorage().approvedTargets[targetSigner].add(_req.approvedTargets[i]); } @@ -138,7 +144,7 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 { virtual returns (bool success, address signer) { - signer = _recoverAddress(req, signature); + signer = _recoverAddress(_encodeRequest(req), signature); success = !_accountPermissionsStorage().executed[req.uid] && isAdmin(signer); } @@ -168,34 +174,30 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 { uint256 len = allSigners.length; uint256 numOfActiveSigners = 0; - bool[] memory isSignerActive = new bool[](len); for (uint256 i = 0; i < len; i += 1) { - address signer = allSigners[i]; - - bool isActive = isActiveSigner(signer); - isSignerActive[i] = isActive; - if (isActive) { + if (isActiveSigner(allSigners[i])) { numOfActiveSigners++; + } else { + allSigners[i] = address(0); } } signers = new SignerPermissions[](numOfActiveSigners); uint256 index = 0; for (uint256 i = 0; i < len; i += 1) { - if (!isSignerActive[i]) { - continue; + if (allSigners[i] != address(0)) { + address signer = allSigners[i]; + SignerPermissionsStatic memory permissions = _accountPermissionsStorage().signerPermissions[signer]; + + signers[index++] = SignerPermissions( + signer, + _accountPermissionsStorage().approvedTargets[signer].values(), + permissions.nativeTokenLimitPerTransaction, + permissions.startTimestamp, + permissions.endTimestamp + ); } - address signer = allSigners[i]; - SignerPermissionsStatic memory permissions = _accountPermissionsStorage().signerPermissions[signer]; - - signers[index++] = SignerPermissions( - signer, - _accountPermissionsStorage().approvedTargets[signer].values(), - permissions.nativeTokenLimitPerTransaction, - permissions.startTimestamp, - permissions.endTimestamp - ); } } @@ -225,13 +227,8 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 { } /// @dev Returns the address of the signer of the request. - function _recoverAddress(SignerPermissionRequest calldata _req, bytes calldata _signature) - internal - view - virtual - returns (address) - { - return _hashTypedDataV4(keccak256(_encodeRequest(_req))).recover(_signature); + function _recoverAddress(bytes memory _encoded, bytes calldata _signature) internal view virtual returns (address) { + return _hashTypedDataV4(keccak256(_encoded)).recover(_signature); } /// @dev Encodes a request for recovery of the signer in `recoverAddress`. @@ -240,6 +237,7 @@ abstract contract AccountPermissions is IAccountPermissions, EIP712 { abi.encode( TYPEHASH, _req.signer, + _req.isAdmin, keccak256(abi.encodePacked(_req.approvedTargets)), _req.nativeTokenLimitPerTransaction, _req.permissionStartTimestamp, diff --git a/contracts/legacy-contracts/extension/BatchMintMetadata_V1.sol b/contracts/legacy-contracts/extension/BatchMintMetadata_V1.sol new file mode 100644 index 000000000..7158488dd --- /dev/null +++ b/contracts/legacy-contracts/extension/BatchMintMetadata_V1.sol @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +/// @author thirdweb + +/** + * @title Batch-mint Metadata + * @notice The `BatchMintMetadata` is a contract extension for any base NFT contract. It lets the smart contract + * using this extension set metadata for `n` number of NFTs all at once. This is enabled by storing a single + * base URI for a batch of `n` NFTs, where the metadata for each NFT in a relevant batch is `baseURI/tokenId`. + */ + +contract BatchMintMetadata_V1 { + /// @dev Largest tokenId of each batch of tokens with the same baseURI. + uint256[] private batchIds; + + /// @dev Mapping from id of a batch of tokens => to base URI for the respective batch of tokens. + mapping(uint256 => string) private baseURI; + + /** + * @notice Returns the count of batches of NFTs. + * @dev Each batch of tokens has an in ID and an associated `baseURI`. + * See {batchIds}. + */ + function getBaseURICount() public view returns (uint256) { + return batchIds.length; + } + + /** + * @notice Returns the ID for the batch of tokens at the given index. + * @dev See {getBaseURICount}. + * @param _index Index of the desired batch in batchIds array. + */ + function getBatchIdAtIndex(uint256 _index) public view returns (uint256) { + if (_index >= getBaseURICount()) { + revert("Invalid index"); + } + return batchIds[_index]; + } + + /// @dev Returns the id for the batch of tokens the given tokenId belongs to. + function _getBatchId(uint256 _tokenId) internal view returns (uint256 batchId, uint256 index) { + uint256 numOfTokenBatches = getBaseURICount(); + uint256[] memory indices = batchIds; + + for (uint256 i = 0; i < numOfTokenBatches; i += 1) { + if (_tokenId < indices[i]) { + index = i; + batchId = indices[i]; + + return (batchId, index); + } + } + + revert("Invalid tokenId"); + } + + /// @dev Returns the baseURI for a token. The intended metadata URI for the token is baseURI + tokenId. + function _getBaseURI(uint256 _tokenId) internal view returns (string memory) { + uint256 numOfTokenBatches = getBaseURICount(); + uint256[] memory indices = batchIds; + + for (uint256 i = 0; i < numOfTokenBatches; i += 1) { + if (_tokenId < indices[i]) { + return baseURI[indices[i]]; + } + } + revert("Invalid tokenId"); + } + + /// @dev Sets the base URI for the batch of tokens with the given batchId. + function _setBaseURI(uint256 _batchId, string memory _baseURI) internal { + baseURI[_batchId] = _baseURI; + } + + /// @dev Mints a batch of tokenIds and associates a common baseURI to all those Ids. + function _batchMintMetadata( + uint256 _startId, + uint256 _amountToMint, + string memory _baseURIForTokens + ) internal returns (uint256 nextTokenIdToMint, uint256 batchId) { + batchId = _startId + _amountToMint; + nextTokenIdToMint = batchId; + + batchIds.push(batchId); + + baseURI[batchId] = _baseURIForTokens; + } +} diff --git a/contracts/legacy-contracts/extension/LazyMintWithTier_V1.sol b/contracts/legacy-contracts/extension/LazyMintWithTier_V1.sol new file mode 100644 index 000000000..141f6f39d --- /dev/null +++ b/contracts/legacy-contracts/extension/LazyMintWithTier_V1.sol @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +/// @author thirdweb + +import "../../extension/interface/ILazyMintWithTier.sol"; +import "./BatchMintMetadata_V1.sol"; + +/** + * The `LazyMint` is a contract extension for any base NFT contract. It lets you 'lazy mint' any number of NFTs + * at once. Here, 'lazy mint' means defining the metadata for particular tokenIds of your NFT contract, without actually + * minting a non-zero balance of NFTs of those tokenIds. + */ + +abstract contract LazyMintWithTier_V1 is ILazyMintWithTier, BatchMintMetadata_V1 { + struct TokenRange { + uint256 startIdInclusive; + uint256 endIdNonInclusive; + } + + struct TierMetadata { + string tier; + TokenRange[] ranges; + string[] baseURIs; + } + + /// @notice The tokenId assigned to the next new NFT to be lazy minted. + uint256 internal nextTokenIdToLazyMint; + + /// @notice Mapping from a tier -> the token IDs grouped under that tier. + mapping(string => TokenRange[]) internal tokensInTier; + + /// @notice A list of tiers used in this contract. + string[] private tiers; + + /** + * @notice Lets an authorized address lazy mint a given amount of NFTs. + * + * @param _amount The number of NFTs to lazy mint. + * @param _baseURIForTokens The base URI for the 'n' number of NFTs being lazy minted, where the metadata for each + * of those NFTs is `${baseURIForTokens}/${tokenId}`. + * @param _data Additional bytes data to be used at the discretion of the consumer of the contract. + * @return batchId A unique integer identifier for the batch of NFTs lazy minted together. + */ + function lazyMint( + uint256 _amount, + string calldata _baseURIForTokens, + string calldata _tier, + bytes calldata _data + ) public virtual override returns (uint256 batchId) { + if (!_canLazyMint()) { + revert("Not authorized"); + } + + if (_amount == 0) { + revert("0 amt"); + } + + uint256 startId = nextTokenIdToLazyMint; + + (nextTokenIdToLazyMint, batchId) = _batchMintMetadata(startId, _amount, _baseURIForTokens); + + // Handle tier info. + if (!(tokensInTier[_tier].length > 0)) { + tiers.push(_tier); + } + tokensInTier[_tier].push(TokenRange(startId, batchId)); + + emit TokensLazyMinted(_tier, startId, startId + _amount - 1, _baseURIForTokens, _data); + + return batchId; + } + + /// @notice Returns all metadata lazy minted for the given tier. + function _getMetadataInTier(string memory _tier) + private + view + returns (TokenRange[] memory tokens, string[] memory baseURIs) + { + tokens = tokensInTier[_tier]; + + uint256 len = tokens.length; + baseURIs = new string[](len); + + for (uint256 i = 0; i < len; i += 1) { + baseURIs[i] = _getBaseURI(tokens[i].startIdInclusive); + } + } + + /// @notice Returns all metadata for all tiers created on the contract. + function getMetadataForAllTiers() external view returns (TierMetadata[] memory metadataForAllTiers) { + string[] memory allTiers = tiers; + uint256 len = allTiers.length; + + metadataForAllTiers = new TierMetadata[](len); + + for (uint256 i = 0; i < len; i += 1) { + (TokenRange[] memory tokens, string[] memory baseURIs) = _getMetadataInTier(allTiers[i]); + metadataForAllTiers[i] = TierMetadata(allTiers[i], tokens, baseURIs); + } + } + + /** + * @notice Returns whether any metadata is lazy minted for the given tier. + * + * @param _tier We check whether this given tier is empty. + */ + function isTierEmpty(string memory _tier) internal view returns (bool) { + return tokensInTier[_tier].length == 0; + } + + /// @dev Returns whether lazy minting can be performed in the given execution context. + function _canLazyMint() internal view virtual returns (bool); +} diff --git a/contracts/legacy-contracts/extension/LazyMint_V1.sol b/contracts/legacy-contracts/extension/LazyMint_V1.sol new file mode 100644 index 000000000..820ea4eed --- /dev/null +++ b/contracts/legacy-contracts/extension/LazyMint_V1.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +/// @author thirdweb + +import "../../extension/interface/ILazyMint.sol"; +import "./BatchMintMetadata_V1.sol"; + +/** + * The `LazyMint` is a contract extension for any base NFT contract. It lets you 'lazy mint' any number of NFTs + * at once. Here, 'lazy mint' means defining the metadata for particular tokenIds of your NFT contract, without actually + * minting a non-zero balance of NFTs of those tokenIds. + */ + +abstract contract LazyMint_V1 is ILazyMint, BatchMintMetadata_V1 { + /// @notice The tokenId assigned to the next new NFT to be lazy minted. + uint256 internal nextTokenIdToLazyMint; + + /** + * @notice Lets an authorized address lazy mint a given amount of NFTs. + * + * @param _amount The number of NFTs to lazy mint. + * @param _baseURIForTokens The base URI for the 'n' number of NFTs being lazy minted, where the metadata for each + * of those NFTs is `${baseURIForTokens}/${tokenId}`. + * @param _data Additional bytes data to be used at the discretion of the consumer of the contract. + * @return batchId A unique integer identifier for the batch of NFTs lazy minted together. + */ + function lazyMint( + uint256 _amount, + string calldata _baseURIForTokens, + bytes calldata _data + ) public virtual override returns (uint256 batchId) { + if (!_canLazyMint()) { + revert("Not authorized"); + } + + if (_amount == 0) { + revert("0 amt"); + } + + uint256 startId = nextTokenIdToLazyMint; + + (nextTokenIdToLazyMint, batchId) = _batchMintMetadata(startId, _amount, _baseURIForTokens); + + emit TokensLazyMinted(startId, startId + _amount - 1, _baseURIForTokens, _data); + + return batchId; + } + + /// @dev Returns whether lazy minting can be performed in the given execution context. + function _canLazyMint() internal view virtual returns (bool); +} diff --git a/contracts/legacy-contracts/pre-builts/SignatureDrop_V4.sol b/contracts/legacy-contracts/pre-builts/SignatureDrop_V4.sol index 30272c2be..7bd07026a 100644 --- a/contracts/legacy-contracts/pre-builts/SignatureDrop_V4.sol +++ b/contracts/legacy-contracts/pre-builts/SignatureDrop_V4.sol @@ -24,7 +24,7 @@ import "../../extension/Royalty.sol"; import "../../extension/PrimarySale.sol"; import "../../extension/Ownable.sol"; import "../../extension/DelayedReveal.sol"; -import "../../extension/LazyMint.sol"; +import "../extension/LazyMint_V1.sol"; import "../../extension/PermissionsEnumerable.sol"; import "../extension/DropSinglePhase_V1.sol"; import "../../extension/SignatureMintERC721Upgradeable.sol"; @@ -37,7 +37,7 @@ contract SignatureDrop_V4 is PrimarySale, Ownable, DelayedReveal, - LazyMint, + LazyMint_V1, PermissionsEnumerable, DropSinglePhase_V1, SignatureMintERC721Upgradeable, diff --git a/contracts/legacy-contracts/smart-wallet/interface/IAccountPermissions_V1.sol b/contracts/legacy-contracts/smart-wallet/interface/IAccountPermissions_V1.sol new file mode 100644 index 000000000..635d382b5 --- /dev/null +++ b/contracts/legacy-contracts/smart-wallet/interface/IAccountPermissions_V1.sol @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +/// @author thirdweb + +interface IAccountPermissions_V1 { + /*/////////////////////////////////////////////////////////////// + Types + //////////////////////////////////////////////////////////////*/ + + /** + * @notice The payload that must be signed by an authorized wallet to set permissions for a signer to use the smart wallet. + * + * @param signer The addres of the signer to give permissions. + * @param approvedTargets The list of approved targets that a role holder can call using the smart wallet. + * @param nativeTokenLimitPerTransaction The maximum value that can be transferred by a role holder in a single transaction. + * @param permissionStartTimestamp The UNIX timestamp at and after which a signer has permission to use the smart wallet. + * @param permissionEndTimestamp The UNIX timestamp at and after which a signer no longer has permission to use the smart wallet. + * @param reqValidityStartTimestamp The UNIX timestamp at and after which a signature is valid. + * @param reqValidityEndTimestamp The UNIX timestamp at and after which a signature is invalid/expired. + * @param uid A unique non-repeatable ID for the payload. + */ + struct SignerPermissionRequest { + address signer; + address[] approvedTargets; + uint256 nativeTokenLimitPerTransaction; + uint128 permissionStartTimestamp; + uint128 permissionEndTimestamp; + uint128 reqValidityStartTimestamp; + uint128 reqValidityEndTimestamp; + bytes32 uid; + } + + /** + * @notice The permissions that a signer has to use the smart wallet. + * + * @param signer The address of the signer. + * @param approvedTargets The list of approved targets that a role holder can call using the smart wallet. + * @param nativeTokenLimitPerTransaction The maximum value that can be transferred by a role holder in a single transaction. + * @param startTimestamp The UNIX timestamp at and after which a signer has permission to use the smart wallet. + * @param endTimestamp The UNIX timestamp at and after which a signer no longer has permission to use the smart wallet. + */ + struct SignerPermissions { + address signer; + address[] approvedTargets; + uint256 nativeTokenLimitPerTransaction; + uint128 startTimestamp; + uint128 endTimestamp; + } + + /** + * @notice Internal struct for storing permissions for a signer (without approved targets). + * + * @param nativeTokenLimitPerTransaction The maximum value that can be transferred by a role holder in a single transaction. + * @param startTimestamp The UNIX timestamp at and after which a signer has permission to use the smart wallet. + * @param endTimestamp The UNIX timestamp at and after which a signer no longer has permission to use the smart wallet. + */ + struct SignerPermissionsStatic { + uint256 nativeTokenLimitPerTransaction; + uint128 startTimestamp; + uint128 endTimestamp; + } + + /*/////////////////////////////////////////////////////////////// + Events + //////////////////////////////////////////////////////////////*/ + + /// @notice Emitted when permissions for a signer are updated. + event SignerPermissionsUpdated( + address indexed authorizingSigner, + address indexed targetSigner, + SignerPermissionRequest permissions + ); + + /// @notice Emitted when an admin is set or removed. + event AdminUpdated(address indexed signer, bool isAdmin); + + /*/////////////////////////////////////////////////////////////// + View functions + //////////////////////////////////////////////////////////////*/ + + /// @notice Returns whether the given account is an admin. + function isAdmin(address signer) external view returns (bool); + + /// @notice Returns whether the given account is an active signer on the account. + function isActiveSigner(address signer) external view returns (bool); + + /// @notice Returns the restrictions under which a signer can use the smart wallet. + function getPermissionsForSigner(address signer) external view returns (SignerPermissions memory permissions); + + /// @notice Returns all active and inactive signers of the account. + function getAllSigners() external view returns (SignerPermissions[] memory signers); + + /// @notice Returns all signers with active permissions to use the account. + function getAllActiveSigners() external view returns (SignerPermissions[] memory signers); + + /// @notice Returns all admins of the account. + function getAllAdmins() external view returns (address[] memory admins); + + /// @dev Verifies that a request is signed by an authorized account. + function verifySignerPermissionRequest(SignerPermissionRequest calldata req, bytes calldata signature) + external + view + returns (bool success, address signer); + + /*/////////////////////////////////////////////////////////////// + External functions + //////////////////////////////////////////////////////////////*/ + + /// @notice Adds / removes an account as an admin. + function setAdmin(address account, bool isAdmin) external; + + /// @notice Sets the permissions for a given signer. + function setPermissionsForSigner(SignerPermissionRequest calldata req, bytes calldata signature) external; +} diff --git a/contracts/package.json b/contracts/package.json index 644c6304f..c8ae602aa 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,7 +1,7 @@ { "name": "@thirdweb-dev/contracts", "description": "Collection of smart contracts deployable via the thirdweb SDK, dashboard and CLI", - "version": "3.10.1", + "version": "3.10.3-1", "license": "Apache-2.0", "repository": { "type": "git", @@ -22,5 +22,8 @@ "@openzeppelin/contracts-upgradeable": "4.7.3", "erc721a-upgradeable": "^3.3.0", "@thirdweb-dev/dynamic-contracts": "^1.1.2" + }, + "engines": { + "node": ">=18.0.0" } } diff --git a/contracts/prebuilts/account/non-upgradeable/Account.sol b/contracts/prebuilts/account/non-upgradeable/Account.sol index 2152b85b4..09938c401 100644 --- a/contracts/prebuilts/account/non-upgradeable/Account.sol +++ b/contracts/prebuilts/account/non-upgradeable/Account.sol @@ -75,8 +75,10 @@ contract Account is AccountCore, ContractMetadata, ERC1271, ERC721Holder, ERC115 } address caller = msg.sender; + EnumerableSet.AddressSet storage approvedTargets = _accountPermissionsStorage().approvedTargets[signer]; + require( - _accountPermissionsStorage().approvedTargets[signer].contains(caller), + approvedTargets.contains(caller) || (approvedTargets.length() == 1 && approvedTargets.at(0) == address(0)), "Account: caller not approved target." ); diff --git a/contracts/prebuilts/account/utils/AccountCore.sol b/contracts/prebuilts/account/utils/AccountCore.sol index 2285f67a2..c59cf8cac 100644 --- a/contracts/prebuilts/account/utils/AccountCore.sol +++ b/contracts/prebuilts/account/utils/AccountCore.sol @@ -76,6 +76,7 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc } /// @notice Returns whether a signer is authorized to perform transactions using the wallet. + /* solhint-disable*/ function isValidSigner(address _signer, UserOperation calldata _userOp) public view virtual returns (bool) { // First, check if the signer is an admin. if (_accountPermissionsStorage().isAdmin[_signer]) { @@ -83,12 +84,13 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc } SignerPermissionsStatic memory permissions = _accountPermissionsStorage().signerPermissions[_signer]; + EnumerableSet.AddressSet storage approvedTargets = _accountPermissionsStorage().approvedTargets[_signer]; // If not an admin, check if the signer is active. if ( permissions.startTimestamp > block.timestamp || block.timestamp >= permissions.endTimestamp || - _accountPermissionsStorage().approvedTargets[_signer].length() == 0 + approvedTargets.length() == 0 ) { // Account: no active permissions. return false; @@ -97,15 +99,24 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc // Extract the function signature from the userOp calldata and check whether the signer is attempting to call `execute` or `executeBatch`. bytes4 sig = getFunctionSignature(_userOp.callData); + // if address(0) is the only approved target, set isWildCard to true (wildcard approved). + bool isWildCard = approvedTargets.length() == 1 && approvedTargets.at(0) == address(0); + if (sig == AccountExtension.execute.selector) { // Extract the `target` and `value` arguments from the calldata for `execute`. (address target, uint256 value) = decodeExecuteCalldata(_userOp.callData); + // if wildcard target is not approved, check that the target is in the approvedTargets set. + if (!isWildCard) { + // Check if the target is approved. + if (!approvedTargets.contains(target)) { + // Account: target not approved. + return false; + } + } + // Check if the value is within the allowed range and if the target is approved. - if ( - permissions.nativeTokenLimitPerTransaction < value || - !_accountPermissionsStorage().approvedTargets[_signer].contains(target) - ) { + if (permissions.nativeTokenLimitPerTransaction < value) { // Account: value too high OR Account: target not approved. return false; } @@ -113,12 +124,19 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc // Extract the `target` and `value` array arguments from the calldata for `executeBatch`. (address[] memory targets, uint256[] memory values, ) = decodeExecuteBatchCalldata(_userOp.callData); + // if wildcard target is not approved, check that the targets are in the approvedTargets set. + if (!isWildCard) { + for (uint256 i = 0; i < targets.length; i++) { + if (!approvedTargets.contains(targets[i])) { + // If any target is not approved, break the loop. + return false; + } + } + } + // For each target+value pair, check if the value is within the allowed range and if the target is approved. for (uint256 i = 0; i < targets.length; i++) { - if ( - permissions.nativeTokenLimitPerTransaction < values[i] || - !_accountPermissionsStorage().approvedTargets[_signer].contains(targets[i]) - ) { + if (permissions.nativeTokenLimitPerTransaction < values[i]) { // Account: value too high OR Account: target not approved. return false; } @@ -131,6 +149,8 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc return true; } + /* solhint-enable */ + /*/////////////////////////////////////////////////////////////// External functions //////////////////////////////////////////////////////////////*/ @@ -141,12 +161,14 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc } /// @notice Withdraw funds for this account from Entrypoint. - function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public onlyAdmin { + function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public { + _onlyAdmin(); entryPoint().withdrawTo(withdrawAddress, amount); } /// @notice Overrides the Entrypoint contract being used. - function setEntrypointOverride(IEntryPoint _entrypointOverride) public virtual onlyAdmin { + function setEntrypointOverride(IEntryPoint _entrypointOverride) public virtual { + _onlyAdmin(); AccountCoreStorage.data().entrypointOverride = address(_entrypointOverride); } @@ -155,12 +177,12 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc //////////////////////////////////////////////////////////////*/ function getFunctionSignature(bytes calldata data) internal pure returns (bytes4 functionSelector) { - require(data.length >= 4, "Data too short"); + require(data.length >= 4, "!Data"); return bytes4(data[:4]); } function decodeExecuteCalldata(bytes calldata data) internal pure returns (address _target, uint256 _value) { - require(data.length >= 4 + 32 + 32, "Data too short"); + require(data.length >= 4 + 32 + 32, "!Data"); // Decode the address, which is bytes 4 to 35 _target = abi.decode(data[4:36], (address)); @@ -178,7 +200,7 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc bytes[] memory _callData ) { - require(data.length >= 4 + 32 + 32 + 32, "Data too short"); + require(data.length >= 4 + 32 + 32 + 32, "!Data"); (_targets, _values, _callData) = abi.decode(data[4:], (address[], uint256[], bytes[])); } @@ -195,8 +217,10 @@ contract AccountCore is IAccountCore, Initializable, Multicall, BaseAccount, Acc if (!isValidSigner(signer, userOp)) return SIG_VALIDATION_FAILED; - uint48 validAfter = uint48(_accountPermissionsStorage().signerPermissions[signer].startTimestamp); - uint48 validUntil = uint48(_accountPermissionsStorage().signerPermissions[signer].endTimestamp); + SignerPermissionsStatic memory permissions = _accountPermissionsStorage().signerPermissions[signer]; + + uint48 validAfter = uint48(permissions.startTimestamp); + uint48 validUntil = uint48(permissions.endTimestamp); return _packValidationData(ValidationData(address(0), validAfter, validUntil)); } diff --git a/contracts/prebuilts/account/utils/AccountExtension.sol b/contracts/prebuilts/account/utils/AccountExtension.sol index 09c3fb2e5..161a124c1 100644 --- a/contracts/prebuilts/account/utils/AccountExtension.sol +++ b/contracts/prebuilts/account/utils/AccountExtension.sol @@ -77,8 +77,10 @@ contract AccountExtension is ContractMetadata, ERC1271, AccountPermissions, ERC7 } address caller = msg.sender; + EnumerableSet.AddressSet storage approvedTargets = _accountPermissionsStorage().approvedTargets[signer]; + require( - _accountPermissionsStorage().approvedTargets[signer].contains(caller), + approvedTargets.contains(caller) || (approvedTargets.length() == 1 && approvedTargets.at(0) == address(0)), "Account: caller not approved target." ); diff --git a/contracts/prebuilts/marketplace/entrypoint/MarketplaceV3.sol b/contracts/prebuilts/marketplace/entrypoint/MarketplaceV3.sol index 3d8f7e2c1..b7ba591fd 100644 --- a/contracts/prebuilts/marketplace/entrypoint/MarketplaceV3.sol +++ b/contracts/prebuilts/marketplace/entrypoint/MarketplaceV3.sol @@ -69,11 +69,11 @@ contract MarketplaceV3 is address nativeTokenWrapper; } - constructor(MarketplaceConstructorParams memory _params) - BaseRouter(_params.extensions) - RoyaltyPaymentsLogic(_params.royaltyEngineAddress) + constructor(MarketplaceConstructorParams memory _marketplaceV3Params) + BaseRouter(_marketplaceV3Params.extensions) + RoyaltyPaymentsLogic(_marketplaceV3Params.royaltyEngineAddress) { - nativeTokenWrapper = _params.nativeTokenWrapper; + nativeTokenWrapper = _marketplaceV3Params.nativeTokenWrapper; _disableInitializers(); } diff --git a/contracts/prebuilts/unaudited/contract-builder/CoreRouter.sol b/contracts/prebuilts/unaudited/contract-builder/CoreRouter.sol new file mode 100644 index 000000000..d504362f2 --- /dev/null +++ b/contracts/prebuilts/unaudited/contract-builder/CoreRouter.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: MIT +// @author: thirdweb (https://github.com/thirdweb-dev/dynamic-contracts) + +pragma solidity ^0.8.0; + +// Interface +import "lib/dynamic-contracts/src/presets/BaseRouter.sol"; + +// Core +import "lib/dynamic-contracts/src/core/Router.sol"; + +// Utils +import "lib/dynamic-contracts/src/lib/StringSet.sol"; +import "./extension/PermissionOverride.sol"; + +// Fixed extensions +import "../../../extension/Ownable.sol"; +import "../../../extension/ContractMetadata.sol"; + +/** + * //////////// + * + * NOTE: This contract is a work in progress, and has not been audited. + * + * //////////// + */ + +contract CoreRouter is BaseRouter, ContractMetadata, Ownable { + using StringSet for StringSet.Set; + + /*/////////////////////////////////////////////////////////////// + Constructor + //////////////////////////////////////////////////////////////*/ + + constructor(address _owner, Extension[] memory _extensions) BaseRouter(_extensions) { + // Initialize extensions + __BaseRouter_init(); + + _setupOwner(_owner); + } + + /*/////////////////////////////////////////////////////////////// + Internal functions + //////////////////////////////////////////////////////////////*/ + + /// @dev Returns whether all relevant permission and other checks are met before any upgrade. + function _isAuthorizedCallToUpgrade() internal view virtual override returns (bool) { + return msg.sender == owner(); + } + + /// @dev Returns whether contract metadata can be set in the given execution context. + function _canSetContractURI() internal view virtual override returns (bool) { + return msg.sender == owner(); + } + + /// @dev Returns whether owner can be set in the given execution context. + function _canSetOwner() internal view virtual override returns (bool) { + return msg.sender == owner(); + } +} diff --git a/contracts/prebuilts/unaudited/contract-builder/extension/PermissionOverride.sol b/contracts/prebuilts/unaudited/contract-builder/extension/PermissionOverride.sol new file mode 100644 index 000000000..ba5ba5dcf --- /dev/null +++ b/contracts/prebuilts/unaudited/contract-builder/extension/PermissionOverride.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +/// @author thirdweb + +/** + * //////////// + * + * NOTE: This contract is a work in progress, and has not been audited. + * + * //////////// + */ + +library PermissionsStorage { + bytes32 public constant PERMISSIONS_STORAGE_POSITION = keccak256("permissions.storage"); + + struct Data { + /// @dev Map from keccak256 hash of a role => a map from address => whether address has role. + mapping(bytes32 => mapping(address => bool)) _hasRole; + /// @dev Map from keccak256 hash of a role to role admin. See {getRoleAdmin}. + mapping(bytes32 => bytes32) _getRoleAdmin; + } + + function permissionsStorage() internal pure returns (Data storage permissionsData) { + bytes32 position = PERMISSIONS_STORAGE_POSITION; + assembly { + permissionsData.slot := position + } + } +} + +contract PermissionOverrideCoreRouter { + bytes32 private constant DEFAULT_ADMIN_ROLE = 0x00; + bytes32 private constant EXTENSION_ROLE = keccak256("EXTENSION_ROLE"); + + function canSetContractURI(address _caller) public view returns (bool) { + return _hasRole(DEFAULT_ADMIN_ROLE, _caller); + } + + function canSetOwner(address _caller) public view returns (bool) { + return _hasRole(DEFAULT_ADMIN_ROLE, _caller); + } + + function canSetExtension(address _caller) public view returns (bool) { + return _hasRole(DEFAULT_ADMIN_ROLE, _caller); + } + + function _hasRole(bytes32 role, address account) internal view returns (bool) { + PermissionsStorage.Data storage data = PermissionsStorage.permissionsStorage(); + return data._hasRole[role][account]; + } +} diff --git a/foundry.toml b/foundry.toml index 8a646b446..406a17df9 100644 --- a/foundry.toml +++ b/foundry.toml @@ -2,7 +2,7 @@ solc-version = "0.8.12" #auto_detect_solc = false cache = true -evm_version = 'shanghai' +evm_version = 'london' force = false gas_reports = [ "DropERC721Benchmark", diff --git a/package.json b/package.json index d992e597f..f064f0f4a 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "@openzeppelin/contracts": "4.7.3", "@openzeppelin/contracts-upgradeable": "4.7.3", "@thirdweb-dev/dynamic-contracts": "^1.1.2", - "@thirdweb-dev/sdk": "^3.10.64", + "@thirdweb-dev/sdk": "^4", + "@thirdweb-dev/chains": "^0.1.54", "@typechain/ethers-v5": "^10.0.0", "@types/fs-extra": "^9.0.13", "@types/mocha": "^9.1.0", diff --git a/scripts/deploy-prebuilt-deterministic/constants.ts b/scripts/deploy-prebuilt-deterministic/constants.ts index 377404f3f..d17207c71 100644 --- a/scripts/deploy-prebuilt-deterministic/constants.ts +++ b/scripts/deploy-prebuilt-deterministic/constants.ts @@ -1,8 +1,45 @@ +import { + Arbitrum, + ArbitrumGoerli, + Avalanche, + AvalancheFuji, + Base, + BaseGoerli, + CeloAlfajoresTestnet, + Ethereum, + Goerli, + Linea, + LineaTestnet, + Mumbai, + Optimism, + OptimismGoerli, + Polygon, + Sepolia, +} from "@thirdweb-dev/chains"; import { ChainId } from "@thirdweb-dev/sdk"; import dotenv from "dotenv"; dotenv.config(); +export const DEFAULT_CHAINS = [ + Ethereum, + Goerli, + Sepolia, + Polygon, + Mumbai, + Optimism, + OptimismGoerli, + Arbitrum, + ArbitrumGoerli, + Avalanche, + AvalancheFuji, + Base, + BaseGoerli, + Linea, + LineaTestnet, + CeloAlfajoresTestnet, +]; + export const chainIdToName: Record = { [ChainId.Mumbai]: "mumbai", [ChainId.Goerli]: "goerli", @@ -12,12 +49,8 @@ export const chainIdToName: Record = { [ChainId.OptimismGoerli]: "optimism-goerli", [ChainId.Arbitrum]: "arbitrum", [ChainId.ArbitrumGoerli]: "arbitrum-goerli", - [ChainId.Fantom]: "fantom", - [ChainId.FantomTestnet]: "fantom-testnet", [ChainId.Avalanche]: "avalanche", [ChainId.AvalancheFujiTestnet]: "avalanche-testnet", - [ChainId.BinanceSmartChainMainnet]: "binance", - [ChainId.BinanceSmartChainTestnet]: "binance-testnet", [84531]: "base-goerli", [8453]: "base", }; @@ -25,6 +58,7 @@ export const chainIdToName: Record = { export const chainIdApiKey: Record = { [ChainId.Mumbai]: process.env.POLYGONSCAN_API_KEY || process.env.SCAN_API_KEY, [ChainId.Goerli]: process.env.ETHERSCAN_API_KEY || process.env.SCAN_API_KEY, + [Sepolia.chainId]: process.env.ETHERSCAN_API_KEY || process.env.SCAN_API_KEY, [ChainId.Polygon]: process.env.POLYGONSCAN_API_KEY || process.env.SCAN_API_KEY, [ChainId.Mainnet]: process.env.ETHERSCAN_API_KEY || process.env.SCAN_API_KEY, [ChainId.Optimism]: process.env.OPTIMISM_SCAN_API_KEY || process.env.SCAN_API_KEY, @@ -37,13 +71,16 @@ export const chainIdApiKey: Record = { [ChainId.AvalancheFujiTestnet]: process.env.SNOWTRACE_API_KEY || process.env.SCAN_API_KEY, [ChainId.BinanceSmartChainMainnet]: process.env.BINANCE_SCAN_API_KEY || process.env.SCAN_API_KEY, [ChainId.BinanceSmartChainTestnet]: process.env.BINANCE_SCAN_API_KEY || process.env.SCAN_API_KEY, - [84531]: "" as string, - [8453]: process.env.BASE_SCAN_API_KEY || process.env.SCAN_API_KEY, + [Base.chainId]: process.env.BASE_SCAN_API_KEY || process.env.SCAN_API_KEY, + [BaseGoerli.chainId]: process.env.BASE_SCAN_API_KEY || process.env.SCAN_API_KEY, + [Linea.chainId]: process.env.LINEA_SCAN_API_KEY || process.env.SCAN_API_KEY, + [LineaTestnet.chainId]: process.env.LINEA_SCAN_API_KEY || process.env.SCAN_API_KEY, }; export const apiMap: Record = { 1: "https://api.etherscan.io/api", 5: "https://api-goerli.etherscan.io/api", + [Sepolia.chainId]: "https://api-sepolia.etherscan.io/api", 10: "https://api-optimistic.etherscan.io/api", 56: "https://api.bscscan.com/api", 97: "https://api-testnet.bscscan.com/api", @@ -58,6 +95,8 @@ export const apiMap: Record = { 80001: "https://api-testnet.polygonscan.com/api", 84531: "https://api-goerli.basescan.org/api", 8453: "https://api.basescan.org/api", + [Linea.chainId]: "https://api.lineascan.build/api", + [LineaTestnet.chainId]: "https://api-testnet.lineascan.build/api", }; export const contractsToDeploy = [ diff --git a/scripts/deploy-prebuilt-deterministic/deploy-deterministic-std-chains.ts b/scripts/deploy-prebuilt-deterministic/deploy-deterministic-std-chains.ts index 157de5783..d1bbafd42 100644 --- a/scripts/deploy-prebuilt-deterministic/deploy-deterministic-std-chains.ts +++ b/scripts/deploy-prebuilt-deterministic/deploy-deterministic-std-chains.ts @@ -13,51 +13,53 @@ import { resolveAddress, } from "@thirdweb-dev/sdk"; import { Signer } from "ethers"; -import { apiMap, chainIdApiKey, chainIdToName } from "./constants"; +import { DEFAULT_CHAINS, apiMap, chainIdApiKey } from "./constants"; ////// To run this script: `npx ts-node scripts/deploy-prebuilt-deterministic/deploy-deterministic-std-chains.ts` ////// ///// MAKE SURE TO PUT IN THE RIGHT CONTRACT NAME HERE AFTER PUBLISHING IT ///// //// THE CONTRACT SHOULD BE PUBLISHED WITH THE NEW PUBLISH FLOW //// -const publishedContractName = "DropERC1155"; -const publisherKey: string = process.env.THIRDWEB_PUBLISHER_PRIVATE_KEY as string; +const publishedContractName = "AccountExtension"; +const publisherAddress: string = "deployer.thirdweb.eth"; const deployerKey: string = process.env.PRIVATE_KEY as string; const secretKey: string = process.env.THIRDWEB_SECRET_KEY as string; -const polygonSDK = ThirdwebSDK.fromPrivateKey(publisherKey, "polygon", { secretKey }); +const polygonSDK = new ThirdwebSDK("polygon", { secretKey }); async function main() { - const publisher = await polygonSDK.wallet.getAddress(); - const latest = await polygonSDK.getPublisher().getLatest(publisher, publishedContractName); + const latest = await polygonSDK.getPublisher().getLatest(publisherAddress, publishedContractName); if (latest && latest.metadataUri) { const { extendedMetadata } = await fetchAndCacheDeployMetadata(latest?.metadataUri, polygonSDK.storage); - for (const [chainId, networkName] of Object.entries(chainIdToName)) { + for (const chain of DEFAULT_CHAINS) { const isNetworkEnabled = - extendedMetadata?.networksForDeployment?.networksEnabled.includes(parseInt(chainId)) || + extendedMetadata?.networksForDeployment?.networksEnabled.includes(chain.chainId) || extendedMetadata?.networksForDeployment?.allNetworks; if (extendedMetadata?.networksForDeployment && !isNetworkEnabled) { - console.log(`Deployment of ${publishedContractName} disabled on ${networkName}\n`); + console.log(`Deployment of ${publishedContractName} disabled on ${chain.slug}\n`); continue; } - console.log(`Deploying ${publishedContractName} on ${networkName}`); - const sdk = ThirdwebSDK.fromPrivateKey(deployerKey, chainId, { secretKey }); // can also hardcode the chain here + console.log(`Deploying ${publishedContractName} on ${chain.slug}`); + const sdk = ThirdwebSDK.fromPrivateKey(deployerKey, chain, { secretKey }); // can also hardcode the chain here const signer = sdk.getSigner() as Signer; // const chainId = (await sdk.getProvider().getNetwork()).chainId; try { - const implAddr = await getThirdwebContractAddress(publishedContractName, parseInt(chainId), sdk.storage); + const implAddr = await getThirdwebContractAddress(publishedContractName, chain.chainId, sdk.storage); if (implAddr) { - console.log(`implementation ${implAddr} already deployed on chainId: ${chainId}`); + console.log(`implementation ${implAddr} already deployed on chainId: ${chain.slug}`); console.log(); continue; } - } catch (error) {} + } catch (error) { + // no-op + } try { - console.log("Deploying as", await signer?.getAddress()); + console.log("Deploying as", await sdk.wallet.getAddress()); + console.log("Balance", await sdk.wallet.balance().then(b => b.displayValue)); // any evm deployment flow // Deploy CREATE2 factory (if not already exists) @@ -133,14 +135,16 @@ async function main() { console.log(); console.log("---------- Verification ---------"); console.log(); - for (const [chainId, networkName] of Object.entries(chainIdToName)) { - const sdk = new ThirdwebSDK(chainId); - console.log("Network: ", networkName); + for (const chain of DEFAULT_CHAINS) { + const sdk = new ThirdwebSDK(chain, { + secretKey, + }); + console.log("Verifying on: ", chain.slug); try { await sdk.verifier.verifyThirdwebContract( publishedContractName, - apiMap[parseInt(chainId)], - chainIdApiKey[parseInt(chainId)] as string, + apiMap[chain.chainId], + chainIdApiKey[chain.chainId] as string, ); console.log(); } catch (error) { diff --git a/scripts/deploy-prebuilt-deterministic/verify.ts b/scripts/deploy-prebuilt-deterministic/verify.ts index 2398b357a..e8ba37f4d 100644 --- a/scripts/deploy-prebuilt-deterministic/verify.ts +++ b/scripts/deploy-prebuilt-deterministic/verify.ts @@ -1,25 +1,32 @@ import { ThirdwebSDK } from "@thirdweb-dev/sdk"; -import { apiMap, chainIdApiKey, chainIdToName } from "./constants"; +import { DEFAULT_CHAINS, apiMap, chainIdApiKey } from "./constants"; ////// To run this script: `npx ts-node scripts/deploy-prebuilt-deterministic/verify.ts` ////// -const deployedContractName = "VoteERC20"; +const deployedContractName = "AccountExtension"; +const secretKey: string = process.env.THIRDWEB_SECRET_KEY as string; async function main() { console.log("---------- Verification ---------"); console.log(); - for (const [chainId, networkName] of Object.entries(chainIdToName)) { - const sdk = new ThirdwebSDK(chainId); - console.log("Network: ", networkName); + for (const chain of DEFAULT_CHAINS) { + const sdk = new ThirdwebSDK(chain, { + secretKey, + }); + console.log("Network: ", chain.slug); try { await sdk.verifier.verifyThirdwebContract( deployedContractName, - apiMap[parseInt(chainId)], - chainIdApiKey[parseInt(chainId)] as string, + apiMap[chain.chainId], + chainIdApiKey[chain.chainId] as string, ); console.log(); } catch (error) { - console.log(error); + if ((error as Error)?.message?.includes("already verified")) { + console.log("Already verified"); + } else { + console.log(error); + } console.log(); } } diff --git a/scripts/package-release.ts b/scripts/package-release.ts index 56137e90d..8babd386d 100644 --- a/scripts/package-release.ts +++ b/scripts/package-release.ts @@ -6,7 +6,12 @@ const artifactsForgeDir = path.join(__dirname, "..", "artifacts_forge"); const contractsDir = path.join(__dirname, "..", "contracts"); const contractArtifactsDir = path.join(__dirname, "..", "contract_artifacts"); -const specialCases: string[] = ["IBaseRouter.sol", "MockContractPublisher.sol"]; +const specialCases: string[] = [ + "IRouterState.sol", + "BaseRouter.sol", + "ExtensionManager.sol", + "MockContractPublisher.sol", +]; async function getAllSolidityFiles(dir: string): Promise { const dirents = await fs.readdir(dir, { withFileTypes: true }); diff --git a/src/test/smart-wallet/Account.t.sol b/src/test/smart-wallet/Account.t.sol index f55e36986..fdd04e811 100644 --- a/src/test/smart-wallet/Account.t.sol +++ b/src/test/smart-wallet/Account.t.sol @@ -56,13 +56,13 @@ contract SimpleAccountTest is BaseTest { event AccountCreated(address indexed account, address indexed accountAdmin); - function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req) internal view - returns (bytes memory signature) + returns (bytes32 typedDataHash) { bytes32 typehashSignerPermissionRequest = keccak256( - "SignerPermissionRequest(address signer,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" + "SignerPermissionRequest(address signer,uint8 isAdmin,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" ); bytes32 nameHash = keccak256(bytes("Account")); bytes32 versionHash = keccak256(bytes("1")); @@ -71,20 +71,32 @@ contract SimpleAccountTest is BaseTest { ); bytes32 domainSeparator = keccak256(abi.encode(typehashEip712, nameHash, versionHash, block.chainid, sender)); - bytes memory encodedRequest = abi.encode( + bytes memory encodedRequestStart = abi.encode( typehashSignerPermissionRequest, _req.signer, + _req.isAdmin, keccak256(abi.encodePacked(_req.approvedTargets)), - _req.nativeTokenLimitPerTransaction, + _req.nativeTokenLimitPerTransaction + ); + + bytes memory encodedRequestEnd = abi.encode( _req.permissionStartTimestamp, _req.permissionEndTimestamp, _req.reqValidityStartTimestamp, _req.reqValidityEndTimestamp, _req.uid ); - bytes32 structHash = keccak256(encodedRequest); - bytes32 typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + bytes32 structHash = keccak256(bytes.concat(encodedRequestStart, encodedRequestEnd)); + typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } + + function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes memory signature) + { + bytes32 typedDataHash = _prepareSignature(_req); (uint8 v, bytes32 r, bytes32 s) = vm.sign(accountAdminPKey, typedDataHash); signature = abi.encodePacked(r, s, v); } @@ -360,6 +372,7 @@ contract SimpleAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -397,6 +410,7 @@ contract SimpleAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -452,6 +466,7 @@ contract SimpleAccountTest is BaseTest { approvedTargets[0] = address(numberContract); IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -592,14 +607,11 @@ contract SimpleAccountTest is BaseTest { EntryPoint(entrypoint).handleOps(userOp, beneficiary); assertEq(SimpleAccount(payable(account)).contractURI(), "https://thirdweb.com"); - address[] memory targets = new address[](0); - uint256[] memory values = new uint256[](0); - bytes[] memory callData = new bytes[](0); - address[] memory approvedTargets = new address[](0); IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, diff --git a/src/test/smart-wallet/AccountVulnPOC.t.sol b/src/test/smart-wallet/AccountVulnPOC.t.sol index 806c10da4..4de05daf2 100644 --- a/src/test/smart-wallet/AccountVulnPOC.t.sol +++ b/src/test/smart-wallet/AccountVulnPOC.t.sol @@ -80,13 +80,13 @@ contract SimpleAccountVulnPOCTest is BaseTest { event AccountCreated(address indexed account, address indexed accountAdmin); - function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req) internal view - returns (bytes memory signature) + returns (bytes32 typedDataHash) { bytes32 typehashSignerPermissionRequest = keccak256( - "SignerPermissionRequest(address signer,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" + "SignerPermissionRequest(address signer,uint8 isAdmin,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" ); bytes32 nameHash = keccak256(bytes("Account")); bytes32 versionHash = keccak256(bytes("1")); @@ -95,20 +95,32 @@ contract SimpleAccountVulnPOCTest is BaseTest { ); bytes32 domainSeparator = keccak256(abi.encode(typehashEip712, nameHash, versionHash, block.chainid, sender)); - bytes memory encodedRequest = abi.encode( + bytes memory encodedRequestStart = abi.encode( typehashSignerPermissionRequest, _req.signer, + _req.isAdmin, keccak256(abi.encodePacked(_req.approvedTargets)), - _req.nativeTokenLimitPerTransaction, + _req.nativeTokenLimitPerTransaction + ); + + bytes memory encodedRequestEnd = abi.encode( _req.permissionStartTimestamp, _req.permissionEndTimestamp, _req.reqValidityStartTimestamp, _req.reqValidityEndTimestamp, _req.uid ); - bytes32 structHash = keccak256(encodedRequest); - bytes32 typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + bytes32 structHash = keccak256(bytes.concat(encodedRequestStart, encodedRequestEnd)); + typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } + + function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes memory signature) + { + bytes32 typedDataHash = _prepareSignature(_req); (uint8 v, bytes32 r, bytes32 s) = vm.sign(accountAdminPKey, typedDataHash); signature = abi.encodePacked(r, s, v); } @@ -231,10 +243,11 @@ contract SimpleAccountVulnPOCTest is BaseTest { Setup //////////////////////////////////////////////////////////////*/ address[] memory approvedTargets = new address[](1); - approvedTargets[0] = address(0); // allowing accountSigner permissions for some random contract, consider it as 0 address here + approvedTargets[0] = address(0x123); // allowing accountSigner permissions for some random contract, consider it as 0 address here IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, diff --git a/src/test/smart-wallet/DynamicAccount.t.sol b/src/test/smart-wallet/DynamicAccount.t.sol index a9599f02c..33f2a8585 100644 --- a/src/test/smart-wallet/DynamicAccount.t.sol +++ b/src/test/smart-wallet/DynamicAccount.t.sol @@ -72,13 +72,13 @@ contract DynamicAccountTest is BaseTest { event AccountCreated(address indexed account, address indexed accountAdmin); - function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req) internal view - returns (bytes memory signature) + returns (bytes32 typedDataHash) { bytes32 typehashSignerPermissionRequest = keccak256( - "SignerPermissionRequest(address signer,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" + "SignerPermissionRequest(address signer,uint8 isAdmin,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" ); bytes32 nameHash = keccak256(bytes("Account")); bytes32 versionHash = keccak256(bytes("1")); @@ -87,20 +87,32 @@ contract DynamicAccountTest is BaseTest { ); bytes32 domainSeparator = keccak256(abi.encode(typehashEip712, nameHash, versionHash, block.chainid, sender)); - bytes memory encodedRequest = abi.encode( + bytes memory encodedRequestStart = abi.encode( typehashSignerPermissionRequest, _req.signer, + _req.isAdmin, keccak256(abi.encodePacked(_req.approvedTargets)), - _req.nativeTokenLimitPerTransaction, + _req.nativeTokenLimitPerTransaction + ); + + bytes memory encodedRequestEnd = abi.encode( _req.permissionStartTimestamp, _req.permissionEndTimestamp, _req.reqValidityStartTimestamp, _req.reqValidityEndTimestamp, _req.uid ); - bytes32 structHash = keccak256(encodedRequest); - bytes32 typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + bytes32 structHash = keccak256(bytes.concat(encodedRequestStart, encodedRequestEnd)); + typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } + + function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes memory signature) + { + bytes32 typedDataHash = _prepareSignature(_req); (uint8 v, bytes32 r, bytes32 s) = vm.sign(accountAdminPKey, typedDataHash); signature = abi.encodePacked(r, s, v); } @@ -472,6 +484,7 @@ contract DynamicAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -509,6 +522,7 @@ contract DynamicAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -566,6 +580,7 @@ contract DynamicAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, diff --git a/src/test/smart-wallet/ManagedAccount.t.sol b/src/test/smart-wallet/ManagedAccount.t.sol index b88b1663b..8900950c5 100644 --- a/src/test/smart-wallet/ManagedAccount.t.sol +++ b/src/test/smart-wallet/ManagedAccount.t.sol @@ -73,13 +73,13 @@ contract ManagedAccountTest is BaseTest { event AccountCreated(address indexed account, address indexed accountAdmin); - function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req) internal view - returns (bytes memory signature) + returns (bytes32 typedDataHash) { bytes32 typehashSignerPermissionRequest = keccak256( - "SignerPermissionRequest(address signer,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" + "SignerPermissionRequest(address signer,uint8 isAdmin,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" ); bytes32 nameHash = keccak256(bytes("Account")); bytes32 versionHash = keccak256(bytes("1")); @@ -88,20 +88,32 @@ contract ManagedAccountTest is BaseTest { ); bytes32 domainSeparator = keccak256(abi.encode(typehashEip712, nameHash, versionHash, block.chainid, sender)); - bytes memory encodedRequest = abi.encode( + bytes memory encodedRequestStart = abi.encode( typehashSignerPermissionRequest, _req.signer, + _req.isAdmin, keccak256(abi.encodePacked(_req.approvedTargets)), - _req.nativeTokenLimitPerTransaction, + _req.nativeTokenLimitPerTransaction + ); + + bytes memory encodedRequestEnd = abi.encode( _req.permissionStartTimestamp, _req.permissionEndTimestamp, _req.reqValidityStartTimestamp, _req.reqValidityEndTimestamp, _req.uid ); - bytes32 structHash = keccak256(encodedRequest); - bytes32 typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + bytes32 structHash = keccak256(bytes.concat(encodedRequestStart, encodedRequestEnd)); + typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } + + function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes memory signature) + { + bytes32 typedDataHash = _prepareSignature(_req); (uint8 v, bytes32 r, bytes32 s) = vm.sign(accountAdminPKey, typedDataHash); signature = abi.encodePacked(r, s, v); } @@ -294,6 +306,7 @@ contract ManagedAccountTest is BaseTest { IEntryPoint(payable(address(entrypoint))), extensions ); + assertTrue(address(factory) != address(0), "factory address should not be zero"); } /*/////////////////////////////////////////////////////////////// @@ -462,6 +475,7 @@ contract ManagedAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -532,6 +546,7 @@ contract ManagedAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, @@ -569,6 +584,7 @@ contract ManagedAccountTest is BaseTest { IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( accountSigner, + 0, approvedTargets, 1 ether, 0, diff --git a/src/test/smart-wallet/account-core/isValidSigner.t.sol b/src/test/smart-wallet/account-core/isValidSigner.t.sol new file mode 100644 index 000000000..453a8a630 --- /dev/null +++ b/src/test/smart-wallet/account-core/isValidSigner.t.sol @@ -0,0 +1,562 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +// Test utils +import { BaseTest } from "../../utils/BaseTest.sol"; +import "contracts/external-deps/openzeppelin/proxy/Clones.sol"; +import "@thirdweb-dev/dynamic-contracts/src/interface/IExtension.sol"; +import { IAccountPermissions } from "contracts/extension/interface/IAccountPermissions.sol"; +import { AccountPermissions, EnumerableSet, ECDSA } from "contracts/extension/upgradeable/AccountPermissions.sol"; + +// Account Abstraction setup for smart wallets. +import { EntryPoint, IEntryPoint } from "contracts/prebuilts/account/utils/Entrypoint.sol"; +import { UserOperation } from "contracts/prebuilts/account/utils/UserOperation.sol"; + +// Target +import { DynamicAccountFactory, DynamicAccount, BaseAccountFactory } from "contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol"; + +/// @dev This is a dummy contract to test contract interactions with Account. +contract Number { + uint256 public num; + + function setNum(uint256 _num) public { + num = _num; + } + + function doubleNum() public { + num *= 2; + } + + function incrementNum() public { + num += 1; + } +} + +contract MyDynamicAccount is DynamicAccount { + using EnumerableSet for EnumerableSet.AddressSet; + + constructor(IEntryPoint _entrypoint, Extension[] memory _defaultExtensions) + DynamicAccount(_entrypoint, _defaultExtensions) + {} + + function setPermissionsForSigner( + address _signer, + uint256 _nativeTokenLimit, + uint256 _startTimestamp, + uint256 _endTimestamp + ) public { + _accountPermissionsStorage().signerPermissions[_signer] = SignerPermissionsStatic( + _nativeTokenLimit, + uint128(_startTimestamp), + uint128(_endTimestamp) + ); + } + + function setApprovedTargetsForSigner(address _signer, address[] memory _approvedTargets) public { + uint256 len = _approvedTargets.length; + for (uint256 i = 0; i < len; i += 1) { + _accountPermissionsStorage().approvedTargets[_signer].add(_approvedTargets[i]); + } + } + + function _setAdmin(address _account, bool _isAdmin) internal virtual override { + _accountPermissionsStorage().isAdmin[_account] = _isAdmin; + } + + function _isAuthorizedCallToUpgrade() internal view virtual override returns (bool) {} +} + +contract AccountCoreTest_isValidSigner is BaseTest { + // Target contracts + EntryPoint private entrypoint; + DynamicAccountFactory private accountFactory; + MyDynamicAccount private account; + + // Mocks + Number internal numberContract; + + // Test params + uint256 private accountAdminPKey = 100; + address private accountAdmin; + + uint256 private accountSignerPKey = 200; + address private accountSigner; + + uint256 private nonSignerPKey = 300; + address private nonSigner; + + address private opSigner; + uint256 private startTimestamp; + uint256 private endTimestamp; + uint256 private nativeTokenLimit; + UserOperation private op; + + bytes internal data = bytes(""); + + function _setupUserOp( + uint256 _signerPKey, + bytes memory _initCode, + bytes memory _callDataForEntrypoint + ) internal returns (UserOperation memory) { + uint256 nonce = entrypoint.getNonce(address(account), 0); + + // Get user op fields + UserOperation memory op = UserOperation({ + sender: address(account), + nonce: nonce, + initCode: _initCode, + callData: _callDataForEntrypoint, + callGasLimit: 5_000_000, + verificationGasLimit: 5_000_000, + preVerificationGas: 5_000_000, + maxFeePerGas: 0, + maxPriorityFeePerGas: 0, + paymasterAndData: bytes(""), + signature: bytes("") + }); + + // Sign UserOp + bytes32 opHash = EntryPoint(entrypoint).getUserOpHash(op); + bytes32 msgHash = ECDSA.toEthSignedMessageHash(opHash); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPKey, msgHash); + bytes memory userOpSignature = abi.encodePacked(r, s, v); + + address recoveredSigner = ECDSA.recover(msgHash, v, r, s); + address expectedSigner = vm.addr(_signerPKey); + assertEq(recoveredSigner, expectedSigner); + + op.signature = userOpSignature; + + return op; + } + + function _setupUserOpExecute( + uint256 _signerPKey, + bytes memory _initCode, + address _target, + uint256 _value, + bytes memory _callData + ) internal returns (UserOperation memory) { + bytes memory callDataForEntrypoint = abi.encodeWithSignature( + "execute(address,uint256,bytes)", + _target, + _value, + _callData + ); + + return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint); + } + + function _setupUserOpExecuteBatch( + uint256 _signerPKey, + bytes memory _initCode, + address[] memory _targets, + uint256[] memory _values, + bytes[] memory _callData + ) internal returns (UserOperation memory) { + bytes memory callDataForEntrypoint = abi.encodeWithSignature( + "executeBatch(address[],uint256[],bytes[])", + _targets, + _values, + _callData + ); + + return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint); + } + + function _setupUserOpInvalidFunction(uint256 _signerPKey, bytes memory _initCode) + internal + returns (UserOperation memory) + { + bytes memory callDataForEntrypoint = abi.encodeWithSignature("invalidFunction()"); + + return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint); + } + + function setUp() public override { + super.setUp(); + + // Setup signers. + accountAdmin = vm.addr(accountAdminPKey); + vm.deal(accountAdmin, 100 ether); + + accountSigner = vm.addr(accountSignerPKey); + nonSigner = vm.addr(nonSignerPKey); + + // Setup contracts + entrypoint = new EntryPoint(); + + IExtension.Extension[] memory extensions; + + // deploy account factory + accountFactory = new DynamicAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions); + // deploy dummy contract + numberContract = new Number(); + + address accountImpl = address(new MyDynamicAccount(IEntryPoint(payable(address(entrypoint))), extensions)); + address _account = Clones.cloneDeterministic(accountImpl, "salt"); + account = MyDynamicAccount(payable(_account)); + account.initialize(accountAdmin, ""); + } + + function test_isValidSigner_whenSignerIsAdmin() public { + opSigner = accountAdmin; + UserOperation memory _op; // empty op since it's not relevant for this check + bool isValid = DynamicAccount(payable(account)).isValidSigner(opSigner, _op); + + assertTrue(isValid); + } + + modifier whenNotAdmin() { + opSigner = accountSigner; + _; + } + + function test_isValidSigner_invalidTimestamps() public whenNotAdmin { + UserOperation memory _op; // empty op since it's not relevant for this check + startTimestamp = 100; + endTimestamp = 200; + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + vm.warp(201); // block timestamp greater than end timestamp + bool isValid = account.isValidSigner(opSigner, _op); + + assertFalse(isValid); + + vm.warp(200); // block timestamp equal to end timestamp + isValid = account.isValidSigner(opSigner, _op); + + assertFalse(isValid); + + vm.warp(99); // block timestamp less than start timestamp + isValid = account.isValidSigner(opSigner, _op); + + assertFalse(isValid); + } + + modifier whenValidTimestamps() { + startTimestamp = 100; + endTimestamp = 200; + vm.warp(150); // block timestamp within start and end timestamps + _; + } + + function test_isValidSigner_noApprovedTargets() public whenNotAdmin whenValidTimestamps { + UserOperation memory _op; // empty op since it's not relevant for this check + address[] memory _approvedTargets; + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + bool isValid = account.isValidSigner(opSigner, _op); + + assertFalse(isValid); + } + + // ================== + // ======= Test branch: wildcard + // ================== + + function test_isValidSigner_wildcardExecute_breachNativeTokenLimit() public whenNotAdmin whenValidTimestamps { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(0); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + op = _setupUserOpExecute(accountSignerPKey, bytes(""), address(0x123), 10, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + function test_isValidSigner_wildcardExecuteBatch_breachNativeTokenLimit() public whenNotAdmin whenValidTimestamps { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(0); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + uint256 count = 3; + address[] memory targets = new address[](count); + uint256[] memory values = new uint256[](count); + bytes[] memory callData = new bytes[](count); + + for (uint256 i = 0; i < count; i += 1) { + targets[i] = address(numberContract); + values[i] = 10; + callData[i] = abi.encodeWithSignature("incrementNum()", i); + } + + op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + modifier whenWithinNativeTokenLimit() { + nativeTokenLimit = 1000; + _; + } + + function test_isValidSigner_wildcardExecute() public whenNotAdmin whenValidTimestamps whenWithinNativeTokenLimit { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(0); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + op = _setupUserOpExecute(accountSignerPKey, bytes(""), address(0x123), 10, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertTrue(isValid); + } + + function test_isValidSigner_wildcardExecuteBatch() + public + whenNotAdmin + whenValidTimestamps + whenWithinNativeTokenLimit + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(0); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + uint256 count = 3; + address[] memory targets = new address[](count); + uint256[] memory values = new uint256[](count); + bytes[] memory callData = new bytes[](count); + + for (uint256 i = 0; i < count; i += 1) { + targets[i] = address(numberContract); + values[i] = 10; + callData[i] = abi.encodeWithSignature("incrementNum()", i); + } + + op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertTrue(isValid); + } + + function test_isValidSigner_wildcardInvalidFunction() + public + whenNotAdmin + whenValidTimestamps + whenWithinNativeTokenLimit + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(0); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + op = _setupUserOpInvalidFunction(accountSignerPKey, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + // ================== + // ======= Test branch: not wildcard + // ================== + + function test_isValidSigner_execute_callingWrongTarget() + public + whenNotAdmin + whenValidTimestamps + whenWithinNativeTokenLimit + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + address wrongTarget = address(0x123); + op = _setupUserOpExecute(accountSignerPKey, bytes(""), wrongTarget, 10, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + function test_isValidSigner_executeBatch_callingWrongTarget() + public + whenNotAdmin + whenValidTimestamps + whenWithinNativeTokenLimit + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + uint256 count = 3; + address[] memory targets = new address[](count); + uint256[] memory values = new uint256[](count); + bytes[] memory callData = new bytes[](count); + address wrongTarget = address(0x123); + for (uint256 i = 0; i < count; i += 1) { + targets[i] = wrongTarget; + values[i] = 10; + callData[i] = abi.encodeWithSignature("incrementNum()", i); + } + + op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + modifier whenCorrectTarget() { + _; + } + + function test_isValidSigner_execute_breachNativeTokenLimit() + public + whenNotAdmin + whenValidTimestamps + whenCorrectTarget + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + op = _setupUserOpExecute(accountSignerPKey, bytes(""), address(numberContract), 10, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + function test_isValidSigner_executeBatch_breachNativeTokenLimit() + public + whenNotAdmin + whenValidTimestamps + whenCorrectTarget + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + uint256 count = 3; + address[] memory targets = new address[](count); + uint256[] memory values = new uint256[](count); + bytes[] memory callData = new bytes[](count); + + for (uint256 i = 0; i < count; i += 1) { + targets[i] = address(numberContract); + values[i] = 10; + callData[i] = abi.encodeWithSignature("incrementNum()", i); + } + + op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } + + function test_isValidSigner_execute() + public + whenNotAdmin + whenValidTimestamps + whenWithinNativeTokenLimit + whenCorrectTarget + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + op = _setupUserOpExecute(accountSignerPKey, bytes(""), address(numberContract), 10, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertTrue(isValid); + } + + function test_isValidSigner_executeBatch() + public + whenNotAdmin + whenValidTimestamps + whenWithinNativeTokenLimit + whenCorrectTarget + { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + uint256 count = 3; + address[] memory targets = new address[](count); + uint256[] memory values = new uint256[](count); + bytes[] memory callData = new bytes[](count); + + for (uint256 i = 0; i < count; i += 1) { + targets[i] = address(numberContract); + values[i] = 10; + callData[i] = abi.encodeWithSignature("incrementNum()", i); + } + + op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertTrue(isValid); + } + + function test_isValidSigner_invalidFunction() public whenNotAdmin whenValidTimestamps whenWithinNativeTokenLimit { + // set wildcard + address[] memory _approvedTargets = new address[](1); + _approvedTargets[0] = address(numberContract); + account.setApprovedTargetsForSigner(opSigner, _approvedTargets); + + // user op execute + op = _setupUserOpInvalidFunction(accountSignerPKey, bytes("")); + + account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp); + + bool isValid = account.isValidSigner(opSigner, op); + + assertFalse(isValid); + } +} diff --git a/src/test/smart-wallet/account-core/isValidSigner.tree b/src/test/smart-wallet/account-core/isValidSigner.tree new file mode 100644 index 000000000..cc4497260 --- /dev/null +++ b/src/test/smart-wallet/account-core/isValidSigner.tree @@ -0,0 +1,46 @@ +isValidSigner(address _signer, UserOperation calldata _userOp) +├── when `_signer` is admin +│ └── it should return true +├── when `_signer` is not admin + └── when timestamp is invalid + │ └── it should return false + └── when timestamp is valid + └── when no approved targets + │ └── it should return false + │ + │ // Case - Wildcard + └── when approved targets length is equal to 1 and contains address(0) + │ └── when calling `execute` function + │ │ └── when the decoded `value` is more than nativeTokenLimitPerTransaction + │ │ │ └── it should return false + │ │ └── when the decoded `value` is within nativeTokenLimitPerTransaction + │ │ └── it should return true + │ └── when calling `batchExecute` function + │ │ └── when any item in the decoded `values` array is more than nativeTokenLimitPerTransaction + │ │ │ └── it should return false + │ │ └── when all items in the decoded `values` array are within nativeTokenLimitPerTransaction + │ │ └── it should return true + │ └── when calling an invalid function + │ └── it should return false + │ + │ // Case - No Wildcard + └── when approved targets length is greater than 1, or doesn't contain address(0) + └── when calling `execute` function + │ └── when approvedTargets doesn't contain the decoded `target` + │ │ └── it should return false + │ └── when approvedTargets contains the decoded `target` + │ └── when the decoded `value` is more than nativeTokenLimitPerTransaction + │ │ └── it should return false + │ └── when the decoded `value` is within nativeTokenLimitPerTransaction + │ └── it should return true + └── when calling `batchExecute` function + │ └── when approvedTargets doesn't contain one or more items in the decoded `targets` array + │ │ └── it should return false + │ └── when approvedTargets contains all items in the decdoded `targets` array + │ └── when any item in the decoded `values` array is more than nativeTokenLimitPerTransaction + │ │ └── it should return false + │ └── when all items in the decoded `values` array are within nativeTokenLimitPerTransaction + │ └── it should return true + └── when calling an invalid function + └── it should return false + \ No newline at end of file diff --git a/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol b/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol new file mode 100644 index 000000000..170db6fcd --- /dev/null +++ b/src/test/smart-wallet/account-permissions/setPermissionsForSigner.t.sol @@ -0,0 +1,639 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +// Test utils +import "../../utils/BaseTest.sol"; +import "@thirdweb-dev/dynamic-contracts/src/interface/IExtension.sol"; +import { IAccountPermissions } from "contracts/extension/interface/IAccountPermissions.sol"; +import { AccountPermissions } from "contracts/extension/upgradeable/AccountPermissions.sol"; +import { AccountExtension } from "contracts/prebuilts/account/utils/AccountExtension.sol"; + +// Account Abstraction setup for smart wallets. +import { EntryPoint, IEntryPoint } from "contracts/prebuilts/account/utils/Entrypoint.sol"; +import { UserOperation } from "contracts/prebuilts/account/utils/UserOperation.sol"; + +// Target +import { Account as SimpleAccount } from "contracts/prebuilts/account/non-upgradeable/Account.sol"; +import { DynamicAccountFactory, DynamicAccount } from "contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol"; + +/// @dev This is a dummy contract to test contract interactions with Account. +contract Number { + uint256 public num; + + function setNum(uint256 _num) public { + num = _num; + } + + function doubleNum() public { + num *= 2; + } + + function incrementNum() public { + num += 1; + } +} + +contract NFTRejector { + function onERC721Received( + address, + address, + uint256, + bytes memory + ) public virtual returns (bytes4) { + revert("NFTs not accepted"); + } +} + +contract AccountPermissionsTest_setPermissionsForSigner is BaseTest { + event AdminUpdated(address indexed signer, bool isAdmin); + + event SignerPermissionsUpdated( + address indexed authorizingSigner, + address indexed targetSigner, + IAccountPermissions.SignerPermissionRequest permissions + ); + + // Target contracts + EntryPoint private entrypoint; + DynamicAccountFactory private accountFactory; + + // Mocks + Number internal numberContract; + + // Test params + uint256 private accountAdminPKey = 100; + address private accountAdmin; + + uint256 private accountSignerPKey = 200; + address private accountSigner; + + uint256 private nonSignerPKey = 300; + address private nonSigner; + + bytes internal data = bytes(""); + + // UserOp terminology: `sender` is the smart wallet. + address private sender = 0xbC12AEae5E1b1a80401dd20A6728f7a01a3A6166; + address payable private beneficiary = payable(address(0x45654)); + + bytes32 private uidCache = bytes32("random uid"); + + event AccountCreated(address indexed account, address indexed accountAdmin); + + function _prepareSignature(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes32 typedDataHash) + { + bytes32 typehashSignerPermissionRequest = keccak256( + "SignerPermissionRequest(address signer,uint8 isAdmin,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)" + ); + bytes32 nameHash = keccak256(bytes("Account")); + bytes32 versionHash = keccak256(bytes("1")); + bytes32 typehashEip712 = keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" + ); + bytes32 domainSeparator = keccak256(abi.encode(typehashEip712, nameHash, versionHash, block.chainid, sender)); + + bytes memory encodedRequestStart = abi.encode( + typehashSignerPermissionRequest, + _req.signer, + _req.isAdmin, + keccak256(abi.encodePacked(_req.approvedTargets)), + _req.nativeTokenLimitPerTransaction + ); + + bytes memory encodedRequestEnd = abi.encode( + _req.permissionStartTimestamp, + _req.permissionEndTimestamp, + _req.reqValidityStartTimestamp, + _req.reqValidityEndTimestamp, + _req.uid + ); + + bytes32 structHash = keccak256(bytes.concat(encodedRequestStart, encodedRequestEnd)); + typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } + + function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes memory signature) + { + bytes32 typedDataHash = _prepareSignature(_req); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(accountAdminPKey, typedDataHash); + signature = abi.encodePacked(r, s, v); + } + + function _signSignerPermissionRequestInvalid(IAccountPermissions.SignerPermissionRequest memory _req) + internal + view + returns (bytes memory signature) + { + bytes32 typedDataHash = _prepareSignature(_req); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(0x111, typedDataHash); + signature = abi.encodePacked(r, s, v); + } + + function _setupUserOp( + uint256 _signerPKey, + bytes memory _initCode, + bytes memory _callDataForEntrypoint + ) internal returns (UserOperation[] memory ops) { + uint256 nonce = entrypoint.getNonce(sender, 0); + + // Get user op fields + UserOperation memory op = UserOperation({ + sender: sender, + nonce: nonce, + initCode: _initCode, + callData: _callDataForEntrypoint, + callGasLimit: 5_000_000, + verificationGasLimit: 5_000_000, + preVerificationGas: 5_000_000, + maxFeePerGas: 0, + maxPriorityFeePerGas: 0, + paymasterAndData: bytes(""), + signature: bytes("") + }); + + // Sign UserOp + bytes32 opHash = EntryPoint(entrypoint).getUserOpHash(op); + bytes32 msgHash = ECDSA.toEthSignedMessageHash(opHash); + + (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPKey, msgHash); + bytes memory userOpSignature = abi.encodePacked(r, s, v); + + address recoveredSigner = ECDSA.recover(msgHash, v, r, s); + address expectedSigner = vm.addr(_signerPKey); + assertEq(recoveredSigner, expectedSigner); + + op.signature = userOpSignature; + + // Store UserOp + ops = new UserOperation[](1); + ops[0] = op; + } + + function _setupUserOpExecute( + uint256 _signerPKey, + bytes memory _initCode, + address _target, + uint256 _value, + bytes memory _callData + ) internal returns (UserOperation[] memory) { + bytes memory callDataForEntrypoint = abi.encodeWithSignature( + "execute(address,uint256,bytes)", + _target, + _value, + _callData + ); + + return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint); + } + + function _setupUserOpExecuteBatch( + uint256 _signerPKey, + bytes memory _initCode, + address[] memory _target, + uint256[] memory _value, + bytes[] memory _callData + ) internal returns (UserOperation[] memory) { + bytes memory callDataForEntrypoint = abi.encodeWithSignature( + "executeBatch(address[],uint256[],bytes[])", + _target, + _value, + _callData + ); + + return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint); + } + + function setUp() public override { + super.setUp(); + + // Setup signers. + accountAdmin = vm.addr(accountAdminPKey); + vm.deal(accountAdmin, 100 ether); + + accountSigner = vm.addr(accountSignerPKey); + nonSigner = vm.addr(nonSignerPKey); + + // Setup contracts + entrypoint = new EntryPoint(); + + // Setting up default extension. + IExtension.Extension memory defaultExtension; + + defaultExtension.metadata = IExtension.ExtensionMetadata({ + name: "AccountExtension", + metadataURI: "ipfs://AccountExtension", + implementation: address(new AccountExtension()) + }); + + defaultExtension.functions = new IExtension.ExtensionFunction[](7); + + defaultExtension.functions[0] = IExtension.ExtensionFunction( + AccountExtension.supportsInterface.selector, + "supportsInterface(bytes4)" + ); + defaultExtension.functions[1] = IExtension.ExtensionFunction( + AccountExtension.execute.selector, + "execute(address,uint256,bytes)" + ); + defaultExtension.functions[2] = IExtension.ExtensionFunction( + AccountExtension.executeBatch.selector, + "executeBatch(address[],uint256[],bytes[])" + ); + defaultExtension.functions[3] = IExtension.ExtensionFunction( + ERC721Holder.onERC721Received.selector, + "onERC721Received(address,address,uint256,bytes)" + ); + defaultExtension.functions[4] = IExtension.ExtensionFunction( + ERC1155Holder.onERC1155Received.selector, + "onERC1155Received(address,address,uint256,uint256,bytes)" + ); + defaultExtension.functions[5] = IExtension.ExtensionFunction( + bytes4(0), // Selector for `receive()` function. + "receive()" + ); + defaultExtension.functions[6] = IExtension.ExtensionFunction( + AccountExtension.isValidSignature.selector, + "isValidSignature(bytes32,bytes)" + ); + + IExtension.Extension[] memory extensions = new IExtension.Extension[](1); + extensions[0] = defaultExtension; + + // deploy account factory + accountFactory = new DynamicAccountFactory(IEntryPoint(payable(address(entrypoint))), extensions); + // deploy dummy contract + numberContract = new Number(); + } + + function _setup_executeTransaction() internal { + bytes memory initCallData = abi.encodeWithSignature("createAccount(address,bytes)", accountAdmin, data); + bytes memory initCode = abi.encodePacked(abi.encodePacked(address(accountFactory)), initCallData); + + UserOperation[] memory userOpCreateAccount = _setupUserOpExecute( + accountAdminPKey, + initCode, + address(0), + 0, + bytes("") + ); + + EntryPoint(entrypoint).handleOps(userOpCreateAccount, beneficiary); + } + + function test_state_targetAdminNotAdmin() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + bool adminStatusBefore = SimpleAccount(payable(account)).isAdmin(accountSigner); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 0, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + + bool adminStatusAfter = SimpleAccount(payable(account)).isAdmin(accountSigner); + + assertEq(adminStatusBefore, false); + assertEq(adminStatusAfter, true); + } + + function test_state_targetAdminIsAdmin() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + { + IAccountPermissions.SignerPermissionRequest memory request = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + bytes memory sig2 = _signSignerPermissionRequest(request); + SimpleAccount(payable(account)).setPermissionsForSigner(request, sig2); + + address[] memory adminsBefore = SimpleAccount(payable(account)).getAllAdmins(); + assertEq(adminsBefore[1], accountSigner); + } + + bool adminStatusBefore = SimpleAccount(payable(account)).isAdmin(accountAdmin); + + uidCache = bytes32("new uid"); + + IAccountPermissions.SignerPermissionRequest memory req = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 2, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + bytes memory sig3 = _signSignerPermissionRequest(req); + SimpleAccount(payable(account)).setPermissionsForSigner(req, sig3); + + bool adminStatusAfter = SimpleAccount(payable(account)).isAdmin(accountSigner); + address[] memory adminsAfter = SimpleAccount(payable(account)).getAllAdmins(); + + assertEq(adminStatusBefore, true); + assertEq(adminStatusAfter, false); + assertEq(adminsAfter.length, 1); + } + + function test_revert_attemptReplayUID() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + + // Attempt replay UID + + IAccountPermissions.SignerPermissionRequest memory permissionsReqTwo = IAccountPermissions + .SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 0, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + sig = _signSignerPermissionRequest(permissionsReqTwo); + vm.expectRevert(); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReqTwo, sig); + } + + function test_event_addAdmin_AdminUpdated() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + + vm.expectEmit(true, false, false, true); + emit AdminUpdated(accountSigner, true); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + } + + function test_event_removeAdmin_AdminUpdated() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 2, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + + vm.expectEmit(true, false, false, true); + emit AdminUpdated(accountSigner, false); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + } + + function test_revert_timeBeforeStart() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 0, + 0, + type(uint128).max, + uint128(block.timestamp + 1000), + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + vm.expectRevert("!period"); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + } + + function test_revert_timeAfterExpiry() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 0, + 0, + type(uint128).max, + 0, + uint128(block.timestamp - 1), + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + vm.expectRevert("!period"); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + } + + function test_revert_SignerNotAdmin() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 0, + approvedTargets, + 0, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig = _signSignerPermissionRequestInvalid(permissionsReq); + vm.expectRevert(bytes("!sig")); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + } + + function test_revert_SignerAlreadyAdmin() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](0); + + { + //set admin status + IAccountPermissions.SignerPermissionRequest memory req = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 1, + approvedTargets, + 0, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig2 = _signSignerPermissionRequest(req); + SimpleAccount(payable(account)).setPermissionsForSigner(req, sig2); + } + + //test set signerPerms as admin + + uidCache = bytes32("new uid"); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 0, + approvedTargets, + 0, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig3 = _signSignerPermissionRequest(permissionsReq); + vm.expectRevert("admin"); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig3); + } + + function test_state_setPermissionsForSigner() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](1); + approvedTargets[0] = address(numberContract); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 0, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + + IAccountPermissions.SignerPermissions[] memory allSigners = SimpleAccount(payable(account)).getAllSigners(); + assertEq(allSigners[0].signer, accountSigner); + assertEq(allSigners[0].approvedTargets[0], address(numberContract)); + assertEq(allSigners[0].nativeTokenLimitPerTransaction, 1); + assertEq(allSigners[0].startTimestamp, 0); + assertEq(allSigners[0].endTimestamp, type(uint128).max); + } + + function test_event_addSigner() public { + _setup_executeTransaction(); + + address account = accountFactory.getAddress(accountAdmin, bytes("")); + address[] memory approvedTargets = new address[](1); + approvedTargets[0] = address(numberContract); + + IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest( + accountSigner, + 0, + approvedTargets, + 1, + 0, + type(uint128).max, + 0, + type(uint128).max, + uidCache + ); + + vm.prank(accountAdmin); + bytes memory sig = _signSignerPermissionRequest(permissionsReq); + + vm.expectEmit(true, true, false, true); + emit SignerPermissionsUpdated(accountAdmin, accountSigner, permissionsReq); + SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig); + } +} diff --git a/src/test/smart-wallet/account-permissions/setPermissionsForSigner.tree b/src/test/smart-wallet/account-permissions/setPermissionsForSigner.tree new file mode 100644 index 000000000..fbe90351d --- /dev/null +++ b/src/test/smart-wallet/account-permissions/setPermissionsForSigner.tree @@ -0,0 +1,33 @@ +function setPermissionsForSigner(SignerPermissionRequest calldata _req, bytes calldata _signature) +├── when reqValidityStartTimestamp is greater than block.timestamp +│ └── it should revert ✅ +├── when reqValidityEndTimestamp is less than block.timestamp +│ └── it should revert ✅ +├── when uid is executed +│ └── it should revert ✅ +├── when the recovered signer is not an admin +│ └── it should revert ✅ +└── when the reqValidityStartTimestamp is less than block.timestamp + └── when reqValidityEndTimestamp is greater than block.timestamp + └── when recovered signer is an admin ✅ + └── when req.uid has not been marked as executed + └── when _req.isAdmin is greater than zero + ├── it should mark req.uid as executed ✅ + ├── when _req.isAdmin is one + │ ├── it should set isAdmin[(targetAdmin)] as true ✅ + │ ├── it should add targetAdmin to allAdmins ✅ + │ └── it should emit AdminUpdated with the parameters targetAdmin, true ✅ + ├── when _req.isAdmin is greater than one + │ ├── it should set isAdmin[(targetAdmin)] as false ✅ + │ ├── it should remove targetAdmin from allAdmins ✅ + │ └── it should emit the event AdminUpdated with the parameters targetAdmin, false ✅ + └── when _req.isAdmin is equal to zero + ├── when targetSigner is an admin + │ └── it should revert ✅ + └── when targetSigner is not an admin + ├── it should mark req.uid as executed ✅ + ├── it should add targetSigner to allSigners ✅ + ├── it should set signerPermissions[(targetSigner)] as a SignerPermissionsStatic(nativeTokenLimitPerTransaction, permissionStartTimestamp, permissionEndTimestamp) ✅ + ├── it should remove current approved targets for targetSigner ✅ + ├── it should add the new approved targets for targetSigner ✅ + └── it should emit the event SignerPermissionsUpdated with the parameters signer, targetSigner, SignerPermissionRequest ✅ \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index ca7c49ca3..a90012b01 100644 --- a/yarn.lock +++ b/yarn.lock @@ -515,22 +515,22 @@ dependencies: antlr4ts "^0.5.0-alpha.4" -"@thirdweb-dev/chains@0.1.54": +"@thirdweb-dev/chains@0.1.54", "@thirdweb-dev/chains@^0.1.54": version "0.1.54" resolved "https://registry.yarnpkg.com/@thirdweb-dev/chains/-/chains-0.1.54.tgz#90e5c372a1d9cd785c51715bfbeba276ca2a203f" integrity sha512-iCuKgtN2KIdfgqbIbZYgB8ObYdOJW9iXW9b5u+WKA4zyGApw1MTOSX0W2aPnadGen1z4iQfAuUDBYQ6JVqDOjg== -"@thirdweb-dev/contracts-js@1.3.13": - version "1.3.13" - resolved "https://registry.yarnpkg.com/@thirdweb-dev/contracts-js/-/contracts-js-1.3.13.tgz#17b07f7ead147177aa979dd8f850030d4b31bc68" - integrity sha512-GKg3tqE7KZOApRBsK7F6YvP78TgcQR0p2u67tsdp8+bp7MYrJwLgGKTmsWMWo2lY+Tbu2y9Rj18Z2MZaoCuQTQ== +"@thirdweb-dev/contracts-js@1.3.15": + version "1.3.15" + resolved "https://registry.yarnpkg.com/@thirdweb-dev/contracts-js/-/contracts-js-1.3.15.tgz#1e9df70cc4333ab2d6f0af5f0e9f2e26b043d9c9" + integrity sha512-ACKwjYPgTkhwvFvAbJudCZ9tQJIKpkaTKeq/tVBt3A2OXfAb8jc33Iu78K5WYoX0E0wvGqSxU8K8vkxij+RsnA== dependencies: - "@thirdweb-dev/contracts" "3.10.0" + "@thirdweb-dev/contracts" "3.10.3-1" -"@thirdweb-dev/contracts@3.10.0": - version "3.10.0" - resolved "https://registry.yarnpkg.com/@thirdweb-dev/contracts/-/contracts-3.10.0.tgz#162d3b090ee576a18f530a3997f820600aab5f73" - integrity sha512-IGnFev/ooS4y/oh+wURuv0E32ztYDeZk7RdWO0si1YY5vDMSbVu/TBhlhmLPcfIZWs0IjCAoA6H/Zy5QV5mkhw== +"@thirdweb-dev/contracts@3.10.3-1": + version "3.10.3-1" + resolved "https://registry.yarnpkg.com/@thirdweb-dev/contracts/-/contracts-3.10.3-1.tgz#0f970fa0b9c821cd9036bb51334ab92083123a96" + integrity sha512-eiGS1Q2+Ge48yEO+kBw0DqR6eTN6ip4vrY0eBoaUrpcFAudd+aW+qszxCFBUuKSHIxvRs3Ul2b64pdYFEXzMIA== dependencies: "@chainlink/contracts" "^0.6.1" "@openzeppelin/contracts" "4.7.3" @@ -548,38 +548,36 @@ resolved "https://registry.yarnpkg.com/@thirdweb-dev/generated-abis/-/generated-abis-0.0.1.tgz#0d788d6aff0ac08f11e9eeb9ae4c8321845272a8" integrity sha512-vO9/3lSLO8smyyH1QVeYravSTzFwV1nf1C/Im1NBDPdH8//YvcbhtETGGiNfHWpyCvSi0vRYwvf+/7FKdwpDGQ== -"@thirdweb-dev/sdk@^3.10.64": - version "3.10.64" - resolved "https://registry.yarnpkg.com/@thirdweb-dev/sdk/-/sdk-3.10.64.tgz#f5c7d0a154267f1a82759ae6ba0c1e9edc312a45" - integrity sha512-kQIkXh2jDHnHMh1uhSZBGFlodtSomqmnjO4E5y1cgcJFxsTD/AID4tSGR/00mjHC92nXNR87ipHEoi1VAvZwYw== +"@thirdweb-dev/sdk@^4": + version "4.0.3" + resolved "https://registry.yarnpkg.com/@thirdweb-dev/sdk/-/sdk-4.0.3.tgz#51dc8eddca91a342467b4e712431d41cc3d8fd89" + integrity sha512-VcS0XjCxddiHYy+bavnMm0VliEnrSexp8hwzU2yBlxAXbnqm+H6EAfNHHVoJwFlWsG9y056s7YPzBlv6yZlk0w== dependencies: "@thirdweb-dev/chains" "0.1.54" - "@thirdweb-dev/contracts-js" "1.3.13" + "@thirdweb-dev/contracts-js" "1.3.15" "@thirdweb-dev/generated-abis" "0.0.1" - "@thirdweb-dev/storage" "1.2.10" + "@thirdweb-dev/storage" "2.0.0" abitype "^0.2.5" bn.js "^5.2.1" bs58 "^5.0.0" buffer "^6.0.3" - cross-fetch "^3.1.8" eventemitter3 "^5.0.1" fast-deep-equal "^3.1.3" merkletreejs "^0.2.24" tiny-invariant "^1.2.0" tweetnacl "^1.0.3" - uuid "^9.0.0" + uuid "^9.0.1" yaml "^2.3.1" - zod "^3.20.2" + zod "^3.22.3" -"@thirdweb-dev/storage@1.2.10": - version "1.2.10" - resolved "https://registry.yarnpkg.com/@thirdweb-dev/storage/-/storage-1.2.10.tgz#04f25211c8a2d23b9a2171aac798211fe49c9de7" - integrity sha512-tUqUfkCfrZfmYcNVxR7j9CLMrIx0C4w/2QkNNo27AlbKGq4x73UCKRBTPZI2ggTKGfq6pyayOfFuVLoN7W/AqA== +"@thirdweb-dev/storage@2.0.0": + version "2.0.0" + resolved "https://registry.yarnpkg.com/@thirdweb-dev/storage/-/storage-2.0.0.tgz#b3e4a34bcbcdd3b2ce3171af76d69a56993e7fa0" + integrity sha512-pfTbiwgrp2N2lrTfa8nLt5E9V1+IGtYKtKU82ReOKKYkRTi0qkqI5ydNuzM2VUcwIyyPnlRR/W7NloHyyBW5/Q== dependencies: cid-tool "^3.0.0" - cross-fetch "^3.1.8" form-data "^4.0.0" - uuid "^9.0.0" + uuid "^9.0.1" "@tsconfig/node10@^1.0.7": version "1.0.8" @@ -1293,13 +1291,6 @@ create-require@^1.1.0: resolved "https://registry.npmjs.org/create-require/-/create-require-1.1.1.tgz" integrity sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ== -cross-fetch@^3.1.8: - version "3.1.8" - resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.8.tgz#0327eba65fd68a7d119f8fb2bf9334a1a7956f82" - integrity sha512-cvA+JwZoU0Xq+h6WkMvAUqPEYy92Obet6UdKLfW60qn99ftItKjB5T+BkyWOFWe2pUyfQ+IJHmpOTznqk1M6Kg== - dependencies: - node-fetch "^2.6.12" - cross-spawn@^6.0.5: version "6.0.5" resolved "https://registry.npmjs.org/cross-spawn/-/cross-spawn-6.0.5.tgz" @@ -2721,13 +2712,6 @@ node-addon-api@^2.0.0: resolved "https://registry.npmjs.org/node-addon-api/-/node-addon-api-2.0.2.tgz" integrity sha512-Ntyt4AIXyaLIuMHF6IOoTakB3K+RWxwtsHNRxllEoA6vPwP9o4866g6YWDLUdnucilZhmkxiHwHr11gAENw+QA== -node-fetch@^2.6.12: - version "2.6.12" - resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.12.tgz#02eb8e22074018e3d5a83016649d04df0e348fba" - integrity sha512-C/fGU2E8ToujUivIO0H+tpQ6HWo4eEmchoPIoXtxCrVghxdKq+QOHqEZW7tuP3KlV3bC8FRMO5nMCC7Zm1VP6g== - dependencies: - whatwg-url "^5.0.0" - node-gyp-build@^4.2.0: version "4.4.0" resolved "https://registry.npmjs.org/node-gyp-build/-/node-gyp-build-4.4.0.tgz" @@ -3422,11 +3406,6 @@ to-regex-range@^5.0.1: dependencies: is-number "^7.0.0" -tr46@~0.0.3: - version "0.0.3" - resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" - integrity sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw== - tree-kill@^1.2.2: version "1.2.2" resolved "https://registry.npmjs.org/tree-kill/-/tree-kill-1.2.2.tgz" @@ -3614,10 +3593,10 @@ util-deprecate@^1.0.1: resolved "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz" integrity sha1-RQ1Nyfpw3nMnYvvS1KKJgUGaDM8= -uuid@^9.0.0: - version "9.0.0" - resolved "https://registry.yarnpkg.com/uuid/-/uuid-9.0.0.tgz#592f550650024a38ceb0c562f2f6aa435761efb5" - integrity sha512-MXcSTerfPa4uqyzStbRoTgt5XIe3x5+42+q1sDuy3R5MDk66URdLMOZe5aPX/SQd+kuYAh0FdP/pO28IkQyTeg== +uuid@^9.0.1: + version "9.0.1" + resolved "https://registry.yarnpkg.com/uuid/-/uuid-9.0.1.tgz#e188d4c8853cc722220392c424cd637f32293f30" + integrity sha512-b+1eJOlsR9K8HJpow9Ok3fiWOWSIcIzXodvv0rQjVoOVNpWMpxf1wZNpt4y9h10odCNrqnYp1OBzRktckBe3sA== v8-compile-cache-lib@^3.0.0: version "3.0.0" @@ -3652,19 +3631,6 @@ web3-utils@^1.3.4: randombytes "^2.1.0" utf8 "3.0.0" -webidl-conversions@^3.0.0: - version "3.0.1" - resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-3.0.1.tgz#24534275e2a7bc6be7bc86611cc16ae0a5654871" - integrity sha512-2JAn3z8AR6rjK8Sm8orRC0h/bcl/DqL7tRPdGZ4I1CjdF+EaMLmYxBHyXuKL849eucPFhvBoxMsflfOb8kxaeQ== - -whatwg-url@^5.0.0: - version "5.0.0" - resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-5.0.0.tgz#966454e8765462e37644d3626f6742ce8b70965d" - integrity sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw== - dependencies: - tr46 "~0.0.3" - webidl-conversions "^3.0.0" - which@2.0.2, which@^2.0.1: version "2.0.2" resolved "https://registry.npmjs.org/which/-/which-2.0.2.tgz" @@ -3786,7 +3752,7 @@ yocto-queue@^0.1.0: resolved "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== -zod@^3.20.2: - version "3.21.4" - resolved "https://registry.yarnpkg.com/zod/-/zod-3.21.4.tgz#10882231d992519f0a10b5dd58a38c9dabbb64db" - integrity sha512-m46AKbrzKVzOzs/DZgVnG5H55N1sv1M8qZU3A8RIKbs3mrACDNeIOeilDymVb2HdmP8uwshOCF4uJ8uM9rCqJw== +zod@^3.22.3: + version "3.22.4" + resolved "https://registry.yarnpkg.com/zod/-/zod-3.22.4.tgz#f31c3a9386f61b1f228af56faa9255e845cf3fff" + integrity sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==