feat: add reward caller signature POC

Add small untested POC for requiring a signature of the reward caller to
prevent reward caller address grieving.

Author: rickstaa

contracts/bonding/BondingManager.sol

@@ -15,16 +15,22 @@ import "../snapshots/IMerkleSnapshot.sol";
 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
@@ -148,7 +154,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
      * - 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
@@ -194,9 +200,14 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
     /**
      * @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) external whenSystemNotPaused {
+    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);
     }
@@ -211,6 +222,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         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

test/unit/BondingManager.js

@@ -6107,37 +6107,72 @@ describe("BondingManager", () => {
 
         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
+                }
 
-            it("should only allow active transcoders to set a RewardCaller", async () => {
-                const registeredTranscoder = signers[2]
-                await bondingManager
-                    .connect(registeredTranscoder)
-                    .bond(1000, registeredTranscoder.address)
-                await bondingManager
-                    .connect(registeredTranscoder)
-                    .transcoder(5, 10)
+                return signer._signTypedData(domain, types, value)
+            }
 
-                const registeredTranscoderTx = bondingManager
-                    .connect(registeredTranscoder)
-                    .setRewardCaller(nonTranscoder.address)
+            before(async () => {
+                chainId = (await ethers.provider.getNetwork()).chainId
+            })
 
-                await expect(registeredTranscoderTx).to.be.revertedWith(
-                    "caller must be an active transcoder"
+            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)
+                    .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)
+                    .setRewardCaller(nonTranscoder.address, signature)
                 await expect(setRewardCallerTx)
                     .to.emit(bondingManager, "RewardCallerSet")
                     .withArgs(transcoder.address, nonTranscoder.address)
@@ -6166,9 +6201,14 @@ describe("BondingManager", () => {
             })
 
             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)
+                    .setRewardCaller(nonTranscoder.address, signature)
                 await expect(setRewardCallerTx)
                     .to.emit(bondingManager, "RewardCallerSet")
                     .withArgs(transcoder.address, nonTranscoder.address)
@@ -6197,25 +6237,35 @@ describe("BondingManager", () => {
             })
 
             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)
+                    .setRewardCaller(nonTranscoder.address, signature)
                 await expect(setRewardCallerTx)
                     .to.emit(bondingManager, "RewardCallerSet")
                     .withArgs(transcoder.address, nonTranscoder.address)
 
                 const setRewardCallerTx2 = bondingManager
                     .connect(transcoder)
-                    .setRewardCaller(nonTranscoder.address)
+                    .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)
+                    .setRewardCaller(nonTranscoder.address, signature)
                 await expect(setRewardCallerTx)
                     .to.emit(bondingManager, "RewardCallerSet")
                     .withArgs(transcoder.address, nonTranscoder.address)
@@ -6229,9 +6279,14 @@ describe("BondingManager", () => {
             })
 
             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)
+                    .setRewardCaller(nonTranscoder.address, signature)
 
                 const rewardCallerTx = await bondingManager
                     .connect(nonTranscoder)