From ae77c12e824f71402473a6b4ce407dd2e8413ee0 Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Fri, 22 Nov 2024 10:41:35 +0000 Subject: [PATCH 1/2] feat: shape name in create lockup --- package.json | 2 +- script/Init.s.sol | 6 ++-- src/SablierBatchLockup.sol | 18 ++++++++---- src/SablierLockup.sol | 17 ++++++++--- src/libraries/Errors.sol | 3 ++ src/types/DataTypes.sol | 15 ++++++++++ test/fork/LockupDynamic.t.sol | 6 ++-- test/fork/LockupLinear.t.sol | 6 ++-- test/fork/LockupTranched.t.sol | 6 ++-- .../createWithTimestamps.t.sol | 19 +++++++++++-- .../createWithTimestamps.tree | 27 ++++++++++-------- .../createWithTimestampsLD.t.sol | 9 ++++++ .../createWithTimestampsLL.t.sol | 8 ++++++ .../createWithTimestampsLT.t.sol | 9 ++++++ .../createWithTimestampsLD.t.sol | 28 ++++++++++++++++++- .../createWithTimestampsLL.t.sol | 26 ++++++++++++++++- .../createWithTimestampsLT.t.sol | 28 ++++++++++++++++++- .../handlers/LockupCreateHandler.sol | 6 ++++ test/utils/BatchLockupBuilder.sol | 18 ++++++++---- test/utils/Defaults.sol | 10 +++++-- test/utils/Modifiers.sol | 4 +++ 21 files changed, 226 insertions(+), 45 deletions(-) diff --git a/package.json b/package.json index c38025a3b..4a1ce9656 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "prepare": "husky", "prettier:check": "prettier --check \"**/*.{json,md,svg,yml}\"", "prettier:write": "prettier --write \"**/*.{json,md,svg,yml}\"", - "test": "forge test --nmt \"testFork\"", + "test": "forge test", "test:lite": "FOUNDRY_PROFILE=lite forge test", "test:optimized": "bun run build:optimized && FOUNDRY_PROFILE=test-optimized forge test" } diff --git a/script/Init.s.sol b/script/Init.s.sol index a60b2ae4f..d5b9f775e 100644 --- a/script/Init.s.sol +++ b/script/Init.s.sol @@ -51,7 +51,8 @@ contract Init is BaseScript { asset: asset, cancelable: true, transferable: true, - broker: Broker(address(0), ud60x18(0)) + broker: Broker(address(0), ud60x18(0)), + shape: "cliff" }), LockupLinear.UnlockAmounts({ start: 0, cliff: 0 }), LockupLinear.Durations({ cliff: cliffDurations[i], total: totalDurations[i] }) @@ -82,7 +83,8 @@ contract Init is BaseScript { asset: asset, cancelable: true, transferable: true, - broker: Broker(address(0), ud60x18(0)) + broker: Broker(address(0), ud60x18(0)), + shape: "dynamic" }), segments ); diff --git a/src/SablierBatchLockup.sol b/src/SablierBatchLockup.sol index da878fa17..1203e50c9 100644 --- a/src/SablierBatchLockup.sol +++ b/src/SablierBatchLockup.sol @@ -59,7 +59,8 @@ contract SablierBatchLockup is ISablierBatchLockup { asset: asset, cancelable: batch[i].cancelable, transferable: batch[i].transferable, - broker: batch[i].broker + broker: batch[i].broker, + shape: batch[i].shape }), batch[i].segmentsWithDuration ); @@ -115,7 +116,8 @@ contract SablierBatchLockup is ISablierBatchLockup { cancelable: batch[i].cancelable, transferable: batch[i].transferable, timestamps: Lockup.Timestamps({ start: batch[i].startTime, end: endTime }), - broker: batch[i].broker + broker: batch[i].broker, + shape: batch[i].shape }), batch[i].segments ); @@ -167,7 +169,8 @@ contract SablierBatchLockup is ISablierBatchLockup { asset: asset, cancelable: batch[i].cancelable, transferable: batch[i].transferable, - broker: batch[i].broker + broker: batch[i].broker, + shape: batch[i].shape }), batch[i].unlockAmounts, batch[i].durations @@ -217,7 +220,8 @@ contract SablierBatchLockup is ISablierBatchLockup { cancelable: batch[i].cancelable, transferable: batch[i].transferable, timestamps: batch[i].timestamps, - broker: batch[i].broker + broker: batch[i].broker, + shape: batch[i].shape }), batch[i].unlockAmounts, batch[i].cliffTime @@ -270,7 +274,8 @@ contract SablierBatchLockup is ISablierBatchLockup { asset: asset, cancelable: batch[i].cancelable, transferable: batch[i].transferable, - broker: batch[i].broker + broker: batch[i].broker, + shape: batch[i].shape }), batch[i].tranchesWithDuration ); @@ -326,7 +331,8 @@ contract SablierBatchLockup is ISablierBatchLockup { cancelable: batch[i].cancelable, transferable: batch[i].transferable, timestamps: Lockup.Timestamps({ start: batch[i].startTime, end: endTime }), - broker: batch[i].broker + broker: batch[i].broker, + shape: batch[i].shape }), batch[i].tranches ); diff --git a/src/SablierLockup.sol b/src/SablierLockup.sol index 50b3214eb..0e4bf3ba2 100644 --- a/src/SablierLockup.sol +++ b/src/SablierLockup.sol @@ -158,7 +158,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { cancelable: params.cancelable, transferable: params.transferable, timestamps: timestamps, - broker: params.broker + broker: params.broker, + shape: params.shape }), segments ); @@ -201,7 +202,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { cancelable: params.cancelable, transferable: params.transferable, timestamps: timestamps, - broker: params.broker + broker: params.broker, + shape: params.shape }), unlockAmounts, cliffTime @@ -236,7 +238,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { cancelable: params.cancelable, transferable: params.transferable, timestamps: timestamps, - broker: params.broker + broker: params.broker, + shape: params.shape }), tranches ); @@ -353,6 +356,11 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { internal returns (Lockup.CreateEventCommon memory) { + // Check: the shape name is not greater than 32 bytes to prevent HTML injection attacks. + if (bytes(params.shape).length > 32) { + revert Errors.SablierLockup_ShapeNameTooLong({ nameLength: bytes(params.shape).length, maxLength: 32 }); + } + // Effect: create the stream. _streams[streamId] = Lockup.Stream({ sender: params.sender, @@ -393,7 +401,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { cancelable: params.cancelable, transferable: params.transferable, timestamps: params.timestamps, - broker: params.broker.account + broker: params.broker.account, + shape: params.shape }); } diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 7fd2b665a..0a0692978 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -171,4 +171,7 @@ library Errors { /// @notice Thrown when a function is called on a stream that does not use the expected Lockup model. error SablierLockup_NotExpectedModel(Lockup.Model actualLockupModel, Lockup.Model expectedLockupModel); + + /// @notice Thrown when the shape name is longer than max length. + error SablierLockup_ShapeNameTooLong(uint256 nameLength, uint256 maxLength); } diff --git a/src/types/DataTypes.sol b/src/types/DataTypes.sol index fc0d5319c..af8c128be 100644 --- a/src/types/DataTypes.sol +++ b/src/types/DataTypes.sol @@ -28,6 +28,7 @@ library BatchLockup { bool transferable; LockupDynamic.SegmentWithDuration[] segmentsWithDuration; Broker broker; + string shape; } /// @notice A struct encapsulating all parameters of {SablierLockup.createWithDurationsLL} except for the asset. @@ -40,6 +41,7 @@ library BatchLockup { LockupLinear.Durations durations; LockupLinear.UnlockAmounts unlockAmounts; Broker broker; + string shape; } /// @notice A struct encapsulating all parameters of {SablierLockup.createWithDurationsLT} except for the asset. @@ -51,6 +53,7 @@ library BatchLockup { bool transferable; LockupTranched.TrancheWithDuration[] tranchesWithDuration; Broker broker; + string shape; } /// @notice A struct encapsulating all parameters of {SablierLockup.createWithTimestampsLD} except for the asset. @@ -63,6 +66,7 @@ library BatchLockup { uint40 startTime; LockupDynamic.Segment[] segments; Broker broker; + string shape; } /// @notice A struct encapsulating all parameters of {SablierLockup.createWithTimestampsLL} except for the asset. @@ -76,6 +80,7 @@ library BatchLockup { uint40 cliffTime; LockupLinear.UnlockAmounts unlockAmounts; Broker broker; + string shape; } /// @notice A struct encapsulating all parameters of {SablierLockup.createWithTimestampsLT} except for the asset. @@ -88,6 +93,7 @@ library BatchLockup { uint40 startTime; LockupTranched.Tranche[] tranches; Broker broker; + string shape; } } @@ -136,6 +142,8 @@ library Lockup { /// @param transferable Boolean indicating whether the stream NFT is transferable or not. /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, all as Unix timestamps. /// @param broker The address of the broker who has helped create the stream, e.g. a front-end website. + /// @param shape The name of the stream shape that it should represent. This could be useful to visualize the stream + /// in the UI. struct CreateEventCommon { address funder; address sender; @@ -146,6 +154,7 @@ library Lockup { bool transferable; Lockup.Timestamps timestamps; address broker; + string shape; } /// @notice Struct encapsulating the parameters of the `createWithDurations` functions. @@ -159,6 +168,8 @@ library Lockup { /// @param transferable Indicates if the stream NFT is transferable. /// @param broker Struct encapsulating (i) the address of the broker assisting in creating the stream, and (ii) the /// percentage fee paid to the broker from `totalAmount`, denoted as a fixed-point number. Both can be set to zero. + /// @param shape An optional name for the stream shape that it should represent. This could be useful to visualize + /// the stream in the UI. struct CreateWithDurations { address sender; address recipient; @@ -167,6 +178,7 @@ library Lockup { bool cancelable; bool transferable; Broker broker; + string shape; } /// @notice Struct encapsulating the parameters of the `createWithTimestamps` functions. @@ -181,6 +193,8 @@ library Lockup { /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, both as Unix timestamps. /// @param broker Struct encapsulating (i) the address of the broker assisting in creating the stream, and (ii) the /// percentage fee paid to the broker from `totalAmount`, denoted as a fixed-point number. Both can be set to zero. + /// @param shape An optional name for the stream shape that it should represent. This could be useful to visualize + /// the stream in the UI. struct CreateWithTimestamps { address sender; address recipient; @@ -190,6 +204,7 @@ library Lockup { bool transferable; Timestamps timestamps; Broker broker; + string shape; } /// @notice Enum representing the different distribution models used to create lockup streams. diff --git a/test/fork/LockupDynamic.t.sol b/test/fork/LockupDynamic.t.sol index 58f222379..55316cec7 100644 --- a/test/fork/LockupDynamic.t.sol +++ b/test/fork/LockupDynamic.t.sol @@ -157,7 +157,8 @@ abstract contract Lockup_Dynamic_Fork_Test is Fork_Test { cancelable: true, transferable: true, timestamps: vars.timestamps, - broker: params.broker.account + broker: params.broker.account, + shape: "Dynamic Shape" }), segments: params.segments }); @@ -172,7 +173,8 @@ abstract contract Lockup_Dynamic_Fork_Test is Fork_Test { cancelable: true, transferable: true, timestamps: vars.timestamps, - broker: params.broker + broker: params.broker, + shape: "Dynamic Shape" }), params.segments ); diff --git a/test/fork/LockupLinear.t.sol b/test/fork/LockupLinear.t.sol index 18f32bec0..e77298381 100644 --- a/test/fork/LockupLinear.t.sol +++ b/test/fork/LockupLinear.t.sol @@ -178,7 +178,8 @@ abstract contract Lockup_Linear_Fork_Test is Fork_Test { cancelable: true, transferable: true, timestamps: params.timestamps, - broker: params.broker.account + broker: params.broker.account, + shape: "Linear Shape" }), cliffTime: params.cliffTime, unlockAmounts: params.unlockAmounts @@ -194,7 +195,8 @@ abstract contract Lockup_Linear_Fork_Test is Fork_Test { cancelable: true, transferable: true, timestamps: params.timestamps, - broker: params.broker + broker: params.broker, + shape: "Linear Shape" }), params.unlockAmounts, params.cliffTime diff --git a/test/fork/LockupTranched.t.sol b/test/fork/LockupTranched.t.sol index 26098d04f..936cb20d6 100644 --- a/test/fork/LockupTranched.t.sol +++ b/test/fork/LockupTranched.t.sol @@ -157,7 +157,8 @@ abstract contract Lockup_Tranched_Fork_Test is Fork_Test { cancelable: true, transferable: true, timestamps: vars.timestamps, - broker: params.broker.account + broker: params.broker.account, + shape: "Tranched Shape" }), tranches: params.tranches }); @@ -172,7 +173,8 @@ abstract contract Lockup_Tranched_Fork_Test is Fork_Test { cancelable: true, transferable: true, timestamps: vars.timestamps, - broker: params.broker + broker: params.broker, + shape: "Tranched Shape" }), params.tranches ); diff --git a/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol b/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol index e15543337..4bb197be7 100644 --- a/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol +++ b/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol @@ -68,7 +68,13 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ expectRevert_DelegateCall(callData); } - function test_RevertWhen_BrokerFeeExceedsMaxValue() external whenNoDelegateCall { + function test_RevertWhen_ShapeNameExceeds32Bytes() external whenNoDelegateCall { + _defaultParams.createWithTimestamps.shape = "this name is longer than 32 bytes"; + vm.expectRevert(abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, 33, 32)); + createDefaultStream(); + } + + function test_RevertWhen_BrokerFeeExceedsMaxValue() external whenNoDelegateCall whenShapeNameNotExceed32Bytes { UD60x18 brokerFee = MAX_BROKER_FEE + ud(1); _defaultParams.createWithTimestamps.broker.fee = brokerFee; vm.expectRevert( @@ -77,7 +83,12 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ createDefaultStream(); } - function test_RevertWhen_SenderZeroAddress() external whenNoDelegateCall whenBrokerFeeNotExceedMaxValue { + function test_RevertWhen_SenderZeroAddress() + external + whenNoDelegateCall + whenShapeNameNotExceed32Bytes + whenBrokerFeeNotExceedMaxValue + { _defaultParams.createWithTimestamps.sender = address(0); vm.expectRevert(abi.encodeWithSelector(Errors.SablierHelpers_SenderZeroAddress.selector)); createDefaultStream(); @@ -86,6 +97,7 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ function test_RevertWhen_RecipientZeroAddress() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress { @@ -97,6 +109,7 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ function test_RevertWhen_DepositAmountZero() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -109,6 +122,7 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ function test_RevertWhen_StartTimeZero() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -122,6 +136,7 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ function test_RevertWhen_AssetNotContract() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress diff --git a/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.tree b/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.tree index ae7893ff9..95f987761 100644 --- a/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.tree +++ b/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.tree @@ -2,20 +2,23 @@ CreateWithTimestamps_Integration_Concrete_Test ├── when delegate call │ └── it should revert └── when no delegate call - ├── when broker fee exceeds max value + ├── when shape name exceeds 32 bytes │ └── it should revert - └── when broker fee not exceed max value - ├── when sender zero address + └── when shape name not exceed 32 bytes + ├── when broker fee exceeds max value │ └── it should revert - └── when sender not zero address - ├── when recipient zero address + └── when broker fee not exceed max value + ├── when sender zero address │ └── it should revert - └── when recipient not zero address - ├── when deposit amount zero + └── when sender not zero address + ├── when recipient zero address │ └── it should revert - └── when deposit amount not zero - ├── when start time zero + └── when recipient not zero address + ├── when deposit amount zero │ └── it should revert - └── when start time not zero - └── when asset not contract - └── it should revert \ No newline at end of file + └── when deposit amount not zero + ├── when start time zero + │ └── it should revert + └── when start time not zero + └── when asset not contract + └── it should revert diff --git a/test/integration/concrete/lockup-dynamic/create-with-timestamps-ld/createWithTimestampsLD.t.sol b/test/integration/concrete/lockup-dynamic/create-with-timestamps-ld/createWithTimestampsLD.t.sol index 9499f8007..a277713e8 100644 --- a/test/integration/concrete/lockup-dynamic/create-with-timestamps-ld/createWithTimestampsLD.t.sol +++ b/test/integration/concrete/lockup-dynamic/create-with-timestamps-ld/createWithTimestampsLD.t.sol @@ -31,6 +31,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_SegmentCountZero() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -46,6 +47,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_SegmentCountExceedsMaxValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -64,6 +66,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_SegmentAmountsSumOverflows() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -83,6 +86,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_StartTimeGreaterThanFirstTimestamp() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -111,6 +115,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_StartTimeEqualsFirstTimestamp() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -139,6 +144,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_TimestampsNotStrictlyIncreasing() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -171,6 +177,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_DepositAmountNotEqualSegmentAmountsSum() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -205,6 +212,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_WhenAssetMissesERC20ReturnValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -224,6 +232,7 @@ contract CreateWithTimestampsLD_Integration_Concrete_Test is CreateWithTimestamp function test_WhenAssetNotMissERC20ReturnValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero diff --git a/test/integration/concrete/lockup-linear/create-with-timestamps-ll/createWithTimestampsLL.t.sol b/test/integration/concrete/lockup-linear/create-with-timestamps-ll/createWithTimestampsLL.t.sol index 8689259f0..08c793d11 100644 --- a/test/integration/concrete/lockup-linear/create-with-timestamps-ll/createWithTimestampsLL.t.sol +++ b/test/integration/concrete/lockup-linear/create-with-timestamps-ll/createWithTimestampsLL.t.sol @@ -22,6 +22,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_CliffUnlockAmountNotZero() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -42,6 +43,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_StartTimeNotLessThanEndTime() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -66,6 +68,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_WhenStartTimeLessThanEndTime() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -81,6 +84,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_StartTimeNotLessThanCliffTime() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -106,6 +110,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_CliffTimeNotLessThanEndTime() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -130,6 +135,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_UnlockAmountsSumExceedsDepositAmount() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -155,6 +161,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_WhenAssetMissesERC20ReturnValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -172,6 +179,7 @@ contract CreateWithTimestampsLL_Integration_Concrete_Test is CreateWithTimestamp function test_WhenAssetNotMissERC20ReturnValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress diff --git a/test/integration/concrete/lockup-tranched/create-with-timestamps-lt/createWithTimestampsLT.t.sol b/test/integration/concrete/lockup-tranched/create-with-timestamps-lt/createWithTimestampsLT.t.sol index 5f233593d..00ea246e1 100644 --- a/test/integration/concrete/lockup-tranched/create-with-timestamps-lt/createWithTimestampsLT.t.sol +++ b/test/integration/concrete/lockup-tranched/create-with-timestamps-lt/createWithTimestampsLT.t.sol @@ -23,6 +23,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_TrancheCountZero() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -36,6 +37,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_TrancheCountExceedsMaxValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -53,6 +55,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_TrancheAmountsSumOverflows() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -71,6 +74,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_StartTimeGreaterThanFirstTimestamp() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -101,6 +105,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_StartTimeEqualsFirstTimestamp() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -128,6 +133,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_TimestampsNotStrictlyIncreasing() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -161,6 +167,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_RevertWhen_DepositAmountNotEqualTrancheAmountsSum() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -195,6 +202,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_WhenAssetMissesERC20ReturnValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress @@ -214,6 +222,7 @@ contract CreateWithTimestampsLT_Integration_Concrete_Test is CreateWithTimestamp function test_WhenAssetNotMissERC20ReturnValue() external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenBrokerFeeNotExceedMaxValue whenSenderNotZeroAddress whenRecipientNotZeroAddress diff --git a/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol b/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol index 158f88f00..244e164b3 100644 --- a/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol +++ b/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol @@ -9,9 +9,26 @@ import { Broker, Lockup, LockupDynamic } from "src/types/DataTypes.sol"; import { Lockup_Dynamic_Integration_Fuzz_Test } from "./LockupDynamic.t.sol"; contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integration_Fuzz_Test { + function testFuzz_RevertWhen_ShapeNameExceeds32Bytes(string memory shapeName) + external + whenNoDelegateCall + whenSenderNotZeroAddress + whenRecipientNotZeroAddress + whenDepositAmountNotZero + { + vm.assume(bytes(shapeName).length > 32); + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, bytes(shapeName).length, 32) + ); + + _defaultParams.createWithTimestamps.shape = shapeName; + createDefaultStream(); + } + function testFuzz_RevertWhen_SegmentCountTooHigh(uint256 segmentCount) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -30,6 +47,7 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat ) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -47,6 +65,7 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat function testFuzz_RevertWhen_StartTimeNotLessThanFirstSegmentTimestamp(uint40 firstTimestamp) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -73,6 +92,7 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat function testFuzz_RevertWhen_DepositAmountNotEqualToSegmentAmountsSum(uint128 depositDiff) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -108,6 +128,7 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat function testFuzz_RevertWhen_BrokerFeeTooHigh(Broker memory broker) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -157,6 +178,7 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat ) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -186,6 +208,9 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat fuzzSegmentTimestamps(segments, params.timestamps.start); params.timestamps.end = segments[segments.length - 1].timestamp; + // If shape name exceeds 32 bytes, use the default value. + if (bytes(params.shape).length > 32) params.shape = defaults.SHAPE_NAME(); + // Fuzz the segment amounts and calculate the total and create amounts (deposit and broker fee). Vars memory vars; (vars.totalAmount, vars.createAmounts) = @@ -225,7 +250,8 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat cancelable: params.cancelable, transferable: params.transferable, timestamps: params.timestamps, - broker: params.broker.account + broker: params.broker.account, + shape: params.shape }), segments: segments }); diff --git a/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol b/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol index 90298b970..abf258b6c 100644 --- a/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol +++ b/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol @@ -10,9 +10,26 @@ import { Broker, Lockup, LockupLinear } from "src/types/DataTypes.sol"; import { Lockup_Linear_Integration_Fuzz_Test } from "./LockupLinear.t.sol"; contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integration_Fuzz_Test { + function testFuzz_RevertWhen_ShapeNameExceeds32Bytes(string memory shapeName) + external + whenNoDelegateCall + whenSenderNotZeroAddress + whenRecipientNotZeroAddress + whenDepositAmountNotZero + { + vm.assume(bytes(shapeName).length > 32); + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, bytes(shapeName).length, 32) + ); + + _defaultParams.createWithTimestamps.shape = shapeName; + createDefaultStream(); + } + function testFuzz_RevertWhen_BrokerFeeTooHigh(Broker memory broker) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -30,6 +47,7 @@ contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integrati function testFuzz_RevertWhen_StartTimeNotLessThanCliffTime(uint40 startTime) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -51,6 +69,7 @@ contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integrati ) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -102,6 +121,7 @@ contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integrati ) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -130,6 +150,9 @@ contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integrati boundUint40(params.timestamps.end, params.timestamps.start + 1 seconds, MAX_UNIX_TIMESTAMP); } + // If shape name exceeds 32 bytes, use the default value. + if (bytes(params.shape).length > 32) params.shape = defaults.SHAPE_NAME(); + // Calculate the fee amounts and the deposit amount. Vars memory vars; @@ -171,7 +194,8 @@ contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integrati cancelable: params.cancelable, transferable: params.transferable, timestamps: Lockup.Timestamps({ start: params.timestamps.start, end: params.timestamps.end }), - broker: params.broker.account + broker: params.broker.account, + shape: params.shape }), cliffTime: cliffTime, unlockAmounts: unlockAmounts diff --git a/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol b/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol index 8befc7388..ca64cc9e1 100644 --- a/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol +++ b/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol @@ -11,9 +11,26 @@ import { Broker, Lockup, LockupTranched } from "src/types/DataTypes.sol"; import { Lockup_Tranched_Integration_Fuzz_Test } from "./LockupTranched.t.sol"; contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integration_Fuzz_Test { + function testFuzz_RevertWhen_ShapeNameExceeds32Bytes(string memory shapeName) + external + whenNoDelegateCall + whenSenderNotZeroAddress + whenRecipientNotZeroAddress + whenDepositAmountNotZero + { + vm.assume(bytes(shapeName).length > 32); + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, bytes(shapeName).length, 32) + ); + + _defaultParams.createWithTimestamps.shape = shapeName; + createDefaultStream(); + } + function testFuzz_RevertWhen_TrancheCountTooHigh(uint256 trancheCount) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -32,6 +49,7 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra ) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -50,6 +68,7 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra function testFuzz_RevertWhen_StartTimeNotLessThanFirstTrancheTimestamp(uint40 firstTimestamp) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -77,6 +96,7 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra function testFuzz_RevertWhen_DepositAmountNotEqualToTrancheAmountsSum(uint128 depositDiff) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -114,6 +134,7 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra function testFuzz_RevertWhen_BrokerFeeTooHigh(Broker memory broker) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -163,6 +184,7 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra ) external whenNoDelegateCall + whenShapeNameNotExceed32Bytes whenSenderNotZeroAddress whenRecipientNotZeroAddress whenDepositAmountNotZero @@ -188,6 +210,9 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra params.timestamps.start = boundUint40(params.timestamps.start, 1, defaults.START_TIME()); params.transferable = true; + // If shape name exceeds 32 bytes, use the default value. + if (bytes(params.shape).length > 32) params.shape = defaults.SHAPE_NAME(); + // Fuzz the tranche timestamps. fuzzTrancheTimestamps(tranches, params.timestamps.start); params.timestamps.end = tranches[tranches.length - 1].timestamp; @@ -231,7 +256,8 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra cancelable: params.cancelable, transferable: params.transferable, timestamps: params.timestamps, - broker: params.broker.account + broker: params.broker.account, + shape: params.shape }), tranches: tranches }); diff --git a/test/invariant/handlers/LockupCreateHandler.sol b/test/invariant/handlers/LockupCreateHandler.sol index 0d565701d..861e6d472 100644 --- a/test/invariant/handlers/LockupCreateHandler.sol +++ b/test/invariant/handlers/LockupCreateHandler.sol @@ -65,6 +65,7 @@ contract LockupCreateHandler is BaseHandler, Calculations { // Create the stream. params.asset = asset; + params.shape = "Dynamic Stream"; uint256 streamId = lockup.createWithDurationsLD(params, segments); // Store the stream ID. @@ -96,6 +97,7 @@ contract LockupCreateHandler is BaseHandler, Calculations { // Create the stream. params.asset = asset; + params.shape = "Linear Stream"; uint256 streamId = lockup.createWithDurationsLL(params, unlockAmounts, durations); // Store the stream ID. @@ -140,6 +142,7 @@ contract LockupCreateHandler is BaseHandler, Calculations { // Create the stream. params.asset = asset; + params.shape = "Tranched Stream"; uint256 streamId = lockup.createWithDurationsLT(params, tranches); // Store the stream ID. @@ -181,6 +184,7 @@ contract LockupCreateHandler is BaseHandler, Calculations { // Create the stream. params.asset = asset; + params.shape = "Dynamic Stream"; params.timestamps.end = segments[segments.length - 1].timestamp; uint256 streamId = lockup.createWithTimestampsLD(params, segments); @@ -213,6 +217,7 @@ contract LockupCreateHandler is BaseHandler, Calculations { // Create the stream. params.asset = asset; + params.shape = "Linear Stream"; uint256 streamId = lockup.createWithTimestampsLL(params, unlockAmounts, cliffTime); // Store the stream ID. @@ -257,6 +262,7 @@ contract LockupCreateHandler is BaseHandler, Calculations { // Create the stream. params.asset = asset; + params.shape = "Tranched Stream"; params.timestamps.end = tranches[tranches.length - 1].timestamp; uint256 streamId = lockup.createWithTimestampsLT(params, tranches); diff --git a/test/utils/BatchLockupBuilder.sol b/test/utils/BatchLockupBuilder.sol index a09e14151..69e33235d 100644 --- a/test/utils/BatchLockupBuilder.sol +++ b/test/utils/BatchLockupBuilder.sol @@ -37,7 +37,8 @@ library BatchLockupBuilder { cancelable: params.cancelable, transferable: params.transferable, segmentsWithDuration: segmentsWithDurations, - broker: params.broker + broker: params.broker, + shape: params.shape }); batch = fillBatch(batchSingle, batchSize); } @@ -77,7 +78,8 @@ library BatchLockupBuilder { transferable: params.transferable, durations: durations, unlockAmounts: unlockAmounts, - broker: params.broker + broker: params.broker, + shape: params.shape }); batch = fillBatch(batchSingle, batchSize); } @@ -115,7 +117,8 @@ library BatchLockupBuilder { cancelable: params.cancelable, transferable: params.transferable, tranchesWithDuration: tranchesWithDuration, - broker: params.broker + broker: params.broker, + shape: params.shape }); batch = fillBatch(batchSingle, batchSize); } @@ -154,7 +157,8 @@ library BatchLockupBuilder { transferable: params.transferable, startTime: params.timestamps.start, segments: segments, - broker: params.broker + broker: params.broker, + shape: params.shape }); batch = fillBatch(batchSingle, batchSize); } @@ -195,7 +199,8 @@ library BatchLockupBuilder { timestamps: params.timestamps, cliffTime: cliffTime, unlockAmounts: unlockAmounts, - broker: params.broker + broker: params.broker, + shape: params.shape }); batch = fillBatch(batchSingle, batchSize); } @@ -234,7 +239,8 @@ library BatchLockupBuilder { transferable: params.transferable, startTime: params.timestamps.start, tranches: tranches, - broker: params.broker + broker: params.broker, + shape: params.shape }); batch = fillBatch(batchSingle, batchSize); } diff --git a/test/utils/Defaults.sol b/test/utils/Defaults.sol index 517af1a16..4d731c905 100644 --- a/test/utils/Defaults.sol +++ b/test/utils/Defaults.sol @@ -31,6 +31,7 @@ contract Defaults is Constants { uint256 public constant MAX_TRANCHE_COUNT = 10_000; uint128 public constant REFUND_AMOUNT = DEPOSIT_AMOUNT - WITHDRAW_AMOUNT; uint256 public constant SEGMENT_COUNT = 2; + string public constant SHAPE_NAME = ""; uint40 public immutable START_TIME; uint128 public constant START_AMOUNT = 0; uint128 public constant STREAMED_AMOUNT_26_PERCENT = 2600e18; @@ -138,7 +139,8 @@ contract Defaults is Constants { cancelable: true, transferable: true, timestamps: timestamps, - broker: users.broker + broker: users.broker, + shape: SHAPE_NAME }); } @@ -219,7 +221,8 @@ contract Defaults is Constants { asset: asset, cancelable: true, transferable: true, - broker: broker() + broker: broker(), + shape: SHAPE_NAME }); } @@ -238,7 +241,8 @@ contract Defaults is Constants { cancelable: true, transferable: true, timestamps: lockupTimestamps(), - broker: broker() + broker: broker(), + shape: SHAPE_NAME }); } diff --git a/test/utils/Modifiers.sol b/test/utils/Modifiers.sol index dbd847d3a..f05768581 100644 --- a/test/utils/Modifiers.sol +++ b/test/utils/Modifiers.sol @@ -117,6 +117,10 @@ abstract contract Modifiers is Fuzzers { _; } + modifier whenShapeNameNotExceed32Bytes() { + _; + } + /*////////////////////////////////////////////////////////////////////////// BATCH //////////////////////////////////////////////////////////////////////////*/ From f937d60ac6699a71053603f6a0b0e4038b88575f Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Tue, 26 Nov 2024 14:01:32 +0000 Subject: [PATCH 2/2] refactor: moves shape name check to Helpers docs: uses assertive comments in natspecs test: makes default shape name with non-zero length Co-authored-by: andreivladbrg --- script/Init.s.sol | 4 +-- src/SablierLockup.sol | 14 ++++----- src/libraries/Errors.sol | 6 ++-- src/libraries/Helpers.sol | 30 ++++++++++++++----- src/types/DataTypes.sol | 12 ++++---- .../createWithTimestamps.t.sol | 2 +- .../createWithTimestampsLD.t.sol | 2 +- .../createWithTimestampsLL.t.sol | 2 +- .../createWithTimestampsLT.t.sol | 2 +- test/utils/Defaults.sol | 2 +- 10 files changed, 45 insertions(+), 31 deletions(-) diff --git a/script/Init.s.sol b/script/Init.s.sol index d5b9f775e..5799b21a4 100644 --- a/script/Init.s.sol +++ b/script/Init.s.sol @@ -52,7 +52,7 @@ contract Init is BaseScript { cancelable: true, transferable: true, broker: Broker(address(0), ud60x18(0)), - shape: "cliff" + shape: "Cliff Linear" }), LockupLinear.UnlockAmounts({ start: 0, cliff: 0 }), LockupLinear.Durations({ cliff: cliffDurations[i], total: totalDurations[i] }) @@ -84,7 +84,7 @@ contract Init is BaseScript { cancelable: true, transferable: true, broker: Broker(address(0), ud60x18(0)), - shape: "dynamic" + shape: "Exponential Dynamic" }), segments ); diff --git a/src/SablierLockup.sol b/src/SablierLockup.sol index 0e4bf3ba2..d612786d0 100644 --- a/src/SablierLockup.sol +++ b/src/SablierLockup.sol @@ -356,11 +356,6 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { internal returns (Lockup.CreateEventCommon memory) { - // Check: the shape name is not greater than 32 bytes to prevent HTML injection attacks. - if (bytes(params.shape).length > 32) { - revert Errors.SablierLockup_ShapeNameTooLong({ nameLength: bytes(params.shape).length, maxLength: 32 }); - } - // Effect: create the stream. _streams[streamId] = Lockup.Stream({ sender: params.sender, @@ -422,7 +417,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { segments: segments, maxCount: MAX_COUNT, brokerFee: params.broker.fee, - maxBrokerFee: MAX_BROKER_FEE + maxBrokerFee: MAX_BROKER_FEE, + shape: params.shape }); // Load the stream ID in a variable. @@ -468,7 +464,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { totalAmount: params.totalAmount, unlockAmounts: unlockAmounts, brokerFee: params.broker.fee, - maxBrokerFee: MAX_BROKER_FEE + maxBrokerFee: MAX_BROKER_FEE, + shape: params.shape }); // Load the stream ID in a variable. @@ -522,7 +519,8 @@ contract SablierLockup is ISablierLockup, SablierLockupBase { tranches: tranches, maxCount: MAX_COUNT, brokerFee: params.broker.fee, - maxBrokerFee: MAX_BROKER_FEE + maxBrokerFee: MAX_BROKER_FEE, + shape: params.shape }); // Load the stream ID in a variable. diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 0a0692978..724280ea4 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -77,6 +77,9 @@ library Errors { /// @notice Thrown when trying to create a stream with the sender as the zero address. error SablierHelpers_SenderZeroAddress(); + /// @notice Thrown when trying to create a stream with shape name exceeding 32 bytes. + error SablierHelpers_ShapeNameExceeds32Bytes(uint256 nameLength); + /// @notice Thrown when trying to create a linear stream with a start time not strictly less than the cliff time, /// when the cliff time does not have a zero value. error SablierHelpers_StartTimeNotLessThanCliffTime(uint40 startTime, uint40 cliffTime); @@ -171,7 +174,4 @@ library Errors { /// @notice Thrown when a function is called on a stream that does not use the expected Lockup model. error SablierLockup_NotExpectedModel(Lockup.Model actualLockupModel, Lockup.Model expectedLockupModel); - - /// @notice Thrown when the shape name is longer than max length. - error SablierLockup_ShapeNameTooLong(uint256 nameLength, uint256 maxLength); } diff --git a/src/libraries/Helpers.sol b/src/libraries/Helpers.sol index 6f88aef3a..8bcca1348 100644 --- a/src/libraries/Helpers.sol +++ b/src/libraries/Helpers.sol @@ -84,7 +84,8 @@ library Helpers { LockupDynamic.Segment[] memory segments, uint256 maxCount, UD60x18 brokerFee, - UD60x18 maxBrokerFee + UD60x18 maxBrokerFee, + string memory shape ) public pure @@ -94,7 +95,7 @@ library Helpers { createAmounts = _checkAndCalculateBrokerFee(totalAmount, brokerFee, maxBrokerFee); // Check: validate the user-provided common parameters. - _checkCreateStream(sender, createAmounts.deposit, timestamps.start); + _checkCreateStream(sender, createAmounts.deposit, timestamps.start, shape); // Check: validate the user-provided segments. _checkSegments(segments, createAmounts.deposit, timestamps, maxCount); @@ -108,7 +109,8 @@ library Helpers { uint128 totalAmount, LockupLinear.UnlockAmounts memory unlockAmounts, UD60x18 brokerFee, - UD60x18 maxBrokerFee + UD60x18 maxBrokerFee, + string memory shape ) public pure @@ -118,7 +120,7 @@ library Helpers { createAmounts = _checkAndCalculateBrokerFee(totalAmount, brokerFee, maxBrokerFee); // Check: validate the user-provided common parameters. - _checkCreateStream(sender, createAmounts.deposit, timestamps.start); + _checkCreateStream(sender, createAmounts.deposit, timestamps.start, shape); // Check: validate the user-provided cliff and end times. _checkTimestampsAndUnlockAmounts(createAmounts.deposit, timestamps, cliffTime, unlockAmounts); @@ -132,7 +134,8 @@ library Helpers { LockupTranched.Tranche[] memory tranches, uint256 maxCount, UD60x18 brokerFee, - UD60x18 maxBrokerFee + UD60x18 maxBrokerFee, + string memory shape ) public pure @@ -142,7 +145,7 @@ library Helpers { createAmounts = _checkAndCalculateBrokerFee(totalAmount, brokerFee, maxBrokerFee); // Check: validate the user-provided common parameters. - _checkCreateStream(sender, createAmounts.deposit, timestamps.start); + _checkCreateStream(sender, createAmounts.deposit, timestamps.start, shape); // Check: validate the user-provided segments. _checkTranches(tranches, createAmounts.deposit, timestamps, maxCount); @@ -229,7 +232,15 @@ library Helpers { } /// @dev Checks the user-provided common parameters across lockup streams. - function _checkCreateStream(address sender, uint128 depositAmount, uint40 startTime) private pure { + function _checkCreateStream( + address sender, + uint128 depositAmount, + uint40 startTime, + string memory shape + ) + private + pure + { // Check: the sender is not the zero address. if (sender == address(0)) { revert Errors.SablierHelpers_SenderZeroAddress(); @@ -244,6 +255,11 @@ library Helpers { if (startTime == 0) { revert Errors.SablierHelpers_StartTimeZero(); } + + // Check: the shape name is not greater than 32 bytes to prevent HTML injection attacks. + if (bytes(shape).length > 32) { + revert Errors.SablierHelpers_ShapeNameExceeds32Bytes(bytes(shape).length); + } } /// @dev Checks: diff --git a/src/types/DataTypes.sol b/src/types/DataTypes.sol index af8c128be..66231417a 100644 --- a/src/types/DataTypes.sol +++ b/src/types/DataTypes.sol @@ -142,8 +142,8 @@ library Lockup { /// @param transferable Boolean indicating whether the stream NFT is transferable or not. /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, all as Unix timestamps. /// @param broker The address of the broker who has helped create the stream, e.g. a front-end website. - /// @param shape The name of the stream shape that it should represent. This could be useful to visualize the stream - /// in the UI. + /// @param shape The name of the stream shape that it should represent. This is useful to differentiate between + /// streams in the UI. struct CreateEventCommon { address funder; address sender; @@ -168,8 +168,8 @@ library Lockup { /// @param transferable Indicates if the stream NFT is transferable. /// @param broker Struct encapsulating (i) the address of the broker assisting in creating the stream, and (ii) the /// percentage fee paid to the broker from `totalAmount`, denoted as a fixed-point number. Both can be set to zero. - /// @param shape An optional name for the stream shape that it should represent. This could be useful to visualize - /// the stream in the UI. + /// @param shape An optional name for the stream shape that it should represent. This is useful to differentiate + /// between streams in the UI. struct CreateWithDurations { address sender; address recipient; @@ -193,8 +193,8 @@ library Lockup { /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, both as Unix timestamps. /// @param broker Struct encapsulating (i) the address of the broker assisting in creating the stream, and (ii) the /// percentage fee paid to the broker from `totalAmount`, denoted as a fixed-point number. Both can be set to zero. - /// @param shape An optional name for the stream shape that it should represent. This could be useful to visualize - /// the stream in the UI. + /// @param shape An optional name for the stream shape that it should represent. This is useful to differentiate + /// between streams in the UI. struct CreateWithTimestamps { address sender; address recipient; diff --git a/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol b/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol index 4bb197be7..bf402e88a 100644 --- a/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol +++ b/test/integration/concrete/lockup-base/create-with-timestamps/createWithTimestamps.t.sol @@ -70,7 +70,7 @@ abstract contract CreateWithTimestamps_Integration_Concrete_Test is Integration_ function test_RevertWhen_ShapeNameExceeds32Bytes() external whenNoDelegateCall { _defaultParams.createWithTimestamps.shape = "this name is longer than 32 bytes"; - vm.expectRevert(abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, 33, 32)); + vm.expectRevert(abi.encodeWithSelector(Errors.SablierHelpers_ShapeNameExceeds32Bytes.selector, 33)); createDefaultStream(); } diff --git a/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol b/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol index 244e164b3..c84fc3cd9 100644 --- a/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol +++ b/test/integration/fuzz/lockup-dynamic/createWithTimestampsLD.t.sol @@ -18,7 +18,7 @@ contract CreateWithTimestampsLD_Integration_Fuzz_Test is Lockup_Dynamic_Integrat { vm.assume(bytes(shapeName).length > 32); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, bytes(shapeName).length, 32) + abi.encodeWithSelector(Errors.SablierHelpers_ShapeNameExceeds32Bytes.selector, bytes(shapeName).length) ); _defaultParams.createWithTimestamps.shape = shapeName; diff --git a/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol b/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol index abf258b6c..600b61f26 100644 --- a/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol +++ b/test/integration/fuzz/lockup-linear/createWithTimestampsLL.t.sol @@ -19,7 +19,7 @@ contract CreateWithTimestampsLL_Integration_Fuzz_Test is Lockup_Linear_Integrati { vm.assume(bytes(shapeName).length > 32); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, bytes(shapeName).length, 32) + abi.encodeWithSelector(Errors.SablierHelpers_ShapeNameExceeds32Bytes.selector, bytes(shapeName).length) ); _defaultParams.createWithTimestamps.shape = shapeName; diff --git a/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol b/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol index ca64cc9e1..cc6a38f79 100644 --- a/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol +++ b/test/integration/fuzz/lockup-tranched/createWithTimestampsLT.t.sol @@ -20,7 +20,7 @@ contract CreateWithTimestampsLT_Integration_Fuzz_Test is Lockup_Tranched_Integra { vm.assume(bytes(shapeName).length > 32); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierLockup_ShapeNameTooLong.selector, bytes(shapeName).length, 32) + abi.encodeWithSelector(Errors.SablierHelpers_ShapeNameExceeds32Bytes.selector, bytes(shapeName).length) ); _defaultParams.createWithTimestamps.shape = shapeName; diff --git a/test/utils/Defaults.sol b/test/utils/Defaults.sol index 4d731c905..0b7bda601 100644 --- a/test/utils/Defaults.sol +++ b/test/utils/Defaults.sol @@ -31,7 +31,7 @@ contract Defaults is Constants { uint256 public constant MAX_TRANCHE_COUNT = 10_000; uint128 public constant REFUND_AMOUNT = DEPOSIT_AMOUNT - WITHDRAW_AMOUNT; uint256 public constant SEGMENT_COUNT = 2; - string public constant SHAPE_NAME = ""; + string public constant SHAPE_NAME = "emits in the event"; uint40 public immutable START_TIME; uint128 public constant START_AMOUNT = 0; uint128 public constant STREAMED_AMOUNT_26_PERCENT = 2600e18;