Ethos Reserve contest - SuperRayss's results

Issues Found

Ethos-Core\contracts\LQTY\CommunityIssuance.sol

There is a loss of precision in calculating rewardPerSecond. This is due to rewardPerSecond being assigned the result of the lossy division of distributionPeriod from amount (line 112). This loss in precision results in a growing deviation between expected and actual issuance of $Oath tokens to the stability pool.

Ethos-Core\test\LQTYIssuanceArithmeticTest.js

The test case on line 170 passes when it shouldn’t due to improper usage of arithmetic operators and type conversion on BigNumber variables, resulting in the test case comparing the integers 4 and 3 (line 181) rather than their intended values. When corrected, this test case does not pass with the current implementation of fund() in CommunityIssuance.sol due to loss of precision with calculating rewardPerSecond. The corrected test case fails with AssertionError: expected 2255118 [Wei] to be at most 1000 [Wei].

The corrected test case:

  it("aggregates multiple funding rounds within a distribution period", async () => {
    await setupFunder(accounts[0], oathToken, communityIssuanceTester, million);
    await communityIssuanceTester.fund(thousand);
    await th.fastForwardTime(604800, web3.currentProvider); // 7 days pass
    await communityIssuanceTester.unprotectedIssueLQTY(); // Issue Oath
    await communityIssuanceTester.fund(thousand);
    await th.fastForwardTime(604800, web3.currentProvider);
    await communityIssuanceTester.unprotectedIssueLQTY();
    await communityIssuanceTester.fund(thousand);
    await th.fastForwardTime(1209600, web3.currentProvider); // 14 days pass
    await communityIssuanceTester.unprotectedIssueLQTY();
    const final = await communityIssuanceTester.totalOATHIssued(); // Get total issued Oath
    th.assertIsApproximatelyEqual(final, thousand.mul(th.toBN(3)), 1000);
  })

Proposed Changes

Ethos-Core\contracts\LQTY\CommunityIssuance.sol

Use DECIMAL_PRECISION (inherited from BaseMath.sol) when assigning rewardPerSecond to minimize loss of precision

Line 44 (optional, but recommended): uint internal rewardPerSecond; Line 112: rewardPerSecond = amount.mul(DECIMAL_PRECISION).div(distributionPeriod);

Create new function getRewardPerSecond instead of accessing the rewardPerSecond property directly

  function getRewardPerSecond() public view returns (uint) {
    return rewardPerSecond.div(DECIMAL_PRECISION);
  }

Create new function getRewardAmount to accurately calculate rewards generated over a given time period

  function getRewardAmount(uint seconds_) public view returns (uint) {
    return rewardPerSecond.mul(seconds_).div(DECIMAL_PRECISION);
  }

Implement getRewardPerSecond and getRewardAmount

Line 89: issuance = getRewardAmount(timePassed); Line 108: uint notIssued = getRewardAmount(timeLeft); Line 116: emit LogRewardPerSecond(getRewardPerSecond());

Ethos-Core\test\LQTYIssuanceArithmeticTest.js

Overhaul test cases to be compatible with proposed changes to CommunityIssuance.sol

Case #1

  it("Issues a set amount of rewards per second", async () => {
    await setupFunder(accounts[0], oathToken, communityIssuanceTester, million);
    await communityIssuanceTester.fund(thousand);
    const blockTimestampOne = await th.getLatestBlockTimestamp(web3);
    await th.fastForwardTime(100, web3.currentProvider);
    await communityIssuanceTester.unprotectedIssueLQTY();
    const issuance = await communityIssuanceTester.totalOATHIssued();
    const blockTimestampTwo = await th.getLatestBlockTimestamp(web3);
    const difference = blockTimestampTwo - blockTimestampOne;
    const rewards = await communityIssuanceTester.getRewardAmount(difference);
    assert.isTrue(issuance.eq(rewards));
  })

Case #2

  it("aggregates multiple funding rounds within a distribution period", async () => {
    await setupFunder(accounts[0], oathToken, communityIssuanceTester, million);
    await communityIssuanceTester.fund(thousand);
    await communityIssuanceTester.fund(thousand);
    await communityIssuanceTester.fund(thousand);
    const final = await communityIssuanceTester.getRewardAmount(1209600); // 14 days - default distribution period
    const compare = thousand.mul(th.toBN(3));
    th.assertIsApproximatelyEqual(final, compare, 1000);
  })

Implement getRewardPerSecond in other test cases

Line 107: const rps = await communityIssuanceTester.getRewardPerSecond(); Line 189: const rps = await communityIssuanceTester.getRewardPerSecond();

Ethos-Core\contracts\TestContracts\CommunityIssuanceTester.sol

Implement getRewardAmount

Line 12: issuance = getRewardAmount(timePassed);