Feat/ fee share #96
Feat/ fee share #96
Conversation
|
|
||
| import "forge-std/Test.sol"; | ||
| import {ObolLidoSplitFactory, ObolLidoSplit, IwstETH} from "src/lido/ObolLidoSplitFactory.sol"; | ||
| import {ERC20} from "solmate/tokens/ERC20.sol"; |
There was a problem hiding this comment.
If we depend on solmate, we probably have to make these files GPL rather than MIT, no? https://github.com/transmissions11/solmate/blob/main/LICENSE I don't believe MIT counts for copyleft
There was a problem hiding this comment.
ah ok, i changed it to gpl
| vm.assume(amountToDistribute > 1 ether); | ||
| vm.assume(amountToDistribute < 10 ether); |
There was a problem hiding this comment.
Any reasoning behind these constraints?
There was a problem hiding this comment.
So we don't exceed the balance of the account being impersonated.
| /// @param _wstETH wstETH address | ||
| constructor(address _feeRecipient, uint256 _feeShare, ERC20 _stETH, ERC20 _wstETH) { | ||
| if (_feeShare >= PERCENTAGE_SCALE) revert Invalid_FeeShare(_feeShare); | ||
| if (_feeShare > 0 && _feeRecipient == address(0)) revert Invalid_FeeRecipient(); |
There was a problem hiding this comment.
Great catch! This would brick the contract because of a revert in the transfer.
| /// @notice fee address | ||
| address public immutable feeRecipient; | ||
|
|
||
| /// @notice fee share | ||
| uint256 public immutable feeShare; |
There was a problem hiding this comment.
I assume this is an intentional decision, but of course these being immutably set on the implementation means they can't ever be updated. If you wanted the ability to change while still setting immutably on the implementation, you'd need to immutably set an address for another contract that holds and returns the fee info, and allows updates.
There was a problem hiding this comment.
yes, we don't want the fee to change. Changing the fee would require a sign-off from Lido and a lot of discussions and most likely require new deployments.
Add the ability to deduct fee share