ERC721Staking.sol

ERC721Staking.sol

// SPDX-License-Identifier: MIT // OK
// Creator: andreitoma8 // OK
pragma solidity ^0.8.4; // OK

import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; // OK
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; // OK
import "@openzeppelin/contracts/token/ERC721/IERC721.sol"; // OK
import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; // OK
import "@openzeppelin/contracts/access/Ownable.sol"; // OK

contract ERC721Staking is ERC721Holder, Ownable { // @audit Is there any point in using ERC721Holder if the receipt function does not link into stake?
    using SafeERC20 for IERC20; // @audit unused: Consider using SafeTransfer

    // Interfaces for ERC20 and ERC721 // OK
    IERC20 public rewardsToken; // @audit rewardsToken can be made immutable to reduce gas costs
    IERC721 public nftCollection; // @audit nftCollection can be made immutable to reduce gas costs

    // Staker info // OK
    struct Staker { // OK
        // Amount of ERC721 Tokens staked // OK
        uint256 amountStaked; // @note The total amount of NFTs staked
        // Last time of details update for this User
        uint256 timeOfLastUpdate; // @note The last time rewards have been accumulated (stake, unstake, claimRewards)
        // Calculated, but unclaimed rewards for the User. The rewards are
        // calculated each time the user writes to the Smart Contract
        uint256 unclaimedRewards; // OK
    }

    // Rewards per hour per token deposited in wei.
    // Rewards are cumulated once every hour.
    uint256 private rewardsPerHour = 100000;

    // Mapping of User Address to Staker info
    mapping(address => Staker) stakers; // @audit timeOfLastUpdate is not public
    // Mapping of Token Id to staker. Made for the SC to remeber
    // who to send back the ERC721 Token to.
    mapping(uint256 => address) stakerAddress; // @audit `stakerAddress` is not public

    // Constructor function
    constructor(address _nftCollection, address _rewardsToken) { // @audit Best Practice: _nftCollection and _rewardsToken can be directly provided as IERC721 and IERC20.
        nftCollection = IERC721(_nftCollection);
        rewardsToken = IERC20(_rewardsToken);
    }

    // If address already has ERC721 Token/s staked, calculate the rewards.
    // For every new Token Id in param transferFrom user to this Smart Contract,
    // increment the amountStaked and map msg.sender to the Token Id of the staked
    // Token to later send back on withdrawal. Finally give timeOfLastUpdate the
    // value of now.
    function stake(uint256[] memory _tokenIds) public { // @audit `stake` can be marked as external // @audit Gas optimization: `_tokenIds` can be provided as calldata.
        if (stakers[msg.sender].amountStaked > 0) { // OK
            uint256 rewards = calculateRewards(msg.sender); // OK
            stakers[msg.sender].unclaimedRewards += rewards; // OK
        }
        for (uint256 i; i < _tokenIds.length; ++i) {
            require( // OK
                nftCollection.ownerOf(_tokenIds[i]) == msg.sender, // OK
                "Can't stake tokens you don't own!" // OK
            ); // OK
            nftCollection.transferFrom(msg.sender, address(this), _tokenIds[i]); // @audit HIGH RISK: Unsafe against reentrancy exploits, can drain the rewards.
            stakers[msg.sender].amountStaked++; // @audit Gas optimization: amountStaked can be incremented with tokenIds.length once
            stakerAddress[_tokenIds[i]] = msg.sender; // OK
        }
        stakers[msg.sender].timeOfLastUpdate = block.timestamp; // OK
    }

    // Check if user has any ERC721 Tokens Staked and if he tried to withdraw,
    // calculate the rewards and store them in the unclaimedRewards and for each
    // ERC721 Token in param: check if msg.sender is the original staker, decrement
    // the amountStaked of the user and transfer the ERC721 token back to them.
    function withdraw(uint256[] memory _tokenIds) public {  // @audit `withdraw` can be marked as external. // @audit Gas optimization: `_tokenIds` can be provided as calldata.
        require( // OK
            stakers[msg.sender].amountStaked > 0, // OK
            "You have no tokens staked" // OK
        ); // OK
        uint256 rewards = calculateRewards(msg.sender); // OK
        stakers[msg.sender].unclaimedRewards += rewards; // OK
        for (uint256 i; i < _tokenIds.length; ++i) { 
            require(stakerAddress[_tokenIds[i]] == msg.sender); // @audit stakerAddress is not reset.
            stakers[msg.sender].amountStaked--; // @audit Gas optimization: amountStaked can be decremented with tokenIds.length once
            nftCollection.transferFrom(address(this), msg.sender, _tokenIds[i]); // @audit HIGH RISK: Unsafe against reentrancy exploits, can drain the rewards.
        }
        stakers[msg.sender].timeOfLastUpdate = block.timestamp;
    }

    // Calculate rewards for the msg.sender, check if there are any rewards
    // claim, set unclaimedRewards to 0 and transfer the ERC20 Reward token
    // to the user.
    function claimRewards() public { // @audit `claimRewards` can be made external.
        uint256 rewards = calculateRewards(msg.sender) + // OK
            stakers[msg.sender].unclaimedRewards; // OK
        require(rewards > 0, "You have no rewards to claim"); // OK
        stakers[msg.sender].timeOfLastUpdate = block.timestamp; // OK
        stakers[msg.sender].unclaimedRewards = 0; // OK
        rewardsToken.transfer(msg.sender, rewards); // @audit safeTransfer should be used here.
    }

    // Set the rewardsPerHour variable // OK
    function setRewardsPerHour(uint256 _newValue) public onlyOwner { // @audit `setRewardsPerHour` can be made external. // @audit `setRewardsPerHour` should emit an event  
        rewardsPerHour = _newValue; // @audit MED: This affects rewards in hindsight!
    } // OK

    //////////
    // View //
    //////////

    function userStakeInfo(address _user) // @audit `userStakeInfo` can be made external.
        public
        view
        returns (uint256 _tokensStaked, uint256 _availableRewards)
    {
        return (stakers[_user].amountStaked, availableRewards(_user)); // @audit userStakeInfo reverts for users without a stake which might be undesirable (see #L116).
    }

    function availableRewards(address _user) internal view returns (uint256) { // OK
        require(stakers[_user].amountStaked > 0, "User has no tokens staked"); // OK
        uint256 _rewards = stakers[_user].unclaimedRewards + // OK
            calculateRewards(_user); // OK
        return _rewards; // OK
    }

    /////////////
    // Internal//
    /////////////

    // Calculate rewards for param _staker by calculating the time passed
    // since last update in hours and mulitplying it to ERC721 Tokens Staked
    // and rewardsPerHour.
    function calculateRewards(address _staker) // OK
        internal // OK
        view // OK
        returns (uint256 _rewards) // OK
    { // OK
        return (
            ((
                (((block.timestamp - stakers[_staker].timeOfLastUpdate) /
                    3600) * stakers[msg.sender].amountStaked)
            ) * rewardsPerHour) // @audit MEDIUM SEVERITY: Division before multiplication can cause significant rounding error here.
        );
    }
}