Skip to content

Commit

Permalink
Convert reward accounting to be tracked on a per-deposit basis
Browse files Browse the repository at this point in the history 
UniStaker tracks rewards earned by individual addresses which are specified as
the beneficiary of deposits. The same address may earn rewards from tokens
which have been staked in multiple deposits.

This commit converts the system to track rewards on a per-deposit basis instead.
Each deposit earns rewards according to the tokens staked in it. The test suite has
been updated to reflect this change.

The meaning of beneficiary changed as well. It is no longer the address that earns
the rewards, but instead is the address that has permissions to withdrawal the
rewards earned by a given deposit.
  • Loading branch information
@apbendi
apbendi committed Sep 25, 2024
1 parent edc13c5 commit 1b75abc
Show file tree
Hide file tree
Showing 4 changed files with 281 additions and 814 deletions.
103 changes: 55 additions & 48 deletions src/GovernanceStaker.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
);

/// @notice Emitted when a beneficiary claims their earned reward.
event RewardClaimed(address indexed beneficiary, uint256 amount);
event RewardClaimed(
DepositIdentifier indexed depositId, address indexed beneficiary, uint256 amount
);

/// @notice Emitted when this contract is notified of a new reward.
event RewardNotified(uint256 amount, address notifier);
Expand Down Expand Up @@ -99,6 +101,9 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
address owner;
address delegatee;
address beneficiary;
uint256 earningPower;
uint256 rewardPerTokenCheckpoint;
uint256 scaledUnclaimedRewardCheckpoint;
}

/// @notice Type hash used when encoding data for `stakeOnBehalf` calls.
Expand All @@ -123,7 +128,7 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
);
/// @notice Type hash used when encoding data for `claimRewardOnBehalf` calls.
bytes32 public constant CLAIM_REWARD_TYPEHASH =
keccak256("ClaimReward(address beneficiary,uint256 nonce,uint256 deadline)");
keccak256("ClaimReward(uint256 depositId,uint256 nonce,uint256 deadline)");

/// @notice ERC20 token in which rewards are denominated and distributed.
IERC20 public immutable REWARD_TOKEN;
Expand Down Expand Up @@ -151,7 +156,7 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
mapping(address depositor => uint256 amount) public depositorTotalStaked;

/// @notice Tracks the total stake actively earning rewards for a given beneficiary account.
mapping(address beneficiary => uint256 amount) public earningPower;
//mapping(address beneficiary => uint256 amount) public earningPower;

/// @notice Stores the metadata associated with a given deposit.
mapping(DepositIdentifier depositId => Deposit deposit) public deposits;
Expand All @@ -177,14 +182,14 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
/// the value of the global accumulator at the last time a given beneficiary's rewards were
/// calculated and stored. The difference between the global value and this value can be
/// used to calculate the interim rewards earned by given account.
mapping(address account => uint256) public beneficiaryRewardPerTokenCheckpoint;
// mapping(address account => uint256) public beneficiaryRewardPerTokenCheckpoint;

/// @notice Checkpoint of the unclaimed rewards earned by a given beneficiary with the scale
/// factor included. This value is stored any time an action is taken that specifically impacts
/// the rate at which rewards are earned by a given beneficiary account. Total unclaimed rewards
/// for an account are thus this value plus all rewards earned after this checkpoint was taken.
/// This value is reset to zero when a beneficiary account claims their earned rewards.
mapping(address account => uint256 amount) public scaledUnclaimedRewardCheckpoint;
//mapping(address account => uint256 amount) public scaledUnclaimedRewardCheckpoint;

/// @notice Maps addresses to whether they are authorized to call `notifyRewardAmount`.
mapping(address rewardNotifier => bool) public isRewardNotifier;
Expand Down Expand Up @@ -248,8 +253,8 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
/// cause rewards to be checkpointed. This external helper method is useful for integrations, and
/// returns the value after it has been scaled down to the reward token's raw decimal amount.
/// @return Live value of the unclaimed rewards earned by a given beneficiary account.
function unclaimedReward(address _beneficiary) external view returns (uint256) {
return _scaledUnclaimedReward(_beneficiary) / SCALE_FACTOR;
function unclaimedReward(DepositIdentifier _depositId) external view returns (uint256) {
return _scaledUnclaimedReward(deposits[_depositId]) / SCALE_FACTOR;
}

/// @notice Stake tokens to a new deposit. The caller must pre-approve the staking contract to
Expand Down Expand Up @@ -565,31 +570,37 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
/// @notice Claim reward tokens the message sender has earned as a stake beneficiary. Tokens are
/// sent to the message sender.
/// @return Amount of reward tokens claimed.
function claimReward() external returns (uint256) {
return _claimReward(msg.sender);
function claimReward(DepositIdentifier _depositId) external returns (uint256) {
Deposit storage deposit = deposits[_depositId];
if (deposit.beneficiary != msg.sender) {
revert GovernanceStaker__Unauthorized("not beneficiary", msg.sender);
}
return _claimReward(_depositId, deposit);
}

/// @notice Claim earned reward tokens for a beneficiary, using a signature to validate the
/// beneficiary's intent. Tokens are sent to the beneficiary.
/// @param _beneficiary Address of the beneficiary who will receive the reward.
/// @param _depositId The identifier for the deposit for which to claim rewards.
/// @param _deadline The timestamp after which the signature should expire.
/// @param _signature Signature of the beneficiary authorizing this reward claim.
/// @return Amount of reward tokens claimed.
function claimRewardOnBehalf(address _beneficiary, uint256 _deadline, bytes memory _signature)
external
returns (uint256)
{
function claimRewardOnBehalf(
DepositIdentifier _depositId,
uint256 _deadline,
bytes memory _signature
) external returns (uint256) {
_revertIfPastDeadline(_deadline);
Deposit storage deposit = deposits[_depositId];
_revertIfSignatureIsNotValidNow(
_beneficiary,
deposit.beneficiary,
_hashTypedDataV4(
keccak256(
abi.encode(CLAIM_REWARD_TYPEHASH, _beneficiary, _useNonce(_beneficiary), _deadline)
abi.encode(CLAIM_REWARD_TYPEHASH, _depositId, _useNonce(deposit.beneficiary), _deadline)
)
),
_signature
);
return _claimReward(_beneficiary);
return _claimReward(_depositId, deposit);
}

/// @notice Called by an authorized rewards notifier to alert the staking contract that a new
Expand Down Expand Up @@ -640,18 +651,15 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
emit RewardNotified(_amount, msg.sender);
}

/// @notice Live value of the unclaimed rewards earned by a given beneficiary account with the
/// @notice Live value of the unclaimed rewards earned by a given deposit with the
/// scale factor included. Used internally for calculating reward checkpoints while minimizing
/// precision loss.
/// @return Live value of the unclaimed rewards earned by a given beneficiary account with the
/// @return Live value of the unclaimed rewards earned by a given deposit with the
/// scale factor included.
/// @dev See documentation for the public, non-scaled `unclaimedReward` method for more details.
function _scaledUnclaimedReward(address _beneficiary) internal view returns (uint256) {
return scaledUnclaimedRewardCheckpoint[_beneficiary]
+ (
earningPower[_beneficiary]
* (rewardPerTokenAccumulated() - beneficiaryRewardPerTokenCheckpoint[_beneficiary])
);
function _scaledUnclaimedReward(Deposit storage deposit) internal view returns (uint256) {
return deposit.scaledUnclaimedRewardCheckpoint
+ (deposit.earningPower * (rewardPerTokenAccumulated() - deposit.rewardPerTokenCheckpoint));
}

/// @notice Allows an address to increment their nonce and therefore invalidate any pending signed
Expand Down Expand Up @@ -705,19 +713,20 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
_revertIfAddressZero(_beneficiary);

_checkpointGlobalReward();
_checkpointReward(_beneficiary);

DelegationSurrogate _surrogate = _fetchOrDeploySurrogate(_delegatee);
_depositId = _useDepositId();

totalStaked += _amount;
depositorTotalStaked[_depositor] += _amount;
earningPower[_beneficiary] += _amount;
deposits[_depositId] = Deposit({
balance: _amount,
owner: _depositor,
delegatee: _delegatee,
beneficiary: _beneficiary
beneficiary: _beneficiary,
earningPower: _amount,
rewardPerTokenCheckpoint: rewardPerTokenAccumulatedCheckpoint,
scaledUnclaimedRewardCheckpoint: 0
});
_stakeTokenSafeTransferFrom(_depositor, address(_surrogate), _amount);
emit StakeDeposited(_depositor, _depositId, _amount, _amount);
Expand All @@ -732,13 +741,13 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
internal
{
_checkpointGlobalReward();
_checkpointReward(deposit.beneficiary);
_checkpointReward(deposit);

DelegationSurrogate _surrogate = surrogates[deposit.delegatee];

totalStaked += _amount;
depositorTotalStaked[deposit.owner] += _amount;
earningPower[deposit.beneficiary] += _amount;
deposit.earningPower += _amount;
deposit.balance += _amount;
_stakeTokenSafeTransferFrom(deposit.owner, address(_surrogate), _amount);
emit StakeDeposited(deposit.owner, _depositId, _amount, deposit.balance);
Expand Down Expand Up @@ -769,14 +778,9 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
address _newBeneficiary
) internal {
_revertIfAddressZero(_newBeneficiary);
_checkpointGlobalReward();
_checkpointReward(deposit.beneficiary);
earningPower[deposit.beneficiary] -= deposit.balance;

_checkpointReward(_newBeneficiary);
emit BeneficiaryAltered(_depositId, deposit.beneficiary, _newBeneficiary);
deposit.beneficiary = _newBeneficiary;
earningPower[_newBeneficiary] += deposit.balance;
}

/// @notice Internal convenience method which withdraws the stake from an existing deposit.
Expand All @@ -786,12 +790,12 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
internal
{
_checkpointGlobalReward();
_checkpointReward(deposit.beneficiary);
_checkpointReward(deposit);

deposit.balance -= _amount; // overflow prevents withdrawing more than balance
totalStaked -= _amount;
depositorTotalStaked[deposit.owner] -= _amount;
earningPower[deposit.beneficiary] -= _amount;
deposit.earningPower -= _amount;
_stakeTokenSafeTransferFrom(address(surrogates[deposit.delegatee]), deposit.owner, _amount);
emit StakeWithdrawn(_depositId, _amount, deposit.balance);
}
Expand All @@ -800,19 +804,22 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce
/// @return Amount of reward tokens claimed.
/// @dev This method must only be called after proper authorization has been completed.
/// @dev See public claimReward methods for additional documentation.
function _claimReward(address _beneficiary) internal returns (uint256) {
function _claimReward(DepositIdentifier _depositId, Deposit storage deposit)
internal
returns (uint256)
{
_checkpointGlobalReward();
_checkpointReward(_beneficiary);
_checkpointReward(deposit);

uint256 _reward = scaledUnclaimedRewardCheckpoint[_beneficiary] / SCALE_FACTOR;
uint256 _reward = deposit.scaledUnclaimedRewardCheckpoint / SCALE_FACTOR;
if (_reward == 0) return 0;

// retain sub-wei dust that would be left due to the precision loss
scaledUnclaimedRewardCheckpoint[_beneficiary] =
scaledUnclaimedRewardCheckpoint[_beneficiary] - (_reward * SCALE_FACTOR);
emit RewardClaimed(_beneficiary, _reward);
deposit.scaledUnclaimedRewardCheckpoint =
deposit.scaledUnclaimedRewardCheckpoint - (_reward * SCALE_FACTOR);
emit RewardClaimed(_depositId, deposit.beneficiary, _reward);

SafeERC20.safeTransfer(REWARD_TOKEN, _beneficiary, _reward);
SafeERC20.safeTransfer(REWARD_TOKEN, deposit.beneficiary, _reward);
return _reward;
}

Expand All @@ -824,13 +831,13 @@ contract GovernanceStaker is INotifiableRewardReceiver, Multicall, EIP712, Nonce

/// @notice Checkpoints the unclaimed rewards and reward per token accumulator of a given
/// beneficiary account.
/// @param _beneficiary The account for which reward parameters will be checkpointed.
/// @param deposit The deposit for which the reward parameters will be checkpointed.
/// @dev This is a sensitive internal helper method that must only be called after global rewards
/// accumulator has been checkpointed. It assumes the global `rewardPerTokenCheckpoint` is up to
/// date.
function _checkpointReward(address _beneficiary) internal {
scaledUnclaimedRewardCheckpoint[_beneficiary] = _scaledUnclaimedReward(_beneficiary);
beneficiaryRewardPerTokenCheckpoint[_beneficiary] = rewardPerTokenAccumulatedCheckpoint;
function _checkpointReward(Deposit storage deposit) internal {
deposit.scaledUnclaimedRewardCheckpoint = _scaledUnclaimedReward(deposit);
deposit.rewardPerTokenCheckpoint = rewardPerTokenAccumulatedCheckpoint;
}

/// @notice Internal helper method which sets the admin address.
Expand Down
27 changes: 0 additions & 27 deletions test/GovernanceStaker.invariants.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ contract GovernanceStakerInvariants is Test {
assertEq(govStaker.totalStaked(), handler.reduceDepositors(0, this.accumulateDeposits));
}

function invariant_Sum_of_beneficiary_earning_power_equals_total_stake() public {
assertEq(govStaker.totalStaked(), handler.reduceBeneficiaries(0, this.accumulateEarningPower));
}

function invariant_Sum_of_surrogate_balance_equals_total_stake() public {
assertEq(govStaker.totalStaked(), handler.reduceDelegates(0, this.accumulateSurrogateBalance));
}
Expand All @@ -70,13 +66,6 @@ contract GovernanceStakerInvariants is Test {
);
}

function invariant_Sum_of_unclaimed_reward_should_be_less_than_or_equal_to_total_rewards() public {
assertLe(
handler.reduceBeneficiaries(0, this.accumulateUnclaimedReward),
rewardToken.balanceOf(address(govStaker))
);
}

function invariant_RewardPerTokenAccumulatedCheckpoint_should_be_greater_or_equal_to_the_last_rewardPerTokenAccumulatedCheckpoint(
) public view {
assertGe(
Expand All @@ -96,22 +85,6 @@ contract GovernanceStakerInvariants is Test {
return balance + govStaker.depositorTotalStaked(depositor);
}

function accumulateEarningPower(uint256 earningPower, address caller)
external
view
returns (uint256)
{
return earningPower + govStaker.earningPower(caller);
}

function accumulateUnclaimedReward(uint256 unclaimedReward, address beneficiary)
external
view
returns (uint256)
{
return unclaimedReward + govStaker.unclaimedReward(beneficiary);
}

function accumulateSurrogateBalance(uint256 balance, address delegate)
external
view
Expand Down
Loading

0 comments on commit 1b75abc

Please sign in to comment.