From 831fd200912c39ec96061395098c11a88471533e Mon Sep 17 00:00:00 2001 From: aboudjem Date: Fri, 13 Oct 2023 15:28:41 +0400 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=93=9D=20=20Add=20Natspec=20comments?= =?UTF-8?q?=20for=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/smart-account/base/Executor.sol | 9 ++++++++ .../smart-account/base/FallbackManager.sol | 9 ++++---- .../smart-account/base/ModuleManager.sol | 18 ++++++++++++++-- .../smart-account/common/ReentrancyGuard.sol | 18 +++++++++++++++- .../common/SecuredTokenTransfer.sol | 15 ++++++++----- .../smart-account/common/SelfAuthorized.sol | 16 ++++++++++++-- .../smart-account/common/SignatureDecoder.sol | 12 +++++++++-- contracts/smart-account/common/Stakeable.sol | 17 +++++++++++++++ contracts/smart-account/deployer/Create3.sol | 13 ++++++++---- contracts/smart-account/deployer/Deployer.sol | 10 +++++++++ .../handler/DefaultCallbackHandler.sol | 21 +++++++++++++++++++ .../modules/AuthorizationModulesConstants.sol | 8 +++++++ .../modules/BaseAuthorizationModule.sol | 11 +++++----- ...aEthSignSupportOwnershipRegistryModule.sol | 14 +++++++++++++ .../modules/MultichainECDSAValidator.sol | 15 ++++--------- .../modules/PasskeyRegistryModule.sol | 12 +++++++++++ .../PasskeyValidationModules/Secp256r1.sol | 21 ++++++++++++------- .../ERC20SessionValidationModule.sol | 1 - 18 files changed, 194 insertions(+), 46 deletions(-) create mode 100644 contracts/smart-account/modules/AuthorizationModulesConstants.sol diff --git a/contracts/smart-account/base/Executor.sol b/contracts/smart-account/base/Executor.sol index a8b9d300c..597565ef1 100644 --- a/contracts/smart-account/base/Executor.sol +++ b/contracts/smart-account/base/Executor.sol @@ -6,6 +6,15 @@ import {Enum} from "../common/Enum.sol"; /// @title Executor - A contract that can execute transactions abstract contract Executor is IExecutor { + /** + * @notice Executes a given operation (either Call or DelegateCall) to a specified address with provided data. + * @param to The address to which the operation should be executed. + * @param value The amount of ether (in wei) to send with the call (only for Call operations). + * @param data The call data to send with the operation. + * @param operation The type of operation to execute (either Call or DelegateCall). + * @param txGas The amount of gas to use for the operation. + * @return success A boolean indicating whether the operation was successful. + */ function _execute( address to, uint256 value, diff --git a/contracts/smart-account/base/FallbackManager.sol b/contracts/smart-account/base/FallbackManager.sol index 883e4edcf..f0f9aa493 100644 --- a/contracts/smart-account/base/FallbackManager.sol +++ b/contracts/smart-account/base/FallbackManager.sol @@ -61,16 +61,17 @@ abstract contract FallbackManager is SelfAuthorized, IFallbackManager { } } + /** + * @notice Sets a new fallback handler. This function will revert if the provided handler address is zero. + * @dev This function is internal and utilizes assembly for optimized storage operations. + * @param handler The address of the new fallback handler. + */ function _setFallbackHandler(address handler) internal { if (handler == address(0)) revert HandlerCannotBeZero(); address previousHandler; assembly { previousHandler := sload(FALLBACK_HANDLER_STORAGE_SLOT) - //} - //bytes32 slot = FALLBACK_HANDLER_STORAGE_SLOT; - - //assembly { sstore(FALLBACK_HANDLER_STORAGE_SLOT, handler) } emit ChangedFallbackHandler(previousHandler, handler); diff --git a/contracts/smart-account/base/ModuleManager.sol b/contracts/smart-account/base/ModuleManager.sol index 124c93865..bf956e611 100644 --- a/contracts/smart-account/base/ModuleManager.sol +++ b/contracts/smart-account/base/ModuleManager.sol @@ -215,8 +215,14 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager { emit DisabledModule(module); } - // TODO: can use not executor.execute, but SmartAccount._call for the unification - + /** + * @notice Executes an operation from a module, emits specific events based on the result. + * @param to The address to which the operation should be executed. + * @param value The amount of ether (in wei) to send with the call (only for Call operations). + * @param data The call data to send with the operation. + * @param operation The type of operation to execute (either Call or DelegateCall). + * @return success A boolean indicating whether the operation was successful. + */ function _executeFromModule( address to, uint256 value, @@ -259,6 +265,14 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager { return initialAuthorizationModule; } + /** + * @notice Sets up a new module by calling a specified setup contract with provided data. + * The function will revert if the setupContract address is zero or if the setup call fails. + * @dev This function is internal and utilizes assembly for low-level call operations and error handling. + * @param setupContract The address of the contract that will be called to set up the module. + * @param setupData The call data to send to the setup contract. + * @return module The address of the newly set up module. + */ function _setupModule( address setupContract, bytes memory setupData diff --git a/contracts/smart-account/common/ReentrancyGuard.sol b/contracts/smart-account/common/ReentrancyGuard.sol index 65fe72249..3be860590 100644 --- a/contracts/smart-account/common/ReentrancyGuard.sol +++ b/contracts/smart-account/common/ReentrancyGuard.sol @@ -1,15 +1,25 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity 0.8.17; -/// @title Reentrancy Guard - reentrancy protection +/** + * @title ReentrancyGuard + * @notice Provides a contract-level guard against reentrancy attacks. + * @dev Uses a single contract-wide status flag for efficiency. + * Use the `nonReentrant` modifier on functions to protect them. + */ abstract contract ReentrancyGuard { uint256 private constant NOT_ENTERED = 1; uint256 private constant ENTERED = 2; uint256 private _reentrancyStatus; + /// @notice Custom error to denote that reentrancy protection has been activated. error ReentrancyProtectionActivated(); + /** + * @notice Modifier to prevent a contract from calling itself, directly or indirectly. + * @dev Checks if the function has been re-entered, and if so, reverts with a custom error. + */ modifier nonReentrant() { if (_reentrancyStatus == ENTERED) revert ReentrancyProtectionActivated(); @@ -18,10 +28,16 @@ abstract contract ReentrancyGuard { _reentrancyStatus = NOT_ENTERED; } + /// @notice Initializes the `ReentrancyGuard` contract, setting the reentrancy status to `NOT_ENTERED`. constructor() { _reentrancyStatus = NOT_ENTERED; } + /** + * @notice Checks if the reentrancy guard is currently activated. + * @dev Returns true if the guard is activated, false otherwise. + * @return A boolean indicating whether the reentrancy guard is activated. + */ function _isReentrancyGuardEntered() internal view returns (bool) { return _reentrancyStatus == ENTERED; } diff --git a/contracts/smart-account/common/SecuredTokenTransfer.sol b/contracts/smart-account/common/SecuredTokenTransfer.sol index b9a61d630..109c0a9e9 100644 --- a/contracts/smart-account/common/SecuredTokenTransfer.sol +++ b/contracts/smart-account/common/SecuredTokenTransfer.sol @@ -3,10 +3,15 @@ pragma solidity 0.8.17; /// @title SecuredTokenTransfer - Secure token transfer abstract contract SecuredTokenTransfer { - /// @dev Transfers a token and returns if it was a success - /// @param token Token that should be transferred - /// @param receiver Receiver to whom the token should be transferred - /// @param amount The amount of tokens that should be transferred + /** + * @dev Transfers a specified amount of ERC20 tokens to a receiver. + * @notice This function utilizes the standard `transfer` function of ERC20 tokens. + * It ensures the token address is valid and that the token contract exists before attempting the transfer. + * @param token The address of the ERC20 token to be transferred. + * @param receiver The address to receive the tokens. + * @param amount The amount of tokens to transfer. + * @return transferred A boolean indicating whether the transfer was successful. + */ function _transferToken( address token, address receiver, @@ -23,7 +28,7 @@ abstract contract SecuredTokenTransfer { assembly { // We write the return value to scratch space. - // See https://docs.soliditylang.org/en/v0.8.17/internals/layout_in_memory.html#layout-in-memory + // See https://docs.soliditylang.org/en/latest/internals/layout_in_memory.html#layout-in-memory let success := call( sub(gas(), 10000), token, diff --git a/contracts/smart-account/common/SelfAuthorized.sol b/contracts/smart-account/common/SelfAuthorized.sol index d59056730..584d625c9 100644 --- a/contracts/smart-account/common/SelfAuthorized.sol +++ b/contracts/smart-account/common/SelfAuthorized.sol @@ -3,14 +3,26 @@ pragma solidity 0.8.17; import {ISelfAuthorized} from "../interfaces/common/ISelfAuthorized.sol"; -/// @title SelfAuthorized - authorizes current contract to perform actions +/** + * @title SelfAuthorized + * @notice This contract provides a modifier to ensure that only the contract itself can call certain functions. + * @dev Functions with the `authorized` modifier can only be called by the contract itself. + * This can be useful for security purposes or to ensure a specific call flow. + */ contract SelfAuthorized is ISelfAuthorized { + /** + * @notice Modifier to ensure a function is only callable by the contract itself. + * @dev Checks if the caller is the current contract. If not, reverts. + */ modifier authorized() { - // This is a function call as it minimized the bytecode size _requireSelfCall(); _; } + /** + * @dev Internal function to check if the caller is the current contract. + * @dev If the caller isn't the contract, it reverts with a specific error. + */ function _requireSelfCall() private view { if (msg.sender != address(this)) revert CallerIsNotSelf(msg.sender); } diff --git a/contracts/smart-account/common/SignatureDecoder.sol b/contracts/smart-account/common/SignatureDecoder.sol index cb10e4a1f..a6a0332a6 100644 --- a/contracts/smart-account/common/SignatureDecoder.sol +++ b/contracts/smart-account/common/SignatureDecoder.sol @@ -3,8 +3,16 @@ pragma solidity 0.8.17; /// @title SignatureDecoder - Decodes signatures that a encoded as bytes abstract contract SignatureDecoder { - /// @dev divides bytes signature into `uint8 v, bytes32 r, bytes32 s`. - /// @param signature concatenated rsv signatures + /** + * @dev Splits a given signature into its `r`, `s`, and `v` components. + * @notice The signature is assumed to be in the compact format: + * r (32 bytes) + s (32 bytes) + v (1 byte). + * This function uses assembly for efficient memory operations. + * @param signature The signature bytes. + * @return v The `v` component of the signature. + * @return r The `r` component of the signature as bytes32. + * @return s The `s` component of the signature as bytes32. + */ function _signatureSplit( bytes memory signature ) internal pure returns (uint8 v, bytes32 r, bytes32 s) { diff --git a/contracts/smart-account/common/Stakeable.sol b/contracts/smart-account/common/Stakeable.sol index 213f79f1c..28e7ee196 100644 --- a/contracts/smart-account/common/Stakeable.sol +++ b/contracts/smart-account/common/Stakeable.sol @@ -14,6 +14,12 @@ contract Stakeable is Ownable, IStakeable { _transferOwnership(_newOwner); } + /** + * @dev Stakes a certain amount of Ether on an EntryPoint. + * @notice The contract should have enough Ether to cover the stake. + * @param epAddress Address of the EntryPoint where the stake is added. + * @param unstakeDelaySec The delay in seconds before the stake can be unlocked. + */ function addStake( address epAddress, uint32 unstakeDelaySec @@ -22,11 +28,22 @@ contract Stakeable is Ownable, IStakeable { IEntryPoint(epAddress).addStake{value: msg.value}(unstakeDelaySec); } + /** + * @dev Unlocks the stake on an EntryPoint. + * @notice This starts the unstaking delay after which funds can be withdrawn. + * @param epAddress Address of the EntryPoint where the stake is unlocked. + */ function unlockStake(address epAddress) external override onlyOwner { require(epAddress != address(0), "Invalid EP address"); IEntryPoint(epAddress).unlockStake(); } + /** + * @dev Withdraws the stake from an EntryPoint to a specified address. + * @notice This can only be done after the unstaking delay has passed since the unlock. + * @param epAddress Address of the EntryPoint where the stake is withdrawn from. + * @param withdrawAddress Address to receive the withdrawn stake. + */ function withdrawStake( address epAddress, address payable withdrawAddress diff --git a/contracts/smart-account/deployer/Create3.sol b/contracts/smart-account/deployer/Create3.sol index 1bd089c17..731dc4ebb 100644 --- a/contracts/smart-account/deployer/Create3.sol +++ b/contracts/smart-account/deployer/Create3.sol @@ -89,6 +89,11 @@ library Create3 { if (!success || codeSize(addr) == 0) revert ErrorCreatingContract(); } + /** + * @dev Computes the CREATE2 proxy address using the provided salt. + * @param _salt The salt used to derive the proxy contract address. + * @return Address of the proxy contract derived using CREATE2. + */ function addressOfProxy(bytes32 _salt) internal view returns (address) { return address( @@ -129,10 +134,10 @@ library Create3 { } /** - @notice Returns the size of the code on a given address - @param _addr Address that may or may not contain code - @return size of the code on the given `_addr` - */ + * @dev Returns the size of the code stored at a specific address. + * @param _addr The address to check. + * @return size The size of the code stored at the given address. + */ function codeSize(address _addr) internal view returns (uint256 size) { assembly { size := extcodesize(_addr) diff --git a/contracts/smart-account/deployer/Deployer.sol b/contracts/smart-account/deployer/Deployer.sol index 7571f009d..9d9926272 100644 --- a/contracts/smart-account/deployer/Deployer.sol +++ b/contracts/smart-account/deployer/Deployer.sol @@ -6,11 +6,21 @@ import "./Create3.sol"; contract Deployer { event ContractDeployed(address indexed contractAddress); + /** + * @dev Deploys a new contract using the Create3 library with a specific salt and initialization code. + * @param _salt The salt used to derive the deployed contract address. + * @param _creationCode The bytecode used to initialize and deploy the contract. + */ function deploy(bytes32 _salt, bytes calldata _creationCode) external { address deployedContract = Create3.create3(_salt, _creationCode); emit ContractDeployed(deployedContract); } + /** + * @dev Computes the final deployed address using the Create3 library and a given salt. + * @param _salt The salt used in the original Create3 deployment. + * @return Address of the final deployed contract. + */ function addressOf(bytes32 _salt) external view returns (address) { return Create3.addressOf(_salt); } diff --git a/contracts/smart-account/handler/DefaultCallbackHandler.sol b/contracts/smart-account/handler/DefaultCallbackHandler.sol index 27b205372..911de8cc3 100644 --- a/contracts/smart-account/handler/DefaultCallbackHandler.sol +++ b/contracts/smart-account/handler/DefaultCallbackHandler.sol @@ -21,6 +21,11 @@ contract DefaultCallbackHandler is string public constant NAME = "Default Callback Handler"; string public constant VERSION = "1.0.0"; + /** + * @dev Checks if the contract supports a given interface. + * @param interfaceId The interface identifier, as specified in ERC-165. + * @return True if the contract implements the given interface, false otherwise. + */ function supportsInterface( bytes4 interfaceId ) external view virtual override returns (bool) { @@ -31,6 +36,10 @@ contract DefaultCallbackHandler is interfaceId == type(IERC165).interfaceId; } + /** + * @dev Handles the receipt of a single ERC1155 token type. + * @return The interface selector for the called function. + */ function onERC1155Received( address, address, @@ -41,6 +50,10 @@ contract DefaultCallbackHandler is return IERC1155Receiver.onERC1155Received.selector; } + /** + * @dev Handles the receipt of multiple ERC1155 token types. + * @return The interface selector for the called function. + */ function onERC1155BatchReceived( address, address, @@ -51,6 +64,10 @@ contract DefaultCallbackHandler is return IERC1155Receiver.onERC1155BatchReceived.selector; } + /** + * @dev Handles the receipt of an ERC721 token. + * @return The interface selector for the called function. + */ function onERC721Received( address, address, @@ -60,6 +77,10 @@ contract DefaultCallbackHandler is return IERC721Receiver.onERC721Received.selector; } + /** + * @dev Handles the receipt of an ERC777 token. + * This function does not have any specific logic as it's implemented for completeness. + */ function tokensReceived( address, address, diff --git a/contracts/smart-account/modules/AuthorizationModulesConstants.sol b/contracts/smart-account/modules/AuthorizationModulesConstants.sol new file mode 100644 index 000000000..161c856ca --- /dev/null +++ b/contracts/smart-account/modules/AuthorizationModulesConstants.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.17; + +/// @dev Contract containing constants related to authorization module results. +contract AuthorizationModulesConstants { + uint256 internal constant VALIDATION_SUCCESS = 0; + uint256 internal constant SIG_VALIDATION_FAILED = 1; +} diff --git a/contracts/smart-account/modules/BaseAuthorizationModule.sol b/contracts/smart-account/modules/BaseAuthorizationModule.sol index 578f33d05..64838799a 100644 --- a/contracts/smart-account/modules/BaseAuthorizationModule.sol +++ b/contracts/smart-account/modules/BaseAuthorizationModule.sol @@ -4,13 +4,12 @@ pragma solidity 0.8.17; /* solhint-disable no-empty-blocks */ import {IBaseAuthorizationModule} from "../interfaces/modules/IBaseAuthorizationModule.sol"; +import {AuthorizationModulesConstants} from "./AuthorizationModulesConstants.sol"; -contract AuthorizationModulesConstants { - uint256 internal constant VALIDATION_SUCCESS = 0; - uint256 internal constant SIG_VALIDATION_FAILED = 1; -} - +/// @dev Base contract for authorization modules abstract contract BaseAuthorizationModule is IBaseAuthorizationModule, AuthorizationModulesConstants -{} +{ + +} diff --git a/contracts/smart-account/modules/Exotic/EcdsaEthSignSupportOwnershipRegistryModule.sol b/contracts/smart-account/modules/Exotic/EcdsaEthSignSupportOwnershipRegistryModule.sol index 378f57b31..2642f03cc 100644 --- a/contracts/smart-account/modules/Exotic/EcdsaEthSignSupportOwnershipRegistryModule.sol +++ b/contracts/smart-account/modules/Exotic/EcdsaEthSignSupportOwnershipRegistryModule.sol @@ -161,6 +161,20 @@ contract EcdsaWithEthSignSupportOwnershipRegistryModule is return size > 0; } + /** + * @dev Splits a signature into its `r`, `s`, and `v` components. + * + * The signature format is expected to be: + * {bytes32 r}{bytes32 s}{uint8 v} + * Where the uint8 is not padded to 32 bytes. + * + * This function uses inline assembly for optimized extraction of the components. + * + * @param signature The concatenated Ethereum signature. + * @return v The recovery id. + * @return r The `r` component of the signature. + * @return s The `s` component of the signature. + */ function _signatureSplit( bytes memory signature ) internal pure returns (uint8 v, bytes32 r, bytes32 s) { diff --git a/contracts/smart-account/modules/MultichainECDSAValidator.sol b/contracts/smart-account/modules/MultichainECDSAValidator.sol index 9a8fdc56e..3749f6642 100644 --- a/contracts/smart-account/modules/MultichainECDSAValidator.sol +++ b/contracts/smart-account/modules/MultichainECDSAValidator.sol @@ -32,7 +32,6 @@ contract MultichainECDSAValidator is EcdsaOwnershipRegistryModule { * @param userOp user operation to be validated * @param userOpHash hash of the userOp provided by the EP */ - function validateUserOp( UserOperation calldata userOp, bytes32 userOpHash @@ -43,13 +42,13 @@ contract MultichainECDSAValidator is EcdsaOwnershipRegistryModule { ); address sender; - //read sender from userOp, which is first userOp member (saves gas) + // read sender from userOp, which is first userOp member (saves gas) assembly { sender := calldataload(userOp) } if (moduleSignature.length == 65) { - //it's not a multichain signature + // it's not a multichain signature return _verifySignature( userOpHash, @@ -60,7 +59,7 @@ contract MultichainECDSAValidator is EcdsaOwnershipRegistryModule { : SIG_VALIDATION_FAILED; } - //otherwise it is a multichain signature + // otherwise it is a multichain signature ( uint48 validUntil, uint48 validAfter, @@ -72,7 +71,7 @@ contract MultichainECDSAValidator is EcdsaOwnershipRegistryModule { (uint48, uint48, bytes32, bytes32[], bytes) ); - //make a leaf out of userOpHash, validUntil and validAfter + // make a leaf out of userOpHash, validUntil and validAfter bytes32 leaf = keccak256( abi.encodePacked(validUntil, validAfter, userOpHash) ); @@ -94,10 +93,4 @@ contract MultichainECDSAValidator is EcdsaOwnershipRegistryModule { ) : SIG_VALIDATION_FAILED; } - - /** - * Inherits isValideSignature method from EcdsaOwnershipRegistryModule - * isValidSignature is intended to work not with a multichain signature - * but with a regular ecdsa signature over a message hash - */ } diff --git a/contracts/smart-account/modules/PasskeyRegistryModule.sol b/contracts/smart-account/modules/PasskeyRegistryModule.sol index ab6efe18d..4c2fb6443 100644 --- a/contracts/smart-account/modules/PasskeyRegistryModule.sol +++ b/contracts/smart-account/modules/PasskeyRegistryModule.sol @@ -74,6 +74,12 @@ contract PasskeyRegistryModule is return bytes4(0xffffffff); } + /** + * @dev Internal utility function to verify a signature. + * @param userOpDataHash The hash of the user operation data. + * @param moduleSignature The signature provided by the module. + * @return True if the signature is valid, false otherwise. + */ function _verifySignature( bytes32 userOpDataHash, bytes memory moduleSignature @@ -108,6 +114,12 @@ contract PasskeyRegistryModule is return Secp256r1.verify(passKey, sigx, sigy, uint256(sigHash)); } + /** + * @dev Internal function to validate a user operation signature. + * @param userOp The user operation to validate. + * @param userOpHash The hash of the user operation. + * @return sigValidationResult Returns 0 if the signature is valid, and SIG_VALIDATION_FAILED otherwise. + */ function _validateSignature( UserOperation calldata userOp, bytes32 userOpHash diff --git a/contracts/smart-account/modules/PasskeyValidationModules/Secp256r1.sol b/contracts/smart-account/modules/PasskeyValidationModules/Secp256r1.sol index 197673aaf..abf508ef7 100644 --- a/contracts/smart-account/modules/PasskeyValidationModules/Secp256r1.sol +++ b/contracts/smart-account/modules/PasskeyValidationModules/Secp256r1.sol @@ -126,9 +126,8 @@ library Secp256r1 { return (x, y); } - /* affineFromJacobian - * @desription returns affine coordinates from a jacobian input follows - * golang elliptic/crypto library + /** + * @notice Returns affine coordinates from a jacobian input. Follows the golang elliptic/crypto library convention. */ function affineFromJacobian( uint256 x, @@ -146,10 +145,13 @@ library Secp256r1 { ay = mulmod(y, mulmod(zinvsq, zinv, PP), PP); } - // Fermats little theorem https://en.wikipedia.org/wiki/Fermat%27s_little_theorem - // a^(p-1) = 1 mod p - // a^(-1) ≅ a^(p-2) (mod p) - // we then use the precompile bigModExp to compute a^(-1) + /** + * @notice Computes a^(-1) mod p using Fermat's Little Theorem + * https://en.wikipedia.org/wiki/Fermat%27s_little_theorem + * - a^(p-1) = 1 mod p + * - a^(-1) ≅ a^(p-2) (mod p) + * Uses the precompiled bigModExp to compute a^(-1). + */ function primemod( uint256 value, uint256 p @@ -158,7 +160,10 @@ library Secp256r1 { return ret; } - // Wrapper for built-in BigNumber_modexp (contract 0x5) as described here. https://github.com/ethereum/EIPs/pull/198 + /** + * @notice Wrapper function for built-in BigNumber_modexp (contract 0x5) as described here: + * - https://github.com/ethereum/EIPs/pull/198 + */ function modexp( uint256 _base, uint256 _exp, diff --git a/contracts/smart-account/modules/SessionValidationModules/ERC20SessionValidationModule.sol b/contracts/smart-account/modules/SessionValidationModules/ERC20SessionValidationModule.sol index 333fe39d1..2da48645b 100644 --- a/contracts/smart-account/modules/SessionValidationModules/ERC20SessionValidationModule.sol +++ b/contracts/smart-account/modules/SessionValidationModules/ERC20SessionValidationModule.sol @@ -12,7 +12,6 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; * * @author Fil Makarov - */ - contract ERC20SessionValidationModule is ISessionValidationModule { /** * @dev validates that the call (destinationContract, callValue, funcCallData) From d87d655a706d54bcd715b70ba6f0bb641c0f7ad4 Mon Sep 17 00:00:00 2001 From: aboudjem Date: Fri, 13 Oct 2023 17:38:12 +0400 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=99=88=20Remove=20ignore=20linting=20?= =?UTF-8?q?rules?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../modules/BatchedSessionRouterModule.sol | 3 +-- .../smart-account/modules/ForwardFlowModule.sol | 13 ++++--------- .../test/upgrades/BaseSmartAccountNew.sol | 4 ---- .../test/upgrades/v1/BaseSmartAccountV1.sol | 4 ---- .../test/upgrades/v1/FallbackManagerV1.sol | 1 - .../test/upgrades/v1/SmartAccountV1.sol | 4 ++-- 6 files changed, 7 insertions(+), 22 deletions(-) diff --git a/contracts/smart-account/modules/BatchedSessionRouterModule.sol b/contracts/smart-account/modules/BatchedSessionRouterModule.sol index f4c7da298..d38efa4f9 100644 --- a/contracts/smart-account/modules/BatchedSessionRouterModule.sol +++ b/contracts/smart-account/modules/BatchedSessionRouterModule.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; -/* solhint-disable function-max-lines,no-unused-import */ +/* solhint-disable function-max-lines */ import {BaseAuthorizationModule} from "./BaseAuthorizationModule.sol"; import {ISessionValidationModule} from "../interfaces/modules/ISessionValidationModule.sol"; @@ -11,7 +11,6 @@ import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.s import {UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol"; import {IBatchedSessionRouterModule} from "../interfaces/modules/IBatchedSessionRouterModule.sol"; import {IAuthorizationModule} from "../interfaces/IAuthorizationModule.sol"; -import {ISignatureValidator} from "../interfaces/ISignatureValidator.sol"; /** * @title Batched Session Router diff --git a/contracts/smart-account/modules/ForwardFlowModule.sol b/contracts/smart-account/modules/ForwardFlowModule.sol index b91f25fb9..8dcf21920 100644 --- a/contracts/smart-account/modules/ForwardFlowModule.sol +++ b/contracts/smart-account/modules/ForwardFlowModule.sol @@ -6,8 +6,6 @@ import {Enum} from "../common/Enum.sol"; import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; -/* solhint-disable function-max-lines */ - struct Transaction { address to; Enum.Operation operation; @@ -248,11 +246,9 @@ contract ForwardFlowModule is ReentrancyGuard { } } - // We require some gas to emit the events (at least 2500) after the execution and some to - // perform code until the execution (7500 = call the external function + checks inside it) - // We also include the 1/64 in the check that is not send along with a call to counteract - // potential shortings because of EIP-150 - // Bitshift left 6 bits means multiplying by 64, just more gas efficient + // We need gas for events post-execution (~2500) and for code pre-execution (~7500: external call + checks). + // Including 1/64th for EIP-150 to address potential gas shortfalls. + // Bitshift left 6 bits (multiply by 64) is used for gas efficiency. if ( gasleft() < Math.max((_tx.targetTxGas << 6) / 63, _tx.targetTxGas + 2500) + 7500 @@ -285,9 +281,8 @@ contract ForwardFlowModule is ReentrancyGuard { } // Transfer transaction costs to tx.origin to avoid intermediate contract payments. - uint256 payment; if (refundInfo.gasPrice != 0) { - payment = _handlePayment( + uint256 payment = _handlePayment( smartAccount, startGas - gasleft(), refundInfo.baseGas, diff --git a/contracts/smart-account/test/upgrades/BaseSmartAccountNew.sol b/contracts/smart-account/test/upgrades/BaseSmartAccountNew.sol index 5e17690c8..8bc47f6ed 100644 --- a/contracts/smart-account/test/upgrades/BaseSmartAccountNew.sol +++ b/contracts/smart-account/test/upgrades/BaseSmartAccountNew.sol @@ -1,10 +1,6 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity 0.8.17; -/* solhint-disable avoid-low-level-calls */ -/* solhint-disable no-inline-assembly */ -/* solhint-disable reason-string */ - import {IAccount} from "@account-abstraction/contracts/interfaces/IAccount.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {UserOperationLib, UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol"; diff --git a/contracts/smart-account/test/upgrades/v1/BaseSmartAccountV1.sol b/contracts/smart-account/test/upgrades/v1/BaseSmartAccountV1.sol index 1b938bba4..8fdcb9582 100644 --- a/contracts/smart-account/test/upgrades/v1/BaseSmartAccountV1.sol +++ b/contracts/smart-account/test/upgrades/v1/BaseSmartAccountV1.sol @@ -1,10 +1,6 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity 0.8.17; -/* solhint-disable avoid-low-level-calls */ -/* solhint-disable no-inline-assembly */ -/* solhint-disable reason-string */ - import {IAccount} from "@account-abstraction/contracts/interfaces/IAccount.sol"; import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol"; import {UserOperationLib, UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol"; diff --git a/contracts/smart-account/test/upgrades/v1/FallbackManagerV1.sol b/contracts/smart-account/test/upgrades/v1/FallbackManagerV1.sol index b46513ba3..8a9bb392e 100644 --- a/contracts/smart-account/test/upgrades/v1/FallbackManagerV1.sol +++ b/contracts/smart-account/test/upgrades/v1/FallbackManagerV1.sol @@ -14,7 +14,6 @@ abstract contract FallbackManagerV1 is SelfAuthorized, IFallbackManager { bytes32 internal constant FALLBACK_HANDLER_STORAGE_SLOT = 0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d4; - // solhint-disable-next-line payable-fallback,no-complex-fallback fallback() external { bytes32 slot = FALLBACK_HANDLER_STORAGE_SLOT; diff --git a/contracts/smart-account/test/upgrades/v1/SmartAccountV1.sol b/contracts/smart-account/test/upgrades/v1/SmartAccountV1.sol index c71bcec19..717790529 100644 --- a/contracts/smart-account/test/upgrades/v1/SmartAccountV1.sol +++ b/contracts/smart-account/test/upgrades/v1/SmartAccountV1.sol @@ -331,7 +331,7 @@ contract SmartAccountV1 is address payable refundReceiver ) private returns (uint256 payment) { if (tokenGasPriceFactor == 0) revert TokenGasPriceFactorCanNotBeZero(); - // solhint-disable-next-line avoid-tx-origin + address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver; @@ -378,7 +378,7 @@ contract SmartAccountV1 is ) external returns (uint256 requiredGas) { require(tokenGasPriceFactor != 0, "invalid tokenGasPriceFactor"); uint256 startGas = gasleft(); - // solhint-disable-next-line avoid-tx-origin + address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver; From 59f886131be0dee0c881fdc01f4039d6c915f284 Mon Sep 17 00:00:00 2001 From: aboudjem Date: Fri, 13 Oct 2023 17:38:48 +0400 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=93=9D=20add=20natspec=20on=20gasesti?= =?UTF-8?q?mator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/smart-account/utils/GasEstimator.sol | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/contracts/smart-account/utils/GasEstimator.sol b/contracts/smart-account/utils/GasEstimator.sol index 1f787c848..9307728d6 100644 --- a/contracts/smart-account/utils/GasEstimator.sol +++ b/contracts/smart-account/utils/GasEstimator.sol @@ -3,14 +3,21 @@ pragma solidity 0.8.17; // Generic contract for estimating gas on any target and data contract GasEstimator { + /** + * @notice Estimates the gas consumption of a call to a given address with specific data. + * @dev This function does not revert if the call fails but instead returns the success status. + * @param _to The address to call. + * @param _data The calldata to send with the call. + * @return success A boolean indicating if the call was successful. + * @return result The bytes data returned from the called function. + * @return gas The amount of gas consumed by the call. + */ function estimate( address _to, bytes calldata _data ) external returns (bool success, bytes memory result, uint256 gas) { - // solhint-disable uint256 initialGas = gasleft(); (success, result) = _to.call(_data); gas = initialGas - gasleft(); - // solhint-enable } } From 935a3964dd2169fe7fe6c3fa36b7aed055bf9075 Mon Sep 17 00:00:00 2001 From: aboudjem Date: Fri, 13 Oct 2023 17:47:39 +0400 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=93=A6=20remove=20redundant=20dotenv?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 93eccba15..359d986e2 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,6 @@ "@typescript-eslint/eslint-plugin": "^5.30.5", "@typescript-eslint/parser": "^5.30.5", "chai": "^4.3.7", - "dotenv": "^16.0.1", "merkletreejs": "^0.3.9", "ts-node": "^10.9.1", "typechain": "^8.1.1", From 3941e63f48ab518771365fdb045e588e521e3674 Mon Sep 17 00:00:00 2001 From: aboudjem Date: Fri, 13 Oct 2023 17:56:07 +0400 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=94=A5=20remove=20ISignatureValidator?= =?UTF-8?q?=20reference?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/smart-account/modules/BatchedSessionRouterModule.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/smart-account/modules/BatchedSessionRouterModule.sol b/contracts/smart-account/modules/BatchedSessionRouterModule.sol index d38efa4f9..efeb33d33 100644 --- a/contracts/smart-account/modules/BatchedSessionRouterModule.sol +++ b/contracts/smart-account/modules/BatchedSessionRouterModule.sol @@ -12,6 +12,7 @@ import {UserOperation} from "@account-abstraction/contracts/interfaces/UserOpera import {IBatchedSessionRouterModule} from "../interfaces/modules/IBatchedSessionRouterModule.sol"; import {IAuthorizationModule} from "../interfaces/IAuthorizationModule.sol"; + /** * @title Batched Session Router * @dev Built to process executeBatch and executeBatch_y6U calls @@ -129,7 +130,6 @@ contract BatchedSessionRouter is } /** - * @inheritdoc ISignatureValidator * @dev isValidSignature according to BaseAuthorizationModule * @param _dataHash Hash of the data to be validated. * @param _signature Signature over the the _dataHash.