Replies: 3 comments 5 replies
-
First and foremost, we’re not entirely sure we plan to keep the "Multiple" functions, as we aim to move entirely to using Secondly, regardless of this decision, integrators will still need to perform some additional checks. These checks can easily be achieved using bool[] memory hasWithdrawn = new bool[](streamIds.length);
hasWithdrawn = lockup.withdrawMultiple(streamIds, amounts);
for (uint256 i = 0; i < streamIds.length; ++i) {
if (!hasWithdrawn[i]) {
// do something
}
} vs lockup.withdrawMultiple(streamIds, amounts);
for (uint256 i = 0; i < streamIds.length; ++i) {
// the amount withdrawn needs to be at least equal to the amount passed in the function
// so this check does the same thing
if (amounts[i] > lockup.getWithdrawnAmount(streamIds[i])) {
// do something
}
} So having said that, we decided that is not worth implementing it. |
Beta Was this translation helpful? Give feedback.
-
I agree with Andrei's comment. I also don't think there is any use case for integrators to use this function in Solidity. There are several ways to call withdraw on multiple stream IDs, such as with Also, I tested the gas difference on Here is another test in Remix:contract Ballot1 {
// 5224 gas with [1,2,3,4,5,6,7,8,9,10]
function abc(uint256[] calldata streamIds) external pure returns (bool[] memory result) {
result = new bool[](streamIds.length);
for (uint i; i < streamIds.length; ++i) {
result[i] = true;
}
}
}
contract Ballot2 {
// 1077 gas with [1,2,3,4,5,6,7,8,9,10]
function abc(uint256[] calldata streamIds) external pure {
for (uint i; i < streamIds.length; ++i) {
}
}
} Regarding |
Beta Was this translation helpful? Give feedback.
-
Alright. |
Beta Was this translation helpful? Give feedback.
-
I've just read finding 1 from Cantina now:
withdrawMultiple
not indicating failed withdrawals can be problematic for calling contractsI agree with their recommendation.
The gas cost for an additional in-memory array of boolean should be small.
Why have you decided to acknowledge it, @sablier-labs/solidity?
Beta Was this translation helpful? Give feedback.
All reactions