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.

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

Conversation

alexkeating
Copy link
Collaborator

@alexkeating alexkeating commented Dec 12, 2024

Description

  • Remove ununsed import
  • Make calculation cleaner
  • Initialize lastOracleUpdateTime
  • Don't consume nonce unless signature is successful

Closes #85

Copy link

Coverage after merging fix/resolve-small-issues into main will be

98.07%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   BinaryEligibilityOracleEarningPowerCalculator.sol100%100%100%100%
   DelegationSurrogate.sol100%100%100%100%
   DelegationSurrogateVotes.sol100%100%100%100%
   GovernanceStaker.sol98.45%100%97.22%98.55%255–257
src/extensions
   GovernanceStakerDelegateSurrogateVotes.sol87.50%50%100%90.91%25–26
   GovernanceStakerOnBehalf.sol100%100%100%100%
   GovernanceStakerPermitAndStake.sol76.47%0%100%76.92%19–20, 48, 76

@alexkeating alexkeating marked this pull request as ready for review December 13, 2024 14:12
@@ -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

Copy link
Collaborator

@garyghayrat garyghayrat left a comment

Choose a reason for hiding this comment

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

left a question but otherwise looks good.

@@ -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.

@@ -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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A combination of small issues
3 participants