Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nonce cannot be incremented by depositor #89

Merged
merged 4 commits into from
Dec 13, 2024
Merged

Nonce cannot be incremented by depositor #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0426d1c
Nonce cannot be incremented by depositor
alexkeating Dec 12, 2024
6ef5eb0
Simplify calculate total earning power
alexkeating Dec 12, 2024
3147844
Remove unused import
alexkeating Dec 12, 2024
5fd5824
Set lastOracle update time on deployment
alexkeating Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/BinaryEligibilityOracleEarningPowerCalculator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ contract BinaryEligibilityOracleEarningPowerCalculator is Ownable, IEarningPower
_setOraclePauseGuardian(_oraclePauseGuardian);
_setDelegateeScoreEligibilityThreshold(_delegateeScoreEligibilityThreshold);
_setUpdateEligibilityDelay(_updateEligibilityDelay);
lastOracleUpdateTime = block.timestamp;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use a test here.

}

/// @notice Calculates the earning power for a given delegatee and staking amount.
Expand Down
6 changes: 1 addition & 5 deletions src/GovernanceStaker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.23;

import {DelegationSurrogate} from "src/DelegationSurrogate.sol";
import {DelegationSurrogateVotes} from "src/DelegationSurrogateVotes.sol";
import {INotifiableRewardReceiver} from "src/interfaces/INotifiableRewardReceiver.sol";
import {IEarningPowerCalculator} from "src/interfaces/IEarningPowerCalculator.sol";
import {IERC20} from "openzeppelin/token/ERC20/IERC20.sol";
Expand Down Expand Up @@ -761,10 +760,7 @@ abstract contract GovernanceStaker is INotifiableRewardReceiver, Multicall {
uint256 _depositNewEarningPower,
uint256 _totalEarningPower
) internal pure returns (uint256 _newTotalEarningPower) {
if (_depositNewEarningPower >= _depositOldEarningPower) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this changed because of a different finding?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was do you think we should keep it the same?

return _totalEarningPower + (_depositNewEarningPower - _depositOldEarningPower);
}
return _totalEarningPower - (_depositOldEarningPower - _depositNewEarningPower);
return _totalEarningPower + _depositNewEarningPower - _depositOldEarningPower;
}

/// @notice Internal helper method which sets the admin address.
Expand Down
9 changes: 5 additions & 4 deletions src/extensions/GovernanceStakerOnBehalf.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,13 +255,14 @@ abstract contract GovernanceStakerOnBehalf is GovernanceStaker, EIP712, Nonces {
_revertIfPastDeadline(_deadline);
Deposit storage deposit = deposits[_depositId];
bytes32 _claimerHash = _hashTypedDataV4(
keccak256(
abi.encode(CLAIM_REWARD_TYPEHASH, _depositId, _useNonce(deposit.claimer), _deadline)
)
keccak256(abi.encode(CLAIM_REWARD_TYPEHASH, _depositId, nonces(deposit.claimer), _deadline))
);
bool _isValidClaimerClaim =
SignatureChecker.isValidSignatureNow(deposit.claimer, _claimerHash, _signature);
if (_isValidClaimerClaim) return _claimReward(_depositId, deposit, deposit.claimer);
if (_isValidClaimerClaim) {
_useNonce(deposit.claimer);
return _claimReward(_depositId, deposit, deposit.claimer);
}

bytes32 _ownerHash = _hashTypedDataV4(
keccak256(abi.encode(CLAIM_REWARD_TYPEHASH, _depositId, _useNonce(deposit.owner), _deadline))
Expand Down
14 changes: 8 additions & 6 deletions test/GovernanceStakerOnBehalf.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,12 +1269,11 @@ contract ClaimRewardOnBehalf is GovernanceStakerRewardsTest {
uint256 _depositAmount,
uint256 _durationPercent,
uint256 _rewardAmount,
address _delegatee,
address _depositor,
uint256 _currentNonce,
uint256 _deadline
) public {
vm.assume(_delegatee != address(0) && _depositor != address(0) && _sender != address(0));
vm.assume(_depositor != address(0) && _sender != address(0));
_claimerPrivateKey = bound(_claimerPrivateKey, 1, 100e18);
address _claimer = vm.addr(_claimerPrivateKey);

Expand All @@ -1284,7 +1283,7 @@ contract ClaimRewardOnBehalf is GovernanceStakerRewardsTest {

// A user deposits staking tokens
(_depositAmount, _depositId) =
_boundMintAndStake(_depositor, _depositAmount, _delegatee, _claimer);
_boundMintAndStake(_depositor, _depositAmount, address(0x01), _claimer);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this change was made?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, this is to avoid a stack too deep error

// The contract is notified of a reward
_mintTransferAndNotifyReward(_rewardAmount);
// A portion of the duration passes
Expand All @@ -1307,10 +1306,12 @@ contract ClaimRewardOnBehalf is GovernanceStakerRewardsTest {
keccak256(abi.encodePacked("\x19\x01", EIP712_DOMAIN_SEPARATOR, _message));
bytes memory _signature = _sign(_claimerPrivateKey, _messageHash);

uint256 _oldDepositorNonce = govStaker.nonces(_depositor);
vm.prank(_sender);
govStaker.claimRewardOnBehalf(_depositId, _deadline, _signature);

assertEq(rewardToken.balanceOf(_claimer), _earned);
assertEq(govStaker.nonces(_depositor), _oldDepositorNonce);
}

function testFuzz_ClaimRewardOnBehalfOfDepositor(
Expand All @@ -1319,12 +1320,11 @@ contract ClaimRewardOnBehalf is GovernanceStakerRewardsTest {
uint256 _depositAmount,
uint256 _durationPercent,
uint256 _rewardAmount,
address _delegatee,
address _claimer,
uint256 _currentNonce,
uint256 _deadline
) public {
vm.assume(_delegatee != address(0) && _claimer != address(0) && _sender != address(0));
vm.assume(_claimer != address(0) && _sender != address(0));
_depositorPrivateKey = bound(_depositorPrivateKey, 1, 100e18);
address _depositor = vm.addr(_depositorPrivateKey);

Expand All @@ -1334,7 +1334,7 @@ contract ClaimRewardOnBehalf is GovernanceStakerRewardsTest {

// A user deposits staking tokens
(_depositAmount, _depositId) =
_boundMintAndStake(_depositor, _depositAmount, _delegatee, _claimer);
_boundMintAndStake(_depositor, _depositAmount, address(0x01), _claimer);
// The contract is notified of a reward
_mintTransferAndNotifyReward(_rewardAmount);
// A portion of the duration passes
Expand All @@ -1357,10 +1357,12 @@ contract ClaimRewardOnBehalf is GovernanceStakerRewardsTest {
keccak256(abi.encodePacked("\x19\x01", EIP712_DOMAIN_SEPARATOR, _message));
bytes memory _signature = _sign(_depositorPrivateKey, _messageHash);

uint256 _oldClaimerNonce = govStaker.nonces(_claimer);
vm.prank(_sender);
govStaker.claimRewardOnBehalf(_depositId, _deadline, _signature);

assertEq(rewardToken.balanceOf(_depositor), _earned);
assertEq(govStaker.nonces(_claimer), _oldClaimerNonce);
}

function testFuzz_ReturnsClaimedRewardAmount(
Expand Down
Loading