Flow

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

Unnecessary computation in 'SablierFlow::depositViaBroker'

Description: 'SablierFlow::depositViaBroker' calculates the broker fee and updates the snapshot debt. However, the broker fee can be pre-calculated off-chain by the broker, as it is simply the amount the broker wants to keep from their deposit. This creates unnecessary computation that increases both contract deployment costs and gas costs when calling 'depositViaBroker'. The broker can deposit via 'SablierFlow::deposit' and not have to pay more gas since 'deposit' does not calculate a broker fee and transfer fees to the broker.

Impact: Broker will pay more gas to deposit via 'depositViaBroker' rather than just calculating what the broker wants to keep off-chain by the broker and the broker calling the 'SablierFlow::deposit' function. Protocol will spend more gas to deploy the contract if 'depositViaBroker' is used.

Proof of Concept:

  1. 'SablierFlow::depositViaBroker' calls 'Helpers::checkAndCalculateBrokerFee' to calculate the broker fee.

  2. 'SablierFlow::_deposit' is called and deposits amount into the stream.

  3. The broker fee is transferred to the broker.

  4. Broker spends more gas than if the broker called 'SablierFlow::deposit' and calculated what the broker wants to keep off-chain.

Recommended Mitigation:

  • Remove the 'SablierFlow::_depositViaBroker' and 'SablierFlow::depositViaBroker' functions and update the documentation to reflect the broker fee can be pre-calculated off-chain by the broker or the broker can decide on a fixed amount to deposit.

- function _depositViaBroker(uint256 streamId, uint128 totalAmount, Broker memory broker) internal {
- // Check: verify the `broker` and calculate the amounts.
- (uint128 brokerFeeAmount, uint128 depositAmount) =
- Helpers.checkAndCalculateBrokerFee(totalAmount, broker, MAX_FEE);
- // off-chain by the broker
- // Checks, Effects, and Interactions: deposit on stream.
- _deposit(streamId, depositAmount);
-
- // Interaction: transfer the broker's amount.
- _streams[streamId].token.safeTransferFrom({ from: msg.sender, to: broker.account, value:
- brokerFeeAmount });
- }
Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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