Skip to content

Commit

Permalink
docs: polish comments
Browse files Browse the repository at this point in the history
test: refactor BatchMock
test: refactor branches in batch tests
test: rename initialize function
  • Loading branch information
PaulRBerg committed Dec 23, 2024
1 parent 1049ab8 commit d4a95b0
Show file tree
Hide file tree
Showing 15 changed files with 89 additions and 99 deletions.
10 changes: 5 additions & 5 deletions src/abstracts/Batch.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ abstract contract Batch is IBatch {
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc IBatch
/// @dev Since `msg.value` can be reused across the calls, be VERY CAREFUL when using it. Refer to
/// https://www.paradigm.xyz/2021/08/two-rights-might-make-a-wrong for more information.
/// @dev Since `msg.value` can be reused across calls, be VERY CAREFUL when using it. Refer to
/// https://paradigm.xyz/2021/08/two-rights-might-make-a-wrong for more information.
function batch(bytes[] calldata calls) external payable override returns (bytes[] memory results) {
uint256 count = calls.length;
results = new bytes[](count);

for (uint256 i = 0; i < count; ++i) {
(bool success, bytes memory result) = address(this).delegatecall(calls[i]);

// Check: If delegatecall fails, load and bubble up the revert reason.
// Check: If the delegate call failed, load and bubble up the revert data.
if (!success) {
assembly {
// Get the length of the result stored in the first 32 bytes.
let resultSize := mload(result)

// Forward the pointer by 32 bytes at the beginning of the result data and revert it.
// Forward the pointer by 32 bytes to skip the length argument, and revert with the result.
revert(add(32, result), resultSize)
}
}

// Push the result onto the results array.
// Push the result into the results array.
results[i] = result;
}
}
Expand Down
12 changes: 4 additions & 8 deletions src/interfaces/IBatch.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@ pragma solidity >=0.8.22;

/// @notice This contract implements logic to batch call any function.
interface IBatch {
/// @notice Allows batched call to self, `this` contract.
/// @dev `results` contains `0x` for calls that do not return anything.
///
/// Note:
/// - Since `msg.value` can be reused across the calls, be VERY CAREFUL when using it. Refer to
/// https://www.paradigm.xyz/2021/08/two-rights-might-make-a-wrong for more information.
///
/// @notice Allows batched calls to self, i.e., `this` contract.
/// @dev Since `msg.value` can be reused across calls, be VERY CAREFUL when using it. Refer to
/// https://paradigm.xyz/2021/08/two-rights-might-make-a-wrong for more information.
/// @param calls An array of inputs for each call.
/// @return results An array of results from each call.
/// @return results An array of results from each call. Empty when the calls do not return anything.
function batch(bytes[] calldata calls) external payable returns (bytes[] memory results);
}
6 changes: 3 additions & 3 deletions tests/integration/Integration.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ abstract contract Integration_Test is Base_Test {
// Set the default Lockup model as Dynamic, we will override the default stream IDs where necessary.
lockupModel = Lockup.Model.LOCKUP_DYNAMIC;

// Initialize default streams IDs.
initializeDefaultStreamIds();
// Initialize default streams.
initializeDefaultStreams();
}

/*//////////////////////////////////////////////////////////////////////////
INITIALIZE-FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

function initializeDefaultStreamIds() internal {
function initializeDefaultStreams() internal {
defaultStreamId = createDefaultStream();
notCancelableStreamId = createDefaultStreamNonCancelable();
notTransferableStreamId = createDefaultStreamNonTransferable();
Expand Down
77 changes: 38 additions & 39 deletions tests/integration/concrete/batch/batch.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22;

import { Solarray } from "solarray/src/Solarray.sol";
import { Errors } from "src/libraries/Errors.sol";

import { Integration_Test } from "../../Integration.t.sol";
Expand Down Expand Up @@ -32,34 +33,34 @@ contract Batch_Integration_Concrete_Test is Integration_Test {
vm.warp(defaults.WARP_26_PERCENT());

bytes[] memory calls = new bytes[](6);
// It will return True.
// It should return True.
calls[0] = abi.encodeCall(lockup.isCancelable, (defaultStreamId));
// It will return the withdrawn amount.
// It should return the withdrawn amount.
calls[1] = abi.encodeCall(lockup.withdrawMax, (notCancelableStreamId, users.recipient));
// It will not return anything.
// It should return nothing.
calls[2] = abi.encodeCall(lockup.cancel, (defaultStreamId));
// It will return the next stream ID.
// It should return the next stream ID.
calls[3] = abi.encodeCall(lockup.nextStreamId, ());
// It will return the stream ID created.
// It should return the stream ID created.
calls[4] = abi.encodeCall(
lockup.createWithTimestampsLL,
(defaults.createWithTimestamps(), defaults.unlockAmounts(), defaults.CLIFF_TIME())
);
// It will not return anything.
// It should return nothing.
calls[5] = abi.encodeCall(lockup.renounce, (notTransferableStreamId));

bytes[] memory results = lockup.batch(calls);
assertEq(results.length, 6);
assertEq(abi.decode(results[0], (bool)), true);
assertEq(abi.decode(results[1], (uint128)), defaults.WITHDRAW_AMOUNT());
assertEq(results[2], hex"");
assertEq(abi.decode(results[3], (uint256)), expectedNextStreamId);
assertEq(abi.decode(results[4], (uint256)), expectedNextStreamId);
assertEq(results[5], hex"");
assertEq(results.length, 6, "batch results length");
assertTrue(abi.decode(results[0], (bool)), "batch results[0]");
assertEq(abi.decode(results[1], (uint128)), defaults.WITHDRAW_AMOUNT(), "batch results[1]");
assertEq(results[2], hex"", "batch results[2]");
assertEq(abi.decode(results[3], (uint256)), expectedNextStreamId, "batch results[3]");
assertEq(abi.decode(results[4], (uint256)), expectedNextStreamId, "batch results[4]");
assertEq(results[5], hex"", "batch results[5]");
}

/// @dev The batch call includes:
/// - ETH value
/// - Payable functions
/// - All create stream functions that return a value
function test_BatchPayable_CreateStreams() external {
uint256 expectedNextStreamId = lockup.nextStreamId();
Expand All @@ -85,18 +86,18 @@ contract Batch_Integration_Concrete_Test is Integration_Test {

// It should return the stream IDs created.
bytes[] memory results = lockup.batch{ value: 1 wei }(calls);
assertEq(results.length, 6);
assertEq(abi.decode(results[0], (uint256)), expectedNextStreamId);
assertEq(abi.decode(results[1], (uint256)), expectedNextStreamId + 1);
assertEq(abi.decode(results[2], (uint256)), expectedNextStreamId + 2);
assertEq(abi.decode(results[3], (uint256)), expectedNextStreamId + 3);
assertEq(abi.decode(results[4], (uint256)), expectedNextStreamId + 4);
assertEq(abi.decode(results[5], (uint256)), expectedNextStreamId + 5);
assertEq(address(lockup).balance, initialEthBalance + 1 wei);
assertEq(results.length, 6, "batch results length");
assertEq(abi.decode(results[0], (uint256)), expectedNextStreamId, "batch results[0]");
assertEq(abi.decode(results[1], (uint256)), expectedNextStreamId + 1, "batch results[1]");
assertEq(abi.decode(results[2], (uint256)), expectedNextStreamId + 2, "batch results[2]");
assertEq(abi.decode(results[3], (uint256)), expectedNextStreamId + 3, "batch results[3]");
assertEq(abi.decode(results[4], (uint256)), expectedNextStreamId + 4, "batch results[4]");
assertEq(abi.decode(results[5], (uint256)), expectedNextStreamId + 5, "batch results[5]");
assertEq(address(lockup).balance, initialEthBalance + 1 wei, "batch contract balance");
}

/// @dev The batch call includes:
/// - ETH value
/// - Payable functions
/// - All recipient related functions with both returns and non-returns
function test_BatchPayable_RecipientFunctions() external {
uint256 initialEthBalance = address(lockup).balance;
Expand Down Expand Up @@ -127,40 +128,38 @@ contract Batch_Integration_Concrete_Test is Integration_Test {
}

/// @dev The batch call includes:
/// - ETH value
/// - Payable functions
/// - All sender related functions with both returns and non-returns
function test_BatchPayable_SenderFunctions() external {
uint256 initialEthBalance = address(lockup).balance;
// Warp to the end time so that `burn` can be added to the call list.
vm.warp(defaults.END_TIME());

bytes[] memory calls = new bytes[](5);
// It should not return anything.
// It should return nothing.
calls[0] = abi.encodeCall(lockup.withdraw, (defaultStreamId, users.recipient, 1));
// It should return the withdrawn amount.
calls[1] = abi.encodeCall(lockup.withdrawMax, (defaultStreamId, users.recipient));

uint256[] memory streamIds = new uint256[](2);
streamIds[0] = streamIds[1] = notCancelableStreamId;
uint128[] memory amounts = new uint128[](2);
amounts[0] = amounts[1] = 1;
// It should not return anything.
calls[2] = abi.encodeCall(lockup.withdrawMultiple, (streamIds, amounts));
uint256[] memory streamIds = Solarray.uint256s(notCancelableStreamId, notCancelableStreamId);
uint128[] memory amounts = Solarray.uint128s(1, 1);

// It should return nothing.
calls[2] = abi.encodeCall(lockup.withdrawMultiple, (streamIds, amounts));
// It should return the withdrawn amount.
calls[3] = abi.encodeCall(lockup.withdrawMaxAndTransfer, (notCancelableStreamId, users.recipient));
// It should not return anything.
// It should return nothing.
calls[4] = abi.encodeCall(lockup.burn, (defaultStreamId));

resetPrank({ msgSender: users.recipient });
bytes[] memory results = lockup.batch{ value: 1 wei }(calls);

assertEq(results.length, 5);
assertEq(results[0], hex"");
assertEq(abi.decode(results[1], (uint128)), defaults.DEPOSIT_AMOUNT() - 1);
assertEq(results[2], hex"");
assertEq(abi.decode(results[3], (uint128)), defaults.DEPOSIT_AMOUNT() - 2);
assertEq(results[4], hex"");
assertEq(address(lockup).balance, initialEthBalance + 1 wei);
assertEq(results.length, 5, "batch results length");
assertEq(results[0], hex"", "batch results[0]");
assertEq(abi.decode(results[1], (uint128)), defaults.DEPOSIT_AMOUNT() - 1, "batch results[1]");
assertEq(results[2], hex"", "batch results[2]");
assertEq(abi.decode(results[3], (uint128)), defaults.DEPOSIT_AMOUNT() - 2, "batch results[3]");
assertEq(results[4], hex"", "batch results[4]");
assertEq(address(lockup).balance, initialEthBalance + 1 wei, "batch contract balance");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ contract RenounceMultiple_Integration_Concrete_Test is Integration_Test {
lockup.renounceMultiple(nullStreamIds);
}

function test_RevertGiven_AtleastOneNullStream() external whenNoDelegateCall whenNonZeroArrayLength {
function test_RevertGiven_AtLeastOneNullStream() external whenNoDelegateCall whenNonZeroArrayLength {
expectRevert_Null({
callData: abi.encodeCall(lockup.renounceMultiple, Solarray.uint256s(streamIds[0], nullStreamId))
});
}

function test_RevertGiven_AtleastOneColdStream()
function test_RevertGiven_AtLeastOneColdStream()
external
whenNoDelegateCall
whenNonZeroArrayLength
Expand Down Expand Up @@ -68,7 +68,7 @@ contract RenounceMultiple_Integration_Concrete_Test is Integration_Test {
lockup.renounceMultiple(streamIds);
}

function test_RevertGiven_AtleastOneNonCancelableStream()
function test_RevertGiven_AtLeastOneNonCancelableStream()
external
whenNoDelegateCall
whenNonZeroArrayLength
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ RenounceMultiple_Integration_Concrete_Test
├── when zero array length
│ └── it should do nothing
└── when non zero array length
├── given atleast one null stream
├── given at least one null stream
│ └── it should revert
└── given no null streams
├── given atleast one cold stream
├── given at least one cold stream
│ └── it should revert
└── given no cold streams
├── when caller unauthorized for any
│ └── it should revert
└── when caller authorized for all streams
├── given atleast one non cancelable stream
├── given at least one non cancelable stream
│ └── it should revert
└── given all streams cancelable
├── it should emit {MetadataUpdate} and {RenounceLockupStream} events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ abstract contract Lockup_Dynamic_Integration_Concrete_Test is Integration_Test {
Integration_Test.setUp();

lockupModel = Lockup.Model.LOCKUP_DYNAMIC;
initializeDefaultStreamIds();
initializeDefaultStreams();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ abstract contract Lockup_Linear_Integration_Concrete_Test is Integration_Test {
Integration_Test.setUp();

lockupModel = Lockup.Model.LOCKUP_LINEAR;
initializeDefaultStreamIds();
initializeDefaultStreams();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ abstract contract Lockup_Tranched_Integration_Concrete_Test is Integration_Test
Integration_Test.setUp();

lockupModel = Lockup.Model.LOCKUP_TRANCHED;
initializeDefaultStreamIds();
initializeDefaultStreams();
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/fuzz/lockup-dynamic/LockupDynamic.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ abstract contract Lockup_Dynamic_Integration_Fuzz_Test is Integration_Test {
Integration_Test.setUp();

lockupModel = Lockup.Model.LOCKUP_DYNAMIC;
initializeDefaultStreamIds();
initializeDefaultStreams();
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/fuzz/lockup-linear/LockupLinear.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ abstract contract Lockup_Linear_Integration_Fuzz_Test is Integration_Test {
Integration_Test.setUp();

lockupModel = Lockup.Model.LOCKUP_LINEAR;
initializeDefaultStreamIds();
initializeDefaultStreams();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ abstract contract Lockup_Tranched_Integration_Fuzz_Test is Integration_Test {
Integration_Test.setUp();

lockupModel = Lockup.Model.LOCKUP_TRANCHED;
initializeDefaultStreamIds();
initializeDefaultStreams();
}
}

Expand Down
22 changes: 11 additions & 11 deletions tests/mocks/BatchMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Batch } from "src/abstracts/Batch.sol";
contract BatchMock is Batch {
error InvalidNumber(uint256);

uint256 internal _number = 10;
uint256 internal _number = 42;

// A view only function.
function getNumber() public view returns (uint256) {
Expand All @@ -18,31 +18,31 @@ contract BatchMock is Batch {
revert InvalidNumber(1);
}

// A state changing function with no return value.
// A state changing function with no payable modifier and no return value.
function setNumber(uint256 number) public {
_number = number;
}

// A state changing function with payable modifier and return value.
// A state changing function with a payable modifier and no return value.
function setNumberWithPayable(uint256 number) public payable {
_number = number;
}

// A state changing function with a payable modifier and a return value.
function setNumberWithPayableAndReturn(uint256 number) public payable returns (uint256) {
_number = number;
return _number;
}

// A state changing function with payable modifier but reverts with custom error.
function setNumberWithPayableAndRevert(uint256 number) public payable {
// A state changing function with a payable modifier, which reverts with a custom error.
function setNumberWithPayableAndRevertError(uint256 number) public payable {
_number = number;
revert InvalidNumber(number);
}

// A state changing function with payable modifier but reverts with string error.
// A state changing function with a payable modifier, which reverts with a reason string.
function setNumberWithPayableAndRevertString(uint256 number) public payable {
_number = number;
revert("You cannot pass");
}

// A state changing function with payable modifier and no return value.
function setNumberWithPayable(uint256 number) public payable {
_number = number;
}
}
Loading

0 comments on commit d4a95b0

Please sign in to comment.