QA Report

[code423n4]

dont use charot in solidtiy files because you can have issues with deployment and testing version causing weird bugs

pragma solidity ^0.8.0;

its in most of the main contracts https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L3

remove dead code

https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/shared/MarketFees.sol#L37-L40

you can remove that gap because they have alot of them in the drevitve contracts

https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/shared/MarketFees.sol#L42

accounting of burning can happen only when there are enough nfts

when burnCounter overflow to zero then we dont know that what is burned which then only has tokendIds and not minus the burnedTokens

      supply = latestTokenId - burnCounter;
    }

https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/collections/SequentialMintCollection.sol#L121

creator can decrease maxTokenId and make it more rare

creator says maxTokens is 100 but then one day the creators is like lets make it more rare so he makes Maxtokens to 50 causing users destrust of how many are going to be out there. There can even have a dos after like 1 users mints because then if max tokens is 2 and admin buy one and a user makes a tx to buy one then the admin can frontrun to make it the users tx revert and cause gas funds to be lost . make maxtokens a immutable to give users more trust in buying nfts from collectors. https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/collections/SequentialMintCollection.sol#L86

if there is one nft left it wouldnt be fair to a user who makes tx then a frontruning bot makes the same tx bu with more gas and the frontrun bot gets the nft.

make it some how better then having a gas off. myabe tell users to use private tx,flashbots to avoid this. https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L170

there an be a underflow but since its not in a unchecked will revert

10-20-1= -11 which since uint and not in unchecked block it will revert so on certin times when users are buying nfts it will revert.

        // 10 -20 - 1 = -11 
        sellerRev = price - totalFees - creatorRev;
      }

https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/shared/MarketFees.sol#L473 make it into a unchecked block

---

this comment didnt happen,replace it or change the varaible

  /// @dev This value was replaced with an immutable version.
  address payable private __gap_was_treasury;

https://github.com/code-423n4/2022-08-foundation/blob/b4e1dc6a9a434d576cb8f74676e7dbb65dacc98d/contracts/mixins/shared/FoundationTreasuryNode.sol#L22

remove todos its just best pratcise

https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/mixins/shared/MarketFees.sol#L193

Make event with 3 fields or more make them indexed

https://github.com/code-423n4/2022-08-foundation/blob/254d384116d22266b12916f1aeb46b9ef0143f1f/contracts/NFTCollectionFactory.sol#L133 https://github.com/code-423n4/2022-08-foundation/blob/254d384116d22266b12916f1aeb46b9ef0143f1f/contracts/NFTCollectionFactory.sol#L158

[HardlyDifficult]

Use fixed pragma

Disagree. We intentionally use a floating pragma in order to make integrating with contracts easier. Other contract developers are looking to interact with our contracts and they may be on a different version than we use. The pragma selected for our contracts is the minimum required in order to correctly compile and function. This way integration is easier if they lag a few versions behind, or if they use the latest but we don't bump our packages frequently enough, and when we do upgrade versions unless there was a breaking solidity change -- it should just swap in by incrementing our npm package version.

Remove commented out code

Disagree - this comment is detailing how that awkwardly placed gap was previously used so future me can understand why it's important for upgrade compatibility that we don't leverage that space again (since those slots are dirty with data).

you can remove that gap because they have alot of them in the drevitve contracts

Disagree. As the comment there is trying to convey, the data at those locations is dirty and should not be leveraged again in the future.

accounting of burning can happen only when there are enough nfts

Invalid, burnCounter cannot be made to overflow.

creator can decrease maxTokenId and make it more rare

Disagree / by design. See also https://github.com/code-423n4/2022-08-foundation-findings/issues/254

[HickupHH3]

if there is one nft left it wouldnt be fair to a user who makes tx then a frontruning bot makes the same tx bu with more gas and the frontrun bot gets the nft.

similar to botting issue #93, though unclear why it applies only to the "1 NFT" and not the entire sale.

there an be a underflow but since its not in a unchecked will revert

disagree, underflow cannot happen because totalFees + creatorRev cannot exceed price.

other issues that were not commented by the sponsor have been mentioned by other wardens, will score accordingly.