Skip to content

Commit

Permalink
bonding: Spike at snapshotting isActiveTranscoder
Browse files Browse the repository at this point in the history 
It might not work, got a key TODO in BondingManager.
  • Loading branch information
@victorges
victorges committed Aug 25, 2023
1 parent 5c4a369 commit 76a06d7
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 52 deletions.
13 changes: 12 additions & 1 deletion contracts/bonding/BondingManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1597,14 +1597,25 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
// in the delegators doesn't get updated on bond or claim earnings though, so we use currentRound() + 1
// which is the only guaranteed round where the currently stored stake will be active.
uint256 startRound = roundsManager().currentRound() + 1;

// This is the same logic as isActiveTranscoder but using startRound instead of currentRound.
assert(
// TODO: Crazy stuff going on here, consider just checkpointing both of activation & deactivation rounds
_transcoder.activationRound <= startRound &&
(_transcoder.deactivationRound <= startRound || _transcoder.deactivationRound == MAX_FUTURE_ROUND)
);
bool isActiveFromStartRound = _transcoder.activationRound <= startRound &&
startRound < _transcoder.deactivationRound;

bondingVotes().checkpointBondingState(
_owner,
startRound,
_delegator.bondedAmount,
_delegator.delegateAddress,
_delegator.delegatedAmount,
_delegator.lastClaimRound,
_transcoder.lastRewardRound
_transcoder.lastRewardRound,
isActiveFromStartRound
);
}

Expand Down
73 changes: 40 additions & 33 deletions contracts/bonding/BondingVotes.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,15 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
* @dev The last round during which the checkpointed account called {BondingManager-reward}. This is needed to
* when calculating pending rewards for a delegator to this transcoder, to find the last earning pool available
* for a given round. In that case we start from the delegator checkpoint and then fetch its delegate address
* checkpoint as well to find the last earning pool.
*
* Notice that this is the only field that comes from the Transcoder struct in BondingManager, not Delegator.
* checkpoint as well to find the last earning pool. This field comes from the Transcoder struct in
* BondingManager, not Delegator.
*/
uint256 lastRewardRound;
/**
* @dev Whether the account was registered as an active transcoder when this checkpoint was made. This field is
* computed from the Transcoder struct in BondingManager throught the `isActiveTranscoder` function.
*/
bool isActiveTranscoder;
}

/**
Expand Down Expand Up @@ -262,7 +266,8 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
address _delegateAddress,
uint256 _delegatedAmount,
uint256 _lastClaimRound,
uint256 _lastRewardRound
uint256 _lastRewardRound,
bool _isActiveTranscoder
) external virtual onlyBondingManager {
if (_startRound != clock() + 1) {
revert InvalidStartRound(_startRound, clock() + 1);
Expand All @@ -282,7 +287,8 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
delegateAddress: _delegateAddress,
delegatedAmount: _delegatedAmount,
lastClaimRound: _lastClaimRound,
lastRewardRound: _lastRewardRound
lastRewardRound: _lastRewardRound,
isActiveTranscoder: _isActiveTranscoder
});
checkpoints.data[_startRound] = bond;

Expand Down Expand Up @@ -373,8 +379,8 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
amount = 0;
} else if (isTranscoder) {
// Address is a registered transcoder so we use its delegated amount. This includes self and delegated stake
// as well as any accrued rewards, even unclaimed ones
amount = bond.delegatedAmount;
// as well as any accrued rewards, even unclaimed ones. Notice that only active transcoder get voting power.
amount = bond.isActiveTranscoder ? bond.delegatedAmount : 0;
} else {
// Address is NOT a registered transcoder so we calculate its cumulative stake for the voting power
amount = delegatorCumulativeStakeAt(bond, _round);
Expand All @@ -386,28 +392,26 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
*/
function onBondingCheckpointChanged(
address _account,
BondingCheckpoint memory previous,
BondingCheckpoint memory current
BondingCheckpoint memory _previous,
BondingCheckpoint memory _current
) internal {
address previousDelegate = previous.delegateAddress;
address newDelegate = current.delegateAddress;
address previousDelegate = _previous.delegateAddress;
address newDelegate = _current.delegateAddress;
if (previousDelegate != newDelegate) {
emit DelegateChanged(_account, previousDelegate, newDelegate);
}

bool isTranscoder = newDelegate == _account;
bool wasTranscoder = previousDelegate == _account;
// we want to register zero "delegate votes" when the account is/was not a transcoder
uint256 previousDelegateVotes = wasTranscoder ? previous.delegatedAmount : 0;
uint256 currentDelegateVotes = isTranscoder ? current.delegatedAmount : 0;
// we want to register zero "delegate votes" when the account is/was not an active transcoder
uint256 previousDelegateVotes = _previous.isActiveTranscoder ? _previous.delegatedAmount : 0;
uint256 currentDelegateVotes = _current.isActiveTranscoder ? _current.delegatedAmount : 0;
if (previousDelegateVotes != currentDelegateVotes) {
emit DelegateVotesChanged(_account, previousDelegateVotes, currentDelegateVotes);
}

// Always send delegator events since transcoders are delegators themselves. The way our rewards work, the
// delegator voting power calculated from events will only reflect their claimed stake without pending rewards.
if (previous.bondedAmount != current.bondedAmount) {
emit DelegatorBondedAmountChanged(_account, previous.bondedAmount, current.bondedAmount);
if (_previous.bondedAmount != _current.bondedAmount) {
emit DelegatorBondedAmountChanged(_account, _previous.bondedAmount, _current.bondedAmount);
}
}

Expand Down Expand Up @@ -461,19 +465,22 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
view
returns (uint256)
{
EarningsPool.Data memory startPool = getTranscoderEarningsPoolForRound(
bond.delegateAddress,
bond.lastClaimRound
);
address transcoder = bond.delegateAddress;
EarningsPool.Data memory startPool = getTranscoderEarningsPoolForRound(transcoder, bond.lastClaimRound);

(uint256 rewardRound, EarningsPool.Data memory endPool) = getLastTranscoderRewardsEarningsPool(
bond.delegateAddress,
_round
);
(
BondingCheckpoint storage transcoderBond,
EarningsPool.Data memory endPool
) = getLastTranscoderRewardsEarningsPool(transcoder, _round);

if (!transcoderBond.isActiveTranscoder) {
// Delegating to an inactive transcoder should not provide any voting power.
return 0;
}

if (rewardRound < bond.lastClaimRound) {
// If the transcoder hasn't called reward() since the last time the delegator claimed earnings, there wil be
// no rewards to add to the delegator's stake so we just return the originally bonded amount.
if (transcoderBond.lastRewardRound < bond.lastClaimRound) {
// If the transcoder hasn't called reward() since the last time the delegator claimed earnings, there will
// be no rewards to add to the delegator's stake so we just return the originally bonded amount.
return bond.bondedAmount;
}

Expand All @@ -493,16 +500,16 @@ contract BondingVotes is ManagerProxyTarget, IBondingVotes {
* returns a zero earning pool is if the transcoder had never called reward() before _round.
* @param _transcoder Address of the transcoder to look for
* @param _round Past round at which we want the valid earning pool from
* @return rewardRound Round in which the returned earning pool was calculated.
* @return bond The BondingCheckpoint from the transcoder at the given _round.
* @return pool EarningsPool.Data struct with the last initialized earning pool.
*/
function getLastTranscoderRewardsEarningsPool(address _transcoder, uint256 _round)
internal
view
returns (uint256 rewardRound, EarningsPool.Data memory pool)
returns (BondingCheckpoint storage bond, EarningsPool.Data memory pool)
{
BondingCheckpoint storage bond = getBondingCheckpointAt(_transcoder, _round);
rewardRound = bond.lastRewardRound;
bond = getBondingCheckpointAt(_transcoder, _round);
uint256 rewardRound = bond.lastRewardRound;

if (rewardRound > 0) {
pool = getTranscoderEarningsPoolForRound(_transcoder, rewardRound);
Expand Down
3 changes: 2 additions & 1 deletion contracts/bonding/IBondingVotes.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ interface IBondingVotes is IVotes {
address _delegateAddress,
uint256 _delegatedAmount,
uint256 _lastClaimRound,
uint256 _lastRewardRound
uint256 _lastRewardRound,
bool _isActiveTranscoder
) external;

function checkpointTotalActiveStake(uint256 _totalStake, uint256 _round) external;
Expand Down
9 changes: 6 additions & 3 deletions contracts/test/mocks/BondingVotesMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ contract BondingVotesMock is GenericMock {
address delegateAddress,
uint256 delegatedAmount,
uint256 lastClaimRound,
uint256 lastRewardRound
uint256 lastRewardRound,
bool isActiveTranscoder
);
event CheckpointTotalActiveStake(uint256 totalStake, uint256 round);

Expand All @@ -22,7 +23,8 @@ contract BondingVotesMock is GenericMock {
address _delegateAddress,
uint256 _delegatedAmount,
uint256 _lastClaimRound,
uint256 _lastRewardRound
uint256 _lastRewardRound,
bool _isActiveTranscoder
) external {
emit CheckpointBondingState(
_account,
Expand All @@ -31,7 +33,8 @@ contract BondingVotesMock is GenericMock {
_delegateAddress,
_delegatedAmount,
_lastClaimRound,
_lastRewardRound
_lastRewardRound,
_isActiveTranscoder
);
}

Expand Down
41 changes: 27 additions & 14 deletions test/unit/BondingVotes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ describe("BondingVotes", () => {
delegateAddress,
delegatedAmount,
lastClaimRound,
lastRewardRound
lastRewardRound,
isActiveTranscoder
}) => {
return bondingVotes.interface.encodeFunctionData(
"checkpointBondingState",
Expand All @@ -87,7 +88,8 @@ describe("BondingVotes", () => {
delegateAddress,
delegatedAmount,
lastClaimRound,
lastRewardRound
lastRewardRound,
isActiveTranscoder
]
)
}
Expand Down Expand Up @@ -366,7 +368,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: 1000,
lastClaimRound: currentRound + 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})

await expect(
Expand All @@ -390,7 +393,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: 1000,
lastClaimRound: currentRound - 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})

await expect(
Expand All @@ -414,7 +418,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: 1000,
lastClaimRound: currentRound + 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})

await expect(
Expand All @@ -438,7 +443,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: 1000,
lastClaimRound: currentRound - 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand All @@ -454,7 +460,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: 1000,
lastClaimRound: currentRound - 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand All @@ -478,7 +485,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: amount,
lastClaimRound: currentRound,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand Down Expand Up @@ -525,7 +533,8 @@ describe("BondingVotes", () => {
delegateAddress,
delegatedAmount,
lastClaimRound: currentRound,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: delegateAddress === account
})
return await fixture.bondingManager.execute(
bondingVotes.address,
Expand Down Expand Up @@ -690,7 +699,8 @@ describe("BondingVotes", () => {
delegateAddress: transcoder.address,
delegatedAmount: 1000,
lastClaimRound: startRound - 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand Down Expand Up @@ -795,11 +805,12 @@ describe("BondingVotes", () => {
const functionData = encodeCheckpointBondingState({
account: transcoder.address,
startRound,
bondedAmount: 1, // doesn't matter, shouldn't be used
bondedAmount: 0, // not used in these tests
delegateAddress: transcoder.address,
delegatedAmount,
lastClaimRound: startRound - 1,
lastRewardRound: 0
lastRewardRound: 0,
isActiveTranscoder: true
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand Down Expand Up @@ -865,7 +876,8 @@ describe("BondingVotes", () => {
delegateAddress: account,
delegatedAmount: 0, // not used in these tests
lastClaimRound: startRound - 1,
lastRewardRound
lastRewardRound,
isActiveTranscoder: true
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand Down Expand Up @@ -903,7 +915,8 @@ describe("BondingVotes", () => {
delegateAddress,
delegatedAmount: 0, // not used for delegators
lastClaimRound,
lastRewardRound: 0 // not used for delegators
lastRewardRound: 0, // not used for delegators
isActiveTranscoder: false
})
await fixture.bondingManager.execute(
bondingVotes.address,
Expand Down

0 comments on commit 76a06d7

Please sign in to comment.