From 3a4056dc6dcb5985925e03ad0adca246e4fd83b6 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 30 Jun 2025 12:50:41 +0200 Subject: [PATCH 01/13] add rewardCallerToTranscoder mapping, reward logic, setter --- contracts/bonding/BondingManager.sol | 28 +++++++++++++++++++++------ contracts/bonding/IBondingManager.sol | 1 + 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index c401324b..0bdeeb07 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,15 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { emit ParameterUpdate("numActiveTranscoders"); } + /** + * @notice Set (or unset using 0 address) a reward caller for a transcoder + * @param _rewardCaller Address of a trusted reward caller + */ + function setRewardCaller(address _rewardCaller) external whenSystemNotPaused { + rewardCallerToTranscoder[_rewardCaller] = msg.sender; + emit RewardCallerUpdated(_rewardCaller, msg.sender); + } + /** * @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 @@ -869,13 +881,17 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { { 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"); + } 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 +924,17 @@ 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); } /** diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index b7482085..21b7b344 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/contracts/bonding/IBondingManager.sol @@ -44,6 +44,7 @@ interface IBondingManager { uint256 startRound, uint256 endRound ); + event RewardCallerUpdated(address indexed rewardCaller, address indexed transcoder); // Deprecated events // These event signatures can be used to construct the appropriate topic hashes to filter for past logs corresponding From 32d094a442f7364e6b43afbee2a0ab57bb97215e Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 30 Jun 2025 13:14:31 +0200 Subject: [PATCH 02/13] add basic tests --- test/unit/BondingManager.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index 161a3f85..37b4cc67 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -6100,6 +6100,42 @@ describe("BondingManager", () => { atCeilingTest("when above limit", 1500) }) }) + + describe("reward delegation", () => { + const transcoderRewards = 1000 + + it("should allow a RewardCaller to call reward", async () => { + // Transcoder should be able to set a non-transcoder as a reward caller + const setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + await expect(setRewardCallerTx) + .to.emit(bondingManager, "RewardCallerUpdated") + .withArgs(nonTranscoder.address, transcoder.address) + + // Non-transcoder should now be able to call reward on behalf of the transcoder + const rewardTx = bondingManager.connect(nonTranscoder).reward() + await expect(rewardTx) + .to.emit(bondingManager, "Reward") + .withArgs(transcoder.address, transcoderRewards) + }) + + it("should allow a transcoder to call reward even if RewardCaller is set", async () => { + // Transcoder should be able to set a non-transcoder as a reward caller + const setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + await expect(setRewardCallerTx) + .to.emit(bondingManager, "RewardCallerUpdated") + .withArgs(nonTranscoder.address, transcoder.address) + + // Non-transcoder should now be able to call reward on behalf of the transcoder + const rewardTx = bondingManager.connect(transcoder).reward() + await expect(rewardTx) + .to.emit(bondingManager, "Reward") + .withArgs(transcoder.address, transcoderRewards) + }) + }) }) describe("updateTranscoderWithFees", () => { From e69bdcb817aec06e3621b91bd3dc7511b7272780 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 30 Jun 2025 13:18:40 +0200 Subject: [PATCH 03/13] ci: enforce test coverage threshold --- .github/workflows/test.yaml | 3 +++ package.json | 1 + 2 files changed, 4 insertions(+) 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/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/**", From 6fe4dfa69f7e0bdc6026aebfcc31d4656ef6bed6 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 30 Jun 2025 16:02:36 +0200 Subject: [PATCH 04/13] add unsetRewardCaller --- contracts/bonding/BondingManager.sol | 21 +++++++-- contracts/bonding/IBondingManager.sol | 3 +- test/unit/BondingManager.js | 62 +++++++++++++++++++++------ 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 0bdeeb07..93ec77bf 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -192,12 +192,25 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { } /** - * @notice Set (or unset using 0 address) a reward caller for a transcoder - * @param _rewardCaller Address of a trusted reward caller + * @notice Set a reward caller for a transcoder + * @param _rewardCaller Address of the new reward caller */ function setRewardCaller(address _rewardCaller) external whenSystemNotPaused { + address transcoder = rewardCallerToTranscoder[_rewardCaller]; + require(transcoder == address(0), "reward caller is already set"); rewardCallerToTranscoder[_rewardCaller] = msg.sender; - emit RewardCallerUpdated(_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 { + address transcoder = rewardCallerToTranscoder[_rewardCaller]; + require(transcoder == msg.sender, "only relevant transcoder can unset"); + rewardCallerToTranscoder[_rewardCaller] = address(0); + emit RewardCallerUnset(msg.sender, _rewardCaller); } /** @@ -884,7 +897,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { address transcoderAddress = msg.sender; if (!isActiveTranscoder(transcoderAddress)) { transcoderAddress = rewardCallerToTranscoder[msg.sender]; - require(isActiveTranscoder(transcoderAddress), "caller must be an active transcoder"); + require(isActiveTranscoder(transcoderAddress), "caller must be an active transcoder or rewardCaller"); } require( transcoders[transcoderAddress].lastRewardRound != currentRound, diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index 21b7b344..ecf9842a 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/contracts/bonding/IBondingManager.sol @@ -44,7 +44,8 @@ interface IBondingManager { uint256 startRound, uint256 endRound ); - event RewardCallerUpdated(address indexed rewardCaller, address indexed transcoder); + 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/test/unit/BondingManager.js b/test/unit/BondingManager.js index 37b4cc67..b813c837 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 () => { @@ -6104,36 +6108,66 @@ describe("BondingManager", () => { describe("reward delegation", () => { const transcoderRewards = 1000 - it("should allow a RewardCaller to call reward", async () => { - // Transcoder should be able to set a non-transcoder as a reward caller + 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, "RewardCallerUpdated") - .withArgs(nonTranscoder.address, transcoder.address) + .to.emit(bondingManager, "RewardCallerSet") + .withArgs(transcoder.address, nonTranscoder.address) - // Non-transcoder should now be able to call reward on behalf of the transcoder - const rewardTx = bondingManager.connect(nonTranscoder).reward() + 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 transcoder to call reward even if RewardCaller is set", async () => { - // Transcoder should be able to set a non-transcoder as a reward caller + it("should allow a RewardCaller to call reward", async () => { const setRewardCallerTx = bondingManager .connect(transcoder) .setRewardCaller(nonTranscoder.address) await expect(setRewardCallerTx) - .to.emit(bondingManager, "RewardCallerUpdated") - .withArgs(nonTranscoder.address, transcoder.address) + .to.emit(bondingManager, "RewardCallerSet") + .withArgs(transcoder.address, nonTranscoder.address) - // Non-transcoder should now be able to call reward on behalf of the transcoder - const rewardTx = bondingManager.connect(transcoder).reward() + 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" + ) }) }) }) From a3bd4306c93aa0595d13296cbf091d0e62c114dd Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 30 Jun 2025 16:28:47 +0200 Subject: [PATCH 05/13] full test coverage --- test/unit/BondingManager.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index b813c837..af66b8b2 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -6169,6 +6169,38 @@ describe("BondingManager", () => { "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" + ) + }) }) }) From 5bce7ca22eb89df2c3e1375fdfb8cdc08041206f Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 30 Jun 2025 16:58:34 +0200 Subject: [PATCH 06/13] remove unnecessary variable --- contracts/bonding/BondingManager.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 93ec77bf..3c3ef708 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -196,8 +196,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { * @param _rewardCaller Address of the new reward caller */ function setRewardCaller(address _rewardCaller) external whenSystemNotPaused { - address transcoder = rewardCallerToTranscoder[_rewardCaller]; - require(transcoder == address(0), "reward caller is already set"); + require(rewardCallerToTranscoder[_rewardCaller] == address(0), "reward caller is already set"); rewardCallerToTranscoder[_rewardCaller] = msg.sender; emit RewardCallerSet(msg.sender, _rewardCaller); } @@ -207,8 +206,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { * @param _rewardCaller Address of the existing reward caller */ function unsetRewardCaller(address _rewardCaller) external whenSystemNotPaused { - address transcoder = rewardCallerToTranscoder[_rewardCaller]; - require(transcoder == msg.sender, "only relevant transcoder can unset"); + require(rewardCallerToTranscoder[_rewardCaller] == msg.sender, "only relevant transcoder can unset"); rewardCallerToTranscoder[_rewardCaller] = address(0); emit RewardCallerUnset(msg.sender, _rewardCaller); } From f1531dae4b86958580080345f356b670dbb278fe Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Mon, 8 Sep 2025 10:26:21 +0200 Subject: [PATCH 07/13] use propose + confirm pattern to prevent front-running --- contracts/bonding/BondingManager.sol | 62 ++++++++----- contracts/bonding/IBondingManager.sol | 5 +- test/unit/BondingManager.js | 127 +++++++++++++++++--------- 3 files changed, 125 insertions(+), 69 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 3c3ef708..890ae99b 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -102,8 +102,11 @@ 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; + // Transcoder addresses proposed by the RewardCallers + mapping(address => address) public rewardCallerToTranscoderProposed; + + // Transcoder addresses confirmed by the transcoders + mapping(address => address) public rewardCallerToTranscoderConfirmed; // Check if sender is TicketBroker modifier onlyTicketBroker() { @@ -192,23 +195,36 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { } /** - * @notice Set a reward caller for a transcoder + * @notice Propose a transcoder for a reward caller + * @param _transcoder Address of the transcoder + * @dev Only callable by the RewardCaller + */ + function proposeRewardCaller(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 proposeRewardCaller */ - 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); + function confirmRewardCaller(address _rewardCaller) external whenSystemNotPaused { + require(rewardCallerToTranscoderProposed[_rewardCaller] == msg.sender, "reward caller was not proposed"); + rewardCallerToTranscoderProposed[_rewardCaller] = address(0); + rewardCallerToTranscoderConfirmed[_rewardCaller] = msg.sender; + emit RewardCallerConfirmed(msg.sender, _rewardCaller); } /** - * @notice Unset a reward caller for a transcoder + * @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 unsetRewardCaller(address _rewardCaller) external whenSystemNotPaused { - require(rewardCallerToTranscoder[_rewardCaller] == msg.sender, "only relevant transcoder can unset"); - rewardCallerToTranscoder[_rewardCaller] = address(0); - emit RewardCallerUnset(msg.sender, _rewardCaller); + function removeRewardCaller(address _rewardCaller) external whenSystemNotPaused { + require(rewardCallerToTranscoderConfirmed[_rewardCaller] == msg.sender, "only relevant transcoder can unset"); + rewardCallerToTranscoderConfirmed[_rewardCaller] = address(0); + emit RewardCallerRemoved(msg.sender, _rewardCaller); } /** @@ -888,21 +904,20 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { public whenSystemNotPaused currentRoundInitialized - autoCheckpoint(msg.sender) { uint256 currentRound = roundsManager().currentRound(); - address transcoderAddress = msg.sender; - if (!isActiveTranscoder(transcoderAddress)) { - transcoderAddress = rewardCallerToTranscoder[msg.sender]; - require(isActiveTranscoder(transcoderAddress), "caller must be an active transcoder or rewardCaller"); + address transcoder = msg.sender; + if (!isActiveTranscoder(transcoder)) { + transcoder = rewardCallerToTranscoderConfirmed[msg.sender]; + require(isActiveTranscoder(transcoder), "caller must be an active transcoder or rewardCaller"); } require( - transcoders[transcoderAddress].lastRewardRound != currentRound, + transcoders[transcoder].lastRewardRound != currentRound, "caller has already called reward for the current round" ); - Transcoder storage t = transcoders[transcoderAddress]; + Transcoder storage t = transcoders[transcoder]; EarningsPool.Data storage earningsPool = t.earningsPoolPerRound[currentRound]; // Set last round that transcoder called reward @@ -935,17 +950,20 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { mtr.trustedTransferTokens(trsry, treasuryRewards); - emit TreasuryReward(transcoderAddress, trsry, treasuryRewards); + emit TreasuryReward(transcoder, trsry, treasuryRewards); } uint256 transcoderRewards = totalRewardTokens.sub(treasuryRewards); - updateTranscoderWithRewards(transcoderAddress, transcoderRewards, currentRound, _newPosPrev, _newPosNext); + updateTranscoderWithRewards(transcoder, transcoderRewards, currentRound, _newPosPrev, _newPosNext); // Set last round that transcoder called reward t.lastRewardRound = currentRound; - emit Reward(transcoderAddress, transcoderRewards); + emit Reward(transcoder, transcoderRewards); + + // Manual execution of the `autoCheckpoint` modifier due to conditional nature of `transcoder` + _checkpointBondingState(transcoder, delegators[transcoder], transcoders[transcoder]); } /** diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index ecf9842a..4369523f 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/contracts/bonding/IBondingManager.sol @@ -44,8 +44,9 @@ interface IBondingManager { uint256 startRound, uint256 endRound ); - event RewardCallerSet(address indexed transcoder, address indexed rewardCaller); - event RewardCallerUnset(address indexed transcoder, address indexed rewardCaller); + 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 diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index af66b8b2..2695462a 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -6109,11 +6109,17 @@ describe("BondingManager", () => { const transcoderRewards = 1000 it("should allow a transcoder to call reward even if RewardCaller is set", async () => { - const setRewardCallerTx = bondingManager + const proposeRewardCallerTx = bondingManager + .connect(nonTranscoder) + .proposeRewardCaller(transcoder.address) + await expect(proposeRewardCallerTx) + .to.emit(bondingManager, "RewardCallerProposed") + .withArgs(transcoder.address, nonTranscoder.address) + const confirmRewardCallerTx = bondingManager .connect(transcoder) - .setRewardCaller(nonTranscoder.address) - await expect(setRewardCallerTx) - .to.emit(bondingManager, "RewardCallerSet") + .confirmRewardCaller(nonTranscoder.address) + await expect(confirmRewardCallerTx) + .to.emit(bondingManager, "RewardCallerConfirmed") .withArgs(transcoder.address, nonTranscoder.address) const rewardTx = bondingManager.connect(transcoder).reward() @@ -6126,11 +6132,12 @@ describe("BondingManager", () => { currentRound + 3 ) - const unsetRewardCallerTx = bondingManager + // should allow a transcoder to call reward after removing RewardCaller + const removeRewardCallerTx = bondingManager .connect(transcoder) - .unsetRewardCaller(nonTranscoder.address) - await expect(unsetRewardCallerTx) - .to.emit(bondingManager, "RewardCallerUnset") + .removeRewardCaller(nonTranscoder.address) + await expect(removeRewardCallerTx) + .to.emit(bondingManager, "RewardCallerRemoved") .withArgs(transcoder.address, nonTranscoder.address) const rewardTx2 = bondingManager.connect(transcoder).reward() @@ -6139,16 +6146,20 @@ describe("BondingManager", () => { .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) + it("should allow a RewardCaller to call reward only after confirmation", async () => { + await bondingManager + .connect(nonTranscoder) + .proposeRewardCaller(transcoder.address) + const rewardTx1 = bondingManager.connect(nonTranscoder).reward() + await expect(rewardTx1).to.be.revertedWith( + "caller must be an active transcoder or rewardCaller" + ) - const rewardTx = bondingManager.connect(nonTranscoder).reward() - await expect(rewardTx) + await bondingManager + .connect(transcoder) + .confirmRewardCaller(nonTranscoder.address) + const rewardTx2 = bondingManager.connect(nonTranscoder).reward() + await expect(rewardTx2) .to.emit(bondingManager, "Reward") .withArgs(transcoder.address, transcoderRewards) @@ -6156,51 +6167,77 @@ describe("BondingManager", () => { functionSig("currentRound()"), currentRound + 3 ) - - const unsetRewardCallerTx = bondingManager + await 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( + .removeRewardCaller(nonTranscoder.address) + const rewardTx3 = bondingManager.connect(nonTranscoder).reward() + await expect(rewardTx3).to.be.revertedWith( "caller must be an active transcoder or rewardCaller" ) }) - it("impossible to set the same RewardCaller twice", async () => { - const setRewardCallerTx = bondingManager + it("should not allow a RewardCaller to be confirmed more than once", async () => { + await bondingManager + .connect(nonTranscoder) + .proposeRewardCaller(transcoder.address) + await bondingManager .connect(transcoder) - .setRewardCaller(nonTranscoder.address) - await expect(setRewardCallerTx) - .to.emit(bondingManager, "RewardCallerSet") - .withArgs(transcoder.address, nonTranscoder.address) + .confirmRewardCaller(nonTranscoder.address) + const confirmRewardCallerTx2 = bondingManager + .connect(transcoder) + .confirmRewardCaller(nonTranscoder.address) + await expect(confirmRewardCallerTx2).to.be.revertedWith( + "reward caller was not proposed" + ) + }) - const setRewardCallerTx2 = bondingManager + it("impossible to confirm RewardCaller without the proposal", async () => { + const confirmRewardCallerTx = bondingManager .connect(transcoder) - .setRewardCaller(nonTranscoder.address) - await expect(setRewardCallerTx2).to.be.revertedWith( - "reward caller is already set" + .confirmRewardCaller(nonTranscoder.address) + await expect(confirmRewardCallerTx).to.be.revertedWith( + "reward caller was not proposed" ) }) - it("impossible to unset the RewardCaller for another transcoder", async () => { - const setRewardCallerTx = bondingManager + it("impossible to remove the RewardCaller for another transcoder", async () => { + await bondingManager + .connect(nonTranscoder) + .proposeRewardCaller(transcoder.address) + await bondingManager .connect(transcoder) - .setRewardCaller(nonTranscoder.address) - await expect(setRewardCallerTx) - .to.emit(bondingManager, "RewardCallerSet") - .withArgs(transcoder.address, nonTranscoder.address) + .confirmRewardCaller(nonTranscoder.address) - const unsetRewardCallerTx = bondingManager + const removeRewardCallerTx = bondingManager .connect(nonTranscoder) - .unsetRewardCaller(nonTranscoder.address) - await expect(unsetRewardCallerTx).to.be.revertedWith( + .removeRewardCaller(nonTranscoder.address) + await expect(removeRewardCallerTx).to.be.revertedWith( "only relevant transcoder can unset" ) }) + + it("should always checkpoint the reward recipient, not the RewardCaller", async () => { + await bondingManager + .connect(nonTranscoder) + .proposeRewardCaller(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 + }) + }) }) }) From 9289d4f00adfbd5cd64e6daf34806399477962b2 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Wed, 10 Sep 2025 13:55:55 +0200 Subject: [PATCH 08/13] adjust error message --- contracts/bonding/BondingManager.sol | 2 +- test/unit/BondingManager.js | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 890ae99b..2c70ff16 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -910,7 +910,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { address transcoder = msg.sender; if (!isActiveTranscoder(transcoder)) { transcoder = rewardCallerToTranscoderConfirmed[msg.sender]; - require(isActiveTranscoder(transcoder), "caller must be an active transcoder or rewardCaller"); + require(isActiveTranscoder(transcoder), "transcoder must be active"); } require( transcoders[transcoder].lastRewardRound != currentRound, diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index 2695462a..7447972f 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -5474,9 +5474,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 or rewardCaller" - ) + ).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 () => { @@ -5486,9 +5484,7 @@ describe("BondingManager", () => { ) await expect( bondingManager.connect(transcoder).reward() - ).to.be.revertedWith( - "caller must be an active transcoder or rewardCaller" - ) + ).to.be.revertedWith("transcoder must be active") }) it("should fail if caller already called reward during the current round", async () => { @@ -6152,7 +6148,7 @@ describe("BondingManager", () => { .proposeRewardCaller(transcoder.address) const rewardTx1 = bondingManager.connect(nonTranscoder).reward() await expect(rewardTx1).to.be.revertedWith( - "caller must be an active transcoder or rewardCaller" + "transcoder must be active" ) await bondingManager @@ -6172,7 +6168,7 @@ describe("BondingManager", () => { .removeRewardCaller(nonTranscoder.address) const rewardTx3 = bondingManager.connect(nonTranscoder).reward() await expect(rewardTx3).to.be.revertedWith( - "caller must be an active transcoder or rewardCaller" + "transcoder must be active" ) }) From 40454bd68c25989e96741c2f98f13489fd72ba62 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Wed, 10 Sep 2025 13:58:21 +0200 Subject: [PATCH 09/13] adjust proposeRewardCaller -> proposeTranscoderForRewardCaller --- contracts/bonding/BondingManager.sol | 4 ++-- test/unit/BondingManager.js | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 2c70ff16..bed48db7 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -199,7 +199,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { * @param _transcoder Address of the transcoder * @dev Only callable by the RewardCaller */ - function proposeRewardCaller(address _transcoder) external whenSystemNotPaused { + function proposeTranscoderForRewardCaller(address _transcoder) external whenSystemNotPaused { rewardCallerToTranscoderProposed[msg.sender] = _transcoder; emit RewardCallerProposed(_transcoder, msg.sender); } @@ -207,7 +207,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { /** * @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 proposeRewardCaller + * @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"); diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index 7447972f..6018326c 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -6105,10 +6105,10 @@ describe("BondingManager", () => { const transcoderRewards = 1000 it("should allow a transcoder to call reward even if RewardCaller is set", async () => { - const proposeRewardCallerTx = bondingManager + const proposeTranscoderForRewardCallerTx = bondingManager .connect(nonTranscoder) - .proposeRewardCaller(transcoder.address) - await expect(proposeRewardCallerTx) + .proposeTranscoderForRewardCaller(transcoder.address) + await expect(proposeTranscoderForRewardCallerTx) .to.emit(bondingManager, "RewardCallerProposed") .withArgs(transcoder.address, nonTranscoder.address) const confirmRewardCallerTx = bondingManager @@ -6145,7 +6145,7 @@ describe("BondingManager", () => { it("should allow a RewardCaller to call reward only after confirmation", async () => { await bondingManager .connect(nonTranscoder) - .proposeRewardCaller(transcoder.address) + .proposeTranscoderForRewardCaller(transcoder.address) const rewardTx1 = bondingManager.connect(nonTranscoder).reward() await expect(rewardTx1).to.be.revertedWith( "transcoder must be active" @@ -6175,7 +6175,7 @@ describe("BondingManager", () => { it("should not allow a RewardCaller to be confirmed more than once", async () => { await bondingManager .connect(nonTranscoder) - .proposeRewardCaller(transcoder.address) + .proposeTranscoderForRewardCaller(transcoder.address) await bondingManager .connect(transcoder) .confirmRewardCaller(nonTranscoder.address) @@ -6199,7 +6199,7 @@ describe("BondingManager", () => { it("impossible to remove the RewardCaller for another transcoder", async () => { await bondingManager .connect(nonTranscoder) - .proposeRewardCaller(transcoder.address) + .proposeTranscoderForRewardCaller(transcoder.address) await bondingManager .connect(transcoder) .confirmRewardCaller(nonTranscoder.address) @@ -6215,7 +6215,7 @@ describe("BondingManager", () => { it("should always checkpoint the reward recipient, not the RewardCaller", async () => { await bondingManager .connect(nonTranscoder) - .proposeRewardCaller(transcoder.address) + .proposeTranscoderForRewardCaller(transcoder.address) await bondingManager .connect(transcoder) .confirmRewardCaller(nonTranscoder.address) From c58cda23c62c27a59bf616963d736ed89f75f657 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Wed, 10 Sep 2025 16:30:34 +0200 Subject: [PATCH 10/13] improved rewardCaller mapping comment --- contracts/bonding/BondingManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index bed48db7..798d9773 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -105,7 +105,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { // Transcoder addresses proposed by the RewardCallers mapping(address => address) public rewardCallerToTranscoderProposed; - // Transcoder addresses confirmed by the transcoders + // RewardCaller addresses confirmed by the transcoders mapping(address => address) public rewardCallerToTranscoderConfirmed; // Check if sender is TicketBroker From 639e3243b031822e718864b53ea73fb4fd561266 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Wed, 10 Sep 2025 17:05:02 +0200 Subject: [PATCH 11/13] improve dev comment for the rewardWithHint --- contracts/bonding/BondingManager.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 798d9773..0846d6a5 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -894,9 +894,9 @@ 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 */ From 84cd2fe0c69ed7af25d584da313f1536249ecdd1 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Wed, 10 Sep 2025 17:09:09 +0200 Subject: [PATCH 12/13] add missing interfaces --- contracts/bonding/IBondingManager.sol | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index 4369523f..380550e9 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/contracts/bonding/IBondingManager.sol @@ -74,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); From 7c38b7101331916011014dd3201465bfd51712c1 Mon Sep 17 00:00:00 2001 From: SidestreamSweatyPumpkin Date: Wed, 10 Sep 2025 17:22:53 +0200 Subject: [PATCH 13/13] add additional check: reward caller is already set --- contracts/bonding/BondingManager.sol | 3 ++- test/unit/BondingManager.js | 25 +++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 0846d6a5..457d443a 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -211,6 +211,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { */ 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); @@ -222,7 +223,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { * @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 unset"); + require(rewardCallerToTranscoderConfirmed[_rewardCaller] == msg.sender, "only relevant transcoder can remove"); rewardCallerToTranscoderConfirmed[_rewardCaller] = address(0); emit RewardCallerRemoved(msg.sender, _rewardCaller); } diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index 6018326c..27c05f57 100644 --- a/test/unit/BondingManager.js +++ b/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( @@ -6208,7 +6210,26 @@ describe("BondingManager", () => { .connect(nonTranscoder) .removeRewardCaller(nonTranscoder.address) await expect(removeRewardCallerTx).to.be.revertedWith( - "only relevant transcoder can unset" + "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" ) })