Add reward caller (using propose-confirm pattern)

.github/workflows/test.yaml

@@ -47,6 +47,9 @@ jobs:
           name: ${{ github.event.repository.name }}
           token: ${{ secrets.CI_CODECOV_TOKEN }}
 
+      - name: Enforce test coverage threshold
+        run: yarn test:coverage:check
+
   editorconfig:
     name: Run editorconfig checker
     runs-on: ubuntu-latest

contracts/bonding/BondingManager.sol

@@ -102,6 +102,12 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
     // If the balance of the treasury in LPT is above this value, automatic treasury contributions will halt.
     uint256 public treasuryBalanceCeiling;
 
+    // Transcoder addresses proposed by the RewardCallers
+    mapping(address => address) public rewardCallerToTranscoderProposed;
+
+    // RewardCaller addresses confirmed by the transcoders
+    mapping(address => address) public rewardCallerToTranscoderConfirmed;
+
     // Check if sender is TicketBroker
     modifier onlyTicketBroker() {
         _onlyTicketBroker();
@@ -188,6 +194,40 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         emit ParameterUpdate("numActiveTranscoders");
     }
 
+    /**
+     * @notice Propose a transcoder for a reward caller
+     * @param _transcoder Address of the transcoder
+     * @dev Only callable by the RewardCaller
+     */
+    function proposeTranscoderForRewardCaller(address _transcoder) external whenSystemNotPaused {
+        rewardCallerToTranscoderProposed[msg.sender] = _transcoder;
+        emit RewardCallerProposed(_transcoder, msg.sender);
+    }
+
+    /**
+     * @notice Confirm a reward caller for a transcoder
+     * @param _rewardCaller Address of the new reward caller
+     * @dev Only callable by the transcoder, after RewardCaller was proposed via proposeTranscoderForRewardCaller
+     */
+    function confirmRewardCaller(address _rewardCaller) external whenSystemNotPaused {
+        require(rewardCallerToTranscoderProposed[_rewardCaller] == msg.sender, "reward caller was not proposed");
+        require(rewardCallerToTranscoderConfirmed[_rewardCaller] == address(0), "reward caller is already set");
+        rewardCallerToTranscoderProposed[_rewardCaller] = address(0);
+        rewardCallerToTranscoderConfirmed[_rewardCaller] = msg.sender;
+        emit RewardCallerConfirmed(msg.sender, _rewardCaller);
+    }
+
+    /**
+     * @notice Remove a reward caller for a transcoder
+     * @param _rewardCaller Address of the existing reward caller
+     * @dev Only callable by the transcoder, when the _rewardCaller was already proposed
+     */
+    function removeRewardCaller(address _rewardCaller) external whenSystemNotPaused {
+        require(rewardCallerToTranscoderConfirmed[_rewardCaller] == msg.sender, "only relevant transcoder can remove");
+        rewardCallerToTranscoderConfirmed[_rewardCaller] = address(0);
+        emit RewardCallerRemoved(msg.sender, _rewardCaller);
+    }
+
     /**
      * @notice Sets commission rates as a transcoder and if the caller is not in the transcoder pool tries to add it
      * @dev Percentages are represented as numerators of fractions over MathUtils.PERC_DIVISOR
@@ -855,27 +895,30 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
 
     /**
      * @notice Mint token rewards for an active transcoder and its delegators and update the transcoder pool using an optional list hint if needed
-     * @dev If the caller is in the transcoder pool, the caller can provide an optional hint for its insertion position in the
-     * pool via the `_newPosPrev` and `_newPosNext` params. A linear search will be executed starting at the hint to find the correct position.
-     * In the best case, the hint is the correct position so no search is executed. See SortedDoublyLL.sol for details on list hints
+     * @dev If the caller (or the transcoder associated with the RewardCaller) is in the transcoder pool, they can provide an optional hint for its
+     * insertion position in the pool via the `_newPosPrev` and `_newPosNext` params. A linear search will be executed starting at the hint to find the
+     * correct position. In the best case, the hint is the correct position so no search is executed. See SortedDoublyLL.sol for details on list hints
      * @param _newPosPrev Address of previous transcoder in pool if the caller is in the pool
      * @param _newPosNext Address of next transcoder in pool if the caller is in the pool
      */
     function rewardWithHint(address _newPosPrev, address _newPosNext)
         public
         whenSystemNotPaused
         currentRoundInitialized
-        autoCheckpoint(msg.sender)
     {
         uint256 currentRound = roundsManager().currentRound();
 
-        require(isActiveTranscoder(msg.sender), "caller must be an active transcoder");
+        address transcoder = msg.sender;
+        if (!isActiveTranscoder(transcoder)) {
+            transcoder = rewardCallerToTranscoderConfirmed[msg.sender];
+            require(isActiveTranscoder(transcoder), "transcoder must be active");
+        }
         require(
-            transcoders[msg.sender].lastRewardRound != currentRound,
+            transcoders[transcoder].lastRewardRound != currentRound,
             "caller has already called reward for the current round"
         );
 
-        Transcoder storage t = transcoders[msg.sender];
+        Transcoder storage t = transcoders[transcoder];
         EarningsPool.Data storage earningsPool = t.earningsPoolPerRound[currentRound];
 
         // Set last round that transcoder called reward
@@ -908,17 +951,20 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
 
             mtr.trustedTransferTokens(trsry, treasuryRewards);
 
-            emit TreasuryReward(msg.sender, trsry, treasuryRewards);
+            emit TreasuryReward(transcoder, trsry, treasuryRewards);
         }
 
         uint256 transcoderRewards = totalRewardTokens.sub(treasuryRewards);
 
-        updateTranscoderWithRewards(msg.sender, transcoderRewards, currentRound, _newPosPrev, _newPosNext);
+        updateTranscoderWithRewards(transcoder, transcoderRewards, currentRound, _newPosPrev, _newPosNext);
 
         // Set last round that transcoder called reward
         t.lastRewardRound = currentRound;
 
-        emit Reward(msg.sender, transcoderRewards);
+        emit Reward(transcoder, transcoderRewards);
+
+        // Manual execution of the `autoCheckpoint` modifier due to conditional nature of `transcoder`
+        _checkpointBondingState(transcoder, delegators[transcoder], transcoders[transcoder]);
     }
 
     /**

contracts/bonding/IBondingManager.sol

@@ -44,6 +44,9 @@ interface IBondingManager {
         uint256 startRound,
         uint256 endRound
     );
+    event RewardCallerProposed(address indexed transcoder, address indexed rewardCaller);
+    event RewardCallerConfirmed(address indexed transcoder, address indexed rewardCaller);
+    event RewardCallerRemoved(address indexed transcoder, address indexed rewardCaller);
 
     // Deprecated events
     // These event signatures can be used to construct the appropriate topic hashes to filter for past logs corresponding
@@ -71,6 +74,16 @@ interface IBondingManager {
 
     function setCurrentRoundTotalActiveStake() external;
 
+    function proposeTranscoderForRewardCaller(address _transcoder) external;
+
+    function confirmRewardCaller(address _rewardCaller) external;
+
+    function removeRewardCaller(address _rewardCaller) external;
+
+    function reward() external;
+
+    function rewardWithHint(address _newPosPrev, address _newPosNext) external;
+
     // Public functions
     function getTranscoderPoolSize() external view returns (uint256);
 

package.json

@@ -21,6 +21,7 @@
     "clean": "rm -rf cache artifacts typechain",
     "compile": "npx hardhat compile",
     "test:coverage": "npx hardhat coverage",
+    "test:coverage:check": "npx istanbul check-coverage ./coverage.json --statements 100 --branches 100 --functions 100 --lines 100",
     "test": "npx hardhat test",
     "test:unit": "npx hardhat test test/unit/*.*",
     "test:integration": "npx hardhat test test/integration/**",

test/unit/BondingManager.js

@@ -5414,12 +5414,14 @@ describe("BondingManager", () => {
 
     describe("reward", () => {
         let transcoder
+        let transcoder2
         let nonTranscoder
         let currentRound
 
         beforeEach(async () => {
             transcoder = signers[0]
-            nonTranscoder = signers[1]
+            transcoder2 = signers[1]
+            nonTranscoder = signers[2]
             currentRound = 100
 
             await fixture.roundsManager.setMockBool(
@@ -5474,7 +5476,7 @@ describe("BondingManager", () => {
         it("should fail if caller is not a transcoder", async () => {
             await expect(
                 bondingManager.connect(nonTranscoder).reward()
-            ).to.be.revertedWith("caller must be an active transcoder")
+            ).to.be.revertedWith("transcoder must be active")
         })
 
         it("should fail if caller is registered but not an active transcoder yet in the current round", async () => {
@@ -5484,7 +5486,7 @@ describe("BondingManager", () => {
             )
             await expect(
                 bondingManager.connect(transcoder).reward()
-            ).to.be.revertedWith("caller must be an active transcoder")
+            ).to.be.revertedWith("transcoder must be active")
         })
 
         it("should fail if caller already called reward during the current round", async () => {
@@ -6100,6 +6102,160 @@ describe("BondingManager", () => {
                 atCeilingTest("when above limit", 1500)
             })
         })
+
+        describe("reward delegation", () => {
+            const transcoderRewards = 1000
+
+            it("should allow a transcoder to call reward even if RewardCaller is set", async () => {
+                const proposeTranscoderForRewardCallerTx = bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder.address)
+                await expect(proposeTranscoderForRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerProposed")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+                const confirmRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+                await expect(confirmRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerConfirmed")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+
+                const rewardTx = bondingManager.connect(transcoder).reward()
+                await expect(rewardTx)
+                    .to.emit(bondingManager, "Reward")
+                    .withArgs(transcoder.address, transcoderRewards)
+
+                await fixture.roundsManager.setMockUint256(
+                    functionSig("currentRound()"),
+                    currentRound + 3
+                )
+
+                // should allow a transcoder to call reward after removing RewardCaller
+                const removeRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .removeRewardCaller(nonTranscoder.address)
+                await expect(removeRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerRemoved")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+
+                const rewardTx2 = bondingManager.connect(transcoder).reward()
+                await expect(rewardTx2)
+                    .to.emit(bondingManager, "Reward")
+                    .withArgs(transcoder.address, transcoderRewards)
+            })
+
+            it("should allow a RewardCaller to call reward only after confirmation", async () => {
+                await bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder.address)
+                const rewardTx1 = bondingManager.connect(nonTranscoder).reward()
+                await expect(rewardTx1).to.be.revertedWith(
+                    "transcoder must be active"
+                )
+
+                await bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+                const rewardTx2 = bondingManager.connect(nonTranscoder).reward()
+                await expect(rewardTx2)
+                    .to.emit(bondingManager, "Reward")
+                    .withArgs(transcoder.address, transcoderRewards)
+
+                await fixture.roundsManager.setMockUint256(
+                    functionSig("currentRound()"),
+                    currentRound + 3
+                )
+                await bondingManager
+                    .connect(transcoder)
+                    .removeRewardCaller(nonTranscoder.address)
+                const rewardTx3 = bondingManager.connect(nonTranscoder).reward()
+                await expect(rewardTx3).to.be.revertedWith(
+                    "transcoder must be active"
+                )
+            })
+
+            it("should not allow a RewardCaller to be confirmed more than once", async () => {
+                await bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder.address)
+                await bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+                const confirmRewardCallerTx2 = bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+                await expect(confirmRewardCallerTx2).to.be.revertedWith(
+                    "reward caller was not proposed"
+                )
+            })
+
+            it("impossible to confirm RewardCaller without the proposal", async () => {
+                const confirmRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+                await expect(confirmRewardCallerTx).to.be.revertedWith(
+                    "reward caller was not proposed"
+                )
+            })
+
+            it("impossible to remove the RewardCaller for another transcoder", async () => {
+                await bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder.address)
+                await bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+
+                const removeRewardCallerTx = bondingManager
+                    .connect(nonTranscoder)
+                    .removeRewardCaller(nonTranscoder.address)
+                await expect(removeRewardCallerTx).to.be.revertedWith(
+                    "only relevant transcoder can remove"
+                )
+            })
+
+            it("impossible to confirm RewardCaller for a second transcoder", async () => {
+                await bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder.address)
+                await bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+                await bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder2.address)
+
+                const removeRewardCallerTx = bondingManager
+                    .connect(transcoder2)
+                    .confirmRewardCaller(nonTranscoder.address)
+                await expect(removeRewardCallerTx).to.be.revertedWith(
+                    "reward caller is already set"
+                )
+            })
+
+            it("should always checkpoint the reward recipient, not the RewardCaller", async () => {
+                await bondingManager
+                    .connect(nonTranscoder)
+                    .proposeTranscoderForRewardCaller(transcoder.address)
+                await bondingManager
+                    .connect(transcoder)
+                    .confirmRewardCaller(nonTranscoder.address)
+
+                const rewardCallerTx = await bondingManager
+                    .connect(nonTranscoder)
+                    .reward()
+
+                await expectCheckpoints(fixture, rewardCallerTx, {
+                    account: transcoder.address,
+                    startRound: currentRound + 2,
+                    bondedAmount: 1000,
+                    delegateAddress: transcoder.address,
+                    delegatedAmount: 2000,
+                    lastClaimRound: currentRound,
+                    lastRewardRound: currentRound + 1
+                })
+            })
+        })
     })
 
     describe("updateTranscoderWithFees", () => {