Add RewardsDistributorAdmin, AutoRewardsDistributor and IRewardsDistributor by ElliotFriedman · Pull Request #202 · fei-protocol/fei-protocol-core

ElliotFriedman · 2021-09-29T01:08:42Z

Todo:

DAO script
E2E tests

Done:

Add emergency kill switch to RewardsDistributorAdmin to zero out supply or borrow speeds. This function is callable by governor and guardian
Unit testing
AutoRewardsDistributor
RewardsDistributorAdmin
Add passthrough functions in the RewardsDistributorAdmin function to enable all necessary calls to the RewardsDistributor function
Added ACL to RewardsDistributorAdmin that does not leverage core to make devops and deployment process simpler and lighter weight
Added pause functionality to both RewardsDistributorAdmin and AutoRewardsDistributor contract to stop setAutoRewardsDistribution in extenuating circumstances

…ibutor

Joeysantoro · 2021-09-29T17:04:04Z

contracts/staking/AutoRewardsDistributor.sol

here and elsewhere use canonical natspec annotations

contracts/staking/AutoRewardsDistributor.sol

contracts/external/rari/IRewardsDistributor.sol

contracts/staking/AutoRewardsDistributor.sol

contracts/staking/RewardsDistributorAdmin.sol

contracts/staking/AutoRewardsDistributor.sol

contracts/feirari/RewardsDistributorAdmin.sol

xklob

Added some comments, but overall looks great so far!

contracts/feirari/AutoRewardsDistributor.sol

contracts/feirari/RewardsDistributorAdmin.sol

test/unit/feirari/AutoRewardsDistributor.test.ts

contracts/mock/MockRewardsDistributor.sol

Joeysantoro · 2021-09-30T21:40:47Z

test/unit/feirari/RewardsDistributorAdmin.test.ts

what is this testing exactly?

testing that initial compSupplySpeeds are 0. I pass 0 address as this doesn't matter in the test because it is just hitting a mock.

contracts/feirari/RewardsDistributorAdmin.sol

Joeysantoro · 2021-10-01T02:31:13Z

contracts/feirari/RewardsDistributorAdmin.sol

call this AUTO_REWARDS_DISTRIBUTOR_ROLE

Joeysantoro · 2021-10-01T02:31:50Z

contracts/feirari/RewardsDistributorAdmin.sol

        /// @notice Governance should create new admin role for this contract
        /// and then grant this role to all AutoRewardsDistributors so that they can call this contract
-        _setContractAdminRole(keccak256("REWARDS_DISTRIBUTOR_ADMIN_ROLE"));
+        _setContractAdminRole(keccak256("TRIBAL_CHIEF_ADMIN_ROLE"));


Add a comment explaining why we reuse the TribalChief one

contracts/feirari/RewardsDistributorAdmin.sol

contracts/feirari/AutoRewardsDistributor.sol

Joeysantoro · 2021-10-01T02:49:55Z

contracts/feirari/AutoRewardsDistributor.sol

isborrowIncentivized is immutable state, same with cTokenAddress. These can be removed from the event. Normally events only want to emit changed/mutable state

Joeysantoro · 2021-10-01T02:50:33Z

contracts/feirari/AutoRewardsDistributor.sol

All of these can be immutable except rewardDistributorAdmin

Joeysantoro · 2021-10-01T02:50:55Z

contracts/feirari/AutoRewardsDistributor.sol

This should have an event

contracts/feirari/RewardsDistributorAdmin.sol

Joeysantoro · 2021-10-01T02:53:47Z

contracts/feirari/RewardsDistributorAdmin.sol

+    }
+
+    /**
+      * @notice Accepts transfer of admin rights. msg.sender must be pendingAdmin


this comment is slightly inaccurate. "this" must be pendingAdmin not the sender

I just copy and pasted this comment from the rari codebase as it is a passthrough.

contracts/feirari/RewardsDistributorAdmin.sol

… & borrow speeds functionality

test/unit/feirari/RewardsDistributorAdmin.test.ts

…nts to poolAllocPoints

ElliotFriedman added 2 commits September 28, 2021 18:03

Add RewardsDistributorAdmin, AutoRewardsDistributor and IRewardsDistr…

a37418a

…ibutor

add comments

5a8dbac

ElliotFriedman requested review from Joeysantoro and xklob September 29, 2021 01:32

Joeysantoro reviewed Sep 29, 2021

View reviewed changes

add mocks, update AutoRewardsDistributor contract

c1a17cc

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Outdated Show resolved Hide resolved

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Outdated Show resolved Hide resolved

xklob reviewed Sep 30, 2021

View reviewed changes

ElliotFriedman added 2 commits September 30, 2021 11:25

update contracts, add more tests for AutoRewardsDistributor

e6d4268

only check if denominator is 0 in autorewards distributor contract

bd29751

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

test/unit/feirari/AutoRewardsDistributor.test.ts Show resolved Hide resolved

add pause functionality into RDA, add RDA unit tests

e46484a

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

test/unit/feirari/AutoRewardsDistributor.test.ts Outdated Show resolved Hide resolved

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

contracts/mock/MockRewardsDistributor.sol Show resolved Hide resolved

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

contracts/mock/MockRewardsDistributor.sol Outdated Show resolved Hide resolved

ElliotFriedman added 4 commits September 30, 2021 13:04

removed unused describe block

d598c62

fix comment on RDA constructor

4487831

change event names to camelcase, remove hardcoded string constant

320cb16

remove this. from tests and upgrade tests to use types

f74f425

Joeysantoro reviewed Sep 30, 2021

View reviewed changes

ElliotFriedman commented Sep 30, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Show resolved Hide resolved

ElliotFriedman added 3 commits September 30, 2021 18:20

update tests and smart contracts, add becomeAdmin function

f4c11a5

update RDA tests

64fad18

add additional assertion in init method

1ca05bb

Joeysantoro reviewed Oct 1, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Outdated Show resolved Hide resolved

Joeysantoro reviewed Oct 1, 2021

View reviewed changes

contracts/feirari/AutoRewardsDistributor.sol Outdated Show resolved Hide resolved

Joeysantoro reviewed Oct 1, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Show resolved Hide resolved

Joeysantoro reviewed Oct 1, 2021

View reviewed changes

update tests and contracts to address feedback

adab227

Joeysantoro reviewed Oct 1, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Outdated Show resolved Hide resolved

ElliotFriedman commented Oct 4, 2021

View reviewed changes

contracts/feirari/RewardsDistributorAdmin.sol Show resolved Hide resolved

100% test coverage on RDA, update comments, add emergency zero supply…

f07d63f

… & borrow speeds functionality

ElliotFriedman requested review from Joeysantoro and xklob October 4, 2021 04:43

Joeysantoro reviewed Oct 4, 2021

View reviewed changes

test/unit/feirari/RewardsDistributorAdmin.test.ts Outdated Show resolved Hide resolved

Joeysantoro approved these changes Oct 4, 2021

View reviewed changes

ElliotFriedman added 7 commits October 4, 2021 09:43

update naming for ARD getter function

93789b6

remove lengthy comment

f7663ef

remove unnecessary comments inside functions

efa1d15

clean up comments

6e5fecc

update constructor param comments, reviewed comments, update allocPoi…

14fe367

…nts to poolAllocPoints

add additional tests with different pool and total allocation points

6912b88

run linter, remove todo comment

f3ebfc0

ElliotFriedman merged commit 6987b6a into master Oct 4, 2021

ElliotFriedman deleted the rari-rewards-distributor-admin branch October 4, 2021 23:17

Conversation

ElliotFriedman commented Sep 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xklob left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

ElliotFriedman commented Sep 29, 2021 •

edited

Loading