From b4ce311e12c823242c611c228d4e2264f0979100 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Thu, 9 Feb 2023 17:22:30 +0200 Subject: [PATCH] AA-142 N-11 Typographical errors (#219) Co-authored-by: shahafn --- contracts/core/BaseAccount.sol | 4 ++-- contracts/core/EntryPoint.sol | 12 +++++------ contracts/interfaces/IAccount.sol | 2 +- contracts/interfaces/IAggregatedAccount.sol | 4 ++-- contracts/interfaces/IAggregator.sol | 2 +- contracts/interfaces/IEntryPoint.sol | 4 ++-- contracts/interfaces/IPaymaster.sol | 6 +++--- contracts/interfaces/IStakeManager.sol | 2 +- contracts/interfaces/UserOperation.sol | 2 +- contracts/samples/DepositPaymaster.sol | 2 +- contracts/samples/SimpleAccount.sol | 2 +- contracts/samples/TokenPaymaster.sol | 7 +++---- contracts/samples/bls/BLSAccount.sol | 6 +++--- contracts/samples/bls/BLSAccountFactory.sol | 4 ++-- contracts/samples/bls/BLSHelper.sol | 4 ++-- .../samples/bls/BLSSignatureAggregator.sol | 6 +++--- contracts/samples/gnosis/EIP4337Manager.sol | 2 +- contracts/test/TestAggregatedAccount.sol | 2 +- .../test/TestAggregatedAccountFactory.sol | 2 +- reports/gas-checker.txt | 20 +++++++++---------- 20 files changed, 47 insertions(+), 48 deletions(-) diff --git a/contracts/core/BaseAccount.sol b/contracts/core/BaseAccount.sol index 66f7fde..8d3530f 100644 --- a/contracts/core/BaseAccount.sol +++ b/contracts/core/BaseAccount.sol @@ -67,13 +67,13 @@ abstract contract BaseAccount is IAccount { * validate the signature is valid for this message. * @param userOp validate the userOp.signature field * @param userOpHash convenient field: the hash of the request, to check the signature against - * (also hashes the entrypoint and chain-id) + * (also hashes the entrypoint and chain id) * @param aggregator the current aggregator. can be ignored by accounts that don't use aggregators * @return sigTimeRange signature validation status and time-range of this operation * sigCheckStatus - (1) to mark signature failure, 0 for valid signature. * <8-byte> validUntil - last timestamp this operation is valid. 0 for "indefinite" * <8-byte> validAfter - first timestamp this operation is valid - * The an account doesn't use time-range, it is enough to return SIG_VALIDATION_FAILED value (1) for signature failure. + * If the account doesn't use time-range, it is enough to return SIG_VALIDATION_FAILED value (1) for signature failure. * Note that the validation code cannot use block.timestamp (or block.number) directly. */ function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash, address aggregator) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 2029cdc..3f65167 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -50,7 +50,7 @@ contract EntryPoint is IEntryPoint, StakeManager { /** * execute a user op - * @param opIndex into into the opInfo array + * @param opIndex index into the opInfo array * @param userOp the userOp to execute * @param opInfo the opInfo filled by validatePrepayment for this userOp. * @return collected the total amount this userOp paid. @@ -69,7 +69,7 @@ contract EntryPoint is IEntryPoint, StakeManager { } // handleOps was called with gas limit too low. abort entire bundle. if (innerRevertCode == INNER_OUT_OF_GAS) { - //report paymaster, since if it is deliberately caused by the bundler, + //report paymaster, since if it is not deliberately caused by the bundler, // it must be a revert caused by paymaster. revert FailedOp(opIndex, opInfo.mUserOp.paymaster, "AA95 out of gas"); } @@ -80,7 +80,7 @@ contract EntryPoint is IEntryPoint, StakeManager { } /** - * Execute a batch of UserOperation. + * Execute a batch of UserOperations. * no signature aggregator is used. * if any account requires an aggregator (that is, it returned an aggregator when * performing simulateValidation), then handleAggregatedOps() must be used instead. @@ -560,7 +560,7 @@ contract EntryPoint is IEntryPoint, StakeManager { * process post-operation. * called just after the callData is executed. * if a paymaster is defined and its validation returned a non-empty context, its postOp is called. - * the excess amount is refunded to the account (or paymaster - if it is was used in the request) + * the excess amount is refunded to the account (or paymaster - if it was used in the request) * @param opIndex index in the batch * @param mode - whether is called from innerHandleOp, or outside (postOpReverted) * @param opInfo userOp fields and info collected during validation @@ -590,7 +590,7 @@ contract EntryPoint is IEntryPoint, StakeManager { revert FailedOp(opIndex, paymaster, reason); } catch { - revert FailedOp(opIndex, paymaster, "A50 postOp revert"); + revert FailedOp(opIndex, paymaster, "AA50 postOp revert"); } } } @@ -598,7 +598,7 @@ contract EntryPoint is IEntryPoint, StakeManager { actualGas += preGas - gasleft(); actualGasCost = actualGas * gasPrice; if (opInfo.prefund < actualGasCost) { - revert FailedOp(opIndex, paymaster, "A51 prefund below actualGasCost"); + revert FailedOp(opIndex, paymaster, "AA51 prefund below actualGasCost"); } uint256 refund = opInfo.prefund - actualGasCost; _incrementDeposit(refundAddress, refund); diff --git a/contracts/interfaces/IAccount.sol b/contracts/interfaces/IAccount.sol index 1f93734..c35ab24 100644 --- a/contracts/interfaces/IAccount.sol +++ b/contracts/interfaces/IAccount.sol @@ -26,7 +26,7 @@ interface IAccount { * sigFailure - (1) to mark signature failure, 0 for valid signature. * <8-byte> validUntil - last timestamp this operation is valid. 0 for "indefinite" * <8-byte> validAfter - first timestamp this operation is valid - * The an account doesn't use time-range, it is enough to return SIG_VALIDATION_FAILED value (1) for signature failure. + * If an account doesn't use time-range, it is enough to return SIG_VALIDATION_FAILED value (1) for signature failure. * Note that the validation code cannot use block.timestamp (or block.number) directly. */ function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash, address aggregator, uint256 missingAccountFunds) diff --git a/contracts/interfaces/IAggregatedAccount.sol b/contracts/interfaces/IAggregatedAccount.sol index ca189bd..d351c50 100644 --- a/contracts/interfaces/IAggregatedAccount.sol +++ b/contracts/interfaces/IAggregatedAccount.sol @@ -6,9 +6,9 @@ import "./IAccount.sol"; import "./IAggregator.sol"; /** - * Aggregated account, that support IAggregator. + * Aggregated account that support IAggregator. * - the validateUserOp will be called only after the aggregator validated this account (with all other accounts of this aggregator). - * - the validateUserOp MUST valiate the aggregator parameter, and MAY ignore the userOp.signature field. + * - the validateUserOp MUST validate the aggregator parameter, and MAY ignore the userOp.signature field. */ interface IAggregatedAccount is IAccount { diff --git a/contracts/interfaces/IAggregator.sol b/contracts/interfaces/IAggregator.sol index e439d17..086c6f3 100644 --- a/contracts/interfaces/IAggregator.sol +++ b/contracts/interfaces/IAggregator.sol @@ -17,7 +17,7 @@ interface IAggregator { /** * validate signature of a single userOp * This method is should be called by bundler after EntryPoint.simulateValidation() returns (reverts) with ValidationResultWithAggregation - * First it validates the signature over the userOp. then it return data to be used when creating the handleOps: + * First it validates the signature over the userOp. Then it returns data to be used when creating the handleOps. * @param userOp the userOperation received from the user. * @return sigForUserOp the value to put into the signature field of the userOp when calling handleOps. * (usually empty, unless account and aggregator support some kind of "multisig" diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol index 0fc6da4..2fafda7 100644 --- a/contracts/interfaces/IEntryPoint.sol +++ b/contracts/interfaces/IEntryPoint.sol @@ -71,7 +71,7 @@ interface IEntryPoint is IStakeManager { * Successful result from simulateValidation. * @param returnInfo gas and time-range returned values * @param senderInfo stake information about the sender - * @param factoryInfo stake information about the factor (if any) + * @param factoryInfo stake information about the factory (if any) * @param paymasterInfo stake information about the paymaster (if any) */ error ValidationResult(ReturnInfo returnInfo, @@ -81,7 +81,7 @@ interface IEntryPoint is IStakeManager { * Successful result from simulateValidation, if the account returns a signature aggregator * @param returnInfo gas and time-range returned values * @param senderInfo stake information about the sender - * @param factoryInfo stake information about the factor (if any) + * @param factoryInfo stake information about the factory (if any) * @param paymasterInfo stake information about the paymaster (if any) * @param aggregatorInfo signature aggregation info (if the account requires signature aggregator) * bundler MUST use it to verify the signature, or reject the UserOperation diff --git a/contracts/interfaces/IPaymaster.sol b/contracts/interfaces/IPaymaster.sol index 590ef15..85ef35c 100644 --- a/contracts/interfaces/IPaymaster.sol +++ b/contracts/interfaces/IPaymaster.sol @@ -12,11 +12,11 @@ interface IPaymaster { enum PostOpMode { opSucceeded, // user op succeeded opReverted, // user op reverted. still has to pay for gas. - postOpReverted //user op succeeded, but caused postOp to revert. Now its a 2nd call, after user's op was deliberately reverted. + postOpReverted //user op succeeded, but caused postOp to revert. Now it's a 2nd call, after user's op was deliberately reverted. } /** - * payment validation: check if paymaster agree to pay. + * payment validation: check if paymaster agrees to pay. * Must verify sender is the entryPoint. * Revert to reject this request. * Note that bundlers will reject this method if it changes the state, unless the paymaster is trusted (whitelisted) @@ -27,7 +27,7 @@ interface IPaymaster { * @return context value to send to a postOp * zero length to signify postOp is not required. * @return sigTimeRange signature and time-range of this operation, encoded the same as the return value of validateUserOperation - * sigFailure - (1) to mark signature failure (needed only if paymaster uses signature-based validation,) + * sigFailure - (1) to mark signature failure (needed only if paymaster uses signature-based validation) * <8-byte> validUntil - last timestamp this operation is valid. 0 for "indefinite" * <8-byte> validAfter - first timestamp this operation is valid * Note that the validation code cannot use block.timestamp (or block.number) directly. diff --git a/contracts/interfaces/IStakeManager.sol b/contracts/interfaces/IStakeManager.sol index b8fa7ac..4823063 100644 --- a/contracts/interfaces/IStakeManager.sol +++ b/contracts/interfaces/IStakeManager.sol @@ -48,7 +48,7 @@ interface IStakeManager { * and the rest fit into a 2nd cell. * 112 bit allows for 10^15 eth * 64 bit for full timestamp - * 32 bit allow 150 years for unstake delay + * 32 bit allows 150 years for unstake delay */ struct DepositInfo { uint112 deposit; diff --git a/contracts/interfaces/UserOperation.sol b/contracts/interfaces/UserOperation.sol index a9003b8..dfff427 100644 --- a/contracts/interfaces/UserOperation.sol +++ b/contracts/interfaces/UserOperation.sol @@ -14,7 +14,7 @@ pragma solidity ^0.8.12; * @param preVerificationGas gas not calculated by the handleOps method, but added to the gas paid. Covers batch overhead. * @param maxFeePerGas same as EIP-1559 gas parameter. * @param maxPriorityFeePerGas same as EIP-1559 gas parameter. - * @param paymasterAndData if set, this field hold the paymaster address and "paymaster-specific-data". the paymaster will pay for the transaction instead of the sender. + * @param paymasterAndData if set, this field holds the paymaster address and paymaster-specific data. the paymaster will pay for the transaction instead of the sender. * @param signature sender-verified signature over the entire request, the EntryPoint address and the chain ID. */ struct UserOperation { diff --git a/contracts/samples/DepositPaymaster.sol b/contracts/samples/DepositPaymaster.sol index 0da51a1..b8f9f86 100644 --- a/contracts/samples/DepositPaymaster.sol +++ b/contracts/samples/DepositPaymaster.sol @@ -11,7 +11,7 @@ import "../core/BasePaymaster.sol"; import "./IOracle.sol"; /** - * A token-based paymaster that accepts token deposit + * A token-based paymaster that accepts token deposits * The deposit is only a safeguard: the user pays with his token balance. * only if the user didn't approve() the paymaster, or if the token balance is not enough, the deposit will be used. * thus the required deposit is to cover just one method call. diff --git a/contracts/samples/SimpleAccount.sol b/contracts/samples/SimpleAccount.sol index 7ea3bae..1a1b3cd 100644 --- a/contracts/samples/SimpleAccount.sol +++ b/contracts/samples/SimpleAccount.sol @@ -66,7 +66,7 @@ contract SimpleAccount is BaseAccount, UUPSUpgradeable, Initializable { } /** - * execute a sequence of transaction + * execute a sequence of transactions */ function executeBatch(address[] calldata dest, bytes[] calldata func) external { _requireFromEntryPointOrOwner(); diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol index 09de25f..acffbc1 100644 --- a/contracts/samples/TokenPaymaster.sol +++ b/contracts/samples/TokenPaymaster.sol @@ -8,10 +8,10 @@ import "./SimpleAccount.sol"; import "../core/BasePaymaster.sol"; /** - * A sample paymaster that define itself as a token to pay for gas. + * A sample paymaster that defines itself as a token to pay for gas. * The paymaster IS the token to use, since a paymaster cannot use an external contract. * Also, the exchange rate has to be fixed, since it can't reference an external Uniswap or other exchange contract. - * subclass should override "getTokenValueOfEth to provide actual token exchange rate, settable by the owner. + * subclass should override "getTokenValueOfEth" to provide actual token exchange rate, settable by the owner. * Known Limitation: this paymaster is exploitable when put into a batch with multiple ops (of different accounts): * - while a single op can't exploit the paymaster (if postOp fails to withdraw the tokens, the user's op is reverted, * and then we know we can withdraw the tokens), multiple ops with different senders (all using this paymaster) @@ -66,8 +66,7 @@ contract TokenPaymaster is BasePaymaster, ERC20 { /** * validate the request: - * if this is a constructor call, make sure it is a known account (that is, a contract that - * we trust that in its constructor will set + * if this is a constructor call, make sure it is a known account. * verify the sender has enough tokens. * (since the paymaster is also the token, there is no notion of "approval") */ diff --git a/contracts/samples/bls/BLSAccount.sol b/contracts/samples/bls/BLSAccount.sol index 79db231..4f92dd1 100644 --- a/contracts/samples/bls/BLSAccount.sol +++ b/contracts/samples/bls/BLSAccount.sol @@ -6,17 +6,17 @@ import "./IBLSAccount.sol"; /** * Minimal BLS-based account that uses an aggregated signature. - * The account must maintain its own BLS public-key, and expose its trusted signature aggregator. + * The account must maintain its own BLS public key, and expose its trusted signature aggregator. * Note that unlike the "standard" SimpleAccount, this account can't be called directly * (normal SimpleAccount uses its "signer" address as both the ecrecover signer, and as a legitimate - * Ethereum sender address. Obviously, a BLS public is not a valid Ethereum sender address.) + * Ethereum sender address. Obviously, a BLS public key is not a valid Ethereum sender address.) */ contract BLSAccount is SimpleAccount, IBLSAccount { address public immutable aggregator; uint256[4] private publicKey; // The constructor is used only for the "implementation" and only sets immutable values. - // Mutable values slots for proxy accounts are set by the 'initialize' function. + // Mutable value slots for proxy accounts are set by the 'initialize' function. constructor(IEntryPoint anEntryPoint, address anAggregator) SimpleAccount(anEntryPoint) { aggregator = anAggregator; } diff --git a/contracts/samples/bls/BLSAccountFactory.sol b/contracts/samples/bls/BLSAccountFactory.sol index cdeb507..a109e93 100644 --- a/contracts/samples/bls/BLSAccountFactory.sol +++ b/contracts/samples/bls/BLSAccountFactory.sol @@ -9,7 +9,7 @@ import "./BLSAccount.sol"; /* solhint-disable no-inline-assembly */ /** - * Based n SimpleAccountFactory + * Based on SimpleAccountFactory * can't be a subclass, since both constructor and createAccount depend on the * actual wallet contract constructor and initializer */ @@ -25,7 +25,7 @@ contract BLSAccountFactory { * returns the address even if the account is already deployed. * Note that during UserOperation execution, this method is called only if the account is not deployed. * This method returns an existing account address so that entryPoint.getSenderAddress() would work even after account creation - * Also note that our BLSSignatureAggregator requires that the public-key is the last parameter + * Also note that our BLSSignatureAggregator requires that the public key is the last parameter */ function createAccount(uint256 salt, uint256[4] calldata aPublicKey) public returns (BLSAccount) { diff --git a/contracts/samples/bls/BLSHelper.sol b/contracts/samples/bls/BLSHelper.sol index 2c3121f..fdb60e0 100644 --- a/contracts/samples/bls/BLSHelper.sol +++ b/contracts/samples/bls/BLSHelper.sol @@ -29,7 +29,7 @@ library BLSHelper { ret.y = y; } - /// @dev Adds two points (x1, y1, z1) and (x2 y2, z2). + /// @dev Adds two points (x1, y1, z1) and (x2, y2, z2). /// @param _x1 coordinate x of P1 /// @param _y1 coordinate y of P1 /// @param _z1 coordinate z of P1 @@ -134,7 +134,7 @@ library BLSHelper { return q; } - /// @dev Doubles a points (x, y, z). + /// @dev Doubles a point (x, y, z). /// @param _x coordinate x of P1 /// @param _y coordinate y of P1 /// @param _z coordinate z of P1 diff --git a/contracts/samples/bls/BLSSignatureAggregator.sol b/contracts/samples/bls/BLSSignatureAggregator.sol index 2e34f95..60ea291 100644 --- a/contracts/samples/bls/BLSSignatureAggregator.sol +++ b/contracts/samples/bls/BLSSignatureAggregator.sol @@ -38,7 +38,7 @@ contract BLSSignatureAggregator is IAggregator { */ function getTrailingPublicKey(bytes memory data) public pure returns (uint256[4] memory publicKey) { uint len = data.length; - require(len > 32 * 4, "data to short for sig"); + require(len > 32 * 4, "data too short for sig"); /* solhint-disable-next-line no-inline-assembly */ assembly { @@ -92,7 +92,7 @@ contract BLSSignatureAggregator is IAggregator { /** * return the BLS "message" for the given UserOp. - * the account checks the signature over this value using its public-key + * the account checks the signature over this value using its public key */ function userOpToMessage(UserOperation memory userOp) public view returns (uint256[2] memory) { bytes32 publicKeyHash = _getPublicKeyHash(getUserOpPublicKey(userOp)); @@ -155,7 +155,7 @@ contract BLSSignatureAggregator is IAggregator { /** * allow staking for this aggregator - * there is no limit on stake or delay, but it is not a problem, since it is a permissionless + * there is no limit on stake or delay, but it is not a problem, since it is a permissionless * signature aggregator, which doesn't support unstaking. */ function addStake(IEntryPoint entryPoint, uint32 delay) external payable { diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol index 903db13..93a94e4 100644 --- a/contracts/samples/gnosis/EIP4337Manager.sol +++ b/contracts/samples/gnosis/EIP4337Manager.sol @@ -137,7 +137,7 @@ contract EIP4337Manager is GnosisSafe, IAccount { */ function validateEip4337(GnosisSafe safe, EIP4337Manager manager) public { - // this prevent mistaken replaceEIP4337Manager to disable the module completely. + // this prevents mistaken replaceEIP4337Manager to disable the module completely. // minimal signature that pass "recover" bytes memory sig = new bytes(65); sig[64] = bytes1(uint8(27)); diff --git a/contracts/test/TestAggregatedAccount.sol b/contracts/test/TestAggregatedAccount.sol index b193d4a..074c32d 100644 --- a/contracts/test/TestAggregatedAccount.sol +++ b/contracts/test/TestAggregatedAccount.sol @@ -15,7 +15,7 @@ contract TestAggregatedAccount is SimpleAccount, IAggregatedAccount { address public immutable aggregator; // The constructor is used only for the "implementation" and only sets immutable values. - // Mutable values slots for proxy accounts are set by the 'initialize' function. + // Mutable value slots for proxy accounts are set by the 'initialize' function. constructor(IEntryPoint anEntryPoint, address anAggregator) SimpleAccount(anEntryPoint) { aggregator = anAggregator; } diff --git a/contracts/test/TestAggregatedAccountFactory.sol b/contracts/test/TestAggregatedAccountFactory.sol index a408f07..819f750 100644 --- a/contracts/test/TestAggregatedAccountFactory.sol +++ b/contracts/test/TestAggregatedAccountFactory.sol @@ -7,7 +7,7 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import "./TestAggregatedAccount.sol"; /** - * Based n SimpleAccountFactory + * Based on SimpleAccountFactory * can't be a subclass, since both constructor and createAccount depend on the * actual wallet contract constructor, initializer */ diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 363ce44..1379136 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -8,27 +8,27 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 78111 │ │ ║ +║ simple │ 1 │ 78087 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 43585 │ 12540 ║ +║ simple - diff from previous │ 2 │ │ 43573 │ 12528 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 470497 │ │ ║ +║ simple │ 10 │ 470473 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 43780 │ 12735 ║ +║ simple - diff from previous │ 11 │ │ 43744 │ 12699 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 84387 │ │ ║ +║ simple paymaster │ 1 │ 84411 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple paymaster with diff │ 2 │ │ 42598 │ 11553 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 467930 │ │ ║ +║ simple paymaster │ 10 │ 467942 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 42770 │ 11725 ║ +║ simple paymaster with diff │ 11 │ │ 42758 │ 11713 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 179834 │ │ ║ +║ big tx 5k │ 1 │ 179846 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 144934 │ 17639 ║ +║ big tx - diff from previous │ 2 │ │ 144910 │ 17615 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1489677 │ │ ║ +║ big tx 5k │ 10 │ 1489641 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx - diff from previous │ 11 │ │ 146391 │ 19096 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝