Sablier

Sablier
DeFiFoundry
53,440 USDC
View results
Submission Details
Severity: low
Invalid

Non Critical issues (47-54) of 73

NC047 - Use of override is unnecessary:

Starting with Solidity version 0.8.8, using the override keyword when the function solely overrides an interface function, and the function doesn't exist in multiple base contracts, is unnecessary.

Click to show 64 findings
File: v2-core/src/SablierV2LockupDynamic.sol
function getSegments(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupDynamic.Segment[] memory segments)
{
segments = _segments[streamId];
}
function getStream(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupDynamic.StreamLD memory stream)
{
// Retrieve the Lockup stream from storage.
Lockup.Stream memory lockupStream = _streams[streamId];
// Settled streams cannot be canceled.
if (_statusOf(streamId) == Lockup.Status.SETTLED) {
lockupStream.isCancelable = false;
}
stream = LockupDynamic.StreamLD({
amounts: lockupStream.amounts,
asset: lockupStream.asset,
endTime: lockupStream.endTime,
isCancelable: lockupStream.isCancelable,
isDepleted: lockupStream.isDepleted,
isStream: lockupStream.isStream,
isTransferable: lockupStream.isTransferable,
recipient: _ownerOf(streamId),
segments: _segments[streamId],
sender: lockupStream.sender,
startTime: lockupStream.startTime,
wasCanceled: lockupStream.wasCanceled
});
}
function getTimestamps(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupDynamic.Timestamps memory timestamps)
{
timestamps = LockupDynamic.Timestamps({ start: _streams[streamId].startTime, end: _streams[streamId].endTime });
}
function createWithDurations(LockupDynamic.CreateWithDurations calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Generate the canonical segments.
LockupDynamic.Segment[] memory segments = Helpers.calculateSegmentTimestamps(params.segments);
// Checks, Effects and Interactions: create the stream.
streamId = _create(
LockupDynamic.CreateWithTimestamps({
sender: params.sender,
recipient: params.recipient,
totalAmount: params.totalAmount,
asset: params.asset,
cancelable: params.cancelable,
transferable: params.transferable,
startTime: uint40(block.timestamp),
segments: segments,
broker: params.broker
})
);
}
function createWithTimestamps(LockupDynamic.CreateWithTimestamps calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Checks, Effects and Interactions: create the stream.
streamId = _create(params);
}
function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
// If the start time is in the future, return zero.
uint40 blockTimestamp = uint40(block.timestamp);
if (_streams[streamId].startTime >= blockTimestamp) {
return 0;
}
// If the end time is not in the future, return the deposited amount.
uint40 endTime = _streams[streamId].endTime;
if (endTime <= blockTimestamp) {
return _streams[streamId].amounts.deposited;
}
if (_segments[streamId].length > 1) {
// If there is more than one segment, it may be required to iterate over all of them.
return _calculateStreamedAmountForMultipleSegments(streamId);
} else {
// Otherwise, there is only one segment, and the calculation is simpler.
return _calculateStreamedAmountForOneSegment(streamId);
}
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L191:211

File: v2-core/src/SablierV2LockupLinear.sol
function getStream(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupLinear.StreamLL memory stream)
{
// Retrieve the Lockup stream from storage.
Lockup.Stream memory lockupStream = _streams[streamId];
// Settled streams cannot be canceled.
if (_statusOf(streamId) == Lockup.Status.SETTLED) {
lockupStream.isCancelable = false;
}
stream = LockupLinear.StreamLL({
amounts: lockupStream.amounts,
asset: lockupStream.asset,
cliffTime: _cliffs[streamId],
endTime: lockupStream.endTime,
isCancelable: lockupStream.isCancelable,
isTransferable: lockupStream.isTransferable,
isDepleted: lockupStream.isDepleted,
isStream: lockupStream.isStream,
recipient: _ownerOf(streamId),
sender: lockupStream.sender,
startTime: lockupStream.startTime,
wasCanceled: lockupStream.wasCanceled
});
}
function getTimestamps(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupLinear.Timestamps memory timestamps)
{
timestamps = LockupLinear.Timestamps({
start: _streams[streamId].startTime,
cliff: _cliffs[streamId],
end: _streams[streamId].endTime
});
}
function createWithDurations(LockupLinear.CreateWithDurations calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Set the current block timestamp as the stream's start time.
LockupLinear.Timestamps memory timestamps;
timestamps.start = uint40(block.timestamp);
// Calculate the cliff time and the end time. It is safe to use unchecked arithmetic because {_create} will
// nonetheless check that the end time is greater than the cliff time, and also that the cliff time, if set,
// is greater than or equal to the start time.
unchecked {
if (params.durations.cliff > 0) {
timestamps.cliff = timestamps.start + params.durations.cliff;
}
timestamps.end = timestamps.start + params.durations.total;
}
// Checks, Effects and Interactions: create the stream.
streamId = _create(
LockupLinear.CreateWithTimestamps({
sender: params.sender,
recipient: params.recipient,
totalAmount: params.totalAmount,
asset: params.asset,
cancelable: params.cancelable,
transferable: params.transferable,
timestamps: timestamps,
broker: params.broker
})
);
}
function createWithTimestamps(LockupLinear.CreateWithTimestamps calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Checks, Effects and Interactions: create the stream.
streamId = _create(params);
}
function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
// If the cliff time is in the future, return zero.
uint256 cliffTime = uint256(_cliffs[streamId]);
uint256 blockTimestamp = block.timestamp;
if (cliffTime > blockTimestamp) {
return 0;
}
// If the end time is not in the future, return the deposited amount.
uint256 endTime = uint256(_streams[streamId].endTime);
if (blockTimestamp >= endTime) {
return _streams[streamId].amounts.deposited;
}
// In all other cases, calculate the amount streamed so far. Normalization to 18 decimals is not needed
// because there is no mix of amounts with different decimals.
unchecked {
// Calculate how much time has passed since the stream started, and the stream's total duration.
uint256 startTime = uint256(_streams[streamId].startTime);
UD60x18 elapsedTime = ud(blockTimestamp - startTime);
UD60x18 totalDuration = ud(endTime - startTime);
// Divide the elapsed time by the stream's total duration.
UD60x18 elapsedTimePercentage = elapsedTime.div(totalDuration);
// Cast the deposited amount to UD60x18.
UD60x18 depositedAmount = ud(_streams[streamId].amounts.deposited);
// Calculate the streamed amount by multiplying the elapsed time percentage by the deposited amount.
UD60x18 streamedAmount = elapsedTimePercentage.mul(depositedAmount);
// Although the streamed amount should never exceed the deposited amount, this condition is checked
// without asserting to avoid locking funds in case of a bug. If this situation occurs, the withdrawn
// amount is considered to be the streamed amount, and the stream is effectively frozen.
if (streamedAmount.gt(depositedAmount)) {
return _streams[streamId].amounts.withdrawn;
}
// Cast the streamed amount to uint128. This is safe due to the check above.
return uint128(streamedAmount.intoUint256());
}
}
function getCliffTime(uint256 streamId) external view override notNull(streamId) returns (uint40 cliffTime) {
cliffTime = _cliffs[streamId];
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L71:73

File: v2-core/src/SablierV2LockupTranched.sol
function getStream(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupTranched.StreamLT memory stream)
{
// Retrieve the Lockup stream from storage.
Lockup.Stream memory lockupStream = _streams[streamId];
// Settled streams cannot be canceled.
if (_statusOf(streamId) == Lockup.Status.SETTLED) {
lockupStream.isCancelable = false;
}
stream = LockupTranched.StreamLT({
amounts: lockupStream.amounts,
asset: lockupStream.asset,
endTime: lockupStream.endTime,
isCancelable: lockupStream.isCancelable,
isDepleted: lockupStream.isDepleted,
isStream: lockupStream.isStream,
isTransferable: lockupStream.isTransferable,
recipient: _ownerOf(streamId),
sender: lockupStream.sender,
startTime: lockupStream.startTime,
tranches: _tranches[streamId],
wasCanceled: lockupStream.wasCanceled
});
}
function getTimestamps(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupTranched.Timestamps memory timestamps)
{
timestamps = LockupTranched.Timestamps({ start: _streams[streamId].startTime, end: _streams[streamId].endTime });
}
function createWithDurations(LockupTranched.CreateWithDurations calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Generate the canonical tranches.
LockupTranched.Tranche[] memory tranches = Helpers.calculateTrancheTimestamps(params.tranches);
// Checks, Effects and Interactions: create the stream.
streamId = _create(
LockupTranched.CreateWithTimestamps({
sender: params.sender,
recipient: params.recipient,
totalAmount: params.totalAmount,
asset: params.asset,
cancelable: params.cancelable,
transferable: params.transferable,
startTime: uint40(block.timestamp),
tranches: tranches,
broker: params.broker
})
);
}
function createWithTimestamps(LockupTranched.CreateWithTimestamps calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Checks, Effects and Interactions: create the stream.
streamId = _create(params);
}
function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
uint40 blockTimestamp = uint40(block.timestamp);
LockupTranched.Tranche[] memory tranches = _tranches[streamId];
// If the first tranche's timestamp is in the future, return zero.
if (tranches[0].timestamp > blockTimestamp) {
return 0;
}
// If the end time is not in the future, return the deposited amount.
if (_streams[streamId].endTime <= blockTimestamp) {
return _streams[streamId].amounts.deposited;
}
// Sum the amounts in all tranches that have already been vested.
// Using unchecked arithmetic is safe because the sum of the tranche amounts is equal to the total amount
// at this point.
uint128 streamedAmount = tranches[0].amount;
for (uint256 i = 1; i < tranches.length; ++i) {
// The loop breaks at the first tranche with a timestamp in the future. A tranche is considered vested if
// its timestamp is less than or equal to the block timestamp.
if (tranches[i].timestamp > blockTimestamp) {
break;
}
unchecked {
streamedAmount += tranches[i].amount;
}
}
return streamedAmount;
}
function getTranches(uint256 streamId)
external
view
override
notNull(streamId)
returns (LockupTranched.Tranche[] memory tranches)
{
tranches = _tranches[streamId];
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L118:126

File: v2-core/src/SablierV2NFTDescriptor.sol
function tokenURI(IERC721Metadata sablier, uint256 streamId) external view override returns (string memory uri) {
TokenURIVars memory vars;
// Load the contracts.
vars.sablier = ISablierV2Lockup(address(sablier));
vars.sablierModel = mapSymbol(sablier);
vars.sablierStringified = address(sablier).toHexString();
vars.asset = address(vars.sablier.getAsset(streamId));
vars.assetSymbol = safeAssetSymbol(vars.asset);
vars.depositedAmount = vars.sablier.getDepositedAmount(streamId);
// Load the stream's data.
vars.status = stringifyStatus(vars.sablier.statusOf(streamId));
vars.streamedPercentage = calculateStreamedPercentage({
streamedAmount: vars.sablier.streamedAmountOf(streamId),
depositedAmount: vars.depositedAmount
});
// Generate the SVG.
vars.svg = NFTSVG.generateSVG(
NFTSVG.SVGParams({
accentColor: generateAccentColor(address(sablier), streamId),
amount: abbreviateAmount({ amount: vars.depositedAmount, decimals: safeAssetDecimals(vars.asset) }),
assetAddress: vars.asset.toHexString(),
assetSymbol: vars.assetSymbol,
duration: calculateDurationInDays({
startTime: vars.sablier.getStartTime(streamId),
endTime: vars.sablier.getEndTime(streamId)
}),
sablierAddress: vars.sablierStringified,
progress: stringifyPercentage(vars.streamedPercentage),
progressNumerical: vars.streamedPercentage,
status: vars.status,
sablierModel: vars.sablierModel
})
);
// Performs a low-level call to handle older deployments that miss the `isTransferable` function.
(vars.success, vars.returnData) =
address(vars.sablier).staticcall(abi.encodeCall(ISablierV2Lockup.isTransferable, (streamId)));
// When the call has failed, the stream NFT is assumed to be transferable.
vars.isTransferable = vars.success ? abi.decode(vars.returnData, (bool)) : true;
// Generate the JSON metadata.
vars.json = string.concat(
'{"attributes":',
generateAttributes({
assetSymbol: vars.assetSymbol,
sender: vars.sablier.getSender(streamId).toHexString(),
status: vars.status
}),
',"description":"',
generateDescription({
sablierModel: vars.sablierModel,
assetSymbol: vars.assetSymbol,
sablierStringified: vars.sablierStringified,
assetAddress: vars.asset.toHexString(),
streamId: streamId.toString(),
isTransferable: vars.isTransferable
}),
'","external_url":"https://sablier.com","name":"',
generateName({ sablierModel: vars.sablierModel, streamId: streamId.toString() }),
'","image":"data:image/svg+xml;base64,',
Base64.encode(bytes(vars.svg)),
'"}'
);
// Encode the JSON metadata in Base64.
uri = string.concat("data:application/json;base64,", Base64.encode(bytes(vars.json)));
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L47:117

File: v2-core/src/abstracts/SablierV2Lockup.sol
function tokenURI(uint256 streamId) public view override(IERC721Metadata, ERC721) returns (string memory uri) {
// Check: the stream NFT exists.
_requireOwned({ tokenId: streamId });
// Generate the URI describing the stream NFT.
uri = nftDescriptor.tokenURI({ sablier: this, streamId: streamId });
}
function getAsset(uint256 streamId) external view override notNull(streamId) returns (IERC20 asset) {
asset = _streams[streamId].asset;
}
function getDepositedAmount(uint256 streamId)
external
view
override
notNull(streamId)
returns (uint128 depositedAmount)
{
depositedAmount = _streams[streamId].amounts.deposited;
}
function getEndTime(uint256 streamId) external view override notNull(streamId) returns (uint40 endTime) {
endTime = _streams[streamId].endTime;
}
function getRecipient(uint256 streamId) external view override returns (address recipient) {
// Check the stream NFT exists and return the owner, which is the stream's recipient.
recipient = _requireOwned({ tokenId: streamId });
}
function getRefundedAmount(uint256 streamId)
external
view
override
notNull(streamId)
returns (uint128 refundedAmount)
{
refundedAmount = _streams[streamId].amounts.refunded;
}
function getSender(uint256 streamId) external view override notNull(streamId) returns (address sender) {
sender = _streams[streamId].sender;
}
function getStartTime(uint256 streamId) external view override notNull(streamId) returns (uint40 startTime) {
startTime = _streams[streamId].startTime;
}
function getWithdrawnAmount(uint256 streamId)
external
view
override
notNull(streamId)
returns (uint128 withdrawnAmount)
{
withdrawnAmount = _streams[streamId].amounts.withdrawn;
}
function isCancelable(uint256 streamId) external view override notNull(streamId) returns (bool result) {
if (_statusOf(streamId) != Lockup.Status.SETTLED) {
result = _streams[streamId].isCancelable;
}
}
function isCold(uint256 streamId) external view override notNull(streamId) returns (bool result) {
Lockup.Status status = _statusOf(streamId);
result = status == Lockup.Status.SETTLED || status == Lockup.Status.CANCELED || status == Lockup.Status.DEPLETED;
}
function isDepleted(uint256 streamId) external view override notNull(streamId) returns (bool result) {
result = _streams[streamId].isDepleted;
}
function isStream(uint256 streamId) external view override returns (bool result) {
result = _streams[streamId].isStream;
}
function isTransferable(uint256 streamId) external view override notNull(streamId) returns (bool result) {
result = _streams[streamId].isTransferable;
}
function isWarm(uint256 streamId) external view override notNull(streamId) returns (bool result) {
Lockup.Status status = _statusOf(streamId);
result = status == Lockup.Status.PENDING || status == Lockup.Status.STREAMING;
}
function refundableAmountOf(uint256 streamId)
external
view
override
notNull(streamId)
returns (uint128 refundableAmount)
{
// These checks are needed because {_calculateStreamedAmount} does not look up the stream's status. Note that
// checking for `isCancelable` also checks if the stream `wasCanceled` thanks to the protocol invariant that
// canceled streams are not cancelable anymore.
if (_streams[streamId].isCancelable && !_streams[streamId].isDepleted) {
refundableAmount = _streams[streamId].amounts.deposited - _calculateStreamedAmount(streamId);
}
// Otherwise, the result is implicitly zero.
}
function statusOf(uint256 streamId) external view override notNull(streamId) returns (Lockup.Status status) {
status = _statusOf(streamId);
}
function streamedAmountOf(uint256 streamId)
public
view
override
notNull(streamId)
returns (uint128 streamedAmount)
{
streamedAmount = _streamedAmountOf(streamId);
}
function wasCanceled(uint256 streamId) external view override notNull(streamId) returns (bool result) {
result = _streams[streamId].wasCanceled;
}
function withdrawableAmountOf(uint256 streamId)
external
view
override
notNull(streamId)
returns (uint128 withdrawableAmount)
{
withdrawableAmount = _withdrawableAmountOf(streamId);
}
function burn(uint256 streamId) external override noDelegateCall notNull(streamId) {
// Check: only depleted streams can be burned.
if (!_streams[streamId].isDepleted) {
revert Errors.SablierV2Lockup_StreamNotDepleted(streamId);
}
// Check:
// 1. NFT exists (see {IERC721.getApproved}).
// 2. `msg.sender` is either the owner of the NFT or an approved third party.
if (!_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}
// Effect: burn the NFT.
_burn({ tokenId: streamId });
}
function cancel(uint256 streamId) public override noDelegateCall notNull(streamId) {
// Check: the stream is neither depleted nor canceled.
if (_streams[streamId].isDepleted) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
} else if (_streams[streamId].wasCanceled) {
revert Errors.SablierV2Lockup_StreamCanceled(streamId);
}
// Check: `msg.sender` is the stream's sender.
if (!_isCallerStreamSender(streamId)) {
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}
// Checks, Effects and Interactions: cancel the stream.
_cancel(streamId);
}
function cancelMultiple(uint256[] calldata streamIds) external override noDelegateCall {
// Iterate over the provided array of stream IDs and cancel each stream.
uint256 count = streamIds.length;
for (uint256 i = 0; i < count; ++i) {
// Effects and Interactions: cancel the stream.
cancel(streamIds[i]);
}
}
function renounce(uint256 streamId) external override noDelegateCall notNull(streamId) updateMetadata(streamId) {
// Check: the stream is not cold.
Lockup.Status status = _statusOf(streamId);
if (status == Lockup.Status.DEPLETED) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
} else if (status == Lockup.Status.CANCELED) {
revert Errors.SablierV2Lockup_StreamCanceled(streamId);
} else if (status == Lockup.Status.SETTLED) {
revert Errors.SablierV2Lockup_StreamSettled(streamId);
}
// Check: `msg.sender` is the stream's sender.
if (!_isCallerStreamSender(streamId)) {
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}
// Checks and Effects: renounce the stream.
_renounce(streamId);
// Log the renouncement.
emit ISablierV2Lockup.RenounceLockupStream(streamId);
// Interaction: if the recipient is a contract, try to invoke the renounce hook on the recipient without
// reverting if the hook is not implemented, and also without bubbling up any potential revert.
address recipient = _ownerOf(streamId);
if (recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamRenounced(streamId) { } catch { }
}
}
function setNFTDescriptor(ISablierV2NFTDescriptor newNFTDescriptor) external override onlyAdmin {
// Effect: set the NFT descriptor.
ISablierV2NFTDescriptor oldNftDescriptor = nftDescriptor;
nftDescriptor = newNFTDescriptor;
// Log the change of the NFT descriptor.
emit ISablierV2Lockup.SetNFTDescriptor({
admin: msg.sender,
oldNFTDescriptor: oldNftDescriptor,
newNFTDescriptor: newNFTDescriptor
});
// Refresh the NFT metadata for all streams.
emit BatchMetadataUpdate({ _fromTokenId: 1, _toTokenId: nextStreamId - 1 });
}
function withdraw(
uint256 streamId,
address to,
uint128 amount
)
public
override
noDelegateCall
notNull(streamId)
updateMetadata(streamId)
{
// Check: the stream is not depleted.
if (_streams[streamId].isDepleted) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
}
// Check: the withdrawal address is not zero.
if (to == address(0)) {
revert Errors.SablierV2Lockup_WithdrawToZeroAddress(streamId);
}
// Check: the withdraw amount is not zero.
if (amount == 0) {
revert Errors.SablierV2Lockup_WithdrawAmountZero(streamId);
}
// Retrieve the recipient from storage.
address recipient = _ownerOf(streamId);
// Check: if `msg.sender` is neither the stream's recipient nor an approved third party, the withdrawal address
// must be the recipient.
if (to != recipient && !_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierV2Lockup_WithdrawalAddressNotRecipient(streamId, msg.sender, to);
}
// Check: the withdraw amount is not greater than the withdrawable amount.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (amount > withdrawableAmount) {
revert Errors.SablierV2Lockup_Overdraw(streamId, amount, withdrawableAmount);
}
// Retrieve the sender from storage.
address sender = _streams[streamId].sender;
// Effects and Interactions: make the withdrawal.
_withdraw(streamId, to, amount);
// Interaction: if `msg.sender` is not the recipient and the recipient is a contract, try to invoke the
// withdraw hook on it without reverting if the hook is not implemented, and also without bubbling up
// any potential revert.
if (msg.sender != recipient && recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}
// Interaction: if `msg.sender` is not the sender, the sender is a contract and is different from the
// recipient, try to invoke the withdraw hook on it without reverting if the hook is not implemented, and also
// without bubbling up any potential revert.
if (msg.sender != sender && sender.code.length > 0 && sender != recipient) {
try ISablierV2Sender(sender).onLockupStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}
}
function withdrawMax(uint256 streamId, address to) external override {
withdraw({ streamId: streamId, to: to, amount: _withdrawableAmountOf(streamId) });
}
function withdrawMaxAndTransfer(
uint256 streamId,
address newRecipient
)
external
override
noDelegateCall
notNull(streamId)
{
// Check: the caller is the current recipient. This also checks that the NFT was not burned.
address currentRecipient = _ownerOf(streamId);
if (msg.sender != currentRecipient) {
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}
// Skip the withdrawal if the withdrawable amount is zero.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (withdrawableAmount > 0) {
withdraw({ streamId: streamId, to: currentRecipient, amount: withdrawableAmount });
}
// Checks and Effects: transfer the NFT.
_transfer({ from: currentRecipient, to: newRecipient, tokenId: streamId });
}
function withdrawMultiple(
uint256[] calldata streamIds,
uint128[] calldata amounts
)
external
override
noDelegateCall
{
// Check: there is an equal number of `streamIds` and `amounts`.
uint256 streamIdsCount = streamIds.length;
uint256 amountsCount = amounts.length;
if (streamIdsCount != amountsCount) {
revert Errors.SablierV2Lockup_WithdrawArrayCountsNotEqual(streamIdsCount, amountsCount);
}
// Iterate over the provided array of stream IDs, and withdraw from each stream to the recipient.
for (uint256 i = 0; i < streamIdsCount; ++i) {
// Checks, Effects and Interactions: check the parameters and make the withdrawal.
withdraw({ streamId: streamIds[i], to: _ownerOf(streamIds[i]), amount: amounts[i] });
}
}
function _update(
address to,
uint256 streamId,
address auth
)
internal
override
updateMetadata(streamId)
returns (address)
{
address from = _ownerOf(streamId);
if (from != address(0) && to != address(0) && !_streams[streamId].isTransferable) {
revert Errors.SablierV2Lockup_NotTransferable(streamId);
}
return super._update(to, streamId, auth);
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L522:539

File: v2-core/src/abstracts/Adminable.sol
function transferAdmin(address newAdmin) public virtual override onlyAdmin {
// Effect: update the admin.
admin = newAdmin;
// Log the transfer of the admin.
emit IAdminable.TransferAdmin({ oldAdmin: msg.sender, newAdmin: newAdmin });
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/Adminable.sol#L34:40

File: v2-periphery/src/SablierV2BatchLockup.sol
function createWithDurationsLD(
ISablierV2LockupDynamic lockupDynamic,
IERC20 asset,
BatchLockup.CreateWithDurationsLD[] calldata batch
)
external
override
returns (uint256[] memory streamIds)
{
// Check that the batch size is not zero.
uint256 batchSize = batch.length;
if (batchSize == 0) {
revert Errors.SablierV2BatchLockup_BatchSizeZero();
}
// Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
// transactions will revert if there is overflow.
uint256 i;
uint256 transferAmount;
for (i = 0; i < batchSize; ++i) {
unchecked {
transferAmount += batch[i].totalAmount;
}
}
// Perform the ERC-20 transfer and approve {SablierV2LockupDynamic} to spend the amount of assets.
_handleTransfer(address(lockupDynamic), asset, transferAmount);
// Create a stream for each element in the parameter array.
streamIds = new uint256[](batchSize);
for (i = 0; i < batchSize; ++i) {
// Create the stream.
streamIds[i] = lockupDynamic.createWithDurations(
LockupDynamic.CreateWithDurations({
sender: batch[i].sender,
recipient: batch[i].recipient,
totalAmount: batch[i].totalAmount,
asset: asset,
cancelable: batch[i].cancelable,
transferable: batch[i].transferable,
segments: batch[i].segments,
broker: batch[i].broker
})
);
}
}
function createWithTimestampsLD(
ISablierV2LockupDynamic lockupDynamic,
IERC20 asset,
BatchLockup.CreateWithTimestampsLD[] calldata batch
)
external
override
returns (uint256[] memory streamIds)
{
// Check that the batch size is not zero.
uint256 batchSize = batch.length;
if (batchSize == 0) {
revert Errors.SablierV2BatchLockup_BatchSizeZero();
}
// Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
// transactions will revert if there is overflow.
uint256 i;
uint256 transferAmount;
for (i = 0; i < batchSize; ++i) {
unchecked {
transferAmount += batch[i].totalAmount;
}
}
// Perform the ERC-20 transfer and approve {SablierV2LockupDynamic} to spend the amount of assets.
_handleTransfer(address(lockupDynamic), asset, transferAmount);
// Create a stream for each element in the parameter array.
streamIds = new uint256[](batchSize);
for (i = 0; i < batchSize; ++i) {
// Create the stream.
streamIds[i] = lockupDynamic.createWithTimestamps(
LockupDynamic.CreateWithTimestamps({
sender: batch[i].sender,
recipient: batch[i].recipient,
totalAmount: batch[i].totalAmount,
asset: asset,
cancelable: batch[i].cancelable,
transferable: batch[i].transferable,
startTime: batch[i].startTime,
segments: batch[i].segments,
broker: batch[i].broker
})
);
}
}
function createWithDurationsLL(
ISablierV2LockupLinear lockupLinear,
IERC20 asset,
BatchLockup.CreateWithDurationsLL[] calldata batch
)
external
override
returns (uint256[] memory streamIds)
{
// Check that the batch size is not zero.
uint256 batchSize = batch.length;
if (batchSize == 0) {
revert Errors.SablierV2BatchLockup_BatchSizeZero();
}
// Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
// transactions will revert if there is overflow.
uint256 i;
uint256 transferAmount;
for (i = 0; i < batchSize; ++i) {
unchecked {
transferAmount += batch[i].totalAmount;
}
}
// Perform the ERC-20 transfer and approve {SablierV2LockupLinear} to spend the amount of assets.
_handleTransfer(address(lockupLinear), asset, transferAmount);
// Create a stream for each element in the parameter array.
streamIds = new uint256[](batchSize);
for (i = 0; i < batchSize; ++i) {
// Create the stream.
streamIds[i] = lockupLinear.createWithDurations(
LockupLinear.CreateWithDurations({
sender: batch[i].sender,
recipient: batch[i].recipient,
totalAmount: batch[i].totalAmount,
asset: asset,
cancelable: batch[i].cancelable,
transferable: batch[i].transferable,
durations: batch[i].durations,
broker: batch[i].broker
})
);
}
}
function createWithTimestampsLL(
ISablierV2LockupLinear lockupLinear,
IERC20 asset,
BatchLockup.CreateWithTimestampsLL[] calldata batch
)
external
override
returns (uint256[] memory streamIds)
{
// Check that the batch is not empty.
uint256 batchSize = batch.length;
if (batchSize == 0) {
revert Errors.SablierV2BatchLockup_BatchSizeZero();
}
// Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
// transactions will revert if there is overflow.
uint256 i;
uint256 transferAmount;
for (i = 0; i < batchSize; ++i) {
unchecked {
transferAmount += batch[i].totalAmount;
}
}
// Perform the ERC-20 transfer and approve {SablierV2LockupLinear} to spend the amount of assets.
_handleTransfer(address(lockupLinear), asset, transferAmount);
// Create a stream for each element in the parameter array.
streamIds = new uint256[](batchSize);
for (i = 0; i < batchSize; ++i) {
// Create the stream.
streamIds[i] = lockupLinear.createWithTimestamps(
LockupLinear.CreateWithTimestamps({
sender: batch[i].sender,
recipient: batch[i].recipient,
totalAmount: batch[i].totalAmount,
asset: asset,
cancelable: batch[i].cancelable,
transferable: batch[i].transferable,
timestamps: batch[i].timestamps,
broker: batch[i].broker
})
);
}
}
function createWithDurationsLT(
ISablierV2LockupTranched lockupTranched,
IERC20 asset,
BatchLockup.CreateWithDurationsLT[] calldata batch
)
external
override
returns (uint256[] memory streamIds)
{
// Check that the batch size is not zero.
uint256 batchSize = batch.length;
if (batchSize == 0) {
revert Errors.SablierV2BatchLockup_BatchSizeZero();
}
// Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
// transactions will revert if there is overflow.
uint256 i;
uint256 transferAmount;
for (i = 0; i < batchSize; ++i) {
unchecked {
transferAmount += batch[i].totalAmount;
}
}
// Perform the ERC-20 transfer and approve {SablierV2LockupTranched} to spend the amount of assets.
_handleTransfer(address(lockupTranched), asset, transferAmount);
// Create a stream for each element in the parameter array.
streamIds = new uint256[](batchSize);
for (i = 0; i < batchSize; ++i) {
// Create the stream.
streamIds[i] = lockupTranched.createWithDurations(
LockupTranched.CreateWithDurations({
sender: batch[i].sender,
recipient: batch[i].recipient,
totalAmount: batch[i].totalAmount,
asset: asset,
cancelable: batch[i].cancelable,
transferable: batch[i].transferable,
tranches: batch[i].tranches,
broker: batch[i].broker
})
);
}
}
function createWithTimestampsLT(
ISablierV2LockupTranched lockupTranched,
IERC20 asset,
BatchLockup.CreateWithTimestampsLT[] calldata batch
)
external
override
returns (uint256[] memory streamIds)
{
// Check that the batch size is not zero.
uint256 batchSize = batch.length;
if (batchSize == 0) {
revert Errors.SablierV2BatchLockup_BatchSizeZero();
}
// Calculate the sum of all of stream amounts. It is safe to use unchecked addition because one of the create
// transactions will revert if there is overflow.
uint256 i;
uint256 transferAmount;
for (i = 0; i < batchSize; ++i) {
unchecked {
transferAmount += batch[i].totalAmount;
}
}
// Perform the ERC-20 transfer and approve {SablierV2LockupTranched} to spend the amount of assets.
_handleTransfer(address(lockupTranched), asset, transferAmount);
// Create a stream for each element in the parameter array.
streamIds = new uint256[](batchSize);
for (i = 0; i < batchSize; ++i) {
// Create the stream.
streamIds[i] = lockupTranched.createWithTimestamps(
LockupTranched.CreateWithTimestamps({
sender: batch[i].sender,
recipient: batch[i].recipient,
totalAmount: batch[i].totalAmount,
asset: asset,
cancelable: batch[i].cancelable,
transferable: batch[i].transferable,
startTime: batch[i].startTime,
tranches: batch[i].tranches,
broker: batch[i].broker
})
);
}
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2BatchLockup.sol#L274:320

File: v2-periphery/src/SablierV2MerkleLL.sol
function claim(
uint256 index,
address recipient,
uint128 amount,
bytes32[] calldata merkleProof
)
external
override
returns (uint256 streamId)
{
// Generate the Merkle tree leaf by hashing the corresponding parameters. Hashing twice prevents second
// preimage attacks.
bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(index, recipient, amount))));
// Check: validate the function.
_checkClaim(index, leaf, merkleProof);
// Effect: mark the index as claimed.
_claimedBitMap.set(index);
// Interaction: create the stream via {SablierV2LockupLinear}.
streamId = LOCKUP_LINEAR.createWithDurations(
LockupLinear.CreateWithDurations({
sender: admin,
recipient: recipient,
totalAmount: amount,
asset: ASSET,
cancelable: CANCELABLE,
transferable: TRANSFERABLE,
durations: streamDurations,
broker: Broker({ account: address(0), fee: ud(0) })
})
);
// Log the claim.
emit Claim(index, recipient, amount, streamId);
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLL.sol#L59:95

File: v2-periphery/src/SablierV2MerkleLT.sol
function claim(
uint256 index,
address recipient,
uint128 amount,
bytes32[] calldata merkleProof
)
external
override
returns (uint256 streamId)
{
// Generate the Merkle tree leaf by hashing the corresponding parameters. Hashing twice prevents second
// preimage attacks.
bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(index, recipient, amount))));
// Check: validate the function.
_checkClaim(index, leaf, merkleProof);
// Calculate the tranches based on the unlock percentages.
LockupTranched.TrancheWithDuration[] memory tranches = _calculateTranches(amount);
// Effect: mark the index as claimed.
_claimedBitMap.set(index);
// Interaction: create the stream via {SablierV2LockupTranched}.
streamId = LOCKUP_TRANCHED.createWithDurations(
LockupTranched.CreateWithDurations({
sender: admin,
recipient: recipient,
totalAmount: amount,
asset: ASSET,
cancelable: CANCELABLE,
transferable: TRANSFERABLE,
tranches: tranches,
broker: Broker({ account: address(0), fee: ZERO })
})
);
// Log the claim.
emit Claim(index, recipient, amount, streamId);
}
function getTranchesWithPercentages() external view override returns (MerkleLT.TrancheWithPercentage[] memory) {
return _tranchesWithPercentages;
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLT.sol#L65:67

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
function getFirstClaimTime() external view override returns (uint40) {
return _firstClaimTime;
}
function hasClaimed(uint256 index) public view override returns (bool) {
return _claimedBitMap.get(index);
}
function hasExpired() public view override returns (bool) {
return EXPIRATION > 0 && EXPIRATION <= block.timestamp;
}
function name() external view override returns (string memory) {
return string(abi.encodePacked(NAME));
}
function clawback(address to, uint128 amount) external override onlyAdmin {
// Check: current timestamp is over the grace period and the campaign has not expired.
if (_hasGracePeriodPassed() && !hasExpired()) {
revert Errors.SablierV2MerkleLockup_ClawbackNotAllowed({
blockTimestamp: block.timestamp,
expiration: EXPIRATION,
firstClaimTime: _firstClaimTime
});
}
// Effect: transfer the tokens to the provided address.
ASSET.safeTransfer(to, amount);
// Log the clawback.
emit Clawback(admin, to, amount);
}

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L107:122

NC048 - Non-external/public function names should begin with an underscore:

According to the Solidity Style Guide, non-external/public function names should begin with an underscore

File: v2-core/src/SablierV2NFTDescriptor.sol
133 function abbreviateAmount(uint256 amount, uint256 decimals) internal pure returns (string memory) {
171 function calculateDurationInDays(uint256 startTime, uint256 endTime) internal pure returns (string memory) {
190 function calculateStreamedPercentage(
206 function generateAccentColor(address sablier, uint256 streamId) internal view returns (string memory) {
240 function generateAttributes(
261 function generateDescription(
301 function generateName(string memory sablierModel, string memory streamId) internal pure returns (string memory) {
307 function mapSymbol(IERC721Metadata sablier) internal view returns (string memory) {
322 function safeAssetDecimals(address asset) internal view returns (uint8) {
334 function safeAssetSymbol(address asset) internal view returns (string memory) {
355 function stringifyFractionalAmount(uint256 fractionalAmount) internal pure returns (string memory) {
372 function stringifyPercentage(uint256 percentage) internal pure returns (string memory) {
384 function stringifyStatus(Lockup.Status status) internal pure returns (string memory) {

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L0:0

NC049 - Unused import:

The identifier is imported but never used within the file

Click to show 11 findings
File: v2-core/src/SablierV2LockupDynamic.sol
6 import { ERC721 } from "hot/node_modules/@openzeppelin/contracts/token/ERC721/ERC721.sol";
7 import { PRBMathCastingUint128 as CastingUint128 } from "hot/@prb/math/src/casting/Uint128.sol";
8 import { PRBMathCastingUint40 as CastingUint40 } from "hot/@prb/math/src/casting/Uint40.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L0:0

File: v2-core/src/SablierV2LockupLinear.sol
6 import { ERC721 } from "hot/node_modules/@openzeppelin/contracts/token/ERC721/ERC721.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L0:0

File: v2-core/src/SablierV2LockupTranched.sol
6 import { ERC721 } from "hot/node_modules/@openzeppelin/contracts/token/ERC721/ERC721.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L0:0

File: v2-core/src/abstracts/SablierV2Lockup.sol
8 import { IERC721Metadata } from "hot/node_modules/@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L0:0

File: v2-core/src/libraries/Helpers.sol
6 import { Lockup, LockupDynamic, LockupLinear, LockupTranched } from "../types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/Helpers.sol#L0:0

File: v2-periphery/src/SablierV2BatchLockup.sol
13 import { BatchLockup } from "./types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2BatchLockup.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLL.sol
13 import { MerkleLockup } from "./types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLL.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLT.sol
13 import { MerkleLockup, MerkleLT } from "./types/DataTypes.sol";
13 import { MerkleLockup, MerkleLT } from "./types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLT.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLockupFactory.sol
7 import { LockupLinear } from "hot/@sablier/v2-core/src/types/DataTypes.sol";
15 import { MerkleLockup, MerkleLT } from "./types/DataTypes.sol";
15 import { MerkleLockup, MerkleLT } from "./types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L0:0

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
11 import { MerkleLockup } from "../types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L0:0

File: v2-periphery/src/types/DataTypes.sol
7 import { Broker, LockupDynamic, LockupLinear, LockupTranched } from "hot/@sablier/v2-core/src/types/DataTypes.sol";
7 import { Broker, LockupDynamic, LockupLinear, LockupTranched } from "hot/@sablier/v2-core/src/types/DataTypes.sol";
7 import { Broker, LockupDynamic, LockupLinear, LockupTranched } from "hot/@sablier/v2-core/src/types/DataTypes.sol";

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/types/DataTypes.sol#L0:0

NC050 - Use string.concat() on strings instead of abi.encodePacked() for clearer semantic meaning:

Starting with version 0.8.12, Solidity has the string.concat() function, which allows one to concatenate a list of strings, without extra padding. Using this function rather than abi.encodePacked() makes the intended operation more clear, leading to less reviewer confusion.

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
74 NAME = bytes32(abi.encodePacked(params.name));
99 return string(abi.encodePacked(NAME));

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L0:0

NC051 - Unspecific Compiler Version Pragma:

Click to show 19 findings
File: v2-core/src/SablierV2LockupDynamic.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L2:2

File: v2-core/src/SablierV2LockupLinear.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L2:2

File: v2-core/src/SablierV2LockupTranched.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L2:2

File: v2-core/src/SablierV2NFTDescriptor.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2NFTDescriptor.sol#L3:3

File: v2-core/src/abstracts/Adminable.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/Adminable.sol#L2:2

File: v2-core/src/abstracts/NoDelegateCall.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/NoDelegateCall.sol#L2:2

File: v2-core/src/abstracts/SablierV2Lockup.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L2:2

File: v2-core/src/libraries/Errors.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/Errors.sol#L2:2

File: v2-core/src/libraries/Helpers.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/Helpers.sol#L2:2

File: v2-core/src/libraries/NFTSVG.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/NFTSVG.sol#L3:3

File: v2-core/src/libraries/SVGElements.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/SVGElements.sol#L3:3

File: v2-core/src/types/DataTypes.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/types/DataTypes.sol#L2:2

File: v2-periphery/src/SablierV2BatchLockup.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2BatchLockup.sol#L2:2

File: v2-periphery/src/SablierV2MerkleLL.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2MerkleLL.sol#L2:2

File: v2-periphery/src/SablierV2MerkleLT.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2MerkleLT.sol#L2:2

File: v2-periphery/src/SablierV2MerkleLockupFactory.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2MerkleLockupFactory.sol#L2:2

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2MerkleLockup.sol#L2:2

File: v2-periphery/src/libraries/Errors.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/Errors.sol#L2:2

File: v2-periphery/src/types/DataTypes.sol
pragma solidity >=0.8.22;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/types/DataTypes.sol#L2:2

NC052 - Use bytes.concat() on bytes instead of abi.encodePacked() for clearer semantic meaning:

Starting with version 0.8.4, Solidity has the bytes.concat() function, which allows one to concatenate a list of bytes/strings, without extra padding. Using this function rather than abi.encodePacked() makes the intended operation more clear, leading to less reviewer confusion.

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
99 return string(abi.encodePacked(NAME));

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L0:0

NC053 - Style guide: State and local variables should be named using lowerCamelCase:

The Solidity style guide says to use mixedCase for local and state variable names. Note that while OpenZeppelin may not follow this advice, it still is the recommended way of naming variables.

Click to show 6 findings
File: v2-core/src/SablierV2LockupDynamic.sol
50 uint256 public immutable override MAX_SEGMENT_COUNT;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L0:0

File: v2-core/src/SablierV2LockupTranched.sol
45 uint256 public immutable override MAX_TRANCHE_COUNT;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L0:0

File: v2-core/src/abstracts/NoDelegateCall.sol
10 address private immutable ORIGINAL;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/NoDelegateCall.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLL.sol
29 ISablierV2LockupLinear public immutable override LOCKUP_LINEAR;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2MerkleLL.sol#L0:0

File: v2-periphery/src/SablierV2MerkleLT.sol
29 ISablierV2LockupTranched public immutable override LOCKUP_TRANCHED;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2MerkleLT.sol#L0:0

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
28 IERC20 public immutable override ASSET;
31 bool public immutable override CANCELABLE;
34 uint40 public immutable override EXPIRATION;
37 bytes32 public immutable override MERKLE_ROOT;
40 bytes32 internal immutable NAME;
43 bool public immutable override TRANSFERABLE;

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L0:0

NC054 - Unusual loop variable:

The normal name for loop variables is i, and when there is a nested loop, to use j. Not following this convention may lead to some reviewer confusion.

File: v2-core/src/libraries/Helpers.sol
252 for (uint256 index = 0; index < count; ++index) {
253 // Add the current segment amount to the sum.
254 segmentAmountsSum += segments[index].amount;
255
256 // Check: the current timestamp is strictly greater than the previous timestamp.
257 currentSegmentTimestamp = segments[index].timestamp;
258 if (currentSegmentTimestamp <= previousSegmentTimestamp) {
259 revert Errors.SablierV2LockupDynamic_SegmentTimestampsNotOrdered(
260 index, previousSegmentTimestamp, currentSegmentTimestamp
261 );
262 }
263
264 // Make the current timestamp the previous timestamp of the next loop iteration.
265 previousSegmentTimestamp = currentSegmentTimestamp;
266 }
315 for (uint256 index = 0; index < count; ++index) {
316 // Add the current tranche amount to the sum.
317 trancheAmountsSum += tranches[index].amount;
318
319 // Check: the current timestamp is strictly greater than the previous timestamp.
320 currentTrancheTimestamp = tranches[index].timestamp;
321 if (currentTrancheTimestamp <= previousTrancheTimestamp) {
322 revert Errors.SablierV2LockupTranched_TrancheTimestampsNotOrdered(
323 index, previousTrancheTimestamp, currentTrancheTimestamp
324 );
325 }
326
327 // Make the current timestamp the previous timestamp of the next loop iteration.
328 previousTrancheTimestamp = currentTrancheTimestamp;
329 }

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/libraries/Helpers.sol#L0:0

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.