-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fee splitter #45
base: master
Are you sure you want to change the base?
fee splitter #45
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #45 +/- ##
==========================================
+ Coverage 92.15% 92.74% +0.59%
==========================================
Files 13 14 +1
Lines 306 331 +25
Branches 68 71 +3
==========================================
+ Hits 282 307 +25
Misses 7 7
Partials 17 17 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main one is ensuring accountA / B are set before splitting
src/periphery/FeeSplitter.sol
Outdated
IAsset public immutable cashAsset; | ||
|
||
uint public subAcc; | ||
uint public splitPercent = 0.5e18; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why not just force it in the constructor. Seems like an important value to not forget to set.
|
||
/// @notice Set the % split | ||
function setSplit(uint _splitPercent) external onlyOwner { | ||
if (_splitPercent > 1e18) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cash and managers allow amount=0 transfers, but might be worth double checking.
E.g. if _splitPercent
is 0 or 1, you'd need to make 0 amount transfers in split()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed test for this 👌
int splitAmountA = balance.multiplyDecimal(int(splitPercent)); | ||
int splitAmountB = balance - splitAmountA; | ||
|
||
ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a check that accountA
and accountB
are NOT 0. Otherwise I think you might accidentally send fees to the subaccountId 0 lol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or would run setSubaccounts
in constructor.
Create a utility contract for allowing fees to be collected into a smart contract and then split based on a set percentage to two different accounts.