feat/add reward caller/reward caller to transcoder signature

.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

@@ -15,21 +15,27 @@
 import "./IBondingVotes.sol";
 
 import "@openzeppelin/contracts/utils/math/SafeMath.sol";
+import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
+import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 
 /**
  * @title BondingManager
  * @notice Manages bonding, transcoder and rewards/fee accounting related operations of the Livepeer protocol
  */
-contract BondingManager is ManagerProxyTarget, IBondingManager {
+contract BondingManager is ManagerProxyTarget, IBondingManager, EIP712 {
     using SafeMath for uint256;
     using SortedDoublyLL for SortedDoublyLL.Data;
     using EarningsPool for EarningsPool.Data;
     using EarningsPoolLIP36 for EarningsPool.Data;
+    using ECDSA for bytes32;
+
+    bytes32 private constant REWARD_CALLER_TYPEHASH =
+        keccak256("RewardCallerApproval(address rewardCaller,address transcoder)");
 
     // Constants
     // Occurances are replaced at compile time
     // and computed to a single value if possible by the optimizer
     uint256 constant MAX_FUTURE_ROUND = 2**256 - 1;
 
     // Time between unbonding and possible withdrawl in rounds
     uint64 public unbondingPeriod;
@@ -102,6 +108,9 @@
     // If the balance of the treasury in LPT is above this value, automatic treasury contributions will halt.
     uint256 public treasuryBalanceCeiling;
 
+    // Allow reward() calls by pre-defined set of addresses
+    mapping(address => address) private rewardCallerToTranscoder;
+
     // Check if sender is TicketBroker
     modifier onlyTicketBroker() {
         _onlyTicketBroker();
@@ -145,7 +154,7 @@
      * - setNumActiveTranscoders()
      * @param _controller Address of Controller that this contract will be registered with
      */
-    constructor(address _controller) Manager(_controller) {}
+    constructor(address _controller) Manager(_controller) EIP712("BondingManager", "1") {}
 
     /**
      * @notice Set unbonding period. Only callable by Controller owner
@@ -188,6 +197,32 @@
         emit ParameterUpdate("numActiveTranscoders");
     }
 
+    /**
+     * @notice Set a reward caller for a transcoder
+     * @param _rewardCaller Address of the new reward caller
+     * @param _sig Signature from _rewardCaller approving msg.sender
+     */
+    function setRewardCaller(address _rewardCaller, bytes calldata _sig) external whenSystemNotPaused {
+        require(rewardCallerToTranscoder[_rewardCaller] == address(0), "reward caller is already set");
+        bytes32 structHash = keccak256(abi.encode(REWARD_CALLER_TYPEHASH, _rewardCaller, msg.sender));
+        bytes32 digest = _hashTypedDataV4(structHash);
+        address signer = ECDSA.recover(digest, _sig);
+        require(signer == _rewardCaller, "invalid reward caller signature");
+        rewardCallerToTranscoder[_rewardCaller] = msg.sender;
+        emit RewardCallerSet(msg.sender, _rewardCaller);
+    }
+
+    /**
+     * @notice Unset a reward caller for a transcoder
+     * @param _rewardCaller Address of the existing reward caller
+     */
+    function unsetRewardCaller(address _rewardCaller) external whenSystemNotPaused {
+        require(rewardCallerToTranscoder[_rewardCaller] == msg.sender, "only relevant transcoder can unset");
+        rewardCallerToTranscoder[_rewardCaller] = address(0);
+        emit RewardCallerUnset(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
@@ -865,17 +900,20 @@
         public
         whenSystemNotPaused
         currentRoundInitialized
-        autoCheckpoint(msg.sender)
     {
         uint256 currentRound = roundsManager().currentRound();
 
-        require(isActiveTranscoder(msg.sender), "caller must be an active transcoder");
+        address transcoderAddress = msg.sender;
+        if (!isActiveTranscoder(transcoderAddress)) {
+            transcoderAddress = rewardCallerToTranscoder[msg.sender];
+            require(isActiveTranscoder(transcoderAddress), "caller must be an active transcoder or rewardCaller");
+        }
         require(
-            transcoders[msg.sender].lastRewardRound != currentRound,
+            transcoders[transcoderAddress].lastRewardRound != currentRound,
             "caller has already called reward for the current round"
         );
 
-        Transcoder storage t = transcoders[msg.sender];
+        Transcoder storage t = transcoders[transcoderAddress];
         EarningsPool.Data storage earningsPool = t.earningsPoolPerRound[currentRound];
 
         // Set last round that transcoder called reward
@@ -908,17 +946,20 @@
 
             mtr.trustedTransferTokens(trsry, treasuryRewards);
 
-            emit TreasuryReward(msg.sender, trsry, treasuryRewards);
+            emit TreasuryReward(transcoderAddress, trsry, treasuryRewards);
         }
 
         uint256 transcoderRewards = totalRewardTokens.sub(treasuryRewards);
 
-        updateTranscoderWithRewards(msg.sender, transcoderRewards, currentRound, _newPosPrev, _newPosNext);
+        updateTranscoderWithRewards(transcoderAddress, transcoderRewards, currentRound, _newPosPrev, _newPosNext);
 
         // Set last round that transcoder called reward
         t.lastRewardRound = currentRound;
 
-        emit Reward(msg.sender, transcoderRewards);
+        emit Reward(transcoderAddress, transcoderRewards);
+
+        // Manual execution of the `autoCheckpoint` modifier due to conditional nature of `transcoderAddress`
+        _checkpointBondingState(transcoderAddress, delegators[transcoderAddress], transcoders[transcoderAddress]);
     }
 
     /**

contracts/bonding/IBondingManager.sol

@@ -44,6 +44,8 @@ interface IBondingManager {
         uint256 startRound,
         uint256 endRound
     );
+    event RewardCallerSet(address indexed transcoder, address indexed rewardCaller);
+    event RewardCallerUnset(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

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

@@ -5474,7 +5474,9 @@ 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(
+                "caller must be an active transcoder or rewardCaller"
+            )
         })
 
         it("should fail if caller is registered but not an active transcoder yet in the current round", async () => {
@@ -5484,7 +5486,9 @@ describe("BondingManager", () => {
             )
             await expect(
                 bondingManager.connect(transcoder).reward()
-            ).to.be.revertedWith("caller must be an active transcoder")
+            ).to.be.revertedWith(
+                "caller must be an active transcoder or rewardCaller"
+            )
         })
 
         it("should fail if caller already called reward during the current round", async () => {
@@ -6100,6 +6104,205 @@ describe("BondingManager", () => {
                 atCeilingTest("when above limit", 1500)
             })
         })
+
+        describe("reward delegation", () => {
+            const transcoderRewards = 1000
+            let chainId
+
+            const getRewardCallerSignature = async (
+                signer,
+                rewardCallerAddress,
+                transcoderAddress
+            ) => {
+                const domain = {
+                    name: "BondingManager",
+                    version: "1",
+                    chainId,
+                    verifyingContract: bondingManager.address
+                }
+                const types = {
+                    RewardCallerApproval: [
+                        {name: "rewardCaller", type: "address"},
+                        {name: "transcoder", type: "address"}
+                    ]
+                }
+                const value = {
+                    rewardCaller: rewardCallerAddress,
+                    transcoder: transcoderAddress
+                }
+
+                return signer._signTypedData(domain, types, value)
+            }
+
+            before(async () => {
+                chainId = (await ethers.provider.getNetwork()).chainId
+            })
+
+            it("should require reward caller signature to set a RewardCaller", async () => {
+                const badSignature = await getRewardCallerSignature(
+                    transcoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                await expect(
+                    bondingManager
+                        .connect(transcoder)
+                        .setRewardCaller(nonTranscoder.address, badSignature)
+                ).to.be.revertedWith("invalid reward caller signature")
+
+                const signature = await getRewardCallerSignature(
+                    nonTranscoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                const activeTranscoderTx = bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+
+                await expect(activeTranscoderTx)
+                    .to.emit(bondingManager, "RewardCallerSet")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+            })
+
+            it("should allow a transcoder to call reward even if RewardCaller is set", async () => {
+                const signature = await getRewardCallerSignature(
+                    nonTranscoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                const setRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+                await expect(setRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerSet")
+                    .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
+                )
+
+                const unsetRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .unsetRewardCaller(nonTranscoder.address)
+                await expect(unsetRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerUnset")
+                    .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", async () => {
+                const signature = await getRewardCallerSignature(
+                    nonTranscoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                const setRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+                await expect(setRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerSet")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+
+                const rewardTx = bondingManager.connect(nonTranscoder).reward()
+                await expect(rewardTx)
+                    .to.emit(bondingManager, "Reward")
+                    .withArgs(transcoder.address, transcoderRewards)
+
+                await fixture.roundsManager.setMockUint256(
+                    functionSig("currentRound()"),
+                    currentRound + 3
+                )
+
+                const unsetRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .unsetRewardCaller(nonTranscoder.address)
+                await expect(unsetRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerUnset")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+
+                const rewardTx2 = bondingManager.connect(nonTranscoder).reward()
+                await expect(rewardTx2).to.be.revertedWith(
+                    "caller must be an active transcoder or rewardCaller"
+                )
+            })
+
+            it("impossible to set the same RewardCaller twice", async () => {
+                const signature = await getRewardCallerSignature(
+                    nonTranscoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                const setRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+                await expect(setRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerSet")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+
+                const setRewardCallerTx2 = bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+                await expect(setRewardCallerTx2).to.be.revertedWith(
+                    "reward caller is already set"
+                )
+            })
+
+            it("impossible to unset the RewardCaller for another transcoder", async () => {
+                const signature = await getRewardCallerSignature(
+                    nonTranscoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                const setRewardCallerTx = bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+                await expect(setRewardCallerTx)
+                    .to.emit(bondingManager, "RewardCallerSet")
+                    .withArgs(transcoder.address, nonTranscoder.address)
+
+                const unsetRewardCallerTx = bondingManager
+                    .connect(nonTranscoder)
+                    .unsetRewardCaller(nonTranscoder.address)
+                await expect(unsetRewardCallerTx).to.be.revertedWith(
+                    "only relevant transcoder can unset"
+                )
+            })
+
+            it("should always checkpoint the reward recipient, not the RewardCaller", async () => {
+                const signature = await getRewardCallerSignature(
+                    nonTranscoder,
+                    nonTranscoder.address,
+                    transcoder.address
+                )
+                await bondingManager
+                    .connect(transcoder)
+                    .setRewardCaller(nonTranscoder.address, signature)
+
+                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", () => {