second-state/interest-earner-smart-contract

A Solidity smart contract which allows users to stake, earn and un-stake.

interest-earner-smart-contract's Issues

Add a requirement to improve error message

The following code prevents a user from staking after the time period has ended. However, it gives a generic error which is most likely the .sub in the require (shown below) which is actually failing due to built in overflow checks of Solidity 0.8 and later.

require(releaseEpoch.sub(block.timestamp) > 0, "There is not enough time left to stake for this current round, please un-stake first");
uint256 secondsRemaining = releaseEpoch.sub(block.timestamp);

This can be improved to provide better error message. The following line can be added.

require(block.timestamp < releaseEpoch, "Term has already ended");

Need to remove step which checks for zero interest & compare total expected interest vs interest to pay out

When un-staking, the unstakeAllTokensAndWithdrawInterestEarned checks to see if the interestToPayOut a.k.a. the expectedInterest[msg.sender] is zero.

The stakeTokens (and also new re-stake) function actually does not allow staking if the secondsRemaining, or the weiPerSecond (values which the contract multiplies to get interestEarnedForThisStake ) are zero.

This means that the interestToPayOut can never be zero. It is simpler, less code, and more efficient to remove the if statement which check for zero interest.

This also prevents creating 2 separate safeTransfer calls; these can be combined like this

token.safeTransfer(msg.sender, interestToPayOut.add(amountToUnstake));

In that same section (the if statement) the following requirement can be removed; because this should always ring true anyway

require(totalExpectedInterest >= interestToPayOut);

Transfer tokens out of reserve pool (requires slight change)

At present the following line uses only a less than operator

require(amount < token.balanceOf(address(this)).sub((totalExpectedInterest.add(totalStateStaked))), "Can only remove tokens which are spare i.e. not put aside for end user pay out");

It can be upgraded to use <= instead.

require(amount <= token.balanceOf(address(this)).sub((totalExpectedInterest.add(totalStateStaked))), "Can only remove tokens which are spare i.e. not put aside for end user pay out");

Update order of logic to improve error message (where interest earning round has already ended)

The following lines of code around line 203 correctly revert the stakeTokens function when the interest earning round has already ended for the calling account.

uint256 secondsRemaining = releaseEpoch.sub(block.timestamp);
// We must ensure that there is a quantifiable amount of time remaining (so we can calculate some interest; albeit proportional)
require(secondsRemaining > 0, "There is not enough time left to stake for this current round");

The only issue with this order of logic is that the error from safeMath's .sub call is generic i.e. no message; just JSON RPC Error.

If we update this code to the following code, then the error message will be more accurate.

require(releaseEpoch.sub(block.timestamp) > 0, "There is not enough time left to stake for this current round, please un-stake first");
uint256 secondsRemaining = releaseEpoch.sub(block.timestamp);

Potential improvement - re-invest without withdrawl

The following code is designed to allow a reinvest (of principle and interest) without withdrawing funds.
It is achieved by adjusting internal figures and emitting event logs which would have the same effect as withdraw (principle and interest) and then reinvest by adding interest made from last round (into this next round).

    // SPDX-License-Identifier: MIT
    // WARNING this contract has not been independently tested or audited
    // DO NOT use this contract with funds of real value until officially tested and audited by an independent expert or group

    /// @dev Allows user to unstake tokens and withdraw their interest after the correct time period has elapsed and then reinvest automatically.
    /// @param token - address of the official ERC20 token which is being unlocked here.
    /// Reinvests all principle and all interest earned during the most recent term
    function reinvestAlreadyStakedTokensAndInterestEarned(IERC20 token) public timePeriodIsSet percentageIsSet noReentrant {
        // Ensure that there is a current round of interest at play
        require(initialStakingTimestamp[msg.sender] != 0, "No tokens staked at present");
        // Ensure that the current time period has elapsed and that funds are ready to be unstaked
        require(block.timestamp > (initialStakingTimestamp[msg.sender].add(timePeriod)), "Locking time period is still active, please try again later");
        // Ensure the official ERC20 contract is being referenced
        require(token == erc20Contract, "Token parameter must be the same as the erc20 contract address which was passed into the constructor");
        // Ensure there is enough reserve pool for this to proceed
        require(expectedInterest[msg.sender].add(balances[msg.sender]) <= token.balanceOf(address(this)), "Not enough STATE tokens in the reserve pool to pay out the interest earned, please contact owner of this contract");
        uint256 newAmountToInvest = expectedInterest[msg.sender].add(balances[msg.sender]);
        require(newAmountToInvest > 315360000000, "Amount to stake must be greater than 0.00000031536 ETH");
        require(newAmountToInvest < MAX_INT.div(10000), "Maximum amount must be smaller, please try again");
        // Transfer expected previous interest to staked state
        emit TokensUnstaked(msg.sender, balances[msg.sender]);
        balances[msg.sender] = balances[msg.sender].add(expectedInterest[msg.sender]);
        // Adjust totals
        // Increment the total State staked
        totalStateStaked = totalStateStaked.add(expectedInterest[msg.sender]);
        // Decrease total expected interest for this users past stake
        totalExpectedInterest = totalExpectedInterest.sub(expectedInterest[msg.sender]);
        emit InterestWithdrawn(msg.sender, expectedInterest[msg.sender]);
        // Reset msg senders expected interest
        expectedInterest[msg.sender] = 0;
        // Start a new time period
        initialStakingTimestamp[msg.sender] = block.timestamp;
        // Let's calculate the maximum amount which can be earned per annum (start with mul calculation first so we avoid values lower than one)
        uint256 interestEarnedPerAnnum_pre = newAmountToInvest.mul(percentageBasisPoints);
        // We use basis points so that Ethereum's uint256 (which does not have decimals) can have percentages of 0.01% increments. The following line caters for the basis points offset
        uint256 interestEarnedPerAnnum_post = interestEarnedPerAnnum_pre.div(10000);
        // Let's calculate how many wei are earned per second
        uint256 weiPerSecond = interestEarnedPerAnnum_post.div(31536000);
        require(weiPerSecond > 0, "Interest on this amount is too low to calculate, please try a greater amount");
        // Let's calculate the release date
        uint256 releaseEpoch = initialStakingTimestamp[msg.sender].add(timePeriod);
        // Let's fragment the interest earned per annum down to the remaining time left on this staking round
        uint256 secondsRemaining = releaseEpoch.sub(block.timestamp);
        // We must ensure that there is a quantifiable amount of time remaining (so we can calculate some interest; albeit proportional)
        require(secondsRemaining > 0, "There is not enough time left to stake for this current round");
        // There are 31536000 seconds per annum, so let's calculate the interest for this remaining time period
        uint256 interestEarnedForThisStake = weiPerSecond.mul(secondsRemaining);
        // Make sure that contract's reserve pool has enough to service this transaction. I.e. there is enough STATE in this contract to pay this user's interest (not including/counting any previous end user's staked STATE or interest which they will eventually take as a pay out)
        require(token.balanceOf(address(this)) >= totalStateStaked.add(totalExpectedInterest).add(interestEarnedForThisStake), "Not enough STATE tokens in the reserve pool, to facilitate this restake, please contact owner of this contract");
        // Adding this user's new expected interest
        totalExpectedInterest = totalExpectedInterest.add(interestEarnedForThisStake);
        // Increment the new expected interest for this user (up from being reset to zero)
        expectedInterest[msg.sender] = expectedInterest[msg.sender].add(interestEarnedForThisStake);
        emit TokensStaked(msg.sender, newAmountToInvest);
        emit InterestEarned(msg.sender, interestEarnedForThisStake);
    }

There is a walk through of this code which uses psuedo data as a way to help clarify the logic (we are not doing actual transfers in this code, we are altering the internal balances which makes it a little different that normal staking/un-staking). Same result overall i.e. when a user finally un-stakes, the culmination of their principle and interest will safeTransfer from the contract in the end (and back into their wallet).