RewardTracker Manual Review Findings

RTR-01M: Inexistent Transfer of Stake-Related Balances

Type	Severity	Location
Logical Fault		RewardTracker.sol:L224-L235, L263, L264, L279, L283

Description:

The RewardTracker::_transfer function will not properly transfer the stakedAmounts of the user nor their depositBalances, effectively preventing the recipient of the EIP-20 asset from unstaking.

Impact:

A user who mints the RewardTracker and sells the token to another member will continue to acquire rewards, and will be unable to unstake their funds.

Example:

Copy
224function _transfer(address _sender, address _recipient, uint256 _amount) private {
225    require(_sender != address(0), "RewardTracker: transfer from the zero address");
226    require(_recipient != address(0), "RewardTracker: transfer to the zero address");
227
228    if (inPrivateTransferMode) { _validateHandler(); }
229
230    require(balances[_sender] >= _amount, "RewardTracker: transfer amount exceeds balance");
231    balances[_sender] = balances[_sender] - _amount;
232    balances[_recipient] = balances[_recipient] + _amount;
233
234    emit Transfer(_sender, _recipient,_amount);
235}

Recommendation:

We advise the RewardTracker to properly transfer the staking-related accounting values to the new recipient, ensuring that they are able to unstake the EIP-20 value they have acquired.

Alleviation (b02fae335f):

The Moby team has clarified that the inPrivateTransferMode and inPrivateStakingMode variables will remain true forever, however, this contradicts their dedicated setter functions in the codebase.

We advise the functions to be removed entirely, the inPrivateTransferMode and inPrivateStakingMode variables to be omitted, and all conditional logic to instead always execute for this exhibit to be considered alleviated in this regard.

Alleviation (a8720219a6):

Our recommendation has been adhered to, removing the dedicated setter functions as well as inPrivateTransferMode and inPrivateStakingMode mode concepts from the contract and executing as if they are true at all times.

As such, we consider this exhibit fully alleviated.

RTR-02M: Invalid Multi-Asset Deposit System

Type	Severity	Location
Logical Fault		RewardTracker.sol:L71-L74, L79-L81, L251-L252, L267

Description:

The RewardTracker permits multiple tokens to be permitted for staking which is invalid as the RewardTracker::_stake mechanism will mint an equivalent amount of tokens to the amount transferred in.

Impact:

The RewardTracker staking system permits multiple different tokens to be staked each of which is considered to be equal to one another, an invalid assumption as the tokens may have different decimals and / or value.

Example:

Copy
250function _stake(address _fundingAccount, address _account, address _depositToken, uint256 _amount) private {
251    require(_amount > 0, "RewardTracker: invalid _amount");
252    require(isDepositToken[_depositToken], "RewardTracker: invalid _depositToken");
253
254    // send deposit token to RewardTracker
255    IERC20(_depositToken).safeTransferFrom(_fundingAccount, address(this), _amount);
256
257    // update rewards for the account
258    _updateRewards(_account);
259
260    // increase staked amounts
261    // - increase deposit balances of the account
262    // - increase total deposit supply
263    stakedAmounts[_account] = stakedAmounts[_account] + _amount;
264    depositBalances[_account][_depositToken] = depositBalances[_account][_depositToken] + _amount;
265    totalDepositSupply[_depositToken] = totalDepositSupply[_depositToken] + _amount;
266
267    _mint(_account, _amount);
268}

Recommendation:

We advise this approach to be revisited, as tokens may have different decimals and will always have different evaluations even if they are stablecoins.

Alleviation (b02fae335f):

The Moby team has clarified that only a single token will be utilized throughout the contract, however, this contradicts the contract's implementation to theoretically support multiple assets.

In order to accept this as an alleviation, all multi-asset related functions should be removed from the codebase and the contract should implement code as if there is a single deposit token at all times.

Alleviation (a8720219a6):

The codebase has been refactored to incorporate a single deposit token, effectively adhering to our recommendation and rendering this exhibit fully alleviated.