Merge pull request #7 from curvefi/fix/fiddy

Fix/fiddy

contracts/RewardForwarder.vy

@@ -20,21 +20,28 @@ def __init__(_gauge: address):
 @external
 def deposit_reward_token(_reward_token: address):
     """
+    @param _reward_token The address of token that gets forwarded to the gauge
     @notice Deposit a reward token in the gauge
     @dev This contract should be set as the distributor of `_reward_token` in the
         gauge, else the tx will fail.
+    @dev Any user can forward deposited reward tokens to a gauge, after calling
+        `allow`.
     """
     Gauge(GAUGE).deposit_reward_token(_reward_token, ERC20(_reward_token).balanceOf(self))
 
 
 @external
 def allow(_reward_token: address):
     """
+    @param _reward_token The address of token that gets forwarded to the gauge
     @notice Allow `_reward_token` to be transferred from self to Gauge
+    @dev This method is a more gas efficient way to handle token approvals, by avoiding
+        a storage variable to verify it token was approved, and calls to the token to
+        check for approval.
     """
     response: Bytes[32] = raw_call(
         _reward_token,
-        _abi_encode(GAUGE, MAX_UINT256, method_id=method_id("approve(address,uint256")),
+        _abi_encode(GAUGE, MAX_UINT256, method_id=method_id("approve(address,uint256)")),
         max_outsize=32,
     )
     if len(response) != 0:

tests/fixtures/deployments.py

@@ -35,6 +35,12 @@ def reward_token(alice):
     return ERC20("Dummy Reward Token", "dRT", 18, deployer=alice)
 
 
+@pytest.fixture(scope="module")
+def unauthorised_token(alice):
+    """This is for testing unauthorised token"""
+    return ERC20("Dummy Unauthorised Reward Token", "dURT", 18, deployer=alice)
+
+
 @pytest.fixture(scope="module")
 def child_gauge_impl(alice, child_crv_token, ChildGauge, child_gauge_factory):
     impl = ChildGauge.deploy(child_crv_token, child_gauge_factory, {"from": alice})
@@ -48,6 +54,11 @@ def child_gauge(alice, child_gauge_impl, child_gauge_factory, lp_token, ChildGau
     return Contract.from_abi("Child Gauge", gauge_addr, ChildGauge.abi)
 
 
+@pytest.fixture(scope="module")
+def reward_forwarder(child_gauge, alice, RewardForwarder):
+    return RewardForwarder.deploy(child_gauge, {"from": alice})
+
+
 # ROOT CHAIN DAO
 
 
@@ -119,3 +130,8 @@ def root_gauge_impl(
 def root_gauge(alice, chain, root_gauge_factory, root_gauge_impl, RootGauge):
     gauge_addr = root_gauge_factory.deploy_gauge(chain.id, 0x0, {"from": alice}).return_value
     return Contract.from_abi("Root Gauge", gauge_addr, RootGauge.abi)
+
+
+@pytest.fixture(scope="module")
+def root_gauge_factory_proxy(alice, RootGaugeFactoryProxy):
+    return RootGaugeFactoryProxy.deploy({"from": alice})

tests/reward_forwarder/test_deposit_reward.py

@@ -0,0 +1,106 @@
+import brownie
+import pytest
+
+WEEK = 86400 * 7
+
+
+@pytest.fixture(scope="module", autouse=True)
+def setup(alice, bob, charlie, child_gauge, reward_token, lp_token):
+
+    lp_token.approve(child_gauge, 10**21, {"from": alice})
+    lp_token._mint_for_testing(alice, 10**21, {"from": alice})
+    child_gauge.deposit(10**21, {"from": alice})
+
+    reward_token._mint_for_testing(charlie, 10**26, {"from": alice})
+    child_gauge.set_manager(bob, {"from": alice})
+
+
+@pytest.fixture(scope="module", autouse=False)
+def setup_with_deposited_rewards(alice, bob, charlie, reward_forwarder, child_gauge, reward_token):
+
+    child_gauge.add_reward(reward_token, reward_forwarder, {"from": bob})
+    reward_token.transfer(reward_forwarder, 10**20, {"from": charlie})
+
+    reward_forwarder.allow(reward_token, {"from": alice})
+    reward_forwarder.deposit_reward_token(reward_token, {"from": charlie})
+
+
+def test_reward_deposit(alice, bob, charlie, child_gauge, reward_forwarder, reward_token):
+
+    reward_forwarder.allow(reward_token, {"from": alice})
+    child_gauge.add_reward(reward_token, reward_forwarder, {"from": bob})
+    reward_token.transfer(reward_forwarder, 10**20, {"from": charlie})
+    reward_forwarder.deposit_reward_token(reward_token, {"from": charlie})
+
+    assert reward_token.balanceOf(child_gauge) == 10**20
+    assert child_gauge.reward_data(reward_token)["rate"] > 0
+
+
+def test_reward_deposit_reverts_without_allowance(
+    bob, charlie, child_gauge, reward_forwarder, reward_token
+):
+
+    reward_token.transfer(reward_forwarder, 10**20, {"from": charlie})
+    child_gauge.add_reward(reward_token, reward_forwarder, {"from": bob})
+
+    # empty reward_forwarder cannot transfer tokens unless `allow` is called
+    with brownie.reverts():
+        reward_forwarder.deposit_reward_token(reward_token, {"from": charlie})
+
+
+def test_reward_deposit_reverts_with_unauthorised_distributor(
+    alice, charlie, reward_forwarder, reward_token
+):
+
+    reward_forwarder.allow(reward_token, {"from": alice})
+    reward_token.transfer(reward_forwarder, 10**20, {"from": charlie})
+
+    # reward_forwarder cannot deposit unless it is added as a distributor for that token
+    # in the gauge contract
+    with brownie.reverts():
+        reward_forwarder.deposit_reward_token(reward_token, {"from": charlie})
+
+
+def test_reward_deposit_revert_for_unauthorised_token(
+    alice, bob, charlie, child_gauge, reward_forwarder, reward_token, unauthorised_token
+):
+
+    unauthorised_token._mint_for_testing(charlie, 10**26, {"from": alice})
+
+    reward_forwarder.allow(reward_token, {"from": alice})
+    reward_forwarder.allow(unauthorised_token, {"from": alice})
+
+    # only add one token to gauge rewards
+    child_gauge.add_reward(reward_token, reward_forwarder, {"from": bob})
+
+    reward_token.transfer(reward_forwarder, 10**20, {"from": charlie})
+    unauthorised_token.transfer(reward_forwarder, 10**20, {"from": charlie})
+
+    with brownie.reverts():
+        reward_forwarder.deposit_reward_token(unauthorised_token, {"from": charlie})
+
+
+def test_reward_claim_when_reward_rate_is_zero(
+    alice,
+    bob,
+    charlie,
+    child_gauge,
+    chain,
+    reward_forwarder,
+    reward_token,
+    setup_with_deposited_rewards,
+):
+
+    chain.sleep(WEEK + 1)  # sleep for 1 week and 1 second
+
+    # reward forwarder has zero balance. Transferring zero reward tokens.
+    reward_forwarder.deposit_reward_token(reward_token, {"from": charlie})
+
+    # Every deposit of a reward token checkpoints reward distribution, updating integrals for users
+    # Token distribution rate should become zero:
+    assert child_gauge.reward_data(reward_token)[2] == 0
+
+    # check alice's balance before and after claims:
+    assert reward_token.balanceOf(alice) == 0
+    child_gauge.claim_rewards({"from": alice})
+    assert reward_token.balanceOf(alice) > 0  # even if rate is zero, user can claim.

tests/root_gauge_factory_proxy/test_root_gauge_factory_proxy_admin.py

@@ -0,0 +1,227 @@
+import brownie
+import pytest
+from brownie import ETH_ADDRESS
+
+
+@pytest.fixture(scope="module")
+def default_owner(root_gauge_factory_proxy):
+    yield root_gauge_factory_proxy.ownership_admin()
+
+
+@pytest.fixture(scope="module")
+def default_e_admin(root_gauge_factory_proxy):
+    yield root_gauge_factory_proxy.emergency_admin()
+
+
+@pytest.fixture(scope="module")
+def transfer_factory_ownership_to_proxy(alice, bob, root_gauge_factory, root_gauge_factory_proxy):
+    root_gauge_factory.commit_transfer_ownership(root_gauge_factory_proxy, {"from": alice})
+    root_gauge_factory_proxy.accept_transfer_ownership(root_gauge_factory, {"from": bob})
+
+
+def test_set_manager_reverts_for_unauthorised_users(bob, charlie, root_gauge_factory_proxy):
+
+    with brownie.reverts():
+        root_gauge_factory_proxy.set_manager(charlie, {"from": bob})
+
+
+def test_set_manager_success_for_authorised_users(
+    alice, bob, root_gauge_factory_proxy, chain, default_owner, default_e_admin
+):
+
+    for acct in [alice, default_owner, default_e_admin]:
+        root_gauge_factory_proxy.set_manager(bob, {"from": acct})
+        assert root_gauge_factory_proxy.manager() == bob
+        chain.undo()
+
+
+def test_commit_admin_reverts_for_unauthorised_users(
+    alice, bob, charlie, root_gauge_factory_proxy, default_e_admin
+):
+
+    for user in [alice, bob, charlie, default_e_admin]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.commit_set_admins(bob, charlie, {"from": user})
+
+
+@pytest.fixture(scope="module")
+def test_commit_admin_successful_for_authorised_user(
+    alice, bob, charlie, root_gauge_factory_proxy, default_owner
+):
+
+    root_gauge_factory_proxy.commit_set_admins(bob, charlie, {"from": default_owner})
+    assert root_gauge_factory_proxy.future_ownership_admin() == bob
+    assert root_gauge_factory_proxy.future_emergency_admin() == charlie
+
+
+def test_accept_admin_revert_for_unauthorised_user(
+    alice,
+    charlie,
+    root_gauge_factory_proxy,
+    test_commit_admin_successful_for_authorised_user,
+    default_owner,
+    default_e_admin,
+):
+
+    for acct in [alice, charlie, default_owner, default_e_admin]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.accept_set_admins({"from": acct})
+
+
+def test_accept_admin_success_for_future_ownership_admin(
+    root_gauge_factory_proxy, bob, charlie, test_commit_admin_successful_for_authorised_user
+):
+
+    root_gauge_factory_proxy.accept_set_admins({"from": bob})
+    assert root_gauge_factory_proxy.ownership_admin() == bob
+    assert root_gauge_factory_proxy.emergency_admin() == charlie
+
+
+def test_commit_transfer_ownership_reverts_for_unauthorised_users(
+    bob, charlie, root_gauge_factory, root_gauge_factory_proxy, default_owner, default_e_admin
+):
+
+    for acct in [bob, charlie, root_gauge_factory_proxy, default_owner, default_e_admin]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.commit_transfer_ownership(
+                root_gauge_factory, root_gauge_factory_proxy, {"from": acct}
+            )
+
+
+def test_commit_transfer_ownership_success_for_factory_owner(
+    alice, root_gauge_factory, root_gauge_factory_proxy
+):
+
+    root_gauge_factory.commit_transfer_ownership(root_gauge_factory_proxy, {"from": alice})
+    assert root_gauge_factory.future_owner() == root_gauge_factory_proxy
+    assert root_gauge_factory.owner() == alice
+
+
+def test_accept_transfer_ownership_success_for_authorised_admin(
+    alice, bob, charlie, chain, root_gauge_factory, root_gauge_factory_proxy
+):
+
+    root_gauge_factory.commit_transfer_ownership(root_gauge_factory_proxy, {"from": alice})
+    for acct in [alice, bob, charlie]:
+        root_gauge_factory_proxy.accept_transfer_ownership(root_gauge_factory, {"from": acct})
+        assert root_gauge_factory.owner() == root_gauge_factory_proxy
+        chain.undo()
+
+
+def test_set_killed_success_for_authorised_admin(
+    chain,
+    root_gauge,
+    root_gauge_factory_proxy,
+    transfer_factory_ownership_to_proxy,
+    default_owner,
+    default_e_admin,
+):
+
+    for authorised_admin in [default_owner, default_e_admin]:
+        root_gauge_factory_proxy.set_killed(root_gauge, True, {"from": authorised_admin})
+        assert root_gauge.is_killed()
+        assert root_gauge.inflation_params()[0] == 0  # inflation rate of root gauge should be 0
+        chain.undo()
+
+
+def test_set_killed_reverts_for_unauthorised_users(
+    alice, bob, charlie, root_gauge, root_gauge_factory_proxy, transfer_factory_ownership_to_proxy
+):
+
+    for unauthorised_acct in [alice, bob, charlie]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.set_killed(root_gauge, True, {"from": unauthorised_acct})
+
+
+def test_set_bridger_success_for_authorised_users(
+    root_gauge_factory,
+    root_gauge_factory_proxy,
+    chain,
+    transfer_factory_ownership_to_proxy,
+    default_owner,
+):
+
+    manager = root_gauge_factory_proxy.manager()
+    for acct in [manager, default_owner]:
+        root_gauge_factory_proxy.set_bridger(
+            root_gauge_factory, chain.id, ETH_ADDRESS, {"from": acct}
+        )
+        assert root_gauge_factory.get_bridger(chain.id) == ETH_ADDRESS
+        chain.undo()
+
+
+def test_set_bridger_revert_for_unauthorised_users(
+    bob,
+    charlie,
+    root_gauge_factory,
+    root_gauge_factory_proxy,
+    chain,
+    transfer_factory_ownership_to_proxy,
+    default_e_admin,
+):
+
+    for acct in [bob, charlie, default_e_admin]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.set_bridger(
+                root_gauge_factory, chain.id, ETH_ADDRESS, {"from": acct}
+            )
+
+
+def test_set_implementation_success_for_authorised_users(
+    root_gauge_factory,
+    root_gauge_factory_proxy,
+    chain,
+    transfer_factory_ownership_to_proxy,
+    default_owner,
+):
+
+    manager = root_gauge_factory_proxy.manager()
+    for acct in [manager, default_owner]:
+        root_gauge_factory_proxy.set_implementation(root_gauge_factory, ETH_ADDRESS, {"from": acct})
+        assert root_gauge_factory.get_implementation() == ETH_ADDRESS
+        chain.undo()
+
+
+def test_set_implementation_revert_for_unauthorised_users(
+    bob,
+    charlie,
+    root_gauge_factory,
+    root_gauge_factory_proxy,
+    transfer_factory_ownership_to_proxy,
+    default_e_admin,
+):
+
+    for acct in [bob, charlie, default_e_admin]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.set_implementation(
+                root_gauge_factory, ETH_ADDRESS, {"from": acct}
+            )
+
+
+def test_set_call_proxy_success_for_authorised_users(
+    root_gauge_factory,
+    root_gauge_factory_proxy,
+    chain,
+    transfer_factory_ownership_to_proxy,
+    default_owner,
+):
+
+    manager = root_gauge_factory_proxy.manager()
+    for acct in [manager, default_owner]:
+        root_gauge_factory_proxy.set_call_proxy(root_gauge_factory, ETH_ADDRESS, {"from": acct})
+        assert root_gauge_factory.call_proxy() == ETH_ADDRESS
+        chain.undo()
+
+
+def test_set_call_proxy_revert_for_unauthorised_users(
+    bob,
+    charlie,
+    root_gauge_factory,
+    root_gauge_factory_proxy,
+    transfer_factory_ownership_to_proxy,
+    default_e_admin,
+):
+
+    for acct in [bob, charlie, default_e_admin]:
+        with brownie.reverts():
+            root_gauge_factory_proxy.set_call_proxy(root_gauge_factory, ETH_ADDRESS, {"from": acct})