Wild Credit contest - a_delamo's results

Handle

a_delamo

Vulnerability details

On RewardDistribution the function _pendingRewardPerToken can be called twice in the same transaction.

A call to distributeReward, will produce two calls _distributeReward and then, this method will call accruePool(pid) and _transferReward(_account, _pendingAccountReward(pid, _account))

  function _distributeReward(address _account, address _pair, address _token, bool _isSupply) internal {
    if (_poolExists(_pair, _token, _isSupply)) {

      uint pid = _getPid(_pair, _token, _isSupply);

      accruePool(pid);
      _transferReward(_account, _pendingAccountReward(pid, _account));

      Pool memory pool = _getPool(_pair, _token, _isSupply);
      rewardSnapshot[pid][_account] = pool.accRewardsPerToken;
    }
  }

Both accruePool and _transferReward will call _pendingAccountReward

  function _pendingRewardPerToken(Pool memory _pool) internal view returns(uint) {
    uint totalStaked = _stakedTotal(_pool);

    if (_pool.lastRewardBlock == 0 || totalStaked == 0) {
      return 0;
    }

    uint blocksElapsed = block.number - _pool.lastRewardBlock;
    return blocksElapsed * _poolRewardRate(_pool.pair, _pool.token, _pool.isSupply) * 1e12 / totalStaked;
  }

So adding a validation at the begging of the method to check if blocksElapsed == 0, we would avoid some calculations/gas

Handle

a_delamo

Vulnerability details

Impact

The method _distributeReward in RewardDistribution looks like the following. This method is doing multiple reads to a state variable, instead of doing one read and keep the result as a storage variable.

  function _distributeReward(address _account, address _pair, address _token, bool _isSupply) internal {
    if (_poolExists(_pair, _token, _isSupply)) { // First read

      uint pid = _getPid(_pair, _token, _isSupply); // Second read

      accruePool(pid);
      _transferReward(_account, _pendingAccountReward(pid, _account));

      Pool memory pool = _getPool(_pair, _token, _isSupply); // Third read
      rewardSnapshot[pid][_account] = pool.accRewardsPerToken;
    }
  }

First, _poolExists will check if the pool exists

  function _poolExists(address _pair, address _token, bool _isSupply) internal view returns(bool) {
    return pidByPairToken[_pair][_token][_isSupply].added;
  }

Then _getPid, will do again the same validation as previously.

  function _getPid(address _pair, address _token, bool _isSupply) internal view returns(uint) {
    PoolPosition memory poolPosition = pidByPairToken[_pair][_token][_isSupply];
    require(poolPosition.added, "RewardDistribution: invalid pool");

    return pidByPairToken[_pair][_token][_isSupply].pid;
  }

After accruePool will load the pool and run some operations

  function accruePool(uint _pid) public {
    Pool storage pool = pools[_pid];
    pool.accRewardsPerToken += _pendingRewardPerToken(pool);
    pool.lastRewardBlock = block.number;
  }

Next, _pendingAccountReward will load again the pool

  function _pendingAccountReward(uint _pid, address _account) internal view returns(uint) {
    Pool memory pool = pools[_pid];

    pool.accRewardsPerToken += _pendingRewardPerToken(pool);
    uint rewardsPerTokenDelta = pool.accRewardsPerToken - rewardSnapshot[_pid][_account];
    return rewardsPerTokenDelta * _stakedAccount(pool, _account) / 1e12;
  }

Finally, _getPool will load it one more time.

  function _getPool(address _pair, address _token, bool _isSupply) internal view returns(Pool memory) {
    return pools[_getPid(_pair, _token, _isSupply)];
  }

Handle

a_delamo

Vulnerability details

Impact

On LPTokenMaster.sol, there are two state variables declared as string when could be declared as bytes7. This change would save slots and gas.

  string public constant name     = "WILD-LP";
  string public constant symbol   = "WILD-LP";

bytes7 constant public name = "WILD-LP";
bytes7 constant public symbol = "WILD-LP";