SIZE contest - Aymen0909's results

QA Report

Summary

Findings

1- Auction can be created with zero totalBaseAmount :

In the createAuction function a user can start a new auction without transferring any baseToken, this is possible because there are no checks on the values of auctionParams, one of them being totalBaseAmount which could be set to zero. This will create an auction with zero base tokens and bidders will still be able to bid on it but in the end no fund will be distributed.

Risk : Low

Proof of Concept

Instances include:

File: src/SizeSealed.sol Line 98-100

SafeTransferLib.safeTransferFrom(
    ERC20(auctionParams.baseToken), msg.sender, address(this), auctionParams.totalBaseAmount
);

Mitigation

Add a check in the createAuction function to verify that the totalBaseAmount of the auction is not equal to 0.

2- Named return variables not used anywhere in the function :

When Named return variable are declared they should be used inside the function instead of the return statement or if not they should be removed to avoid confusion.

Risk : NON CRITICAL

Proof of Concept

Instances include:

File: src/SizeSealed.sol

returns (uint128 tokensAvailable)

File: src/util/ECCMath.sol

returns (bytes32 decryptedMessage)

Mitigation

Either use the named return variables inplace of the return statement or remove them.

3- constant should be defined rather than using magic numbers :

It is best practice to use constant variables rather than hex/numeric literal values to make the code easier to understand and maintain, but if they are used those numbers should be well docummented.

Risk : NON CRITICAL

Proof of Concept

Instances include:

File: src/SizeSealed.sol

24 hours

24 hours

24 hours

File: src/util/ECCMath.sol

6000

Mitigation

Replace the hex/numeric literals aforementioned with constants.

Gas Optimizations

Findings

Findings

1- Using storage pointer to a struct cost more gas tham memory when it's values are called multiple times :

In general using the storage keyword instead of memory for reading from struct/array/mapping cost less gas, but this is only true if you don't need to read all or many members multiple times.

There are 3 instances of this issue:

File: src/SizeSealed.sol Line 181

Auction storage a = idToAuction[auctionId];

File: src/SizeSealed.sol Line 359

Auction storage a = idToAuction[auctionId];

File: src/SizeSealed.sol Line 418

Auction storage a = idToAuction[auctionId];

2- x += y/x -= y costs more gas than x = x + y/x = x - y for state variables :

Using the addition operator instead of plus-equals saves 113 gas as explained here

There is 1 instance of this issue:

File: src/SizeSealed.sol Line 373

b.baseWithdrawn += baseTokensAvailable;

3- Use calldata instead of memory for function parameters type :

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

There is 1 instance of this issue:

File: src/SizeSealed.sol Line 217

function finalize(uint256 auctionId, uint256[] memory bidIndices, uint128 clearingBase, uint128 clearingQuote)

4- ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as in the case when used in for & while loops :

There are 2 instances of this issue:

File: src/SizeSealed.sol Line 244

for (uint256 i; i < bidIndices.length; i++)

File: src/SizeSealed.sol Line 302

for (uint256 i; i < seenBidMap.length - 1; i++)