From 352fe89f1413aed3d607a576054674d17173fd81 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 13:55:26 -0400 Subject: [PATCH 01/13] support tokens in RD, tests todo --- script/arb1-deploy.s.sol | 8 ++++---- script/deploy.s.sol | 2 +- script/nova-deploy.s.sol | 8 ++++---- src/RewardDistributor.sol | 22 ++++++++++++++++++---- test/RewardDistributor.t.sol | 36 ++++++++++++++++++------------------ 5 files changed, 45 insertions(+), 31 deletions(-) diff --git a/script/arb1-deploy.s.sol b/script/arb1-deploy.s.sol index cff56fa..5a099fb 100644 --- a/script/arb1-deploy.s.sol +++ b/script/arb1-deploy.s.sol @@ -19,25 +19,25 @@ contract DeployScript is Script { vm.startBroadcast(); recipients[0] = l2TreasuryTimelock; - RewardDistributor rd_l2base = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l2base = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l2base.transferOwnership(l2Executor); console.log("Deployed L2 Base at: "); console.log(address(rd_l2base)); recipients[0] = l2TreasuryTimelock; - RewardDistributor rd_l2surplus = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l2surplus = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l2surplus.transferOwnership(l2Executor); console.log("Deployed L2 Surplus at: "); console.log(address(rd_l2surplus)); recipients[0] = l2OffchainLabsMultisig; - RewardDistributor rd_l1base = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l1base = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l1base.transferOwnership(l2Executor); console.log("Deployed L1 Base at: "); console.log(address(rd_l1base)); recipients[0] = l2TreasuryTimelock; - RewardDistributor rd_l1surplus = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l1surplus = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l1surplus.transferOwnership(l2Executor); console.log("Deployed L1 Surplus at: "); console.log(address(rd_l1surplus)); diff --git a/script/deploy.s.sol b/script/deploy.s.sol index 5c70936..7b12063 100644 --- a/script/deploy.s.sol +++ b/script/deploy.s.sol @@ -15,7 +15,7 @@ contract DeployScript is Script { weights[0] = 10000; vm.startBroadcast(); - RewardDistributor rd = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); console.log("Deployed RewardDistributor at: "); console.log(address(rd)); vm.stopBroadcast(); diff --git a/script/nova-deploy.s.sol b/script/nova-deploy.s.sol index 249d923..2098cef 100644 --- a/script/nova-deploy.s.sol +++ b/script/nova-deploy.s.sol @@ -42,7 +42,7 @@ contract DeployScript is Script { weights[5] = 373; recipients[6] = Opensea; weights[6] = 133; - RewardDistributor rd_l2base = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l2base = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l2base.transferOwnership(l2ExecutorNova); console.log("Deployed Nova L2 Base at: "); console.log(address(rd_l2base)); @@ -52,19 +52,19 @@ contract DeployScript is Script { weights[0] = 10000; recipients[0] = l1TimelockAlias; - RewardDistributor rd_l2surplus = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l2surplus = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l2surplus.transferOwnership(l2ExecutorNova); console.log("Deployed Nova L2 Surplus at: "); console.log(address(rd_l2surplus)); recipients[0] = l2OffchainLabsMultisigNova; - RewardDistributor rd_l1base = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l1base = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l1base.transferOwnership(l2ExecutorNova); console.log("Deployed Nova L1 Base at: "); console.log(address(rd_l1base)); recipients[0] = l1TimelockAlias; - RewardDistributor rd_l1surplus = new RewardDistributor({recipients: recipients, weights: weights}); + RewardDistributor rd_l1surplus = new RewardDistributor({_token: address(0), recipients: recipients, weights: weights}); rd_l1surplus.transferOwnership(l2ExecutorNova); console.log("Deployed Nova L1 Surplus at: "); console.log(address(rd_l1surplus)); diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index d3278f4..b0a2504 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.16; import {BASIS_POINTS, hashAddresses, hashWeights, uncheckedInc} from "./Util.sol"; import "openzeppelin-contracts/contracts/access/Ownable.sol"; +import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; error TooManyRecipients(); error EmptyRecipients(); @@ -13,10 +15,12 @@ error NoFundsToDistribute(); error InputLengthMismatch(); error InvalidTotalWeight(uint256 totalWeight); -/// @title A distributor of ether +/// @title A distributor of ether or ERC20 tokens /// @notice You can use this contract to distribute ether according to defined weights between a group of participants managed by an owner. /// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. contract RewardDistributor is Ownable { + using SafeERC20 for IERC20; + /// @notice Amount of gas forwarded to each transfer call. /// @dev The recipient group is assumed to be a known group of contracts that won't consume more than this amount. uint256 public constant PER_RECIPIENT_GAS = 100_000; @@ -25,6 +29,8 @@ contract RewardDistributor is Ownable { /// @dev This ensures that all sends may always happen within a block. uint64 public constant MAX_RECIPIENTS = 64; + IERC20 public immutable token; + /// @notice Hash of concat'ed recipient group. bytes32 public currentRecipientGroup; /// @notice Hash of concat'ed recipient weights. @@ -40,10 +46,12 @@ contract RewardDistributor is Ownable { event RecipientsUpdated(bytes32 recipientGroup, address[] recipients, bytes32 recipientWeights, uint256[] weights); /// @notice It is assumed that all recipients are able to receive eth when called with value but no data + /// @param _token Address of the ERC20 token to distribute. Use address(0) for ether. /// @param recipients Addresses to receive rewards. /// @param weights Weights of each recipient in basis points. - constructor(address[] memory recipients, uint256[] memory weights) Ownable() { + constructor(address _token, address[] memory recipients, uint256[] memory weights) Ownable() { setRecipients(recipients, weights); + token = IERC20(_token); } /// @notice allows eth to be deposited into this contract @@ -93,7 +101,7 @@ contract RewardDistributor is Ownable { } // calculate individual reward - uint256 rewards = address(this).balance; + uint256 rewards = address(token) == address(0) ? address(this).balance : token.balanceOf(address(this)); // the reminder will be kept in the contract uint256 rewardPerBps = rewards / BASIS_POINTS; if (rewardPerBps == 0) { @@ -109,7 +117,13 @@ contract RewardDistributor is Ownable { // if the recipient reentry to steal funds, the contract will not have sufficient // funds and revert when trying to send fund to the next recipient // if the recipient is the last, it doesn't matter since there are no extra fund to steal - (bool success,) = recipients[r].call{value: individualRewards, gas: PER_RECIPIENT_GAS}(""); + bool success; + if (address(token) == address(0)) { + (success,) = recipients[r].call{value: individualRewards, gas: PER_RECIPIENT_GAS}(""); + } else { + token.safeTransfer(recipients[r], individualRewards); + success = true; + } // if the funds failed to send we send them to the owner for safe keeping // then the owner will have the opportunity to distribute them out of band diff --git a/test/RewardDistributor.t.sol b/test/RewardDistributor.t.sol index 41a1e7c..1b18a86 100644 --- a/test/RewardDistributor.t.sol +++ b/test/RewardDistributor.t.sol @@ -61,7 +61,7 @@ contract RewardDistributorTest is Test { emit RecipientsUpdated( keccak256(abi.encodePacked(recipients)), recipients, keccak256(abi.encodePacked(weights)), weights ); - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); assertEq(rd.currentRecipientGroup(), keccak256(abi.encodePacked(recipients))); assertEq(rd.owner(), owner); @@ -69,12 +69,12 @@ contract RewardDistributorTest is Test { function testConstructorDoesNotAcceptEmpty() public withContext(0) { vm.expectRevert(EmptyRecipients.selector); - new RewardDistributor(recipients, weights); + new RewardDistributor(address(0), recipients, weights); } function testConstructorDoesNotAcceptPastLimit() public withContext(65) { vm.expectRevert(TooManyRecipients.selector); - new RewardDistributor(recipients, weights); + new RewardDistributor(address(0), recipients, weights); } function testConstructorInputLengthMismatch() public withContext(3) { @@ -82,11 +82,11 @@ contract RewardDistributorTest is Test { shortWeights[0] = weights[0]; shortWeights[1] = weights[1]; vm.expectRevert(InputLengthMismatch.selector); - new RewardDistributor(recipients, shortWeights); + new RewardDistributor(address(0), recipients, shortWeights); } function testUpdateDoesNotAcceptInvalidValues() public withContext(5) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd uint256 reward = 1e8; @@ -124,7 +124,7 @@ contract RewardDistributorTest is Test { } function testDistributeAndUpdateRecipients() public withContext(64) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd uint256 reward = 1e8; @@ -145,7 +145,7 @@ contract RewardDistributorTest is Test { } function testDistributeAndUpdateRecipientsNotOwner() public withContext(64) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); vm.stopPrank(); vm.startPrank(nobody); @@ -159,7 +159,7 @@ contract RewardDistributorTest is Test { } function testDistributeAndUpdateRecipientsBadPrevious() public withContext(64) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); uint256 reward = 1e8; vm.deal(address(rd), reward); @@ -182,7 +182,7 @@ contract RewardDistributorTest is Test { // see testLowSend vm.assume(reward >= BASIS_POINTS); - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd vm.deal(address(rd), reward); @@ -210,7 +210,7 @@ contract RewardDistributorTest is Test { function testLowSend(uint256 rewards) public withContext(8) { vm.assume(rewards < BASIS_POINTS); - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); vm.deal(address(rd), rewards); @@ -220,7 +220,7 @@ contract RewardDistributorTest is Test { function testDistributeRewardsDoesRefundsOwner(uint256 reward) public withContext(3) { vm.assume(reward >= BASIS_POINTS); - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // the empty contract will revert when sending funds to it, as it doesn't // have a fallback. We set the c address to have this code @@ -248,7 +248,7 @@ contract RewardDistributorTest is Test { } function testDistributeRewardsDoesNotDistributeToEmpty() public withContext(3) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd uint256 reward = 1e8; @@ -262,7 +262,7 @@ contract RewardDistributorTest is Test { } function testDistributeRewardsDoesNotDistributeWrongRecipients() public withContext(3) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd uint256 reward = 1e8; @@ -282,7 +282,7 @@ contract RewardDistributorTest is Test { } function testDistributeRewardsDoesNotDistributeWrongWeights() public withContext(3) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd uint256 reward = 1e8; @@ -303,7 +303,7 @@ contract RewardDistributorTest is Test { } function testDistributeRewardsDoesNotDistributeToWrongCount() public withContext(3) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // increase the balance of rd uint256 reward = 1e8; @@ -320,7 +320,7 @@ contract RewardDistributorTest is Test { } function testDistributeRewardsFailsToRefundsOwner() public withContext(3) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); // the empty contract will revert when sending funds to it, as it doesn't // have a fallback. We set the c address and the owner to have this code @@ -342,7 +342,7 @@ contract RewardDistributorTest is Test { } function testDistributeRewardsInputLengthMismatch() public withContext(3) { - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); uint256[] memory shortWeights = new uint256[](2); shortWeights[0] = weights[0]; shortWeights[1] = weights[1]; @@ -356,7 +356,7 @@ contract RewardDistributorTest is Test { for (uint256 i = 0; i < recipients.length; i++) { recipients[i] = address(new Reverter()); } - RewardDistributor rd = new RewardDistributor(recipients, weights); + RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); assertEq(MAX_RECIPIENTS, rd.MAX_RECIPIENTS()); uint256 rewards = 5 ether; From 518ff7b471369bb511fea5c4ed0fd82e67897d4f Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 14:05:06 -0400 Subject: [PATCH 02/13] protect receive() --- src/RewardDistributor.sol | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index b0a2504..926d94a 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -6,6 +6,7 @@ import "openzeppelin-contracts/contracts/access/Ownable.sol"; import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; +error CannotReceiveNative(); error TooManyRecipients(); error EmptyRecipients(); error InvalidRecipientGroup(bytes32 currentRecipientGroup, bytes32 providedRecipientGroup); @@ -55,8 +56,12 @@ contract RewardDistributor is Ownable { } /// @notice allows eth to be deposited into this contract - /// @dev this contract is expected to handle ether appearing in its balance as well as an explicit deposit - receive() external payable {} + /// @dev this contract is expected to handle ether appearing in its balance as well as an explicit deposit as long as token == address(0) + receive() external payable { + if (address(token) != address(0)) { + revert CannotReceiveNative(); + } + } /** * @notice Distributes previous rewards then updates the recipients to a new group. From 4a20501ee33f369972f7bcad5138abb43f288494 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 15:27:47 -0400 Subject: [PATCH 03/13] erc20 mode tests --- test/RewardDistributor.t.sol | 38 +++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/test/RewardDistributor.t.sol b/test/RewardDistributor.t.sol index 1b18a86..f4172a5 100644 --- a/test/RewardDistributor.t.sol +++ b/test/RewardDistributor.t.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.16; import "../src/RewardDistributor.sol"; import "./Reverter.sol"; import "./Empty.sol"; +import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; import "forge-std/Test.sol"; contract RewardDistributorTest is Test { @@ -177,15 +178,17 @@ contract RewardDistributorTest is Test { address zero = 0x0000000000000000000000000000000000000000; - function testDistributeRewards(uint256 reward) public withContext(3) { + function _distributeRewards(uint256 reward, bool useToken) public { // If reward is less than recipient.length, we expect to throw an error // see testLowSend vm.assume(reward >= BASIS_POINTS); - RewardDistributor rd = new RewardDistributor(address(0), recipients, weights); + ERC20PresetMinterPauser token = new ERC20PresetMinterPauser("token", "TKN"); + + RewardDistributor rd = new RewardDistributor(useToken ? address(token) : address(0), recipients, weights); // increase the balance of rd - vm.deal(address(rd), reward); + useToken ? token.mint(address(rd), reward) : vm.deal(address(rd), reward); vm.expectEmit(true, false, false, true); emit RecipientRecieved(recipients[0], reward / BASIS_POINTS * weights[0]); @@ -199,12 +202,17 @@ contract RewardDistributorTest is Test { // anyone should be able to call distributeRewards rd.distributeRewards(recipients, weights); - assertEq(recipients[0].balance, reward / BASIS_POINTS * weights[0], "a balance"); - assertEq(recipients[1].balance, reward / BASIS_POINTS * weights[1], "b balance"); - assertEq(recipients[2].balance, reward / BASIS_POINTS * weights[2], "c balance"); - assertEq(owner.balance, 0, "owner balance"); - assertEq(nobody.balance, 0, "nobody balance"); - assertEq(address(rd).balance, reward % BASIS_POINTS, "rewards balance"); + assertEq(useToken ? token.balanceOf(recipients[0]) : recipients[0].balance, reward / BASIS_POINTS * weights[0], "a balance"); + assertEq(useToken ? token.balanceOf(recipients[1]) : recipients[1].balance, reward / BASIS_POINTS * weights[1], "b balance"); + assertEq(useToken ? token.balanceOf(recipients[2]) : recipients[2].balance, reward / BASIS_POINTS * weights[2], "c balance"); + assertEq(useToken ? token.balanceOf(owner) : owner.balance, 0, "owner balance"); + assertEq(useToken ? token.balanceOf(nobody) : nobody.balance, 0, "nobody balance"); + assertEq(useToken ? token.balanceOf(address(rd)) : address(rd).balance, reward % BASIS_POINTS, "rewards balance"); + } + + function testDistributeRewards(uint256 reward) public withContext(3) { + _distributeRewards(reward, false); + _distributeRewards(reward, true); } function testLowSend(uint256 rewards) public withContext(8) { @@ -414,4 +422,16 @@ contract RewardDistributorTest is Test { actual = uncheckedInc(type(uint256).max); assertEq(actual, expected, "incorrect overflow increment"); } + + function testCannotReceiveInERC20Mode() public { + recipients = makeRecipientGroup(3); + weights = makeRecipientWeights(3); + RewardDistributor rd = new RewardDistributor(address(1), recipients, weights); + + vm.deal(address(this), 1); + (bool success, bytes memory ret) = address(rd).call{value: 1}(""); + + assertFalse(success); + assertEq(ret, abi.encodeWithSelector(CannotReceiveNative.selector)); + } } From ab63ae39bf9a82ee83c541ca3f4e292ba4b5bfaa Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 15:29:31 -0400 Subject: [PATCH 04/13] fix e2e test --- test/e2e/FundRouters.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/FundRouters.test.ts b/test/e2e/FundRouters.test.ts index 342e87d..771c2f0 100644 --- a/test/e2e/FundRouters.test.ts +++ b/test/e2e/FundRouters.test.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import { TestSetup, testSetup } from "./testSetup"; -import { BigNumber, utils, Wallet } from "ethers"; +import { BigNumber, ethers, utils, Wallet } from "ethers"; import { ParentToChildRewardRouter__factory, ParentToChildRewardRouter, @@ -115,7 +115,7 @@ describe("Router e2e test", () => { rewardDistributor = await new RewardDistributor__factory( setup.l2Signer - ).deploy([childToParentRewardRouter.address], [10000]); + ).deploy(ethers.constants.AddressZero, [childToParentRewardRouter.address], [10000]); console.log("Reward Distributor deployed:", rewardDistributor.address); }); From faa73ab02b2b3d1aa061462d9beb211edf077009 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 15:31:51 -0400 Subject: [PATCH 05/13] fix import --- test/RewardDistributor.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/RewardDistributor.t.sol b/test/RewardDistributor.t.sol index f4172a5..72f9358 100644 --- a/test/RewardDistributor.t.sol +++ b/test/RewardDistributor.t.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.16; import "../src/RewardDistributor.sol"; import "./Reverter.sol"; import "./Empty.sol"; -import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; +import {ERC20PresetMinterPauser} from "openzeppelin-contracts/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; import "forge-std/Test.sol"; contract RewardDistributorTest is Test { From e6057787604560412db80c0f7a625cdf1cd2c5fe Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 15:33:34 -0400 Subject: [PATCH 06/13] natspec --- src/RewardDistributor.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index 926d94a..fd8f957 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -16,9 +16,10 @@ error NoFundsToDistribute(); error InputLengthMismatch(); error InvalidTotalWeight(uint256 totalWeight); -/// @title A distributor of ether or ERC20 tokens -/// @notice You can use this contract to distribute ether according to defined weights between a group of participants managed by an owner. +/// @title A distributor of ether or an ERC20 token +/// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. /// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. +/// A RewardDistributor can only handle a single, specific asset defined at deployment. contract RewardDistributor is Ownable { using SafeERC20 for IERC20; From 5fd6cd0e4dcab35cfcac3e610ddfe5c033d17acd Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 16:59:19 -0400 Subject: [PATCH 07/13] token comments --- src/RewardDistributor.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index fd8f957..3d7905a 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -20,6 +20,7 @@ error InvalidTotalWeight(uint256 totalWeight); /// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. /// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. /// A RewardDistributor can only handle a single, specific asset defined at deployment. +/// This contract assumes that the token is not blacklistable or has other non standard behavior. contract RewardDistributor is Ownable { using SafeERC20 for IERC20; @@ -127,6 +128,7 @@ contract RewardDistributor is Ownable { if (address(token) == address(0)) { (success,) = recipients[r].call{value: individualRewards, gas: PER_RECIPIENT_GAS}(""); } else { + // we assume that this will never revert, because we know we have enough token and the token is "normal" token.safeTransfer(recipients[r], individualRewards); success = true; } @@ -136,6 +138,7 @@ contract RewardDistributor is Ownable { if (success) { emit RecipientRecieved(recipients[r], individualRewards); } else { + // this case will never be hit if we are using an ERC20 token // cache owner in memory address _owner = owner(); (bool ownerSuccess,) = _owner.call{value: individualRewards}(""); From fafc2512f2754221a789fa9e22120c2f37b91575 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 21 Mar 2025 17:03:41 -0400 Subject: [PATCH 08/13] rephrase --- src/RewardDistributor.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index 3d7905a..5773b0b 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -20,7 +20,7 @@ error InvalidTotalWeight(uint256 totalWeight); /// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. /// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. /// A RewardDistributor can only handle a single, specific asset defined at deployment. -/// This contract assumes that the token is not blacklistable or has other non standard behavior. +/// This contract assumes that the token does not have a blacklist or other non standard behavior. contract RewardDistributor is Ownable { using SafeERC20 for IERC20; From c434d728b24818a49fb82bdddb60052ee875f160 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 18 Apr 2025 13:25:47 -0500 Subject: [PATCH 09/13] Update src/RewardDistributor.sol Co-authored-by: gzeon --- src/RewardDistributor.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index 5773b0b..81bfb6c 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -20,7 +20,7 @@ error InvalidTotalWeight(uint256 totalWeight); /// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. /// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. /// A RewardDistributor can only handle a single, specific asset defined at deployment. -/// This contract assumes that the token does not have a blacklist or other non standard behavior. +/// This contract assumes that the token does not have a blacklist, callback or other non standard ERC20 behaviors. contract RewardDistributor is Ownable { using SafeERC20 for IERC20; From edd4c2c9c758cef027c9e67fe7fabe35ee478dc5 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 18 Apr 2025 13:26:36 -0500 Subject: [PATCH 10/13] add rescue function to RD --- src/RewardDistributor.sol | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index 81bfb6c..e5b023d 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -15,6 +15,8 @@ error OwnerFailedRecieve(address owner, address recipient, uint256 value); error NoFundsToDistribute(); error InputLengthMismatch(); error InvalidTotalWeight(uint256 totalWeight); +error CannotCallRescueToToken(); +error CannotCallRescueWithValue(); /// @title A distributor of ether or an ERC20 token /// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. @@ -190,4 +192,27 @@ contract RewardDistributor is Ownable { emit RecipientsUpdated(recipientGroup, recipients, recipientWeights, weights); } + + /** + * @notice Allows the owner to call any address with a value and data. + * If the wrong asset is sent to the contract then this allows the owner to recover it. + * @dev Calls to the token address are not allowed. + * Calls with value are not allowed if the token address is 0. + * @param to Address to call + * @param value Callvalue to send + * @param data Calldata to send + */ + function rescue(address to, uint256 value, bytes memory data) external onlyOwner { + if (address(token) == to) { + revert CannotCallRescueToToken(); + } + if (address(token) == address(0) && value > 0) { + revert CannotCallRescueWithValue(); + } + + (bool success,) = to.call{value: value}(data); + if (!success) { + revert OwnerFailedRecieve(owner(), to, value); + } + } } From 6f757134d296b33ed9557dfbc51a8dcc54ed3618 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 18 Apr 2025 13:30:51 -0500 Subject: [PATCH 11/13] testRescue --- src/RewardDistributor.sol | 6 +++--- test/RewardDistributor.t.sol | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index e5b023d..500c4ef 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -203,12 +203,12 @@ contract RewardDistributor is Ownable { * @param data Calldata to send */ function rescue(address to, uint256 value, bytes memory data) external onlyOwner { - if (address(token) == to) { - revert CannotCallRescueToToken(); - } if (address(token) == address(0) && value > 0) { revert CannotCallRescueWithValue(); } + if (address(token) == to) { + revert CannotCallRescueToToken(); + } (bool success,) = to.call{value: value}(data); if (!success) { diff --git a/test/RewardDistributor.t.sol b/test/RewardDistributor.t.sol index 72f9358..d1b488e 100644 --- a/test/RewardDistributor.t.sol +++ b/test/RewardDistributor.t.sol @@ -434,4 +434,24 @@ contract RewardDistributorTest is Test { assertFalse(success); assertEq(ret, abi.encodeWithSelector(CannotReceiveNative.selector)); } + + function testRescue() public { + recipients = makeRecipientGroup(3); + weights = makeRecipientWeights(3); + RewardDistributor rdEth = new RewardDistributor(address(0), recipients, weights); + RewardDistributor rdToken = new RewardDistributor(address(1), recipients, weights); + + // rescue should be only owner + vm.prank(nobody); + vm.expectRevert("Ownable: caller is not the owner"); + rdEth.rescue(address(0), 1, ""); + + // cannot send value in rdEth.rescue + vm.expectRevert(CannotCallRescueWithValue.selector); + rdEth.rescue(address(0), 1, ""); + + // cannot call token contract in rdToken.rescue + vm.expectRevert(CannotCallRescueToToken.selector); + rdToken.rescue(address(1), 1, ""); + } } From 188314c2fcbd580a51203d37ec70538b8c762320 Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 18 Apr 2025 13:36:27 -0500 Subject: [PATCH 12/13] typos --- src/RewardDistributor.sol | 22 +++++++++++----------- test/RewardDistributor.t.sol | 18 +++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index 500c4ef..55ac16e 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -11,7 +11,7 @@ error TooManyRecipients(); error EmptyRecipients(); error InvalidRecipientGroup(bytes32 currentRecipientGroup, bytes32 providedRecipientGroup); error InvalidRecipientWeights(bytes32 currentRecipientWeights, bytes32 providedRecipientWeights); -error OwnerFailedRecieve(address owner, address recipient, uint256 value); +error OwnerFailedReceive(address owner, address recipient, uint256 value); error NoFundsToDistribute(); error InputLengthMismatch(); error InvalidTotalWeight(uint256 totalWeight); @@ -20,7 +20,7 @@ error CannotCallRescueWithValue(); /// @title A distributor of ether or an ERC20 token /// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. -/// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. +/// @dev If a particular recipient is not able to receive funds at their address, the payment will fallback to the owner. /// A RewardDistributor can only handle a single, specific asset defined at deployment. /// This contract assumes that the token does not have a blacklist, callback or other non standard ERC20 behaviors. contract RewardDistributor is Ownable { @@ -42,10 +42,10 @@ contract RewardDistributor is Ownable { bytes32 public currentRecipientWeights; /// @notice The recipient couldn't receive rewards, so fallback to owner was triggered. - event OwnerRecieved(address indexed owner, address indexed recipient, uint256 value); + event OwnerReceived(address indexed owner, address indexed recipient, uint256 value); /// @notice Address successfully received rewards. - event RecipientRecieved(address indexed recipient, uint256 value); + event RecipientReceived(address indexed recipient, uint256 value); /// @notice New recipients have been set event RecipientsUpdated(bytes32 recipientGroup, address[] recipients, bytes32 recipientWeights, uint256[] weights); @@ -111,7 +111,7 @@ contract RewardDistributor is Ownable { // calculate individual reward uint256 rewards = address(token) == address(0) ? address(this).balance : token.balanceOf(address(this)); - // the reminder will be kept in the contract + // the remainder will be kept in the contract uint256 rewardPerBps = rewards / BASIS_POINTS; if (rewardPerBps == 0) { revert NoFundsToDistribute(); @@ -138,7 +138,7 @@ contract RewardDistributor is Ownable { // if the funds failed to send we send them to the owner for safe keeping // then the owner will have the opportunity to distribute them out of band if (success) { - emit RecipientRecieved(recipients[r], individualRewards); + emit RecipientReceived(recipients[r], individualRewards); } else { // this case will never be hit if we are using an ERC20 token // cache owner in memory @@ -148,9 +148,9 @@ contract RewardDistributor is Ownable { // it's important that this fail in order to preserve the accounting in this contract. // if we dont fail here we enable a re-entrancy attack if (!ownerSuccess) { - revert OwnerFailedRecieve(_owner, recipients[r], individualRewards); + revert OwnerFailedReceive(_owner, recipients[r], individualRewards); } - emit OwnerRecieved(_owner, recipients[r], individualRewards); + emit OwnerReceived(_owner, recipients[r], individualRewards); } } } @@ -182,11 +182,11 @@ contract RewardDistributor is Ownable { revert InvalidTotalWeight(totalWeight); } - // create a committment to the recipient group and update current + // create a commitment to the recipient group and update current bytes32 recipientGroup = hashAddresses(recipients); currentRecipientGroup = recipientGroup; - // create a committment to the recipient weights and update current + // create a commitment to the recipient weights and update current bytes32 recipientWeights = hashWeights(weights); currentRecipientWeights = recipientWeights; @@ -212,7 +212,7 @@ contract RewardDistributor is Ownable { (bool success,) = to.call{value: value}(data); if (!success) { - revert OwnerFailedRecieve(owner(), to, value); + revert OwnerFailedReceive(owner(), to, value); } } } diff --git a/test/RewardDistributor.t.sol b/test/RewardDistributor.t.sol index d1b488e..fc609b6 100644 --- a/test/RewardDistributor.t.sol +++ b/test/RewardDistributor.t.sol @@ -9,8 +9,8 @@ import {ERC20PresetMinterPauser} from "openzeppelin-contracts/contracts/token/ER import "forge-std/Test.sol"; contract RewardDistributorTest is Test { - event OwnerRecieved(address indexed owner, address indexed recipient, uint256 value); - event RecipientRecieved(address indexed recipient, uint256 value); + event OwnerReceived(address indexed owner, address indexed recipient, uint256 value); + event RecipientReceived(address indexed recipient, uint256 value); event RecipientsUpdated(bytes32 recipientGroup, address[] recipients, bytes32 recipientWeights, uint256[] weights); event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); @@ -191,11 +191,11 @@ contract RewardDistributorTest is Test { useToken ? token.mint(address(rd), reward) : vm.deal(address(rd), reward); vm.expectEmit(true, false, false, true); - emit RecipientRecieved(recipients[0], reward / BASIS_POINTS * weights[0]); + emit RecipientReceived(recipients[0], reward / BASIS_POINTS * weights[0]); vm.expectEmit(true, false, false, true); - emit RecipientRecieved(recipients[1], reward / BASIS_POINTS * weights[1]); + emit RecipientReceived(recipients[1], reward / BASIS_POINTS * weights[1]); vm.expectEmit(true, false, false, true); - emit RecipientRecieved(recipients[2], reward / BASIS_POINTS * weights[2]); + emit RecipientReceived(recipients[2], reward / BASIS_POINTS * weights[2]); vm.stopPrank(); vm.startPrank(nobody); @@ -239,11 +239,11 @@ contract RewardDistributorTest is Test { vm.deal(address(rd), reward); vm.expectEmit(true, false, false, true); - emit RecipientRecieved(recipients[0], reward / BASIS_POINTS * weights[0]); + emit RecipientReceived(recipients[0], reward / BASIS_POINTS * weights[0]); vm.expectEmit(true, false, false, true); - emit RecipientRecieved(recipients[1], reward / BASIS_POINTS * weights[1]); + emit RecipientReceived(recipients[1], reward / BASIS_POINTS * weights[1]); vm.expectEmit(true, false, false, true); - emit OwnerRecieved(owner, recipients[2], reward / BASIS_POINTS * weights[2]); + emit OwnerReceived(owner, recipients[2], reward / BASIS_POINTS * weights[2]); rd.distributeRewards(recipients, weights); @@ -342,7 +342,7 @@ contract RewardDistributorTest is Test { vm.expectRevert( abi.encodeWithSelector( - OwnerFailedRecieve.selector, owner, recipients[2], (reward / BASIS_POINTS * weights[2]) + OwnerFailedReceive.selector, owner, recipients[2], (reward / BASIS_POINTS * weights[2]) ) ); From f0e72807b215f38bed9d4b889fa1857bbd34e19d Mon Sep 17 00:00:00 2001 From: Henry <11198460+godzillaba@users.noreply.github.com> Date: Fri, 18 Apr 2025 13:39:55 -0500 Subject: [PATCH 13/13] Revert "typos" This reverts commit 188314c2fcbd580a51203d37ec70538b8c762320. --- src/RewardDistributor.sol | 22 +++++++++++----------- test/RewardDistributor.t.sol | 18 +++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/RewardDistributor.sol b/src/RewardDistributor.sol index 55ac16e..500c4ef 100644 --- a/src/RewardDistributor.sol +++ b/src/RewardDistributor.sol @@ -11,7 +11,7 @@ error TooManyRecipients(); error EmptyRecipients(); error InvalidRecipientGroup(bytes32 currentRecipientGroup, bytes32 providedRecipientGroup); error InvalidRecipientWeights(bytes32 currentRecipientWeights, bytes32 providedRecipientWeights); -error OwnerFailedReceive(address owner, address recipient, uint256 value); +error OwnerFailedRecieve(address owner, address recipient, uint256 value); error NoFundsToDistribute(); error InputLengthMismatch(); error InvalidTotalWeight(uint256 totalWeight); @@ -20,7 +20,7 @@ error CannotCallRescueWithValue(); /// @title A distributor of ether or an ERC20 token /// @notice You can use this contract to distribute ether/token according to defined weights between a group of participants managed by an owner. -/// @dev If a particular recipient is not able to receive funds at their address, the payment will fallback to the owner. +/// @dev If a particular recipient is not able to recieve funds at their address, the payment will fallback to the owner. /// A RewardDistributor can only handle a single, specific asset defined at deployment. /// This contract assumes that the token does not have a blacklist, callback or other non standard ERC20 behaviors. contract RewardDistributor is Ownable { @@ -42,10 +42,10 @@ contract RewardDistributor is Ownable { bytes32 public currentRecipientWeights; /// @notice The recipient couldn't receive rewards, so fallback to owner was triggered. - event OwnerReceived(address indexed owner, address indexed recipient, uint256 value); + event OwnerRecieved(address indexed owner, address indexed recipient, uint256 value); /// @notice Address successfully received rewards. - event RecipientReceived(address indexed recipient, uint256 value); + event RecipientRecieved(address indexed recipient, uint256 value); /// @notice New recipients have been set event RecipientsUpdated(bytes32 recipientGroup, address[] recipients, bytes32 recipientWeights, uint256[] weights); @@ -111,7 +111,7 @@ contract RewardDistributor is Ownable { // calculate individual reward uint256 rewards = address(token) == address(0) ? address(this).balance : token.balanceOf(address(this)); - // the remainder will be kept in the contract + // the reminder will be kept in the contract uint256 rewardPerBps = rewards / BASIS_POINTS; if (rewardPerBps == 0) { revert NoFundsToDistribute(); @@ -138,7 +138,7 @@ contract RewardDistributor is Ownable { // if the funds failed to send we send them to the owner for safe keeping // then the owner will have the opportunity to distribute them out of band if (success) { - emit RecipientReceived(recipients[r], individualRewards); + emit RecipientRecieved(recipients[r], individualRewards); } else { // this case will never be hit if we are using an ERC20 token // cache owner in memory @@ -148,9 +148,9 @@ contract RewardDistributor is Ownable { // it's important that this fail in order to preserve the accounting in this contract. // if we dont fail here we enable a re-entrancy attack if (!ownerSuccess) { - revert OwnerFailedReceive(_owner, recipients[r], individualRewards); + revert OwnerFailedRecieve(_owner, recipients[r], individualRewards); } - emit OwnerReceived(_owner, recipients[r], individualRewards); + emit OwnerRecieved(_owner, recipients[r], individualRewards); } } } @@ -182,11 +182,11 @@ contract RewardDistributor is Ownable { revert InvalidTotalWeight(totalWeight); } - // create a commitment to the recipient group and update current + // create a committment to the recipient group and update current bytes32 recipientGroup = hashAddresses(recipients); currentRecipientGroup = recipientGroup; - // create a commitment to the recipient weights and update current + // create a committment to the recipient weights and update current bytes32 recipientWeights = hashWeights(weights); currentRecipientWeights = recipientWeights; @@ -212,7 +212,7 @@ contract RewardDistributor is Ownable { (bool success,) = to.call{value: value}(data); if (!success) { - revert OwnerFailedReceive(owner(), to, value); + revert OwnerFailedRecieve(owner(), to, value); } } } diff --git a/test/RewardDistributor.t.sol b/test/RewardDistributor.t.sol index fc609b6..d1b488e 100644 --- a/test/RewardDistributor.t.sol +++ b/test/RewardDistributor.t.sol @@ -9,8 +9,8 @@ import {ERC20PresetMinterPauser} from "openzeppelin-contracts/contracts/token/ER import "forge-std/Test.sol"; contract RewardDistributorTest is Test { - event OwnerReceived(address indexed owner, address indexed recipient, uint256 value); - event RecipientReceived(address indexed recipient, uint256 value); + event OwnerRecieved(address indexed owner, address indexed recipient, uint256 value); + event RecipientRecieved(address indexed recipient, uint256 value); event RecipientsUpdated(bytes32 recipientGroup, address[] recipients, bytes32 recipientWeights, uint256[] weights); event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); @@ -191,11 +191,11 @@ contract RewardDistributorTest is Test { useToken ? token.mint(address(rd), reward) : vm.deal(address(rd), reward); vm.expectEmit(true, false, false, true); - emit RecipientReceived(recipients[0], reward / BASIS_POINTS * weights[0]); + emit RecipientRecieved(recipients[0], reward / BASIS_POINTS * weights[0]); vm.expectEmit(true, false, false, true); - emit RecipientReceived(recipients[1], reward / BASIS_POINTS * weights[1]); + emit RecipientRecieved(recipients[1], reward / BASIS_POINTS * weights[1]); vm.expectEmit(true, false, false, true); - emit RecipientReceived(recipients[2], reward / BASIS_POINTS * weights[2]); + emit RecipientRecieved(recipients[2], reward / BASIS_POINTS * weights[2]); vm.stopPrank(); vm.startPrank(nobody); @@ -239,11 +239,11 @@ contract RewardDistributorTest is Test { vm.deal(address(rd), reward); vm.expectEmit(true, false, false, true); - emit RecipientReceived(recipients[0], reward / BASIS_POINTS * weights[0]); + emit RecipientRecieved(recipients[0], reward / BASIS_POINTS * weights[0]); vm.expectEmit(true, false, false, true); - emit RecipientReceived(recipients[1], reward / BASIS_POINTS * weights[1]); + emit RecipientRecieved(recipients[1], reward / BASIS_POINTS * weights[1]); vm.expectEmit(true, false, false, true); - emit OwnerReceived(owner, recipients[2], reward / BASIS_POINTS * weights[2]); + emit OwnerRecieved(owner, recipients[2], reward / BASIS_POINTS * weights[2]); rd.distributeRewards(recipients, weights); @@ -342,7 +342,7 @@ contract RewardDistributorTest is Test { vm.expectRevert( abi.encodeWithSelector( - OwnerFailedReceive.selector, owner, recipients[2], (reward / BASIS_POINTS * weights[2]) + OwnerFailedRecieve.selector, owner, recipients[2], (reward / BASIS_POINTS * weights[2]) ) );