From 62b3e196ba19772435586f8c2da59a1e1eadecac Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Fri, 25 Aug 2023 19:28:27 -0300 Subject: [PATCH] bonding: Treasury rewards contribution (#616) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * package.json: Update @openzeppelin libraries Will be required for new checkpoints code and later governor implementation. * bonding: Create SortedArrays library Used for checkpointing logic in bonding state checkpoints * bonding: Create BondingCheckpoints contract Handles historic checkpointing ("snapshotting") and lookup of the bonding state. * bonding: Checkpoint bonding state on changes * test/bonding: Test BondingManager and Checkpoints - unit tests - integration tests - gas-report - fork test for upgrade * bonding: Migrate to custom error types * bonding: Allow querying unbonded+uncheckpointed accounts This will provide a cleaner experience using governance tools, since users that don't have any stake won't get errors when querying their voting power. For users that do have stake, we will make sure to checkpoint them on first deploy. * treasury: Create treasury governance contracts * test/treasury: Add unit test fro BondingCheckpointsVotes * test/treasury: Test GovernorCountingOverridable * test/treasury: Test LivepeerGovernor * test/treasury: A couple additional Governor tests 100% coverage 😎 * test/treasury: Rename Counting unit test mock "Harness" seems to make more sense, I could only think of that now. * Apply suggestions from code review Co-authored-by: Chase Adams * treasury: Fix storage layout situation * treasury: Move governor initial params to configs * bonding: Implement treasury contribution * test/bonding: Add tests for treasury contribution * bonding: Update reward cut logic to match LIP It is a little less exact (might overmint on the last reward call), but the simpler logic might be just worth it. * bonding: Make sure we checkpoint up to once per op * bonding: Make bonding checkpoints implement IVotes * bonding: Read votes from the end of the round Meaning the start of next round instead of the current round. This is more compatible with the way OZ expects the timepoints to work in the clock and snapshots on the Governor framework. * bonding: Make checkpoint reading revert-free This changes the BondingVotes implementation to stop having so many reverts on supposedly invalid states. Instead, redefine the voting power (and checkpointed stake) as being zero before the first round to be checkpointed. This had other implications in the code like removing changes in BondingManager to make the state complete (always having an earnings pool on lastClaimRound). Instead, the BondingVotes implementaiton is resilient to that as well and just assumes reward() had never been called. This also fixed the redundant reverts between BondingVotes and SortedArrays, as now there are almost no reverts. * bonding: Address minor code review comments * treasury: Migrate to the new BondingVotes contract * treasury: Address PR comments * bonding: Move constructor to after modifiers Just for consistency with other contracts. Some docs as a bonus * bonding: Avoid returning payable treasury address * bonding: Update treasury cut rate only on the next round * test: Fix TicketBroker tests flakiness! * test/mocks: Remove mock functions that moved to other mock * bonding: Implement ERC20 metadata on votes This is to increase compatibility with some tools out there like Tally, which require only these functions from the ERC20 spec, not the full implementation. So we can have these from the get-go to make things easier if we want to make something with them. * bonding: Address PR comments * bonding: Address BondingVotes review comments * treasury: Merge BondingCheckpoints and nits * bonding: Move internal func to the right section Also add docs --------- Co-authored-by: Chase Adams --- contracts/bonding/BondingManager.sol | 81 ++++++- contracts/bonding/IBondingManager.sol | 1 + contracts/test/mocks/MinterMock.sol | 5 + test/unit/BondingManager.js | 324 ++++++++++++++++++++++++++ test/unit/TicketBroker.js | 7 +- test/unit/helpers/Fixture.js | 1 + 6 files changed, 414 insertions(+), 5 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 68571263..93e06ba0 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -94,6 +94,14 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { // in the pool are locked into the active set for round N + 1 SortedDoublyLL.Data private transcoderPool; + // The % of newly minted rewards to be routed to the treasury. Represented as a PreciseMathUtils percPoint value. + uint256 public treasuryRewardCutRate; + // The value for `treasuryRewardCutRate` to be set on the next round initialization. + uint256 public nextRoundTreasuryRewardCutRate; + + // If the balance of the treasury in LPT is above this value, automatic treasury contributions will halt. + uint256 public treasuryBalanceCeiling; + // Check if sender is TicketBroker modifier onlyTicketBroker() { _onlyTicketBroker(); @@ -150,6 +158,27 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { emit ParameterUpdate("unbondingPeriod"); } + /** + * @notice Set treasury reward cut rate. Only callable by Controller owner. Notice that the change will only be + * effective on the next round. + * @param _cutRate Percentage of newly minted rewards to route to the treasury. Must be a valid PreciseMathUtils + * percentage (<100% specified with 27-digits precision). + */ + function setTreasuryRewardCutRate(uint256 _cutRate) external onlyControllerOwner { + _setTreasuryRewardCutRate(_cutRate); + } + + /** + * @notice Set treasury balance ceiling. Only callable by Controller owner + * @param _ceiling Balance at which treasury reward contributions should halt. Specified in LPT fractional units + * (18-digit precision). + */ + function setTreasuryBalanceCeiling(uint256 _ceiling) external onlyControllerOwner { + treasuryBalanceCeiling = _ceiling; + + emit ParameterUpdate("treasuryBalanceCeiling"); + } + /** * @notice Set maximum number of active transcoders. Only callable by Controller owner * @param _numActiveTranscoders Number of active transcoders @@ -321,6 +350,11 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { totalStake, currentRoundTotalActiveStake ); + + // Deduct what would have been the treasury rewards + uint256 treasuryRewards = MathUtils.percOf(rewards, treasuryRewardCutRate); + rewards = rewards.sub(treasuryRewards); + uint256 transcoderCommissionRewards = MathUtils.percOf(rewards, earningsPool.transcoderRewardCut); uint256 delegatorsRewards = rewards.sub(transcoderCommissionRewards); @@ -428,6 +462,12 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { function setCurrentRoundTotalActiveStake() external onlyRoundsManager { currentRoundTotalActiveStake = nextRoundTotalActiveStake; + if (nextRoundTreasuryRewardCutRate != treasuryRewardCutRate) { + treasuryRewardCutRate = nextRoundTreasuryRewardCutRate; + // The treasury cut rate changes in a delayed fashion so we want to emit the parameter update event here + emit ParameterUpdate("treasuryRewardCutRate"); + } + bondingVotes().checkpointTotalActiveStake(currentRoundTotalActiveStake, roundsManager().currentRound()); } @@ -828,16 +868,35 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { earningsPool.setStake(t.earningsPoolPerRound[lastUpdateRound].totalStake); } + if (treasuryBalanceCeiling > 0) { + uint256 treasuryBalance = livepeerToken().balanceOf(treasury()); + if (treasuryBalance >= treasuryBalanceCeiling && nextRoundTreasuryRewardCutRate > 0) { + // halt treasury contributions until the cut rate param is updated again + _setTreasuryRewardCutRate(0); + } + } + // Create reward based on active transcoder's stake relative to the total active stake // rewardTokens = (current mintable tokens for the round * active transcoder stake) / total active stake - uint256 rewardTokens = minter().createReward(earningsPool.totalStake, currentRoundTotalActiveStake); + IMinter mtr = minter(); + uint256 totalRewardTokens = mtr.createReward(earningsPool.totalStake, currentRoundTotalActiveStake); + uint256 treasuryRewards = PreciseMathUtils.percOf(totalRewardTokens, treasuryRewardCutRate); + if (treasuryRewards > 0) { + address trsry = treasury(); + + mtr.trustedTransferTokens(trsry, treasuryRewards); - updateTranscoderWithRewards(msg.sender, rewardTokens, currentRound, _newPosPrev, _newPosNext); + emit TreasuryReward(msg.sender, trsry, treasuryRewards); + } + + uint256 transcoderRewards = totalRewardTokens.sub(treasuryRewards); + + updateTranscoderWithRewards(msg.sender, transcoderRewards, currentRound, _newPosPrev, _newPosNext); // Set last round that transcoder called reward t.lastRewardRound = currentRound; - emit Reward(msg.sender, rewardTokens); + emit Reward(msg.sender, transcoderRewards); } /** @@ -1110,6 +1169,18 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { return delegators[_delegator].unbondingLocks[_unbondingLockId].withdrawRound > 0; } + /** + * @dev Internal version of setTreasuryRewardCutRate. Sets the treasury reward cut rate for the next round and emits + * corresponding event. + */ + function _setTreasuryRewardCutRate(uint256 _cutRate) internal { + require(PreciseMathUtils.validPerc(_cutRate), "_cutRate is invalid precise percentage"); + + nextRoundTreasuryRewardCutRate = _cutRate; + + emit ParameterUpdate("nextRoundTreasuryRewardCutRate"); + } + /** * @notice Return an EarningsPool.Data struct with cumulative factors for a given round that are rescaled if needed * @param _transcoder Storage pointer to a transcoder struct @@ -1569,6 +1640,10 @@ contract BondingManager is ManagerProxyTarget, IBondingManager { return IRoundsManager(controller.getContract(keccak256("RoundsManager"))); } + function treasury() internal view returns (address) { + return controller.getContract(keccak256("Treasury")); + } + function bondingVotes() internal view returns (IBondingVotes) { return IBondingVotes(controller.getContract(keccak256("BondingVotes"))); } diff --git a/contracts/bonding/IBondingManager.sol b/contracts/bonding/IBondingManager.sol index f981d7a7..b7482085 100644 --- a/contracts/bonding/IBondingManager.sol +++ b/contracts/bonding/IBondingManager.sol @@ -11,6 +11,7 @@ interface IBondingManager { event TranscoderDeactivated(address indexed transcoder, uint256 deactivationRound); event TranscoderSlashed(address indexed transcoder, address finder, uint256 penalty, uint256 finderReward); event Reward(address indexed transcoder, uint256 amount); + event TreasuryReward(address indexed transcoder, address treasury, uint256 amount); event Bond( address indexed newDelegate, address indexed oldDelegate, diff --git a/contracts/test/mocks/MinterMock.sol b/contracts/test/mocks/MinterMock.sol index 13c1d5f1..dabd0215 100644 --- a/contracts/test/mocks/MinterMock.sol +++ b/contracts/test/mocks/MinterMock.sol @@ -5,8 +5,13 @@ import "./GenericMock.sol"; contract MinterMock is GenericMock { event TrustedWithdrawETH(address to, uint256 amount); + event TrustedTransferTokens(address to, uint256 amount); function trustedWithdrawETH(address _to, uint256 _amount) external { emit TrustedWithdrawETH(_to, _amount); } + + function trustedTransferTokens(address _to, uint256 _amount) external { + emit TrustedTransferTokens(_to, _amount); + } } diff --git a/test/unit/BondingManager.js b/test/unit/BondingManager.js index a449c74c..326da04c 100644 --- a/test/unit/BondingManager.js +++ b/test/unit/BondingManager.js @@ -138,6 +138,114 @@ describe("BondingManager", () => { }) }) + describe("setTreasuryRewardCutRate", () => { + const FIFTY_PCT = math.precise.percPoints(BigNumber.from(50), 100) + + let currentRound + + beforeEach(async () => { + currentRound = 100 + + await fixture.roundsManager.setMockUint256( + functionSig("currentRound()"), + currentRound + ) + }) + + it("should start as zero", async () => { + assert.equal( + await bondingManager.treasuryRewardCutRate(), + 0, + "initial treasuryRewardCutRate not zero" + ) + }) + + it("should fail if caller is not Controller owner", async () => { + await expect( + bondingManager + .connect(signers[2]) + .setTreasuryRewardCutRate(FIFTY_PCT) + ).to.be.revertedWith("caller must be Controller owner") + }) + + it("should set only nextRoundTreasuryRewardCutRate", async () => { + const tx = await bondingManager.setTreasuryRewardCutRate(FIFTY_PCT) + await expect(tx) + .to.emit(bondingManager, "ParameterUpdate") + .withArgs("nextRoundTreasuryRewardCutRate") + + assert.equal( + await bondingManager.nextRoundTreasuryRewardCutRate(), + FIFTY_PCT.toString(), + "wrong nextRoundTreasuryRewardCutRate" + ) + assert.equal( + await bondingManager.treasuryRewardCutRate(), + 0, + "wrong treasuryRewardCutRate" + ) + }) + + it("should set treasuryRewardCutRate on the next round", async () => { + await bondingManager.setTreasuryRewardCutRate(FIFTY_PCT) + + await fixture.roundsManager.setMockUint256( + functionSig("currentRound()"), + currentRound + 1 + ) + const tx = await fixture.roundsManager.execute( + bondingManager.address, + functionSig("setCurrentRoundTotalActiveStake()") + ) + await expect(tx) + .to.emit(bondingManager, "ParameterUpdate") + .withArgs("treasuryRewardCutRate") + + assert.equal( + await bondingManager.treasuryRewardCutRate(), + FIFTY_PCT.toString(), + "wrong treasuryRewardCutRate" + ) + // sanity check that this hasn't changed either + assert.equal( + await bondingManager.nextRoundTreasuryRewardCutRate(), + FIFTY_PCT.toString(), + "wrong nextRoundTreasuryRewardCutRate" + ) + }) + }) + + describe("setTreasuryBalanceCeiling", () => { + const HUNDRED_LPT = ethers.utils.parseEther("100") + + it("should start as zero", async () => { + assert.equal( + await bondingManager.treasuryBalanceCeiling(), + 0, + "initial treasuryBalanceCeiling not zero" + ) + }) + + it("should fail if caller is not Controller owner", async () => { + await expect( + bondingManager + .connect(signers[2]) + .setTreasuryBalanceCeiling(HUNDRED_LPT) + ).to.be.revertedWith("caller must be Controller owner") + }) + + it("should set treasuryBalanceCeiling", async () => { + await bondingManager.setTreasuryBalanceCeiling(HUNDRED_LPT) + + const newValue = await bondingManager.treasuryBalanceCeiling() + assert.equal( + newValue.toString(), + HUNDRED_LPT.toString(), + "wrong treasuryBalanceCeiling" + ) + }) + }) + describe("transcoder", () => { const currentRound = 100 beforeEach(async () => { @@ -4690,6 +4798,222 @@ describe("BondingManager", () => { .to.emit(bondingManager, "Reward") .withArgs(transcoder.address, 1000) }) + + describe("treasury contribution", () => { + const TREASURY_CUT = math.precise.percPoints( + BigNumber.from(631), + 10000 + ) // 6.31% + + beforeEach(async () => { + await fixture.token.setMockUint256( + functionSig("balanceOf(address)"), + 0 + ) + + await bondingManager.setTreasuryRewardCutRate(TREASURY_CUT) + await bondingManager.setTreasuryBalanceCeiling(1000) + + // treasury cut rate update only takes place on the next round + await fixture.roundsManager.setMockUint256( + functionSig("currentRound()"), + currentRound + 1 + ) + await fixture.roundsManager.execute( + bondingManager.address, + functionSig("setCurrentRoundTotalActiveStake()") + ) + }) + + it("should update caller with rewards after treasury contribution", async () => { + const startDelegatedAmount = ( + await bondingManager.getDelegator(transcoder.address) + )[3] + const startTotalStake = + await bondingManager.transcoderTotalStake( + transcoder.address + ) + const startNextTotalStake = + await bondingManager.nextRoundTotalActiveStake() + await bondingManager.connect(transcoder).reward() + + const endDelegatedAmount = ( + await bondingManager.getDelegator(transcoder.address) + )[3] + const endTotalStake = await bondingManager.transcoderTotalStake( + transcoder.address + ) + const endNextTotalStake = + await bondingManager.nextRoundTotalActiveStake() + + const earningsPool = + await bondingManager.getTranscoderEarningsPoolForRound( + transcoder.address, + currentRound + 1 + ) + + const expRewardFactor = constants.PERC_DIVISOR_PRECISE.add( + math.precise.percPoints( + BigNumber.from(469), // (1000 - 6.31% = 937) - 50% = 469 (cuts are calculated first and subtracted) + BigNumber.from(1000) + ) + ) + assert.equal( + earningsPool.cumulativeRewardFactor.toString(), + expRewardFactor.toString(), + "should update cumulativeRewardFactor in earningsPool" + ) + + assert.equal( + endDelegatedAmount.sub(startDelegatedAmount), + 937, + "should update delegatedAmount with rewards after treasury cut" + ) + assert.equal( + endTotalStake.sub(startTotalStake), + 937, + "should update transcoder's total stake in the pool with rewards after treasury cut" + ) + assert.equal( + endNextTotalStake.sub(startNextTotalStake), + 937, + "should update next total stake with rewards after treasury cut" + ) + }) + + it("should transfer tokens to the treasury", async () => { + const tx = await bondingManager.connect(transcoder).reward() + + await expect(tx) + .to.emit(fixture.minter, "TrustedTransferTokens") + .withArgs(fixture.treasury.address, 63) + }) + + it("should emit TreasuryReward event", async () => { + const tx = await bondingManager.connect(transcoder).reward() + + await expect(tx) + .to.emit(bondingManager, "TreasuryReward") + .withArgs(transcoder.address, fixture.treasury.address, 63) + }) + + describe("ceiling behavior", () => { + describe("under the limit", () => { + beforeEach(async () => { + await fixture.token.setMockUint256( + functionSig("balanceOf(address)"), + 990 + ) + }) + + it("should contribute normally", async () => { + const tx = await bondingManager + .connect(transcoder) + .reward() + + await expect(tx) + .to.emit(fixture.minter, "TrustedTransferTokens") + .withArgs(fixture.treasury.address, 63) + }) + + it("should not clear treasuryRewardCutRate param", async () => { + await bondingManager.connect(transcoder).reward() + + const cutRate = + await bondingManager.treasuryRewardCutRate() + assert.equal( + cutRate.toString(), + TREASURY_CUT.toString(), + "cut rate updated" + ) + }) + }) + + const atCeilingTest = (title, balance) => { + describe(title, () => { + beforeEach(async () => { + await fixture.token.setMockUint256( + functionSig("balanceOf(address)"), + balance + ) + }) + + it("should zero the nextRoundTreasuryRewardCutRate", async () => { + const tx = await bondingManager + .connect(transcoder) + .reward() + + // it should still send treasury rewards + await expect(tx).to.emit( + fixture.minter, + "TrustedTransferTokens" + ) + await expect(tx).to.emit( + bondingManager, + "TreasuryReward" + ) + + await expect(tx) + .to.emit(bondingManager, "ParameterUpdate") + .withArgs("nextRoundTreasuryRewardCutRate") + assert.equal( + await bondingManager.nextRoundTreasuryRewardCutRate(), + 0 + ) + }) + + it("should not mint any treasury rewards in the next round", async () => { + await bondingManager.connect(transcoder).reward() + + await fixture.roundsManager.setMockUint256( + functionSig("currentRound()"), + currentRound + 2 + ) + await fixture.roundsManager.execute( + bondingManager.address, + functionSig("setCurrentRoundTotalActiveStake()") + ) + + const tx = await bondingManager + .connect(transcoder) + .reward() + await expect(tx).not.to.emit( + fixture.minter, + "TrustedTransferTokens" + ) + await expect(tx).not.to.emit( + bondingManager, + "TreasuryReward" + ) + }) + + it("should also clear treasuryRewardCutRate param in the next round", async () => { + await bondingManager.connect(transcoder).reward() + + await fixture.roundsManager.setMockUint256( + functionSig("currentRound()"), + currentRound + 2 + ) + await fixture.roundsManager.execute( + bondingManager.address, + functionSig("setCurrentRoundTotalActiveStake()") + ) + + const cutRate = + await bondingManager.treasuryRewardCutRate() + assert.equal( + cutRate.toNumber(), + 0, + "cut rate not cleared" + ) + }) + }) + } + + atCeilingTest("when at limit", 1000) + atCeilingTest("when above limit", 1500) + }) + }) }) describe("updateTranscoderWithFees", () => { diff --git a/test/unit/TicketBroker.js b/test/unit/TicketBroker.js index 0ec71c98..3d122bcc 100644 --- a/test/unit/TicketBroker.js +++ b/test/unit/TicketBroker.js @@ -664,8 +664,11 @@ describe("TicketBroker", () => { expect(endSenderInfo.sender.deposit).to.be.equal(deposit) expect(endSenderInfo.reserve.fundsRemaining).to.be.equal(reserve) - expect(tx).to.changeEtherBalance(funder, -(deposit + reserve)) - expect(tx).to.changeEtherBalance(fixture.minter, deposit + reserve) + await expect(tx).to.changeEtherBalance(funder, -(deposit + reserve)) + await expect(tx).to.changeEtherBalance( + fixture.minter, + deposit + reserve + ) }) describe("msg.sender = _addr", () => { diff --git a/test/unit/helpers/Fixture.js b/test/unit/helpers/Fixture.js index c21e3fb0..f58d0f64 100644 --- a/test/unit/helpers/Fixture.js +++ b/test/unit/helpers/Fixture.js @@ -29,6 +29,7 @@ export default class Fixture { this.token = await this.deployAndRegister(GenericMock, "LivepeerToken") this.minter = await this.deployAndRegister(MinterMock, "Minter") + this.treasury = await this.deployAndRegister(GenericMock, "Treasury") this.bondingManager = await this.deployAndRegister( BondingManagerMock, "BondingManager"