SIZE contest - B2's results

Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields.

    event AuctionCreated(
        uint256 auctionId, address seller, AuctionParameters params, Timings timings, bytes encryptedPrivKey

File: src/interfaces/ISizeSealed.sol (line 97-98)

    event Bid(
        address sender,
        uint256 auctionId,
        uint256 bidIndex,
        uint128 quoteAmount,
        bytes32 commitment,
        ECCMath.Point pubKey,
        bytes32 encryptedMessage,
        bytes encryptedPrivateKey

File: src/interfaces/ISizeSealed.sol (line 103-111)

       Other instances of this issue are:

• https://github.com/code-423n4/2022-11-size/blob/main/src/interfaces/ISizeSealed.sol#L118
• https://github.com/code-423n4/2022-11-size/blob/main/src/interfaces/ISizeSealed.sol#L122

Use of block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers, locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

        if (block.timestamp < a.timings.startTimestamp) {

File: src/SizeSealed.sol (line 29)

        } else if (block.timestamp < a.timings.endTimestamp) {

File: src/SizeSealed.sol (line 31)

       Other instances of this issue are:

• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L35
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L37
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L60
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L425
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L426
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L460

TYPOS

    ///@audit: @`runnning`
    /// @notice Bid on a runnning auction

File: src/SizeSealed.sol (line 112)

    ///@audit: @`futher`
    // Prevent any futher access to this EncryptedBid

File: src/SizeSealed.sol (line 431)

revert() should have descriptive reason strings

       30            if (_state != States.Created) revert InvalidState();
      
       32            if (_state != States.AcceptingBids) revert InvalidState();
 
       34            if (_state != States.Finalized) revert InvalidState();

       36            if (_state != States.RevealPeriod) revert InvalidState();

       38            if (_state != States.Voided) revert InvalidState();

       40            revert();

File: src/SizeSealed.sol (https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol)

All revert() in the file src/SizeSealed.sol lacks descriptive reason strings .

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

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.

        uint128 quoteAmount,

File: src/SizeSealed.sol (line 124)

        uint32 startTimestamp;
        uint32 endTimestamp;
        uint32 vestingStartTimestamp;
        uint32 vestingEndTimestamp;

File: src/interfaces/ISizeSealed.sol(line 56-59)

       Other instances of this issue are:

• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L205-L206
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L217
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L365
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L451
• https://github.com/code-423n4/2022-11-size/blob/main/src/util/CommonTokenMath.sol#L48-L52
• https://github.com/code-423n4/2022-11-size/blob/main/src/interfaces/ISizeSealed.sol#L65-L67

NatSpec is incomplete

     /// @audit Missing: '@param:auctionId`
     /// @notice Reveals the private key of the seller
     /// @dev All valid bids are decrypted after this
     ///      finalizeData should be empty if seller does not wish to finalize in this tx
     /// @param privateKey Private key corresponding to the auctions public key
     /// @param finalizeData Calldata that will be sent to finalize()
    function reveal(uint256 auctionId, uint256 privateKey, bytes calldata finalizeData)

• File: /src/SizeSealed.sol (line 172-177)

     /// @audit Missing: '@return`
    /// @notice calculates point^scalar
    /// @dev returns (1,1) if the ecMul failed or invalid parameters
    /// @return corresponding point
    function ecMul(Point memory point, uint256 scalar) internal view returns (Point memory) {

• File: src/util/ECCMath (line 22-25)

       Other instances of this issue are:

• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L33-L37
  ///@audit Missing: @retun
• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L48-L51
  ///@audit Missing: @retun
• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L16-L18
  ///@audit Missing: @retun&@param
• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L59-L60
  ///@audit Missing: @retun&@param

Use calldata instead of memory

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.

Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

    function encryptMessage(Point memory encryptToPub, uint256 encryptWithPriv, bytes32 message)

File: src/util/ECCMath.sol (line 37)

    function decryptMessage(Point memory sharedPoint, bytes32 encryptedMessage)

File: src/util/ECCMath.sol (line 51)

	Other instances of this issue are:

• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L60
• https://github.com/code-423n4/2022-11-size/blob/main/src/util/ECCMath.sol#L25
• https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L217

Use unchecked to save gas

                        baseAmount - cliffAmount, currentTime - vestingStart, vestingEnd - vestingStart

File: src/util/CommonTokenMath.sol (line 65)

Ununsed named return

Using both named returns and a return statement isn’t necessary. Removing one of those can improve code clarity:

        return encryptedMessage ^ hashPoint(sharedPoint);

File: src/util/ECCMath.sol (line 56)

        return CommonTokenMath.tokensAvailableAtTime(

File: src/SizeSealed.sol (line 56)

internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

    function encryptMessage(Point memory encryptToPub, uint256 encryptWithPriv, bytes32 message)
        internal

File: src/util/ECCMath.sol (line 37-38)

Cache a.timings to save gas

a.timings is used multiple times cache it in a memory variable to save gas.

    ///@audit: `a.timings` is used more than one time
        if (block.timestamp < a.timings.startTimestamp) {
            if (_state != States.Created) revert InvalidState();
        } else if (block.timestamp < a.timings.endTimestamp) {
            if (_state != States.AcceptingBids) revert InvalidState();
        } else if (a.data.lowestQuote != type(uint128).max) {
            if (_state != States.Finalized) revert InvalidState();
        } else if (block.timestamp <= a.timings.endTimestamp + 24 hours) {
            if (_state != States.RevealPeriod) revert InvalidState();
        } else if (block.timestamp > a.timings.endTimestamp + 24 hours) {

File: src/SizeSealed.sol (line 29-37)

internal functions not called by the contract should be removed to save deployment gas

    function decryptMessage(Point memory sharedPoint, bytes32 encryptedMessage)
        internal

File: src/util/ECCMath.sol (line 51-52)

abi.encode() is less efficient than abi.encodePacked()

        return keccak256(abi.encode(message));

File: src/SizeSealed.sol (line 467)