QA Report

[code423n4]

See the markdown file with the details of this report here.

[GalloDaSballo]

1.1. The value > MAX_MSG_VALUE check in MsgValueSimulator.fallback() should be made before the value != 0 check R

1.2. Add verifyingContract to EIP712_DOMAIN_TYPEHASH R

1.3. BytecodeCompressor.publishCompressedBytecode() can receive funds 1.4. No event emitted when updating a state variable R

1.5. Lack of a L2EthToken.burn method only callable by the Bootloader Invalid

1.6. Unjustified unchecked NC

1.7. Create an onlyDeployerSystemContract modifier and avoid a missing a friendly revert string NC

1.8. modifier onlyBootloader is declared at 3 places and is missing a friendly revert string at one NC

1.9. (OOS) Discouraged use of safeApprove. Use safeIncreaseAllowance here instead Disagree of the warning in this context, beside being OOS I think approve usage is fine.

2.1. Offsets for META_HEAP_SIZE_OFFSET are excessively confusing NC

2.2. No need to check that v == 27 || v == 28 with ecrecover R

2.3. Tautology when checking recoveredAddress R

2.4. Document the mechanism under SystemContext.getBlockNumberAndTimestamp() NC

2.5. Document why all fields under ZkSyncMeta aren't returned in SystemContractHelper.getZkSyncMeta() R

2.6. require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking R

2.7. Typos NC

2.8. Consider only declaring 1 contract, or 1 library, or 1 interface, or 1 abstract contract in a file NC

2.9. Default Visibility R

2.10. Adding a return statement when the function defines a named return variable, is redundant R

2.11. 2500000000000000 should be changed to 2.5e15 for readability reasons NC

2.12. Use string.concat() or bytes.concat() along with a more recent version of Solidity NC

2.13. Import declarations should import specific identifiers, rather than the whole file NC

[GalloDaSballo]

9R 10NC

[c4-judge]

GalloDaSballo marked the issue as grade-a