diff --git a/src/access/AccessPoint.sol b/src/access/AccessPoint.sol index 659547ab8..877df1cf2 100644 --- a/src/access/AccessPoint.sol +++ b/src/access/AccessPoint.sol @@ -17,12 +17,7 @@ import "../interfaces/IAccessRegistry.sol"; /** * @title AccessPoint */ -contract AccessPoint is - IAccessPoint, - Initializable, - BaseAccount, - TokenCallbackHandler -{ +contract AccessPoint is IAccessPoint, Initializable, BaseAccount, TokenCallbackHandler { using UserOperationLib for UserOperation; using ECDSA for bytes32; @@ -69,13 +64,7 @@ contract AccessPoint is /* ============ View Functions ============ */ - function getNonce() - public - view - virtual - override(BaseAccount, IAccessPoint) - returns (uint256) - { + function getNonce() public view virtual override(BaseAccount, IAccessPoint) returns (uint256) { return super.getNonce(); } @@ -112,10 +101,7 @@ contract AccessPoint is /// @notice Executes a DELEGATECALL to the provided target with the provided data. /// @dev Shared logic between the constructor and the `execute` function. - function _execute(address target, bytes memory data) - internal - returns (bytes memory response) - { + function _execute(address target, bytes memory data) internal returns (bytes memory response) { if (!registry.isWorkflowAllowed(target)) { revert WorkflowUnauthorized(target); } @@ -147,13 +133,16 @@ contract AccessPoint is } /// @notice Valides that owner signed the user operation - function _validateSignature( - UserOperation calldata userOp, - bytes32 userOpHash - ) internal virtual override returns (uint256 validationData) { + function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash) + internal + virtual + override + returns (uint256 validationData) + { bytes32 hash = userOpHash.toEthSignedMessageHash(); - if (owner != ECDSA.recover(hash, userOp.signature)) + if (owner != ECDSA.recover(hash, userOp.signature)) { return SIG_VALIDATION_FAILED; + } return 0; } } diff --git a/src/access/AccessRegistry.sol b/src/access/AccessRegistry.sol index b81e65e5b..568ee1035 100644 --- a/src/access/AccessRegistry.sol +++ b/src/access/AccessRegistry.sol @@ -19,12 +19,7 @@ import "../proxy/SafeBeaconProxy.sol"; import "../interfaces/IAccessRegistry.sol"; -contract AccessRegistry is - Initializable, - UUPSUpgradeable, - OwnableUpgradeable, - IAccessRegistry -{ +contract AccessRegistry is Initializable, UUPSUpgradeable, OwnableUpgradeable, IAccessRegistry { /* ============ Constants ============ */ /* ============ State Variables ============ */ @@ -75,16 +70,9 @@ contract AccessRegistry is * @param newImpl The new implementation */ function upgradeAll(IAccessPoint newImpl) external override onlyOwner { - require( - address(newImpl) != address(0) && - address(newImpl) != beacon.implementation(), - "invalid address" - ); + require(address(newImpl) != address(0) && address(newImpl) != beacon.implementation(), "invalid address"); factoryVersion++; - emit AccessPointFactoryUpgraded( - beacon.implementation(), - address(newImpl) - ); + emit AccessPointFactoryUpgraded(beacon.implementation(), address(newImpl)); beacon.upgradeTo(address(newImpl)); } @@ -93,12 +81,7 @@ contract AccessRegistry is * @param newImplementation address of the new implementation */ // This function is called by the proxy contract when the factory is upgraded - function _authorizeUpgrade(address newImplementation) - internal - view - override - onlyOwner - { + function _authorizeUpgrade(address newImplementation) internal view override onlyOwner { (newImplementation); } @@ -110,29 +93,21 @@ contract AccessRegistry is } /// @inheritdoc IAccessRegistry - function getAccessPoint(address user) - external - view - returns (IAccessPoint accessPoint) - { + function getAccessPoint(address user) external view returns (IAccessPoint accessPoint) { accessPoint = _accessPoints[user]; } /// @inheritdoc IAccessRegistry function getAddress(address owner) public view override returns (address) { - return - Create2.computeAddress( - bytes32(abi.encodePacked(owner)), - keccak256( - abi.encodePacked( - type(SafeBeaconProxy).creationCode, - abi.encode( - address(beacon), - abi.encodeCall(IAccessPoint.initialize, (owner)) - ) - ) + return Create2.computeAddress( + bytes32(abi.encodePacked(owner)), + keccak256( + abi.encodePacked( + type(SafeBeaconProxy).creationCode, + abi.encode(address(beacon), abi.encodeCall(IAccessPoint.initialize, (owner))) ) - ); + ) + ); } /// @dev TODO:Should be removed. Added because Kinto EntryPoint needs this function. @@ -167,31 +142,19 @@ contract AccessRegistry is /* ============ Internal ============ */ /// @dev See `deploy`. - function _deploy(address owner) - internal - returns (IAccessPoint accessPoint) - { + function _deploy(address owner) internal returns (IAccessPoint accessPoint) { // Use the address of the owner as the CREATE2 salt. bytes32 salt = bytes32(abi.encodePacked(owner)); // Deploy the accessPoint with CREATE2. accessPoint = IAccessPoint( - payable( - new SafeBeaconProxy{salt: salt}( - address(beacon), - abi.encodeCall(IAccessPoint.initialize, (owner)) - ) - ) + payable(new SafeBeaconProxy{salt: salt}(address(beacon), abi.encodeCall(IAccessPoint.initialize, (owner)))) ); // Associate the owner and the accessPoint. _accessPoints[owner] = accessPoint; // Log the creation of the accessPoint. - emit DeployAccessPoint({ - operator: msg.sender, - owner: owner, - accessPoint: accessPoint - }); + emit DeployAccessPoint({operator: msg.sender, owner: owner, accessPoint: accessPoint}); } } diff --git a/src/access/workflows/WithdrawWorkflow.sol b/src/access/workflows/WithdrawWorkflow.sol index df85bc970..05892f0d7 100644 --- a/src/access/workflows/WithdrawWorkflow.sol +++ b/src/access/workflows/WithdrawWorkflow.sol @@ -24,7 +24,7 @@ contract WithdrawWorkflow { function withdrawNative(uint256 amount) external { address owner = _getOwner(); - (bool sent, ) = owner.call{value: amount}(""); + (bool sent,) = owner.call{value: amount}(""); if (!sent) { revert NativeWithdrawalFailed(); } diff --git a/src/interfaces/IAccessPoint.sol b/src/interfaces/IAccessPoint.sol index 44d0aa6c0..5c647296e 100644 --- a/src/interfaces/IAccessPoint.sol +++ b/src/interfaces/IAccessPoint.sol @@ -54,8 +54,5 @@ interface IAccessPoint { /// @param target The address of the target contract. /// @param data Function selector plus ABI encoded data. /// @return response The response received from the target contract, if any. - function execute(address target, bytes calldata data) - external - payable - returns (bytes memory response); + function execute(address target, bytes calldata data) external payable returns (bytes memory response); } diff --git a/src/interfaces/IAccessRegistry.sol b/src/interfaces/IAccessRegistry.sol index 06340b889..17ff4f6a9 100644 --- a/src/interfaces/IAccessRegistry.sol +++ b/src/interfaces/IAccessRegistry.sol @@ -22,23 +22,13 @@ interface IAccessRegistry { /* ============ Events ============ */ /// @notice Emitted when workflow is allowed or dissallowed. - event WorkflowStatusChanged( - address indexed workflow, - bool indexed status - ); + event WorkflowStatusChanged(address indexed workflow, bool indexed status); /// @notice Emitted when access point is upgraded. - event AccessPointFactoryUpgraded( - address indexed beacon, - address indexed accessPoint - ); + event AccessPointFactoryUpgraded(address indexed beacon, address indexed accessPoint); /// @notice Emitted when a new access point is deployed. - event DeployAccessPoint( - address indexed operator, - address indexed owner, - IAccessPoint accessPoint - ); + event DeployAccessPoint(address indexed operator, address indexed owner, IAccessPoint accessPoint); /* ============ Structs ============ */ @@ -49,10 +39,7 @@ interface IAccessRegistry { /// @notice Retrieves the accessPoint for the provided user. /// @param user The user address for the query. - function getAccessPoint(address user) - external - view - returns (IAccessPoint accessPoint); + function getAccessPoint(address user) external view returns (IAccessPoint accessPoint); /** * @dev Calculates the counterfactual address of this account as it would be returned by deploy() @@ -84,7 +71,5 @@ interface IAccessRegistry { /// /// @param user The address that will own the access point. /// @return accessPoint The address of the newly deployed access point. - function deployFor(address user) - external - returns (IAccessPoint accessPoint); + function deployFor(address user) external returns (IAccessPoint accessPoint); } diff --git a/src/paymasters/SignaturePaymaster.sol b/src/paymasters/SignaturePaymaster.sol index 9a44cccae..a2e4d8adf 100644 --- a/src/paymasters/SignaturePaymaster.sol +++ b/src/paymasters/SignaturePaymaster.sol @@ -32,9 +32,7 @@ contract SignaturePaymaster is BasePaymaster, Initializable, UUPSUpgradeable { // ========== Constructor & Upgrades ============ - constructor(IEntryPoint _entryPoint, address _verifyingSigner) - BasePaymaster(_entryPoint) - { + constructor(IEntryPoint _entryPoint, address _verifyingSigner) BasePaymaster(_entryPoint) { _disableInitializers(); verifyingSigner = _verifyingSigner; } @@ -53,22 +51,14 @@ contract SignaturePaymaster is BasePaymaster, Initializable, UUPSUpgradeable { * @param newImplementation address of the new implementation */ // This function is called by the proxy contract when the implementation is upgraded - function _authorizeUpgrade(address newImplementation) - internal - view - override - { + function _authorizeUpgrade(address newImplementation) internal view override { require(msg.sender == owner(), "VP: not owner"); (newImplementation); } /* =============== Viewers & Validation ============= */ - function pack(UserOperation calldata userOp) - internal - pure - returns (bytes memory ret) - { + function pack(UserOperation calldata userOp) internal pure returns (bytes memory ret) { // lighter signature scheme. must match UserOp.ts#packUserOp bytes calldata pnd = userOp.paymasterAndData; // copy directly the userOp from calldata up to (but not including) the paymasterAndData. @@ -92,24 +82,18 @@ contract SignaturePaymaster is BasePaymaster, Initializable, UUPSUpgradeable { * note that this signature covers all fields of the UserOperation, except the "paymasterAndData", * which will carry the signature itself. */ - function getHash( - UserOperation calldata userOp, - uint48 validUntil, - uint48 validAfter - ) public view returns (bytes32) { + function getHash(UserOperation calldata userOp, uint48 validUntil, uint48 validAfter) + public + view + returns (bytes32) + { //can't use userOp.hash(), since it contains also the paymasterAndData itself. - return - keccak256( - abi.encode( - pack(userOp), - block.chainid, - address(this), - senderNonce[userOp.getSender()], - validUntil, - validAfter - ) - ); + return keccak256( + abi.encode( + pack(userOp), block.chainid, address(this), senderNonce[userOp.getSender()], validUntil, validAfter + ) + ); } /** @@ -120,27 +104,22 @@ contract SignaturePaymaster is BasePaymaster, Initializable, UUPSUpgradeable { * paymasterAndData[20:84] : abi.encode(validUntil, validAfter) * paymasterAndData[84:] : signature */ - function _validatePaymasterUserOp( - UserOperation calldata userOp, - bytes32, /*userOpHash*/ - uint256 requiredPreFund - ) internal override returns (bytes memory context, uint256 validationData) { + function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32, /*userOpHash*/ uint256 requiredPreFund) + internal + override + returns (bytes memory context, uint256 validationData) + { (requiredPreFund); - ( - uint48 validUntil, - uint48 validAfter, - bytes calldata signature - ) = parsePaymasterAndData(userOp.paymasterAndData); + (uint48 validUntil, uint48 validAfter, bytes calldata signature) = + parsePaymasterAndData(userOp.paymasterAndData); //ECDSA library supports both 64 and 65-byte long signatures. // we only "require" it here so that the revert reason on invalid signature will be of "SignaturePaymaster", and not "ECDSA" require( signature.length == 64 || signature.length == 65, "SignaturePaymaster: invalid signature length in paymasterAndData" ); - bytes32 hash = ECDSA.toEthSignedMessageHash( - getHash(userOp, validUntil, validAfter) - ); + bytes32 hash = ECDSA.toEthSignedMessageHash(getHash(userOp, validUntil, validAfter)); senderNonce[userOp.getSender()]++; //don't revert on signature failure: return SIG_VALIDATION_FAILED @@ -156,16 +135,10 @@ contract SignaturePaymaster is BasePaymaster, Initializable, UUPSUpgradeable { function parsePaymasterAndData(bytes calldata paymasterAndData) public pure - returns ( - uint48 validUntil, - uint48 validAfter, - bytes calldata signature - ) + returns (uint48 validUntil, uint48 validAfter, bytes calldata signature) { - (validUntil, validAfter) = abi.decode( - paymasterAndData[VALID_TIMESTAMP_OFFSET:SIGNATURE_OFFSET], - (uint48, uint48) - ); + (validUntil, validAfter) = + abi.decode(paymasterAndData[VALID_TIMESTAMP_OFFSET:SIGNATURE_OFFSET], (uint48, uint48)); signature = paymasterAndData[SIGNATURE_OFFSET:]; } } diff --git a/test/WitthdrawWorkflow.t.sol b/test/WitthdrawWorkflow.t.sol index 317086be5..388205e29 100644 --- a/test/WitthdrawWorkflow.t.sol +++ b/test/WitthdrawWorkflow.t.sol @@ -41,16 +41,10 @@ contract WithdrawWorkflowTest is UserOp { entryPoint = IKintoEntryPoint(address(new EntryPoint{salt: 0}())); IAccessRegistry accessRegistryImpl = new AccessRegistry(); - UUPSProxy accessRegistryProxy = new UUPSProxy{salt: 0}( - address(accessRegistryImpl), - "" - ); + UUPSProxy accessRegistryProxy = new UUPSProxy{salt: 0}(address(accessRegistryImpl), ""); accessRegistry = AccessRegistry(address(accessRegistryProxy)); - IAccessPoint accessPointImpl = new AccessPoint( - entryPoint, - accessRegistry - ); + IAccessPoint accessPointImpl = new AccessPoint(entryPoint, accessRegistry); accessRegistry.initialize(accessPointImpl); accessPoint = accessRegistry.deployFor(address(_user)); @@ -73,11 +67,7 @@ contract WithdrawWorkflowTest is UserOp { address(withdrawWorkflow), accessPoint.getNonce(), _userPk, - abi.encodeWithSelector( - WithdrawWorkflow.withdrawERC20.selector, - IERC20(token), - defaultAmount - ) + abi.encodeWithSelector(WithdrawWorkflow.withdrawERC20.selector, IERC20(token), defaultAmount) ); entryPoint.handleOps(userOps, payable(_user)); @@ -88,11 +78,8 @@ contract WithdrawWorkflowTest is UserOp { function testWithdrawERC20() public { token.mint(address(accessPoint), defaultAmount); - bytes memory data = abi.encodeWithSelector( - WithdrawWorkflow.withdrawERC20.selector, - IERC20(token), - defaultAmount - ); + bytes memory data = + abi.encodeWithSelector(WithdrawWorkflow.withdrawERC20.selector, IERC20(token), defaultAmount); vm.prank(_user); accessPoint.execute(address(withdrawWorkflow), data); @@ -103,10 +90,7 @@ contract WithdrawWorkflowTest is UserOp { function testWithdrawNativeViaPaymaster() public { vm.deal(address(accessPoint), defaultAmount); - abi.encodeWithSelector( - WithdrawWorkflow.withdrawNative.selector, - defaultAmount - ); + abi.encodeWithSelector(WithdrawWorkflow.withdrawNative.selector, defaultAmount); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = createUserOperationWithPaymaster( @@ -115,10 +99,7 @@ contract WithdrawWorkflowTest is UserOp { address(withdrawWorkflow), accessPoint.getNonce(), _userPk, - abi.encodeWithSelector( - WithdrawWorkflow.withdrawNative.selector, - defaultAmount - ) + abi.encodeWithSelector(WithdrawWorkflow.withdrawNative.selector, defaultAmount) ); entryPoint.handleOps(userOps, payable(address(accessPoint))); @@ -129,10 +110,7 @@ contract WithdrawWorkflowTest is UserOp { function testWithdrawNative() public { vm.deal(address(accessPoint), defaultAmount); - bytes memory data = abi.encodeWithSelector( - WithdrawWorkflow.withdrawNative.selector, - defaultAmount - ); + bytes memory data = abi.encodeWithSelector(WithdrawWorkflow.withdrawNative.selector, defaultAmount); vm.prank(_user); accessPoint.execute(address(withdrawWorkflow), data); @@ -149,10 +127,7 @@ contract WithdrawWorkflowTest is UserOp { paymaster = new SignaturePaymaster{salt: 0}(entryPoint, _verifier); // deploy _proxy contract and point it to _implementation - UUPSProxy proxyPaymaster = new UUPSProxy{salt: 0}( - address(paymaster), - "" - ); + UUPSProxy proxyPaymaster = new UUPSProxy{salt: 0}(address(paymaster), ""); // wrap in ABI to support easier calls paymaster = SignaturePaymaster(address(proxyPaymaster)); @@ -180,11 +155,7 @@ contract WithdrawWorkflowTest is UserOp { _nonce, _privateKey, _bytesOp, - abi.encodePacked( - paymaster, - abi.encode(validUntil, validAfter), - new bytes(65) - ) + abi.encodePacked(paymaster, abi.encode(validUntil, validAfter), new bytes(65)) ); bytes32 hash = paymaster.getHash(op, validUntil, validAfter); @@ -192,20 +163,15 @@ contract WithdrawWorkflowTest is UserOp { (uint8 v, bytes32 r, bytes32 s) = vm.sign(_verifierPk, hash); bytes memory signature = abi.encodePacked(r, s, v); - return - createUserOperation( - 1, - address(accessPoint), - address(withdrawWorkflow), - accessPoint.getNonce(), - _userPk, - _bytesOp, - abi.encodePacked( - paymaster, - abi.encode(validUntil, validAfter), - signature - ) - ); + return createUserOperation( + 1, + address(accessPoint), + address(withdrawWorkflow), + accessPoint.getNonce(), + _userPk, + _bytesOp, + abi.encodePacked(paymaster, abi.encode(validUntil, validAfter), signature) + ); } function createUserOperation( @@ -232,12 +198,7 @@ contract WithdrawWorkflowTest is UserOp { }); uint256[] memory keys = new uint256[](1); keys[0] = _privateKey; - op.signature = _signUserOp( - op, - AccessPoint(payable(_from)).entryPoint(), - _chainID, - keys - ); + op.signature = _signUserOp(op, AccessPoint(payable(_from)).entryPoint(), _chainID, keys); return op; } } diff --git a/test/helpers/ERC20Mock.sol b/test/helpers/ERC20Mock.sol index 0c9cb86b3..f61f8f76b 100644 --- a/test/helpers/ERC20Mock.sol +++ b/test/helpers/ERC20Mock.sol @@ -7,11 +7,7 @@ import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; contract ERC20Mock is ERC20 { uint8 private immutable _decimals; - constructor( - string memory name, - string memory symbol, - uint8 decimals_ - ) ERC20(name, symbol) { + constructor(string memory name, string memory symbol, uint8 decimals_) ERC20(name, symbol) { _decimals = decimals_; }