From 28ba32079f6086f2bbcb21c674c6530011db2258 Mon Sep 17 00:00:00 2001
From: Fynn <me@enriquefynn.com>
Date: Wed, 20 Dec 2023 23:01:19 -0300
Subject: [PATCH] Do not include the feeNumerator on the collector's state.

1. The user uses this endpoint to calculate the RewardsCollector address
   to pass to ChorusDeposit.deposit as withdrawalCredential before
deploying this RewardsCollector contract
2. The owner of FeeRewardsManager changes the defaultFeeNumerator

The user would not be able to deploy the RewardsCollector at the
predicted address since defaultFeeNumerator has been changed so the fees
get locked in that address unless the owner changes the
defaultFeeNumerator back to the value at step 1. so that the user would
be able to deploy the correct RewardsCollector.
---
 src/FeeRewardsManager.sol    | 21 +++++++++------------
 test/FeeRewardsManager.t.sol | 30 ++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/FeeRewardsManager.sol b/src/FeeRewardsManager.sol
index d9a6bd9..6e19131 100644
--- a/src/FeeRewardsManager.sol
+++ b/src/FeeRewardsManager.sol
@@ -71,9 +71,8 @@ contract RewardsCollector {
         );
     }
 
-    constructor(address _withdrawalCredential, uint32 _feeNumerator) {
+    constructor(address _withdrawalCredential) {
         withdrawalCredential = _withdrawalCredential;
-        feeNumerator = _feeNumerator;
         parentContract = msg.sender;
     }
 
@@ -108,15 +107,13 @@ contract FeeRewardsManager is Ownable {
         bytes32 withdrawalCredentialBytes = bytes32(
             uint256(uint160(_withdrawalCredential))
         );
-        address addr = address(
-            // Uses CREATE2 opcode.
-            new RewardsCollector{salt: withdrawalCredentialBytes}(
-                _withdrawalCredential,
-                defaultFeeNumerator
-            )
-        );
-        emit ContractDeployed(addr, defaultFeeNumerator);
-        return payable(addr);
+        // Uses CREATE2 opcode.
+        RewardsCollector rewardsCollector = new RewardsCollector{
+            salt: withdrawalCredentialBytes
+        }(_withdrawalCredential);
+        rewardsCollector.changeFeeNumerator(defaultFeeNumerator);
+        emit ContractDeployed(address(rewardsCollector), defaultFeeNumerator);
+        return payable(address(rewardsCollector));
     }
 
     // Predicts the address of a new contract that will be a `fee_recipient` of
@@ -131,7 +128,7 @@ contract FeeRewardsManager is Ownable {
         bytes memory bytecode = type(RewardsCollector).creationCode;
         bytecode = abi.encodePacked(
             bytecode,
-            abi.encode(_withdrawalCredential, defaultFeeNumerator)
+            abi.encode(_withdrawalCredential)
         );
         bytes32 withdrawalCredentialBytes = bytes32(
             uint256(uint160(_withdrawalCredential))
diff --git a/test/FeeRewardsManager.t.sol b/test/FeeRewardsManager.t.sol
index 76773d7..2e85302 100644
--- a/test/FeeRewardsManager.t.sol
+++ b/test/FeeRewardsManager.t.sol
@@ -213,4 +213,34 @@ contract FeeRewardsTest is Test {
         vm.expectRevert("Invalid fee numerator");
         feeRewardsManager.changeFeeNumerator(payable(addr), 10_001);
     }
+
+    function testChangeFeeNumeratorAndWatchPredictedContract() public {
+        address withdrawalCredential = address(100);
+        vm.deal(address(0), 10 ether);
+        address derivedAddr = feeRewardsManager.predictFeeContractAddress(
+            withdrawalCredential
+        );
+        feeRewardsManager.changeDefaultFee(10_000);
+        address derivedAddr2 = feeRewardsManager.predictFeeContractAddress(
+            withdrawalCredential
+        );
+        assert(derivedAddr == derivedAddr2);
+    }
+
+    function testDerivedAddress() public {
+        address withdrawalCredential = address(100);
+        vm.deal(address(0), 10 ether);
+        address derivedAddr = feeRewardsManager.predictFeeContractAddress(
+            withdrawalCredential
+        );
+
+        vm.deal(derivedAddr, 10 ether);
+
+        address payable addr = feeRewardsManager.createFeeContract(
+            withdrawalCredential
+        );
+
+        //derived address matches the function to get one.
+        assertEq(derivedAddr, addr);
+    }
 }