From 328386141f21b00bb10a47ebef52a8a33b1819f2 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 13:22:07 +0100 Subject: [PATCH 01/15] fix: Cantina 154 --- src/BaseRewardStreams.sol | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/BaseRewardStreams.sol b/src/BaseRewardStreams.sol index e3a7808..156f8c7 100644 --- a/src/BaseRewardStreams.sol +++ b/src/BaseRewardStreams.sol @@ -447,18 +447,19 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard /// @notice Transfers a specified amount of a token to a given address. /// @dev This function uses `IERC20.safeTransfer` to move tokens. - /// @dev This function reverts if the recipient is zero address or is a known non-owner EVC account. + /// @dev This function reverts if the recipient is zero address OR is a known non-owner EVC account and the token is + /// not EVC compatible /// @param token The ERC20 token to transfer. /// @param to The address to transfer the tokens to. /// @param amount The amount of tokens to transfer. function pushToken(IERC20 token, address to, uint256 amount) internal { address owner = evc.getAccountOwner(to); - if (to == address(0) || (owner != address(0) && owner != to)) { + if (to == address(0) || (owner != address(0) && owner != to && !isEVCCompatibleAsset(token))) { revert InvalidRecipient(); } - IERC20(token).safeTransfer(to, amount); + token.safeTransfer(to, amount); } /// @notice Returns the reward token amount for an epoch, given a pre-computed distribution storage pointer. @@ -662,4 +663,13 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard return 0; } } + + /// @notice Checks if a given ERC20 token is EVC compatible. + /// @dev This function performs a static call to the token contract to check for the presence of the EVC function. + /// @param token The ERC20 token to check for EVC compatibility. + /// @return bool Returns true if the token is EVC compatible, false otherwise. + function isEVCCompatibleAsset(IERC20 token) internal view returns (bool) { + (bool success, bytes memory result) = address(token).staticcall(abi.encodeCall(EVCUtil.EVC, ())); + return success && result.length == 32 && address(evc) == abi.decode(result, (address)); + } } From b55e0dc7b6907a6e011f01b03fd97a342d3c6277 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 13:22:46 +0100 Subject: [PATCH 02/15] test: update tests due to Cantina 154 fix --- test/unit/Scenarios.t.sol | 26 ++++++++++++++++++++++++++ test/unit/Staking.t.sol | 24 ++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/test/unit/Scenarios.t.sol b/test/unit/Scenarios.t.sol index 8e2c641..840a473 100644 --- a/test/unit/Scenarios.t.sol +++ b/test/unit/Scenarios.t.sol @@ -2327,5 +2327,31 @@ contract ScenarioTest is Test { if (i != 0) vm.expectRevert(BaseRewardStreams.InvalidRecipient.selector); trackingDistributor.updateReward(_rewarded, _reward, __receiver); } + + // make the reward token EVC-compatible + vm.mockCall(_reward, abi.encodeWithSignature("EVC()"), abi.encode(address(evc))); + + for (uint160 i = 0; i < 256; ++i) { + address __receiver = address(uint160(_receiver) ^ i); + + // if known non-owner is the recipient, but the token is EVC-compatible, proceed + stakingDistributor.setAccountEarnedData( + address(this), _rewarded, _reward, BaseRewardStreams.EarnStorage(1, 0) + ); + stakingDistributor.claimReward(_rewarded, _reward, __receiver, _forfeitRecentReward); + + trackingDistributor.setAccountEarnedData( + address(this), _rewarded, _reward, BaseRewardStreams.EarnStorage(1, 0) + ); + trackingDistributor.claimReward(_rewarded, _reward, __receiver, _forfeitRecentReward); + + stakingDistributor.setAccountEarnedData(address(0), _rewarded, _reward, BaseRewardStreams.EarnStorage(1, 0)); + stakingDistributor.updateReward(_rewarded, _reward, __receiver); + + trackingDistributor.setAccountEarnedData( + address(0), _rewarded, _reward, BaseRewardStreams.EarnStorage(1, 0) + ); + trackingDistributor.updateReward(_rewarded, _reward, __receiver); + } } } diff --git a/test/unit/Staking.t.sol b/test/unit/Staking.t.sol index 2ea879c..810b23b 100644 --- a/test/unit/Staking.t.sol +++ b/test/unit/Staking.t.sol @@ -23,11 +23,11 @@ contract StakingTest is Test { function test_StakeAndUnstake(address participant, uint64 amount, address recipient) external { vm.assume( - participant != address(0) && participant != address(evc) && participant != address(distributor) + uint160(participant) > 255 && participant != address(evc) && participant != address(distributor) && participant != rewarded ); vm.assume( - recipient != address(0) && recipient != address(evc) && recipient != address(distributor) + uint160(recipient) > 255 && recipient != address(evc) && recipient != address(distributor) && recipient != rewarded ); vm.assume(amount > 0); @@ -107,12 +107,32 @@ contract StakingTest is Test { } // but if owner is the recipient, it should work + uint256 snapshot = vm.snapshot(); preBalanceRecipient = MockERC20(rewarded).balanceOf(recipient); preBalanceDistributor = MockERC20(rewarded).balanceOf(address(distributor)); distributor.unstake(rewarded, type(uint256).max, recipient, false); assertEq(MockERC20(rewarded).balanceOf(recipient), preBalanceRecipient + preBalanceDistributor); assertEq(MockERC20(rewarded).balanceOf(address(distributor)), 0); + // it should also work if the rewarded token is EVC-compatible + vm.revertTo(snapshot); + vm.mockCall(rewarded, abi.encodeWithSignature("EVC()"), abi.encode(address(evc))); + + for (uint160 i = 1; i < 256; ++i) { + snapshot = vm.snapshot(); + + address _recipient = address(uint160(recipient) ^ i); + + // if known non-owner is the recipient, but the rewarded token is EVC-compatible, proceed + preBalanceRecipient = MockERC20(rewarded).balanceOf(_recipient); + preBalanceDistributor = MockERC20(rewarded).balanceOf(address(distributor)); + distributor.unstake(rewarded, type(uint256).max, _recipient, false); + assertEq(MockERC20(rewarded).balanceOf(_recipient), preBalanceRecipient + preBalanceDistributor); + assertEq(MockERC20(rewarded).balanceOf(address(distributor)), 0); + + vm.revertTo(snapshot); + } + vm.stopPrank(); } } From b364c44afbd67028228c159078e2ebb219b876c2 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 14:23:03 +0100 Subject: [PATCH 03/15] fix: cantina 167 --- README.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5efe8b6..e82b52d 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,7 @@ Unlike other permissioned distributors based on the billion-dollar algorithm, Re `6e6 * block_time_sec * expected_apr_percent * 10**reward_token_decimals * price_of_rewarded_token / 10**rewarded_token_decimals / price_of_reward_token > 1` -For example, for the SHIB-USDC reward-rewarded pair, the above condition will not be met, even with an unusually high assumed APR of 1000%: +For example, for the SHIB-USDC rewarded-reward pair, the above condition will not be met, even with an unusually high assumed APR of 1000%: `block_time_sec = 12` `expected_apr_percent = 1000` `rewarded_token_decimals = 18` @@ -134,10 +134,14 @@ For example, for the SHIB-USDC reward-rewarded pair, the above condition will no `6e6 * 12 * 1000 * 10**6 * 0.00002 / 10**18 / 1` is less than `1`. 6. **If nobody earns rewards at the moment (i.e. nobody staked/deposited yet), they're being virtually accrued by address(0) and may be claimed by anyone**: This feature is designed to prevent reward tokens from being lost when nobody earns them at the moment. However, it also means that unclaimed rewards could potentially be claimed by anyone. -7. **Distributor contracts do not have an owner or admin meaning that none of the assets can be directly recovered from them**: This feature is required for the system to work in a permissionless manner. However, it also means that if a mistake is made in the distribution of rewards, the assets cannot be directly recovered from the distributor contracts. -8. **Distributor contracts do not support rebasing and fee-on-transfer tokens**: This limitation is in place due to internal accounting system limitations. Neither reward nor rewarded tokens may be rebasing or fee-on-transfer tokens. -9. **Precision loss may lead to the portion of rewards being lost to the distributor**: Precision loss is inherent to calculations in Solidity due to its use of fixed-point arithmetic. In some configurations of the distributor and streams, depending on the accumulator update frequency, a dust portion of the rewards registered might get irrevocably lost to the distributor. However, the amount lost should be negligible as long as the condition from 5. is met. -10. **Permissionless nature of the distributor may lead to DOS for popular reward-rewarded token pairs**: The distributor allows anyone to incentivize any token with any reward. A bad actor may grief the party willing to legitimately incentivize a given reward-rewarded token pair by registering a tiny reward stream long before the party decides to register the reward stream themselves. Such a reward stream, if not updated frequently, may lead to a situation where the legitimate party is forced to update it themselves since the time the bad actor set up their stream. This may be costly in terms of gas. +7. **If nobody earns rewards at the moment, despite being virtually accrued by address(0) and claimable by anyone, they might still get lost due to rounding**: This limitation may occur due to unfortunate rounding errors during internal calculations, which can result in registered rewards being irrevocably lost. To ensure that the value lost due to rounding is not significant, one must ensure that 1 wei of the reward token multiplied by epoch duration has negligible value. + +For example, if the epoch duration is 2 weeks (which corresponds to ~1.2e6 seconds) and the reward token is WBTC, in one rounding, up to ~1.2e6 WBTC may be lost. At the current BTC price, this value corresponds to ~$700, which is a significant value to lose for just one update of the reward stream. Hence, one must either avoid adding rewards that have a significant value of 1 wei or make sure that someone earns rewards at all times. + +8. **Distributor contracts do not have an owner or admin meaning that none of the assets can be directly recovered from them**: This feature is required for the system to work in a permissionless manner. However, it also means that if a mistake is made in the distribution of rewards, the assets cannot be directly recovered from the distributor contracts. +9. **Distributor contracts do not support rebasing and fee-on-transfer tokens**: This limitation is in place due to internal accounting system limitations. Neither reward nor rewarded tokens may be rebasing or fee-on-transfer tokens. +10. **Precision loss may lead to the portion of rewards being lost to the distributor**: Precision loss is inherent to calculations in Solidity due to its use of fixed-point arithmetic. In some configurations of the distributor and streams, depending on the accumulator update frequency, a dust portion of the rewards registered might get irrevocably lost to the distributor. However, the amount lost should be negligible as long as the condition from 5. is met. +11. **Permissionless nature of the distributor may lead to DOS for popular reward-rewarded token pairs**: The distributor allows anyone to incentivize any token with any reward. A bad actor may grief the party willing to legitimately incentivize a given reward-rewarded token pair by registering a tiny reward stream long before the party decides to register the reward stream themselves. Such a reward stream, if not updated frequently, may lead to a situation where the legitimate party is forced to update it themselves since the time the bad actor set up their stream. This may be costly in terms of gas. ## Install From 967f6525e58ba8992a4ce0e41f2fd56a016494a0 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 14:31:00 +0100 Subject: [PATCH 04/15] fix: cantina 231 --- src/TrackingRewardStreams.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/TrackingRewardStreams.sol b/src/TrackingRewardStreams.sol index 5a8dba3..0a7bb54 100644 --- a/src/TrackingRewardStreams.sol +++ b/src/TrackingRewardStreams.sol @@ -27,7 +27,8 @@ contract TrackingRewardStreams is BaseRewardStreams, ITrackingRewardStreams { /// @notice Executes the balance tracking hook for an account /// @param account The account address to execute the hook for /// @param newAccountBalance The new balance of the account - /// @param forfeitRecentReward Whether to forfeit the most recent reward and not update the accumulator + /// @param forfeitRecentReward Whether to forfeit the most recent reward and not update the accumulator. Ignored + /// when the new balance is greater than the current balance. function balanceTrackerHook( address account, uint256 newAccountBalance, @@ -38,6 +39,8 @@ contract TrackingRewardStreams is BaseRewardStreams, ITrackingRewardStreams { uint256 currentAccountBalance = accountStorage.balance; address[] memory rewards = accountStorage.enabledRewards.get(); + if (newAccountBalance > currentAccountBalance) forfeitRecentReward = false; + for (uint256 i = 0; i < rewards.length; ++i) { address reward = rewards[i]; DistributionStorage storage distributionStorage = distributions[rewarded][reward]; From 8fd3d52d9aef4a3d7db37368339d390ed7081a8c Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:12:16 +0100 Subject: [PATCH 05/15] test: add test for cantina 231 fix --- test/unit/Scenarios.t.sol | 85 ++++++++++++++++++++++++++++++++++++++- test/utils/MockERC20.sol | 22 ++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/test/unit/Scenarios.t.sol b/test/unit/Scenarios.t.sol index 8e2c641..7c63e72 100644 --- a/test/unit/Scenarios.t.sol +++ b/test/unit/Scenarios.t.sol @@ -6,7 +6,7 @@ import "forge-std/Test.sol"; import "evc/EthereumVaultConnector.sol"; import "../harness/StakingRewardStreamsHarness.sol"; import "../harness/TrackingRewardStreamsHarness.sol"; -import {MockERC20, MockERC20BalanceForwarder} from "../utils/MockERC20.sol"; +import {MockERC20, MockERC20BalanceForwarder, MockERC20BalanceForwarderMessedUp} from "../utils/MockERC20.sol"; import {MockController} from "../utils/MockController.sol"; import {boundAddr} from "../utils/TestUtils.sol"; @@ -1920,7 +1920,7 @@ contract ScenarioTest is Test { vm.stopPrank(); // forward the time to the middle of the second epoch of the distribution scheme - vm.warp(stakingDistributor.getEpochStartTimestamp(stakingDistributor.currentEpoch() + 1) + 15 days); + vm.warp(trackingDistributor.getEpochStartTimestamp(trackingDistributor.currentEpoch() + 1) + 15 days); // verify earnings assertApproxEqRel( @@ -2237,6 +2237,87 @@ contract ScenarioTest is Test { assertEq(MockERC20(trackingRewarded).balanceOf(PARTICIPANT_1), preBalance - 10e18); } + // reward and rewarded are the same + function test_Scenario_MaliciousBalanceForwarder(uint48 blockTimestamp) external { + blockTimestamp = uint48(bound(blockTimestamp, 1, type(uint48).max - 365 days)); + + uint256 ALLOWED_DELTA = 1e12; // 0.0001% + + address trackingRewardedMessedUp = address( + new MockERC20BalanceForwarderMessedUp(evc, trackingDistributor, "Tracking Rewarded Malicious", "SFRWDDM") + ); + + // mint the tracking rewarded token to participants + MockERC20(trackingRewardedMessedUp).mint(PARTICIPANT_1, 10e18); + MockERC20(trackingRewardedMessedUp).mint(PARTICIPANT_2, 10e18); + MockERC20(trackingRewardedMessedUp).mint(PARTICIPANT_3, 10e18); + + vm.warp(blockTimestamp); + + // prepare the amounts; 3 epochs + uint128[] memory amounts = new uint128[](3); + amounts[0] = 10e18; + amounts[1] = 10e18; + amounts[2] = 10e18; + + // register the distribution scheme + vm.startPrank(seeder); + trackingDistributor.registerReward(trackingRewardedMessedUp, reward, 0, amounts); + vm.stopPrank(); + + // enable reward and balance forwarding for participant 1 and participant 2 + vm.startPrank(PARTICIPANT_1); + MockERC20BalanceForwarder(trackingRewardedMessedUp).enableBalanceForwarding(); + trackingDistributor.enableReward(trackingRewardedMessedUp, reward); + vm.stopPrank(); + + vm.startPrank(PARTICIPANT_2); + MockERC20BalanceForwarder(trackingRewardedMessedUp).enableBalanceForwarding(); + trackingDistributor.enableReward(trackingRewardedMessedUp, reward); + vm.stopPrank(); + + // forward the time to the end of the first epoch of the distribution scheme + vm.warp(trackingDistributor.getEpochStartTimestamp(trackingDistributor.currentEpoch() + 1) + 10 days); + + // lock in current earnings + trackingDistributor.updateReward(trackingRewardedMessedUp, reward, address(0)); + + // verify earnings + assertApproxEqRel( + trackingDistributor.earnedReward(PARTICIPANT_1, trackingRewardedMessedUp, reward, false), + 5e18, + ALLOWED_DELTA + ); + assertApproxEqRel( + trackingDistributor.earnedReward(PARTICIPANT_2, trackingRewardedMessedUp, reward, false), + 5e18, + ALLOWED_DELTA + ); + assertEq(trackingDistributor.earnedReward(address(0), trackingRewardedMessedUp, reward, false), 0); + + // forward the time + vm.warp(block.timestamp + 20 days); + + // participant 3 (who doesn't have balance forwarding enabled) transfers tokens to participant 1 (who has + // balance forwarding enabled). despite messed up balance forwarder integration, the accounting will be correct + // because the forfeitRecentReward flag setting will be overriden + vm.prank(PARTICIPANT_3); + MockERC20(trackingRewardedMessedUp).transfer(PARTICIPANT_1, 10e18); + + // verify earnings. both participants should earn the same amount of rewards, despite messed up balance forwarder integrations + assertApproxEqRel( + trackingDistributor.earnedReward(PARTICIPANT_1, trackingRewardedMessedUp, reward, false), + 15e18, + ALLOWED_DELTA + ); + assertApproxEqRel( + trackingDistributor.earnedReward(PARTICIPANT_2, trackingRewardedMessedUp, reward, false), + 15e18, + ALLOWED_DELTA + ); + assertEq(trackingDistributor.earnedReward(address(0), trackingRewardedMessedUp, reward, false), 0); + } + function test_AssertionTrigger( address _account, address _rewarded, diff --git a/test/utils/MockERC20.sol b/test/utils/MockERC20.sol index c84f302..aaea30a 100644 --- a/test/utils/MockERC20.sol +++ b/test/utils/MockERC20.sol @@ -94,3 +94,25 @@ contract MockERC20BalanceForwarder is MockERC20, IBalanceForwarder { balanceTracker.balanceTrackerHook(account, newAccountBalance, forfeitRecentReward); } } + +contract MockERC20BalanceForwarderMessedUp is MockERC20BalanceForwarder { + constructor( + IEVC _evc, + IBalanceTracker _balanceTracker, + string memory _name, + string memory _symbol + ) MockERC20BalanceForwarder(_evc, _balanceTracker, _name, _symbol) {} + + function _update(address from, address to, uint256 value) internal virtual override { + ERC20._update(from, to, value); + + if (forwardingEnabled[from]) { + balanceTracker.balanceTrackerHook(from, balanceOf(from), false); + } + + // always pass forfeitRecentReward = true when increasing balance to mess up the accounting + if (from != to && forwardingEnabled[to]) { + balanceTracker.balanceTrackerHook(to, balanceOf(to), true); + } + } +} From 7db4407864dcd82ee19ffba5345f85ac94e23c58 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:12:36 +0100 Subject: [PATCH 06/15] fix: docs consistency --- README.md | 2 -- src/interfaces/IBalanceTracker.sol | 9 ++++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 5efe8b6..14a8841 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,6 @@ Reward Streams operates in two modes of rewards distribution: staking and balanc The balance-tracking `TrackingRewardStreams` implementation inherits from the `BaseRewardStreams` contract. It defines the `IBalanceTracker.balanceTrackerHook` function, which is required to be called on every transfer of the rewarded token if a user opted in for the hook to be called. -In this mode, the rewarded token contract not only calls the `balanceTrackerHook` function whenever a given account balance changes, but also implements the `IBalanceForwarder` interface. This interface defines two functions: `enableBalanceForwarding` and `disableBalanceForwarding`, which are used to opt in and out of the hook being called. - ### Staking Reward Distribution The staking `StakingRewardStreams` implementation also inherits from the `BaseRewardStreams` contract. It defines two functions: `stake` and `unstake`, which are used to stake and unstake the rewarded token. diff --git a/src/interfaces/IBalanceTracker.sol b/src/interfaces/IBalanceTracker.sol index ae3f899..6eff799 100644 --- a/src/interfaces/IBalanceTracker.sol +++ b/src/interfaces/IBalanceTracker.sol @@ -8,11 +8,10 @@ pragma solidity >=0.8.0; /// @notice Provides an interface for tracking the balance of accounts. interface IBalanceTracker { /// @notice Executes the balance tracking hook for an account. - /// @dev This function is called by the Balance Forwarder contract which was enabled for the account. This function - /// must be called with the current balance of the account when enabling the balance forwarding for it. This - /// function must be called with 0 balance of the account when disabling the balance forwarding for it. This - /// function allows to be called on zero balance transfers, when the newAccountBalance is the same as the previous - /// one. To prevent DOS attacks, forfeitRecentReward should be used appropriately. + /// @dev This function must be called with the current balance of the account when enabling the balance forwarding + /// for it. This function must be called with 0 balance of the account when disabling the balance forwarding for it. + /// This function allows to be called on zero balance transfers, when the newAccountBalance is the same as the + /// previous one. To prevent DOS attacks, forfeitRecentReward should be used appropriately. /// @param account The account address to execute the hook for. /// @param newAccountBalance The new balance of the account. /// @param forfeitRecentReward Whether to forfeit the most recent reward and not update the accumulator. From 929b626d3a1f5a26f010efa19a56c09a05c23f43 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:13:09 +0100 Subject: [PATCH 07/15] fix: formatting --- test/unit/Scenarios.t.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/Scenarios.t.sol b/test/unit/Scenarios.t.sol index 7c63e72..6af3555 100644 --- a/test/unit/Scenarios.t.sol +++ b/test/unit/Scenarios.t.sol @@ -2304,7 +2304,8 @@ contract ScenarioTest is Test { vm.prank(PARTICIPANT_3); MockERC20(trackingRewardedMessedUp).transfer(PARTICIPANT_1, 10e18); - // verify earnings. both participants should earn the same amount of rewards, despite messed up balance forwarder integrations + // verify earnings. both participants should earn the same amount of rewards, despite messed up balance + // forwarder integrations assertApproxEqRel( trackingDistributor.earnedReward(PARTICIPANT_1, trackingRewardedMessedUp, reward, false), 15e18, From be7a1c915832d4b5a5ee4ecb8bf375e7fa1aaf2b Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:19:42 +0100 Subject: [PATCH 08/15] fix: cantina 240 --- src/BaseRewardStreams.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BaseRewardStreams.sol b/src/BaseRewardStreams.sol index e3a7808..a346cba 100644 --- a/src/BaseRewardStreams.sol +++ b/src/BaseRewardStreams.sol @@ -17,7 +17,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard using Set for SetStorage; /// @notice The duration of a reward epoch. - /// @dev Must be longer than 1 week, but no longer than 10 weeks. + /// @dev May not be shorter than 1 week and longer than 10 weeks. uint256 public immutable EPOCH_DURATION; /// @notice The maximum number of epochs in the future that newly registered reward streams can begin. From e2248ec538dcc079a4b484fb27fb70e778c4887c Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:23:42 +0100 Subject: [PATCH 09/15] fix: cantina 273 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5efe8b6..034afd1 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ For example, for the SHIB-USDC reward-rewarded pair, the above condition will no 7. **Distributor contracts do not have an owner or admin meaning that none of the assets can be directly recovered from them**: This feature is required for the system to work in a permissionless manner. However, it also means that if a mistake is made in the distribution of rewards, the assets cannot be directly recovered from the distributor contracts. 8. **Distributor contracts do not support rebasing and fee-on-transfer tokens**: This limitation is in place due to internal accounting system limitations. Neither reward nor rewarded tokens may be rebasing or fee-on-transfer tokens. 9. **Precision loss may lead to the portion of rewards being lost to the distributor**: Precision loss is inherent to calculations in Solidity due to its use of fixed-point arithmetic. In some configurations of the distributor and streams, depending on the accumulator update frequency, a dust portion of the rewards registered might get irrevocably lost to the distributor. However, the amount lost should be negligible as long as the condition from 5. is met. -10. **Permissionless nature of the distributor may lead to DOS for popular reward-rewarded token pairs**: The distributor allows anyone to incentivize any token with any reward. A bad actor may grief the party willing to legitimately incentivize a given reward-rewarded token pair by registering a tiny reward stream long before the party decides to register the reward stream themselves. Such a reward stream, if not updated frequently, may lead to a situation where the legitimate party is forced to update it themselves since the time the bad actor set up their stream. This may be costly in terms of gas. +10. **Permissionless nature of the distributor may lead to DOS for popular reward-rewarded token pairs**: The distributor allows anyone to incentivize any token with any reward. A bad actor may grief the party willing to legitimately incentivize a given reward-rewarded token pair by registering a tiny reward stream long before the party decides to register the reward stream themselves. Such a reward stream, if not updated frequently, may lead to a situation where the legitimate party is forced to update it themselves since the time the bad actor set up their stream. This may be costly in terms of gas or even impossible if enough time elapses that the gas cost to update such a reward stream exceeds the block gas limit. ## Install From 9cfe71349c3fbc48588db2a327405a89f3395337 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:26:07 +0100 Subject: [PATCH 10/15] fix: cantina 313 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5efe8b6..aa322c3 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ As previously explained, rewards distributions are epoch-based. Thanks to that, ### Example: -Alice staked her `ABC` and decided to enable both `DEF` and `GHI` rewards. Alice now wants to unstake her `ABC`, but notices that despite her estimations `GHI` tokens that she earned have very low value. It's been some time since the `GHI` distribution was updated last time hence the gas cost associated with unstaking may be significant. Alice may decide to either call `unstake` with `forgiveRecentReward` set to `true`, which means that both `DEF` and `GHI` rewards that she would earn since the last updates would get lost in favor of the rest of participants. Or she may first call `disableReward(GHI)` with `forgiveRecentReward` set to `true`, which will skip epochs iteration for `GHI` distribution, and then call `unstake` with `forgiveRecentReward` set to `false`, keeping all the `DEF` rewards. +Alice staked her `ABC` and decided to enable both `DEF` and `GHI` rewards. Alice now wants to unstake her `ABC`, but notices that despite her estimations `GHI` tokens that she earned have very low value. It's been some time since the `GHI` distribution was updated last time hence the gas cost associated with unstaking may be significant. Alice may decide to either call `unstake` with `forfeitRecentReward` set to `true`, which means that both `DEF` and `GHI` rewards that she would earn since the last updates would get lost in favor of the rest of participants. Or she may first call `disableReward(GHI)` with `forfeitRecentReward` set to `true`, which will skip epochs iteration for `GHI` distribution, and then call `unstake` with `forfeitRecentReward` set to `false`, keeping all the `DEF` rewards. --- From 47f56d33937e8e82b8efcc5b44b84a50b5c7081c Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:27:53 +0100 Subject: [PATCH 11/15] fix: cantina 339 --- src/BaseRewardStreams.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BaseRewardStreams.sol b/src/BaseRewardStreams.sol index e3a7808..60001f0 100644 --- a/src/BaseRewardStreams.sol +++ b/src/BaseRewardStreams.sol @@ -256,7 +256,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard } /// @notice Enable reward token. - /// @dev There can be at most MAX_REWARDS_ENABLED rewards enabled for the reward token and the account. + /// @dev There can be at most MAX_REWARDS_ENABLED rewards enabled for the rewarded token and the account. /// @param rewarded The address of the rewarded token. /// @param reward The address of the reward token. function enableReward(address rewarded, address reward) external virtual override { From 6165fd67723d5752e4e841356a45be8456ee8269 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 16:45:39 +0100 Subject: [PATCH 12/15] fix: cantina 233 --- README.md | 4 ++-- src/BaseRewardStreams.sol | 24 ++++++++++++------------ src/interfaces/IRewardStreams.sol | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 5efe8b6..a0c2638 100644 --- a/README.md +++ b/README.md @@ -97,9 +97,9 @@ Given this, Alice decides to enable only the `GHI` reward and keep the `DEF` rew --- -Multiple functions of the distributors contain an additional boolean parameter called `forfeitRecentReward`. It allows a user to optimize gas consumption in case it is not worth to iterate over multiple distribution epochs and updating contract storage. It also allows for "emergency exit" for operations like disabling reward and claiming, and DOS protection (i.e. in liquidation flows). +Multiple functions of the distributors contain an additional boolean parameter called `forfeitRecentReward`/`ignoreRecentReward`. It allows a user to optimize gas consumption in case it is not worth to iterate over multiple distribution epochs and updating contract storage. It also allows for "emergency exit" for operations like disabling reward and claiming, and DOS protection (i.e. in liquidation flows). -As previously explained, rewards distributions are epoch-based. Thanks to that, each epoch may have a different reward rate, but also it is possible for the reward streams to be registered permissionlessly in additive manner. However, the downside of this approach is the fact that whenever a user stakes or unstakes (or, for balance-tracking version of the distributor, transfers/mints/burns the rewarded token), the distributor contract needs to iterate over all the epochs since the last time given distribution, defined by `rewarded` and `reward` token, was updated. Moreover, a user may be earning multiple rewards for a single rewarded token, so the distributor contract needs to iterate over all the epochs since the last update for all the rewards the user is earning. If updates happen rarely (i.e. due to low staking/unstaking activity of the `rewarded` token for a given `reward`), the gas cost associated with iterating may be significant, affecting user's profitability. Hence, when disabling or claiming reward, if the user wants to skip the epochs iteration, they can call the relevant function with `forfeitRecentReward` set to `true`. This will grant the rewards earned since the last distribution update, which would normally be earned by the user, to the rest of the distribution participants, lowering the gas cost for the user. +As previously explained, rewards distributions are epoch-based. Thanks to that, each epoch may have a different reward rate, but also it is possible for the reward streams to be registered permissionlessly in additive manner. However, the downside of this approach is the fact that whenever a user stakes or unstakes (or, for balance-tracking version of the distributor, transfers/mints/burns the rewarded token), the distributor contract needs to iterate over all the epochs since the last time given distribution, defined by `rewarded` and `reward` token, was updated. Moreover, a user may be earning multiple rewards for a single rewarded token, so the distributor contract needs to iterate over all the epochs since the last update for all the rewards the user is earning. If updates happen rarely (i.e. due to low staking/unstaking activity of the `rewarded` token for a given `reward`), the gas cost associated with iterating may be significant, affecting user's profitability. Hence, i.e. when disabling or claiming reward, if the user wants to skip the epochs iteration, they can call the relevant function with `forfeitRecentReward`/`ignoreRecentReward` set to `true`. This will, depending on the operation, either grant the rewards earned since the last distribution update, which would normally be earned by the user, to the rest of the distribution participants or ignore them by skiping the iteration, lowering the gas cost for the user. `forfeitRecentReward` parameter may also come handy for the rewarded token contract which calls `balanceTrackerHook` on the balance changes. In case of i.e. liquidation, where user may have incentive to perform DOS attack and increase gas cost of the token transfer by enabling multiple rewards for distributions of low activity, the rewarded token contract may call `balanceTrackerHook` with `forfeitRecentReward` set to `true` to lower the gas cost of the transfer. Unfortunately, this may lead to the user losing part of their rewards. diff --git a/src/BaseRewardStreams.sol b/src/BaseRewardStreams.sol index e3a7808..a2cedc0 100644 --- a/src/BaseRewardStreams.sol +++ b/src/BaseRewardStreams.sol @@ -230,12 +230,12 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard /// @param rewarded The address of the rewarded token. /// @param reward The address of the reward token. /// @param recipient The address to receive the claimed reward tokens. - /// @param forfeitRecentReward Whether to forfeit the recent rewards and not update the accumulator. + /// @param ignoreRecentReward Whether to ignore the most recent reward and not update the accumulator. function claimReward( address rewarded, address reward, address recipient, - bool forfeitRecentReward + bool ignoreRecentReward ) external virtual override nonReentrant { address msgSender = _msgSender(); @@ -249,7 +249,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard rewarded, reward, currentAccountBalance, - forfeitRecentReward + ignoreRecentReward ); claim(msgSender, rewarded, reward, recipient); @@ -313,20 +313,20 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard /// @param account The address of the account. /// @param rewarded The address of the rewarded token. /// @param reward The address of the reward token. - /// @param forfeitRecentReward Whether to forfeit the recent rewards and not update the accumulator. + /// @param ignoreRecentReward Whether to ignore the most recent reward and not update the accumulator. /// @return The earned reward token amount for the account and rewarded token. function earnedReward( address account, address rewarded, address reward, - bool forfeitRecentReward + bool ignoreRecentReward ) external view virtual override returns (uint256) { // If the account disables the rewards we pass an account balance of zero to not accrue any. AccountStorage storage accountStorage = accounts[account][rewarded]; uint256 currentAccountBalance = accountStorage.enabledRewards.contains(reward) ? accountStorage.balance : 0; (,, uint96 claimable, uint96 deltaAccountZero) = calculateRewards( - distributions[rewarded][reward], accountStorage.earned[reward], currentAccountBalance, forfeitRecentReward + distributions[rewarded][reward], accountStorage.earned[reward], currentAccountBalance, ignoreRecentReward ); // If we have spillover rewards, we add them to `address(0)`. @@ -532,17 +532,17 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard /// @param rewarded The address of the rewarded token. /// @param reward The address of the reward token. /// @param currentAccountBalance The current rewarded token balance of the account. - /// @param forfeitRecentReward Whether to forfeit the recent rewards and not update the accumulator. + /// @param ignoreRecentReward Whether to ignore the most recent reward and not update the accumulator. function updateRewardInternal( DistributionStorage storage distributionStorage, EarnStorage storage accountEarnStorage, address rewarded, address reward, uint256 currentAccountBalance, - bool forfeitRecentReward + bool ignoreRecentReward ) internal virtual { (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) = - calculateRewards(distributionStorage, accountEarnStorage, currentAccountBalance, forfeitRecentReward); + calculateRewards(distributionStorage, accountEarnStorage, currentAccountBalance, ignoreRecentReward); // Update the distribution data. distributionStorage.lastUpdated = lastUpdated; @@ -566,7 +566,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard /// @param distributionStorage Pointer to the storage of the distribution. /// @param accountEarnStorage Pointer to the storage of the account's earned amount and accumulator. /// @param currentAccountBalance The current rewarded token balance of the account. - /// @param forfeitRecentReward Whether to forfeit the recent rewards and not update the accumulator. + /// @param ignoreRecentReward Whether to ignore the most recent reward and not update the accumulator. /// @return lastUpdated The next value for the last update timestamp. /// @return accumulator The next value for the distribution accumulator. /// @return claimable The next value for the account's claimable amount. @@ -575,7 +575,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard DistributionStorage storage distributionStorage, EarnStorage storage accountEarnStorage, uint256 currentAccountBalance, - bool forfeitRecentReward + bool ignoreRecentReward ) internal view @@ -591,7 +591,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard return (lastUpdated, accumulator, claimable, 0); } - if (!forfeitRecentReward) { + if (!ignoreRecentReward) { // Get the start and end epochs based on the last updated timestamp of the distribution. uint48 epochStart = getEpoch(lastUpdated); uint48 epochEnd = currentEpoch() + 1; diff --git a/src/interfaces/IRewardStreams.sol b/src/interfaces/IRewardStreams.sol index 0175522..7cae777 100644 --- a/src/interfaces/IRewardStreams.sol +++ b/src/interfaces/IRewardStreams.sol @@ -15,10 +15,10 @@ interface IRewardStreams { function MAX_REWARDS_ENABLED() external view returns (uint256); function registerReward(address rewarded, address reward, uint48 startEpoch, uint128[] calldata rewardAmounts) external; function updateReward(address rewarded, address reward, address recipient) external; - function claimReward(address rewarded, address reward, address recipient, bool forfeitRecentReward) external; + function claimReward(address rewarded, address reward, address recipient, bool ignoreRecentReward) external; function enableReward(address rewarded, address reward) external; function disableReward(address rewarded, address reward, bool forfeitRecentReward) external; - function earnedReward(address account, address rewarded, address reward, bool forfeitRecentReward) external view returns (uint256); + function earnedReward(address account, address rewarded, address reward, bool ignoreRecentReward) external view returns (uint256); function enabledRewards(address account, address rewarded) external view returns (address[] memory); function balanceOf(address account, address rewarded) external view returns (uint256); function rewardAmount(address rewarded, address reward) external view returns (uint256); From 098795075355d83606c56d895da4f9e792ae7880 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Tue, 2 Jul 2024 21:21:48 +0100 Subject: [PATCH 13/15] fix: typo --- src/BaseRewardStreams.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BaseRewardStreams.sol b/src/BaseRewardStreams.sol index a346cba..6e13a37 100644 --- a/src/BaseRewardStreams.sol +++ b/src/BaseRewardStreams.sol @@ -17,7 +17,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard using Set for SetStorage; /// @notice The duration of a reward epoch. - /// @dev May not be shorter than 1 week and longer than 10 weeks. + /// @dev Must be between 1 and 10 weeks, inclusive. uint256 public immutable EPOCH_DURATION; /// @notice The maximum number of epochs in the future that newly registered reward streams can begin. From 32c2c2f86779024bcaf79a83caac8774d8f3bfb7 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Fri, 5 Jul 2024 13:12:19 +0100 Subject: [PATCH 14/15] fix: cantina 31 --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1e4962c..800857d 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ The staking `StakingRewardStreams` implementation also inherits from the `BaseRe In both modes, each distributor contract defines an `EPOCH_DURATION` constant, which is the duration of a single epoch. This duration cannot be less than 1 week and more than 10 weeks. -When registering a new reward stream for the `rewarded` token, one needs to specify the `startEpoch` number when the new stream will come into effect. `startEpoch` cannot be more than 5 epochs into the future. Moreover, one needs to specify `rewardAmounts` array which instructs the contract how much `reward` one wants to distribute in each epoch starting from `startEpoch`. The `rewardAmounts` array must have a length of at most 25. +When registering a new reward stream for the `rewarded` token, one must specify the `startEpoch` number when the new stream will come into effect. To protect users from obvious mistakes, the distributor contract enforces a soft requirement that ensures the `startEpoch` is not more than 5 epochs into the future. Moreover, one must specify the `rewardAmounts` array, which instructs the contract how much `reward` one wants to distribute in each epoch starting from `startEpoch`. The `rewardAmounts` array must have a length of at most 25 for one function call. If rewarded epochs of multiple reward streams overlap, the amounts will be combined and the effective distribution will be the sum of the amounts in the overlapping epochs. @@ -114,7 +114,7 @@ Unlike other permissioned distributors based on the billion-dollar algorithm, Re ## Known limitations 1. **Epoch duration may not be shorter than 1 week and longer than 10 weeks**: This limitation is in place to ensure the stability and efficiency of the distribution system. The longer the epoch, the more gas efficient the distribution is. -2. **New reward stream may start at most 5 epochs ahead and be at most 25 epochs long**: This limitation is in place not to register distribution too far in the future and lasting for too long. +2. **Registered reward stream can start at most 5 epochs ahead and can last for a maximum of 25 epochs**: This limitation ensures that user inputs are reasonable and helps protect them from making obvious mistakes. 3. **A user may have at most 5 rewards enabled at a time for a given rewarded token**: This limitation is in place to prevent users from enabling an excessive number of rewards, which could lead to increased gas costs and potential system instability. 4. **During its lifetime, a distributor may distribute at most `type(uint160).max / 2e19` units of a reward token per rewarded token**: This limitation is in place not to allow accumulator overflow. 5. **Not all rewarded-reward token pairs may be compatible with the distributor**: This limitation may occur due to unfortunate rounding errors during internal calculations, which can result in registered rewards being irrevocably lost. To avoid this, one must ensure that the following condition, based on an empirical formula, holds true: From 920605c48539e52b6a76284d5c136bf943af29ae Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Sat, 13 Jul 2024 23:19:17 +0200 Subject: [PATCH 15/15] refactor: DistributionStorage refactor --- src/BaseRewardStreams.sol | 8 ++++---- test/harness/BaseRewardStreamsHarness.sol | 6 +++--- test/harness/StakingRewardStreamsHarness.sol | 2 +- test/harness/TrackingRewardStreamsHarness.sol | 2 +- test/unit/Staking.t.sol | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/BaseRewardStreams.sol b/src/BaseRewardStreams.sol index 4ac0f04..8908141 100644 --- a/src/BaseRewardStreams.sol +++ b/src/BaseRewardStreams.sol @@ -49,7 +49,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard /// @notice The last timestamp when the distribution was updated. uint48 lastUpdated; /// @notice The most recent accumulator value. - uint208 accumulator; + uint160 accumulator; /// @notice Total rewarded token that are eligible for rewards. uint256 totalEligible; /// @notice Total reward token that have been transferred into this contract for rewards. @@ -542,7 +542,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard uint256 currentAccountBalance, bool ignoreRecentReward ) internal virtual { - (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) = + (uint48 lastUpdated, uint160 accumulator, uint96 claimable, uint96 deltaAccountZero) = calculateRewards(distributionStorage, accountEarnStorage, currentAccountBalance, ignoreRecentReward); // Update the distribution data. @@ -552,7 +552,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard // Update the account's earned amount. Snapshot new accumulator value for the account. // Downcasting is safe because the `totalRegistered <= type(uint160).max / SCALER`. accountEarnStorage.claimable = claimable; - accountEarnStorage.accumulator = uint160(accumulator); + accountEarnStorage.accumulator = accumulator; // If there were excess rewards, allocate them to address(0). // Overflow safe because `totalRegistered <= type(uint160).max / SCALER < type(uint96).max`. @@ -581,7 +581,7 @@ abstract contract BaseRewardStreams is IRewardStreams, EVCUtil, ReentrancyGuard internal view virtual - returns (uint48 lastUpdated, uint208 accumulator, uint96 claimable, uint96 deltaAccountZero) + returns (uint48 lastUpdated, uint160 accumulator, uint96 claimable, uint96 deltaAccountZero) { // If the distribution is not initialized, return. lastUpdated = distributionStorage.lastUpdated; diff --git a/test/harness/BaseRewardStreamsHarness.sol b/test/harness/BaseRewardStreamsHarness.sol index acc623b..b2fe905 100644 --- a/test/harness/BaseRewardStreamsHarness.sol +++ b/test/harness/BaseRewardStreamsHarness.sol @@ -42,7 +42,7 @@ contract BaseRewardStreamsHarness is BaseRewardStreams { address rewarded, address reward, uint48 lastUpdated, - uint208 accumulator, + uint160 accumulator, uint256 totalEligible, uint128 totalRegistered, uint128 totalClaimed @@ -119,10 +119,10 @@ contract BaseRewardStreamsHarness is BaseRewardStreams { return timeElapsedInEpoch(epoch, lastUpdated); } - function getUpdatedAccumulator(address rewarded, address reward) external view returns (uint208) { + function getUpdatedAccumulator(address rewarded, address reward) external view returns (uint160) { DistributionStorage storage distributionStorage = distributions[rewarded][reward]; EarnStorage storage earnStorage = accounts[msg.sender][rewarded].earned[reward]; - (, uint208 accumulator,,) = calculateRewards(distributionStorage, earnStorage, 0, false); + (, uint160 accumulator,,) = calculateRewards(distributionStorage, earnStorage, 0, false); return accumulator; } diff --git a/test/harness/StakingRewardStreamsHarness.sol b/test/harness/StakingRewardStreamsHarness.sol index 35eee33..72680cf 100644 --- a/test/harness/StakingRewardStreamsHarness.sol +++ b/test/harness/StakingRewardStreamsHarness.sol @@ -42,7 +42,7 @@ contract StakingRewardStreamsHarness is StakingRewardStreams { address rewarded, address reward, uint48 lastUpdated, - uint208 accumulator, + uint160 accumulator, uint256 totalEligible, uint128 totalRegistered, uint128 totalClaimed diff --git a/test/harness/TrackingRewardStreamsHarness.sol b/test/harness/TrackingRewardStreamsHarness.sol index e0f9e54..1c63f7f 100644 --- a/test/harness/TrackingRewardStreamsHarness.sol +++ b/test/harness/TrackingRewardStreamsHarness.sol @@ -43,7 +43,7 @@ contract TrackingRewardStreamsHarness is TrackingRewardStreams { address rewarded, address reward, uint48 lastUpdated, - uint208 accumulator, + uint160 accumulator, uint256 totalEligible, uint128 totalRegistered, uint128 totalClaimed diff --git a/test/unit/Staking.t.sol b/test/unit/Staking.t.sol index 810b23b..44dc303 100644 --- a/test/unit/Staking.t.sol +++ b/test/unit/Staking.t.sol @@ -27,8 +27,8 @@ contract StakingTest is Test { && participant != rewarded ); vm.assume( - uint160(recipient) > 255 && recipient != address(evc) && recipient != address(distributor) - && recipient != rewarded + uint160(recipient) > 255 && recipient != address(evc) + && !evc.haveCommonOwner(recipient, address(distributor)) && recipient != rewarded ); vm.assume(amount > 0);