Flow
Sablier
FoundryDeFi
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Lack of Check for Identical Protocol Fee Settings, Resulting in Redundant Execution, Additional GAS Costs, and Potential Confusion in Communication with Off-Chain Tools (e.g., APIs, websites, databases).

theirrationalone

Summary

In the SablierFlowBase.sol file, there is an external function, SablierFlowBase::setProtocolFee, which allows an
admin to set or update the protocol fee. While the function works as expected, it lacks optimization. Specifically, it
does not check if the new protocol fee matches the current fee, which can lead to redundant function executions and
unnecessary GAS costs.

See the issue here:
SablierFlowBase::setProtocolFee issue:

Vulnerability Details

/// @inheritdoc ISablierFlowBase
function setProtocolFee(IERC20 token, UD60x18 newProtocolFee) external override onlyAdmin {
// Check: the new protocol fee is not greater than the maximum allowed.
if (newProtocolFee > MAX_FEE) {
revert Errors.SablierFlowBase_ProtocolFeeTooHigh(newProtocolFee, MAX_FEE);
}
​
UD60x18 oldProtocolFee = protocolFee[token];
​
@> // Missing check for identical protocol fee
​
// Effects: set the new protocol fee.
protocolFee[token] = newProtocolFee;
​
// Log the change of the protocol fee.
emit ISablierFlowBase.SetProtocolFee({
admin: msg.sender,
token: token,
oldProtocolFee: oldProtocolFee,
newProtocolFee: newProtocolFee
});
​
// Refresh the NFT metadata for all streams.
emit BatchMetadataUpdate({ _fromTokenId: 1, _toTokenId: nextStreamId - 1 });
}

Currently, the function lacks a check to prevent setting the protocol fee to the existing value, leading to redundant
operations. Optimizing this with a simple check would ensure efficiency, prevent unnecessary gas expenditure, and
streamline contract execution.

Impact

Currently,

  • there's an unnecessary gas expenditure, and streamline contract execution.

  • Emitting two events unnecessarily:

    • Event: BatchMetadataUpdate

    • Event: SetProtocolFee

This redundancy can lead to off-chain tools—such as websites, APIs, and databases—receiving duplicate data, potentially
increasing costs associated with bandwidth usage and creating less efficient communication with the protocol.

Tools Used

Manual Review

Recommendations

Add a condition to check if the new protocol fee is the same as the existing one, preventing redundant updates. Below is
the suggested modification to the setProtocolFee function:

  • Updated Code

In SablierFlowBase.sol:

...
​
/// @inheritdoc ISablierFlowBase
function setProtocolFee(IERC20 token, UD60x18 newProtocolFee) external override onlyAdmin {
// Check: the new protocol fee is not greater than the maximum allowed.
if (newProtocolFee > MAX_FEE) {
revert Errors.SablierFlowBase_ProtocolFeeTooHigh(newProtocolFee, MAX_FEE);
}
​
UD60x18 oldProtocolFee = protocolFee[token];
​
+ // Prevent redundant update if the new protocol fee matches the old one.
+ if (newProtocolFee == oldProtocolFee) {
+ revert Errors.SablierFlowBase__SameProtocolFee(newProtocolFee, oldProtocolFee);
+ }
​
// Effects: set the new protocol fee.
protocolFee[token] = newProtocolFee;
​
// Log the change of the protocol fee.
emit ISablierFlowBase.SetProtocolFee({
admin: msg.sender,
token: token,
oldProtocolFee: oldProtocolFee,
newProtocolFee: newProtocolFee
});
​
// Refresh the NFT metadata for all streams.
emit BatchMetadataUpdate({ _fromTokenId: 1, _toTokenId: nextStreamId - 1 });
}
​
...

In Errors.sol:

/// @title Errors
/// @notice Library with custom errors used across the Flow contract.
library Errors {
/*//////////////////////////////////////////////////////////////////////////
GENERICS
//////////////////////////////////////////////////////////////////////////*/
​
/// @notice Thrown when an unexpected error occurs during a batch call.
error BatchError(bytes errorData);
​
...
...
​
/// @notice Thrown when attempting to set the protocol fee above the allowed maximum.
error SablierFlowBase_ProtocolFeeTooHigh(UD60x18 newProtocolFee, UD60x18 maxFee);
​
+ /// @notice Thrown when attempting to set the protocol fee to its current value.
+ error SablierFlowBase__SameProtocolFee(UD60x18 newProtocolFee, UD60x18 oldProtocolFee);
​
/// @notice Thrown when attempting to recover a token with zero surplus.
error SablierFlowBase_SurplusZero(address token);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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