Platform: Code4rena
Start Date: 26/05/2023
Pot Size: $100,000 USDC
Total HM: 0
Participants: 33
Period: 14 days
Judge: leastwood
Id: 241
League: ETH
Rank: 3/33
Findings: 1
Award: $8,029.15
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: rbserver
Also found by: 0x73696d616f, 0xTheC0der, 0xdeadbeef, 0xhacksmithh, Bauchibred, GalloDaSballo, KKat7531, Madalad, MohammedRizwan, Rolezn, SAAJ, SanketKogekar, Sathish9098, VictoryGod, brgltd, btk, codeslide, descharre, hunter_w3b, jauvany, kaveyjoe, ladboy233, nadin, niser93, shealtielanz, souilos, trysam2003, yongskiws
8029.1527 USDC - $8,029.15
| Issue | Contexts | |
|---|---|---|
| LOW‑1 | Add to blacklist function | 9 |
| LOW‑2 | Avoid using tx.origin | 2 |
| LOW‑3 | Event is missing parameters | 3 |
| LOW‑4 | Possible rounding issue | 3 |
| LOW‑5 | Missing Contract-existence Checks Before Low-level Calls | 1 |
| LOW‑6 | Contracts are not using their OZ Upgradeable counterparts | 9 |
| LOW‑7 | TransferOwnership Should Be Two Step | 3 |
| LOW‑8 | Unbounded loop | 1 |
| LOW‑9 | Unsafe downcast | 5 |
| LOW‑10 | No Storage Gap For Upgradeable Contracts | 1 |
| LOW‑11 | Upgrade OpenZeppelin Contract Dependency | 2 |
| LOW‑12 | Use Ownable2Step rather than Ownable | 6 |
| LOW‑13 | Use safeTransferOwnership instead of transferOwnership function | 3 |
Total: 48 contexts over 13 issues
| Issue | Contexts | |
|---|---|---|
| NC‑1 | Avoid Floating Pragmas: The Version Should Be Locked | 3 |
| NC‑2 | No need for == true or == false checks | 1 |
| NC‑3 | Consider disabling renounceOwnership() | 8 |
| NC‑4 | Consider using named mappings | 3 |
| NC‑5 | Constants Should Be Defined Rather Than Using Magic Numbers | 6 |
| NC‑6 | Contract does not follow the Solidity style guide's suggested layout ordering | 2 |
| NC‑7 | Critical Changes Should Use Two-step Procedure | 6 |
| NC‑8 | Duplicated require()/revert() Checks Should Be Refactored To A Modifier Or Function | 2 |
| NC‑9 | block.timestamp is already used when emitting events, no need to input timestamp | 1 |
| NC‑10 | Event emit should emit a parameter | 1 |
| NC‑11 | Function writing that does not comply with the Solidity Style Guide | 2 |
| NC‑12 | Large or complicated code bases should implement fuzzing tests | 1 |
| NC‑13 | Use delete to Clear Variables | 1 |
| NC‑14 | Imports can be grouped together | 15 |
| NC‑15 | No need to initialize uints to zero | 2 |
| NC‑16 | Initial value check is missing in Set Functions | 5 |
| NC‑17 | Missing event for critical parameter change | 1 |
| NC‑18 | NatSpec comments should be increased in contracts | 1 |
| NC‑19 | NatSpec @param is missing | 2 |
| NC‑20 | NatSpec @return argument is missing | 1 |
| NC‑21 | Use a more recent version of Solidity | 19 |
| NC‑22 | Omissions in Events | 3 |
| NC‑23 | Public Functions Not Called By The Contract Should Be Declared External Instead | 3 |
| NC‑24 | Empty blocks should be removed or emit something | 10 |
| NC‑25 | Use bytes.concat() | 1 |
Total: 100 contexts over 25 issues
blacklist functionNFT thefts have increased recently, so with the addition of hacked NFTs to the platform, NFTs can be converted into liquidity. To prevent this, I recommend adding the blacklist function.
Marketplaces such as Opensea have a blacklist feature that will not list NFTs that have been reported theft, NFT projects such as Manifold have blacklist functions in their smart contracts.
Here is the project example; Manifold
Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code
modifier nonBlacklistRequired(address extension) { require(!_blacklistedExtensions.contains(extension), "Extension blacklisted"); _; }
Add to Blacklist function and modifier.
tx.origintx.origin is a global variable in Solidity that returns the address of the account that sent the transaction.
Using the variable could make a contract vulnerable if an authorized account calls a malicious contract. You can impersonate a user using a third party contract.
This can make it easier to create a vault on behalf of another user with an external administrator (by receiving it as an argument).
417: if (success == false && tx.origin == Constants.ESTIMATION_ADDRESS) {
</details>465: if (msg.sender != tx.origin) {
The following functions are missing critical parameters when emitting an event.
When dealing with source address which uses the value of msg.sender, the msg.sender value must be specified in every transaction, a contract or web page listening to events cannot react to users, emit does not serve the purpose. Basically, this event cannot be used.
function deleteL2Outputs(uint256 _l2OutputIndex) external { require( msg.sender == CHALLENGER, "L2OutputOracle: only the challenger address can delete outputs" ); require( _l2OutputIndex < l2Outputs.length, "L2OutputOracle: cannot delete outputs after the latest output index" ); require( block.timestamp - l2Outputs[_l2OutputIndex].timestamp < FINALIZATION_PERIOD_SECONDS, "L2OutputOracle: cannot delete outputs that have already been finalized" ); uint256 prevNextL2OutputIndex = nextOutputIndex(); assembly { sstore(l2Outputs.slot, _l2OutputIndex) } emit OutputsDeleted(prevNextL2OutputIndex, _l2OutputIndex); }
function proposeL2Output( bytes32 _outputRoot, uint256 _l2BlockNumber, bytes32 _l1BlockHash, uint256 _l1BlockNumber ) external payable { require( msg.sender == PROPOSER, "L2OutputOracle: only the proposer address can propose new outputs" ); require( _l2BlockNumber == nextBlockNumber(), "L2OutputOracle: block number must be equal to next expected block number" ); require( computeL2Timestamp(_l2BlockNumber) < block.timestamp, "L2OutputOracle: cannot propose L2 output in the future" ); require( _outputRoot != bytes32(0), "L2OutputOracle: L2 output proposal cannot be the zero hash" ); if (_l1BlockHash != bytes32(0)) { require( blockhash(_l1BlockNumber) == _l1BlockHash, "L2OutputOracle: block hash does not match the hash at the expected height" ); } emit OutputProposed(_outputRoot, nextOutputIndex(), _l2BlockNumber, block.timestamp); l2Outputs.push( Types.OutputProposal({ outputRoot: _outputRoot, timestamp: uint128(block.timestamp), l2BlockNumber: uint128(_l2BlockNumber) }) ); }
</details>function depositTransaction( address _to, uint256 _value, uint64 _gasLimit, bool _isCreation, bytes memory _data ) public payable metered(_gasLimit) { if (_isCreation) { require( _to == address(0), "OptimismPortal: must send to address(0) when creating a contract" ); } require( _gasLimit >= minimumGasLimit(uint64(_data.length)), "OptimismPortal: gas limit too small" ); require(_data.length <= 120_000, "OptimismPortal: data too large"); address from = msg.sender; if (msg.sender != tx.origin) { from = AddressAliasHelper.applyL1ToL2Alias(msg.sender); } bytes memory opaqueData = abi.encodePacked( msg.value, _value, _gasLimit, _isCreation, _data ); emit TransactionDeposited(from, _to, DEPOSIT_VERSION, opaqueData); }
Add msg.sender parameter in event-emit
Division by large numbers may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator
97: int256 targetResourceLimit = int256(uint256(config.maxResourceLimit)) / int256(uint256(config.elasticityMultiplier)); 155: uint256 gasCost = resourceCost / Math.max(block.basefee, 1 gwei);
</details>48: uint256 scaled = unscaled / divisor;
Low-level calls return success if there is no code present at the specified address.
</details>405: bool success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.value, _tx.data);
In addition to the zero-address checks, add a check to verify that <address>.code.length > 0
The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behaviour.
5: import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
4: import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
4: import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
4: import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; 5: import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
4: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
</details>5: import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
Where applicable, use the contracts from @openzeppelin/contracts-upgradeable instead of @openzeppelin/contracts.
See https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/tree/master/contracts for list of available upgradeable contracts
Recommend considering implementing a two step process where the owner or admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.
135: function initialize: transferOwnership(_owner);
37: function transferOwnership: function transferOwnership(address _owner, bool _isLocal) external onlyOwner {
</details>41: function transferOwnership: _transferOwnership(_owner);
Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
New items are pushed into the following arrays but there is no option to pop them out. Currently, the array can grow indefinitely. E.g. there's no maximum limit and there's no functionality to remove array values.
If the array grows too large, calling relevant functions might run out of gas and revert. Calling these functions could result in a DOS condition.
</details>222: l2Outputs.push(
Add a functionality to delete array values or add a maximum size limit for arrays.
When a type is downcast to a smaller type, the higher order bits are truncated, effectively applying a modulo to the original value. Without any other checks, this wrapping will lead to unexpected behavior and bugs
225: timestamp: uint128(block.timestamp), 226: l2BlockNumber: uint128(_l2BlockNumber)
312: timestamp: uint128(block.timestamp),
</details>137: params.prevBlockNum = uint64(block.number); 183: prevBlockNum: uint64(block.number)
For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments". Otherwise it may be very difficult to write new implementation code. Without storage gap, the variable in child contract might be overwritten by the upgraded base contract if new variables are added to the base contract. This could have unintended and very serious consequences to the child contracts.
Refer to the bottom part of this article: https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable
However, the contract doesn't contain a storage gap. The storage gap is essential for upgradeable contract because "It allows us to freely add new state variables in the future without compromising the storage compatibility with existing deployments". See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
<details></details>SystemConfig.sol
Recommend adding appropriate storage gap at the end of upgradeable contracts such as the below. Please reference OpenZeppelin upgradeable contract templates.
uint256[50] private __gap;
An outdated OZ version is used (which has known vulnerabilities, see: https://github.com/OpenZeppelin/openzeppelin-contracts/security/advisories). Release: https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v4.9.0
Require dependencies to be at least version of 4.9.0 to include critical vulnerability patches.
<details>"@openzeppelin/contracts": "4.7.3"
</details>"@openzeppelin/contracts-upgradeable": "4.7.3"
Update OpenZeppelin Contracts Usage in package.json and require at least version of 4.9.0 via >=4.9.0
Ownable2Step rather than OwnableOwnable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.
5: OwnableUpgradeable 6: OwnableUpgradeable 16: OwnableUpgradeable
4: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
</details>6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
safeTransferOwnership instead of transferOwnership functionUse safeTransferOwnership which is safer. Use it as it is more secure due to 2-stage ownership transfer.
4: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
</details>6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
Use <a href="https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol">Ownable2Step.sol</a>
function acceptOwnership() external { address sender = _msgSender(); require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner"); _transferOwnership(sender); } }
Avoid floating pragmas for non-library contracts.
While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.
A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.
It is recommended to pin to a concrete compiler version.
Found usage of floating pragmas ^0.8.0 of Solidity in [CrossDomainOwnable.sol]
Found usage of floating pragmas ^0.8.0 of Solidity in [CrossDomainOwnable2.sol]
</details>Found usage of floating pragmas ^0.8.0 of Solidity in [CrossDomainOwnable3.sol]
== true or == false checksThere is no need to verify that == true or == false when the variable checked upon is a boolean as well.
</details>417: if (success == false && tx.origin == Constants.ESTIMATION_ADDRESS) {
Instead simply check for variable or !variable
renounceOwnership()If the plan for your project does not include eventually giving up all ownership control, consider overwriting OpenZeppelin's Ownable's renounceOwnership() function in order to disable it.
5: OwnableUpgradeable 6: OwnableUpgradeable 16: OwnableUpgradeable 16: contract SystemConfig is OwnableUpgradeable, Semver {
14: abstract contract CrossDomainOwnable is Ownable {
15: abstract contract CrossDomainOwnable2 is Ownable {
</details>15: abstract contract CrossDomainOwnable3 is Ownable { 18: * it uses the standard Ownable _checkOwner function.
Consider moving to solidity version 0.8.18 or later, and using <a href='https://ethereum.stackexchange.com/a/145555'>named mappings</a> to make it easier to understand the purpose of each mapping
72: mapping(bytes32 => bool) public finalizedWithdrawals; 77: mapping(bytes32 => ProvenWithdrawal) public provenWithdrawals;
</details>32: mapping(bytes32 => bool) public sentMessages;
Even <a href="https://github.com/code-423n4/2022-05-opensea-seaport/blob/9d7ce4d08bf3c3010304a0476a785c70c0e90ae7/contracts/lib/TokenTransferrer.sol#L35-L39">assembly</a> can benefit from using readable constants instead of hex/numeric literals
461: require(_data.length <= 120_000, "OptimismPortal: data too large");
119: if (blockDiff > 1) {
274: _config.baseFeeMaxChangeDenominator > 1,
122: total += 4;
124: total += 16;
</details>128: return unsigned + (68 * 16);
The <a href="https://docs.soliditylang.org/en/v0.8.20/style-guide.html#order-of-layout">style guide</a> says that, within a contract, the ordering should be 1) Type declarations, 2) State variables, 3) Events, 4) Errors 5) Modifiers, and 6) Functions, but the contract(s) below do not follow this ordering
OptimismPortal.sol
</details>GasPriceOracle.sol
The critical procedures should be two step process.
See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure
180: function setUnsafeBlockSigner(address _unsafeBlockSigner) external onlyOwner {
192: function setBatcherHash(bytes32 _batcherHash) external onlyOwner {
205: function setGasConfig(uint256 _overhead, uint256 _scalar) external onlyOwner {
218: function setGasLimit(uint64 _gasLimit) external onlyOwner {
256: function setResourceConfig(ResourceMetering.ResourceConfig memory _config) external onlyOwner {
</details>79: function setL1BlockValues(
Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
require()/revert() Checks Should Be Refactored To A Modifier Or FunctionSaves deployment costs
</details>142: require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low"); 219: require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low");
</details>220: emit OutputProposed(_outputRoot, nextOutputIndex(), _l2BlockNumber, block.timestamp);
Some emitted events do not have any emitted parameters. It is recommended to add some parameter such as state changes or value changes when events are emitted
</details>220: emit OutputProposed(_outputRoot, nextOutputIndex()
Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.
https://docs.soliditylang.org/en/v0.8.17/style-guide.html
Functions should be grouped according to their visibility and ordered:
OptimismPortal.sol
</details>SystemConfig.sol
Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement <a href="https://medium.com/coinmonks/smart-contract-fuzzing-d9b88e0b0a05">fuzzing tests</a>. Fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.
Various in-scope contract files.
delete to Clear Variablesdelete a assigns the initial value for the type to a. i.e. for integers it is equivalent to a = 0, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x.
The delete key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete statements.
</details>136: params.prevBoughtGas = 0;
Consider importing OZ first, then all interfaces, then all utils.
4: import { ERC721Bridge } from "../universal/ERC721Bridge.sol"; 5: import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; 6: import { L2ERC721Bridge } from "../L2/L2ERC721Bridge.sol"; 7: import { Semver } from "../universal/Semver.sol";
4: import { Predeploys } from "../libraries/Predeploys.sol"; 5: import { L2CrossDomainMessenger } from "./L2CrossDomainMessenger.sol"; 6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
4: import { Predeploys } from "../libraries/Predeploys.sol"; 5: import { L2CrossDomainMessenger } from "./L2CrossDomainMessenger.sol"; 6: import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
</details>4: import { ERC721Bridge } from "../universal/ERC721Bridge.sol"; 5: import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; 6: import { L1ERC721Bridge } from "../L1/L1ERC721Bridge.sol"; 7: import { IOptimismMintableERC721 } from "../universal/IOptimismMintableERC721.sol"; 8: import { Semver } from "../universal/Semver.sol";
There is no need to initialize uint variables to zero as their default value is 0
269: uint256 lo = 0;
</details>118: uint256 total = 0;
A check regarding whether the current value and the new value are the same should be added
180: function setUnsafeBlockSigner(address _unsafeBlockSigner) external onlyOwner { _setUnsafeBlockSigner(_unsafeBlockSigner); bytes memory data = abi.encode(_unsafeBlockSigner); emit ConfigUpdate(VERSION, UpdateType.UNSAFE_BLOCK_SIGNER, data); }
192: function setBatcherHash(bytes32 _batcherHash) external onlyOwner { batcherHash = _batcherHash; bytes memory data = abi.encode(_batcherHash); emit ConfigUpdate(VERSION, UpdateType.BATCHER, data); }
205: function setGasConfig(uint256 _overhead, uint256 _scalar) external onlyOwner { overhead = _overhead; scalar = _scalar; bytes memory data = abi.encode(_overhead, _scalar); emit ConfigUpdate(VERSION, UpdateType.GAS_CONFIG, data); }
218: function setGasLimit(uint64 _gasLimit) external onlyOwner { require(_gasLimit >= minimumGasLimit(), "SystemConfig: gas limit too low"); gasLimit = _gasLimit; bytes memory data = abi.encode(_gasLimit); emit ConfigUpdate(VERSION, UpdateType.GAS_LIMIT, data); }
</details>256: function setResourceConfig(ResourceMetering.ResourceConfig memory _config) external onlyOwner { _setResourceConfig(_config); }
When changing state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.
</details>79: function setL1BlockValues(
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html
All in-scope contracts
NatSpec comments should be increased in contracts
@param is missingfunction initialize(bool _paused) public initializer {
</details>189: /** * @notice Computes the minimum gas limit for a deposit. The minimum gas limit * linearly increases based on the size of the calldata. This is to prevent * users from creating L2 resource usage without paying for it. This function * can be used when interacting with the portal to ensure forwards compatibility. * */ function minimumGasLimit(uint64 _byteCount) public pure returns (uint64) {
@return argument is missing</details>189: /** * @notice Computes the minimum gas limit for a deposit. The minimum gas limit * linearly increases based on the size of the calldata. This is to prevent * users from creating L2 resource usage without paying for it. This function * can be used when interacting with the portal to ensure forwards compatibility. * */ function minimumGasLimit(uint64 _byteCount) public pure returns (uint64) {
<a href="https://blog.soliditylang.org/2021/04/21/solidity-0.8.4-release-announcement/">0.8.4</a>: bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>)
<a href="https://blog.soliditylang.org/2022/02/16/solidity-0.8.12-release-announcement/">0.8.12</a>: string.concat() instead of abi.encodePacked(<str>,<str>)
<a href="https://blog.soliditylang.org/2022/03/16/solidity-0.8.13-release-announcement/">0.8.13</a>: Ability to use using for with a list of free functions
<a href="https://blog.soliditylang.org/2022/05/18/solidity-0.8.14-release-announcement/">0.8.14</a>:
ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.
<a href="https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/">0.8.15</a>:
Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.
<a href="https://blog.soliditylang.org/2022/08/08/solidity-0.8.16-release-announcement/">0.8.16</a>:
Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.
<a href="https://blog.soliditylang.org/2022/09/08/solidity-0.8.17-release-announcement/">0.8.17</a>:
Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.
<a href="https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/">0.8.19</a>: SMTChecker: New trusted mode that assumes that any compile-time available code is the actual used code, even in external calls. Bug Fixes:
<a href="https://blog.soliditylang.org/2023/05/10/solidity-0.8.20-release-announcement/">0.8.20</a>:
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity ^0.8.0;
pragma solidity ^0.8.0;
pragma solidity ^0.8.0;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
pragma solidity 0.8.15;
</details>pragma solidity 0.8.15;
Consider updating to a more recent solidity version.
Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters The following events should also add the previous original value in addition to the new value.
20: event OverheadUpdated(uint256 overhead);
21: event ScalarUpdated(uint256 scalar);
</details>22: event DecimalsUpdated(uint256 decimals);
The events should include the new value and old value where possible.
Contracts are allowed to override their parents’ functions and change the visibility from external to public.
function gasPrice() public view returns (uint256) {
function baseFee() public view returns (uint256) {
</details>function l1FeeWallet() public view returns (address) {
31: {}
101: {}
19: constructor(address _recipient) FeeVault(_recipient, 10 ether) Semver(1, 1, 0) {}
33: constructor() Semver(1, 0, 0) {}
65: constructor() Semver(1, 0, 0) {}
19: constructor(address _recipient) FeeVault(_recipient, 10 ether) Semver(1, 1, 0) {}
31: {}
69: {}
70: constructor() Semver(1, 0, 0) {}
</details>20: constructor(address _recipient) FeeVault(_recipient, 10 ether) Semver(1, 1, 0) {}
bytes.concat()Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))
</details>472: bytes memory opaqueData = abi.encodePacked( msg.value, _value, _gasLimit, _isCreation, _data )
Use bytes.concat() and upgrade to at least Solidity version 0.8.4 if required.
#0 - c4-judge
2023-06-16T15:06:40Z
0xleastwood marked the issue as grade-b
#1 - c4-judge
2023-06-16T15:07:17Z
0xleastwood marked the issue as grade-a