From 30be8e42d28de242fc75dbf2e14d356f32e5eed0 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Thu, 1 Feb 2024 12:44:18 +0100 Subject: [PATCH] Make splits receivers lists calldata (#345) --- src/Drips.sol | 8 ++++---- src/Splits.sol | 22 +++++++++++----------- test/Splits.t.sol | 36 ++++++++++++++++++++++-------------- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/Drips.sol b/src/Drips.sol index 21dc719c..ab6f861b 100644 --- a/src/Drips.sol +++ b/src/Drips.sol @@ -471,7 +471,7 @@ contract Drips is Managed, Streams, Splits { /// @return collectableAmt The amount made collectable for the account /// on top of what was collectable before. /// @return splitAmt The amount split to the account's splits receivers - function splitResult(uint256 accountId, SplitsReceiver[] memory currReceivers, uint128 amount) + function splitResult(uint256 accountId, SplitsReceiver[] calldata currReceivers, uint128 amount) public view onlyProxy @@ -503,7 +503,7 @@ contract Drips is Managed, Streams, Splits { /// @return collectableAmt The amount made collectable for the account /// on top of what was collectable before. /// @return splitAmt The amount split to the account's splits receivers - function split(uint256 accountId, IERC20 erc20, SplitsReceiver[] memory currReceivers) + function split(uint256 accountId, IERC20 erc20, SplitsReceiver[] calldata currReceivers) public onlyProxy returns (uint128 collectableAmt, uint128 splitAmt) @@ -747,7 +747,7 @@ contract Drips is Managed, Streams, Splits { /// This is usually unwanted, because if splitting is repeated, /// funds split to themselves will be again split using the current configuration. /// Splitting 100% to self effectively blocks splitting unless the configuration is updated. - function setSplits(uint256 accountId, SplitsReceiver[] memory receivers) + function setSplits(uint256 accountId, SplitsReceiver[] calldata receivers) public onlyProxy onlyDriver(accountId) @@ -766,7 +766,7 @@ contract Drips is Managed, Streams, Splits { /// @param receivers The list of the splits receivers. /// Must be sorted by the account IDs, without duplicate account IDs and without 0 weights. /// @return receiversHash The hash of the list of splits receivers. - function hashSplits(SplitsReceiver[] memory receivers) + function hashSplits(SplitsReceiver[] calldata receivers) public pure returns (bytes32 receiversHash) diff --git a/src/Splits.sol b/src/Splits.sol index 4a88d785..c7809ae4 100644 --- a/src/Splits.sol +++ b/src/Splits.sol @@ -115,11 +115,11 @@ abstract contract Splits { /// @return collectableAmt The amount made collectable for the account /// on top of what was collectable before. /// @return splitAmt The amount split to the account's splits receivers - function _splitResult(uint256 accountId, SplitsReceiver[] memory currReceivers, uint128 amount) - internal - view - returns (uint128 collectableAmt, uint128 splitAmt) - { + function _splitResult( + uint256 accountId, + SplitsReceiver[] calldata currReceivers, + uint128 amount + ) internal view returns (uint128 collectableAmt, uint128 splitAmt) { _assertCurrSplits(accountId, currReceivers); if (amount == 0) { return (0, 0); @@ -147,7 +147,7 @@ abstract contract Splits { /// @return collectableAmt The amount made collectable for the account /// on top of what was collectable before. /// @return splitAmt The amount split to the account's splits receivers - function _split(uint256 accountId, IERC20 erc20, SplitsReceiver[] memory currReceivers) + function _split(uint256 accountId, IERC20 erc20, SplitsReceiver[] calldata currReceivers) internal returns (uint128 collectableAmt, uint128 splitAmt) { @@ -231,7 +231,7 @@ abstract contract Splits { /// This is usually unwanted, because if splitting is repeated, /// funds split to themselves will be again split using the current configuration. /// Splitting 100% to self effectively blocks splitting unless the configuration is updated. - function _setSplits(uint256 accountId, SplitsReceiver[] memory receivers) internal { + function _setSplits(uint256 accountId, SplitsReceiver[] calldata receivers) internal { SplitsState storage state = _splitsStorage().splitsStates[accountId]; bytes32 newSplitsHash = _hashSplits(receivers); if (newSplitsHash == state.splitsHash) return; @@ -244,13 +244,13 @@ abstract contract Splits { /// @notice Validates a list of splits receivers and emits events for them /// @param receivers The list of splits receivers /// Must be sorted by the account IDs, without duplicate account IDs and without 0 weights. - function _assertSplitsValid(SplitsReceiver[] memory receivers) private pure { + function _assertSplitsValid(SplitsReceiver[] calldata receivers) private pure { unchecked { require(receivers.length <= _MAX_SPLITS_RECEIVERS, "Too many splits receivers"); uint256 totalWeight = 0; uint256 prevAccountId = 0; for (uint256 i = 0; i < receivers.length; i++) { - SplitsReceiver memory receiver = receivers[i]; + SplitsReceiver calldata receiver = receivers[i]; uint256 weight = receiver.weight; require(weight != 0, "Splits receiver weight is zero"); if (weight > _TOTAL_SPLITS_WEIGHT) weight = _TOTAL_SPLITS_WEIGHT + 1; @@ -267,7 +267,7 @@ abstract contract Splits { /// @param accountId The account ID. /// @param currReceivers The list of the account's current splits receivers. /// If the splits have never been set, pass an empty array. - function _assertCurrSplits(uint256 accountId, SplitsReceiver[] memory currReceivers) + function _assertCurrSplits(uint256 accountId, SplitsReceiver[] calldata currReceivers) internal view { @@ -287,7 +287,7 @@ abstract contract Splits { /// @param receivers The list of the splits receivers. /// If the splits have never been set, pass an empty array. /// @return receiversHash The hash of the list of splits receivers. - function _hashSplits(SplitsReceiver[] memory receivers) + function _hashSplits(SplitsReceiver[] calldata receivers) internal pure returns (bytes32 receiversHash) diff --git a/test/Splits.t.sol b/test/Splits.t.sol index 6f6794f6..3a03ca74 100644 --- a/test/Splits.t.sol +++ b/test/Splits.t.sol @@ -55,7 +55,7 @@ contract SplitsTest is Test, Splits { assertSplits(usedAccountId, currSplits); } - function setSplitsExternal(uint256 usedAccountId, SplitsReceiver[] memory newReceivers) + function setSplitsExternal(uint256 usedAccountId, SplitsReceiver[] calldata newReceivers) external { Splits._setSplits(usedAccountId, newReceivers); @@ -74,6 +74,13 @@ contract SplitsTest is Test, Splits { internal view { + this.assertSplitsExternal(usedAccountId, expectedReceivers); + } + + function assertSplitsExternal( + uint256 usedAccountId, + SplitsReceiver[] calldata expectedReceivers + ) external view { Splits._assertCurrSplits(usedAccountId, expectedReceivers); } @@ -84,7 +91,7 @@ contract SplitsTest is Test, Splits { function setSplits(uint256 usedAccountId, SplitsReceiver[] memory newReceivers) internal { assertSplits(usedAccountId, currSplitsReceivers[usedAccountId]); - Splits._setSplits(usedAccountId, newReceivers); + this.setSplitsExternal(usedAccountId, newReceivers); assertSplits(usedAccountId, newReceivers); delete currSplitsReceivers[usedAccountId]; for (uint256 i = 0; i < newReceivers.length; i++) { @@ -95,17 +102,17 @@ contract SplitsTest is Test, Splits { function splitExternal( uint256 usedAccountId, IERC20 usedErc20, - SplitsReceiver[] memory currReceivers - ) external { - Splits._split(usedAccountId, usedErc20, currReceivers); + SplitsReceiver[] calldata currReceivers + ) external returns (uint128 collectableAmt, uint128 splitAmt) { + return Splits._split(usedAccountId, usedErc20, currReceivers); } function splitResultExternal( uint256 usedAccountId, - SplitsReceiver[] memory currReceivers, + SplitsReceiver[] calldata currReceivers, uint128 amount - ) external view { - Splits._splitResult(usedAccountId, currReceivers, amount); + ) external view returns (uint128 collectableAmt, uint128 splitAmt) { + return Splits._splitResult(usedAccountId, currReceivers, amount); } function split(uint256 usedAccountId, uint128 expectedCollectable, uint128 expectedSplit) @@ -116,11 +123,12 @@ contract SplitsTest is Test, Splits { SplitsReceiver[] memory receivers = getCurrSplitsReceivers(usedAccountId); uint128 amt = Splits._splittable(usedAccountId, erc20); (uint128 collectableRes, uint128 splitRes) = - Splits._splitResult(usedAccountId, receivers, amt); + this.splitResultExternal(usedAccountId, receivers, amt); assertEq(collectableRes, expectedCollectable, "Invalid result collectable amount"); assertEq(splitRes, expectedSplit, "Invalid result split amount"); - (uint128 collectableAmt, uint128 splitAmt) = Splits._split(usedAccountId, erc20, receivers); + (uint128 collectableAmt, uint128 splitAmt) = + this.splitExternal(usedAccountId, erc20, receivers); assertEq(collectableAmt, expectedCollectable, "Invalid collectable amount"); assertEq(splitAmt, expectedSplit, "Invalid split amount"); @@ -269,7 +277,7 @@ contract SplitsTest is Test, Splits { addSplittable(receiver1, 20); (uint128 collectableAmt, uint128 splitAmt) = - Splits._split(receiver1, erc20, getCurrSplitsReceivers(receiver1)); + this.splitExternal(receiver1, erc20, getCurrSplitsReceivers(receiver1)); assertEq(collectableAmt, 6, "Invalid collectable amount"); assertEq(splitAmt, 14, "Invalid split amount"); @@ -280,7 +288,7 @@ contract SplitsTest is Test, Splits { // Splitting 10 which has been split to receiver1 themselves in the previous step (collectableAmt, splitAmt) = - Splits._split(receiver1, erc20, getCurrSplitsReceivers(receiver1)); + this.splitExternal(receiver1, erc20, getCurrSplitsReceivers(receiver1)); assertEq(collectableAmt, 3, "Invalid collectable amount"); assertEq(splitAmt, 7, "Invalid split amount"); @@ -387,9 +395,9 @@ contract SplitsTest is Test, Splits { SplitsReceiver[] memory receivers = sanitizeReceivers(receiversRaw, receiversLengthRaw, totalWeightRaw); Splits._addSplittable(usedAccountId, usedErc20, amt); - Splits._setSplits(usedAccountId, receivers); + this.setSplitsExternal(usedAccountId, receivers); (uint128 collectableAmt, uint128 splitAmt) = - Splits._split(usedAccountId, usedErc20, receivers); + this.splitExternal(usedAccountId, usedErc20, receivers); assertEq(collectableAmt + splitAmt, amt, "Invalid split results"); uint128 collectedAmt = Splits._collect(usedAccountId, usedErc20); assertEq(collectedAmt, collectableAmt, "Invalid collected amount");