Caviar contest - ret2basic's results

A fully on-chain NFT AMM that allows you to trade every NFT in a collection.

General Information

Platform: Code4rena

Start Date: 12/12/2022

Pot Size: $36,500 USDC

Total HM: 8

Participants: 103

Period: 7 days

Judge: berndartmueller

Id: 193

League: ETH

Caviar

Findings Distribution

Researcher Performance

Rank: 75/103

Findings: 1

Award: $14.83

Gas:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

14.833 USDC - $14.83

Labels

bug
G (Gas Optimization)
grade-b
G-21

External Links

Caviar Contest Gas Optimization Report

Summary

The following gas optimization issues were found during the code audit:

  1. Use unchecked{} to suppress overflow/underflow check (7 instances)
  2. Using bools for storage incurs overhead (1 instance)
  3. Empty blocks should be removed or emit something (2 instances)
  4. Split require(xxx && yyy) to require(xxx) and require(yyy) (1 instance)
  5. Use private instead of public for constants (2 instances)
  6. Use shift right/left instead of division/multiplication if possible (1 instance)
  7. Functions guaranteed to revert when called by normal users can be marked payable (2 instances)

Total 16 instances of 7 issues.

1. Use unchecked{} to suppress overflow/underflow check (7 instances)

Starting from version 0.8.0, Solidity does overflow/underflow checks by default. It is a good feature to prevent vulnerabilities but it has a significant overhead, especially when used in for loop. When using uint256/int256, it is extremely hard to trigger overflow, so it makes sense to skip these checks. To suppress the overflow/underflow checks, use unchecked {}. For increment situations, just use unchecked {} directly; for decrement situations, add a require() statement before decrementing to prevent underflow.

src/Pair.sol::238 => for (uint256 i = 0; i < tokenIds.length; i++) {

src/Pair.sol::258 => for (uint256 i = 0; i < tokenIds.length; i++) {

src/Pair.sol::468 => for (uint256 i = 0; i < tokenIds.length; i++) {

src/lib/SafeERC20Namer.sol::13 => for (uint256 j = 0; j < 32; j++) {

src/lib/SafeERC20Namer.sol::22 => for (uint256 j = 0; j < charCount; j++) {

src/lib/SafeERC20Namer.sol::33 => for (uint256 i = 32; i < 64; i++) {

src/lib/SafeERC20Namer.sol::39 => for (uint256 i = 0; i < charCount; i++) {

2. Using bools for storage incurs overhead (1 instance)

Use uint256(1) and uint256(2) for true/false. 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.

src/Pair.sol::469 => bool isValid = MerkleProofLib.verify(proofs[i], merkleRoot, keccak256(abi.encodePacked(tokenIds[i])));

3. Empty blocks should be removed or emit something (2 instances)

Empty blocks exist in the code. The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

src/Caviar.sol::21 => constructor() Owned(msg.sender) {}

src/LpToken.sol::14 => {}

4. Split require(xxx && yyy) to require(xxx) and require(yyy) (1 instance)

Instead of using operator && on single require check, using double require() checks can save more gas.

src/Pair.sol::71 => require(baseTokenAmount > 0 && fractionalTokenAmount > 0, "Input token amount is zero");

5. Use private instead of public for constants (2 instances)

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table.

src/Pair.sol::20 => uint256 public constant ONE = 1e18;

src/Pair.sol::21 => uint256 public constant CLOSE_GRACE_PERIOD = 7 days;

6. Use shift right/left instead of division/multiplication if possible (1 instance)

A division/multiplication by any number x being a power of 2 can be calculated by shifting log2(x) to the right/left. While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.

src/lib/SafeERC20Namer.sol::55 => return Strings.toHexString(uint160(token) >> (160 - 4 * 4));

7. Functions guaranteed to revert when called by normal users can be marked payable (2 instances)

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. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

src/LpToken.sol::19 => function mint(address to, uint256 amount) public onlyOwner {

src/LpToken.sol::26 => function burn(address from, uint256 amount) public onlyOwner {

#0 - c4-judge

2023-01-14T17:15:41Z

berndartmueller marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter