Inheritance NFT Staking
Smart Contract Audit Report
Inheritance is building a new staking contract for users to deposit NFTs and earn yield.
For this audit, we reviewed the project team's NFTStaking contract at commit 819ae555c1b4cacccec620f08f9504502d578dde on the team's private GitHub repository.
We previously reviewed the project team's token contract here.
Please ensure trust in the team prior to investing as they have some control in the ecosystem.
Date: March 7th, 2022.
Updated: March 8th, 2022 to address improvements made by the team.
Finding #1 - NFTStaking - Low (Resolved)Description: The isStaked property is not updated when NFTs are unstaked in the unStake() function.
Risk/Impact: Users can unstake their NFTs and continue to claim rewards until the NFT lifespan is reached.
Recommendation: The token's isStaked property should be updated to "false" in the unStake() function, once the token is transferred to the user.
Resolution: The token's isStaked property is updated when the token is unstaked.
Finding #2 - NFTStaking - Informational (Resolved)Description: Several functions are declared public, but are never called internally.Recommendation: We recommend declaring these functions external for additional gas savings on each call.
Resolution: The team has declared the functions external.
Finding #3 - NFTStaking - Informational (Resolved)Description: The constMulitplier variable can never be modified, but is not declared constant.
Recommendation: We recommend declaring the state variable constant for additional gas savings on each call.
Resolution: The team has declared the variable constant.
- This contract allows non-blacklisted users to stake one or more NFTs in order to earn rewards in the form of a designated reward token.
- To stake, users' NFTs must belong to a tier as determined in an external Collection contract.
- Additionally, the user's staked amount of BOM tokens in the Database contract must be greater than the minimum stake amount.
- Users receive a reward amount based on the NFT tier, time staked, and current "mint factor". When claimed, a fee is deducted from the reward amount and transferred to the deployer of the contract.
- The NFT tier, reward rate, and tax rate are all set in an external Collection contract.
- Users' reward rate and tax rate are calculated based on the number of full months staked. Additional days inherit the corresponding month's reward and tax rate.
- The calculation of the reward and tax rate are completely dependent on an external Collection contract.
- The maximum stake time limits users from gaining rewards on staked NFTs after a certain duration.
- Users can unstake their NFT and/or claim rewards at any time. Rewards are automatically claimed when an NFT is unstaked.
- The Collection, Database, and MockToken contracts were not provided in the scope of this audit, so we are unable to provide an assessment of these contracts with regards to security.
- The owner and Collection address are given the Default Admin role upon deployment.
- The owner can grant any address the Default Admin role.
- The Default Admin role can set the owner of any NFT to any address at any time.
- The Default Admin role can unstake any NFT which also sends the pending rewards to the NFT's owner at any time.
- The Default Admin Role can set the mint factor to any value higher than its previous value at any time.
- The Default Admin Role can set the minimum stake amount at any time.
- The Default Admin Role can set the NFT lifespan to any value higher than the previous value at any time.
- The Default Admin Role can add or remove any address from the blacklist at any time.
- The contract implements an onERC721Received() function to allow for safe transfer functionality.
- As the contract is implemented with Solidity v0.8.X, it is protected from any underflow/overflow attacks.
|Arbitrary Jump/Storage Write||N/A||PASS|
|Centralization of Control||The Default Admin role can set the owner of any NFT to any address at any time; therefore, can also withdraw any staked NFTs at any time.||WARNING|
|Delegate Call to Untrusted Contract||N/A||PASS|
|Dependence on Predictable Variables||N/A||PASS|
|Improper Authorization Scheme||N/A||PASS|
|Outdated Compiler Version||N/A||PASS|
|Overall Contract Safety||WARNING|
($) = payable function # = non-constant function Int = Internal Ext = External Pub = Public + [Int] IERC721Receiver - [Ext] onERC721Received # + [Lib] EnumerableSet - [Prv] _add # - [Prv] _remove # - [Prv] _contains - [Prv] _length - [Prv] _at - [Prv] _values - [Int] add # - [Int] remove # - [Int] contains - [Int] length - [Int] at - [Int] values - [Int] add # - [Int] remove # - [Int] contains - [Int] length - [Int] at - [Int] values - [Int] add # - [Int] remove # - [Int] contains - [Int] length - [Int] at - [Int] values + [Int] IAccessControl - [Ext] hasRole - [Ext] getRoleAdmin - [Ext] grantRole # - [Ext] revokeRole # - [Ext] renounceRole # + Context - [Int] _msgSender - [Int] _msgData + [Lib] Strings - [Int] toString - [Int] toHexString - [Int] toHexString + [Int] IERC165 - [Ext] supportsInterface + ERC165 (IERC165) - [Pub] supportsInterface + AccessControl (Context, IAccessControl, ERC165) - [Pub] supportsInterface - [Pub] hasRole - [Int] _checkRole - [Pub] getRoleAdmin - [Pub] grantRole # - modifiers: onlyRole - [Pub] revokeRole # - modifiers: onlyRole - [Pub] renounceRole # - [Int] _setupRole # - [Int] _setRoleAdmin # - [Int] _grantRole # - [Int] _revokeRole # + [Int] ICollection - [Ext] mint # - [Ext] mintBatch # - [Ext] safeTransferFrom # - [Ext] getMintingPower - [Ext] getMonthWeight - [Ext] getTax - [Ext] getTier + [Int] IDatabase - [Ext] setMinMaxStake # - [Ext] setPenaltyDay # - [Ext] fundPool # - [Ext] withdrawERC20 # - [Ext] setExternalAddress # - [Ext] createPool # - [Ext] editPool # - [Ext] removePool # - [Ext] withdrawReserve # - [Ext] deposit # - [Ext] withdraw # - [Ext] transferAndRemove # - [Ext] getPools - [Ext] getMinMaxStake - [Ext] getStakerIndex - [Ext] getCurrentStakerIndex - [Ext] getStaker - [Ext] getPoolInfo - [Ext] getStakerAmount - [Ext] getPenaltyDays - [Ext] getPoolsLength + [Int] IERC20 - [Ext] totalSupply - [Ext] balanceOf - [Ext] transfer # - [Ext] allowance - [Ext] approve # - [Ext] transferFrom # + [Int] IERC20Metadata (IERC20) - [Ext] name - [Ext] symbol - [Ext] decimals + ERC20 (Context, IERC20, IERC20Metadata) - [Pub]
# - [Pub] name - [Pub] symbol - [Pub] decimals - [Pub] totalSupply - [Pub] balanceOf - [Pub] transfer # - [Pub] allowance - [Pub] approve # - [Pub] transferFrom # - [Pub] increaseAllowance # - [Pub] decreaseAllowance # - [Int] _transfer # - [Int] _mint # - [Int] _burn # - [Int] _approve # - [Int] _spendAllowance # - [Int] _beforeTokenTransfer # - [Int] _afterTokenTransfer # + Pausable (Context) - [Pub] # - [Pub] paused - [Int] _pause # - modifiers: whenNotPaused - [Int] _unpause # - modifiers: whenPaused + iAI (ERC20, Pausable, AccessControl) - [Pub] # - modifiers: ERC20 - [Pub] pause # - modifiers: onlyRole - [Pub] unpause # - modifiers: onlyRole - [Pub] mint # - modifiers: onlyRole - [Pub] withdrawERC20 # - modifiers: onlyRole - [Int] _beforeTokenTransfer # - modifiers: whenNotPaused + [Int] INFTStaking - [Ext] setNFTLifespan # - [Ext] setMintFactor # - [Ext] setMinStakeAmount # - [Ext] setOwner # + [Lib] SafeMath - [Int] tryAdd - [Int] trySub - [Int] tryMul - [Int] tryDiv - [Int] tryMod - [Int] add - [Int] sub - [Int] mul - [Int] div - [Int] mod - [Int] sub - [Int] div - [Int] mod + MockToken (Context, IERC20) - [Pub] # - [Pub] name - [Pub] symbol - [Pub] decimals - [Pub] totalSupply - [Pub] balanceOf - [Pub] transfer # - [Pub] allowance - [Pub] approve # - [Pub] transferFrom # - [Pub] increaseAllowance # - [Pub] decreaseAllowance # - [Int] _transfer # - [Pub] _mint # - [Int] _burn # - [Int] _approve # - [Int] _setupDecimals # - [Int] _beforeTokenTransfer # + NFTStaking (INFTStaking, AccessControl, IERC721Receiver) - [Pub] # - [Pub] stake # - modifiers: hasTime - [Pub] stakeBatch # - [Ext] unstake # - [Pub] claimReward # - modifiers: isStaked,hasTime - [Pub] calculateReward - [Int] _calculateRewardDays - [Int] _calculateRewardMonths - [Pub] getRemainingTime - [Ext] recoverERC721 # - modifiers: onlyRole - [Ext] setOwner # - modifiers: onlyRole - [Prv] _setOwner # - [Ext] setNFTLifespan # - modifiers: onlyRole - [Ext] setMintFactor # - modifiers: onlyRole - [Pub] getWeight - [Ext] setMinStakeAmount # - modifiers: onlyRole - [Ext] addBlacklistAddress # - modifiers: onlyRole - [Ext] deleteBlacklistAddress # - modifiers: onlyRole - [Ext] onERC721Received
About Solidity Finance
Solidity Finance was founded in 2020 and quickly grew to have one of the most experienced and well-equipped smart contract auditing teams in the industry. Our team has conducted 1000+ solidity smart contract audits covering all major project types and protocols, securing a total of over $50 billion U.S. dollars in on-chain value across 1500 projects!.
Our firm is well-reputed in the community and is trusted as a top smart contract auditing company for the review of solidity code, no matter how complex. Our team of experienced solidity smart contract auditors performs audits for tokens, NFTs, crowdsales, marketplaces, gambling games, financial protocols, and more!
Contact us today to get a free quote for a smart contract audit of your project!
What is a Solidity Audit?
Typically, a smart contract audit is a comprehensive review process designed to discover logical errors, security vulnerabilities, and optimization opportunities within code. A Solidity Audit takes this a step further by verifying economic logic to ensure the stability of smart contracts and highlighting privileged functionality to create a report that is easy to understand for developers and community members alike.
How Do I Interpret the Findings?
Each of our Findings will be labeled with a Severity level. We always recommend the team resolve High, Medium, and Low severity findings prior to deploying the code to the mainnet. Here is a breakdown on what each Severity level means for the project:
- High severity indicates that the issue puts a large number of users' funds at risk and has a high probability of exploitation, or the smart contract contains serious logical issues which can prevent the code from operating as intended.
- Medium severity issues are those which place at least some users' funds at risk and has a medium to high probability of exploitation.
- Low severity issues have a relatively minor risk association; these issues have a low probability of occurring or may have a minimal impact.
- Informational issues pose no immediate risk, but inform the project team of opportunities for gas optimizations and following smart contract security best practices.