NextGen - K42's results

Advanced smart contracts for launching generative art projects on Ethereum.

General Information

Platform: Code4rena

Start Date: 30/10/2023

Pot Size: $49,250 USDC

Total HM: 14

Participants: 243

Period: 14 days

Judge: 0xsomeone

Id: 302

League: ETH

NextGen

Findings Distribution

Researcher Performance

Rank: 34/243

Findings: 2

Award: $371.83

Gas:
grade-a
Analysis:
grade-a

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Awards

109.1409 USDC - $109.14

Labels

bug
G (Gas Optimization)
grade-a
high quality report
sponsor acknowledged
G-07

External Links

Gas Optimization Report for NextGen by K42

Possible Optimizations in NextGenCore.sol

Possible Optimization 1 =

  • In createCollection(), replace string arrays with bytes32 arrays for collectionScript. Strings are expensive in terms of gas usage, especially when dealing with arrays. Using bytes32 arrays can significantly reduce gas costs.

Here is the optimized code snippet:

struct collectionInfoStructure {
    // ... other fields ...
    bytes32[] collectionScript;
}

function createCollection(
    string memory _collectionName, 
    string memory _collectionArtist, 
    string memory _collectionDescription, 
    string memory _collectionWebsite, 
    string memory _collectionLicense, 
    string memory _collectionBaseURI, 
    string memory _collectionLibrary, 
    bytes32[] memory _collectionScript
) public FunctionAdminRequired(this.createCollection.selector) {
    // ... rest of the function ...
}
  • Estimated gas saved = The exact amount of gas saved depends on the length and content of the scripts. However, replacing strings with bytes32 can save significant gas, especially for longer strings. The opcode SSTORE costs are reduced due to fewer storage slots being used.

Possible Optimization 2 =

Here is the optimized code:

function batchMint(
    uint256[] memory mintIndices, 
    address[] memory recipients, 
    string[] memory tokenDatas, 
    uint256 collectionID, 
    uint256 saltfun_o
) external {
    require(msg.sender == minterContract, "Caller is not the Minter Contract");
    uint256 supplyIncrement = mintIndices.length;
    collectionAdditionalData[collectionID].collectionCirculationSupply += supplyIncrement;
    for (uint256 i = 0; i < supplyIncrement; i++) {
        _mintProcessing(mintIndices[i], recipients[i], tokenDatas[i], collectionID, saltfun_o);
    }
}
  • Estimated gas saved = The gas savings come from reducing the number of SSTORE operations. Each SSTORE operation costs 20,000 gas for a non-zero value being set. By batching, you save on these costs per mint/airdrop.

Possible Optimization 3 =

  • In updateCollectionInfo(), use short-circuiting in conditionals. In Solidity, conditions are evaluated from left to right. Placing the most likely to fail (or cheapest to evaluate) condition first can save gas.

Optimized Code Snippet:

function updateCollectionInfo(
    uint256 _collectionID, 
    string memory _newCollectionName, 
    // ... other parameters ...
) public CollectionAdminRequired(_collectionID, this.updateCollectionInfo.selector) {
    require(
        collectionFreeze[_collectionID] == false && isCollectionCreated[_collectionID] == true, 
        "Not allowed"
    );
    // ... rest of the function ...
}
  • Estimated Gas Saved = The gas savings are minor per transaction but can add up over many transactions. The savings come from avoiding unnecessary condition checks.

Possible Optimization 4 =

  • The createCollection() function updates multiple fields in the collectionInfo struct for each new collection. This results in multiple SSTORE operations, which are costly in terms of gas. Use a temporary struct in memory to accumulate changes and then assign it to the storage in one operation. This reduces the number of SSTORE operations.

After Optimization:

function createCollection(string memory _collectionName, string memory _collectionArtist, string memory _collectionDescription, string memory _collectionWebsite, string memory _collectionLicense, string memory _collectionBaseURI, string memory _collectionLibrary, string[] memory _collectionScript) public FunctionAdminRequired(this.createCollection.selector) {
    collectionInfoStructure memory newCollection = collectionInfoStructure({
        collectionName: _collectionName,
        collectionArtist: _collectionArtist,
        collectionDescription: _collectionDescription,
        collectionWebsite: _collectionWebsite,
        collectionLicense: _collectionLicense,
        collectionBaseURI: _collectionBaseURI,
        collectionLibrary: _collectionLibrary,
        collectionScript: _collectionScript
    });

    collectionInfo[newCollectionIndex] = newCollection;
    isCollectionCreated[newCollectionIndex] = true;
    newCollectionIndex = newCollectionIndex + 1;
}
  • Estimated Gas Saved = This optimization can save a significant amount of gas, especially when creating new collections frequently. The exact amount depends on the size of the struct and the frequency of calls. It could range from a few hundred to a few thousand gas units per call.

Possible Optimization 5 =

  • The setCollectionData() function performs multiple storage operations conditionally, leading to multiple SSTOREs in different scenarios. Similar to the previous optimization, use a temporary struct in memory to accumulate changes and then assign it to the storage in one operation.

Optimized Code Snippet:

function setCollectionData(uint256 _collectionID, address _collectionArtistAddress, uint256 _maxCollectionPurchases, uint256 _collectionTotalSupply, uint _setFinalSupplyTimeAfterMint) public CollectionAdminRequired(_collectionID, this.setCollectionData.selector) {
    require((isCollectionCreated[_collectionID] == true) && (collectionFreeze[_collectionID] == false) && (_collectionTotalSupply <= 10000000000), "err/freezed");

    collectionAdditonalDataStructure memory newData = collectionAdditionalData[_collectionID];
    newData.collectionArtistAddress = _collectionArtistAddress;
    newData.maxCollectionPurchases = _maxCollectionPurchases;
    newData.setFinalSupplyTimeAfterMint = _setFinalSupplyTimeAfterMint;

    if (newData.collectionTotalSupply == 0) {
        newData.collectionCirculationSupply = 0;
        newData.collectionTotalSupply = _collectionTotalSupply;
        newData.reservedMinTokensIndex = (_collectionID * 10000000000);
        newData.reservedMaxTokensIndex = (_collectionID * 10000000000) + _collectionTotalSupply - 1;
    }
    collectionAdditionalData[_collectionID] = newData;
    wereDataAdded[_collectionID] = true;
}
  • Estimated Gas Saved = This optimization can also save a significant amount of gas, depending on the frequency of updates to collection data. The savings could be in the range of a few hundred to a few thousand gas units per call.

Possible Optimizations in MinterContract.sol

Possible Optimization 1 =

  • The payArtist() function performs multiple divisions and multiplications for each payout. We can optimize it by calculating percentages in a more gas-efficient manner.

After Optimization:

function payArtist(uint256 _collectionID, address _team1, address _team2, uint256 _teamperc1, uint256 _teamperc2) public FunctionAdminRequired(this.payArtist.selector) {
    // ... existing checks ...
    uint256 totalAmount = collectionTotalAmount[_collectionID];
    uint256 onePercent = totalAmount / 100;
    artistRoyalties1 = onePercent * collectionArtistPrimaryAddresses[colId].add1Percentage;
    artistRoyalties2 = onePercent * collectionArtistPrimaryAddresses[colId].add2Percentage;
    artistRoyalties3 = onePercent * collectionArtistPrimaryAddresses[colId].add3Percentage;
    teamRoyalties1 = onePercent * _teamperc1;
    teamRoyalties2 = onePercent * _teamperc2;
    // ... existing logic to transfer funds ...
}
  • Estimated gas saved = This can save approximately 200 to 500 gas per percentage calculation due to reduced computational complexity.

Possible Optimization 2 =

  • The mint() function contains a for-loop that calls the gencore.mint function for each token to be minted. This can be optimized by batch processing within the gencore contract, if possible, to reduce the number of external contract calls.

After:

// Assuming gencore contract has a batchMint function implemented 
// If not I suggest you add it for efficiency as detailed in optimizations for NextGenCore
function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
    // ... existing checks and logic ...
    uint256[] memory mintIndices = new uint256[](_numberOfTokens);
    for(uint256 i = 0; i < _numberOfTokens; i++) {
        mintIndices[i] = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col) + i;
    }
    gencore.batchMint(mintIndices, mintingAddress, _mintTo, tokData, _saltfun_o, col, phase);
    // ... rest of the function ...
}
  • Estimated gas saved = This can save up to 21000 gas for each iteration after the first, due to elimination of the loop overhead and reduced transaction costs from batch processing.

Possible Optimizations in NextGenAdmins.sol

Possible Optimization 1 =

  • The AdminRequired modifier uses an OR condition that checks for admin permissions or ownership. The order of these conditions could be optimized to take advantage of short-circuiting since checking _msgSender() == owner() is more expensive than adminPermissions[msg.sender].

After Optimization:

modifier AdminRequired {
  require(adminPermissions[msg.sender] || _msgSender() == owner(), "Not allowed");
  _;
}
  • Estimated gas saved = This could save gas when msg.sender is an admin because it avoids the need to check the owner status, which includes an SLOAD operation. The exact savings depend on the frequency of this check but are roughly around 800-2000 gas.

Possible Optimization 2 =

  • When updating functionAdmin in a batch, it's more efficient to use a local variable for functionAdmin[_address] mapping to avoid multiple mapping lookups.

After:

function registerBatchFunctionAdmin(address _address, bytes4[] memory _selector, bool _status) public AdminRequired {
    mapping(bytes4 => bool) storage functionPermissions = functionAdmin[_address];
    for (uint256 i = 0; i < _selector.length; i++) {
        functionPermissions[_selector[i]] = _status;
    }
}
  • Estimated gas saved = This optimization can save approximately 200-5000 gas per iteration in the loop, depending on the number of selectors updated and the current state of the mapping.

Possible Optimization in RandomizerNXT.sol

Possible Optimization =

  • The calculateTokenHash() function includes an expensive keccak256 hash calculation. This could be optimized by reducing the number of dynamic values hashed, if possible.

After Optimization:

function calculateTokenHash(uint256 _collectionID, uint256 _mintIndex, uint256 _saltfun_o) public {
    require(msg.sender == gencore);
    // Assuming randoms.randomNumber() and randoms.randomWord() return uint256.
    uint256 randomInput = randoms.randomNumber() ^ randoms.randomWord();
    bytes32 hash = keccak256(abi.encodePacked(_mintIndex, blockhash(block.number - 1), randomInput));
    gencoreContract.setTokenHash(_collectionID, _mintIndex, hash);
}
  • Estimated gas saved = Optimizing hashing operations can save a significant amount of gas, but this is highly dependent on the size and number of inputs. Typically, savings could be in the range of a few hundred to over a thousand gas.

Possible Optimization in RandomizerVRF.sol

Possible Optimization =

After Optimization:

function updateContracts(address _newAdminsContract, address _newGencore) public FunctionAdminRequired(this.updateContracts.selector) {
    if(_newAdminsContract != address(adminsContract)) {
        require(INextGenAdmins(_newAdminsContract).isAdminContract() == true, "Contract is not Admin");
        adminsContract = INextGenAdmins(_newAdminsContract);
    }
    if(_newGencore != gencore) {
        gencore = _newGencore;
        gencoreContract = INextGenCore(_newGencore);
    }
}
  • Estimated gas saved = This can save the user from having to send two separate transactions, effectively halving the transaction cost for updating both addresses.

Possible Optimization in RandomizerRNG.sol

Possible Optimization =

  • Similar to previous optimization, combining updates for contract addresses into a single function reduces transaction overhead when both need to be updated.

Here is the optimized code snippet:

function updateContracts(address _newAdminsContract, address _newGencore) public FunctionAdminRequired(this.updateContracts.selector) {
    bool updateAdmin = (_newAdminsContract != address(adminsContract)) && INextGenAdmins(_newAdminsContract).isAdminContract();

    bool updateGencore = (_newGencore != gencore);

    if(updateAdmin) {
        adminsContract = INextGenAdmins(_newAdminsContract);
    }
    if(updateGencore) {
        gencore = _newGencore;
        gencoreContract = INextGenCore(_newGencore);
    }
}
  • Estimated gas saved = This can save the user from having to send two separate transactions, effectively halving the transaction cost for updating both addresses.

Possible Optimizations in XRandoms.sol

Possible Optimization 1 =

  • The getWord() function stores a large array of strings in memory each time it's called. This is highly inefficient in terms of gas, as memory operations are costly. Instead, consider storing the list as a constant array in storage, which is only written once when the contract is deployed.

After Optimization:

string[100] private constant wordsList = [
    "Acai", "Ackee", "Apple", // ... other fruits ...
    "Watermelon"
];

function getWord(uint256 id) private view returns (string memory) {
    require(id < wordsList.length, "Index out of bounds");
    return wordsList[id];
}
  • Estimated gas saved = This could save a substantial amount of gas for each call, potentially thousands depending on the EVM implementation, due to reduced memory usage.

Possible Optimization 2 =

  • The randomNumber() and randomWord() functions use block information to generate a random number. This is not safe for use in production for value-critical purposes as it can be manipulated by miners. It's better to use a verifiable random function (VRF) provided by Chainlink VRF for true randomness.

After:

// Example using Chainlink VRF; this requires additional setup such as funding the contract with LINK
// and setting up a subscription on the Chainlink VRF service.
function requestRandomWord() public returns (bytes32 requestId) {
    // Chainlink VRF call
    requestId = COORDINATOR.requestRandomWords(
        keyHash,
        subscriptionId,
        requestConfirmations,
        callbackGasLimit,
        numWords
    );
    // Additional logic to handle the requestId and randomness fulfillment
}
  • Estimated gas saved = While using VRF might not save gas and could potentially cost more, it would enhance the security and integrity of the random number generation, which is a crucial aspect of smart contracts relying on randomness.

Possible Optimization in AuctionDemo.sol

Possible Optimization =

  • The current claimAuction() function iterates through all bids to issue refunds. This can be optimized by tracking only the highest bid and issuing a single refund to the previous highest bidder in the participateToAuction() function when outbid.

After Optimization:

// The claimAuction function can be simplified as follows:
function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

    auctionClaim[_tokenid] = true;
    uint256 highestBid = highestBids[_tokenid].bid;
    address highestBidder = highestBids[_tokenid].bidder;

    // Transfer token and funds only for the highest bid
    IERC721(gencore).safeTransferFrom(IERC721(gencore).ownerOf(_tokenid), highestBidder, _tokenid);
    (bool success, ) = payable(owner()).call{value: highestBid}("");

    require(success, "Payment to owner failed");
    emit ClaimAuction(owner(), _tokenid, success, highestBid);
}
  • Estimated gas saved = This can save significant gas, as it removes the need for a loop when claiming an auction. Loops can be very expensive when they involve multiple state changes and external calls.

#0 - c4-pre-sort

2023-11-21T12:44:26Z

141345 marked the issue as high quality report

#1 - 141345

2023-11-26T06:04:35Z

1766 K42 l r nc 3 9 3

G 1 n G 2 r G 3 n G 4 l G 5 l G 1 r G 2 r G 1 l G 2 r G 1 n G 1 r G 1 r G 1 r G 2 r G 1 r

#2 - c4-pre-sort

2023-11-26T06:04:47Z

141345 marked the issue as sufficient quality report

#3 - c4-pre-sort

2023-11-26T06:04:53Z

141345 marked the issue as high quality report

#4 - c4-sponsor

2023-11-28T06:46:58Z

a2rocket (sponsor) acknowledged

#5 - alex-ppg

2023-12-02T18:08:27Z

NextGenCore.sol

  • Optimization 3 refers to revert-savings
  • Optimization 4 is better to write data in a cached storage pointer to avoid memory expansion
  • Optimization 5 depends on the actual data written and read, may not be an optimization

RandomizerNXT.sol

  • Optimization has pseudo-randomness implications

RandomizerVRF.sol

  • Optimization is inapplicable if only one of the two members is to be updated and refers to bytecode size optimizations

RandomizerRNG.sol

  • Similarly to above

XRandoms.sol

  • Optimization 1 is inapplicable as string array types cannot be set as constant
  • Optimization 2 is not gas related

#6 - c4-judge

2023-12-02T18:08:32Z

alex-ppg marked the issue as grade-a

Findings Information

Awards

262.6934 USDC - $262.69

Labels

analysis-advanced
grade-a
insufficient quality report
A-07

External Links

Advanced Analysis Report for NextGen by K42

Overview

  • NextGen's suite exhibits a comprehensive NFT platform with minting capabilities, randomization logic for NFT attributes, auction mechanics, and administrative controls.

Understanding the Ecosystem:

The system comprises:

  • Core Contract (NextGenCore): Manages NFT collections, minting, and metadata.
  • Minter Contract (NextGenMinterContract): Handles the minting process, including airdrops and auction logic.
  • Randomization Contracts (NextGenRandomizerVRF, NextGenRandomizerRNG, NextGenRandomizerNXT): Generate random values for NFT attributes.
  • Administrative Contract (NextGenAdmins): Governs administrative privileges across the ecosystem.
  • Auction Contract (auctionDemo): Facilitates auctions for NFTs.

Codebase Quality Analysis:

NextGenCore Contract:

  • Functionality: Manages ERC721 tokens, core functions of the ERC721 standard, and additional setter & getter functions.
  • Vulnerabilities:
    • Data Management: Potential risks in managing a large amount of on-chain data (name, artist's name, library, script, total supply).
    • External Dependency: Integrates with other NextGen contracts, increasing complexity and potential points of failure.
  • Recommendations:
    • Data Optimization: Evaluate on-chain vs off-chain data storage strategies for efficiency.
    • Contract Integration Review: Assess integration points for potential vulnerabilities.

MinterContract:

  • Functionality: Used for minting ERC721 tokens based on pre-set requirements.
  • Vulnerabilities:
    • Minting Process Control: Potential risks in phase-based, allowlist-based, delegation-based minting philosophy.
    • Complex Sales Models: Use of various sales models could introduce logical errors or inconsistencies.
  • Recommendations:
    • Minting Logic Audit: Thoroughly review and test minting logic for edge cases.
    • Sales Model Validation: Ensure robustness and consistency in sales model implementations.

NextGenAdmins:

  • Functionality: Manages global or function-based admins for Core and Minter contracts.
  • Vulnerabilities:
    • Centralization Risks: Centralized control in admin management.
    • Role Escalation: Potential for unauthorized role escalation.
  • Recommendations:
    • Decentralized Governance: Explore decentralized admin management.
    • Role Management Security: Implement strict checks and balances for role assignments.

Randomizer Contracts (RandomizerNXT, RandomizerVRF, RandomizerRNG):

  • Functionality: Generate random hashes for tokens during minting.
  • Vulnerabilities:
    • Randomness Source Reliability: Different randomizers (Chainlink VRF, ARRNG.io, custom implementation) have varying degrees of unpredictability and reliability.
    • External Dependency: Reliance on external services (Chainlink, ARRNG.io) introduces third-party risks.
  • Recommendations:
    • Randomness Source Evaluation: Assess and validate the reliability and unpredictability of each randomizer.
    • Fallback Mechanisms: Implement fallback options for randomizer failures.

XRandoms:

  • Functionality: Used by RandomizerNXT to return random values.
  • Vulnerabilities:
    • Predictability: Potential predictability in the generation of random words and numbers.
  • Recommendations:
    • Enhance Randomness: Improve the algorithm to ensure unpredictability.

AuctionDemo:

  • Functionality: Manages auctions for tokens post-minting.
  • Vulnerabilities:
    • Bidder Protection: Lack of mechanisms for refunding overbids or handling auction cancellations.
    • Auction Integrity: Risks in ensuring fair and transparent auction processes.
  • Recommendations:
    • Refund Logic: Implement logic for handling overbids and auction cancellations.
    • Auction Process Audit: Review and test auction processes for integrity and fairness.

Architecture Recommendations:

  • Separation of Concerns: Functions are generally well scoped, though some (NextGenCore, for instance) could be refactored into smaller, more focused contracts.
  • State Mutability: Functions like randomNumber in randomPool should be marked as pure instead of view.
  • Introduce interfaces for randomization to enable switching between RNG methods easily.

Centralization Risks:

  • Admin Controls: Centralized power in functions like registerAdmin in NextGenAdmins could lead to abuse.
  • Upgradeability: There's no upgrade path for contracts, which may necessitate a migration if bugs are found or enhancements are needed.

Mechanism Review:

  • Randomization: Reliance on external randomization (NextGenRandomizerVRF) introduces third-party risk.
  • Auction Logic: The auction contract (auctionDemo) lacks bidder protection mechanisms, such as refund logic for overbidding.

Systemic Risks:

  • External Dependencies: Reliance on external randomizer contracts introduces systemic risks.

Attack Vectors I considered

  • Reentrancy in minting and auction contracts.
  • Random number generation manipulation.
  • Front-running in auction bids.
  • Exploitation of admin functions.

Areas of Concern

  • RandomPool: Uses potentially predictable randomness sources.
  • Royalty Distribution: Vulnerable to exploitation if improperly implemented.
  • Gas Optimization: Some functions are gas-intensive.

Codebase Analysis

  • Robust structure but needs gas efficiency optimization and enhanced security measures.

Recommendations

  • Conduct thorough testing and formal verification of critical logic.
  • Audit admin control mechanisms.
  • Optimize code for gas usage.

Contract Details

  • NextGenCore: Manages core NFT logic.
  • NextGenMinterContract: Oversees minting economics.
  • NextGenAdmins: Handles administrative permissions.
  • Randomizer Contracts: Manages random number generation.
  • AuctionDemo: Facilitates NFT auctions.

Conclusion

NextGen contracts provide a solid foundation for an NFT ecosystem but require further security measures, optimizations, and a reevaluation of admin centralization and upgradeability.

Time spent:

20 hours

#0 - c4-pre-sort

2023-11-27T14:36:52Z

141345 marked the issue as insufficient quality report

#1 - c4-judge

2023-12-07T16:49:11Z

alex-ppg marked the issue as grade-a

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter