diff --git a/src/FeeRewardsManager.sol b/src/FeeRewardsManager.sol index f7777ad..ca48a06 100644 --- a/src/FeeRewardsManager.sol +++ b/src/FeeRewardsManager.sol @@ -14,10 +14,10 @@ contract RewardsCollector is Ownable { // 1 - fee % will go to the user in this address. address public withdrawalCredential; - // Nominator of the fee. - uint32 public feeNominator; + // Fee's numerator. + uint32 public feeNumerator; - // Fee denominator, if `feeNominator = 500`, + // Fee denominator, if `feeNumerator = 500`, // the tax is 500/10000 = 5/100 = 5%. uint32 public constant FEE_DENOMINATOR = 10000; @@ -25,7 +25,7 @@ contract RewardsCollector is Ownable { receive() external payable {} function collectRewards() public payable { - uint256 ownerAmount = (address(this).balance * feeNominator) / + uint256 ownerAmount = (address(this).balance * feeNumerator) / FEE_DENOMINATOR; uint256 returnedAmount = address(this).balance - ownerAmount; require( @@ -48,43 +48,43 @@ contract RewardsCollector is Ownable { require(sent, "Failed to send Ether back to withdrawal credential"); } - constructor(address _withdrawalCredential, uint32 _feeNominator) { + constructor(address _withdrawalCredential, uint32 _feeNumerator) { withdrawalCredential = _withdrawalCredential; - feeNominator = _feeNominator; + feeNumerator = _feeNumerator; } - function changeFee(uint32 _newFee) public onlyOwner { - feeNominator = _newFee; + function changeFee(uint32 _newFeeNumerator) public onlyOwner { + feeNumerator = _newFeeNumerator; } } contract FeeRewardsManager is Ownable { - uint32 public defaultFeeNominator; + uint32 public defaultFeeNumerator; - constructor(uint32 _defaultFeeNominator) { - defaultFeeNominator = _defaultFeeNominator; + constructor(uint32 _defaultFeeNumerator) { + defaultFeeNumerator = _defaultFeeNumerator; } - event ContractDeployed(address contractAddress, uint32 feeNominator); + event ContractDeployed(address contractAddress, uint32 feeNumerator); - function changeDefaultFee(uint32 _newFeeNominator) public onlyOwner { - defaultFeeNominator = _newFeeNominator; + function changeDefaultFee(uint32 _newFeeNumerator) public onlyOwner { + defaultFeeNumerator = _newFeeNumerator; } function createFeeContract( address _withdrawalCredential ) public returns (address payable) { bytes32 withdrawalCredentialBytes = bytes32( - uint256(uint160(_withdrawalCredential)) << 96 + uint256(uint160(_withdrawalCredential)) ); address addr = address( // Uses CREATE2 opcode. new RewardsCollector{salt: withdrawalCredentialBytes}( _withdrawalCredential, - defaultFeeNominator + defaultFeeNumerator ) ); - emit ContractDeployed(addr, defaultFeeNominator); + emit ContractDeployed(addr, defaultFeeNumerator); return payable(addr); } @@ -94,10 +94,10 @@ contract FeeRewardsManager is Ownable { bytes memory bytecode = type(RewardsCollector).creationCode; bytecode = abi.encodePacked( bytecode, - abi.encode(_withdrawalCredential, defaultFeeNominator) + abi.encode(_withdrawalCredential, defaultFeeNumerator) ); bytes32 withdrawalCredentialBytes = bytes32( - uint256(uint160(_withdrawalCredential)) << 96 + uint256(uint160(_withdrawalCredential)) ); bytes32 hash = keccak256( abi.encodePacked( @@ -127,6 +127,7 @@ contract FeeRewardsManager is Ownable { receive() external payable {} + // Withdraws Eth from the manager contract. function getEth(address addr) external onlyOwner { payable(addr).transfer(address(this).balance); } diff --git a/test/FeeRewardsManager.t.sol b/test/FeeRewardsManager.t.sol index f9ace75..a8da436 100644 --- a/test/FeeRewardsManager.t.sol +++ b/test/FeeRewardsManager.t.sol @@ -99,7 +99,7 @@ contract FeeRewardsTest is Test { function testChangeDefaultFee() public { feeRewardsManager.changeDefaultFee(100); - assertEq(feeRewardsManager.defaultFeeNominator(), 100); + assertEq(feeRewardsManager.defaultFeeNumerator(), 100); address addr = address( createWithdrawalSimulateRewards(address(100), 10 ether) @@ -145,7 +145,7 @@ contract FeeRewardsTest is Test { rewards ); uint256 chorusAmount = (address(collector).balance * - uint256(collector.feeNominator())) / collector.FEE_DENOMINATOR(); + uint256(collector.feeNumerator())) / collector.FEE_DENOMINATOR(); uint256 withdrawalCredentialsAmount = address(collector).balance - chorusAmount; uint256 chorusBalanceBefore = address(feeRewardsManager).balance; @@ -161,6 +161,9 @@ contract FeeRewardsTest is Test { ); } + // Test calling `collectRewards` from a contract that calls `collectRewards` + // again, this will revert as the Ether is divided just to Chorus and the + // withdrawal credential. function testReentrantAttack() public { ReentrantAttack withdrawalCredentialContract = new ReentrantAttack(); address addr = address(