From 3f4341ea9503c96f6f329fe7c59dcbc66daf2b57 Mon Sep 17 00:00:00 2001 From: Ben DiFrancesco Date: Fri, 10 Jan 2025 17:34:55 -0500 Subject: [PATCH] Small improvements, cleanup and updates * Remove unused variables in tests to resolve compiler warnings * Bump the compiler version to 0.8.28 and re-run the gas report * Turn on Slither in the CI config --- .github/workflows/ci.yml | 40 ++++++++++----------- foundry.toml | 2 +- src/Staker.sol | 3 -- src/interfaces/IERC20Staking.sol | 1 - src/notifiers/MintRewardNotifier.sol | 2 +- test/Staker.t.sol | 4 --- test/fakes/FakeMinter.sol | 1 - test/gas-reports/Staker.g.sol | 2 +- test/gas-reports/gov-staker-gas-report.json | 22 ++++++------ test/harnesses/StakerHarness.sol | 2 -- test/lib/GasReport.sol | 2 +- test/mocks/MockERC20Votes.sol | 1 - 12 files changed, 35 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 694e659..43a187f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,23 +120,23 @@ jobs: scopelint --version scopelint check - # slither-analyze: - # runs-on: ubuntu-latest - # permissions: - # contents: read - # security-events: write - # steps: - # - uses: actions/checkout@v3 - - # - name: Run Slither - # uses: crytic/slither-action@v0.3.0 - # id: slither # Required to reference this step in the next step. - # with: - # fail-on: none # Required to avoid failing the CI run regardless of findings. - # sarif: results.sarif - # slither-args: --filter-paths "./lib|./test" --exclude naming-convention,solc-version - - # - name: Upload SARIF file - # uses: github/codeql-action/upload-sarif@v2 - # with: - # sarif_file: ${{ steps.slither.outputs.sarif }} + slither-analyze: + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@v3 + + - name: Run Slither + uses: crytic/slither-action@v0.3.0 + id: slither # Required to reference this step in the next step. + with: + fail-on: none # Required to avoid failing the CI run regardless of findings. + sarif: results.sarif + slither-args: --filter-paths "./lib|./test" --exclude naming-convention,solc-version + + - name: Upload SARIF file + uses: github/codeql-action/upload-sarif@v2 + with: + sarif_file: ${{ steps.slither.outputs.sarif }} diff --git a/foundry.toml b/foundry.toml index 8ad918b..0e9e985 100644 --- a/foundry.toml +++ b/foundry.toml @@ -6,7 +6,7 @@ optimizer = true optimizer_runs = 10_000_000 remappings = ["openzeppelin/=lib/openzeppelin-contracts/contracts"] - solc_version = "0.8.26" + solc_version = "0.8.28" verbosity = 3 [profile.ci] diff --git a/src/Staker.sol b/src/Staker.sol index 137a0b7..ab8d5e7 100644 --- a/src/Staker.sol +++ b/src/Staker.sol @@ -8,9 +8,6 @@ import {IERC20} from "openzeppelin/token/ERC20/IERC20.sol"; import {SafeERC20} from "openzeppelin/token/ERC20/utils/SafeERC20.sol"; import {Multicall} from "openzeppelin/utils/Multicall.sol"; import {SafeCast} from "openzeppelin/utils/math/SafeCast.sol"; -import {Nonces} from "openzeppelin/utils/Nonces.sol"; -import {SignatureChecker} from "openzeppelin/utils/cryptography/SignatureChecker.sol"; -import {EIP712} from "openzeppelin/utils/cryptography/EIP712.sol"; /// @title Staker /// @author [ScopeLift](https://scopelift.co) diff --git a/src/interfaces/IERC20Staking.sol b/src/interfaces/IERC20Staking.sol index da42edf..26ccb3b 100644 --- a/src/interfaces/IERC20Staking.sol +++ b/src/interfaces/IERC20Staking.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.23; import {IERC20Permit} from "openzeppelin/token/ERC20/extensions/IERC20Permit.sol"; -import {IERC20} from "openzeppelin/token/ERC20/IERC20.sol"; import {IERC20Delegates} from "src/interfaces/IERC20Delegates.sol"; /// @notice The interface of an ERC20 that supports a governor staker and all of the created diff --git a/src/notifiers/MintRewardNotifier.sol b/src/notifiers/MintRewardNotifier.sol index f980f25..a4a782e 100644 --- a/src/notifiers/MintRewardNotifier.sol +++ b/src/notifiers/MintRewardNotifier.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity ^0.8.23; -import {INotifiableRewardReceiver, IERC20} from "src/interfaces/INotifiableRewardReceiver.sol"; +import {INotifiableRewardReceiver} from "src/interfaces/INotifiableRewardReceiver.sol"; import {IMintable} from "src/interfaces/IMintable.sol"; import {RewardTokenNotifierBase} from "src/notifiers/RewardTokenNotifierBase.sol"; import {Ownable} from "openzeppelin/access/Ownable.sol"; diff --git a/test/Staker.t.sol b/test/Staker.t.sol index e676dd5..20d0006 100644 --- a/test/Staker.t.sol +++ b/test/Staker.t.sol @@ -2840,7 +2840,6 @@ contract BumpEarningPower is StakerRewardsTest { vm.assume(_tipReceiver != address(0) && _tipReceiver != address(govStaker)); _stakeAmount = _boundToRealisticStake(_stakeAmount); _rewardAmount = _boundToRealisticReward(_rewardAmount); - uint256 _initialTipReceiverBalance = rewardToken.balanceOf(_tipReceiver); _earningPowerIncrease = uint96(bound(_earningPowerIncrease, 1, type(uint48).max)); // A user deposits staking tokens @@ -2863,8 +2862,6 @@ contract BumpEarningPower is StakerRewardsTest { vm.prank(_bumpCaller); govStaker.bumpEarningPower(_depositId, _tipReceiver, _requestedTip); - uint256 _tipReceiverBalanceIncrease = - rewardToken.balanceOf(_tipReceiver) - _initialTipReceiverBalance; assertEq(govStaker.unclaimedReward(_depositId), _unclaimedRewards - _requestedTip); } @@ -3035,7 +3032,6 @@ contract BumpEarningPower is StakerRewardsTest { _stakeAmount = _boundToRealisticStake(_stakeAmount); _rewardAmount = bound(_rewardAmount, maxBumpTip + 1, 10_000_000e18); _earningPowerDecrease = bound(_earningPowerDecrease, 1, _stakeAmount); - uint256 _initialTipReceiverBalance = rewardToken.balanceOf(_tipReceiver); // A user deposits staking tokens (, Staker.DepositIdentifier _depositId) = diff --git a/test/fakes/FakeMinter.sol b/test/fakes/FakeMinter.sol index 7a9859a..6f48b39 100644 --- a/test/fakes/FakeMinter.sol +++ b/test/fakes/FakeMinter.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity ^0.8.23; -import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol"; import {IMintable} from "src/interfaces/IMintable.sol"; contract FakeMinter is IMintable { diff --git a/test/gas-reports/Staker.g.sol b/test/gas-reports/Staker.g.sol index 52c82cc..3269c85 100644 --- a/test/gas-reports/Staker.g.sol +++ b/test/gas-reports/Staker.g.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.26; +pragma solidity ^0.8.23; import {Vm, Test, stdStorage, StdStorage, console2, stdError} from "forge-std/Test.sol"; import {GasReport} from "test/lib/GasReport.sol"; diff --git a/test/gas-reports/gov-staker-gas-report.json b/test/gas-reports/gov-staker-gas-report.json index ad5eb93..3bf7089 100644 --- a/test/gas-reports/gov-staker-gas-report.json +++ b/test/gas-reports/gov-staker-gas-report.json @@ -1,5 +1,5 @@ { - "generatedAt": 1734023383, + "generatedAt": 1736548728, "reportName:": "gov-stakerGasReport", "results": [ { @@ -16,27 +16,27 @@ }, { "scenarioName": "Stake more after initial stake", - "gasUsed": 101615 + "gasUsed": 101547 }, { "scenarioName": "Alter delegatee with new delegatee", - "gasUsed": 218503 + "gasUsed": 218435 }, { "scenarioName": "Alter delegatee with existing delegatee", - "gasUsed": 86397 + "gasUsed": 86329 }, { "scenarioName": "Alter claimer to a new address", - "gasUsed": 69468 + "gasUsed": 69400 }, { "scenarioName": "Withdraw full stake", - "gasUsed": 110594 + "gasUsed": 110554 }, { "scenarioName": "Withdraw partial stake", - "gasUsed": 124994 + "gasUsed": 124954 }, { "scenarioName": "Claim reward when no reward", @@ -44,7 +44,7 @@ }, { "scenarioName": "Claim reward when reward", - "gasUsed": 164908 + "gasUsed": 164840 }, { "scenarioName": "Notify reward", @@ -56,15 +56,15 @@ }, { "scenarioName": "Bump earning power up no reward", - "gasUsed": 80178 + "gasUsed": 80502 }, { "scenarioName": "Bump earning power down with reward", - "gasUsed": 108922 + "gasUsed": 109274 }, { "scenarioName": "Bump earning power up with reward", - "gasUsed": 108922 + "gasUsed": 109274 } ] } \ No newline at end of file diff --git a/test/harnesses/StakerHarness.sol b/test/harnesses/StakerHarness.sol index c558e30..48e69ae 100644 --- a/test/harnesses/StakerHarness.sol +++ b/test/harnesses/StakerHarness.sol @@ -8,10 +8,8 @@ import {StakerOnBehalf} from "src/extensions/StakerOnBehalf.sol"; import {StakerDelegateSurrogateVotes} from "src/extensions/StakerDelegateSurrogateVotes.sol"; import {IERC20} from "openzeppelin/token/ERC20/IERC20.sol"; -import {SafeERC20} from "openzeppelin/token/ERC20/utils/SafeERC20.sol"; import {EIP712} from "openzeppelin/utils/cryptography/EIP712.sol"; import {IERC20Staking} from "src/interfaces/IERC20Staking.sol"; -import {IERC20Delegates} from "src/interfaces/IERC20Delegates.sol"; import {IEarningPowerCalculator} from "src/interfaces/IEarningPowerCalculator.sol"; import {DelegationSurrogate} from "src/DelegationSurrogate.sol"; diff --git a/test/lib/GasReport.sol b/test/lib/GasReport.sol index e92eaa9..4aa6b7a 100644 --- a/test/lib/GasReport.sol +++ b/test/lib/GasReport.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.26; +pragma solidity ^0.8.23; import {Test, console2} from "forge-std/Test.sol"; import {Vm} from "forge-std/Vm.sol"; diff --git a/test/mocks/MockERC20Votes.sol b/test/mocks/MockERC20Votes.sol index 21e8649..e02836d 100644 --- a/test/mocks/MockERC20Votes.sol +++ b/test/mocks/MockERC20Votes.sol @@ -5,7 +5,6 @@ import { ERC20, ERC20Permit, IERC20Permit } from "openzeppelin/token/ERC20/extensions/ERC20Permit.sol"; import {IERC20} from "openzeppelin/token/ERC20/IERC20.sol"; -import {IERC20Delegates} from "src/interfaces/IERC20Delegates.sol"; import {IERC20Staking} from "src/interfaces/IERC20Staking.sol"; import {IMintable} from "src/interfaces/IMintable.sol";