Skip to content

Instantly share code, notes, and snippets.

@Picodes
Created March 10, 2023 20:42
Show Gist options
  • Save Picodes/4fda93fac0db99cf9f2e260a073b38e4 to your computer and use it in GitHub Desktop.
Save Picodes/4fda93fac0db99cf9f2e260a073b38e4 to your computer and use it in GitHub Desktop.

Report

Gas Optimizations

Issue Instances
GAS-1 Use selfbalance() instead of address(this).balance 3
GAS-2 Use assembly to check for address(0) 4
GAS-3 Cache array length outside of loop 1
GAS-4 Use calldata instead of memory for function arguments that do not get mutated 1
GAS-5 Use Custom Errors 54
GAS-6 Don't initialize variables with default value 6
GAS-7 Long revert strings 18
GAS-8 Use shift Right/Left instead of division/multiplication if possible 28
GAS-9 Splitting require() statements that use && saves gas 1
GAS-10 Use storage instead of memory for structs/arrays 52
GAS-11 Increments can be unchecked in for-loops 4
GAS-12 Use != 0 instead of > 0 for unsigned integer comparison 8
GAS-13 internal functions not called by the contract should be removed 53

[GAS-1] Use selfbalance() instead of address(this).balance

Use assembly when getting a contract's balance of ETH.

You can use selfbalance() instead of address(this).balance when getting your contract's balance of ETH to save gas. Additionally, you can use balance(address) instead of address.balance() when getting an external contract's balance of ETH.

Saves 15 gas when checking internal balance, 6 for external

Instances (3):

File: contracts/DefaultAccount.sol

103:         require(totalRequiredBalance <= address(this).balance, "Not enough balance for fee + value");
File: contracts/openzeppelin/utils/Address.sol

62:             address(this).balance >= amount,

156:             address(this).balance >= value,

[GAS-2] Use assembly to check for address(0)

Saves 6 gas per instance

Instances (4):

File: contracts/DefaultAccount.sol

93:         bytes32 txHash = _suggestedSignedHash != bytes32(0) ? _suggestedSignedHash : _transaction.encodeHash();

191:         return recoveredAddress == address(this) && recoveredAddress != address(0);
File: contracts/KnownCodesStorage.sol

128:         require(version == 1 && _bytecodeHash[1] == bytes1(0), "Incorrectly formatted bytecodeHash");
File: contracts/libraries/TransactionHelper.sol

405:         if (address(uint160(_transaction.paymaster)) != address(0)) {

[GAS-3] Cache array length outside of loop

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/BytecodeCompressor.sol

49:             for (uint256 encodedDataPointer = 0; encodedDataPointer < encodedData.length; encodedDataPointer += 2) {

[GAS-4] Use calldata instead of memory for function arguments that do not get mutated

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 (1):

File: contracts/interfaces/IL1Messenger.sol

10:     function sendToL1(bytes memory _message) external returns (bytes32);

[GAS-5] Use Custom Errors

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 (54):

File: contracts/AccountCodeStorage.sol

25:         require(msg.sender == address(DEPLOYER_SYSTEM_CONTRACT), "Callable only by the deployer system contract");

36:         require(Utils.isContractConstructing(_hash), "Code hash is not for a contract on constructor");

50:         require(Utils.isContractConstructing(codeHash), "Code hash is not for a contract on constructor");
File: contracts/BootloaderUtilities.sol

36:             revert("Unsupported tx type");

91:             require(vInt == 27 || vInt == 28, "Invalid v value");

190:             require(vInt == 27 || vInt == 28, "Invalid v value");

285:             require(vInt == 27 || vInt == 28, "Invalid v value");
File: contracts/BytecodeCompressor.sol

42:             require(dictionary.length % 8 == 0, "Dictionary length should be a multiple of 8");

43:             require(dictionary.length <= 2 ** 16 * 8, "Dictionary is too big");

51:                 require(indexOfEncodedChunk < dictionary.length, "Encoded chunk index is out of bounds");

56:                 require(encodedChunk == realChunk, "Encoded chunk does not match the original bytecode");
File: contracts/ContractDeployer.sol

27:         require(msg.sender == address(this), "Callable only by self");

233:         require(msg.sender == FORCE_DEPLOYER, "Can only be called by FORCE_DEPLOYER_CONTRACT");

242:         require(msg.value == sumOfValues, "`value` provided is not equal to the combined `value`s of deployments");

255:         require(_bytecodeHash != bytes32(0x0), "BytecodeHash can not be zero");

256:         require(uint160(_newAddress) > MAX_SYSTEM_CONTRACT_ADDRESS, "Can not deploy contracts in kernel space");

264:         require(NONCE_HOLDER_SYSTEM_CONTRACT.getRawNonce(_newAddress) == 0x00, "Account is occupied");

296:         require(knownCodeMarker > 0, "The code hash is not known");
File: contracts/DefaultAccount.sol

96:             require(_transaction.data.length >= 4, "Invalid call to ContractDeployer");

103:         require(totalRequiredBalance <= address(this).balance, "Not enough balance for fee + value");

163:         require(_signature.length == 65, "Signature length is incorrect");

176:         require(v == 27 || v == 28, "v is neither 27 nor 28");

187:         require(uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0, "Invalid s");

205:         require(success, "Failed to pay the fee to the operator");
File: contracts/ImmutableSimulator.sol

34:         require(msg.sender == address(DEPLOYER_SYSTEM_CONTRACT), "Callable only by the deployer system contract");
File: contracts/KnownCodesStorage.sol

19:         require(msg.sender == BOOTLOADER_FORMAL_ADDRESS, "Callable only by the bootloader");

24:         require(msg.sender == address(BYTECODE_COMPRESSOR_CONTRACT), "Callable only by the bytecode compressor");

112:         require(precompileCallSuccess, "Failed to charge gas");

128:         require(version == 1 && _bytecodeHash[1] == bytes1(0), "Incorrectly formatted bytecodeHash");

130:         require(Utils.bytecodeLenInWords(_bytecodeHash) % 2 == 1, "Code length in words must be odd");
File: contracts/L2EthToken.sol

29:         require(msg.sender == BOOTLOADER_FORMAL_ADDRESS, "Callable only by the bootloader");

49:         require(fromBalance >= _amount, "Transfer amount exceeds balance");
File: contracts/NonceHolder.sol

65:         require(_value <= MAXIMAL_MIN_NONCE_INCREMENT, "The value for incrementing the nonce is too high");

84:         require(_value != 0, "Nonce value can not be set to 0");

88:             require(isNonceUsed(msg.sender, _key - 1), "Previous nonce has not been used");

114:         require(oldMinNonce == _expectedNonce, "Incorrect nonce");

135:         require(msg.sender == address(DEPLOYER_SYSTEM_CONTRACT), "");

162:             revert("Reusing the same nonce twice");

164:             revert("The nonce was not set as used");
File: contracts/SystemContext.sol

116:         require(_newTimestamp >= currentBlockTimestamp, "Timestamps should be incremental");

117:         require(currentBlockNumber + 1 == _expectedNewNumber, "The provided block number is not correct");
File: contracts/libraries/EfficientCall.sol

37:         require(returnData.length == 32, "keccak256 returned invalid data");

46:         require(returnData.length == 32, "sha returned invalid data");
File: contracts/libraries/SystemContractHelper.sol

310:         require(index < 10, "There are only 10 accessible registers");
File: contracts/libraries/TransactionHelper.sol

111:             revert("Encoding unsupported tx");

362:         require(_transaction.paymasterInput.length >= 4, "The standard paymaster input must be at least 4 bytes long");

387:             revert("Unsupported paymaster flow");
File: contracts/libraries/Utils.sol

20:         require(_x <= type(uint128).max, "Overflow");

26:         require(_x <= type(uint32).max, "Overflow");

32:         require(_x <= type(uint24).max, "Overflow");

78:         require(_bytecode.length % 32 == 0, "po");

81:         require(bytecodeLenInWords < 2 ** 16, "pp"); // bytecode length must be less than 2^16 words

82:         require(bytecodeLenInWords % 2 == 1, "pr"); // bytecode length in words must be odd
File: contracts/openzeppelin/utils/Address.sol

266:                 require(isContract(target), "Address: call to non-contract");

[GAS-6] Don't initialize variables with default value

Instances (6):

File: contracts/BytecodeCompressor.sol

49:             for (uint256 encodedDataPointer = 0; encodedDataPointer < encodedData.length; encodedDataPointer += 2) {
File: contracts/ContractDeployer.sol

238:         uint256 sumOfValues = 0;

239:         for (uint256 i = 0; i < deploymentsLength; ++i) {

244:         for (uint256 i = 0; i < deploymentsLength; ++i) {
File: contracts/ImmutableSimulator.sol

37:             for (uint256 i = 0; i < immutablesLength; ++i) {
File: contracts/KnownCodesStorage.sol

34:             for (uint256 i = 0; i < hashesLen; ++i) {

[GAS-7] Long revert strings

Instances (18):

File: contracts/AccountCodeStorage.sol

25:         require(msg.sender == address(DEPLOYER_SYSTEM_CONTRACT), "Callable only by the deployer system contract");

36:         require(Utils.isContractConstructing(_hash), "Code hash is not for a contract on constructor");

50:         require(Utils.isContractConstructing(codeHash), "Code hash is not for a contract on constructor");
File: contracts/BytecodeCompressor.sol

42:             require(dictionary.length % 8 == 0, "Dictionary length should be a multiple of 8");

51:                 require(indexOfEncodedChunk < dictionary.length, "Encoded chunk index is out of bounds");

56:                 require(encodedChunk == realChunk, "Encoded chunk does not match the original bytecode");
File: contracts/ContractDeployer.sol

233:         require(msg.sender == FORCE_DEPLOYER, "Can only be called by FORCE_DEPLOYER_CONTRACT");

242:         require(msg.value == sumOfValues, "`value` provided is not equal to the combined `value`s of deployments");

256:         require(uint160(_newAddress) > MAX_SYSTEM_CONTRACT_ADDRESS, "Can not deploy contracts in kernel space");
File: contracts/DefaultAccount.sol

103:         require(totalRequiredBalance <= address(this).balance, "Not enough balance for fee + value");

205:         require(success, "Failed to pay the fee to the operator");
File: contracts/ImmutableSimulator.sol

34:         require(msg.sender == address(DEPLOYER_SYSTEM_CONTRACT), "Callable only by the deployer system contract");
File: contracts/KnownCodesStorage.sol

24:         require(msg.sender == address(BYTECODE_COMPRESSOR_CONTRACT), "Callable only by the bytecode compressor");

128:         require(version == 1 && _bytecodeHash[1] == bytes1(0), "Incorrectly formatted bytecodeHash");
File: contracts/NonceHolder.sol

65:         require(_value <= MAXIMAL_MIN_NONCE_INCREMENT, "The value for incrementing the nonce is too high");
File: contracts/SystemContext.sol

117:         require(currentBlockNumber + 1 == _expectedNewNumber, "The provided block number is not correct");
File: contracts/libraries/SystemContractHelper.sol

310:         require(index < 10, "There are only 10 accessible registers");
File: contracts/libraries/TransactionHelper.sol

362:         require(_transaction.paymasterInput.length >= 4, "The standard paymaster input must be at least 4 bytes long");

[GAS-8] Use shift Right/Left instead of division/multiplication if possible

Shifting left by N is like multiplying by 2^N and shifting right by N is like dividing by 2^N

Instances (28):

File: contracts/BootloaderUtilities.sol

97:             }
File: contracts/BytecodeCompressor.sol

44:             require(

44:             require(

46:                 "Encoded data length should be 4 times shorter than the original bytecode"

51:                 require(indexOfEncodedChunk < dictionary.length, "Encoded chunk index is out of bounds");

55: 

82:             encodedData = _rawCompressedData[2 + dictionaryLen * 8:];

83:         }
File: contracts/Constants.sol

19: 

23: 

75: 
File: contracts/L1Messenger.sol

38:         }

38:         }
File: contracts/NonceHolder.sol

28:     /// The minNonce can be increased by at 2^32 at a time to prevent it from

31: 
File: contracts/SystemContext.sol

49: 
File: contracts/libraries/RLPEncoder.sol

33: 

68: 
File: contracts/libraries/SystemContractsCaller.sol

42: uint256 constant META_HEAP_SIZE_OFFSET = 8 * 8;

43: uint256 constant META_AUX_HEAP_SIZE_OFFSET = 12 * 8;

44: uint256 constant META_SHARD_ID_OFFSET = 28 * 8;

45: uint256 constant META_CALLER_SHARD_ID_OFFSET = 29 * 8;

46: uint256 constant META_CODE_SHARD_ID_OFFSET = 30 * 8;

47: 
File: contracts/libraries/Utils.sol

40:     }

46:         }

81:         require(bytecodeLenInWords < 2 ** 16, "pp"); // bytecode length must be less than 2^16 words

82:         require(bytecodeLenInWords % 2 == 1, "pr"); // bytecode length in words must be odd

[GAS-9] Splitting require() statements that use && saves gas

Instances (1):

File: contracts/KnownCodesStorage.sol

128:         require(version == 1 && _bytecodeHash[1] == bytes1(0), "Incorrectly formatted bytecodeHash");

[GAS-10] Use storage instead of memory for structs/arrays

Using memory copies the struct or array in memory. Use storage to save the location in storage and have cheaper reads:

Instances (52):

File: contracts/BootloaderUtilities.sol

52:         // Encode `gasPrice` and `gasLimit` together to prevent "stack too deep error".

56:             bytes memory encodedGasLimit = RLPEncoder.encodeUint256(_transaction.gasLimit);

57:             encodedGasParam = bytes.concat(encodedGasPrice, encodedGasLimit);

61:         bytes memory encodedValue = RLPEncoder.encodeUint256(_transaction.value);

62:         // Encode only the length of the transaction data, and not the data itself,

143:             bytes memory encodedNonce = RLPEncoder.encodeUint256(_transaction.nonce);

144:             bytes memory encodedGasPrice = RLPEncoder.encodeUint256(_transaction.maxFeePerGas);

145:             bytes memory encodedGasLimit = RLPEncoder.encodeUint256(_transaction.gasLimit);

146:             bytes memory encodedTo = RLPEncoder.encodeAddress(address(uint160(_transaction.to)));

147:             bytes memory encodedValue = RLPEncoder.encodeUint256(_transaction.value);

148:             encodedFixedLengthParams = bytes.concat(

176: 

236:             bytes memory encodedNonce = RLPEncoder.encodeUint256(_transaction.nonce);

237:             bytes memory encodedMaxPriorityFeePerGas = RLPEncoder.encodeUint256(_transaction.maxPriorityFeePerGas);

238:             bytes memory encodedMaxFeePerGas = RLPEncoder.encodeUint256(_transaction.maxFeePerGas);

239:             bytes memory encodedGasLimit = RLPEncoder.encodeUint256(_transaction.gasLimit);

240:             bytes memory encodedTo = RLPEncoder.encodeAddress(address(uint160(_transaction.to)));

241:             bytes memory encodedValue = RLPEncoder.encodeUint256(_transaction.value);

242:             encodedFixedLengthParams = bytes.concat(

271: 
File: contracts/ContractDeployer.sol

40:         if (info.supportedAAVersion != AccountAbstractionVersion.None) {

71: 

324:         ImmutableData[] memory immutables = abi.decode(returnData, (ImmutableData[]));

325:         IMMUTABLE_SIMULATOR_SYSTEM_CONTRACT.setImmutables(_newAddress, immutables);
File: contracts/L2EthToken.sol

91:         L1_MESSENGER_CONTRACT.sendToL1(message);
File: contracts/NonceHolder.sol

83: 
File: contracts/libraries/EfficientCall.sol

37:         require(returnData.length == 32, "keccak256 returned invalid data");

46:         require(returnData.length == 32, "sha returned invalid data");
File: contracts/libraries/TransactionHelper.sol

155:         // Encode `gasPrice` and `gasLimit` together to prevent "stack too deep error".

159:             bytes memory encodedGasLimit = RLPEncoder.encodeUint256(_transaction.gasLimit);

160:             encodedGasParam = bytes.concat(encodedGasPrice, encodedGasLimit);

164:         bytes memory encodedValue = RLPEncoder.encodeUint256(_transaction.value);

165:         // Encode only the length of the transaction data, and not the data itself,

228:             bytes memory encodedNonce = RLPEncoder.encodeUint256(_transaction.nonce);

229:             bytes memory encodedGasPrice = RLPEncoder.encodeUint256(_transaction.maxFeePerGas);

230:             bytes memory encodedGasLimit = RLPEncoder.encodeUint256(_transaction.gasLimit);

231:             bytes memory encodedTo = RLPEncoder.encodeAddress(address(uint160(_transaction.to)));

232:             bytes memory encodedValue = RLPEncoder.encodeUint256(_transaction.value);

233:             encodedFixedLengthParams = bytes.concat(

261: 

298:             bytes memory encodedNonce = RLPEncoder.encodeUint256(_transaction.nonce);

299:             bytes memory encodedMaxPriorityFeePerGas = RLPEncoder.encodeUint256(_transaction.maxPriorityFeePerGas);

300:             bytes memory encodedMaxFeePerGas = RLPEncoder.encodeUint256(_transaction.maxFeePerGas);

301:             bytes memory encodedGasLimit = RLPEncoder.encodeUint256(_transaction.gasLimit);

302:             bytes memory encodedTo = RLPEncoder.encodeAddress(address(uint160(_transaction.to)));

303:             bytes memory encodedValue = RLPEncoder.encodeUint256(_transaction.value);

304:             encodedFixedLengthParams = bytes.concat(

333: 
File: contracts/openzeppelin/token/ERC20/utils/SafeERC20.sol

140:             data,
File: contracts/openzeppelin/utils/Address.sol

160:             data

202:         return

241:         return

[GAS-11] Increments can be unchecked in for-loops

Instances (4):

File: contracts/ContractDeployer.sol

239:         for (uint256 i = 0; i < deploymentsLength; ++i) {

244:         for (uint256 i = 0; i < deploymentsLength; ++i) {
File: contracts/ImmutableSimulator.sol

37:             for (uint256 i = 0; i < immutablesLength; ++i) {
File: contracts/KnownCodesStorage.sol

34:             for (uint256 i = 0; i < hashesLen; ++i) {

[GAS-12] Use != 0 instead of > 0 for unsigned integer comparison

Instances (8):

File: contracts/AccountCodeStorage.sol

87:         if (codeHash == 0x00 && NONCE_HOLDER_SYSTEM_CONTRACT.getRawNonce(account) > 0) {
File: contracts/ContractDeployer.sol

296:         require(knownCodeMarker > 0, "The code hash is not known");

317:         if (value > 0) {
File: contracts/NonceHolder.sol

148:         return (_nonce < getMinNonce(_address) || nonceValues[addressAsKey][_nonce] > 0);
File: contracts/libraries/Utils.sol

2: pragma solidity >=0.8.0;
File: contracts/openzeppelin/token/ERC20/utils/SafeERC20.sol

143:         if (returndata.length > 0) {
File: contracts/openzeppelin/utils/Address.sol

41:         return account.code.length > 0;

297:         if (returndata.length > 0) {

[GAS-13] internal functions not called by the contract should be removed

If the functions are required by an interface, the contract should inherit from that interface and use the override keyword

Instances (53):

File: contracts/libraries/EfficientCall.sol

35:     function keccak(bytes calldata _data) internal view returns (bytes32) {

44:     function sha(bytes calldata _data) internal view returns (bytes32) {

56:     function call(

85:     function delegateCall(

102:     function mimicCall(
File: contracts/libraries/RLPEncoder.sol

6:     function encodeAddress(address _val) internal pure returns (bytes memory encoded) {

19:     function encodeUint256(uint256 _val) internal pure returns (bytes memory encoded) {

44:     function encodeNonSingleBytesLen(uint64 _len) internal pure returns (bytes memory) {

51:     function encodeListLen(uint64 _len) internal pure returns (bytes memory) {
File: contracts/libraries/SystemContractHelper.sol

48:     function toL1(bool _isService, bytes32 _key, bytes32 _value) internal {

63:     function getCodeAddress() internal view returns (address addr) {

74:     function loadCalldataIntoActivePtr() internal view {

85:     function ptrPackIntoActivePtr(uint256 _farCallAbi) internal view {

94:     function ptrAddIntoActive(uint32 _value) internal view {

106:     function ptrShrinkIntoActive(uint32 _shrink) internal view {

124:     function packPrecompileParams(

147:     function precompileCall(uint256 _rawParams, uint32 _gasToBurn) internal view returns (bool success) {

164:     function setValueForNextFarCall(uint128 _value) internal returns (bool success) {

177:     function eventInitialize(uint256 initializer, uint256 value1) internal {

187:     function eventWrite(uint256 value1, uint256 value2) internal {

231:     function getHeapSizeFromMeta(uint256 meta) internal pure returns (uint32 heapSize) {

240:     function getAuxHeapSizeFromMeta(uint256 meta) internal pure returns (uint32 auxHeapSize) {

272:     function getZkSyncMeta() internal view returns (ZkSyncMeta memory meta) {

298:     function getCalldataPtr() internal view returns (uint256 ptr) {

309:     function getExtraAbiData(uint256 index) internal view returns (uint256 extraAbiData) {

320:     function isSystemCall() internal view returns (bool) {

329:     function isSystemContract(address _address) internal pure returns (bool) {
File: contracts/libraries/SystemContractsCaller.sol

149:     function systemCallWithPropagatedRevert(

213:     function getFarCallABI(
File: contracts/libraries/TransactionHelper.sol

93:     function isEthToken(uint256 _addr) internal pure returns (bool) {

99:     function encodeHash(Transaction calldata _transaction) internal view returns (bytes32 resultHash) {

361:     function processPaymasterInput(Transaction calldata _transaction) internal {

394:     function payToTheBootloader(Transaction calldata _transaction) internal returns (bool success) {

404:     function totalRequiredBalance(Transaction calldata _transaction) internal pure returns (uint256 requiredBalance) {
File: contracts/libraries/UnsafeBytesCalldata.sol

18:     function readUint16(bytes calldata _bytes, uint256 _start) internal pure returns (uint16 result) {

25:     function readUint64(bytes calldata _bytes, uint256 _start) internal pure returns (uint64 result) {
File: contracts/libraries/Utils.sol

19:     function safeCastToU128(uint256 _x) internal pure returns (uint128) {

25:     function safeCastToU32(uint256 _x) internal pure returns (uint32) {

31:     function safeCastToU24(uint256 _x) internal pure returns (uint24) {

38:     function bytecodeLenInBytes(bytes32 _bytecodeHash) internal pure returns (uint256 codeLength) {

50:     function isContractConstructing(bytes32 _bytecodeHash) internal pure returns (bool) {

57:     function constructingBytecodeHash(bytes32 _bytecodeHash) internal pure returns (bytes32) {

76:     function hashL2Bytecode(bytes calldata _bytecode) internal view returns (bytes32 hashedBytecode) {
File: contracts/openzeppelin/token/ERC20/utils/SafeERC20.sol

22:     function safeTransfer(

33:     function safeTransferFrom(

52:     function safeApprove(

70:     function safeIncreaseAllowance(

86:     function safeDecreaseAllowance(

109:     function safePermit(
File: contracts/openzeppelin/utils/Address.sol

60:     function sendValue(address payable recipient, uint256 amount) internal {

91:     function functionCall(address target, bytes memory data)

110:     function functionCall(

280:     function verifyCallResult(

Non Critical Issues

Issue Instances
NC-1 Missing checks for address(0) when assigning values to address state variables 1
NC-2 require() / revert() statements should have descriptive reason strings 3
NC-3 TODO Left in the code 1
NC-4 Event is missing indexed fields 11
NC-5 Functions not used internally could be marked external 7

[NC-1] Missing checks for address(0) when assigning values to address state variables

Instances (1):

File: contracts/SystemContext.sol

61:         origin = _newOrigin;

[NC-2] require() / revert() statements should have descriptive reason strings

Instances (3):

File: contracts/L1Messenger.sol

47:         require(precompileCallSuccess);
File: contracts/SystemContext.sol

16:         require(msg.sender == BOOTLOADER_FORMAL_ADDRESS);
File: contracts/libraries/SystemContractHelper.sol

152:         require(gasleft() >= _gasToBurn);

[NC-3] TODO Left in the code

TODOs may signal that a feature is missing or not ready for audit, consider resolving the issue and removing the TODO comment

Instances (1):

File: contracts/L2EthToken.sol

25:     // TODO: Remove this variable with the new upgrade.

[NC-4] Event is missing indexed fields

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 (11):

File: contracts/interfaces/IContractDeployer.sol

39:     event AccountNonceOrderingUpdated(address indexed accountAddress, AccountNonceOrdering nonceOrdering);

41:     event AccountVersionUpdated(address indexed accountAddress, AccountAbstractionVersion aaVersion);
File: contracts/interfaces/IEthToken.sol

22:     event Mint(address indexed account, uint256 amount);

24:     event Transfer(address indexed from, address indexed to, uint256 value);

26:     event Withdrawal(address indexed _l2Sender, address indexed _l1Receiver, uint256 _amount);
File: contracts/interfaces/IL1Messenger.sol

8:     event L1MessageSent(address indexed _sender, bytes32 indexed _hash, bytes _message);
File: contracts/interfaces/IL2StandardToken.sol

6:     event BridgeMint(address indexed _account, uint256 _amount);

8:     event BridgeBurn(address indexed _account, uint256 _amount);
File: contracts/interfaces/INonceHolder.sol

14:     event ValueSetUnderNonce(address indexed accountAddress, uint256 indexed key, uint256 value);
File: contracts/openzeppelin/token/ERC20/IERC20.sol

16:     event Transfer(address indexed from, address indexed to, uint256 value);

22:     event Approval(address indexed owner, address indexed spender, uint256 value);

[NC-5] Functions not used internally could be marked external

Instances (7):

File: contracts/ContractDeployer.sol

38:     function extendedAccountVersion(address _address) public view returns (AccountAbstractionVersion) {
File: contracts/NonceHolder.sol

56:     function getRawNonce(address _address) public view returns (uint256) {

64:     function increaseMinNonce(uint256 _value) public onlySystemCall returns (uint256 oldMinNonce) {

81:     function setValueUnderNonce(uint256 _key, uint256 _value) public onlySystemCall {

101:     function getValueUnderNonce(uint256 _key) public view returns (uint256) {
File: contracts/SystemContext.sol

91:     function getBlockNumber() public view returns (uint256 blockNumber) {

97:     function getBlockTimestamp() public view returns (uint256 timestamp) {

Low Issues

Issue Instances
L-1 abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() 2
L-2 Do not use deprecated library functions 3
L-3 Empty Function Body - Consider commenting why 2
L-4 Initializers could be front-run 2
L-5 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 (2):

File: contracts/libraries/TransactionHelper.sol

132:                 keccak256(abi.encodePacked(_transaction.factoryDeps)),

141:         return keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));

[L-2] Do not use deprecated library functions

Instances (3):

File: contracts/libraries/TransactionHelper.sol

381:                 IERC20(token).safeApprove(paymaster, 0);

382:                 IERC20(token).safeApprove(paymaster, minAllowance);
File: contracts/openzeppelin/token/ERC20/utils/SafeERC20.sol

52:     function safeApprove(

[L-3] Empty Function Body - Consider commenting why

Instances (2):

File: contracts/EmptyContract.sol

11:     fallback() external payable {}

13:     receive() external payable {}

[L-4] Initializers could be front-run

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

Instances (2):

File: contracts/libraries/SystemContractHelper.sol

177:     function eventInitialize(uint256 initializer, uint256 value1) internal {

180:             pop(call(initializer, callAddr, value1, 0, 0xFFFF, 0, 0))

[L-5] Unspecific compiler version pragma

Instances (1):

File: contracts/libraries/Utils.sol

2: pragma solidity >=0.8.0;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment