Merge branch 'vg/feat/bonding-checkpoints' into vg/spike/treasury

livepeer · Jun 29, 2023 · 237b790 · 237b790
2 parents 9eb97c9 + 1b1d318
commit 237b790
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 29 deletions.
diff --git a/contracts/bonding/BondingCheckpoints.sol b/contracts/bonding/BondingCheckpoints.sol
@@ -169,17 +169,16 @@ contract BondingCheckpoints is ManagerProxyTarget, IBondingCheckpoints {
     function getTotalActiveStakeAt(uint256 _round) public view virtual returns (uint256) {
         require(_round <= clock(), "getTotalActiveStakeAt: future lookup");
 
-        // most of the time we will have the checkpoint from exactly the round we want
         uint256 activeStake = totalActiveStakeCheckpoints[_round];
-        if (activeStake > 0) {
-            return activeStake;
+
+        if (activeStake == 0) {
+            uint256 lastInitialized = totalStakeCheckpointRounds.findLowerBound(_round);
+
+            // Check that the round was in fact initialized so we don't return a 0 value accidentally.
+            require(lastInitialized == _round, "getTotalActiveStakeAt: round was not initialized");
         }
 
-        // TODO: We may want to remove this possibility for the total stake and instead require every round to have a
-        // checkpoint. If it doesn't, it means it wasn't initialized and we could have weird issues if we treat it
-        // normally here (e.g. inconsistent total supply depending on if you call before or after initializeRound).
-        uint256 round = totalStakeCheckpointRounds.findLowerBound(_round);
-        return totalActiveStakeCheckpoints[round];
+        return activeStake;
     }
 
     /**

diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol
@@ -612,19 +612,12 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
     }
 
     /**
-     * @notice Initializes the bonding checkpoint for a given account. This can only be called for accounts that have no
-     * checkpoints yet.
-     * @dev This is a migration strategy so we can snapshot all accounts' stake immediately after deploying the new
-     * BondingCheckpoints contract.
+     * @notice Checkpoints the bonding state for a given account.
+     * @dev This is to allow checkpointing an account that has an inconsistent checkpoint with its current state.
+     * Implemented as a deploy utility to checkpoint the existing state when deploying the BondingCheckpoints contract.
      * @param _account The account to initialize the bonding checkpoint for
      */
-    function initBondingCheckpoint(address _account) external {
-        IBondingCheckpoints checkpoints = bondingCheckpoints();
-
-        require(address(checkpoints) != address(0), "bonding checkpoints not available");
-
-        require(!checkpoints.hasCheckpoint(_account), "account already checkpointed");
-
+    function checkpointBonding(address _account) external {
         checkpointBonding(_account, delegators[_account], transcoders[_account]);
     }
 

diff --git a/test/integration/BondingCheckpoints.js b/test/integration/BondingCheckpoints.js
@@ -93,7 +93,7 @@ describe.only("BondingCheckpoints", () => {
             await nextRound()
 
             for (const account of [transcoder, delegator]) {
-                await bondingManager.initBondingCheckpoint(account.address)
+                await bondingManager.checkpointBonding(account.address)
             }
 
             // Round R-2
@@ -245,7 +245,7 @@ describe.only("BondingCheckpoints", () => {
             await nextRound()
 
             for (const account of [...transcoders, ...delegators]) {
-                await bondingManager.initBondingCheckpoint(account.address)
+                await bondingManager.checkpointBonding(account.address)
             }
 
             // Round R-2
@@ -406,7 +406,7 @@ describe.only("BondingCheckpoints", () => {
             await nextRound()
 
             for (const account of [transcoder, delegator, delegatorEarly]) {
-                await bondingManager.initBondingCheckpoint(account.address)
+                await bondingManager.checkpointBonding(account.address)
             }
 
             // Round R-202
@@ -558,7 +558,7 @@ describe.only("BondingCheckpoints", () => {
             await nextRound()
 
             for (const account of [transcoder, delegator]) {
-                await bondingManager.initBondingCheckpoint(account.address)
+                await bondingManager.checkpointBonding(account.address)
             }
 
             // Round R-1

diff --git a/test/unit/BondingCheckpoints.js b/test/unit/BondingCheckpoints.js
@@ -45,7 +45,7 @@ describe.only("BondingCheckpoints", () => {
     })
 
     // TODO: Move this to BondingManager tests
-    describe("initBondingCheckpoint", () => {
+    describe("checkpointBonding", () => {
         let bondingManager
 
         let transcoder
@@ -142,15 +142,15 @@ describe.only("BondingCheckpoints", () => {
             await fixture.register("BondingCheckpoints", constants.AddressZero)
 
             await expect(
-                bondingManager.initBondingCheckpoint(transcoder.address)
+                bondingManager.checkpointBonding(transcoder.address)
             ).to.be.revertedWith("bonding checkpoints not available")
         })
 
         it("should revert if account already initialized", async () => {
-            await bondingManager.initBondingCheckpoint(transcoder.address)
+            await bondingManager.checkpointBonding(transcoder.address)
 
             await expect(
-                bondingManager.initBondingCheckpoint(transcoder.address)
+                bondingManager.checkpointBonding(transcoder.address)
             ).to.be.revertedWith("account already checkpointed")
         })
 
@@ -159,7 +159,7 @@ describe.only("BondingCheckpoints", () => {
                 "findLowerBound: empty array"
             )
 
-            await bondingManager.initBondingCheckpoint(transcoder.address)
+            await bondingManager.checkpointBonding(transcoder.address)
 
             // Round R+1
             await setRound(currentRound + 1)
@@ -173,7 +173,7 @@ describe.only("BondingCheckpoints", () => {
         })
 
         it("should have no problems if state gets updated again in round", async () => {
-            await bondingManager.initBondingCheckpoint(transcoder.address)
+            await bondingManager.checkpointBonding(transcoder.address)
 
             await bondingManager.connect(transcoder).reward()