Git Product home page Git Product logo

lockdealnft.builders's Introduction

LockDealNFT.Builders

Build and Test codecov CodeFactor npm version License

Builders are Solidity smart contracts designed to mass-create NFTs using the LockDealNFT system.

Navigation

Installation

Install the packages:

npm i
yarn

Compile contracts:

npx hardhat compile

Run tests:

npx hardhat test

Run coverage:

npx hardhat coverage

Deploy:

npx truffle dashboard
npx hardhat run ./scripts/deploySimpleBuilder.ts --network truffleDashboard
npx hardhat run ./scripts/deploySimpleRefundBuilder.ts --network truffleDashboard

License

The-Poolz Contracts is released under the MIT License.

lockdealnft.builders's People

Contributors

youstillalive avatar dependabot[bot] avatar lomet avatar

Stargazers

maryam avatar

Watchers

avatar

Forkers

saharnaz-ca

lockdealnft.builders's Issues

Redundant computation

Risk Level
Severity: Gas

function _getParamsData(
        uint256 collateraPoolId,
        uint256 tokenAmount,
        uint256 firstAmount
    ) internal view returns (ParamsData memory paramsData) {
        // get refund pool ID
        uint256 refundPoolId = collateraPoolId - 2;
        // Ensure valid refund pool ID
        require(
            lockDealNFT.poolIdToProvider(refundPoolId) == refundProvider,
            "SimpleRefundBuilder: invalid refundPoolId"
        );
        paramsData.token = lockDealNFT.tokenOf(refundPoolId);
        paramsData.mainCoin = lockDealNFT.tokenOf(collateraPoolId);
        paramsData.provider = ISimpleProvider(
            address(lockDealNFT.poolIdToProvider(refundPoolId + 1))
        );
        paramsData.mainCoinAmount = tokenAmount.calcAmount(
            collateralProvider.getParams(collateraPoolId)[2]
        );
        paramsData.simpleParams = paramsData.provider.getParams(
            refundPoolId + 1
        );
        paramsData.simpleParams[0] = firstAmount;
    }

Description
The highlighted computation is redundant. Storing the computed result in a new variable and using it twice would save some gas.

Recommendation
Consider changing to:

function _getParamsData(
        uint256 collateraPoolId,
        uint256 tokenAmount,
        uint256 firstAmount
    ) internal view returns (ParamsData memory paramsData) {
        // get refund pool ID
        uint256 refundPoolId = collateraPoolId - 2;
        // Ensure valid refund pool ID
        require(
            lockDealNFT.poolIdToProvider(refundPoolId) == refundProvider,
            "SimpleRefundBuilder: invalid refundPoolId"
        );
        paramsData.token = lockDealNFT.tokenOf(refundPoolId);
        paramsData.mainCoin = lockDealNFT.tokenOf(collateraPoolId);
        uint256 poolId = refundPoolId + 1;
        paramsData.provider = ISimpleProvider(
            address(lockDealNFT.poolIdToProvider(poolId))
        );
        paramsData.mainCoinAmount = tokenAmount.calcAmount(collateralProvider.getParams(collateraPoolId)[2]
        );
        paramsData.simpleParams = paramsData.provider.getParams(
            poolId
        );
        paramsData.simpleParams[0] = firstAmount;
    }

State variables only set in the constructor should be declared as immutable

Risk Level
Severity: Gas

Code Segment

// at BuilderState.sol
ILockDealNFT public lockDealNFT;


// at RefundBuilderState.sol
// Instance of the refund provider contract
IProvider public refundProvider;
// Instance of the collateral provider contract
IProvider public collateralProvider;


// at TokenNFTConnector.sol
ISwapRouter public swapRouter;
IDelayVaultProvider public delayVaultProvider;
IERC20 public pairToken;
uint24 private poolFee; // last pair fee

Description

The BuilderState.lockDealNFT, RefundBuilderState.refundProvider, RefundBuilderState.collateralProvider, TokenNFTConnector.swapRouter, TokenNFTConnector.delayVaultProvider, TokenNFTConnector.pairToken and TokenNFTConnector.poolFee state variables are only assigned in the constructor. These variables can be declared as immutable to make them read-only, but only assignable in the constructor, reducing gas costs.

Code Location
TokenNFTConnector/contracts/TokenNFTConnector.sol
LockDealNFT.Builders/contracts/Builder/BuilderState.sol
LockDealNFT.Builders/contracts/SimpleRefundBuilder/RefundBuilderState.sol

Recommendation
Consider marking the mentioned state variables as immutable.

Using an older version of OpenZeppelin libraries can pose significant risks

Risk Level
Severity: Low, Likelihood: Low
Code segment

"dependencies": {
        "@ironblocks/firewall-consumer": "^1.0.5",
        "@openzeppelin/contracts": "^4.9.6",
        "@poolzfinance/poolz-helper-v2": "^2.3.5",
        "@poolzfinance/lockdeal-nft": "^0.8.0",
        "@poolzfinance/refund-provider": "^0.8.0",
        "@poolzfinance/collateral-provider": "^0.8.1"
    }

Description
Currently the protocol is using version 4.9.6 of the OpenZeppelin contracts which are continuously updated to eliminate any bugs and vulnerabilities.

Code Location
LockDealNFT.Builders/package.json

Recommendation
Consider using the latest version (5.0.2 so far).

Typo in comments

Issue ID
LDN-10

Risk Level
Severity: Informational

Description
The following comment, which occurs twice, contains a typo:

// one time transfer for deacrease number transactions

Code Location

LockDealNFT.Builders/contracts/SimpleBuilder/SimpleBuilder.sol
LockDealNFT.Builders/contracts/SimpleRefundBuilder/SimpleRefundBuilder.sol

"one time transfer for decreasing the number of transactions"

Missing zero address checks could lead to redeploying contracts

Risk Level
Severity: Low, Likelihood: Low

Code Segment

constructor(ILockDealNFT _nft, IProvider _refund, IProvider _collateral) {
        lockDealNFT = _nft;
        refundProvider = _refund;
        collateralProvider = _collateral;
    }
constructor(ILockDealNFT _nft) {
        lockDealNFT = _nft;
    }

Description

During the SimpleRefundBuilder deployment, the deployer can mistakenly provide a zero address for either lockDealNFT, refundProvider or collateralProvider with no way of updating them after the deployment. The same applies for lockDealNFT during the SimpleBuilder deployment. This could lead to redeploying the entire contract and incurring gas loss due to the initial deployment.

Code Location
LockDealNFT.Builders/contracts/SimpleRefundBuilder/SimpleRefundBuilder.sol
LockDealNFT.Builders/contracts/SimpleBuilder/SimpleBuilder.sol

Recommendation
Consider adding zero address checks for all the specified state variables

constructor(ILockDealNFT _nft, IProvider _refund, IProvider _collateral) {
        require(
            address(_nft) != address(0),
            "SimpleRefundBuilder: zero address"
        );
        require(
            address(_refund) != address(0),
            "SimpleRefundBuilder: zero address"
        );
        require(
            address(_collateral) != address(0),
            "SimpleRefundBuilder: zero address"
        );
        lockDealNFT = _nft;
        refundProvider = _refund;
        collateralProvider = _collateral;
    }
constructor(ILockDealNFT _nft) {
        require(
            address(_nft) != address(0),
            "SimpleBuilder: zero address"
        );
        lockDealNFT = _nft;
    }

Add `onERC721Received` option to `SimpleRefundBuilder`

Steps:

  • SimpleRefundBuilder will receive CollateralProvider NFT from Project Owner
  • Decode Users data bytes
  • Deposite Tokens and MainCoins
  • Create more NFTs for Users
  • Return the Collateral NFT to the Project Owner with additional pools.

Inadequate naming could lead to confusion

Issue ID
LDN-11

Risk Level
Severity: Informational

Code Segment

function onERC721Received(
        address operator,
        address user,
        uint256 poolId,
        bytes calldata data
    )
/// @notice Creates the collateral provider
    /// @param data Rebuilder struct containing mainCoin data
    /// @param collateralFinishTime Finish time for refund
    /// @return poolId Collateral pool ID
    function _createCollateralProvider(
        Rebuilder memory data,
        uint256 collateralFinishTime
    ) internal firewallProtectedSig(0x4516d406) returns (uint256 poolId) {
/// @notice Iterates over user data to create refund pools for each user
    /// @param data Users data, paramsData, tokenPoolId, simple provider address
    function _userDataIterator(
        Rebuilder memory data
    ) internal firewallProtectedSig(0xbbc1f709)

Description
In the functions mentioned above, the poolId is described as "the ID of the Collateral NFT". Given the presence of multiple ID types within the codebase (simple pool ID, refund pool ID, collateral pool ID), it is advisable to rename it to collateralPoolId. This adjustment helps prevent confusion when interpreting the code, rather than relying solely on NatSpec documentation.
Moreover, the _userDataIterator function's name lacks clarity regarding its functionality, which involves the creation of bulk refund pools. Therefore, renaming it to something like _buildMassPools would enhance readability and comprehension.

Code Location

LockDealNFT.Builders/contracts/SimpleRefundBuilder/SimpleRefundBuilder.sol
LockDealNFT.Builders/contracts/SimpleRefundBuilder/RefundBuilderInternal.sol

Recommendation
Consider changing to:

function onERC721Received(
        address operator,
        address user,
        uint256 collateralPoolId,
        bytes calldata data
    )
  function _createCollateralProvider(
        Rebuilder memory data,
        uint256 collateralFinishTime
    ) internal firewallProtectedSig(0x4516d406) returns (uint256 collateralPoolId) {
    function _buildMassPools(
        Rebuilder memory data
    ) internal firewallProtectedSig(0xbbc1f709) {

cover `Builders` with `NatSpec` 

contracts/Builder/BuilderInternal.sol:9
BuilderInternal:_concatParams
@param amount is missing
@param params is missing
@return result is missing
 
contracts/Builder/BuilderInternal.sol:21
BuilderInternal:_createNewNFT
Natspec is missing
 
contracts/Builder/BuilderInternal.sol:34
BuilderInternal:_createFirstNFT
Natspec is missing
 
contracts/Builder/BuilderModifiers.sol:19
BuilderModifiers:_notZeroAmount
Natspec is missing
 
contracts/Builder/BuilderModifiers.sol:23
BuilderModifiers:_notZeroAddress
Natspec is missing
 
contracts/Builder/BuilderModifiers.sol:27
BuilderModifiers:_validParamsLength
Natspec is missing
 
contracts/Builder/BuilderModifiers.sol:8
BuilderModifiers:notZeroAddress
Natspec is missing
 
contracts/Builder/BuilderModifiers.sol:13
BuilderModifiers:validUserData
Natspec is missing
 
contracts/Builder/BuilderState.sol:9
BuilderState:Builder
Natspec is missing
 
contracts/Builder/BuilderState.sol:14
BuilderState:UserPool
Natspec is missing
 
contracts/SimpleBuilder/SimpleBuilder.sol:15
SimpleBuilder:MassPoolsLocals
Natspec is missing
 
contracts/SimpleRefundBuilder/RefundBuilderState.sol:18
RefundBuilderState:ParamsData
Natspec is missing
 
contracts/SimpleRefundBuilder/RefundBuilderState.sol:27
RefundBuilderState:Rebuilder
Natspec is missing

Invalid revert message

Risk Level
Severity: Low, Likelihood: Medium

function _userDataIterator(
        Rebuilder memory data
    ) internal firewallProtectedSig(0xbbc1f709) {
        uint256 length = data.userData.userPools.length;
        require(
            length > 0,
            "SimpleRefundBuilder: addressParams must contain exactly 3 addresses"
        );

Description

The highlighted require statement verifies if the user has supplied at least one pool data, however, it reverts with a misleading error message.

Code Location
LockDealNFT.Builders/contracts/SimpleRefundBuilder/RefundBuilderInternal.sol

Recommendation
Consider changing the revert message to:

function _userDataIterator(
        Rebuilder memory data
    ) internal firewallProtectedSig(0xbbc1f709) {
        uint256 length = data.userData.userPools.length;
        require(
            length > 0,
            "SimpleRefundBuilder: invalid user length"
        );

Loop gas usage optimization

Risk Level
Severity: Gas

Code Segment

function _concatParams(
        uint amount,
        uint256[] calldata params
    ) internal pure returns (uint256[] memory result) {
        uint256 length = params.length;
        result = new uint256[](length + 1);
        result[0] = amount;
        for (uint256 i = 0; i < length; ) {
            result[i + 1] = params[i];
            unchecked {
                ++i;
            }
        }
    }

Description
It was identified that the for loops employed in the contracts can be gas
optimized by the following principles:
• Unnecessary reading of the array length on each iteration wastes gas.
• Loop counters do not need to be set to 0, since uint256 is already initialized to 0.
• It is also possible to further optimize loops by using unchecked loop index incrementing
and decrementing.

Recommendation
Consider changing to:

for (uint256 i; i < length; ) {
            result[i + 1] = params[i];
            unchecked {
                ++i;
            }
        }

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.