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

Remove isCanceled and isStream from Stream struct #19

Closed
wants to merge 1 commit into from

Conversation

smol-ninja
Copy link
Member

This PR removes isCanceled and isStream fields from the Stream struct. Functions isCanceled(uint256 streamId) and function isStream(uint256 streamId) are modified as the following:

function isCanceled(uint256 streamId) public view override notNull(streamId) returns (bool result) {
    result = _streams[streamId].ratePerSecond == 0 && _streams[streamId].sender != address(0);
}

function isStream(uint256 streamId) public view returns (bool result) {
    result = _streams[streamId].sender != address(0);
}

Results

Function name Max (previous) Max (new) Gas efficiency
adjustRatePerSecond 21008 19022 9.45%
cancel 73332 68310 6.85%
create 105213 104501 0.68%
createAndDeposit 105012 104443 0.54%
deposit 49295 49309 -0.03%
refundFromStream 29469 29483 -0.05%
restartStream 34788 32441 6.75%
restartStreamAndDeposit 37156 35919 3.33%
withdraw 49950 47964 3.98%

@andreivladbrg
Copy link
Member

andreivladbrg commented Apr 9, 2024

Thank you @smol-ninja for the PR.

I consider relying on specific boolean variables, that are toggled only upon certain actions, to be a safer method, despite the gas implications.

Also, the current design is similar to the one in lockup contracts - it is a good thing to have similar design patterns

Are you ok with closing this PR?

@smol-ninja
Copy link
Member Author

it is a good thing to have similar design patterns

Agree on this point. Closing this.

@smol-ninja smol-ninja closed this Apr 10, 2024
@smol-ninja smol-ninja deleted the feat/optimizing-struct branch April 10, 2024 01:03
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

Successfully merging this pull request may close these issues.

3 participants