Platform: Code4rena
Start Date: 11/08/2022
Pot Size: $40,000 USDC
Total HM: 8
Participants: 108
Period: 4 days
Judge: hickuphh3
Total Solo HM: 2
Id: 152
League: ETH
Rank: 89/108
Findings: 1
Award: $21.30
馃専 Selected for report: 0
馃殌 Solo Findings: 0
馃専 Selected for report: Dravee
Also found by: 0x040, 0x1f8b, 0xDjango, 0xHarry, 0xNazgul, 0xSmartContract, 0xbepresent, 0xkatana, Amithuddar, Aymen0909, Bnke0x0, Chom, CodingNameKiki, Deivitto, DevABDee, Diraco, ElKu, Fitraldys, Funen, IllIllI, JC, LeoS, Metatron, MiloTruck, Noah3o6, ReyAdmirado, Rohan16, Rolezn, Saw-mon_and_Natalie, Sm4rty, SpaceCake, TomJ, Tomio, Trabajo_de_mates, Waze, Yiko, __141345__, ajtra, apostle0x01, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, d3e4, durianSausage, erictee, fatherOfBlocks, gerdusx, gogo, hakerbaya, ignacio, jag, joestakey, ladboy233, medikko, mics, newfork01, oyc_109, pfapostol, robee, rvierdiiev, sach1r0, saian, samruna, sikorico, simon135, wagmi, zeesaw, zkhorse, zuhaibmohd
21.2979 USDC - $21.30
https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/NFTCollectionFactory.sol
diff --git a/contracts/NFTCollectionFactory.sol b/contracts/NFTCollectionFactory.sol index 008ad9b..22022ce 100644 --- a/contracts/NFTCollectionFactory.sol +++ b/contracts/NFTCollectionFactory.sol @@ -202,19 +202,22 @@ contract NFTCollectionFactory is ICollectionFactory, Initializable, Gap10000 { function adminUpdateNFTCollectionImplementation(address _implementation) external onlyAdmin { require(_implementation.isContract(), "NFTCollectionFactory: Implementation is not a contract"); implementationNFTCollection = _implementation; + + //Use local variable to point versionNFTCollection + uint32 _versionNFTCollection; unchecked { // Version cannot overflow 256 bits. - versionNFTCollection++; + _versionNFTCollection = ++versionNFTCollection; } // The implementation is initialized when assigned so that others may not claim it as their own. INFTCollectionInitializer(_implementation).initialize( payable(address(rolesContract)), - string.concat("NFT Collection Implementation v", versionNFTCollection.toString()), - string.concat("NFTv", versionNFTCollection.toString()) + string.concat("NFT Collection Implementation v", _versionNFTCollection.toString()), + string.concat("NFTv", _versionNFTCollection.toString()) ); - emit ImplementationNFTCollectionUpdated(_implementation, versionNFTCollection); + emit ImplementationNFTCollectionUpdated(_implementation, _versionNFTCollection); } /** @@ -226,18 +229,21 @@ contract NFTCollectionFactory is ICollectionFactory, Initializable, Gap10000 { function adminUpdateNFTDropCollectionImplementation(address _implementation) external onlyAdmin { require(_implementation.isContract(), "NFTCollectionFactory: Implementation is not a contract"); implementationNFTDropCollection = _implementation; + + //Use local variable to point versionNFTDropCollection + uint32 _versionNFTDropCollection; unchecked { // Version cannot overflow 256 bits. - versionNFTDropCollection++; + _versionNFTDropCollection = ++versionNFTDropCollection; } - emit ImplementationNFTDropCollectionUpdated(_implementation, versionNFTDropCollection); + emit ImplementationNFTDropCollectionUpdated(_implementation, _versionNFTDropCollection); // The implementation is initialized when assigned so that others may not claim it as their own. INFTDropCollectionInitializer(_implementation).initialize( payable(address(this)), - string.concat("NFT Drop Collection Implementation v", versionNFTDropCollection.toString()), - string.concat("NFTDropV", versionNFTDropCollection.toString()), + string.concat("NFT Drop Collection Implementation v", _versionNFTDropCollection.toString()), + string.concat("NFTDropV", _versionNFTDropCollection.toString()), "ipfs://bafybeibvxnuaqtvaxu26gdgly2rm4g2piu7b2tqlx2dsz6wwhqbey2gddy/", 0x1337000000000000000000000000000000000000000000000000000000001337, 1,
https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/NFTDropCollection.sol
diff --git a/contracts/NFTDropCollection.sol b/contracts/NFTDropCollection.sol index df52ae3..7032c4f 100644 --- a/contracts/NFTDropCollection.sol +++ b/contracts/NFTDropCollection.sol @@ -171,19 +171,23 @@ contract NFTDropCollection is function mintCountTo(uint16 count, address to) external onlyMinterOrAdmin returns (uint256 firstTokenId) { require(count != 0, "NFTDropCollection: `count` must be greater than 0"); + // Use local variable to point latestTokenId and update the final value later + uint32 _latestTokenId = latestTokenId; unchecked { // If +1 overflows then +count would also overflow, unless count==0 in which case the loop would exceed gas limits - firstTokenId = latestTokenId + 1; + firstTokenId = _latestTokenId + 1; } - latestTokenId = latestTokenId + count; - require(latestTokenId <= maxTokenId, "NFTDropCollection: Exceeds max tokenId"); + _latestTokenId = _latestTokenId + count; + require(_latestTokenId <= maxTokenId, "NFTDropCollection: Exceeds max tokenId"); - for (uint256 i = firstTokenId; i <= latestTokenId; ) { + for (uint256 i = firstTokenId; i <= _latestTokenId; ) { _mint(to, i); unchecked { ++i; } } + //Update state variable latestTokenId + latestTokenId = _latestTokenId; } /** @@ -239,7 +243,8 @@ contract NFTDropCollection is postRevealBaseURIHash = _postRevealBaseURIHash; baseURI = _baseURI; - emit URIUpdated(baseURI, postRevealBaseURIHash); + //Use local variable _postRevealBaseURIHash instead of postRevealBaseURIHash + emit URIUpdated(baseURI, _postRevealBaseURIHash); }
https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/mixins/shared/MarketFees.sol
diff --git a/contracts/mixins/shared/MarketFees.sol b/contracts/mixins/shared/MarketFees.sol index 3a29acf..9369005 100644 --- a/contracts/mixins/shared/MarketFees.sol +++ b/contracts/mixins/shared/MarketFees.sol @@ -122,8 +122,10 @@ abstract contract MarketFees is FoundationTreasuryNode, MarketSharedCore, SendVa ); // Pay the creator(s) + // Use local variable instead of memory + uint256 _len = creatorRecipients.length; unchecked { - for (uint256 i = 0; i < creatorRecipients.length; ++i) { + for (uint256 i; i < _len; ++i) { _sendValueWithFallbackWithdraw( creatorRecipients[i], creatorShares[i], @@ -195,8 +197,13 @@ abstract contract MarketFees is FoundationTreasuryNode, MarketSharedCore, SendVa ); // Sum the total creator rev from shares - for (uint256 i = 0; i < creatorShares.length; ++i) { - creatorRev += creatorShares[i]; + // Use local variable instead of memory + uint256 _len = creatorShares.length; + // use unchecked + unchecked { + for (uint256 i; i < _len; ++i) { + creatorRev += creatorShares[i]; + } } } @@ -481,7 +488,7 @@ abstract contract MarketFees is FoundationTreasuryNode, MarketSharedCore, SendVa uint256 totalShares; if (creatorRecipients.length > 1) { unchecked { - for (uint256 i = 0; i < creatorRecipients.length; ++i) { + for (uint256 i; i < creatorRecipients.length; ++i) { if (creatorShares[i] > BASIS_POINTS) { // If the numbers are >100% we ignore the fee recipients and pay just the first instead totalShares = 0;
https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/libraries/BytesLibrary.sol
diff --git a/contracts/libraries/BytesLibrary.sol b/contracts/libraries/BytesLibrary.sol index 4274a65..ac3b0ff 100644 --- a/contracts/libraries/BytesLibrary.sol +++ b/contracts/libraries/BytesLibrary.sol @@ -22,7 +22,7 @@ library BytesLibrary { bytes memory newData = abi.encodePacked(newAddress); unchecked { // An address is 20 bytes long - for (uint256 i = 0; i < 20; ++i) { + for (uint256 i; i < 20; ++i) { uint256 dataLocation = startLocation + i; if (data[dataLocation] != expectedData[i]) { revert BytesLibrary_Expected_Address_Not_Found(); @@ -41,7 +41,7 @@ library BytesLibrary { return false; } unchecked { - for (uint256 i = 0; i < 4; ++i) { + for (uint256 i; i < 4; ++i) { if (callData[i] != functionSig[i]) { return false; }
diff --git a/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol b/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol index 3818d48..b5f8f25 100644 --- a/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol +++ b/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol @@ -153,7 +153,8 @@ abstract contract NFTDropMarketFixedPriceSale is MarketFees { saleConfig.seller = payable(msg.sender); saleConfig.price = price; saleConfig.limitPerAccount = limitPerAccount; - emit CreateFixedPriceSale(nftContract, saleConfig.seller, saleConfig.price, saleConfig.limitPerAccount); + //Use local variables in event emit + emit CreateFixedPriceSale(nftContract, payable(msg.sender), price, limitPerAccount); } /**
diff --git a/contracts/mixins/collections/SequentialMintCollection.sol b/contracts/mixins/collections/SequentialMintCollection.sol index 34abef1..54c912b 100644 --- a/contracts/mixins/collections/SequentialMintCollection.sol +++ b/contracts/mixins/collections/SequentialMintCollection.sol @@ -85,7 +85,9 @@ abstract contract SequentialMintCollection is ITokenCreator, Initializable, ERC7 */ function _updateMaxTokenId(uint32 _maxTokenId) internal { require(_maxTokenId != 0, "SequentialMintCollection: Max token ID may not be cleared"); - require(maxTokenId == 0 || _maxTokenId < maxTokenId, "SequentialMintCollection: Max token ID may not increase"); + //Use local variable to refer __maxTokenId + uint32 __maxTokenId = maxTokenId; + require(__maxTokenId == 0 || _maxTokenId < __maxTokenId, "SequentialMintCollection: Max token ID may not increase"); require(latestTokenId <= _maxTokenId, "SequentialMintCollection: Max token ID must be >= last mint"); maxTokenId = _maxTokenId;
https://github.com/code-423n4/2022-08-foundation/blob/main/contracts/NFTCollection.sol
diff --git a/contracts/NFTCollection.sol b/contracts/NFTCollection.sol index e99ed42..c1229cf 100644 --- a/contracts/NFTCollection.sol +++ b/contracts/NFTCollection.sol @@ -265,7 +265,9 @@ contract NFTCollection is unchecked { // Number of tokens cannot overflow 256 bits. tokenId = ++latestTokenId; - require(maxTokenId == 0 || tokenId <= maxTokenId, "NFTCollection: Max token count has already been minted"); + //Use local variable to refer _maxTokenId + uint32 _maxTokenId = maxTokenId; + require(_maxTokenId == 0 || tokenId <= _maxTokenId, "NFTCollection: Max token count has already been minted"); cidToMinted[tokenCID] = true; _tokenCIDs[tokenId] = tokenCID; _mint(msg.sender, tokenId);
https://github.com/code-423n4/2022-08-foundation/blob/main/gas-stories.txt
diff --git a/gas-stories.txt b/gas-stories.txt index d516392..8f1df28 100644 --- a/gas-stories.txt +++ b/gas-stories.txt @@ -3,19 +3,19 @@ Market NFTDropMarketFixedPriceSale 路路路路路路路路路路路路路路路路路路路路路路路路路路路 [Collector] Mint - 134,500 1st mint - 146,800 1st mint w. buy referrer - 117,400 2nd mint - 129,700 2nd mint w. buy referrer + 134,400 1st mint + 146,700 1st mint w. buy referrer + 117,300 2nd mint + 129,600 2nd mint w. buy referrer [Collector] Mint Batch - 359,100 1st Mint 10 - 371,400 1st Mint 10 w. buy referrer - 342,000 2nd Mint 10 - 354,300 2nd Mint 10 w. buy referrer + 357,900 1st Mint 10 + 370,200 1st Mint 10 w. buy referrer + 340,800 2nd Mint 10 + 353,100 2nd Mint 10 w. buy referrer createFixedPriceSale - 73,400 + 73,300 NFT ========================================================= @@ -49,8 +49,8 @@ Create NFTDropCollection NFTDropCollection 路路路路路路路路路路路路路路路路路路路路路路路路路路路 [Creator] Mint - 81,700 1st mint - 64,600 2nd mint + 81,600 1st mint + 64,500 2nd mint Burn 39,300 Last NFT @@ -60,7 +60,7 @@ Self Destruct 34,500 Update - 33,200 MaxTokenId - 41,200 PreReveal Content + 33,100 MaxTokenId + 41,100 PreReveal Content 34,700 Reveal Collection
#0 - HardlyDifficult
2022-08-17T19:29:26Z
Good report. The formatting could use a little love, but there were some good wins here that don't get lost in a sea of links. And thanks for using the gas stories to show the full diff! 馃挴
Use local variable to refer state variable.
Valid but not going to change, this is a rarely called admin-only function so I'd prefer to keep the code simple.
Use pre increment (++i instead of i++)
Valid will fix.
Use local variable
Valid - tests show as much as 1,400 gas saved when minting 10.
Optimize for loop by referring to local variable for condition check.
May be theoretically valid, but won't fix. I tested this: gas-reporter and our gas-stories suite is reporting a small regression using this technique. It also hurts readability a bit so we wouldn't want to include it unless it was a clear win.
No need to initialize variable to 0
Invalid. This optimization technique is no longer applicable with the current version of Solidity.
Use local variables for event CreateFixedPriceSale
Valid, will fix. Tests show 43 gas saved.