fee splitter #45
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. |
joshpwrk
left a comment
joshpwrk
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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.