First Flight #4: Boss Bridge
First Flight #4
Beginner FriendlyFoundryBridge
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Static Analyzer Report (Gas Risk) - Still In Development

alsaher

[I-01] Constructor Can Be Marked As Payable

Severity

  • Impact: Low

  • Likelihood: Low

Description

payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided.

A constructor can safely be marked as payable, since only the deployer would be able to pass funds, and the project itself would not pass any funds.

Number Of Instances Found

4

Code Location

Click to show findings
Path: ./src/L1Token.sol
​
9: constructor() ERC20("BossBridgeToken", "BBT") { // @audit-issue

GitHub: 9

Path: ./src/L1Vault.sol
​
15: constructor(IERC20 _token) Ownable(msg.sender) { // @audit-issue

GitHub: 15

Path: ./src/TokenFactory.sol
​
16: constructor() Ownable(msg.sender) { } // @audit-issue

GitHub: 16

Path: ./src/L1BossBridge.sol
​
42: constructor(IERC20 _token) Ownable(msg.sender) { // @audit-issue

GitHub: 42

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

Severity

  • Impact: Low

  • Likelihood: Low

Description

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.

Number Of Instances Found

4

Code Location

Click to show findings
Path: ./src/TokenFactory.sol
​
23: function deployToken(string memory symbol, bytes memory contractBytecode) public onlyOwner returns (address addr) { // @audit-issue
​
23: function deployToken(string memory symbol, bytes memory contractBytecode) public onlyOwner returns (address addr) { // @audit-issue
​
31: function getTokenAddressFromSymbol(string memory symbol) public view returns (address addr) { // @audit-issue

GitHub: 23, 23, 31

Path: ./src/L1BossBridge.sol
​
112: function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused { // @audit-issue

GitHub: 112

[I-03] Operator >=/<= costs less gas than operator >/<

Severity

  • Impact: Low

  • Likelihood: Low

Description

The compiler uses opcodes GT and ISZERO for code that uses >, but only requires LT for >=. A similar behaviour applies for >, which uses opcodes LT and ISZERO, but only requires GT for <=.

Number Of Instances Found

1

Code Location

Click to show findings
Path: ./src/L1BossBridge.sol
​
71: if (token.balanceOf(address(vault)) + amount > DEPOSIT_LIMIT) { // @audit-issue

GitHub: 71

[I-04] Using bools for storage incurs overhead

Severity

  • Impact: Low

  • Likelihood: Low

Description

// Booleans are more expensive than uint256 or any type that takes up a full
// word because each write operation emits an extra SLOAD to first read the
// slot's contents, replace the bits taken up by the boolean, and then write
// back. This is the compiler's defense against contract upgrades and
// pointer aliasing, and it cannot be disabled.

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past

Number Of Instances Found

2

Code Location

Click to show findings
Path: ./src/L1BossBridge.sol
​
34: mapping(address account => bool isSigner) public signers; // @audit-issue
​
121: (bool success,) = target.call{ value: value }(data); // @audit-issue

GitHub: 34, 121

[I-05] Consider using bytes32 rather than a string

Severity

  • Impact: Low

  • Likelihood: Low

Description

Using the bytes types for fixed-length strings is more efficient than having the EVM have to incur the overhead of string processing. Consider whether the value needs to be a string. A good reason to keep it as a string would be if the variable is defined in an interface that this project does not own.

Number Of Instances Found

2

Code Location

Click to show findings
Path: ./src/TokenFactory.sol
​
12: mapping(string tokenSymbol => address tokenAddress) private s_tokenToAddress; // @audit-issue
​
14: event TokenDeployed(string symbol, address addr); // @audit-issue

GitHub: 12, 14

[I-06] Use assembly to emit an event

Severity

  • Impact: Low

  • Likelihood: Low

Description

To efficiently emit events, it's possible to utilize assembly by making use of scratch space and the free memory pointer. This approach has the advantage of potentially avoiding the costs associated with memory expansion.

However, it's important to note that in order to safely optimize this process, it is preferable to cache and restore the free memory pointer.

A good example of such practice can be seen in Solady's codebase.

Number Of Instances Found

2

Code Location

Click to show findings
Path: ./src/TokenFactory.sol
​
28: emit TokenDeployed(symbol, addr); // @audit-issue

GitHub: 28

Path: ./src/L1BossBridge.sol
​
77: emit Deposit(from, l2Recipient, amount); // @audit-issue

GitHub: 77

[I-07] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

Severity

  • Impact: Low

  • Likelihood: Low

Description

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

Number Of Instances Found

2

Code Location

Click to show findings
Path: ./src/L1BossBridge.sol
​
91: function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external { // @audit-issue
​
112: function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused { // @audit-issue

GitHub: 91, 112

[I-08] Missing Constant Modifier For State Variables

Severity

  • Impact: Low

  • Likelihood: Low

Description

State variables that are never re-assigned after their initial assignment should typically be declared with the constant modifier. This ensures that these variables remain unmodified and communicates their unchanging nature. Using the constant modifier also offers potential gas savings, as accessing these variables does not require any storage operations.

Impact

Number Of Instances Found

1

Code Location

Click to show findings
Path: ./src/L1BossBridge.sol
​
30: uint256 public DEPOSIT_LIMIT = 100_000 ether; // @audit-issue

GitHub: 30

[I-09] State Variables Only Set in The Constructor Should Be Declared immutable

Severity

  • Impact: Low

  • Likelihood: Low

Description

Avoids a Gsset(** 20000 gas**) in the constructor, and replaces the first access in each transaction(Gcoldsload - ** 2100 gas **) and each access thereafter(Gwarmacces - ** 100 gas ) with aPUSH32( 3 gas **).

Whilestrings are not value types, and therefore cannot beimmutable / constant if not hard - coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for thestring accessors, and having a child contract override the functions with the hard - coded implementation - specific values.

Impact

Number Of Instances Found

1

Code Location

Click to show findings
Path: ./src/L1Vault.sol
​
13: IERC20 public token; // @audit-issue

GitHub: 13

[I-10] State Variables That Are Used Multiple Times In a Function Should Be Cached In Stack Variables

Severity

  • Impact: Low

  • Likelihood: Low

Description

When performing multiple operations on a state variable in a function, it is recommended to cache it first. Either multiple reads or multiple writes to a state variable can save gas by caching it on the stack. 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.

Impact

Mistakenly using public for functions that should be labeled external not only leads to increased gas costs but also enlarges the attack surface of a contract. This can expose the contract to unexpected behaviors and potential vulnerabilities. Moreover, from a code clarity perspective, incorrectly using public can mislead other developers or auditors about the intended usage of the function, leading to potential misunderstandings or oversights. Ensuring the right visibility is chosen based on the function's usage can enhance security, efficiency, and maintainability of the smart contract.

Number Of Instances Found

4

Code Location

Click to show findings
Path: ./src/L1BossBridge.sol
​
70: function depositTokensToL2(address from, address l2Recipient, uint256 amount) external whenNotPaused {
71: if (token.balanceOf(address(vault)) + amount > DEPOSIT_LIMIT) { // @audit-issue
72: revert L1BossBridge__DepositLimitReached();
73: }
74: token.safeTransferFrom(from, address(vault), amount);
75:
76: // Our off-chain service picks up this event and mints the corresponding tokens on L2
77: emit Deposit(from, l2Recipient, amount);
78: }
​
70: function depositTokensToL2(address from, address l2Recipient, uint256 amount) external whenNotPaused {
71: if (token.balanceOf(address(vault)) + amount > DEPOSIT_LIMIT) { // @audit-issue
72: revert L1BossBridge__DepositLimitReached();
73: }
74: token.safeTransferFrom(from, address(vault), amount);
75:
76: // Our off-chain service picks up this event and mints the corresponding tokens on L2
77: emit Deposit(from, l2Recipient, amount);
78: }
​
91: function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
92: sendToL1(
93: v,
94: r,
95: s,
96: abi.encode(
97: address(token), // @audit-issue
98: 0, // value
99: abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
100: )
101: );
102: }
​
91: function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
92: sendToL1(
93: v,
94: r,
95: s,
96: abi.encode(
97: address(token),
98: 0, // value
99: abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount)) // @audit-issue
100: )
101: );
102: }

GitHub: 71, 71, 97, 99

[I-11] Functions guaranteed to revert when called by normal users can be marked payable

Severity

  • Impact: Low

  • Likelihood: Low

Description

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Number Of Instances Found

5

Code Location

Click to show findings
Path: ./src/L1Vault.sol
​
19: function approveTo(address target, uint256 amount) external onlyOwner { // @audit-issue

GitHub: 19

Path: ./src/TokenFactory.sol
​
23: function deployToken(string memory symbol, bytes memory contractBytecode) public onlyOwner returns (address addr) { // @audit-issue

GitHub: 23

Path: ./src/L1BossBridge.sol
​
49: function pause() external onlyOwner { // @audit-issue
​
53: function unpause() external onlyOwner { // @audit-issue
​
57: function setSigner(address account, bool enabled) external onlyOwner { // @audit-issue

GitHub: 49, 53, 57

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.