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 c401324b..a7eda6bd 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 from one pre-defined address per transcoder + mapping(address => address) public transcoderToRewardCaller; + // Check if sender is TicketBroker modifier onlyTicketBroker() { _onlyTicketBroker(); @@ -188,6 +191,16 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { emit ParameterUpdate("numActiveTranscoders"); } + /** + * @notice Set a reward caller for a transcoder + * @param _rewardCaller Address of the new reward caller + * @dev By providing address(0) the reward caller can be unset + */ + function setRewardCaller(address _rewardCaller) external whenSystemNotPaused { + transcoderToRewardCaller[msg.sender] = _rewardCaller; + emit RewardCallerSet(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 @@ -294,6 +307,15 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { rewardWithHint(address(0), address(0)); } + /** + * @notice Mint token rewards for an active transcoder and its delegators + * @param _transcoder Address of the transcoder on behalf of which the reward is called + * @dev Only callable by trusted rewardCaller + */ + function rewardForTranscoder(address _transcoder) external { + rewardForTranscoderWithHint(_transcoder, address(0), address(0)); + } + /** * @notice Update transcoder's fee pool. Only callable by the TicketBroker * @param _transcoder Transcoder address @@ -861,21 +883,53 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { * @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) - { + function rewardWithHint(address _newPosPrev, address _newPosNext) public { + _rewardWithHint(msg.sender, _newPosPrev, _newPosNext); + } + + /** + * @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 `_transcoder` 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 Only callable by trusted rewardCaller + * @param _transcoder Address of the transcoder on behalf of which the reward is called + * @param _newPosPrev Address of previous transcoder in pool if the `_transcoder` is in the pool + * @param _newPosNext Address of previous transcoder in pool if the `_transcoder` is in the pool + */ + function rewardForTranscoderWithHint( + address _transcoder, + address _newPosPrev, + address _newPosNext + ) public { + address rewardCaller = transcoderToRewardCaller[_transcoder]; + require(rewardCaller == msg.sender, "caller must be a reward caller set by the transcoder"); + _rewardWithHint(_transcoder, _newPosPrev, _newPosNext); + } + + /** + * @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 `_transcoder` 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 + * @param _transcoder Address of the transcoder on behalf of which the reward is called + * @param _newPosPrev Address of previous transcoder in pool if `_transcoder` is in the pool + * @param _newPosNext Address of next transcoder in pool if `_transcoder` is in the pool + */ + function _rewardWithHint( + address _transcoder, + address _newPosPrev, + address _newPosNext + ) private whenSystemNotPaused currentRoundInitialized autoCheckpoint(_transcoder) { uint256 currentRound = roundsManager().currentRound(); - require(isActiveTranscoder(msg.sender), "caller must be an active transcoder"); + 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 +962,17 @@ 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); } /** diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index b7482085..b044e256 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/contracts/bonding/IBondingManager.sol @@ -44,6 +44,7 @@ interface IBondingManager { uint256 startRound, uint256 endRound ); + event RewardCallerSet(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 +72,20 @@ interface IBondingManager { function setCurrentRoundTotalActiveStake() external; + function setRewardCaller(address _rewardCaller) external; + + function reward() external; + + function rewardWithHint(address _newPosPrev, address _newPosNext) external; + + function rewardForTranscoder(address _transcoder) external; + + function rewardForTranscoderWithHint( + address _transcoder, + address _newPosPrev, + address _newPosNext + ) external; + // Public functions function getTranscoderPoolSize() external view returns (uint256); 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 161a3f85..42de876b 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -5474,7 +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") + ).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 +5484,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 +6100,130 @@ 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 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) + .setRewardCaller(ZERO_ADDRESS) + await expect(unsetRewardCallerTx) + .to.emit(bondingManager, "RewardCallerSet") + .withArgs(transcoder.address, ZERO_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) + .rewardForTranscoder(transcoder.address) + await expect(rewardTx) + .to.emit(bondingManager, "Reward") + .withArgs(transcoder.address, transcoderRewards) + + await fixture.roundsManager.setMockUint256( + functionSig("currentRound()"), + currentRound + 3 + ) + + const unsetRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(ZERO_ADDRESS) + await expect(unsetRewardCallerTx) + .to.emit(bondingManager, "RewardCallerSet") + .withArgs(transcoder.address, ZERO_ADDRESS) + + const rewardTx2 = bondingManager + .connect(nonTranscoder) + .rewardForTranscoder(transcoder.address) + await expect(rewardTx2).to.be.revertedWith( + "caller must be a reward caller set by the transcoder" + ) + }) + + it("should fail if system is paused", async () => { + await bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + + await fixture.controller.pause() + const setRewardCallerTx = bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + await expect(setRewardCallerTx).to.be.revertedWith( + "system is paused" + ) + + const rewardTx = bondingManager + .connect(nonTranscoder) + .rewardForTranscoder(transcoder.address) + await expect(rewardTx).to.be.revertedWith("system is paused") + }) + + it("should fail if current round is not initialized", async () => { + await bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + await fixture.roundsManager.setMockBool( + functionSig("currentRoundInitialized()"), + false + ) + const rewardTx = bondingManager + .connect(nonTranscoder) + .rewardForTranscoder(transcoder.address) + await expect(rewardTx).to.be.revertedWith( + "current round is not initialized" + ) + }) + + it("should always checkpoint the reward recipient, not the RewardCaller", async () => { + await bondingManager + .connect(transcoder) + .setRewardCaller(nonTranscoder.address) + const rewardCallerTx = await bondingManager + .connect(nonTranscoder) + .rewardForTranscoder(transcoder.address) + + await expectCheckpoints(fixture, rewardCallerTx, { + account: transcoder.address, + startRound: currentRound + 2, + bondedAmount: 1000, + delegateAddress: transcoder.address, + delegatedAmount: 2000, + lastClaimRound: currentRound, + lastRewardRound: currentRound + 1 + }) + }) + }) }) describe("updateTranscoderWithFees", () => {