From 8302ef7f22282d7d9df6e915adc94049ee897cb3 Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Fri, 16 Feb 2024 10:12:32 +1100 Subject: [PATCH 1/8] initial commit for fee splitter --- src/periphery/FeeSplitter.sol | 28 ++++++++++++++++++++++++++++ test/periphery/FeeSplitter.t.sol | 19 +++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 src/periphery/FeeSplitter.sol create mode 100644 test/periphery/FeeSplitter.t.sol diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol new file mode 100644 index 0000000..9916391 --- /dev/null +++ b/src/periphery/FeeSplitter.sol @@ -0,0 +1,28 @@ +pragma solidity ^0.8.18; + +import {Ownable2Step} from "openzeppelin/access/Ownable2Step.sol"; +import "v2-core/src/interfaces/ISubAccounts.sol"; + +contract FeeSplitter is Ownable2Step { + ISubAccounts public immutable subAccounts; + uint public immutable subAcc; + + constructor(ISubAccounts _subAccounts, IManager manager) { + subAccounts = _subAccounts; + subAcc = _subAccounts.createAccount(address(this), manager); + } + + /////////// + // Admin // + /////////// + + function setSplit(uint splitPercent) external onlyOwner {} + + function setSubAccounts(uint accountA, uint accountB) external onlyOwner {} + + ////////////// + // External // + ////////////// + /// @notice Work out the balance of the subaccount held by this contract, and split it based on the % split + function split() external {} +} \ No newline at end of file diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol new file mode 100644 index 0000000..3c7b1a7 --- /dev/null +++ b/test/periphery/FeeSplitter.t.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.8.18; + +import "../../src/periphery/FeeSplitter.sol"; +import "v2-core/test/integration-tests/shared/IntegrationTestBase.t.sol"; + + +contract FeeSplitterTest is IntegrationTestBase { + FeeSplitter public feeSplitter; + + function setUp() public { + _setupIntegrationTestComplete(); + + feeSplitter = new FeeSplitter(subAccounts, srm); + } + + function test_split() public { + feeSplitter.split(); + } +} From 04c7c01eee5ccce319ab318717900efa82400ed6 Mon Sep 17 00:00:00 2001 From: Dillon Lin <30581721+DillonLin@users.noreply.github.com> Date: Wed, 21 Feb 2024 12:00:56 +1100 Subject: [PATCH 2/8] Testing (#46) --- src/periphery/FeeSplitter.sol | 51 +++++++++++++++++++++++++++++--- test/periphery/FeeSplitter.t.sol | 31 +++++++++++++++---- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol index 9916391..7519e86 100644 --- a/src/periphery/FeeSplitter.sol +++ b/src/periphery/FeeSplitter.sol @@ -1,11 +1,15 @@ pragma solidity ^0.8.18; import {Ownable2Step} from "openzeppelin/access/Ownable2Step.sol"; -import "v2-core/src/interfaces/ISubAccounts.sol"; +import {IManager} from "v2-core/src/interfaces/IManager.sol"; +import {ISubAccounts} from "v2-core/src/interfaces/ISubAccounts.sol"; contract FeeSplitter is Ownable2Step { ISubAccounts public immutable subAccounts; uint public immutable subAcc; + uint private splitPercent; + uint private accountA; + uint private accountB; constructor(ISubAccounts _subAccounts, IManager manager) { subAccounts = _subAccounts; @@ -16,13 +20,52 @@ contract FeeSplitter is Ownable2Step { // Admin // /////////// - function setSplit(uint splitPercent) external onlyOwner {} + function setSplit(uint splitPercent) external onlyOwner { + require(_splitPercent <= 100, "Invalid percentage"); + splitPercent = _splitPercent; + } - function setSubAccounts(uint accountA, uint accountB) external onlyOwner {} + function setSubAccounts(uint _accountA, uint _accountB) external onlyOwner { + accountA = _accountA; + accountB = _accountB; + } ////////////// // External // ////////////// /// @notice Work out the balance of the subaccount held by this contract, and split it based on the % split - function split() external {} + function split() external { + ISubAccounts.AssetBalance[] memory balances = subAccounts.getAccountBalances(subAcc); + + for (uint i = 0; i < balances.length; i++) { + IAsset asset = balances[i].asset; + uint subId = balances[i].subId; + int balance = balances[i].balance; + + if (balance > 0) { + uint splitAmountA = uint(balance) * splitPercent / 100; + uint splitAmountB = uint(balance) - splitAmountA; + + ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); + transfers[0] = ISubAccounts.AssetTransfer({ + fromAcc: subAcc, + toAcc: accountA, + asset: asset, + subId: subId, + amount: int(splitAmountA), + assetData: bytes32(0) + }); + transfers[1] = ISubAccounts.AssetTransfer({ + fromAcc: subAcc, + toAcc: accountB, + asset: asset, + subId: subId, + amount: int(splitAmountB), + assetData: bytes32(0) + }); + + subAccounts.submitTransfers(transfers, ""); + } + } + } } \ No newline at end of file diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol index 3c7b1a7..9c83af6 100644 --- a/test/periphery/FeeSplitter.t.sol +++ b/test/periphery/FeeSplitter.t.sol @@ -1,19 +1,40 @@ pragma solidity ^0.8.18; -import "../../src/periphery/FeeSplitter.sol"; -import "v2-core/test/integration-tests/shared/IntegrationTestBase.t.sol"; +import {IntegrationTestBase} from "v2-core/test/integration-tests/shared/IntegrationTestBase.t.sol"; +import {IManager} from "v2-core/src/interfaces/IManager.sol"; +import {IAsset} from "v2-core/src/interfaces/IAsset.sol"; +import {ISubAccounts} from "v2-core/src/interfaces/ISubAccounts.sol"; +import "forge-std/console2.sol"; +import "../../src/periphery/FeeSplitter.sol"; contract FeeSplitterTest is IntegrationTestBase { FeeSplitter public feeSplitter; + IAsset public asset; + uint public accountA; + uint public accountB; - function setUp() public { - _setupIntegrationTestComplete(); - + function setUp() public override { + super.setUp(); feeSplitter = new FeeSplitter(subAccounts, srm); + feeSplitter.setSubAccounts(accountA, accountB); + feeSplitter.setSplit(50); + uint amount = 100e18; + cashAsset.mint(address(this), amount); + cashAsset.approve(address(subAccounts), amount); + cashAsset.deposit(feeSplitter.subAcc(), amount); } function test_split() public { + // before split + assertEq(subAccounts.getBalance(feeSplitter.subAcc(), cashAsset, 0), int(100e18), "Incorrect initial balance in FeeSplitter"); + assertEq(subAccounts.getBalance(accountA, cashAsset, 0), 0, "Account A should initially have 0 balance"); + assertEq(subAccounts.getBalance(accountB, cashAsset, 0), 0, "Account B should initially have 0 balance"); + feeSplitter.split(); + + // after split + assertEq(subAccounts.getBalance(accountA, cashAsset, 0), int(50e18), "Account A should have half of the funds after split"); + assertEq(subAccounts.getBalance(accountB, cashAsset, 0), int(50e18), "Account B should have half of the funds after split"); } } From 39a128d64a4134d3c3d2805498faec7658fbe59f Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Wed, 21 Feb 2024 13:54:03 +1100 Subject: [PATCH 3/8] fixes --- src/periphery/FeeSplitter.sol | 94 ++++++++++++++++++++------------ test/periphery/FeeSplitter.t.sol | 30 +++++----- 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol index 7519e86..5356098 100644 --- a/src/periphery/FeeSplitter.sol +++ b/src/periphery/FeeSplitter.sol @@ -1,27 +1,36 @@ pragma solidity ^0.8.18; +import "lyra-utils/decimals/SignedDecimalMath.sol"; + import {Ownable2Step} from "openzeppelin/access/Ownable2Step.sol"; import {IManager} from "v2-core/src/interfaces/IManager.sol"; -import {ISubAccounts} from "v2-core/src/interfaces/ISubAccounts.sol"; +import {ISubAccounts, IAsset} from "v2-core/src/interfaces/ISubAccounts.sol"; contract FeeSplitter is Ownable2Step { + using SignedDecimalMath for int; + ISubAccounts public immutable subAccounts; + IAsset public immutable cashAsset; uint public immutable subAcc; - uint private splitPercent; - uint private accountA; - uint private accountB; - constructor(ISubAccounts _subAccounts, IManager manager) { + uint public splitPercent; + uint public accountA; + uint public accountB; + + constructor(ISubAccounts _subAccounts, IManager manager, IAsset _cashAsset) { subAccounts = _subAccounts; subAcc = _subAccounts.createAccount(address(this), manager); + cashAsset = _cashAsset; } /////////// // Admin // /////////// - function setSplit(uint splitPercent) external onlyOwner { - require(_splitPercent <= 100, "Invalid percentage"); + function setSplit(uint _splitPercent) external onlyOwner { + if (_splitPercent > 1e18) { + revert FS_InvalidSplitPercentage(); + } splitPercent = _splitPercent; } @@ -30,6 +39,10 @@ contract FeeSplitter is Ownable2Step { accountB = _accountB; } + function recoverSubAccount(address recipient) external onlyOwner { + subAccounts.transferFrom(address(this), recipient, subAcc); + } + ////////////// // External // ////////////// @@ -38,34 +51,45 @@ contract FeeSplitter is Ownable2Step { ISubAccounts.AssetBalance[] memory balances = subAccounts.getAccountBalances(subAcc); for (uint i = 0; i < balances.length; i++) { - IAsset asset = balances[i].asset; - uint subId = balances[i].subId; - int balance = balances[i].balance; - - if (balance > 0) { - uint splitAmountA = uint(balance) * splitPercent / 100; - uint splitAmountB = uint(balance) - splitAmountA; - - ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); - transfers[0] = ISubAccounts.AssetTransfer({ - fromAcc: subAcc, - toAcc: accountA, - asset: asset, - subId: subId, - amount: int(splitAmountA), - assetData: bytes32(0) - }); - transfers[1] = ISubAccounts.AssetTransfer({ - fromAcc: subAcc, - toAcc: accountB, - asset: asset, - subId: subId, - amount: int(splitAmountB), - assetData: bytes32(0) - }); - - subAccounts.submitTransfers(transfers, ""); - } + IAsset asset = balances[i].asset; + + if (asset != cashAsset) { + continue; + } + + int balance = balances[i].balance; + + if (balance <= 0) { + return; + } + + int splitAmountA = balance.multiplyDecimal(int(splitPercent)); + int splitAmountB = balance - splitAmountA; + + ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); + transfers[0] = ISubAccounts.AssetTransfer({ + fromAcc: subAcc, + toAcc: accountA, + asset: cashAsset, + subId: 0, + amount: int(splitAmountA), + assetData: bytes32(0) + }); + transfers[1] = ISubAccounts.AssetTransfer({ + fromAcc: subAcc, + toAcc: accountB, + asset: cashAsset, + subId: 0, + amount: int(splitAmountB), + assetData: bytes32(0) + }); + + subAccounts.submitTransfers(transfers, ""); } } + + //////////// + // Errors // + //////////// + error FS_InvalidSplitPercentage(); } \ No newline at end of file diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol index 9c83af6..08a3552 100644 --- a/test/periphery/FeeSplitter.t.sol +++ b/test/periphery/FeeSplitter.t.sol @@ -14,27 +14,31 @@ contract FeeSplitterTest is IntegrationTestBase { uint public accountA; uint public accountB; - function setUp() public override { - super.setUp(); - feeSplitter = new FeeSplitter(subAccounts, srm); + function setUp() public { + _setupIntegrationTestComplete(); + + accountA = subAccounts.createAccount(address(this), srm); + accountB = subAccounts.createAccount(address(this), markets["weth"].pmrm); + + feeSplitter = new FeeSplitter(subAccounts, srm, cash); feeSplitter.setSubAccounts(accountA, accountB); - feeSplitter.setSplit(50); - uint amount = 100e18; - cashAsset.mint(address(this), amount); - cashAsset.approve(address(subAccounts), amount); - cashAsset.deposit(feeSplitter.subAcc(), amount); + feeSplitter.setSplit(0.5e18); + uint amount = 100e6; + usdc.mint(address(this), amount); + usdc.approve(address(cash), amount); + cash.deposit(feeSplitter.subAcc(), amount); } function test_split() public { // before split - assertEq(subAccounts.getBalance(feeSplitter.subAcc(), cashAsset, 0), int(100e18), "Incorrect initial balance in FeeSplitter"); - assertEq(subAccounts.getBalance(accountA, cashAsset, 0), 0, "Account A should initially have 0 balance"); - assertEq(subAccounts.getBalance(accountB, cashAsset, 0), 0, "Account B should initially have 0 balance"); + assertEq(subAccounts.getBalance(feeSplitter.subAcc(), cash, 0), int(100e18), "Incorrect initial balance in FeeSplitter"); + assertEq(subAccounts.getBalance(accountA, cash, 0), 0, "Account A should initially have 0 balance"); + assertEq(subAccounts.getBalance(accountB, cash, 0), 0, "Account B should initially have 0 balance"); feeSplitter.split(); // after split - assertEq(subAccounts.getBalance(accountA, cashAsset, 0), int(50e18), "Account A should have half of the funds after split"); - assertEq(subAccounts.getBalance(accountB, cashAsset, 0), int(50e18), "Account B should have half of the funds after split"); + assertEq(subAccounts.getBalance(accountA, cash, 0), int(50e18), "Account A should have half of the funds after split"); + assertEq(subAccounts.getBalance(accountB, cash, 0), int(50e18), "Account B should have half of the funds after split"); } } From 3b00832f32ecb4673c650150fef33b244bc1faf9 Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Wed, 21 Feb 2024 14:47:25 +1100 Subject: [PATCH 4/8] fee splitter tests --- src/periphery/FeeSplitter.sol | 26 +++++++++++++------------- test/periphery/FeeSplitter.t.sol | 29 ++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol index 5356098..fffb8c3 100644 --- a/src/periphery/FeeSplitter.sol +++ b/src/periphery/FeeSplitter.sol @@ -68,20 +68,20 @@ contract FeeSplitter is Ownable2Step { ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); transfers[0] = ISubAccounts.AssetTransfer({ - fromAcc: subAcc, - toAcc: accountA, - asset: cashAsset, - subId: 0, - amount: int(splitAmountA), - assetData: bytes32(0) + fromAcc: subAcc, + toAcc: accountA, + asset: cashAsset, + subId: 0, + amount: int(splitAmountA), + assetData: bytes32(0) }); transfers[1] = ISubAccounts.AssetTransfer({ - fromAcc: subAcc, - toAcc: accountB, - asset: cashAsset, - subId: 0, - amount: int(splitAmountB), - assetData: bytes32(0) + fromAcc: subAcc, + toAcc: accountB, + asset: cashAsset, + subId: 0, + amount: int(splitAmountB), + assetData: bytes32(0) }); subAccounts.submitTransfers(transfers, ""); @@ -92,4 +92,4 @@ contract FeeSplitter is Ownable2Step { // Errors // //////////// error FS_InvalidSplitPercentage(); -} \ No newline at end of file +} diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol index 08a3552..01cf678 100644 --- a/test/periphery/FeeSplitter.t.sol +++ b/test/periphery/FeeSplitter.t.sol @@ -29,16 +29,39 @@ contract FeeSplitterTest is IntegrationTestBase { cash.deposit(feeSplitter.subAcc(), amount); } + function test_constructor() public { + assertEq(address(feeSplitter.subAccounts()), address(subAccounts), "Incorrect subAccounts"); + assertEq(address(feeSplitter.cashAsset()), address(cash), "Incorrect cashAsset"); + assertFalse(feeSplitter.subAcc() == 0, "Incorrect subAcc"); + assertEq(feeSplitter.splitPercent(), 0.5e18, "Incorrect splitPercent"); + assertEq(feeSplitter.accountA(), accountA, "Incorrect accountA"); + assertEq(feeSplitter.accountB(), accountB, "Incorrect accountB"); + } + + function test_setSplit() public { + feeSplitter.setSplit(0.6e18); + assertEq(feeSplitter.splitPercent(), 0.6e18, "Incorrect splitPercent"); + + vm.expectRevert(FeeSplitter.FS_InvalidSplitPercentage.selector); + feeSplitter.setSplit(1.1e18); + } + function test_split() public { // before split - assertEq(subAccounts.getBalance(feeSplitter.subAcc(), cash, 0), int(100e18), "Incorrect initial balance in FeeSplitter"); + assertEq( + subAccounts.getBalance(feeSplitter.subAcc(), cash, 0), int(100e18), "Incorrect initial balance in FeeSplitter" + ); assertEq(subAccounts.getBalance(accountA, cash, 0), 0, "Account A should initially have 0 balance"); assertEq(subAccounts.getBalance(accountB, cash, 0), 0, "Account B should initially have 0 balance"); feeSplitter.split(); // after split - assertEq(subAccounts.getBalance(accountA, cash, 0), int(50e18), "Account A should have half of the funds after split"); - assertEq(subAccounts.getBalance(accountB, cash, 0), int(50e18), "Account B should have half of the funds after split"); + assertEq( + subAccounts.getBalance(accountA, cash, 0), int(50e18), "Account A should have half of the funds after split" + ); + assertEq( + subAccounts.getBalance(accountB, cash, 0), int(50e18), "Account B should have half of the funds after split" + ); } } From 5baab5cd162dfa20031029254a8b3e4793c9f407 Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Wed, 21 Feb 2024 15:17:53 +1100 Subject: [PATCH 5/8] coverage --- src/periphery/FeeSplitter.sol | 89 ++++++++++++++++++-------------- test/periphery/FeeSplitter.t.sol | 14 +++++ 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol index fffb8c3..4aa7a8d 100644 --- a/src/periphery/FeeSplitter.sol +++ b/src/periphery/FeeSplitter.sol @@ -11,9 +11,9 @@ contract FeeSplitter is Ownable2Step { ISubAccounts public immutable subAccounts; IAsset public immutable cashAsset; - uint public immutable subAcc; - uint public splitPercent; + uint public subAcc; + uint public splitPercent = 0.5e18; uint public accountA; uint public accountB; @@ -32,15 +32,23 @@ contract FeeSplitter is Ownable2Step { revert FS_InvalidSplitPercentage(); } splitPercent = _splitPercent; + + emit SplitPercentSet(_splitPercent); } function setSubAccounts(uint _accountA, uint _accountB) external onlyOwner { accountA = _accountA; accountB = _accountB; + + emit SubAccountsSet(_accountA, _accountB); } function recoverSubAccount(address recipient) external onlyOwner { - subAccounts.transferFrom(address(this), recipient, subAcc); + uint oldSubAcc = subAcc; + subAccounts.transferFrom(address(this), recipient, oldSubAcc); + subAcc = subAccounts.createAccount(address(this), subAccounts.manager(oldSubAcc)); + + emit SubAccountRecovered(oldSubAcc, recipient, subAcc); } ////////////// @@ -48,48 +56,49 @@ contract FeeSplitter is Ownable2Step { ////////////// /// @notice Work out the balance of the subaccount held by this contract, and split it based on the % split function split() external { - ISubAccounts.AssetBalance[] memory balances = subAccounts.getAccountBalances(subAcc); - - for (uint i = 0; i < balances.length; i++) { - IAsset asset = balances[i].asset; - - if (asset != cashAsset) { - continue; - } - - int balance = balances[i].balance; - - if (balance <= 0) { - return; - } - - int splitAmountA = balance.multiplyDecimal(int(splitPercent)); - int splitAmountB = balance - splitAmountA; - - ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); - transfers[0] = ISubAccounts.AssetTransfer({ - fromAcc: subAcc, - toAcc: accountA, - asset: cashAsset, - subId: 0, - amount: int(splitAmountA), - assetData: bytes32(0) - }); - transfers[1] = ISubAccounts.AssetTransfer({ - fromAcc: subAcc, - toAcc: accountB, - asset: cashAsset, - subId: 0, - amount: int(splitAmountB), - assetData: bytes32(0) - }); - - subAccounts.submitTransfers(transfers, ""); + int balance = subAccounts.getBalance(subAcc, cashAsset, 0); + + if (balance <= 0) { + revert FS_NoBalanceToSplit(); } + + int splitAmountA = balance.multiplyDecimal(int(splitPercent)); + int splitAmountB = balance - splitAmountA; + + ISubAccounts.AssetTransfer[] memory transfers = new ISubAccounts.AssetTransfer[](2); + transfers[0] = ISubAccounts.AssetTransfer({ + fromAcc: subAcc, + toAcc: accountA, + asset: cashAsset, + subId: 0, + amount: int(splitAmountA), + assetData: bytes32(0) + }); + transfers[1] = ISubAccounts.AssetTransfer({ + fromAcc: subAcc, + toAcc: accountB, + asset: cashAsset, + subId: 0, + amount: int(splitAmountB), + assetData: bytes32(0) + }); + + subAccounts.submitTransfers(transfers, ""); + + emit BalanceSplit(subAcc, accountA, accountB, splitAmountA, splitAmountB); } //////////// // Errors // //////////// error FS_InvalidSplitPercentage(); + error FS_NoBalanceToSplit(); + + //////////// + // Events // + //////////// + event SplitPercentSet(uint splitPercent); + event SubAccountsSet(uint accountA, uint accountB); + event SubAccountRecovered(uint oldSubAcc, address recipient, uint newSubAcc); + event BalanceSplit(uint subAcc, uint accountA, uint accountB, int splitAmountA, int splitAmountB); } diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol index 01cf678..bc8d966 100644 --- a/test/periphery/FeeSplitter.t.sol +++ b/test/periphery/FeeSplitter.t.sol @@ -63,5 +63,19 @@ contract FeeSplitterTest is IntegrationTestBase { assertEq( subAccounts.getBalance(accountB, cash, 0), int(50e18), "Account B should have half of the funds after split" ); + + vm.expectRevert(FeeSplitter.FS_NoBalanceToSplit.selector); + feeSplitter.split(); + } + + function test_recover() public { + uint oldSubAcc = feeSplitter.subAcc(); + feeSplitter.recoverSubAccount(address(this)); + uint newSubAcc = feeSplitter.subAcc(); + assertEq( + subAccounts.ownerOf(newSubAcc), address(feeSplitter), "New subAcc should be owned by fee splitter contract" + ); + assertEq(subAccounts.ownerOf(oldSubAcc), address(this), "Old subAcc should be owned by this contract"); + assertFalse(oldSubAcc == newSubAcc); } } From bf5b28d61ae3675a45364fedda4e10c269297f47 Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Wed, 21 Feb 2024 15:20:28 +1100 Subject: [PATCH 6/8] add natspec comments --- src/periphery/FeeSplitter.sol | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol index 4aa7a8d..3a09d55 100644 --- a/src/periphery/FeeSplitter.sol +++ b/src/periphery/FeeSplitter.sol @@ -6,6 +6,11 @@ import {Ownable2Step} from "openzeppelin/access/Ownable2Step.sol"; import {IManager} from "v2-core/src/interfaces/IManager.sol"; import {ISubAccounts, IAsset} from "v2-core/src/interfaces/ISubAccounts.sol"; +/** + * @title FeeSplitter + * @notice FeeSplitter is a contract that splits the balance of a subaccount held by this contract based on a % split + * @author Lyra + */ contract FeeSplitter is Ownable2Step { using SignedDecimalMath for int; @@ -27,6 +32,7 @@ contract FeeSplitter is Ownable2Step { // Admin // /////////// + /// @notice Set the % split function setSplit(uint _splitPercent) external onlyOwner { if (_splitPercent > 1e18) { revert FS_InvalidSplitPercentage(); @@ -36,6 +42,7 @@ contract FeeSplitter is Ownable2Step { emit SplitPercentSet(_splitPercent); } + /// @notice Set the subaccounts to split the balance between function setSubAccounts(uint _accountA, uint _accountB) external onlyOwner { accountA = _accountA; accountB = _accountB; @@ -43,6 +50,7 @@ contract FeeSplitter is Ownable2Step { emit SubAccountsSet(_accountA, _accountB); } + /// @notice Recover a subaccount held by this contract, creating a new one in its place function recoverSubAccount(address recipient) external onlyOwner { uint oldSubAcc = subAcc; subAccounts.transferFrom(address(this), recipient, oldSubAcc); From 5519e8965dc3176d58f490a609474f69be8342f9 Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Fri, 23 Feb 2024 12:50:09 +1100 Subject: [PATCH 7/8] add initialisers in constructor and more tests --- src/periphery/FeeSplitter.sol | 27 +++++++++++++++++++++-- test/periphery/FeeSplitter.t.sol | 38 +++++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/periphery/FeeSplitter.sol b/src/periphery/FeeSplitter.sol index 3a09d55..c47a455 100644 --- a/src/periphery/FeeSplitter.sol +++ b/src/periphery/FeeSplitter.sol @@ -18,14 +18,24 @@ contract FeeSplitter is Ownable2Step { IAsset public immutable cashAsset; uint public subAcc; - uint public splitPercent = 0.5e18; + uint public splitPercent; uint public accountA; uint public accountB; - constructor(ISubAccounts _subAccounts, IManager manager, IAsset _cashAsset) { + constructor( + ISubAccounts _subAccounts, + IManager manager, + IAsset _cashAsset, + uint _splitPercent, + uint _accountA, + uint _accountB + ) { subAccounts = _subAccounts; subAcc = _subAccounts.createAccount(address(this), manager); cashAsset = _cashAsset; + + _setSplit(_splitPercent); + _setSubAccounts(_accountA, _accountB); } /////////// @@ -34,6 +44,10 @@ contract FeeSplitter is Ownable2Step { /// @notice Set the % split function setSplit(uint _splitPercent) external onlyOwner { + _setSplit(_splitPercent); + } + + function _setSplit(uint _splitPercent) internal { if (_splitPercent > 1e18) { revert FS_InvalidSplitPercentage(); } @@ -44,6 +58,14 @@ contract FeeSplitter is Ownable2Step { /// @notice Set the subaccounts to split the balance between function setSubAccounts(uint _accountA, uint _accountB) external onlyOwner { + _setSubAccounts(_accountA, _accountB); + } + + function _setSubAccounts(uint _accountA, uint _accountB) internal { + if (_accountA == 0 || _accountB == 0) { + revert FS_InvalidSubAccount(); + } + accountA = _accountA; accountB = _accountB; @@ -100,6 +122,7 @@ contract FeeSplitter is Ownable2Step { // Errors // //////////// error FS_InvalidSplitPercentage(); + error FS_InvalidSubAccount(); error FS_NoBalanceToSplit(); //////////// diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol index bc8d966..c49eea4 100644 --- a/test/periphery/FeeSplitter.t.sol +++ b/test/periphery/FeeSplitter.t.sol @@ -20,9 +20,7 @@ contract FeeSplitterTest is IntegrationTestBase { accountA = subAccounts.createAccount(address(this), srm); accountB = subAccounts.createAccount(address(this), markets["weth"].pmrm); - feeSplitter = new FeeSplitter(subAccounts, srm, cash); - feeSplitter.setSubAccounts(accountA, accountB); - feeSplitter.setSplit(0.5e18); + feeSplitter = new FeeSplitter(subAccounts, srm, cash, 0.5e18, accountA, accountB); uint amount = 100e6; usdc.mint(address(this), amount); usdc.approve(address(cash), amount); @@ -78,4 +76,38 @@ contract FeeSplitterTest is IntegrationTestBase { assertEq(subAccounts.ownerOf(oldSubAcc), address(this), "Old subAcc should be owned by this contract"); assertFalse(oldSubAcc == newSubAcc); } + + function test_split100percent() public { + feeSplitter.setSplit(1e18); + feeSplitter.split(); + assertEq( + subAccounts.getBalance(accountA, cash, 0), int(100e18), "Account A should have all of the funds after split" + ); + assertEq( + subAccounts.getBalance(accountB, cash, 0), 0, "Account B should have 0 balance after split" + ); + } + + function test_split0percent() public { + feeSplitter.setSplit(0); + feeSplitter.split(); + assertEq( + subAccounts.getBalance(accountA, cash, 0), 0, "Account A should have 0 balance after split" + ); + assertEq( + subAccounts.getBalance(accountB, cash, 0), int(100e18), "Account B should have all of the funds after split" + ); + } + + function test_setSubAccounts() public { + feeSplitter.setSubAccounts(accountB, accountA); + assertEq(feeSplitter.accountA(), accountB, "Incorrect accountA"); + assertEq(feeSplitter.accountB(), accountA, "Incorrect accountB"); + + vm.expectRevert(FeeSplitter.FS_InvalidSubAccount.selector); + feeSplitter.setSubAccounts(0, accountA); + + vm.expectRevert(FeeSplitter.FS_InvalidSubAccount.selector); + feeSplitter.setSubAccounts(accountB, 0); + } } From 3b5c569915b25f7901ce5b3804ad9a4ace91940d Mon Sep 17 00:00:00 2001 From: Dominic Romanowski Date: Fri, 23 Feb 2024 12:52:25 +1100 Subject: [PATCH 8/8] chore:fmt --- test/periphery/FeeSplitter.t.sol | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/periphery/FeeSplitter.t.sol b/test/periphery/FeeSplitter.t.sol index c49eea4..560bd65 100644 --- a/test/periphery/FeeSplitter.t.sol +++ b/test/periphery/FeeSplitter.t.sol @@ -83,17 +83,13 @@ contract FeeSplitterTest is IntegrationTestBase { assertEq( subAccounts.getBalance(accountA, cash, 0), int(100e18), "Account A should have all of the funds after split" ); - assertEq( - subAccounts.getBalance(accountB, cash, 0), 0, "Account B should have 0 balance after split" - ); + assertEq(subAccounts.getBalance(accountB, cash, 0), 0, "Account B should have 0 balance after split"); } function test_split0percent() public { feeSplitter.setSplit(0); feeSplitter.split(); - assertEq( - subAccounts.getBalance(accountA, cash, 0), 0, "Account A should have 0 balance after split" - ); + assertEq(subAccounts.getBalance(accountA, cash, 0), 0, "Account A should have 0 balance after split"); assertEq( subAccounts.getBalance(accountB, cash, 0), int(100e18), "Account B should have all of the funds after split" );