From 5f00eabf8db7f5c2f76c4a1b78617ba1c546f8b1 Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Thu, 29 Jun 2023 18:20:32 -0300 Subject: [PATCH 1/2] bonding: Allow explicit checkpointing anytime --- contracts/bonding/BondingManager.sol | 15 ++++----------- test/unit/BondingCheckpoints.js | 18 +++++++++--------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol index 00d1f689..ed0aeacb 100644 --- a/contracts/bonding/BondingManager.sol +++ b/contracts/bonding/BondingManager.sol @@ -591,19 +591,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/unit/BondingCheckpoints.js b/test/unit/BondingCheckpoints.js index 4a5a208b..74da547a 100644 --- a/test/unit/BondingCheckpoints.js +++ b/test/unit/BondingCheckpoints.js @@ -158,15 +158,15 @@ describe("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") }) @@ -175,7 +175,7 @@ describe("BondingCheckpoints", () => { "findLowerBound: empty array" ) - await bondingManager.initBondingCheckpoint(transcoder.address) + await bondingManager.checkpointBonding(transcoder.address) // Round R+1 await setRound(currentRound + 1) @@ -189,7 +189,7 @@ describe("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() @@ -258,7 +258,7 @@ describe("BondingCheckpoints", () => { await setRound(0) for (const account of [transcoder, delegator]) { - await bondingManager.initBondingCheckpoint(account.address) + await bondingManager.checkpointBonding(account.address) } // Round R-2 @@ -397,7 +397,7 @@ describe("BondingCheckpoints", () => { ) for (const account of [...transcoders, ...delegators]) { - await bondingManager.initBondingCheckpoint(account.address) + await bondingManager.checkpointBonding(account.address) } // Round R-2 @@ -615,7 +615,7 @@ describe("BondingCheckpoints", () => { await setRound(0) for (const account of [transcoder, delegator, delegatorEarly]) { - await bondingManager.initBondingCheckpoint(account.address) + await bondingManager.checkpointBonding(account.address) } // Round R-200 @@ -800,7 +800,7 @@ describe("BondingCheckpoints", () => { await setRound(0) for (const account of [transcoder, delegator]) { - await bondingManager.initBondingCheckpoint(account.address) + await bondingManager.checkpointBonding(account.address) } // Round R-1 From 1b1d31837a4443fd18081db2f1eeca02d1c3e1c8 Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Thu, 29 Jun 2023 18:25:54 -0300 Subject: [PATCH 2/2] [WIP] bonding: Avoid returning inconsistent total stake --- contracts/bonding/BondingCheckpoints.sol | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/contracts/bonding/BondingCheckpoints.sol b/contracts/bonding/BondingCheckpoints.sol index 9c94e77d..96864296 100644 --- 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; } /**