| Issue | Instances | |
|---|---|---|
| GAS-1 | Use assembly to check for address(0) |
27 |
| GAS-2 | Using bools for storage incurs overhead | 1 |
| GAS-3 | Cache array length outside of loop | 1 |
| GAS-4 | State variables should be cached in stack variables rather than re-reading them from storage | 1 |
| GAS-5 | Use calldata instead of memory for function arguments that do not get mutated | 16 |
| GAS-6 | Use Custom Errors | 71 |
| GAS-7 | Don't initialize variables with default value | 19 |
| GAS-8 | ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) |
12 |
| GAS-9 | Using private rather than public for constants, saves gas |
4 |
| GAS-10 | Use != 0 instead of > 0 for unsigned integer comparison | 24 |
| GAS-11 | internal functions not called by the contract should be removed |
8 |
Saves 6 gas per instance
Instances (27):
File: contracts/smart-contract-wallet/SmartAccount.sol
110: require(_newOwner != address(0), "Smart Account:: new Signatory address cannot be zero");
128: require(_newEntryPoint != address(0), "Smart Account:: new entry point address cannot be zero");
167: require(owner == address(0), "Already initialized");
168: require(address(_entryPoint) == address(0), "Already initialized");
169: require(_owner != address(0),"Invalid owner");
170: require(_entryPointAddress != address(0), "Invalid Entrypoint");
171: require(_handler != address(0), "Invalid Entrypoint");
174: if (_handler != address(0)) internalSetFallbackHandler(_handler);
257: address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
258: if (gasToken == address(0)) {
281: address payable receiver = refundReceiver == address(0) ? payable(tx.origin) : refundReceiver;
282: if (gasToken == address(0)) {
450: require(dest != address(0), "this action will burn your funds");
511: require(owner == hash.recover(userOp.signature) || tx.origin == address(0), "account: wrong signature");
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
18: require(_baseImpl != address(0), "base wallet address can not be zero");
40: require(address(proxy) != address(0), "Create2 call failed");
File: contracts/smart-contract-wallet/base/ModuleManager.sol
21: require(modules[SENTINEL_MODULES] == address(0), "BSA100");
23: if (to != address(0))
34: require(module != address(0) && module != SENTINEL_MODULES, "BSA101");
36: require(modules[module] == address(0), "BSA102");
49: require(module != address(0) && module != SENTINEL_MODULES, "BSA101");
68: require(msg.sender != SENTINEL_MODULES && modules[msg.sender] != address(0), "BSA104");
106: return SENTINEL_MODULES != module && modules[module] != address(0);
File: contracts/smart-contract-wallet/paymasters/verifying/singleton/VerifyingSingletonPaymaster.sol
36: require(address(_entryPoint) != address(0), "VerifyingPaymaster: Entrypoint can not be zero address");
37: require(_verifyingSigner != address(0), "VerifyingPaymaster: signer of paymaster can not be zero address");
50: require(paymasterId != address(0), "Paymaster Id can not be zero address");
66: require(_newVerifyingSigner != address(0), "VerifyingPaymaster: new signer can not be zero address");
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.
Instances (1):
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
15: mapping (address => bool) public isAccountExist;
If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).
Instances (1):
File: contracts/smart-contract-wallet/SmartAccount.sol
468: for (uint i = 0; i < dest.length;) {
[GAS-4] State variables should be cached in stack variables rather than re-reading them from storage
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
Saves 100 gas per instance
Instances (1):
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
42: emit SmartAccountCreated(proxy,_defaultImpl,_owner, VERSION, _index);
Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.
Instances (16):
File: contracts/smart-contract-wallet/BaseSmartAccount.sol
117: Transaction memory _tx,
119: FeeRefund memory refundInfo,
120: bytes memory signatures) public payable virtual returns (bool success);
File: contracts/smart-contract-wallet/SmartAccount.sol
193: Transaction memory _tx,
195: FeeRefund memory refundInfo,
196: bytes memory signatures
304: bytes memory data,
305: bytes memory signatures
425: Transaction memory _tx,
426: FeeRefund memory refundInfo,
File: contracts/smart-contract-wallet/base/ModuleManager.sol
64: bytes memory data,
83: bytes memory data,
File: contracts/smart-contract-wallet/interfaces/ISignatureValidator.sol
19: function isValidSignature(bytes memory _data, bytes memory _signature) public view virtual returns (bytes4);
19: function isValidSignature(bytes memory _data, bytes memory _signature) public view virtual returns (bytes4);
File: contracts/smart-contract-wallet/libs/MultiSend.sol
26: function multiSend(bytes memory transactions) public payable {
File: contracts/smart-contract-wallet/libs/MultiSendCallOnly.sol
21: function multiSend(bytes memory transactions) public payable {
Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.
Instances (71):
File: contracts/smart-contract-wallet/BaseSmartAccount.sol
74: require(msg.sender == address(entryPoint()), "account: not from EntryPoint");
File: contracts/smart-contract-wallet/SmartAccount.sol
77: require(msg.sender == owner, "Smart Account:: Sender is not authorized");
83: require(msg.sender == owner || msg.sender == address(this),"Only owner or self");
89: require(msg.sender == address(entryPoint()), "wallet: not from EntryPoint");
110: require(_newOwner != address(0), "Smart Account:: new Signatory address cannot be zero");
121: require(_implementation.isContract(), "INVALID_IMPLEMENTATION");
128: require(_newEntryPoint != address(0), "Smart Account:: new entry point address cannot be zero");
167: require(owner == address(0), "Already initialized");
168: require(address(_entryPoint) == address(0), "Already initialized");
169: require(_owner != address(0),"Invalid owner");
170: require(_entryPointAddress != address(0), "Invalid Entrypoint");
171: require(_handler != address(0), "Invalid Entrypoint");
224: require(gasleft() >= max((_tx.targetTxGas * 64) / 63,_tx.targetTxGas + 2500) + 500, "BSA010");
232: require(success || _tx.targetTxGas != 0 || refundInfo.gasPrice != 0, "BSA013");
262: require(success, "BSA011");
265: require(transferToken(gasToken, receiver, payment), "BSA012");
286: require(success, "BSA011");
289: require(transferToken(gasToken, receiver, payment), "BSA012");
322: require(uint256(s) >= uint256(1) * 65, "BSA021");
325: require(uint256(s) + 32 <= signatures.length, "BSA022");
333: require(uint256(s) + 32 + contractSignatureLen <= signatures.length, "BSA023");
342: require(ISignatureValidator(_signer).isValidSignature(data, contractSignature) == EIP1271_MAGIC_VALUE, "BSA024");
348: require(_signer == owner, "INVALID_SIGNATURE");
351: require(_signer == owner, "INVALID_SIGNATURE");
450: require(dest != address(0), "this action will burn your funds");
452: require(success,"transfer failed");
467: require(dest.length == func.length, "wrong array lengths");
491: require(success, "Userop Failed");
495: require(msg.sender == address(entryPoint()) || msg.sender == owner, "account: not Owner or EntryPoint");
502: require(nonces[0]++ == userOp.nonce, "account: invalid nonce");
511: require(owner == hash.recover(userOp.signature) || tx.origin == address(0), "account: wrong signature");
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
18: require(_baseImpl != address(0), "base wallet address can not be zero");
40: require(address(proxy) != address(0), "Create2 call failed");
File: contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol
36: require(beneficiary != address(0), "AA90 invalid beneficiary");
38: require(success, "AA91 failed send to beneficiary");
170: require(msg.sender == address(this), "AA92 internal call only");
213: require(paymasterAndData.length >= 20, "AA93 invalid paymasterAndData");
353: require(verificationGasLimit > gasUsedByValidateAccountPrepayment, "AA41 too little verificationGas");
397: require(maxGasValues <= type(uint120).max, "AA94 gas values overflow");
File: contracts/smart-contract-wallet/aa-4337/core/StakeManager.sol
41: require(newAmount <= type(uint112).max, "deposit overflow");
61: require(_unstakeDelaySec > 0, "must specify unstake delay");
62: require(_unstakeDelaySec >= info.unstakeDelaySec, "cannot decrease unstake time");
64: require(stake > 0, "no stake specified");
65: require(stake < type(uint112).max, "stake overflow");
82: require(info.unstakeDelaySec != 0, "not staked");
83: require(info.staked, "already unstaking");
99: require(stake > 0, "No stake to withdraw");
100: require(info.withdrawTime > 0, "must call unlockStake() first");
101: require(info.withdrawTime <= block.timestamp, "Stake withdrawal is not due");
107: require(success, "failed to withdraw stake");
117: require(withdrawAmount <= info.deposit, "Withdraw amount too large");
121: require(success, "failed to withdraw");
File: contracts/smart-contract-wallet/base/ModuleManager.sol
21: require(modules[SENTINEL_MODULES] == address(0), "BSA100");
25: require(execute(to, 0, data, Enum.Operation.DelegateCall, gasleft()), "BSA000");
34: require(module != address(0) && module != SENTINEL_MODULES, "BSA101");
36: require(modules[module] == address(0), "BSA102");
49: require(module != address(0) && module != SENTINEL_MODULES, "BSA101");
50: require(modules[prevModule] == module, "BSA103");
68: require(msg.sender != SENTINEL_MODULES && modules[msg.sender] != address(0), "BSA104");
File: contracts/smart-contract-wallet/libs/MultiSend.sol
27: require(address(this) != multisendSingleton, "MultiSend should only be called via delegatecall");
File: contracts/smart-contract-wallet/paymasters/BasePaymaster.sol
52: revert("must override");
File: contracts/smart-contract-wallet/paymasters/verifying/singleton/VerifyingSingletonPaymaster.sol
36: require(address(_entryPoint) != address(0), "VerifyingPaymaster: Entrypoint can not be zero address");
37: require(_verifyingSigner != address(0), "VerifyingPaymaster: signer of paymaster can not be zero address");
42: revert("Deposit must be for a paymasterId. Use depositFor");
49: require(!Address.isContract(paymasterId), "Paymaster Id can not be smart contract address");
50: require(paymasterId != address(0), "Paymaster Id can not be zero address");
57: require(amount <= currentBalance, "Insufficient amount to withdraw");
66: require(_newVerifyingSigner != address(0), "VerifyingPaymaster: new signer can not be zero address");
107: require(sigLength == 64 || sigLength == 65, "VerifyingPaymaster: invalid signature length in paymasterAndData");
108: require(verifyingSigner == hash.toEthSignedMessageHash().recover(paymasterData.signature), "VerifyingPaymaster: wrong signature");
109: require(requiredPreFund <= paymasterIdBalances[paymasterData.paymasterId], "Insufficient balance for paymaster id");
Instances (19):
File: contracts/smart-contract-wallet/SmartAccount.sol
234: uint256 payment = 0;
310: uint256 i = 0;
468: for (uint i = 0; i < dest.length;) {
File: contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol
74: for (uint256 i = 0; i < opslen; i++) {
78: uint256 collected = 0;
80: for (uint256 i = 0; i < opslen; i++) {
99: uint256 totalOps = 0;
100: for (uint256 i = 0; i < opasLen; i++) {
106: uint256 opIndex = 0;
107: for (uint256 a = 0; a < opasLen; a++) {
112: for (uint256 i = 0; i < opslen; i++) {
126: uint256 collected = 0;
128: for (uint256 a = 0; a < opasLen; a++) {
134: for (uint256 i = 0; i < opslen; i++) {
313: uint256 missingAccountFunds = 0;
File: contracts/smart-contract-wallet/base/ModuleManager.sol
119: uint256 moduleCount = 0;
File: contracts/smart-contract-wallet/libs/Math.sol
206: uint256 result = 0;
259: uint256 result = 0;
310: uint256 result = 0;
Saves 5 gas per loop
Instances (12):
File: contracts/smart-contract-wallet/SmartAccount.sol
216: nonces[batchId]++;
502: require(nonces[0]++ == userOp.nonce, "account: invalid nonce");
File: contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol
74: for (uint256 i = 0; i < opslen; i++) {
80: for (uint256 i = 0; i < opslen; i++) {
100: for (uint256 i = 0; i < opasLen; i++) {
107: for (uint256 a = 0; a < opasLen; a++) {
112: for (uint256 i = 0; i < opslen; i++) {
114: opIndex++;
128: for (uint256 a = 0; a < opasLen; a++) {
134: for (uint256 i = 0; i < opslen; i++) {
136: opIndex++;
File: contracts/smart-contract-wallet/base/ModuleManager.sol
124: moduleCount++;
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table
Instances (4):
File: contracts/smart-contract-wallet/SmartAccount.sol
36: string public constant VERSION = "1.0.2"; // using AA 0.3.0
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
11: string public constant VERSION = "1.0.2";
File: contracts/smart-contract-wallet/handler/DefaultCallbackHandler.sol
12: string public constant NAME = "Default Callback Handler";
13: string public constant VERSION = "1.0.0";
Instances (24):
File: contracts/smart-contract-wallet/SmartAccount.sol
236: if (refundInfo.gasPrice > 0) {
File: contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol
174: if (callData.length > 0) {
178: if (result.length > 0) {
212: if (paymasterAndData.length > 0) {
452: if (context.length > 0) {
File: contracts/smart-contract-wallet/aa-4337/core/StakeManager.sol
61: require(_unstakeDelaySec > 0, "must specify unstake delay");
64: require(stake > 0, "no stake specified");
99: require(stake > 0, "No stake to withdraw");
100: require(info.withdrawTime > 0, "must call unlockStake() first");
File: contracts/smart-contract-wallet/aa-4337/utils/Exec.sol
2: pragma solidity >=0.7.5 <0.9.0;
File: contracts/smart-contract-wallet/libs/Math.sol
147: if (rounding == Rounding.Up && mulmod(x, y, denominator) > 0) {
208: if (value >> 128 > 0) {
212: if (value >> 64 > 0) {
216: if (value >> 32 > 0) {
220: if (value >> 16 > 0) {
224: if (value >> 8 > 0) {
228: if (value >> 4 > 0) {
232: if (value >> 2 > 0) {
236: if (value >> 1 > 0) {
312: if (value >> 128 > 0) {
316: if (value >> 64 > 0) {
320: if (value >> 32 > 0) {
324: if (value >> 16 > 0) {
328: if (value >> 8 > 0) {
If the functions are required by an interface, the contract should inherit from that interface and use the override keyword
Instances (8):
File: contracts/smart-contract-wallet/common/Singleton.sol
20: function _getImplementation() internal view returns (address _imp) {
File: contracts/smart-contract-wallet/libs/LibAddress.sol
11: function isContract(address account) internal view returns (bool) {
File: contracts/smart-contract-wallet/libs/Math.sol
19: function max(uint256 a, uint256 b) internal pure returns (uint256) {
34: function average(uint256 a, uint256 b) internal pure returns (uint256) {
45: function ceilDiv(uint256 a, uint256 b) internal pure returns (uint256) {
File: contracts/smart-contract-wallet/paymasters/PaymasterHelpers.sol
24: function paymasterContext(
34: function decodePaymasterData(UserOperation calldata op) internal pure returns (PaymasterData memory) {
43: function decodePaymasterContext(bytes memory context) internal pure returns (PaymasterContext memory) {
| Issue | Instances | |
|---|---|---|
| NC-1 | require() / revert() statements should have descriptive reason strings |
3 |
| NC-2 | Event is missing indexed fields |
9 |
| NC-3 | Constants should be defined rather than using magic numbers | 1 |
| NC-4 | Functions not used internally could be marked external | 17 |
Instances (3):
File: contracts/smart-contract-wallet/SmartAccount.sol
371: require(execute(to, value, data, operation, gasleft()));
528: require(req);
File: contracts/smart-contract-wallet/libs/Math.sol
78: require(denominator > prod1);
Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
Instances (9):
File: contracts/smart-contract-wallet/Proxy.sol
13: event Received(uint indexed value, address indexed sender, bytes data);
File: contracts/smart-contract-wallet/SmartAccount.sol
64: event ImplementationUpdated(address _scw, string version, address newImplementation);
65: event EntryPointChanged(address oldEntryPoint, address newEntryPoint);
67: event WalletHandlePayment(bytes32 txHash, uint256 payment);
File: contracts/smart-contract-wallet/base/Executor.sol
9: event ExecutionFailure(address to, uint256 value, bytes data, Enum.Operation operation, uint256 txGas);
10: event ExecutionSuccess(address to, uint256 value, bytes data, Enum.Operation operation, uint256 txGas);
File: contracts/smart-contract-wallet/base/FallbackManager.sol
9: event ChangedFallbackHandler(address handler);
File: contracts/smart-contract-wallet/base/ModuleManager.sol
11: event EnabledModule(address module);
12: event DisabledModule(address module);
Instances (1):
File: contracts/smart-contract-wallet/base/FallbackManager.sol
43: mstore(calldatasize(), shl(96, caller()))
Instances (17):
File: contracts/smart-contract-wallet/SmartAccount.sol
155: function getNonce(uint256 batchId)
166: function init(address _owner, address _entryPointAddress, address _handler) public override initializer {
389: function getTransactionHash(
518: function getDeposit() public view returns (uint256) {
525: function addDeposit() public payable {
536: function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public onlyOwner {
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
33: function deployCounterFactualWallet(address _owner, address _entryPoint, address _handler, uint _index) public returns(address proxy){
53: function deployWallet(address _owner, address _entryPoint, address _handler) public returns(address proxy){
File: contracts/smart-contract-wallet/base/FallbackManager.sol
26: function setFallbackHandler(address handler) public authorized {
File: contracts/smart-contract-wallet/base/ModuleManager.sol
32: function enableModule(address module) public authorized {
47: function disableModule(address prevModule, address module) public authorized {
80: function execTransactionFromModuleReturnData(
105: function isModuleEnabled(address module) public view returns (bool) {
File: contracts/smart-contract-wallet/libs/MultiSend.sol
26: function multiSend(bytes memory transactions) public payable {
File: contracts/smart-contract-wallet/libs/MultiSendCallOnly.sol
21: function multiSend(bytes memory transactions) public payable {
File: contracts/smart-contract-wallet/paymasters/verifying/singleton/VerifyingSingletonPaymaster.sol
48: function depositFor(address paymasterId) public payable {
55: function withdrawTo(address payable withdrawAddress, uint256 amount) public override {
| Issue | Instances | |
|---|---|---|
| L-1 | abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() |
4 |
| L-2 | Unspecific compiler version pragma | 1 |
[L-1] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()
Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.
If all arguments are strings and or bytes, bytes.concat() should be used instead
Instances (4):
File: contracts/smart-contract-wallet/SmartAccount.sol
347: _signer = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
File: contracts/smart-contract-wallet/SmartAccountFactory.sol
34: bytes32 salt = keccak256(abi.encodePacked(_owner, address(uint160(_index))));
70: bytes32 salt = keccak256(abi.encodePacked(_owner, address(uint160(_index))));
71: bytes32 hash = keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, keccak256(code)));
Instances (1):
File: contracts/smart-contract-wallet/aa-4337/utils/Exec.sol
2: pragma solidity >=0.7.5 <0.9.0;
| Issue | Instances | |
|---|---|---|
| M-1 | Centralization Risk for trusted owners | 14 |
Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.
Instances (14):
File: contracts/smart-contract-wallet/SmartAccount.sol
76: modifier onlyOwner {
82: modifier mixedAuth {
109: function setOwner(address _newOwner) external mixedAuth {
120: function updateImplementation(address _implementation) external mixedAuth {
127: function updateEntryPoint(address _newEntryPoint) external mixedAuth {
449: function transfer(address payable dest, uint amount) external nonReentrant onlyOwner {
455: function pullTokens(address token, address dest, uint256 amount) external onlyOwner {
536: function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public onlyOwner {
File: contracts/smart-contract-wallet/paymasters/BasePaymaster.sol
16: abstract contract BasePaymaster is IPaymaster, Ownable {
24: function setEntryPoint(IEntryPoint _entryPoint) public onlyOwner {
67: function withdrawTo(address payable withdrawAddress, uint256 amount) public virtual onlyOwner {
75: function addStake(uint32 unstakeDelaySec) external payable onlyOwner {
90: function unlockStake() external onlyOwner {
99: function withdrawStake(address payable withdrawAddress) external onlyOwner {