From 66d1d84d819520180e3f8b96754f1a318f733a12 Mon Sep 17 00:00:00 2001 From: Rick Staa Date: Sat, 3 Jan 2026 12:55:08 +0100 Subject: [PATCH 1/2] feat: add reward call delegation Implements an alternative discussed approach that allows orchestrators to configure a custom reward caller. Co-authored-by: SidestreamSweatyPumpkin --- .github/workflows/test.yaml | 3 + contracts/bonding/BondingManager.sol | 43 ++++++-- contracts/bonding/IBondingManager.sol | 2 + package.json | 1 + test/unit/BondingManager.js | 152 +++++++++++++++++++++++++- 5 files changed, 192 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a8db0943..5dba1360 100644 --- a/.github/workflows/test.yaml +++ b/.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 diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 7d3ddab8..3872ae61 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -102,6 +102,9 @@ 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; + // Allow reward() calls by pre-defined set of addresses + mapping(address => address) private rewardCallerToTranscoder; + // Check if sender is TicketBroker modifier onlyTicketBroker() { _onlyTicketBroker(); @@ -188,6 +191,26 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { emit ParameterUpdate("numActiveTranscoders"); } + /** + * @notice Set a reward caller for a transcoder + * @param _rewardCaller Address of the new reward caller + */ + function setRewardCaller(address _rewardCaller) external whenSystemNotPaused { + require(rewardCallerToTranscoder[_rewardCaller] == address(0), "reward caller is already set"); + 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 +888,20 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { 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 +934,20 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { 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]); } /** diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index b7482085..ecf9842a 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/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 diff --git a/package.json b/package.json index e0f26e8f..8ae362bc 100644 --- a/package.json +++ b/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/**", diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index f476eb01..f812069e 100644 --- a/test/unit/BondingManager.js +++ b/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,150 @@ describe("BondingManager", () => { atCeilingTest("when above limit", 1500) }) }) + + describe("reward delegation", () => { + const transcoderRewards = 1000 + + 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) + + const registeredTranscoderTx = bondingManager + .connect(registeredTranscoder) + .setRewardCaller(nonTranscoder.address) + + await expect(registeredTranscoderTx).to.be.revertedWith( + "caller must be an active transcoder" + ) + + const activeTranscoderTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + + 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 setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + 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 setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + 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 setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + await expect(setRewardCallerTx) + .to.emit(bondingManager, "RewardCallerSet") + .withArgs(transcoder.address, nonTranscoder.address) + + const setRewardCallerTx2 = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + await expect(setRewardCallerTx2).to.be.revertedWith( + "reward caller is already set" + ) + }) + + it("impossible to unset the RewardCaller for another transcoder", async () => { + const setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + 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 () => { + await bondingManager + .connect(transcoder) + .setRewardCaller(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", () => { From 3bbf03ec0a8925c1d29554684503d61ea3c23388 Mon Sep 17 00:00:00 2001 From: Rick Staa Date: Sat, 3 Jan 2026 13:30:00 +0100 Subject: [PATCH 2/2] feat: add reward caller signature POC Add small untested POC for requiring a signature of the reward caller to prevent reward caller address grieving. --- contracts/bonding/BondingManager.sol | 18 +++++- test/unit/BondingManager.js | 95 ++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 23 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 3872ae61..8a9b4af3 100644 --- a/contracts/bonding/BondingManager.sol +++ b/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 diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index f812069e..3157f9c4 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -6107,27 +6107,57 @@ 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") @@ -6135,9 +6165,14 @@ describe("BondingManager", () => { }) 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)