Unknown behavior of updatePendingInfo function

Critical  | Resolved

‍Entire contract depends on this function, but little or no details were provided on how often this func will be called, what conditions, what limits of each user, how many users, etc.

Additionally, this function actually contains two actions: users pending info update and pop per block calculation.

Worth to mention, that right now devaddr has too much power.

Recommendation:

If this is expected behavior (for example davaddr is a contract), please provide as many details as possible how users’ funds are protected and stake pool is fair. And it would be better to do pop per block calculation “on the fly”, more details on this in next issue.

Re-Audit:

Pop masternode running time is logged in backend and backend cron job will call updatePendingInfo function everyday with the updated Info with dev address permission.

‍

The owner can transfer user tokens to any address

Critical  |  Resolved

‍‍Function tokenTransfer allows the owner of the contract to transfer all pop tokens from the balance of the staking contract to any address.

function tokenTransfer(address _to, IERC20 _token, uint256 _amount) public onlyOwner {
    uint256 tokenBal = _token.balanceOf(address(this));
    if (_amount > tokenBal) {
        _token.transfer(_to, tokenBal);
    } else {
        _token.transfer(_to, _amount);
    }
}

Recommendation:

Set these tokens to be not salvageable.

Re-Audit:

Fixed.

‍

popPerBlock may not be updated in time

High  |  Resolved

It is updated only on updatePendingInfo call. If, for any reason, the call does not happen in time – popPerBlock becomes outdated.

Additionally:

• Before cycle 1 popPerBlock value is 0, which will reset claimable value to 0 due to this multiplication: user.amount.mul(popPerBlock);
• After cycle four there is no reset of the popPerBlock variable to a default value. So it will get stuck on popPerBlockCycleFour value (unless this is requirement).

Recommendation:

Do calculation of popPerBlock automatically, example:

function getPopPerBlock() public view returns (uint256) {
    //pass & save uint256[] _popPerBlockAllCycles; in constructor
    if(block.timestamp < startTime)return 1;
    uint256 cycle = (block.timestamp - startTime) / 90 days;
    //we can hardcode 3 to save gas     
    (_popPerBlockAllCycles.length - 1)
    if(cycle > 3)cycle = 3;
    return _popPerBlockAllCycles[cycle];
}

Re-Audit:

Fixed.

‍

Constructor could be improved

Medium  |  Resolved

There are several options of improvements, based on popPerBlock calculation improve.

Recommendation:

• If only one pool: you can simply set popPerBlockCycleOne...Four as constants, and set popPerBlock on initialization;
• If several pools, then you need to have that calculation in one place. Please create getPopPerBlock() (example of it shown above) and remove what is not needed from the constructor.

Re-Audit:

Fixed.

‍

No check for 0 value

Medium  |  Resolved

Functions deposit / withdraw have no check for an amount equal to 0.

Recommendation:

Functions deposit / withdraw have no check for an amount equal to 0.

require(amount > 0, " deposit: can’t deposit 0");
require(amount > 0, "withdraw: can’t withdraw 0");

Re-Audit:

Fixed.

‍

Unnecessary variable initialization

Medium  |  Resolved

In constructor at line 61 no need to initialize popPerBlock variables, since it’s already been initialized by default.

Recommendation:

Remove line #61:

popPerBlock = 0;

Re-Audit:

Fixed.

‍

No practical use in function getMultiplier

Informational  |  Unresolved

Function getMultiplier simply subtracts one number from another. It has no practical use for the end-user.

Recommendation:

It should return user reward multiplier instead:

function getMultiplier() public view returns (uint256) {
    UserInfo storage user = userInfo[msg.sender];
        return user.rewardMultiplier
}

‍

No SPDX-License-Identifier found

Informational  |  Resolved

Trust in smart contracts can be better established if their source code is available. Since making source code available always touches on legal problems with regards to copyright, the Solidity compiler encourages the use of machine-readable SPDX license identifiers.

Recommendation:

Every source file should start with a comment indicating its license.

Example:

// SPDX-License-Identifier: UNLICENSED

Re-Audit:

Fixed.

The rich text element allows you to create and format headings, paragraphs, blockquotes, images, and video all in one place instead of having to add and format them individually. Just double-click and easily create content.

Static and dynamic content editing

A rich text element can be used with static or dynamic content. For static content, just drop it into any page and begin editing. For dynamic content, add a rich text field to any collection and then connect a rich text element to that field in the settings panel. Voila!

How to customize formatting for each rich text

Headings, paragraphs, blockquotes, figures, images, and figure captions can all be styled after a class is added to the rich text element using the "When inside of" nested selector system.