Findings

Issue 01

Type

Severity

Location

Status

Owner Capabilities

High

withdrawERC20

✔️ Resolved

Description

The owner of the contract can withdraw users' staked tokens at any time.

Recommendation

If the purpose of this function is to withdraw tokens accidentally sent to the contract, consider adding a require statement that makes sure the withdrawn token by the owner is not the native token. If it is the native token, add a variable that tracks the tokens staked and stops the owner with withdrawing them.

Second Revision

The owner of the contract can modify the token address, and therefore can still withdraw any token.

Issue 02

Type

Severity

Location

Status

Gas Optimization

Informational

receive

✔️ Resolved

Description

By default, ETH can't be sent to smart contracts unless they have a receive function.

Recommendation

Consider removing the receive function as it's not necessary.

Issue 03

Type

Severity

Location

Status

Volatile Code

High

deposit / fundPool

✔️ Resolved

Description

transferFrom function can silently fail without being reverted.

Recommendation

Use safeTransferFrom or check the return value of the transferFrom function.

Issue 04

Type

Severity

Location

Status

Logical Issue

Medium

withdraw

✔️ Resolved

Description

The following line can trigger a math underflow if mainPen (that is based on the staked amount) is larger than the reward

uint256 totalReward = (reward - (mainPen + subPen));

Issue 05

Type

Severity

Location

Status

Logical Issue

Informational

emergancyWithdraw

✔️ Resolved

Description

In emergencyWithdraw function, stakedAmount is not decreased.

Issue 06

Type

Severity

Location

Status

Logical Issue

Medium

calculateRew

✔️ Resolved

According to the docs if

* | apy = 10000000000000000000=> %10 Monthly

The equation for calculating the rewards is wrong.

Issue 07

Type

Severity

Location

Status

Best Practice

Low

calculateRew

Not Resolved

Description

Penalty and APY percentages use a 1e18 multiplier, that allows for extremely high precision. However, it is customary to use a lower multiplier like 100 or 1,000, to minimize the risk of human error.

Recommendation

We suggest reducing the 1e18 multiplier to 1,000, which provides a precision of up to 0.1%

Issue 08

Type

Severity

Location

Status

Best Practice

Low

calculateFutureRew / calculateRewFromAmount / calculateRew

Not Resolved

Description

It is bad practice to duplicate core functionality of the code (e.g. APY calculation).

Recommendation

Use only one function that calculates the APY and calls it with the appropriate parameters every time.

We recommend using one calculate APY function that receives:

amount, apy, duration.

Issue 09

Type

Severity

Location

Status

Logical Issue

High

claimPending

✔️ Resolved

Description

If a pool has no mainPenalty, mainAmount will be 0 which means the user won't be able to withdraw his original staked amount.

General Notes

  • Consider adding an emergency withdrawal function that would let the user simply withdraw his staked tokens without doing extra logic, even if the token is paused. Note that if this suggestion is accepted, it may allow users to avoid penalties by calling emergencyWithdraw - The team has decided to enable emergency withdrawals only when the staking contract is paused.

  • isFinished modifier checks if the staker status is not finished, consider renaming the function to reflect this logic.

  • The code does not support a token with transfer fees - staker.amount and the reserves of the pool may not be correct which will every function that withdraws it.

Last updated