Skip to content

Commit

Permalink
bonding: Make sure we checkpoint up to once per op
Browse files Browse the repository at this point in the history
  • Loading branch information
@victorges
victorges committed Aug 12, 2023
1 parent 5571808 commit 8b78ba7
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 135 deletions.
74 changes: 41 additions & 33 deletions contracts/bonding/BondingManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
_;
}

modifier autoCheckpoint(address account) {
_;
checkpointBondingState(account);
}

/**
* @notice BondingManager constructor. Only invokes constructor of base Manager contract with provided Controller address
* @dev This constructor will not initialize any state variables besides `controller`. The following setter functions
Expand Down Expand Up @@ -382,9 +387,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
address _finder,
uint256 _slashAmount,
uint256 _finderFee
) external whenSystemNotPaused onlyVerifier {
_autoClaimEarnings(_transcoder);

) external whenSystemNotPaused onlyVerifier autoClaimEarnings(_transcoder) autoCheckpoint(_transcoder) {
Delegator storage del = delegators[_transcoder];

if (del.bondedAmount > 0) {
Expand All @@ -405,8 +408,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
);
}

checkpointBondingState(_transcoder, del, transcoders[_transcoder]);

// Account for penalty
uint256 burnAmount = penalty;

Expand Down Expand Up @@ -434,7 +435,12 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
* @notice Claim token pools shares for a delegator from its lastClaimRound through the end round
* @param _endRound The last round for which to claim token pools shares for a delegator
*/
function claimEarnings(uint256 _endRound) external whenSystemNotPaused currentRoundInitialized {
function claimEarnings(uint256 _endRound)
external
whenSystemNotPaused
currentRoundInitialized
autoCheckpoint(msg.sender)
{
// Silence unused param compiler warning
_endRound;

Expand Down Expand Up @@ -582,8 +588,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
// Update bonded amount
del.bondedAmount = currentBondedAmount.add(_amount);

checkpointBondingState(_owner, del, transcoders[_owner]);

increaseTotalStake(_to, delegationAmount, _currDelegateNewPosPrev, _currDelegateNewPosNext);

if (_amount > 0) {
Expand All @@ -592,6 +596,9 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
}

emit Bond(_to, currentDelegate, _owner, _amount, del.bondedAmount);

// the `autoCheckpoint` modifier has been replaced with its internal function as a `Stack too deep` error work-around
checkpointBondingState(_owner, del, transcoders[_to]);
}

/**
Expand Down Expand Up @@ -624,7 +631,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
* 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 checkpointBondingState(address _account) external {
function checkpointBondingState(address _account) public {
checkpointBondingState(_account, delegators[_account], transcoders[_account]);
}

Expand Down Expand Up @@ -749,7 +756,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
uint256 _amount,
address _newPosPrev,
address _newPosNext
) public whenSystemNotPaused currentRoundInitialized autoClaimEarnings(msg.sender) {
) public whenSystemNotPaused currentRoundInitialized autoClaimEarnings(msg.sender) autoCheckpoint(msg.sender) {
require(delegatorStatus(msg.sender) == DelegatorStatus.Bonded, "caller must be bonded");

Delegator storage del = delegators[msg.sender];
Expand Down Expand Up @@ -780,9 +787,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
}
}

// No problem that startRound may have been cleared above, checkpoints are always made for currentRound()+1
checkpointBondingState(msg.sender, del, transcoders[msg.sender]);

// If msg.sender was resigned this statement will only decrease delegators[currentDelegate].delegatedAmount
decreaseTotalStake(currentDelegate, _amount, _newPosPrev, _newPosNext);

Expand Down Expand Up @@ -849,6 +853,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
public
whenSystemNotPaused
currentRoundInitialized
autoCheckpoint(msg.sender)
{
uint256 currentRound = roundsManager().currentRound();

Expand Down Expand Up @@ -901,8 +906,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
// Set last round that transcoder called reward
t.lastRewardRound = currentRound;

checkpointBondingState(msg.sender, delegators[msg.sender], t);

emit Reward(msg.sender, transcoderRewards);
}

Expand Down Expand Up @@ -1291,6 +1294,19 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
uint256 _amount,
address _newPosPrev,
address _newPosNext
) internal autoCheckpoint(_delegate) {
return increaseTotalStakeUncheckpointed(_delegate, _amount, _newPosPrev, _newPosNext);
}

/**
* Implementation of increaseTotalStake that does not checkpoint the caller, to be used by functions that handle the
* checkpointing themselves.
*/
function increaseTotalStakeUncheckpointed(
address _delegate,
uint256 _amount,
address _newPosPrev,
address _newPosNext
) internal {
Transcoder storage t = transcoders[_delegate];

Expand Down Expand Up @@ -1322,12 +1338,8 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
}
}

Delegator storage del = delegators[_delegate];

// Increase delegate's delegated amount
del.delegatedAmount = newStake;

checkpointBondingState(_delegate, del, t);
delegators[_delegate].delegatedAmount = newStake;
}

/**
Expand All @@ -1340,7 +1352,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
uint256 _amount,
address _newPosPrev,
address _newPosNext
) internal {
) internal autoCheckpoint(_delegate) {
Transcoder storage t = transcoders[_delegate];

uint256 currStake = transcoderTotalStake(_delegate);
Expand All @@ -1365,12 +1377,8 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
t.earningsPoolPerRound[nextRound].setStake(newStake);
}

Delegator storage del = delegators[_delegate];

// Decrease old delegate's delegated amount
del.delegatedAmount = newStake;

checkpointBondingState(_delegate, del, t);
delegators[_delegate].delegatedAmount = newStake;
}

/**
Expand Down Expand Up @@ -1438,7 +1446,8 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {

/**
* @dev Update a transcoder with rewards and update the transcoder pool with an optional list hint if needed.
* See SortedDoublyLL.sol for details on list hints
* See SortedDoublyLL.sol for details on list hints. This function updates the transcoder state but does not
* checkpoint it as it assumes the caller will ensure that.
* @param _transcoder Address of transcoder
* @param _rewards Amount of rewards
* @param _round Round that transcoder is updated
Expand Down Expand Up @@ -1474,11 +1483,14 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
// the earnings claiming algorithm and instead that amount is accounted for in the transcoder's cumulativeRewards field
earningsPool.updateCumulativeRewardFactor(prevEarningsPool, delegatorsRewards);
// Update transcoder's total stake with rewards
increaseTotalStake(_transcoder, _rewards, _newPosPrev, _newPosNext);
increaseTotalStakeUncheckpointed(_transcoder, _rewards, _newPosPrev, _newPosNext);
}

/**
* @dev Update a delegator with token pools shares from its lastClaimRound through a given round
*
* Notice that this function udpates the delegator storage but does not checkpoint its state. Since it is internal
* it assumes the top-level caller will checkpoint it instead.
* @param _delegator Delegator address
* @param _endRound The last round for which to update a delegator's stake with earnings pool shares
* @param _lastClaimRound The round for which a delegator has last claimed earnings
Expand Down Expand Up @@ -1537,8 +1549,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
// Rewards are bonded by default
del.bondedAmount = currentBondedAmount;
del.fees = currentFees;

checkpointBondingState(_delegator, del, transcoders[_delegator]);
}

/**
Expand All @@ -1554,7 +1564,7 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
uint256 _unbondingLockId,
address _newPosPrev,
address _newPosNext
) internal {
) internal autoCheckpoint(_delegator) {
Delegator storage del = delegators[_delegator];
UnbondingLock storage lock = del.unbondingLocks[_unbondingLockId];

Expand All @@ -1564,8 +1574,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
// Increase delegator's bonded amount
del.bondedAmount = del.bondedAmount.add(amount);

checkpointBondingState(_delegator, del, transcoders[_delegator]);

// Delete lock
delete del.unbondingLocks[_unbondingLockId];

Expand Down
Loading

0 comments on commit 8b78ba7

Please sign in to comment.