SIZE contest - lukris02's results

QA Report for SIZE contest

Overview

During the audit, 6 non-critical issues were found.

Non-Critical Risk Findings(6)

NC-1. Order of Functions

Description

According to Style Guide, ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.
Functions should be grouped according to their visibility and ordered:

1. constructor
2. receive function (if exists)
3. fallback function (if exists)
4. external
5. public
6. internal
7. private

Instances

public functions before external:

• function finalize(uint256 auctionId, uint256[] memory bidIndices, uint128 clearingBase, uint128 clearingQuote)
• function tokensAvailableForWithdrawal(uint256 auctionId, uint128 baseAmount)
• function computeCommitment(bytes32 message) public pure returns (bytes32)

Recommendation

Reorder functions where possible.

NC-2. Missing NatSpec

Description

NatSpec is missing for 4 functions in 1 contracts.

Instances

• function computeCommitment(bytes32 message) public pure returns (bytes32) {
• function computeMessage(uint128 baseAmount, bytes16 salt) external pure returns (bytes32) {
• function getTimings(uint256 auctionId) external view returns (Timings memory timings) {
• function getAuctionData(uint256 auctionId) external view returns (AuctionData memory data) {

Recommendation

Add NatSpec for all functions.

NC-3. Typos

Instances

• /// @notice Bid on a runnning auction => running
• /// @notice Finalises an auction by revealing all bids => Finalizes
• /// @dev Transfers `quoteToken` back to bidder and prevents bid from being finalised => finalized
• // Prevent any futher access to this EncryptedBid => further

NC-4. No error message in revert

Instances

• revert();

Recommendation

Add error messages.

NC-5. Unused named return variables

Description

Both named return variable(s) and return statement are used.

Instances

• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L457-L463
• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L56

Recommendation

To improve clarity use only named return variables.
For example, change:

function functionName() returns (uint id) {
    return x;

to

function functionName() returns (uint id) {
    id = x;

NC-6. Open question

Instances

• /// @dev we hash the point because unsure if x,y is normal distribution (source needed)

Gas Optimizations Report for SIZE contest

Overview

During the audit, 4 gas issues were found.
Total savings ~450+.

Gas Optimizations Findings(4)

G-1. Use calldata instead of memory for read-only arguments

Description

Since Solidity v0.6.9, memory and calldata are allowed in all functions regardless of their visibility type (See "Calldata Variables" section here).
When function arguments should not be modified, it is cheaper to use calldata.

Instances

• function finalize(uint256 auctionId, uint256[] memory bidIndices, uint128 clearingBase, uint128 clearingQuote)
• function ecMul(Point memory point, uint256 scalar) internal view returns (Point memory) {
• function encryptMessage(Point memory encryptToPub, uint256 encryptWithPriv, bytes32 message)
• function decryptMessage(Point memory sharedPoint, bytes32 encryptedMessage)
• function hashPoint(Point memory point) internal pure returns (bytes32) {

Recommendation

Consider using calldata where possible.

Saved

This saves at least 60 gas per iteration.
So, ~60*5 = 300

G-2. Use unchecked blocks for incrementing i

Description

In Solidity 0.8+, there’s a default overflow and underflow check on unsigned integers. In the loops, "i" will not overflow because the loop will run out of gas before that.

Instances

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

Recommendation

Change:

for (uint256 i; i < n; ++i) {
 // ...
}

to:

for (uint256 i; i < n;) { 
 // ...
 unchecked { ++i; }
}

Saved

This saves ~30-40 gas per iteration.
So, ~35*2 = 70

G-3. Use unchecked blocks for subtractions where underflow is impossible

Description

In Solidity 0.8+, there’s a default overflow and underflow check on unsigned integers. When an overflow or underflow isn’t possible (after require or if-statement), some gas can be saved by using unchecked blocks.

Instances

• uint128 unsoldBase = data.totalBaseAmount - data.filledBase;
• currentTime - vestingStart

Saved

This saves ~35.
So, ~35*2 = 70

G-4. Elements that are smaller than 32 bytes (256 bits) may increase gas usage

Description

According to docs, 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.

Instances

• uint128 totalBaseAmount;
• uint128 filledBase;
• uint32 startTimestamp;
• uint32 endTimestamp;
• uint32 vestingStartTimestamp;
• uint32 vestingEndTimestamp;
• uint128 cliffPercent;
• uint128 lowestBase;
• uint128 lowestQuote;
• uint128 totalBaseAmount;
• uint128 minimumBidQuote;

Recommendation

Consider using a larger size where needed.