Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dedup _withdrawableAmountOf #455

Closed
PaulRBerg opened this issue Apr 24, 2023 · 5 comments
Closed

Dedup _withdrawableAmountOf #455

PaulRBerg opened this issue Apr 24, 2023 · 5 comments
Assignees

Comments

@PaulRBerg
Copy link
Member

The _withdrawableAmountOf implementations in SablierV2LockupLinear and SablierV2LockupDynamic can be moved to the SablierV2Lockup abstract.

@andreivladbrg
Copy link
Member

andreivladbrg commented Apr 24, 2023

It would require to add _streamedAmountOf and getWithdrawnAmount as a virtual functions
https://github.com/sablierhq/v2-core/blob/2b71d0762706ad04cdad58e37d2c6db0b6274496/src/SablierV2LockupLinear.sol#L348

Wouldn't calling getWithdrawnAmount increase the gas?

@PaulRBerg
Copy link
Member Author

We have the internal function _withdrawableAmountOf defined in SablierV2Lockup.

@andreivladbrg
Copy link
Member

We have the internal function _withdrawableAmountOf defined in SablierV2Lockup.

Yes we do but we don't have the _streamedAmountOf and getWithdrawnAmount functions. If you really want to move it there, we could also move refundableAmountOf.

@PaulRBerg
Copy link
Member Author

We can simply define it like this:

/// @inheritdoc ISablierV2Lockup
function withdrawableAmountOf(uint256 streamId)
    external
    view
    override
    isNotNull(streamId)
    returns (uint128 withdrawableAmount)
{
    withdrawableAmount = _withdrawableAmountOf(streamId);
}

You'll see when I make the PR.

@PaulRBerg
Copy link
Member Author

If you really want to move it there, we could also move refundableAmountOf

We can't do that for reasons I will explain in my PR (related to my status enum updates).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants