Skip to content

Commit

Permalink
Do not include the feeNumerator on the collector's state.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
enriquefynn committed Dec 21, 2023
1 parent 7fb29b2 commit 28ba320
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
21 changes: 9 additions & 12 deletions src/FeeRewardsManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,8 @@ contract RewardsCollector {
);
}

constructor(address _withdrawalCredential, uint32 _feeNumerator) {
constructor(address _withdrawalCredential) {
withdrawalCredential = _withdrawalCredential;
feeNumerator = _feeNumerator;
parentContract = msg.sender;
}

Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down
30 changes: 30 additions & 0 deletions test/FeeRewardsManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit 28ba320

Please sign in to comment.