Re introduce the amount param in withdraw

[andreivladbrg]

After a private discussion with @gavriliumircea, he came up with a good idea to reintroduce the amount parameter in withdraw, which was initially removed due to the explanation provided here. In the meantime, we have introduced the storage variable snapshotDebt, which changes how things work.

So, the proposed change in withdraw would be:

• time --> amount
• calculate coveredDebt to the current block (block.timestamp)
• check wether the amount passed is greater than coveredDebt
• the snapshot debt will be updated to coveredDebt - withdrawAmount
• the snapshot time will be updated to block.timestamp

Actual implementation

// Check: the stream balance is not zero.
if (balance == 0) {
    revert Errors.SablierFlow_WithdrawNoFundsAvailable(streamId);
}

uint128 newSnapshotDebt = _coveredDebtOf(streamId, uint40(block.timestamp));

// Revert if the withdrawal amount is greater than what is available.
if (amount > newSnapshotDebt) {
    revert Errors.SablierFlow_WithdrawAmountTooBig();
}

// Update the snapshot debt, with the amount to be withdrawn.
unchecked {
    newSnapshotDebt -= amount;
}

// Effect: update the snapshot debt and time from storage.
_streams[streamId].snapshotDebt = newSnapshotDebt;
_streams[streamId].snapshotTime = uint40(block.timestamp);

---

This change is going to be helpful for the app, as well as for the users, due to a more concise API (and consistent with lockup contracts).

One more thing: this change would allow us to enable the withdrawal of an amount less than what has been snapshot in the case of adjustRps, which was not possible with a time parameter.

wdyt @sablier-labs/solidity ?

@gavriliumircea lmk if i missed something

[andreivladbrg]

Also, @smol-ninja, wouldn't this fix the precision issue from here?

[smol-ninja]

I also found discrepancies in other states such as depletion time, and snapshot time. Its not just in withdraw.

[gavriliumircea]

Correct me If I'm wrong @smol-ninja but for the withdraw case we won't need any normalisations, right?

[smol-ninja]

Denormalization is required to calculate ongoing debt. So thats why the precision loss arises. We would still have to store snapshot time.

[gavriliumircea]

I see your point but at the withdraw level the only problem that might involve the precision is in the case of the requested amount being very closed to the actual maximum withdrawable amount.(In that specific scenario it is helpful to direct the users towards the withdrawMax function in any case)

[smol-ninja]

Yes you are right about it.

[PaulRBerg]

Great idea (and teamwork!). Nothing against it.

[andreivladbrg]

Great, closing since we all agree