Tessera - Versus contest - IllIllI's results

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L265 https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L283

Vulnerability details

The return values of low-level calls are not checked

Impact

If the fund transfer results in a revert() on the recipient's end, e.g. due to being paused, the code will continue on as if it had been successful, and the Ether will be lost.

Proof of Concept

Return values of call() are not checked, and the amounts are deleted:

// File: src/modules/GroupBuy.sol : GroupBuy.claim()   #1

228        function claim(uint256 _poolId, bytes32[] calldata _mintProof) external {
229            // Reverts if pool ID is not valid
230            _verifyPool(_poolId);
231            // Reverts if purchase has not been made AND termination period has not passed
232            (, , , bool success, ) = _verifySuccessfulState(_poolId);
233            // Reverts if contribution balance of user is insufficient
234 @>         uint256 contribution = userContributions[_poolId][msg.sender];
235            if (contribution == 0) revert InsufficientBalance();
236    
237            // Deletes user contribution from storage
238 @>         delete userContributions[_poolId][msg.sender];
239    
...
264            // Transfers remaining contribution balance back to caller
265 @>         payable(msg.sender).call{value: contribution}("");
266    
267            // Withdraws pending balance of caller if available
268            if (pendingBalances[msg.sender] > 0) withdrawBalance();
269    
270            // Emits event for claiming tokens and receiving ether refund
271            emit Claim(_poolId, msg.sender, totalQty, contribution);
272:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L228-L272

// File: src/modules/GroupBuy.sol : GroupBuy.withdrawBalance()   #2

274        function withdrawBalance() public {
275            // Reverts if caller balance is insufficient
276 @>         uint256 balance = pendingBalances[msg.sender];
277            if (balance == 0) revert InsufficientBalance();
278    
279            // Resets pending balance amount
280 @>         delete pendingBalances[msg.sender];
281    
282            // Transfers pending ether balance to caller
283 @>         payable(msg.sender).call{value: balance}("");
284:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L274-L284

Either of these calls may be being made by a contract that relies on a bunch of other contracts, which may themselves be paused, so if the call reverts, there's no way to try again later.

Tools Used

Code inspection

Recommended Mitigation Steps

Check the return value and require() it to be success

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L204

Vulnerability details

The GroupBuy contract allows users to pool their funds in order to buy specific NFTs once enough funds have been raised. The purchace() function does not do any caller authorization and allows the caller to pass in an arbitrary address for executing the buy. The only checks done after the buy are that the NFT is held by the address that the buy function returns, which can be an arbitrary address owned by the attacker.

Impact

Funds from all of the users who deposited into the pool are used to buy an NFT that doesn't get added to a fractionalized vault, and instead is solely owned by the attacker.

Proof of Concept

The purchace function takes in a _market argument that has no validation done on it:

// File: src/modules/GroupBuy.sol : GroupBuy.purchace()   #1

160        function purchase(
161            uint256 _poolId,
162 @>         address _market,
163            address _nftContract,
164            uint256 _tokenId,
165            uint256 _price,
166            bytes memory _purchaseOrder,
167            bytes32[] memory _purchaseProof
168:       ) external {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L160-L168

After validating that the NFT address provided by the caller matches the expected one, _market has its execute() function called on it, and passes the funds held by the contract, to the function:

// File: src/modules/GroupBuy.sol : GroupBuy.purchase()   #2

198            // Encodes NFT contract and tokenId into purchase order
199            bytes memory nftData = abi.encode(_nftContract, _tokenId);
200            // Encodes arbitrary amount of data based on market buyer to execute purchase
201            _purchaseOrder = abi.encodePacked(nftData, _purchaseOrder);
202    
203            // Executes purchase order transaction through market buyer contract and deploys new vault
204 @>         address vault = IMarketBuyer(_market).execute{value: _price}(_purchaseOrder);
205    
206            // Checks if NFT contract supports ERC165 and interface ID of ERC721 tokens
207            if (ERC165Checker.supportsInterface(_nftContract, _INTERFACE_ID_ERC721)) {
208                // Verifes vault is owner of ERC-721 token
209 @>             if (IERC721(_nftContract).ownerOf(_tokenId) != vault) revert UnsuccessfulPurchase();
210            } else {
211                // Verifies vault is owner of CryptoPunk token
212 @>             if (ICryptoPunk(_nftContract).punkIndexToAddress(_tokenId) != vault)
213                    revert UnsuccessfulPurchase();
214:           }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L194-L214

The only checks done are to ensure that the NFT ends up being held by the vault address, returned by the _market.execute() call.

An attacker can create a contract that implements the IMarketBuyer interface, with an implementation of execute() that buys the NFT and sends it to an address controlled by the attacker, and then has execute() return that address as the 'vault'. No actual vault will be created, and the NFT will be under the attacker's full control.

Tools Used

Code inspection

Recommended Mitigation Steps

Ensure that the vault address returned by IMarketBuyer.execute() is in the vault registry

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L114-L150 https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L301-L303

Vulnerability details

If a user contributes funds after there is no more supply left, and they don't provide a price higher than the current minimum bid, they will be unable to withdraw their funds while the NFT remains unbought.

Impact

Ether becomes stuck until and unless the NFT is bought, which may never happen

Proof of Concept

When making a contribution, the user calls the payable contribute() function. If the supply has already been filled (fillAtAnyPriceQuantity is zero), the bid isn't inserted into the queue, so the new bid is not tracked anywhere. When the function reaches processBidsInQueue()...:

// File: src/modules/GroupBuy.sol : GroupBuy.contribute()   #1

99         function contribute(
100            uint256 _poolId,
101            uint256 _quantity,
102            uint256 _price
103 @>     ) public payable {
104            // Reverts if pool ID is not valid
105            _verifyPool(_poolId);
106            // Reverts if NFT has already been purchased OR termination period has passed
107            (, uint48 totalSupply, , , ) = _verifyUnsuccessfulState(_poolId);
108            // Reverts if ether contribution amount per Rae is less than minimum bid price per Rae
109            if (msg.value < _quantity * minBidPrices[_poolId] || _quantity == 0)
110                revert InvalidContribution();
111            // Reverts if ether payment amount is not equal to total amount being contributed
112            if (msg.value != _quantity * _price) revert InvalidPayment();
113    
114            // Updates user and pool contribution amounts
115            userContributions[_poolId][msg.sender] += msg.value;
116            totalContributions[_poolId] += msg.value;
117    
118            // Calculates remaining supply based on total possible supply and current filled quantity amount
119            uint256 remainingSupply = totalSupply - filledQuantities[_poolId];
120            // Calculates quantity amount being filled at any price
121            uint256 fillAtAnyPriceQuantity = remainingSupply < _quantity ? remainingSupply : _quantity;
122    
123            // Checks if quantity amount being filled is greater than 0
124 @>         if (fillAtAnyPriceQuantity > 0) {
125                // Inserts bid into end of queue
126                bidPriorityQueues[_poolId].insert(msg.sender, _price, fillAtAnyPriceQuantity);
127                // Increments total amount of filled quantities
128                filledQuantities[_poolId] += fillAtAnyPriceQuantity;
129            }
130    
131            // Calculates unfilled quantity amount based on desired quantity and actual filled quantity amount
132            uint256 unfilledQuantity = _quantity - fillAtAnyPriceQuantity;
133            // Processes bids in queue to recalculate unfilled quantity amount
134 @>         unfilledQuantity = processBidsInQueue(_poolId, unfilledQuantity, _price);
135    
136            // Recalculates filled quantity amount based on updated unfilled quantity amount
137            uint256 filledQuantity = _quantity - unfilledQuantity;
138            // Updates minimum reserve price if filled quantity amount is greater than 0
139            if (filledQuantity > 0) minReservePrices[_poolId] = getMinPrice(_poolId);
140    
141            // Emits event for contributing ether to pool based on desired quantity amount and price per Rae
142            emit Contribute(
143                _poolId,
144                msg.sender,
145                msg.value,
146                _quantity,
147                _price,
148                minReservePrices[_poolId]
149            );
150:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L99-L150

...if the price isn't higher than the lowest bid, the while loop is broken out of, with pendingBalances having never been updated, and the function does not revert:

// File: src/modules/GroupBuy.sol : GroupBuy.processBidsInQueue()   #2

291        function processBidsInQueue(
292            uint256 _poolId,
293            uint256 _quantity,
294            uint256 _price
295        ) private returns (uint256 quantity) {
296            quantity = _quantity;
297            while (quantity > 0) {
298                // Retrieves lowest bid in queue
299                Bid storage lowestBid = bidPriorityQueues[_poolId].getMin();
300                // Breaks out of while loop if given price is less than than lowest bid price
301 @>             if (_price < lowestBid.price) {
302 @>                 break;
303 @>             }
304    
305                uint256 lowestBidQuantity = lowestBid.quantity;
306                // Checks if lowest bid quantity amount is greater than given quantity amount
307                if (lowestBidQuantity > quantity) {
308                    // Decrements given quantity amount from lowest bid quantity
309                    lowestBid.quantity -= quantity;
310                    // Calculates partial contribution of bid by quantity amount and price
311                    uint256 contribution = quantity * lowestBid.price;
312    
313:                   // Decrements partial contribution amount of lowest bid from total and user contributions

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L291-L313

In order for a user to get funds back, the amount must have been stored in pendingBalances, and since this is never done, all funds contributed during the contribute() call become property of the GroupBuy contract, with the user being unable to withdraw...:

// File: src/modules/GroupBuy.sol : GroupBuy.withdrawBalance()   #3

274        function withdrawBalance() public {
275            // Reverts if caller balance is insufficient
276 @>         uint256 balance = pendingBalances[msg.sender];
277 @>         if (balance == 0) revert InsufficientBalance();
278    
279            // Resets pending balance amount
280            delete pendingBalances[msg.sender];
281    
282            // Transfers pending ether balance to caller
283            payable(msg.sender).call{value: balance}("");
284:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L274-L284

...until the order has gone through, and they can claim() excess funds, but there likely won't be any, due to the separate MEV bug I raised:

// File: src/modules/GroupBuy.sol : GroupBuy.contribution   #4

228        function claim(uint256 _poolId, bytes32[] calldata _mintProof) external {
229            // Reverts if pool ID is not valid
230            _verifyPool(_poolId);
231            // Reverts if purchase has not been made AND termination period has not passed
232            (, , , bool success, ) = _verifySuccessfulState(_poolId);
233            // Reverts if contribution balance of user is insufficient
234 @>         uint256 contribution = userContributions[_poolId][msg.sender];
235            if (contribution == 0) revert InsufficientBalance();
236    
237            // Deletes user contribution from storage
238            delete userContributions[_poolId][msg.sender];

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L228-L244

Tools Used

Code inspection

Recommended Mitigation Steps

revert() if the price is lower than the min bid, and the queue is already full

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L299-L319

Vulnerability details

The README states If two users place bids at the same price but with different quantities, the queue will pull from the bid with a higher quantity first, but the data-structure used for implementing this logic, is not used properly and essentially has its data corrupted when a large bid that is the current minimum bid, is split into two parts, so that a more favorable price can be used for a fraction of the large bid. The underlying issue is that one of the tree nodes is modified, without re-shuffling that node's location in the tree.

Impact

The minimum bid as told by the priority queue will be wrong, leading to the wrong bids being allowed to withdraw their funds, and being kicked out of the fraction of bids that are used to buy the NFT.

Proof of Concept

The priority queue using a binary tree within an array to efficiently navigate and find the current minimum based on a node and it children. The sorting of the nodes in the tree is based, in part, on the quantity in the case where two bids have the same price:

// File: src/lib/MinPriorityQueue.sol : MinPriorityQueue.isGreater()   #1

111        function isGreater(
112            Queue storage self,
113            uint256 i,
114            uint256 j
115        ) private view returns (bool) {
116            Bid memory bidI = self.bidIdToBidMap[self.bidIdList[i]];
117            Bid memory bidJ = self.bidIdToBidMap[self.bidIdList[j]];
118 @>         if (bidI.price == bidJ.price) {
119 @>             return bidI.quantity <= bidJ.quantity;
120            }
121            return bidI.price > bidJ.price;
122:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L111-L122

The algorithm of the binary tree only works when the nodes are properly sorted. The sorting is corrupted when a node is modified, without removing it from the tree and re-inserting it:

// File: src/modules/GroupBuy.sol : GroupBuy.processBidsInQueue()   #2

299                Bid storage lowestBid = bidPriorityQueues[_poolId].getMin();
300                // Breaks out of while loop if given price is less than than lowest bid price
301                if (_price < lowestBid.price) {
302                    break;
303                }
304    
305                uint256 lowestBidQuantity = lowestBid.quantity;
306                // Checks if lowest bid quantity amount is greater than given quantity amount
307                if (lowestBidQuantity > quantity) {
308                    // Decrements given quantity amount from lowest bid quantity
309 @>                 lowestBid.quantity -= quantity;
310                    // Calculates partial contribution of bid by quantity amount and price
311                    uint256 contribution = quantity * lowestBid.price;
312    
313                    // Decrements partial contribution amount of lowest bid from total and user contributions
314                    totalContributions[_poolId] -= contribution;
315                    userContributions[_poolId][lowestBid.owner] -= contribution;
316                    // Increments pending balance of lowest bid owner
317                    pendingBalances[lowestBid.owner] += contribution;
318    
319:                   // Inserts new bid with given quantity amount into proper position of queue

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L299-L319

Let's say that the tree looks like this:

            A:(p:100,q:10)
            /             \
       B:(p:100,q:10)  C:(<whatever>)
       /           \
D:(whatever)   E:(whatever) 

If A is modified so that q (quantity) goes from 10 to 5, B should now be at the root of the tree, since it has the larger size, and would be considered the smaller node. When another node is added, say, F:(p:100,q:6), the algorithm will see that F has a larger size than A, and so A will be popped out as the min, even though B should have been. All nodes that are under B (which may be a lot of the nodes if they all entered at the same price/quantity) essentially become invisible under various scenarios, which means the users that own those bids will not be able to withdraw their funds, even if they really are the lowest bid that deserves to be pushed out of the queue. Note that the swimming up that is done for F will not re-shuffle B since, according to the algorithm, F will start as a child of C, and B is not in the list of parent nodes of C.

Tools Used

Code inspection

Recommended Mitigation Steps

When modifying nodes of the tree, remove them first, then re-add them after modification

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L40

Vulnerability details

Not all IERC20 implementations revert() when there's a failure in approve(). If one of these tokens returns false, there is no check for whether this has happened during the order listing validation, so it will only be detected when the order is attempted.

Impact

If the approval failure isn't detected, the listing will never be fillable, because the funds won't be able to be pulled from the opensea conduit. Once this happens, and if it's detected, the only way to fix it is to create a counter-listing at a lower price (which may be below the market value of the tokens), waiting for the order to expire (which it may never), or by buying out all of the Rae to cancel the order (very expensive and defeats the purpose of pooling funds in the first place).

Proof of Concept

The return value of approve() isn't checked, so the order will be allowed to be listed without having approved the conduit:

// File: src/seaport/targets/SeaportLister.sol : SeaportLister.validateListing()   #1

29                for (uint256 i; i < ordersLength; ++i) {
30                    uint256 offerLength = _orders[i].parameters.offer.length;
31                    for (uint256 j; j < offerLength; ++j) {
32                        OfferItem memory offer = _orders[i].parameters.offer[j];
33                        address token = offer.token;
34                        ItemType itemType = offer.itemType;
35                        if (itemType == ItemType.ERC721)
36                            IERC721(token).setApprovalForAll(conduit, true);
37                        if (itemType == ItemType.ERC1155)
38                            IERC1155(token).setApprovalForAll(conduit, true);
39                        if (itemType == ItemType.ERC20)
40 @>                         IERC20(token).approve(conduit, type(uint256).max);
41                    }
42                }
43            }
44            // Validates the order on-chain so no signature is required to fill it
45            assert(ConsiderationInterface(_consideration).validate(_orders));
46:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L29-L46

Tools Used

Code inspection

Recommended Mitigation Steps

Use OpenZeppelin's safeApprove(), which checks the return code and reverts if it's not success

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L29-L46

Vulnerability details

Calling approve() without first calling approve(0) if the current approval is non-zero will revert with some tokens, such as Tether (USDT). While Tether is known to do this, it applies to other tokens as well, which are trying to protect against this attack vector.

Impact

Only the first listing will start with the conduit's approval at zero and will be able to change it to the maximum. Thereafter, every attempt to approve that same token will revert, causing any order using this lister to revert, including a re-listing at a lower price, which the protocol allows for.

Proof of Concept

The seaport conduit address is set in the constructor as an immutable variable, so it's not possible to change it once the issue is hit:

// File: src/seaport/targets/SeaportLister.sol : SeaportLister.constructor()   #1

19        constructor(address _conduit) {
20 @>         conduit = _conduit;
21:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L19-L21

The approval is set to the maximum without check whether it's already the maximum:

// File: src/seaport/targets/SeaportLister.sol : SeaportLister.validateListing()   #2

29                for (uint256 i; i < ordersLength; ++i) {
30                    uint256 offerLength = _orders[i].parameters.offer.length;
31                    for (uint256 j; j < offerLength; ++j) {
32                        OfferItem memory offer = _orders[i].parameters.offer[j];
33                        address token = offer.token;
34                        ItemType itemType = offer.itemType;
35                        if (itemType == ItemType.ERC721)
36                            IERC721(token).setApprovalForAll(conduit, true);
37                        if (itemType == ItemType.ERC1155)
38                            IERC1155(token).setApprovalForAll(conduit, true);
39                        if (itemType == ItemType.ERC20)
40 @>                         IERC20(token).approve(conduit, type(uint256).max);
41                    }
42                }
43            }
44            // Validates the order on-chain so no signature is required to fill it
45            assert(ConsiderationInterface(_consideration).validate(_orders));
46:       }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L29-L46

The README states: The Tessera Protocol is designed around the concept of Hyperstructures, which are crypto protocols that can run for free and forever, without maintenance, interruption or intermediaries, and having to deploy a new SeaportLister in order to have a fresh conduit that also needs to be deployed, is not without maintenance or interruption.

Tools Used

Code inspection

Recommended Mitigation Steps

Always reset the approval to zero before changing it to the maximum

Summary

Low Risk Issues

Total: 18 instances over 5 issues

Non-critical Issues

Total: 33 instances over 9 issues

Low Risk Issues

[L‑01] Bid size is an unfair ordering metric

The README states that this is intentional, so I've filed it as Low rather than Medium, but giving priority to bids with the smaller quantity is not a fair ordering mechanic. A person with a lot of funds may have gotten that by pooling externally to the contract, and it's not fair to kick them out of the pool earlier than another address that came in later

There is 1 instance of this issue:

File: /src/lib/MinPriorityQueue.sol

111      function isGreater(
112          Queue storage self,
113          uint256 i,
114          uint256 j
115      ) private view returns (bool) {
116          Bid memory bidI = self.bidIdToBidMap[self.bidIdList[i]];
117          Bid memory bidJ = self.bidIdToBidMap[self.bidIdList[j]];
118          if (bidI.price == bidJ.price) {
119              return bidI.quantity <= bidJ.quantity;
120          }
121          return bidI.price > bidJ.price;
122:     }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L111-L122

[L‑02] Users may DOS themselves with a lot of smalle payments

If a user has to contribute to a pool via lots of dust payments (e.g. if they only have enough money each week to spend a few wei), they may eventually add enough payments that when it's time to claim their excess, their for-loop below exceeds the block gas limit

There is 1 instance of this issue:

File: /src/modules/GroupBuy.sol

247          if (success) {
248              for (uint256 i; i < length; ++i) {
249                  // Gets bid quantity from storage
250:                 Bid storage bid = bidPriorityQueues[_poolId].bidIdToBidMap[bidIds[i]];

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L247-L250

[L‑03] Empty receive()/payable fallback() function does not authorize requests

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))). Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds. If the concern is having to spend a small amount of gas to check the sender against an immutable address, the code should at least have a function to rescue unused Ether.

There are 2 instances of this issue:

File: src/punks/protoforms/PunksMarketBuyer.sol

41:       receive() external payable {}

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/punks/protoforms/PunksMarketBuyer.sol#L41

File: src/seaport/modules/OptimisticListingSeaport.sol

83:       receive() external payable {}

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L83

[L‑04] require() should be used instead of assert()

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

There are 2 instances of this issue:

File: src/seaport/targets/SeaportLister.sol

45:           assert(ConsiderationInterface(_consideration).validate(_orders));

52:           assert(ConsiderationInterface(_consideration).cancel(_orders));

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L45

[L‑05] Missing checks for address(0x0) when assigning values to address state variables

There are 12 instances of this issue:

File: src/punks/protoforms/PunksMarketBuyer.sol

32:           registry = _registry;

33:           wrapper = _wrapper;

34:           listing = _listing;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/punks/protoforms/PunksMarketBuyer.sol#L32

File: src/seaport/modules/OptimisticListingSeaport.sol

71:           registry = _registry;

72:           seaport = _seaport;

73:           zone = _zone;

75:           supply = _supply;

76:           seaportLister = _seaportLister;

77:           feeReceiver = _feeReceiver;

78:           OPENSEA_RECIPIENT = _openseaRecipient;

338:          feeReceiver = _new;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L71

File: src/seaport/targets/SeaportLister.sol

20:           conduit = _conduit;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L20

Non-critical Issues

[N‑01] Debugging functions should be moved to a child class rather than being deployed

There is 1 instance of this issue:

File: /src/modules/GroupBuy.sol

402      function printQueue(uint256 _poolId) public view {
403          uint256 counter;
404          uint256 index = 1;
405          MinPriorityQueue.Queue storage queue = bidPriorityQueues[_poolId];
406          uint256 numBids = queue.numBids;
407          while (counter < numBids) {
408              Bid memory bid = queue.bidIdToBidMap[index];
409              if (bid.bidId == 0) {
410                  ++index;
411                  continue;
412              }
413              ++index;
414              ++counter;
415          }
416:     }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L402-L416

[N‑02] Typos

There are 4 instances of this issue:

File: src/lib/MinPriorityQueue.sol

/// @audit addreses
22:           ///@notice map addreses to bids they own

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L22

File: src/modules/GroupBuy.sol

/// @audit equalt
179:          // Reverts if NFT contract is not equalt to NFT contract set on pool creation

/// @audit Verifes
208:              // Verifes vault is owner of ERC-721 token

/// @audit specifc
286:      /// @notice Attempts to accept bid for specifc quantity and price

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L179

[N‑03] public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

There are 10 instances of this issue:

File: src/lib/MinPriorityQueue.sol

27:       function initialize(Queue storage self) public {

36:       function getNumBids(Queue storage self) public view returns (uint256) {

41:       function getMin(Queue storage self) public view returns (Bid storage) {

90:       function delMin(Queue storage self) public returns (Bid memory) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L27

File: src/modules/GroupBuy.sol

346       function getBidInQueue(uint256 _poolId, uint256 _bidId)
347           public
348           view
349           returns (
350               uint256 bidId,
351               address owner,
352               uint256 price,
353:              uint256 quantity

371:      function getNextBidId(uint256 _poolId) public view returns (uint256) {

377:      function getNumBids(uint256 _poolId) public view returns (uint256) {

384:      function getBidQuantity(uint256 _poolId, uint256 _bidId) public view returns (uint256) {

402:      function printQueue(uint256 _poolId) public view {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L346-L353

File: src/seaport/modules/OptimisticListingSeaport.sol

218:      function list(address _vault, bytes32[] calldata _listProof) public {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L218

[N‑04] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 4 instances of this issue:

File: src/seaport/modules/OptimisticListingSeaport.sol

/// @audit 3
350:          permissions = new Permission[](3);

/// @audit 3
385:              orderParams.totalOriginalConsiderationItems = 3;

/// @audit 40
395:          uint256 openseaFees = _listingPrice / 40;

/// @audit 20
396:          uint256 tesseraFees = _listingPrice / 20;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L350

[N‑05] Missing event and or timelock for critical parameter change

Events help non-contract tools to track changes, and events prevent users from being surprised by changes

There is 1 instance of this issue:

File: src/seaport/modules/OptimisticListingSeaport.sol

336       function updateFeeReceiver(address payable _new) external {
337           if (msg.sender != feeReceiver) revert NotAuthorized();
338           feeReceiver = _new;
339:      }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L336-L339

[N‑06] NatSpec is incomplete

There are 7 instances of this issue:

File: src/modules/GroupBuy.sol

/// @audit Missing: '@return'
344       /// @param _poolId ID of the pool
345       /// @param _bidId ID of the bid in queue
346       function getBidInQueue(uint256 _poolId, uint256 _bidId)
347           public
348           view
349           returns (
350               uint256 bidId,
351               address owner,
352               uint256 price,
353:              uint256 quantity

/// @audit Missing: '@return'
363       /// @notice Gets minimum bid price of queue for given pool
364       /// @param _poolId ID of the pool
365:      function getMinPrice(uint256 _poolId) public view returns (uint256) {

/// @audit Missing: '@return'
369       /// @notice Gets next bidId in queue of given pool
370       /// @param _poolId ID of the pool
371:      function getNextBidId(uint256 _poolId) public view returns (uint256) {

/// @audit Missing: '@return'
375       /// @notice Gets total number of bids in queue for given pool
376       /// @param _poolId ID of the pool
377:      function getNumBids(uint256 _poolId) public view returns (uint256) {

/// @audit Missing: '@return'
382       /// @param _poolId ID of the pool
383       /// @param _bidId ID of the bid in queue
384:      function getBidQuantity(uint256 _poolId, uint256 _bidId) public view returns (uint256) {

/// @audit Missing: '@return'
389       /// @param _poolId ID of the pool
390       /// @param _owner Address of the owner
391       function getOwnerToBidIds(uint256 _poolId, address _owner)
392           public
393           view
394:          returns (uint256[] memory)

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L344-L353

File: src/punks/protoforms/PunksMarketBuyer.sol

/// @audit Missing: '@return'
44        /// @param _order Bytes value of the necessary order parameters
45        /// return vault Address of the deployed vault
46:       function execute(bytes memory _order) external payable returns (address vault) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/punks/protoforms/PunksMarketBuyer.sol#L44-L46

[N‑07] Consider using delete rather than assigning zero to clear values

The delete keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic

There are 4 instances of this issue:

File: src/modules/GroupBuy.sol

253:                  bid.quantity = 0;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L253

File: src/seaport/modules/OptimisticListingSeaport.sol

293:              activeListing.collateral = 0;

325:          pendingBalances[_vault][_to] = 0;

437:          orderParams.totalOriginalConsiderationItems = 0;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L293

[N‑08] Contracts should have full test coverage

While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.

There is 1 instance of this issue:

File: src/lib/MinPriorityQueue.sol

[N‑09] Large or complicated code bases should implement fuzzing tests

Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement fuzzing tests. Fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.

There is 1 instance of this issue:

File: src/lib/MinPriorityQueue.sol

Summary

Gas Optimizations

Total: 39 instances over 14 issues with 5642 gas saved

Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.

Gas Optimizations

[G‑01] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

There are 2 instances of this issue:

File: src/modules/GroupBuy.sol

28        mapping(uint256 => address) public poolToVault;
29        /// @notice Mapping of pool ID to PoolInfo struct
30        mapping(uint256 => PoolInfo) public poolInfo;
31        /// @notice Mapping of pool ID to the priority queue of valid bids
32        mapping(uint256 => MinPriorityQueue.Queue) public bidPriorityQueues;
33        /// @notice Mapping of pool ID to amount of Raes currently filled for the pool
34        mapping(uint256 => uint256) public filledQuantities;
35        /// @notice Mapping of pool ID to minimum ether price of any bid
36        mapping(uint256 => uint256) public minBidPrices;
37        /// @notice Mapping of pool ID to minimum reserve prices
38        mapping(uint256 => uint256) public minReservePrices;
39        /// @notice Mapping of pool ID to total amount of ether contributed
40        mapping(uint256 => uint256) public totalContributions;
41        /// @notice Mapping of pool ID to user address to total amount of ether contributed
42:       mapping(uint256 => mapping(address => uint256)) public userContributions;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L28-L42

File: src/seaport/modules/OptimisticListingSeaport.sol

50        mapping(address => bytes32) public vaultOrderHash;
51        /// @notice Mapping of vault address to active listings on Seaport
52        mapping(address => Listing) public activeListings;
53        /// @notice Mapping of vault address to newly proposed listings
54        mapping(address => Listing) public proposedListings;
55        /// @notice Mapping of vault address to user address to collateral amount
56:       mapping(address => mapping(address => uint256)) public pendingBalances;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L50-L56

[G‑02] Using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved

There are 4 instances of this issue:

File: src/modules/GroupBuy.sol

/// @audit _purchaseProof
160       function purchase(
161           uint256 _poolId,
162           address _market,
163           address _nftContract,
164           uint256 _tokenId,
165           uint256 _price,
166           bytes memory _purchaseOrder,
167:          bytes32[] memory _purchaseProof

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L160-L167

File: src/punks/protoforms/PunksMarketBuyer.sol

/// @audit _order
46:       function execute(bytes memory _order) external payable returns (address vault) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/punks/protoforms/PunksMarketBuyer.sol#L46

File: src/seaport/targets/SeaportLister.sol

/// @audit _orders
26:       function validateListing(address _consideration, Order[] memory _orders) external {

/// @audit _orders
51:       function cancelListing(address _consideration, OrderComponents[] memory _orders) external {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L26

[G‑03] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

There is 1 instance of this issue:

File: src/modules/GroupBuy.sol

408:              Bid memory bid = queue.bidIdToBidMap[index];

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L408

[G‑04] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. 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.

There are 4 instances of this issue:

File: src/modules/GroupBuy.sol

/// @audit currentId on line 74
83:           minBidPrices[currentId] = _initialPrice / _totalSupply;

/// @audit currentId on line 83
86:           bidPriorityQueues[currentId].initialize();

/// @audit currentId on line 86
89:           emit Create(currentId, _nftContract, _tokenIds, msg.sender, _totalSupply, _duration);

/// @audit currentId on line 89
92:           contribute(currentId, _quantity, _raePrice);

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L83

[G‑05] Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata

There are 4 instances of this issue:

File: src/modules/GroupBuy.sol

/// @audit bidPriorityQueues[_poolId] on line 299
320:                  bidPriorityQueues[_poolId].insert(msg.sender, _price, quantity);

/// @audit bidPriorityQueues[_poolId] on line 320
334:                  bidPriorityQueues[_poolId].delMin();

/// @audit bidPriorityQueues[_poolId] on line 334
336:                  bidPriorityQueues[_poolId].insert(msg.sender, _price, lowestBidQuantity);

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L320

File: src/seaport/modules/OptimisticListingSeaport.sol

/// @audit activeListings[_vault] on line 107
115:              _pricePerToken >= activeListings[_vault].pricePerToken

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L115

[G‑06] 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.

There are 5 instances of this issue:

File: src/modules/GroupBuy.sol

419       function _generateRoot(uint256[] calldata _tokenIds)
420           internal
421           pure
422:          returns (bytes32 merkleRoot)

466       function _verifySuccessfulState(uint256 _poolId)
467           internal
468           view
469           returns (
470               address,
471               uint48,
472               uint40,
473               bool,
474:              bytes32

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L419-L422

File: src/punks/protoforms/PunksMarketBuyer.sol

71        function _deployVault(uint256 _punkId)
72            internal
73:           returns (address vault, bytes32[] memory unwrapProof)

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/punks/protoforms/PunksMarketBuyer.sol#L71-L73

File: src/seaport/modules/OptimisticListingSeaport.sol

368       function _constructOrder(
369           address _vault,
370           uint256 _listingPrice,
371:          OfferItem[] calldata _offer

481       function _getOrderHash(OrderParameters memory _orderParams, uint256 _counter)
482           internal
483           view
484:          returns (bytes32 orderHash)

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L368-L371

[G‑07] <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

• storage arrays incur a Gwarmaccess (100 gas)
• memory arrays use MLOAD (3 gas)
• calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 2 instances of this issue:

File: src/lib/MinPriorityQueue.sol

98:           for (uint256 i = 0; i < curUserBids.length; i++) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L98

File: src/seaport/modules/OptimisticListingSeaport.sol

390:              for (uint256 i = 0; i < _offer.length; ++i) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L390

[G‑08] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

There are 2 instances of this issue:

File: src/lib/MinPriorityQueue.sol

98:           for (uint256 i = 0; i < curUserBids.length; i++) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L98

File: src/modules/GroupBuy.sol

248:              for (uint256 i; i < length; ++i) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L248

[G‑09] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 5 instances of this issue:

File: src/lib/MinPriorityQueue.sol

/// @audit initialize(), isEmpty(), getNumBids(), getMin(), insert(), delMin()
12:   library MinPriorityQueue {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L12

File: src/modules/GroupBuy.sol

/// @audit createPool(), contribute(), purchase(), claim(), withdrawBalance(), getBidInQueue(), getMinPrice(), getNextBidId(), getNumBids(), getBidQuantity(), getOwnerToBidIds(), printQueue()
20:   contract GroupBuy is IGroupBuy, MerkleBase, Minter {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L20

File: src/punks/protoforms/PunksMarketBuyer.sol

/// @audit execute()
16:   contract PunksMarketBuyer is IPunksMarketBuyer, Protoform {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/punks/protoforms/PunksMarketBuyer.sol#L16

File: src/seaport/modules/OptimisticListingSeaport.sol

/// @audit propose(), rejectProposal(), rejectActive(), list(), cancel(), cash(), withdrawCollateral(), updateFeeReceiver()
23:   contract OptimisticListingSeaport is

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L23

File: src/seaport/targets/SeaportLister.sol

/// @audit validateListing(), cancelListing()
15:   contract SeaportLister is ISeaportLister {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/targets/SeaportLister.sol#L15

[G‑10] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

There are 2 instances of this issue:

File: src/lib/MinPriorityQueue.sol

60:                   j++;

98:           for (uint256 i = 0; i < curUserBids.length; i++) {

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L60

[G‑11] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

There are 2 instances of this issue:

File: src/seaport/modules/OptimisticListingSeaport.sol

38:       bytes32 public immutable conduitKey;

44:       uint256 public immutable PROPOSAL_PERIOD;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L38

[G‑12] Inverting the condition of an if-else-statement wastes gas

Flipping the true and false blocks instead saves 3 gas

There is 1 instance of this issue:

File: src/seaport/modules/OptimisticListingSeaport.sol

289           if (!_verifySale(_vault)) {
290               revert NotSold();
291           } else if (activeListing.collateral != 0) {
292               uint256 collateral = activeListing.collateral;
293               activeListing.collateral = 0;
294               // Sets collateral amount to pending balances for withdrawal
295               pendingBalances[_vault][activeListing.proposer] = collateral;
296:          }

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/seaport/modules/OptimisticListingSeaport.sol#L289-L296

[G‑13] Division by two should use bit shifting

<x> / 2 is the same as <x> >> 1. While the compiler uses the SHR opcode to accomplish both, the version that uses division incurs an overhead of 20 gas due to JUMPs to and from a compiler utility function that introduces checks which can be avoided by using unchecked {} around the division by two

There are 3 instances of this issue:

File: src/lib/MinPriorityQueue.sol

49:           while (k > 1 && isGreater(self, k / 2, k)) {

50:               exchange(self, k, k / 2);

51:               k = k / 2;

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L49

[G‑14] Use custom errors rather than revert()/require() strings to save gas

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

There are 2 instances of this issue:

File: src/lib/MinPriorityQueue.sol

42:           require(!isEmpty(self), "nothing to return");

91:           require(!isEmpty(self), "nothing to delete");

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/lib/MinPriorityQueue.sol#L42