| Number | Details | Instances |
|---|---|---|
| [Medium-1] | Off-by-one timestamp error | 1 |
| Number | Details | Instances |
|---|---|---|
| [Low-1] | Solidity version 0.8.20 won't work on all chains due to PUSH0 | 1 |
| [Low-2] | Require should be used instead of assert as assert in some solidity versions does not refund remaining gas | 1 |
| [Low-3] | Loss of precision | 3 |
| [Low-4] | Missing zero address check in constructor | 1 |
| [Low-5] | Using zero as a parameter | 3 |
| [Low-6] | Balances directly compared using ==/!= can be dosed | 1 |
| [Low-7] | Functions calling contracts/addresses with transfer hooks are missing reentrancy guards | 2 |
| [Low-8] | Consider a uptime feed on L2 deployments to prevent issues caused by downtime | 3 |
| Number | Details | Instances |
|---|---|---|
| [NonCritical-1] | Code does not follow the best practice of check-effects-interaction | 1 |
| [NonCritical-2] | Floating pragma should be avoided | 1 |
| [NonCritical-3] | In functions which accept an address as a parameter, there should be a zero address check to prevent bugs | 18 |
| [NonCritical-4] | Default int values are manually set | 3 |
| [NonCritical-5] | Revert statements within external and public functions can be used to perform DOS attacks | 4 |
| [NonCritical-6] | Contract lines should not be longer than 120 characters for readability | 2 |
| [NonCritical-7] | Specific imports should be used where possible so only used code is imported | 1 |
| [NonCritical-8] | Old Solidity version | 1 |
| [NonCritical-9] | Not all event definitions are utilizing indexed variables. | 1 |
| [NonCritical-10] | Contracts should have all public/external functions exposed by interfaces | 2 |
| [NonCritical-11] | Functions within contracts are not ordered according to the solidity style guide | 1 |
| [NonCritical-12] | Double type casts create complexity within the code | 1 |
| [NonCritical-13] | A function which defines named returns in it's declaration doesn't need to use return | 1 |
| [NonCritical-14] | Constants should be on the left side of the comparison | 10 |
| [NonCritical-15] | Overly complicated arithmetic | 2 |
| [NonCritical-16] | Redundant else statement | 1 |
| [NonCritical-17] | Cyclomatic complexity in functions | 2 |
| [NonCritical-18] | Events may be emitted out of order due to code not follow the best practice of check-effects-interaction | 1 |
| [NonCritical-19] | Missing events in sensitive functions | 2 |
| [NonCritical-20] | Avoid mutating function parameters | 3 |
| [NonCritical-21] | Constructors should emit an event | 1 |
| [NonCritical-22] | Errors should have parameters | 12 |
| [NonCritical-23] | Avoid arithmetic directly within array indices | 2 |
| [NonCritical-24] | ERC777 tokens can introduce reentrancy risks | 1 |
| Number | Details | Instances | Gas |
|---|---|---|---|
| [Gas-1] | Avoid updating storage when the value hasn't changed | 2 | 3200 |
| [Gas-2] | Using named returns for pure and view functions is cheaper than using regular returns | 15 | 5850 |
| [Gas-3] | Usage of smaller uint/int types causes overhead | 35 | 67375 |
| [Gas-4] | Default int values are manually reset | 1 | 0.0 |
| [Gas-5] | Function calls within for loops | 6 | 0.0 |
| [Gas-6] | For loops in public or external functions should be avoided due to high gas costs and possible DOS | 4 | 0.0 |
| [Gas-7] | Use assembly to check for the zero address | 3 | 0.0 |
| [Gas-8] | Divisions which do not divide by -X cannot overflow or underflow so such operations can be unchecked to save gas | 1 | 0.0 |
| [Gas-9] | Don't use _msgSender() if not supporting EIP-2771 | 2 | 64 |
| [Gas-10] | Consider using OZ EnumerateSet in place of nested mappings | 2 | 4000 |
| [Gas-11] | Use assembly to emit events | 6 | 1368 |
| [Gas-12] | Use solady library where possible to save gas | 1 | 1000 |
| [Gas-13] | Using private rather than public for constants and immutables, saves gas | 3 | 0.0 |
| [Gas-14] | Lack of unchecked in loops | 5 | 2400 |
| [Gas-15] | Use Unchecked for Divisions on Constant or Immutable Values | 2 | 0.0 |
| [Gas-16] | Using nested if to save gas | 7 | 294 |
| [Gas-17] | Optimize Storage with Byte Truncation for Time Related State Variables | 3 | 18000 |
| [Gas-18] | Caching global variables is more expensive than using the actual variable | 1 | 12 |
| [Gas-19] | Solidity versions 0.8.19 and above are more gas efficient | 1 | 1000 |
| [Gas-20] | Variable declared within iteration | 4 | 0.0 |
| [Gas-21] | Calling .length in a for loop wastes gas | 7 | 6790 |
| [Gas-22] | Internal functions only used once can be inlined to save gas | 2 | 120 |
| [Gas-23] | Constructors can be marked as payable to save deployment gas | 3 | 0.0 |
| [Gas-24] | Use OZ Array.unsafeAccess() to avoid repeated array length checks | 3 | 18900 |
| [Gas-25] | Call msg.XYZ directly rather than caching the value to save gas | 1 | 0.0 |
| [Gas-26] | Write direct outcome, instead of performing mathematical operations for constant state variables | 1 | 0.0 |
| [Gas-27] | Consider pre-calculating the address of address(this) to save gas | 3 | 0.0 |
| [Gas-28] | Use constants instead of type(uint).max | 2 | 0.0 |
In Solidity, using >= or <= to compare against block.timestamp (alias now) may introduce off-by-one errors due to the fact that block.timestamp is only updated once per block and its value remains constant throughout the block's execution. If an operation happens at the exact second when block.timestamp changes, it could result in unexpected behavior. To avoid this, it's safer to use strict inequality operators (> or <). For instance, if a condition should only be met after a certain time, use block.timestamp > time rather than block.timestamp >= time. This way, potential off-by-one errors due to the exact timing of block mining are mitigated, leading to safer, more predictable contract behavior.
Num of instances: 1
Click to show findings
['642']
642: function timeElapsedInEpoch(uint48 epoch, uint48 lastUpdated) internal view returns (uint256) { // <= FOUND
643:
644: uint256 startTimestamp = getEpochStartTimestamp(epoch);
645: uint256 endTimestamp = getEpochEndTimestamp(epoch);
646:
647:
648: if (block.timestamp >= endTimestamp) { // <= FOUND
649:
650:
651:
652:
653: return lastUpdated > startTimestamp ? endTimestamp - lastUpdated : EPOCH_DURATION;
654: } else if (block.timestamp >= startTimestamp && block.timestamp < endTimestamp) { // <= FOUND
655:
656:
657:
658: return lastUpdated > startTimestamp ? block.timestamp - lastUpdated : block.timestamp - startTimestamp;
659: } else {
660:
661: return 0;
662: }
663: }Solidity version 0.8.20 uses the new Shanghai EVM which introduces the PUSH0 opcode, this may not be implemented on all chains and L2 thus reducing the portability and compatability of the code. Consider using a earlier solidity version.
Num of instances: 1
[Low-2] Require should be used instead of assert as assert in some solidity versions does not refund remaining gas
In Solidity, require should be used instead of assert for input validation and error handling due to its more efficient and flexible nature. While both functions check for a specified condition and revert the transaction if it evaluates to false, assert consumes all remaining gas upon failure, which can lead to higher costs for users. On the other hand, require refunds unused gas, making it a more gas-efficient option. Additionally, require allows for custom error messages, which improves debugging and error reporting.
Num of instances: 1
Dividing by large numbers in Solidity can cause a loss of precision due to the language's inherent integer division behavior. Solidity does not support floating-point arithmetic, and as a result, division between integers yields an integer result, truncating any fractional part. When dividing by a large number, the resulting value may become significantly smaller, leading to a loss of precision, as the fractional part is discarded.
Num of instances: 3
Click to show findings
['412']
412: function getEpoch(uint48 timestamp) public view override returns (uint48) { // <= FOUND
413: return uint48(timestamp / EPOCH_DURATION); // <= FOUND
414: }['467']
467: function rewardAmount(
468: DistributionStorage storage distributionStorage,
469: uint48 epoch
470: ) internal view returns (uint256) {
471: return distributionStorage.amounts[epoch / EPOCHS_PER_SLOT][epoch % EPOCHS_PER_SLOT]; // <= FOUND
472: }['479']
479: function increaseRewardAmounts(
480: address rewarded,
481: address reward,
482: uint48 startEpoch,
483: uint128[] memory amounts
484: ) internal virtual {
485: mapping(uint256 => uint128[EPOCHS_PER_SLOT]) storage storageAmounts = distributions[rewarded][reward].amounts;
486:
487: for (uint48 i = 0; i < amounts.length; ++i) {
488:
489: unchecked {
490: uint48 epoch = startEpoch + i;
491: storageAmounts[epoch / EPOCHS_PER_SLOT][epoch % EPOCHS_PER_SLOT] += amounts[i]; // <= FOUND
492: }
493: }
494: }In Solidity, constructors often take address parameters to initialize important components of a contract, such as owner or linked contracts. However, without a check, there's a risk that an address parameter could be mistakenly set to the zero address (0x0). This could occur due to a mistake or oversight during contract deployment. A zero address in a crucial role can cause serious issues, as it cannot perform actions like a normal address, and any funds sent to it are irretrievable. Therefore, it's crucial to include a zero address check in constructors to prevent such potential problems. If a zero address is detected, the constructor should revert the transaction.
Num of instances: 1
Click to show findings
['125']
125: constructor(address _evc, uint48 _epochDuration) EVCUtil(_evc) { // <= FOUND
126: if (_epochDuration < MIN_EPOCH_DURATION || _epochDuration > MAX_EPOCH_DURATION) {
127: revert InvalidEpoch();
128: }
129:
130: EPOCH_DURATION = _epochDuration;
131: }Taking 0 as a valid argument in Solidity without checks can lead to severe security issues. A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because '0' can be interpreted as an uninitialized address, leading to transfers to the '0x0' address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code. It's important to always validate input and handle edge cases like 0 appropriately. Use require() statements to enforce conditions and provide clear error messages to facilitate debugging and safer code.
Num of instances: 3
Click to show findings
['138']
138: function registerReward(
139: address rewarded,
140: address reward,
141: uint48 startEpoch,
142: uint128[] calldata rewardAmounts
143: ) external virtual override nonReentrant {
144: uint48 epoch = currentEpoch();
145:
146:
147: if (startEpoch == 0) {
148: startEpoch = epoch + 1;
149: }
150:
151:
152: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) {
153: revert InvalidEpoch();
154: }
155:
156:
157: if (rewardAmounts.length > MAX_DISTRIBUTION_LENGTH) {
158: revert InvalidDistribution();
159: }
160:
161:
162: uint256 totalAmount;
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) {
164: totalAmount += rewardAmounts[i];
165: }
166:
167: if (totalAmount == 0) {
168: revert InvalidAmount();
169: }
170:
171:
172: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
173: if (distributionStorage.lastUpdated == 0) {
174: distributionStorage.lastUpdated = uint48(block.timestamp);
175: } else {
176: updateReward(rewarded, reward, address(0)); // <= FOUND
177: }
178:
179:
180: uint256 totalRegistered = uint256(distributionStorage.totalRegistered) + totalAmount;
181:
182: if (SCALER * totalRegistered > type(uint160).max) {
183: revert AccumulatorOverflow();
184: }
185:
186:
187:
188: distributionStorage.totalRegistered = uint128(totalRegistered);
189:
190:
191: increaseRewardAmounts(rewarded, reward, startEpoch, rewardAmounts);
192:
193:
194: address msgSender = _msgSender();
195: pullToken(IERC20(reward), msgSender, totalAmount);
196:
197: emit RewardRegistered(msgSender, rewarded, reward, startEpoch, rewardAmounts);
198: }['206']
206: function updateReward(address rewarded, address reward, address recipient) public virtual override { // <= FOUND
207: address msgSender = _msgSender();
208:
209:
210: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
211: uint256 currentAccountBalance = accountStorage.enabledRewards.contains(reward) ? accountStorage.balance : 0;
212:
213: updateRewardInternal(
214: distributions[rewarded][reward],
215: accountStorage.earned[reward],
216: rewarded,
217: reward,
218: currentAccountBalance,
219: false
220: );
221:
222: if (recipient != address(0)) {
223: claim(address(0), rewarded, reward, recipient); // <= FOUND
224: }
225: }['261']
261: function enableReward(address rewarded, address reward) external virtual override { // <= FOUND
262: address msgSender = _msgSender();
263: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
264: SetStorage storage accountEnabledRewards = accountStorage.enabledRewards;
265:
266: if (accountEnabledRewards.insert(reward)) {
267: if (accountEnabledRewards.numElements > MAX_REWARDS_ENABLED) {
268: revert TooManyRewardsEnabled();
269: }
270:
271: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
272: uint256 currentAccountBalance = accountStorage.balance;
273:
274:
275:
276: updateRewardInternal(distributionStorage, accountStorage.earned[reward], rewarded, reward, 0, false); // <= FOUND
277:
278: distributionStorage.totalEligible += currentAccountBalance;
279:
280: emit RewardEnabled(msgSender, rewarded, reward);
281: }
282: }Comparing balances directly using the == or != operators in Solidity can lead to potential Denial of Service (DoS) vulnerabilities. This is because direct comparisons assume exact balance amounts, which may not account for dynamic changes due to transfers, fees, or other contract interactions. An attacker could manipulate balances to match or deviate from a specific target, causing functions that rely on these comparisons to behave unpredictably or fail. A more robust approach is to use range checks or tolerate minor deviations in balance comparisons. This approach reduces the risk of DoS attacks by not relying on exact balance equality or inequality for critical contract operations.
Num of instances: 1
Click to show findings
['442']
442: if (token.balanceOf(address(this)) - preBalance != amount) { // <= FOUNDWhile adherence to the check-effects-interaction pattern is commendable, the absence of a reentrancy guard in functions, especially where transfer hooks might be present, can expose the protocol users to risks of read-only reentrancies. Such reentrancy vulnerabilities can be exploited to execute malicious actions even without altering the contract state. Without a reentrancy guard, the only potential mitigation would be to blocklist the entire protocol - an extreme and disruptive measure. Therefore, incorporating a reentrancy guard into these functions is vital to bolster security, as it helps protect against both traditional reentrancy attacks and read-only reentrancies, ensuring robust and safe protocol operations.
Num of instances: 2
Click to show findings
['453']
453: function pushToken(IERC20 token, address to, uint256 amount) internal { // <= FOUND
454: address owner = evc.getAccountOwner(to);
455:
456: if (to == address(0) || (owner != address(0) && owner != to)) {
457: revert InvalidRecipient();
458: } // <= FOUND
459:
460: IERC20(token).safeTransfer(to, amount);
461: } // <= FOUND['453']
453: function pushToken(IERC20 token, address to, uint256 amount) internal {
454: address owner = evc.getAccountOwner(to);
455:
456: if (to == address(0) || (owner != address(0) && owner != to)) {
457: revert InvalidRecipient();
458: }
459:
460: IERC20(token).safeTransfer(to, amount); // <= FOUND
461: }In L2 deployments, incorporating an uptime feed is crucial to mitigate issues arising from sequencer downtime. Downtime can disrupt services, leading to transaction failures or incorrect data readings, affecting overall system reliability. By integrating an uptime feed, you gain insight into the operational status of the L2 network, enabling proactive measures like halting sensitive operations or alerting users. This approach ensures that your contract behaves predictably and securely during network outages, enhancing the robustness and reliability of your decentralized application, which is especially important in mission-critical or high-stakes environments.
Num of instances: 3
Click to show findings
['14']
14: contract StakingRewardStreams is BaseRewardStreams, IStakingRewardStreams // <= FOUND['18']
18: contract TrackingRewardStreams is BaseRewardStreams, ITrackingRewardStreams // <= FOUND['14']
14: abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard // <= FOUNDThe "check-effects-interaction" pattern is a best practice in smart contract development, emphasizing the order of operations in functions to prevent reentrancy attacks. Violations arise when a function interacts with external contracts before settling internal state changes or checks. This misordering can expose the contract to potential threats. To adhere to this pattern, first ensure all conditions or checks are satisfied, then update any internal states, and only after these steps, interact with external contracts or addresses. Rearranging operations to this recommended sequence bolsters contract security and aligns with established best practices in the Ethereum community.
Num of instances: 1
Click to show findings
['27']
27: function stake(address rewarded, uint256 amount) external virtual override nonReentrant { // <= FOUND
28: address msgSender = _msgSender();
29:
30: if (amount == type(uint256).max) {
31: amount = IERC20(rewarded).balanceOf(msgSender); // <= FOUND
32: }
33:
34: if (amount == 0) {
35: revert InvalidAmount();
36: }
37:
38: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
39: uint256 currentAccountBalance = accountStorage.balance;
40: address[] memory rewards = accountStorage.enabledRewards.get();
41:
42: for (uint256 i = 0; i < rewards.length; ++i) {
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }
53:
54: uint256 newAccountBalance = currentAccountBalance + amount;
55: accountStorage.balance = newAccountBalance;
56:
57: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
58:
59: pullToken(IERC20(rewarded), msgSender, amount);
60: }Num of instances: 1
[NonCritical-3] In functions which accept an address as a parameter, there should be a zero address check to prevent bugs
In smart contract development, especially with Solidity, it's crucial to validate inputs to functions. When a function accepts an Ethereum address as a parameter, implementing a zero address check (i.e., ensuring the address is not 0x0) is a best practice to prevent potential bugs and vulnerabilities. The zero address (0x0) is a default value and generally indicates an uninitialized or invalid state. Passing the zero address to certain functions can lead to unintended behaviors, like funds getting locked permanently or transactions failing silently. By checking for and rejecting the zero address, developers can ensure that the function operates as intended and interacts only with valid Ethereum addresses. This check enhances the contract's robustness and security.
Num of instances: 18
Click to show findings
['138']
138: function registerReward(
139: address rewarded,
140: address reward,
141: uint48 startEpoch,
142: uint128[] calldata rewardAmounts
143: ) external virtual override nonReentrant ['233']
233: function claimReward(
234: address rewarded,
235: address reward,
236: address recipient,
237: bool forfeitRecentReward
238: ) external virtual override nonReentrant ['261']
261: function enableReward(address rewarded, address reward) external virtual override ['288']
288: function disableReward(address rewarded, address reward, bool forfeitRecentReward) external virtual override ['343']
343: function enabledRewards(
344: address account,
345: address rewarded
346: ) external view virtual override returns (address[] memory) ['354']
354: function balanceOf(address account, address rewarded) external view virtual override returns (uint256) ['362']
362: function rewardAmount(address rewarded, address reward) external view virtual override returns (uint256) ['370']
370: function totalRewardedEligible(address rewarded, address reward) external view virtual override returns (uint256) ['378']
378: function totalRewardRegistered(address rewarded, address reward) external view returns (uint256) ['386']
386: function totalRewardClaimed(address rewarded, address reward) external view returns (uint256) ['395']
395: function rewardAmount(
396: address rewarded,
397: address reward,
398: uint48 epoch
399: ) public view virtual override returns (uint256) ['438']
438: function pullToken(IERC20 token, address from, uint256 amount) internal ['479']
479: function increaseRewardAmounts(
480: address rewarded,
481: address reward,
482: uint48 startEpoch,
483: uint128[] memory amounts
484: ) internal virtual ['504']
504: function claim(address account, address rewarded, address reward, address recipient) internal virtual ['535']
535: function updateRewardInternal(
536: DistributionStorage storage distributionStorage,
537: EarnStorage storage accountEarnStorage,
538: address rewarded,
539: address reward,
540: uint256 currentAccountBalance,
541: bool forfeitRecentReward
542: ) internal virtual ['27']
27: function stake(address rewarded, uint256 amount) external virtual override nonReentrant ['69']
69: function unstake(
70: address rewarded,
71: uint256 amount,
72: address recipient,
73: bool forfeitRecentReward
74: ) external virtual override nonReentrant ['30']
30: function balanceTrackerHook(
31: address account,
32: uint256 newAccountBalance,
33: bool forfeitRecentReward
34: ) external override In instances where a new variable is defined, there is no need to set it to it's default value.
Num of instances: 3
Click to show findings
['163']
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) { // <= FOUND['487']
487: for (uint48 i = 0; i < amounts.length; ++i) { // <= FOUND['40']
40: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND[NonCritical-5] Revert statements within external and public functions can be used to perform DOS attacks
In Solidity, 'revert' statements are used to undo changes and throw an exception when certain conditions are not met. However, in public and external functions, improper use of revert can be exploited for Denial of Service (DoS) attacks. An attacker can intentionally trigger these 'revert' conditions, causing legitimate transactions to consistently fail. For example, if a function relies on specific conditions from user input or contract state, an attacker could manipulate these to continually force reverts, blocking the function's execution. Therefore, it's crucial to design contract logic to handle exceptions properly and avoid scenarios where revert can be predictably triggered by malicious actors. This includes careful input validation and considering alternative design patterns that are less susceptible to such abuses.
Num of instances: 4
Click to show findings
['138']
138: function registerReward(
139: address rewarded,
140: address reward,
141: uint48 startEpoch,
142: uint128[] calldata rewardAmounts
143: ) external virtual override nonReentrant {
144: uint48 epoch = currentEpoch();
145:
146:
147: if (startEpoch == 0) {
148: startEpoch = epoch + 1;
149: }
150:
151:
152: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) {
153: revert InvalidEpoch(); // <= FOUND
154: }
155:
156:
157: if (rewardAmounts.length > MAX_DISTRIBUTION_LENGTH) {
158: revert InvalidDistribution(); // <= FOUND
159: }
160:
161:
162: uint256 totalAmount;
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) {
164: totalAmount += rewardAmounts[i];
165: }
166:
167: if (totalAmount == 0) {
168: revert InvalidAmount(); // <= FOUND
169: }
170:
171:
172: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
173: if (distributionStorage.lastUpdated == 0) {
174: distributionStorage.lastUpdated = uint48(block.timestamp);
175: } else {
176: updateReward(rewarded, reward, address(0));
177: }
178:
179:
180: uint256 totalRegistered = uint256(distributionStorage.totalRegistered) + totalAmount;
181:
182: if (SCALER * totalRegistered > type(uint160).max) {
183: revert AccumulatorOverflow(); // <= FOUND
184: }
185:
186:
187:
188: distributionStorage.totalRegistered = uint128(totalRegistered);
189:
190:
191: increaseRewardAmounts(rewarded, reward, startEpoch, rewardAmounts);
192:
193:
194: address msgSender = _msgSender();
195: pullToken(IERC20(reward), msgSender, totalAmount);
196:
197: emit RewardRegistered(msgSender, rewarded, reward, startEpoch, rewardAmounts);
198: }['261']
261: function enableReward(address rewarded, address reward) external virtual override {
262: address msgSender = _msgSender();
263: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
264: SetStorage storage accountEnabledRewards = accountStorage.enabledRewards;
265:
266: if (accountEnabledRewards.insert(reward)) {
267: if (accountEnabledRewards.numElements > MAX_REWARDS_ENABLED) {
268: revert TooManyRewardsEnabled(); // <= FOUND
269: }
270:
271: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
272: uint256 currentAccountBalance = accountStorage.balance;
273:
274:
275:
276: updateRewardInternal(distributionStorage, accountStorage.earned[reward], rewarded, reward, 0, false);
277:
278: distributionStorage.totalEligible += currentAccountBalance;
279:
280: emit RewardEnabled(msgSender, rewarded, reward);
281: }
282: }['27']
27: function stake(address rewarded, uint256 amount) external virtual override nonReentrant {
28: address msgSender = _msgSender();
29:
30: if (amount == type(uint256).max) {
31: amount = IERC20(rewarded).balanceOf(msgSender);
32: }
33:
34: if (amount == 0) {
35: revert InvalidAmount(); // <= FOUND
36: }
37:
38: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
39: uint256 currentAccountBalance = accountStorage.balance;
40: address[] memory rewards = accountStorage.enabledRewards.get();
41:
42: for (uint256 i = 0; i < rewards.length; ++i) {
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }
53:
54: uint256 newAccountBalance = currentAccountBalance + amount;
55: accountStorage.balance = newAccountBalance;
56:
57: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
58:
59: pullToken(IERC20(rewarded), msgSender, amount);
60: }['69']
69: function unstake(
70: address rewarded,
71: uint256 amount,
72: address recipient,
73: bool forfeitRecentReward
74: ) external virtual override nonReentrant {
75: address msgSender = _msgSender();
76: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
77: uint256 currentAccountBalance = accountStorage.balance;
78:
79: if (amount == type(uint256).max) {
80: amount = currentAccountBalance;
81: }
82:
83: if (amount == 0 || amount > currentAccountBalance) {
84: revert InvalidAmount(); // <= FOUND
85: }
86:
87: address[] memory rewards = accountStorage.enabledRewards.get();
88:
89: for (uint256 i = 0; i < rewards.length; ++i) {
90: address reward = rewards[i];
91: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
92:
93:
94: updateRewardInternal(
95: distributionStorage,
96: accountStorage.earned[reward],
97: rewarded,
98: reward,
99: currentAccountBalance,
100: forfeitRecentReward
101: );
102:
103: distributionStorage.totalEligible -= amount;
104: }
105:
106: uint256 newAccountBalance = currentAccountBalance - amount;
107: accountStorage.balance = newAccountBalance;
108:
109: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
110:
111: pushToken(IERC20(rewarded), recipient, amount);
112: }Consider spreading these lines over multiple lines to aid in readability and the support of VIM users everywhere.
Num of instances: 2
Click to show findings
['15']
15: function registerReward(address rewarded, address reward, uint48 startEpoch, uint128[] calldata rewardAmounts) external; // <= FOUND['20']
20: function earnedReward(address account, address rewarded, address reward, bool forfeitRecentReward) external view returns (uint256); // <= FOUNDIn many cases only some functionality is used from an import. In such cases it makes more sense to use {} to specify what to import and thus save gas whilst improving readability
Num of instances: 1
Using outdated Solidity versions can lead to security risks and inefficiencies. It's recommended to adopt newer versions, ideally the latest, which as of now is 0.8.24. This ensures access to the latest bug fixes, features, and optimizations, particularly crucial for Layer 2 deployments. Regular updates to versions like 0.8.19 or later, up to 0.8.24, enhance contract security and performance.
Num of instances: 1
Try to index as much as three variables in event declarations as this is more gas efficient when done on value type variables (uint, address etc) however not for bytes and string variables
Num of instances: 1
Click to show findings
['101']
101: event BalanceUpdated(address indexed account, address indexed rewarded, uint256 oldBalance, uint256 newBalance); // <= FOUNDContracts should expose all public and external functions through interfaces. This practice ensures a clear and consistent definition of how the contract can be interacted with, promoting better transparency and integration.
Num of instances: 2
Click to show findings
['378']
378: function totalRewardRegistered(address rewarded, address reward) external view returns (uint256) ['386']
386: function totalRewardClaimed(address rewarded, address reward) external view returns (uint256) The following order should be used within contracts
constructor
receive function (if exists)
fallback function (if exists)
external
public
internal
private
Rearrange the contract functions and contructors to fit this ordering
Num of instances: 1
Click to show findings
['14']
14: abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard // <= FOUNDDouble type casting should be avoided in Solidity contracts to prevent unintended consequences and ensure accurate data representation. Performing multiple type casts in succession can lead to unexpected truncation, rounding errors, or loss of precision, potentially compromising the contract's functionality and reliability. Furthermore, double type casting can make the code less readable and harder to maintain, increasing the likelihood of errors and misunderstandings during development and debugging. To ensure precise and consistent data handling, developers should use appropriate data types and avoid unnecessary or excessive type casting, promoting a more robust and dependable contract execution.
Num of instances: 1
Click to show findings
['628']
628:
629:
630: claimable += uint96(uint256(accumulator - accountEarnStorage.accumulator) * currentAccountBalance / SCALER); // <= FOUND[NonCritical-13] A function which defines named returns in it's declaration doesn't need to use return
Refacter the code to assign to the named return variables rather than using a return statement
Num of instances: 1
Click to show findings
['573']
573: function calculateRewards(
574: DistributionStorage storage distributionStorage,
575: EarnStorage storage accountEarnStorage,
576: uint256 currentAccountBalance,
577: bool forfeitRecentReward
578: )
579: internal
580: view
581: virtual
582: returns (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero)
583: {
584:
585: lastUpdated = distributionStorage.lastUpdated;
586: accumulator = distributionStorage.accumulator;
587: claimable = accountEarnStorage.claimable;
588:
589: if (lastUpdated == 0) {
590: return (lastUpdated, accumulator, claimable, 0); // <= FOUND
591: }
592:
593: if (!forfeitRecentReward) {
594:
595: uint48 epochStart = getEpoch(lastUpdated);
596: uint48 epochEnd = currentEpoch() + 1;
597: uint256 delta;
598:
599:
600: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) {
601:
602:
603:
604: unchecked {
605: delta += rewardAmount(distributionStorage, epoch) * timeElapsedInEpoch(epoch, lastUpdated);
606: }
607: }
608:
609:
610:
611: uint256 currentTotalEligible = distributionStorage.totalEligible;
612: if (currentTotalEligible == 0) {
613:
614: deltaAccountZero = uint96(delta / EPOCH_DURATION);
615: } else {
616:
617: unchecked {
618: accumulator += uint160(delta * SCALER / EPOCH_DURATION / currentTotalEligible);
619: }
620: }
621:
622:
623: lastUpdated = uint48(block.timestamp);
624: }
625:
626:
627:
628: claimable += uint96(uint256(accumulator - accountEarnStorage.accumulator) * currentAccountBalance / SCALER);
629: }Putting constants on the left side of a comparison operator like == or < is a best practice known as "Yoda conditions", which can help prevent accidental assignment instead of comparison. In some programming languages, if a variable is mistakenly put on the left with a single = instead of ==, it assigns the constant's value to the variable without any compiler error. However, doing this with the constant on the left would generate an error, as constants cannot be assigned values. Although Solidity's static typing system prevents accidental assignments within conditionals, adopting this practice enhances code readability and consistency, especially when developers are working across multiple languages that support this convention.
Num of instances: 10
Click to show findings
['147']
147: if (startEpoch == 0) // <= FOUND['167']
167: if (totalAmount == 0) // <= FOUND['173']
173: if (distributionStorage.lastUpdated == 0) // <= FOUND['332']
332: if (account == address(0) && deltaAccountZero != 0) // <= FOUND['509']
509: if (amount != 0) // <= FOUND['557']
557: if (deltaAccountZero != 0) // <= FOUND['589']
589: if (lastUpdated == 0) // <= FOUND['34']
34: if (amount == 0) // <= FOUND['83']
83: if (amount == 0 || amount > currentAccountBalance) // <= FOUND['612']
612: if (currentTotalEligible == 0) // <= FOUNDTo maintain readability in code, particularly in Solidity which can involve complex mathematical operations, it is often recommended to limit the number of arithmetic operations to a maximum of 2-3 per line. Too many operations in a single line can make the code difficult to read and understand, increase the likelihood of mistakes, and complicate the process of debugging and reviewing the code. Consider splitting such operations over more than one line, take special care when dealing with division however. Try to limit the number of arithmetic operations to a maximum of 3 per line.
Num of instances: 2
Click to show findings
['618']
618: accumulator += uint160(delta * SCALER / EPOCH_DURATION / currentTotalEligible); // <= FOUND['628']
628:
629:
630: claimable += uint96(uint256(accumulator - accountEarnStorage.accumulator) * currentAccountBalance / SCALER); // <= FOUNDNum of instances: 1
Click to show findings
['642']
642: function timeElapsedInEpoch(uint48 epoch, uint48 lastUpdated) internal view returns (uint256) { // <= FOUND
643:
644: uint256 startTimestamp = getEpochStartTimestamp(epoch);
645: uint256 endTimestamp = getEpochEndTimestamp(epoch);
646:
647:
648: if (block.timestamp >= endTimestamp) {
649:
650:
651:
652:
653: return lastUpdated > startTimestamp ? endTimestamp - lastUpdated : EPOCH_DURATION;
654: } else if (block.timestamp >= startTimestamp && block.timestamp < endTimestamp) {
655:
656:
657:
658: return lastUpdated > startTimestamp ? block.timestamp - lastUpdated : block.timestamp - startTimestamp;
659: } else {
660:
661: return 0;
662: }
663: }Cyclomatic complexity is a software metric used to measure the complexity of a program. It quantifies the number of linearly independent paths through a program's source code, giving an idea of how complex the control flow is. High cyclomatic complexity may indicate a higher risk of defects and can make the code harder to understand, test, and maintain. It often suggests that a function or method is trying to do too much, and a refactor might be needed. By breaking down complex functions into smaller, more focused pieces, you can improve readability, ease of testing, and overall maintainability.
Num of instances: 2
Click to show findings
[]
138: function registerReward(
139: address rewarded,
140: address reward,
141: uint48 startEpoch,
142: uint128[] calldata rewardAmounts
143: ) external virtual override nonReentrant {
144: uint48 epoch = currentEpoch();
145:
146:
147: if (startEpoch == 0) {
148: startEpoch = epoch + 1;
149: }
150:
151:
152: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) {
153: revert InvalidEpoch();
154: }
155:
156:
157: if (rewardAmounts.length > MAX_DISTRIBUTION_LENGTH) {
158: revert InvalidDistribution();
159: }
160:
161:
162: uint256 totalAmount;
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) {
164: totalAmount += rewardAmounts[i];
165: }
166:
167: if (totalAmount == 0) {
168: revert InvalidAmount();
169: }
170:
171:
172: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
173: if (distributionStorage.lastUpdated == 0) {
174: distributionStorage.lastUpdated = uint48(block.timestamp);
175: } else {
176: updateReward(rewarded, reward, address(0));
177: }
178:
179:
180: uint256 totalRegistered = uint256(distributionStorage.totalRegistered) + totalAmount;
181:
182: if (SCALER * totalRegistered > type(uint160).max) {
183: revert AccumulatorOverflow();
184: }
185:
186:
187:
188: distributionStorage.totalRegistered = uint128(totalRegistered);
189:
190:
191: increaseRewardAmounts(rewarded, reward, startEpoch, rewardAmounts);
192:
193:
194: address msgSender = _msgSender();
195: pullToken(IERC20(reward), msgSender, totalAmount);
196:
197: emit RewardRegistered(msgSender, rewarded, reward, startEpoch, rewardAmounts);
198: }[]
573: function calculateRewards(
574: DistributionStorage storage distributionStorage,
575: EarnStorage storage accountEarnStorage,
576: uint256 currentAccountBalance,
577: bool forfeitRecentReward
578: )
579: internal
580: view
581: virtual
582: returns (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero)
583: {
584:
585: lastUpdated = distributionStorage.lastUpdated;
586: accumulator = distributionStorage.accumulator;
587: claimable = accountEarnStorage.claimable;
588:
589: if (lastUpdated == 0) {
590: return (lastUpdated, accumulator, claimable, 0);
591: }
592:
593: if (!forfeitRecentReward) {
594:
595: uint48 epochStart = getEpoch(lastUpdated);
596: uint48 epochEnd = currentEpoch() + 1;
597: uint256 delta;
598:
599:
600: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) {
601:
602:
603:
604: unchecked {
605: delta += rewardAmount(distributionStorage, epoch) * timeElapsedInEpoch(epoch, lastUpdated);
606: }
607: }
608:
609:
610:
611: uint256 currentTotalEligible = distributionStorage.totalEligible;
612: if (currentTotalEligible == 0) {
613:
614: deltaAccountZero = uint96(delta / EPOCH_DURATION);
615: } else {
616:
617: unchecked {
618: accumulator += uint160(delta * SCALER / EPOCH_DURATION / currentTotalEligible);
619: }
620: }
621:
622:
623: lastUpdated = uint48(block.timestamp);
624: }
625:
626:
627:
628: claimable += uint96(uint256(accumulator - accountEarnStorage.accumulator) * currentAccountBalance / SCALER);
629: }[NonCritical-18] Events may be emitted out of order due to code not follow the best practice of check-effects-interaction
The "check-effects-interaction" pattern also impacts event ordering. When a contract doesn't adhere to this pattern, events might be emitted in a sequence that doesn't reflect the actual logical flow of operations. This can cause confusion during event tracking, potentially leading to erroneous off-chain interpretations. To rectify this, always ensure that checks are performed first, state modifications come next, and interactions with external contracts or addresses are done last. This will ensure events are emitted in a logical, consistent manner, providing a clear and accurate chronological record of on-chain actions for off-chain systems and observers.
Num of instances: 1
Click to show findings
['27']
27: function stake(address rewarded, uint256 amount) external virtual override nonReentrant { // <= FOUND
28: address msgSender = _msgSender();
29:
30: if (amount == type(uint256).max) {
31: amount = IERC20(rewarded).balanceOf(msgSender); // <= FOUND
32: }
33:
34: if (amount == 0) {
35: revert InvalidAmount();
36: }
37:
38: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
39: uint256 currentAccountBalance = accountStorage.balance;
40: address[] memory rewards = accountStorage.enabledRewards.get();
41:
42: for (uint256 i = 0; i < rewards.length; ++i) {
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }
53:
54: uint256 newAccountBalance = currentAccountBalance + amount;
55: accountStorage.balance = newAccountBalance;
56:
57: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance); // <= FOUND
58:
59: pullToken(IERC20(rewarded), msgSender, amount);
60: }Sensitive setter functions in smart contracts often alter critical state variables. Without events emitted in these functions, external observers or dApps cannot easily track or react to these state changes. Missing events can obscure contract activity, hampering transparency and making integration more challenging. To resolve this, incorporate appropriate event emissions within these functions. Events offer an efficient way to log crucial changes, aiding in real-time tracking and post-transaction verification.
Num of instances: 2
Click to show findings
['206']
206: function updateReward(address rewarded, address reward, address recipient) public virtual override { // <= FOUND
207: address msgSender = _msgSender();
208:
209:
210: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
211: uint256 currentAccountBalance = accountStorage.enabledRewards.contains(reward) ? accountStorage.balance : 0;
212:
213: updateRewardInternal(
214: distributions[rewarded][reward],
215: accountStorage.earned[reward],
216: rewarded,
217: reward,
218: currentAccountBalance,
219: false
220: );
221:
222: if (recipient != address(0)) {
223: claim(address(0), rewarded, reward, recipient);
224: }
225: }['535']
535: function updateRewardInternal( // <= FOUND
536: DistributionStorage storage distributionStorage,
537: EarnStorage storage accountEarnStorage,
538: address rewarded,
539: address reward,
540: uint256 currentAccountBalance,
541: bool forfeitRecentReward
542: ) internal virtual {
543: (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) =
544: calculateRewards(distributionStorage, accountEarnStorage, currentAccountBalance, forfeitRecentReward);
545:
546:
547: distributionStorage.lastUpdated = lastUpdated;
548: distributionStorage.accumulator = accumulator;
549:
550:
551:
552: accountEarnStorage.claimable = claimable;
553: accountEarnStorage.accumulator = uint160(accumulator);
554:
555:
556:
557: if (deltaAccountZero != 0) {
558: unchecked {
559: accounts[address(0)][rewarded].earned[reward].claimable += deltaAccountZero;
560: }
561: }
562: }Function parameters in Solidity are passed by value, meaning they are essentially local copies. Mutating them can lead to confusion and errors because the changes don't persist outside the function. By keeping function parameters immutable, you ensure clarity in code behavior, preventing unintended side-effects. If you need to modify a value based on a parameter, use a local variable inside the function, leaving the original parameter unaltered. By adhering to this practice, you maintain a clear distinction between input data and the internal processing logic, improving code readability and reducing the potential for bugs.
Num of instances: 3
Click to show findings
['138']
138: function registerReward(
139: address rewarded,
140: address reward,
141: uint48 startEpoch,
142: uint128[] calldata rewardAmounts
143: ) external virtual override nonReentrant {
144: uint48 epoch = currentEpoch();
145:
146:
147: if (startEpoch == 0) {
148: startEpoch = epoch + 1; // <= FOUND
149: }
150:
151:
152: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) {
153: revert InvalidEpoch();
154: }
155:
156:
157: if (rewardAmounts.length > MAX_DISTRIBUTION_LENGTH) {
158: revert InvalidDistribution();
159: }
160:
161:
162: uint256 totalAmount;
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) {
164: totalAmount += rewardAmounts[i];
165: }
166:
167: if (totalAmount == 0) {
168: revert InvalidAmount();
169: }
170:
171:
172: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
173: if (distributionStorage.lastUpdated == 0) {
174: distributionStorage.lastUpdated = uint48(block.timestamp);
175: } else {
176: updateReward(rewarded, reward, address(0));
177: }
178:
179:
180: uint256 totalRegistered = uint256(distributionStorage.totalRegistered) + totalAmount;
181:
182: if (SCALER * totalRegistered > type(uint160).max) {
183: revert AccumulatorOverflow();
184: }
185:
186:
187:
188: distributionStorage.totalRegistered = uint128(totalRegistered);
189:
190:
191: increaseRewardAmounts(rewarded, reward, startEpoch, rewardAmounts);
192:
193:
194: address msgSender = _msgSender();
195: pullToken(IERC20(reward), msgSender, totalAmount);
196:
197: emit RewardRegistered(msgSender, rewarded, reward, startEpoch, rewardAmounts);
198: }['27']
27: function stake(address rewarded, uint256 amount) external virtual override nonReentrant { // <= FOUND
28: address msgSender = _msgSender();
29:
30: if (amount == type(uint256).max) {
31: amount = IERC20(rewarded).balanceOf(msgSender); // <= FOUND
32: }
33:
34: if (amount == 0) {
35: revert InvalidAmount();
36: }
37:
38: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
39: uint256 currentAccountBalance = accountStorage.balance;
40: address[] memory rewards = accountStorage.enabledRewards.get();
41:
42: for (uint256 i = 0; i < rewards.length; ++i) {
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }
53:
54: uint256 newAccountBalance = currentAccountBalance + amount;
55: accountStorage.balance = newAccountBalance;
56:
57: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
58:
59: pullToken(IERC20(rewarded), msgSender, amount);
60: }['69']
69: function unstake(
70: address rewarded,
71: uint256 amount,
72: address recipient,
73: bool forfeitRecentReward
74: ) external virtual override nonReentrant {
75: address msgSender = _msgSender();
76: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
77: uint256 currentAccountBalance = accountStorage.balance;
78:
79: if (amount == type(uint256).max) {
80: amount = currentAccountBalance; // <= FOUND
81: }
82:
83: if (amount == 0 || amount > currentAccountBalance) {
84: revert InvalidAmount();
85: }
86:
87: address[] memory rewards = accountStorage.enabledRewards.get();
88:
89: for (uint256 i = 0; i < rewards.length; ++i) {
90: address reward = rewards[i];
91: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
92:
93:
94: updateRewardInternal(
95: distributionStorage,
96: accountStorage.earned[reward],
97: rewarded,
98: reward,
99: currentAccountBalance,
100: forfeitRecentReward
101: );
102:
103: distributionStorage.totalEligible -= amount;
104: }
105:
106: uint256 newAccountBalance = currentAccountBalance - amount;
107: accountStorage.balance = newAccountBalance;
108:
109: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
110:
111: pushToken(IERC20(rewarded), recipient, amount);
112: }Emitting an event in a constructor of a smart contract provides transparency and traceability in blockchain applications. This event logs the contract’s creation, aiding in monitoring and verifying contract deployment. Although constructors are executed only once, the emitted event ensures the contract's initialization is recorded on the blockchain.
Num of instances: 1
Click to show findings
['125']
125: constructor(address _evc, uint48 _epochDuration) EVCUtil(_evc) { // <= FOUND
126: if (_epochDuration < MIN_EPOCH_DURATION || _epochDuration > MAX_EPOCH_DURATION) {
127: revert InvalidEpoch();
128: }
129:
130: EPOCH_DURATION = _epochDuration;
131: }In Solidity, custom errors with parameters offer a gas-efficient way to convey detailed information about issues encountered during contract execution. Unlike revert messages, which are strings consuming more gas, custom errors defined with parameters allow developers to specify types and details of errors succinctly. This method enhances debugging, provides clearer insights into contract failures, and improves the developer's and end-user's understanding of what went wrong, all while optimizing for gas usage and maintaining contract efficiency.
Num of instances: 12
Click to show findings
['104']
104:
105: error InvalidEpoch(); // <= FOUND['108']
108:
110: error InvalidAmount(); // <= FOUND['111']
111:
112: error InvalidDistribution(); // <= FOUND['114']
114:
115: error AccumulatorOverflow(); // <= FOUND['117']
117:
118: error TooManyRewardsEnabled(); // <= FOUND['120']
120:
121: error InvalidRecipient(); // <= FOUND['104']
104: error InvalidEpoch(); // <= FOUND['108']
108: error InvalidAmount(); // <= FOUND['111']
111: error InvalidDistribution(); // <= FOUND['114']
114: error AccumulatorOverflow(); // <= FOUND['117']
117: error TooManyRewardsEnabled(); // <= FOUND['120']
120: error InvalidRecipient(); // <= FOUNDAvoiding arithmetic directly within array indices, like test[i + 2], is recommended to prevent errors such as index out of bounds or incorrect data access. This practice enhances code readability and maintainability. Instead, calculate the index beforehand, store it in a variable, and then use that variable to access the array element. This approach reduces mistakes, especially in complex loops or when handling dynamic data, ensuring that each access is clear and verifiable. It's about keeping code clean and understandable, minimizing potential bugs.
Num of instances: 2
Click to show findings
['471']
471: return distributionStorage.amounts[epoch / EPOCHS_PER_SLOT][epoch % EPOCHS_PER_SLOT]; // <= FOUND['491']
491: storageAmounts[epoch / EPOCHS_PER_SLOT][epoch % EPOCHS_PER_SLOT] += amounts[i]; // <= FOUNDERC777 is an advanced token standard that introduces hooks, allowing operators to execute additional logic during transfers. While this feature offers greater flexibility, it also opens up the possibility of reentrancy attacks. Specifically, when tokens are sent, the receiving contract's tokensReceived hook gets called, and this external call can execute arbitrary code. An attacker can exploit this feature to re-enter the original function, potentially leading to double-spending or other types of financial manipulation.
To mitigate reentrancy risks with ERC777, it's crucial to adopt established security measures, such as utilizing reentrancy guards or following the check-effects-interactions pattern. Some developers opt to stick with the simpler ERC20 standard, which does not have these hooks, to minimize this risk. If you do choose to use ERC777, extreme caution and thorough auditing are advised to secure against potential reentrancy vulnerabilities.
Num of instances: 1
Click to show findings
['438']
438: function pullToken(IERC20 token, address from, uint256 amount) internal { // <= FOUND
439: uint256 preBalance = token.balanceOf(address(this));
440: token.safeTransferFrom(from, address(this), amount); // <= FOUND
441:
442: if (token.balanceOf(address(this)) - preBalance != amount) {
443: revert InvalidAmount();
444: }
445: }Num of instances: 2
Click to show findings
['535']
535: function updateRewardInternal(
536: DistributionStorage storage distributionStorage,
537: EarnStorage storage accountEarnStorage,
538: address rewarded,
539: address reward,
540: uint256 currentAccountBalance,
541: bool forfeitRecentReward
542: ) internal virtual {
543: (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) =
544: calculateRewards(distributionStorage, accountEarnStorage, currentAccountBalance, forfeitRecentReward);
545:
546:
547: distributionStorage.lastUpdated = lastUpdated; // <= FOUND
548: distributionStorage.accumulator = accumulator; // <= FOUND
549:
550:
551:
552: accountEarnStorage.claimable = claimable; // <= FOUND
553: accountEarnStorage.accumulator = uint160(accumulator);
554:
555:
556:
557: if (deltaAccountZero != 0) {
558: unchecked {
559: accounts[address(0)][rewarded].earned[reward].claimable += deltaAccountZero;
560: }
561: }
562: }['573']
573: function calculateRewards(
574: DistributionStorage storage distributionStorage,
575: EarnStorage storage accountEarnStorage,
576: uint256 currentAccountBalance,
577: bool forfeitRecentReward
578: )
579: internal
580: view
581: virtual
582: returns (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero)
583: {
584:
585: lastUpdated = distributionStorage.lastUpdated; // <= FOUND
586: accumulator = distributionStorage.accumulator; // <= FOUND
587: claimable = accountEarnStorage.claimable; // <= FOUND
588:
589: if (lastUpdated == 0) {
590: return (lastUpdated, accumulator, claimable, 0);
591: }
592:
593: if (!forfeitRecentReward) {
594:
595: uint48 epochStart = getEpoch(lastUpdated);
596: uint48 epochEnd = currentEpoch() + 1;
597: uint256 delta;
598:
599:
600: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) {
601:
602:
603:
604: unchecked {
605: delta += rewardAmount(distributionStorage, epoch) * timeElapsedInEpoch(epoch, lastUpdated);
606: }
607: }
608:
609:
610:
611: uint256 currentTotalEligible = distributionStorage.totalEligible;
612: if (currentTotalEligible == 0) {
613:
614: deltaAccountZero = uint96(delta / EPOCH_DURATION);
615: } else {
616:
617: unchecked {
618: accumulator += uint160(delta * SCALER / EPOCH_DURATION / currentTotalEligible);
619: }
620: }
621:
622:
623: lastUpdated = uint48(block.timestamp);
624: }
625:
626:
627:
628: claimable += uint96(uint256(accumulator - accountEarnStorage.accumulator) * currentAccountBalance / SCALER);
629: }Num of instances: 15
Click to show findings
['317']
317: function earnedReward(
318: address account,
319: address rewarded,
320: address reward,
321: bool forfeitRecentReward
322: ) external view virtual override returns (uint256) {
323:
324: AccountStorage storage accountStorage = accounts[account][rewarded];
325: uint256 currentAccountBalance = accountStorage.enabledRewards.contains(reward) ? accountStorage.balance : 0;
326:
327: (,, uint96 claimable, uint96 deltaAccountZero) = calculateRewards(
328: distributions[rewarded][reward], accountStorage.earned[reward], currentAccountBalance, forfeitRecentReward
329: );
330:
331:
332: if (account == address(0) && deltaAccountZero != 0) {
333: return claimable + deltaAccountZero; // <= FOUND
334: }
335:
336: return claimable; // <= FOUND
337: }['343']
343: function enabledRewards(
344: address account,
345: address rewarded
346: ) external view virtual override returns (address[] memory) {
347: return accounts[account][rewarded].enabledRewards.get(); // <= FOUND
348: }['354']
354: function balanceOf(address account, address rewarded) external view virtual override returns (uint256) {
355: return accounts[account][rewarded].balance; // <= FOUND
356: }['362']
362: function rewardAmount(address rewarded, address reward) external view virtual override returns (uint256) {
363: return rewardAmount(rewarded, reward, currentEpoch()); // <= FOUND
364: }['370']
370: function totalRewardedEligible(address rewarded, address reward) external view virtual override returns (uint256) {
371: return distributions[rewarded][reward].totalEligible; // <= FOUND
372: }['378']
378: function totalRewardRegistered(address rewarded, address reward) external view returns (uint256) {
379: return distributions[rewarded][reward].totalRegistered; // <= FOUND
380: }['386']
386: function totalRewardClaimed(address rewarded, address reward) external view returns (uint256) {
387: return distributions[rewarded][reward].totalClaimed; // <= FOUND
388: }['395']
395: function rewardAmount(
396: address rewarded,
397: address reward,
398: uint48 epoch
399: ) public view virtual override returns (uint256) {
400: return rewardAmount(distributions[rewarded][reward], epoch); // <= FOUND
401: }['405']
405: function currentEpoch() public view override returns (uint48) {
406: return getEpoch(uint48(block.timestamp)); // <= FOUND
407: }['412']
412: function getEpoch(uint48 timestamp) public view override returns (uint48) {
413: return uint48(timestamp / EPOCH_DURATION); // <= FOUND
414: }['419']
419: function getEpochStartTimestamp(uint48 epoch) public view override returns (uint48) {
420: return uint48(epoch * EPOCH_DURATION); // <= FOUND
421: }['427']
427: function getEpochEndTimestamp(uint48 epoch) public view override returns (uint48) {
428: return uint48(getEpochStartTimestamp(epoch) + EPOCH_DURATION); // <= FOUND
429: }['467']
467: function rewardAmount(
468: DistributionStorage storage distributionStorage,
469: uint48 epoch
470: ) internal view returns (uint256) {
471: return distributionStorage.amounts[epoch / EPOCHS_PER_SLOT][epoch % EPOCHS_PER_SLOT]; // <= FOUND
472: }['573']
573: function calculateRewards(
574: DistributionStorage storage distributionStorage,
575: EarnStorage storage accountEarnStorage,
576: uint256 currentAccountBalance,
577: bool forfeitRecentReward
578: )
579: internal
580: view
581: virtual
582: returns (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero)
583: {
584:
585: lastUpdated = distributionStorage.lastUpdated;
586: accumulator = distributionStorage.accumulator;
587: claimable = accountEarnStorage.claimable;
588:
589: if (lastUpdated == 0) {
590: return (lastUpdated, accumulator, claimable, 0); // <= FOUND
591: }
592:
593: if (!forfeitRecentReward) {
594:
595: uint48 epochStart = getEpoch(lastUpdated);
596: uint48 epochEnd = currentEpoch() + 1;
597: uint256 delta;
598:
599:
600: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) {
601:
602:
603:
604: unchecked {
605: delta += rewardAmount(distributionStorage, epoch) * timeElapsedInEpoch(epoch, lastUpdated);
606: }
607: }
608:
609:
610:
611: uint256 currentTotalEligible = distributionStorage.totalEligible;
612: if (currentTotalEligible == 0) {
613:
614: deltaAccountZero = uint96(delta / EPOCH_DURATION);
615: } else {
616:
617: unchecked {
618: accumulator += uint160(delta * SCALER / EPOCH_DURATION / currentTotalEligible);
619: }
620: }
621:
622:
623: lastUpdated = uint48(block.timestamp);
624: }
625:
626:
627:
628: claimable += uint96(uint256(accumulator - accountEarnStorage.accumulator) * currentAccountBalance / SCALER);
629: }['642']
642: function timeElapsedInEpoch(uint48 epoch, uint48 lastUpdated) internal view returns (uint256) {
643:
644: uint256 startTimestamp = getEpochStartTimestamp(epoch);
645: uint256 endTimestamp = getEpochEndTimestamp(epoch);
646:
647:
648: if (block.timestamp >= endTimestamp) {
649:
650:
651:
652:
653: return lastUpdated > startTimestamp ? endTimestamp - lastUpdated : EPOCH_DURATION; // <= FOUND
654: } else if (block.timestamp >= startTimestamp && block.timestamp < endTimestamp) {
655:
656:
657:
658: return lastUpdated > startTimestamp ? block.timestamp - lastUpdated : block.timestamp - startTimestamp; // <= FOUND
659: } else {
660:
661: return 0; // <= FOUND
662: }
663: }When using a smaller int/uint type it first needs to be converted to it's 258 bit counterpart to be operated, this increases the gass cost and thus should be avoided. However it does make sense to use smaller int/uint values within structs provided you pack the struct properly.
Num of instances: 35
Click to show findings
['49']
49:
50: uint48 lastUpdated; // <= FOUND['125']
125:
128: constructor(address _evc, uint48 _epochDuration) EVCUtil(_evc) { // <= FOUND['138']
138:
143: function registerReward(
144: address rewarded,
145: address reward,
146: uint48 startEpoch, // <= FOUND
147: uint128[] calldata rewardAmounts
148: ) external virtual override nonReentrant {['144']
144: uint48 epoch = currentEpoch(); // <= FOUND['395']
395:
400: function rewardAmount(
401: address rewarded,
402: address reward,
403: uint48 epoch // <= FOUND
404: ) public view virtual override returns (uint256) {['412']
412:
415: function getEpoch(uint48 timestamp) public view override returns (uint48) { // <= FOUND['419']
419:
422: function getEpochStartTimestamp(uint48 epoch) public view override returns (uint48) { // <= FOUND['427']
427:
431: function getEpochEndTimestamp(uint48 epoch) public view override returns (uint48) { // <= FOUND['467']
467:
471: function rewardAmount(
472: DistributionStorage storage distributionStorage,
473: uint48 epoch // <= FOUND
474: ) internal view returns (uint256) {['479']
479:
484: function increaseRewardAmounts(
485: address rewarded,
486: address reward,
487: uint48 startEpoch, // <= FOUND
488: uint128[] memory amounts
489: ) internal virtual {['487']
487: for (uint48 i = 0; i < amounts.length; ++i) { // <= FOUND['490']
490: uint48 epoch = startEpoch + i; // <= FOUND['543']
543: (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) = // <= FOUND
544: calculateRewards(distributionStorage, accountEarnStorage, currentAccountBalance, forfeitRecentReward);['573']
573:
582: function calculateRewards(
583: DistributionStorage storage distributionStorage,
584: EarnStorage storage accountEarnStorage,
585: uint256 currentAccountBalance,
586: bool forfeitRecentReward
587: )
588: internal
589: view
590: virtual
591: returns (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) // <= FOUND
592: {['595']
595:
596: uint48 epochStart = getEpoch(lastUpdated); // <= FOUND['596']
596: uint48 epochEnd = currentEpoch() + 1; // <= FOUND['600']
600:
601: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) { // <= FOUND['642']
642:
653: function timeElapsedInEpoch(uint48 epoch, uint48 lastUpdated) internal view returns (uint256) { // <= FOUND['15']
15: function registerReward(address rewarded, address reward, uint48 startEpoch, uint128[] calldata rewardAmounts) external; // <= FOUND['27']
27: function rewardAmount(address rewarded, address reward, uint48 epoch) external view returns (uint256); // <= FOUND['29']
29: function getEpoch(uint48 timestamp) external view returns (uint48); // <= FOUND['30']
30: function getEpochStartTimestamp(uint48 epoch) external view returns (uint48); // <= FOUND['31']
31: function getEpochEndTimestamp(uint48 epoch) external view returns (uint48); // <= FOUND['21']
21:
24: constructor(address evc, uint48 periodDuration) BaseRewardStreams(evc, periodDuration) {} // <= FOUND['24']
24:
27: constructor(address evc, uint48 epochDuration) BaseRewardStreams(evc, epochDuration) {} // <= FOUND['65']
65:
66: uint96 claimable; // <= FOUND['327']
327: (,, uint96 claimable, uint96 deltaAccountZero) = calculateRewards( // <= FOUND
328: distributions[rewarded][reward], accountStorage.earned[reward], currentAccountBalance, forfeitRecentReward
329: );['55']
55:
56: uint128 totalRegistered; // <= FOUND['57']
57:
58: uint128 totalClaimed; // <= FOUND['506']
506: uint128 amount = accountEarned.claimable; // <= FOUND['511']
511: uint128 totalRegistered = distributionStorage.totalRegistered; // <= FOUND['512']
512: uint128 totalClaimed = distributionStorage.totalClaimed; // <= FOUND['513']
513: uint128 newTotalClaimed = totalClaimed + amount; // <= FOUND['67']
67:
68: uint160 accumulator; // <= FOUND['51']
51:
52: uint208 accumulator; // <= FOUNDUsing .delete is better than resetting a Solidity variable to its default value manually because it frees up storage space on the Ethereum blockchain, resulting in gas cost savings.
Num of instances: 1
Making function calls or external calls within loops in Solidity can lead to inefficient gas usage, potential bottlenecks, and increased vulnerability to attacks. Each function call or external call consumes gas, and when executed within a loop, the gas cost multiplies, potentially causing the transaction to run out of gas or exceed block gas limits. This can result in transaction failure or unpredictable behavior.
Num of instances: 6
Click to show findings
['600']
600: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) {
601:
602:
603:
604: unchecked {
605: delta += rewardAmount(distributionStorage, epoch) * timeElapsedInEpoch(epoch, lastUpdated); // <= FOUND
606: }
607: }['600']
600: for (uint48 epoch = epochStart; epoch < epochEnd; ++epoch) {
601:
602:
603:
604: unchecked {
605: delta += rewardAmount(distributionStorage, epoch) * timeElapsedInEpoch(epoch, lastUpdated); // <= FOUND
606: }
607: }['40']
40: for (uint256 i = 0; i < rewards.length; ++i) {
41: address reward = rewards[i];
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal( // <= FOUND
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }['42']
42: for (uint256 i = 0; i < rewards.length; ++i) {
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal( // <= FOUND
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }['89']
89: for (uint256 i = 0; i < rewards.length; ++i) {
90: address reward = rewards[i];
91: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
92:
93:
94: updateRewardInternal( // <= FOUND
95: distributionStorage,
96: accountStorage.earned[reward],
97: rewarded,
98: reward,
99: currentAccountBalance,
100: forfeitRecentReward
101: );
102:
103: distributionStorage.totalEligible -= amount;
104: }['40']
40: for (uint256 i = 0; i < rewards.length; ++i) {
41: address reward = rewards[i];
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal( // <= FOUND
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }[Gas-6] For loops in public or external functions should be avoided due to high gas costs and possible DOS
In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly
Num of instances: 4
Click to show findings
['138']
138: function registerReward(
139: address rewarded,
140: address reward,
141: uint48 startEpoch,
142: uint128[] calldata rewardAmounts
143: ) external virtual override nonReentrant {
144: uint48 epoch = currentEpoch();
145:
146:
147: if (startEpoch == 0) {
148: startEpoch = epoch + 1;
149: }
150:
151:
152: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) {
153: revert InvalidEpoch();
154: }
155:
156:
157: if (rewardAmounts.length > MAX_DISTRIBUTION_LENGTH) {
158: revert InvalidDistribution();
159: }
160:
161:
162: uint256 totalAmount;
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) { // <= FOUND
164: totalAmount += rewardAmounts[i];
165: }
166:
167: if (totalAmount == 0) {
168: revert InvalidAmount();
169: }
170:
171:
172: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
173: if (distributionStorage.lastUpdated == 0) {
174: distributionStorage.lastUpdated = uint48(block.timestamp);
175: } else {
176: updateReward(rewarded, reward, address(0));
177: }
178:
179:
180: uint256 totalRegistered = uint256(distributionStorage.totalRegistered) + totalAmount;
181:
182: if (SCALER * totalRegistered > type(uint160).max) {
183: revert AccumulatorOverflow();
184: }
185:
186:
187:
188: distributionStorage.totalRegistered = uint128(totalRegistered);
189:
190:
191: increaseRewardAmounts(rewarded, reward, startEpoch, rewardAmounts);
192:
193:
194: address msgSender = _msgSender();
195: pullToken(IERC20(reward), msgSender, totalAmount);
196:
197: emit RewardRegistered(msgSender, rewarded, reward, startEpoch, rewardAmounts);
198: }['27']
27: function stake(address rewarded, uint256 amount) external virtual override nonReentrant {
28: address msgSender = _msgSender();
29:
30: if (amount == type(uint256).max) {
31: amount = IERC20(rewarded).balanceOf(msgSender);
32: }
33:
34: if (amount == 0) {
35: revert InvalidAmount();
36: }
37:
38: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
39: uint256 currentAccountBalance = accountStorage.balance;
40: address[] memory rewards = accountStorage.enabledRewards.get();
41:
42: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }
53:
54: uint256 newAccountBalance = currentAccountBalance + amount;
55: accountStorage.balance = newAccountBalance;
56:
57: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
58:
59: pullToken(IERC20(rewarded), msgSender, amount);
60: }['69']
69: function unstake(
70: address rewarded,
71: uint256 amount,
72: address recipient,
73: bool forfeitRecentReward
74: ) external virtual override nonReentrant {
75: address msgSender = _msgSender();
76: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
77: uint256 currentAccountBalance = accountStorage.balance;
78:
79: if (amount == type(uint256).max) {
80: amount = currentAccountBalance;
81: }
82:
83: if (amount == 0 || amount > currentAccountBalance) {
84: revert InvalidAmount();
85: }
86:
87: address[] memory rewards = accountStorage.enabledRewards.get();
88:
89: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
90: address reward = rewards[i];
91: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
92:
93:
94: updateRewardInternal(
95: distributionStorage,
96: accountStorage.earned[reward],
97: rewarded,
98: reward,
99: currentAccountBalance,
100: forfeitRecentReward
101: );
102:
103: distributionStorage.totalEligible -= amount;
104: }
105:
106: uint256 newAccountBalance = currentAccountBalance - amount;
107: accountStorage.balance = newAccountBalance;
108:
109: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance);
110:
111: pushToken(IERC20(rewarded), recipient, amount);
112: }['30']
30: function balanceTrackerHook(
31: address account,
32: uint256 newAccountBalance,
33: bool forfeitRecentReward
34: ) external override {
35: address rewarded = msg.sender;
36: AccountStorage storage accountStorage = accounts[account][rewarded];
37: uint256 currentAccountBalance = accountStorage.balance;
38: address[] memory rewards = accountStorage.enabledRewards.get();
39:
40: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
41: address reward = rewards[i];
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal(
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }
57:
58: accountStorage.balance = newAccountBalance;
59:
60: emit BalanceUpdated(account, rewarded, currentAccountBalance, newAccountBalance);
61: }Using assembly for address comparisons in Solidity can save gas because it allows for more direct access to the Ethereum Virtual Machine (EVM), reducing the overhead of higher-level operations. Solidity's high-level abstraction simplifies coding but can introduce additional gas costs. Using assembly for simple operations like address comparisons can be more gas-efficient.
Num of instances: 3
Click to show findings
['206']
206: function updateReward(address rewarded, address reward, address recipient) public virtual override {
207: address msgSender = _msgSender();
208:
209:
210: AccountStorage storage accountStorage = accounts[msgSender][rewarded];
211: uint256 currentAccountBalance = accountStorage.enabledRewards.contains(reward) ? accountStorage.balance : 0;
212:
213: updateRewardInternal(
214: distributions[rewarded][reward],
215: accountStorage.earned[reward],
216: rewarded,
217: reward,
218: currentAccountBalance,
219: false
220: );
221:
222: if (recipient != address(0)) { // <= FOUND
223: claim(address(0), rewarded, reward, recipient);
224: }
225: }['317']
317: function earnedReward(
318: address account,
319: address rewarded,
320: address reward,
321: bool forfeitRecentReward
322: ) external view virtual override returns (uint256) {
323:
324: AccountStorage storage accountStorage = accounts[account][rewarded];
325: uint256 currentAccountBalance = accountStorage.enabledRewards.contains(reward) ? accountStorage.balance : 0;
326:
327: (,, uint96 claimable, uint96 deltaAccountZero) = calculateRewards(
328: distributions[rewarded][reward], accountStorage.earned[reward], currentAccountBalance, forfeitRecentReward
329: );
330:
331:
332: if (account == address(0) && deltaAccountZero != 0) { // <= FOUND
333: return claimable + deltaAccountZero;
334: }
335:
336: return claimable;
337: }['453']
453: function pushToken(IERC20 token, address to, uint256 amount) internal {
454: address owner = evc.getAccountOwner(to);
455:
456: if (to == address(0) || (owner != address(0) && owner != to)) { // <= FOUND
457: revert InvalidRecipient();
458: }
459:
460: IERC20(token).safeTransfer(to, amount);
461: }[Gas-8] Divisions which do not divide by -X cannot overflow or underflow so such operations can be unchecked to save gas
Make such found divisions are unchecked when ensured it is safe to do so
Num of instances: 1
Click to show findings
['412']
412: function getEpoch(uint48 timestamp) public view override returns (uint48) { // <= FOUND
413: return uint48(timestamp / EPOCH_DURATION); // <= FOUND
414: }From a gas efficiency perspective, using _msgSender() in a contract not intended to support EIP-2771 could add unnecessary overhead. The _msgSender() function includes checks to determine if the transaction was forwarded, which involves extra function calls that consume more gas than a simple msg.sender.
If a contract doesn't require EIP-2771 meta-transaction support, using msg.sender directly is more gas efficient. msg.sender is a globally accessible variable in Solidity that doesn't require an extra function call, making it a less costly choice.
In the context of Ethereum, where every operation has a gas cost, it's crucial to eliminate unnecessary computations to optimize contract execution and minimize transaction fees. Therefore, if EIP-2771 support isn't necessary, it's recommended to use msg.sender instead of _msgSender().
Num of instances: 2
Click to show findings
['194']
194:
195: address msgSender = _msgSender(); // <= FOUND['194']
194: address msgSender = _msgSender(); // <= FOUNDNested mappings and multi-dimensional arrays in Solidity operate through a process of double hashing, wherein the original storage slot and the first key are concatenated and hashed, and then this hash is again concatenated with the second key and hashed. This process can be quite gas expensive due to the double-hashing operation and subsequent storage operation (sstore).
A possible optimization involves manually concatenating the keys followed by a single hash operation and an sstore. However, this technique introduces the risk of storage collision, especially when there are other nested hash maps in the contract that use the same key types. Because Solidity is unaware of the number and structure of nested hash maps in a contract, it follows a conservative approach in computing the storage slot to avoid possible collisions.
OpenZeppelin's EnumerableSet provides a potential solution to this problem. It creates a data structure that combines the benefits of set operations with the ability to enumerate stored elements, which is not natively available in Solidity. EnumerableSet handles the element uniqueness internally and can therefore provide a more gas-efficient and collision-resistant alternative to nested mappings or multi-dimensional arrays in certain scenarios.
Num of instances: 2
Click to show findings
['81']
81: mapping(address rewarded => mapping(address reward => DistributionStorage)) internal distributions; // <= FOUND['84']
84: mapping(address account => mapping(address rewarded => AccountStorage)) internal accounts; // <= FOUNDWith the use of inline assembly in Solidity, we can take advantage of low-level features like scratch space and the free memory pointer, offering more gas-efficient ways of emitting events. The scratch space is a certain area of memory where we can temporarily store data, and the free memory pointer indicates the next available memory slot. Using these, we can efficiently assemble event data without incurring additional memory expansion costs. However, safety is paramount: to avoid overwriting or leakage, we must cache the free memory pointer before use and restore it afterward, ensuring that it points to the correct memory location post-operation.
Num of instances: 6
Click to show findings
['197']
197: emit RewardRegistered(msgSender, rewarded, reward, startEpoch, rewardAmounts); // <= FOUND['280']
280: emit RewardEnabled(msgSender, rewarded, reward); // <= FOUND['307']
307: emit RewardDisabled(msgSender, rewarded, reward); // <= FOUND['521']
521: emit RewardClaimed(account, rewarded, reward, amount); // <= FOUND['57']
57: emit BalanceUpdated(msgSender, rewarded, currentAccountBalance, newAccountBalance); // <= FOUND['60']
60: emit BalanceUpdated(account, rewarded, currentAccountBalance, newAccountBalance); // <= FOUNDThe following OpenZeppelin imports have a Solady equivalent, as such they can be used to save GAS as Solady modules have been specifically designed to be as GAS efficient as possible
Num of instances: 1
Click to show findings
['6']
6: import {SafeERC20, IERC20} from "openzeppelin-contracts/token/ERC20/utils/SafeERC20.sol"; // <= FOUNDUsing private visibility for constants and immutables in Solidity instead of public can save gas. This is because private elements are not included in the contract's ABI, reducing the deployment and interaction costs. To achieve better efficiency, it is recommended to use private visibility when external access is not needed.
Num of instances: 3
Click to show findings
['23']
23: uint256 public constant MAX_EPOCHS_AHEAD = 5; // <= FOUND['26']
26: uint256 public constant MAX_DISTRIBUTION_LENGTH = 25; // <= FOUND['29']
29: uint256 public constant MAX_REWARDS_ENABLED = 5; // <= FOUNDIn Solidity, the unchecked block allows arithmetic operations to not revert on overflow. Without using unchecked in loops, extra gas is consumed due to overflow checks. If it's certain that overflows won't occur within the loop, using unchecked can make the loop more gas-efficient by skipping unnecessary checks.
Num of instances: 5
Click to show findings
['40']
40: for (uint256 i = 0; i < rewards.length; ++i) {
41: address reward = rewards[i];
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal(
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }['163']
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) {
164: totalAmount += rewardAmounts[i];
165: }['42']
42: for (uint256 i = 0; i < rewards.length; ++i) {
43: address reward = rewards[i];
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }['89']
89: for (uint256 i = 0; i < rewards.length; ++i) {
90: address reward = rewards[i];
91: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
92:
93:
94: updateRewardInternal(
95: distributionStorage,
96: accountStorage.earned[reward],
97: rewarded,
98: reward,
99: currentAccountBalance,
100: forfeitRecentReward
101: );
102:
103: distributionStorage.totalEligible -= amount;
104: }['40']
40: for (uint256 i = 0; i < rewards.length; ++i) {
41: address reward = rewards[i];
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal(
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }When performing divisions in Solidity, the operation costs gas and includes a check for division by zero. However, if you are dividing by a constant or an immutable value that is guaranteed to be non-zero, this check becomes unnecessary, consuming extra gas without adding safety.
Resolution: Utilize the unchecked block for divisions involving constant or immutable values that are assuredly non-zero. This bypasses the additional safety checks, optimizing gas usage. Ensure thorough testing and code reviews are conducted to verify the non-zero condition of the denominator, preventing any potential division by zero errors and maintaining contract safety.
Num of instances: 2
Click to show findings
['413']
413: return uint48(timestamp / EPOCH_DURATION); // <= FOUND['614']
614:
615: deltaAccountZero = uint96(delta / EPOCH_DURATION); // <= FOUNDUsing nested if statements instead of logical AND (&&) operators can potentially save gas in Solidity contracts. When a series of conditions are connected with &&, all conditions must be evaluated even if the first one fails. In contrast, nested if statements allow for short-circuiting; if the first condition fails, the rest are skipped, saving gas. This approach is more gas-efficient, especially when dealing with complex or gas-intensive conditions. However, it's crucial to balance gas savings with code readability and maintainability, ensuring that the code remains clear and easy to understand.
Num of instances: 7
Click to show findings
['152']
152: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) { // <= FOUND
153: revert InvalidEpoch();
154: }['332']
332: if (account == address(0) && deltaAccountZero != 0) { // <= FOUND
333: return claimable + deltaAccountZero;
334: }['456']
456: if (to == address(0) || (owner != address(0) && owner != to)) { // <= FOUND
457: revert InvalidRecipient();
458: }['152']
152:
153: if (!(startEpoch > epoch && startEpoch <= epoch + MAX_EPOCHS_AHEAD)) { // <= FOUND['332']
332:
333: if (account == address(0) && deltaAccountZero != 0) { // <= FOUND['456']
456: if (to == address(0) || (owner != address(0) && owner != to)) { // <= FOUND['654']
654: } else if (block.timestamp >= startTimestamp && block.timestamp < endTimestamp) { // <= FOUNDStorage optimization in Solidity contracts is vital for reducing gas costs, especially when storing time-related state variables. Using uint32 for storing time values like timestamps is often sufficient, given it can represent dates up to the year 2106. By truncating larger default integer types to uint32, you significantly save on storage space and consequently on gas costs for deployment and state modifications. However, ensure that the truncation does not lead to overflow issues and that the variable's size is adequate for the application's expected lifespan and precision requirements. Adopting this optimization practice contributes to more efficient and cost-effective smart contract development.
Num of instances: 3
Click to show findings
['20']
20: uint256 public immutable EPOCH_DURATION; // <= FOUND['32']
32: uint256 internal constant MIN_EPOCH_DURATION = 7 days; // <= FOUND['35']
35: uint256 internal constant MAX_EPOCH_DURATION = 10 * 7 days; // <= FOUNDIn Solidity, caching global or state variables in local variables can be more costly than directly using the state variables. Each assignment to a local variable consumes gas for memory allocation. Accessing state variables directly avoids this overhead, especially if they are only read once, thereby saving gas.
Num of instances: 1
Solidity version 0.8.19 introduced a array of gas optimizations which make contracts which use it more efficient. Provided compatability it may be beneficial to upgrade to this version
Num of instances: 1
Please elaborate and generalise the following with detail and feel free to use your own knowledge and lmit ur words to 100 words please:
Num of instances: 4
Click to show findings
['40']
40: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
41: address reward = rewards[i]; // <= FOUND
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal(
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }['42']
42: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
43: address reward = rewards[i]; // <= FOUND
44: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
45:
46:
47: updateRewardInternal(
48: distributionStorage, accountStorage.earned[reward], rewarded, reward, currentAccountBalance, false
49: );
50:
51: distributionStorage.totalEligible += amount;
52: }['89']
89: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
90: address reward = rewards[i]; // <= FOUND
91: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
92:
93:
94: updateRewardInternal(
95: distributionStorage,
96: accountStorage.earned[reward],
97: rewarded,
98: reward,
99: currentAccountBalance,
100: forfeitRecentReward
101: );
102:
103: distributionStorage.totalEligible -= amount;
104: }['40']
40: for (uint256 i = 0; i < rewards.length; ++i) { // <= FOUND
41: address reward = rewards[i]; // <= FOUND
42: DistributionStorage storage distributionStorage = distributions[rewarded][reward];
43:
44:
45: updateRewardInternal(
46: distributionStorage,
47: accountStorage.earned[reward],
48: rewarded,
49: reward,
50: currentAccountBalance,
51: forfeitRecentReward
52: );
53:
54: distributionStorage.totalEligible =
55: distributionStorage.totalEligible + newAccountBalance - currentAccountBalance;
56: }Rather than calling .length for an array in a for loop declaration, it is far more gas efficient to cache this length before and use that instead. This will prevent the array length from being called every loop iteration
Num of instances: 7
Click to show findings
['487']
487: for (uint48 i = 0; i < amounts.length; ++i) // <= FOUND['40']
40: for (uint256 i = 0; i < rewards.length; ++i) // <= FOUND['163']
163: for (uint256 i = 0; i < rewardAmounts.length; ++i) // <= FOUND['487']
487: for (uint48 i = 0; i < amounts.length; ++i) // <= FOUND['40']
40: for (uint256 i = 0; i < rewards.length; ++i) // <= FOUND['40']
40: for (uint256 i = 0; i < rewards.length; ++i) // <= FOUND['40']
40: for (uint256 i = 0; i < rewards.length; ++i) // <= FOUNDIf a internal function is only used once it doesn't make sense to modularise it unless the function which does call the function would be overly long and complex otherwise
Num of instances: 2
Click to show findings
['479']
479: function increaseRewardAmounts( // <= FOUND
480: address rewarded,
481: address reward,
482: uint48 startEpoch,
483: uint128[] memory amounts
484: ) internal virtual ['642']
642: function timeElapsedInEpoch(uint48 epoch, uint48 lastUpdated) internal view returns (uint256) // <= FOUNDNum of instances: 3
Click to show findings
['125']
125: constructor(address _evc, uint48 _epochDuration) EVCUtil(_evc) {
126: if (_epochDuration < MIN_EPOCH_DURATION || _epochDuration > MAX_EPOCH_DURATION) {
127: revert InvalidEpoch();
128: }
129:
130: EPOCH_DURATION = _epochDuration;
131: }['21']
21: constructor(address evc, uint48 periodDuration) BaseRewardStreams(evc, periodDuration) {}['24']
24: constructor(address evc, uint48 epochDuration) BaseRewardStreams(evc, epochDuration) {}The OpenZeppelin Array.unsafeAccess() method is a optimization strategy for Solidity, aimed at reducing gas costs by bypassing automatic length checks on storage array accesses. In Solidity, every access to an array element involves a hidden gas cost due to a length check, ensuring that accesses do not exceed the array bounds. However, if a developer has already verified the array's bounds earlier in the function or knows through logic that the access is safe, directly accessing the array elements without redundant length checks can save gas. This approach requires careful consideration to avoid out-of-bounds errors, as it trades off safety checks for efficiency.
Num of instances: 3
Click to show findings
['164']
164: totalAmount += rewardAmounts[i]; // <= FOUND['491']
491: storageAmounts[epoch / EPOCHS_PER_SLOT][epoch % EPOCHS_PER_SLOT] += amounts[i]; // <= FOUND['41']
41: address reward = rewards[i]; // <= FOUNDNum of instances: 1
[Gas-26] Write direct outcome, instead of performing mathematical operations for constant state variables
In Solidity, it's highly efficient to directly assign constant values to state variables when these values are known at compile time and will not change. This practice avoids unnecessary computational operations and reduces gas costs for deploying and interacting with smart contracts.
Num of instances: 1
Click to show findings
['35']
35: uint256 internal constant MAX_EPOCH_DURATION = 10 * 7 days; // <= FOUNDConsider saving the address(this) value within a constant using foundry's script.sol or solady's LibRlp.sol to save gas
Num of instances: 3
Click to show findings
['439']
439: uint256 preBalance = token.balanceOf(address(this)); // <= FOUND['440']
440: token.safeTransferFrom(from, address(this), amount); // <= FOUND['442']
442: if (token.balanceOf(address(this)) - preBalance != amount) { // <= FOUNDIn smart contract development, utilizing constants for known maximum or minimum values, rather than computing type(uint<n>).max or assuming 0 for .min, can significantly reduce gas costs. Constants require less runtime computation and storage, optimizing contract efficiency—a crucial strategy for developers aiming for cost-effective and performant code.
Num of instances: 2