-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add libraries for the custom errors, data types, events and validations, optimize pro contract, rename testing contracts #140
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feat: inherit "ERC721" in "SablierV2Linear" feat: inherit "ERC721" in "SablierV2Pro" feat: mint NFT in the "_create" function feat: add "isApprovedOrOwner" function feat: change the modifier to "isAuthorizedForStream" feat: add "tokenURI" function perf: remove the recipient address from the "Stream" struct refactor: swap the order of the checks in the "withdrawTo" function test: change the struct recipient with the "users.recipient" test: make recipient the default caller in the "cancel" function test: order correctly the branches in the "cancel" tree test: order correctly the branches in the "cancelAll" tree test: order correctly the branches in the "withdraw" tree test: when the caller is an approved third party in the "cancel" function test: when the caller is an approved third party in the "cancelAll" funtion test: when the caller is an approved third party in the "withdraw" function test: when the caller is an approved third party in the "withdrawAll" function test: when the caller is an approved third party in the "withdrawAllTo" function test: when the caller is an approved third party in the "withdtawTo" function test: when the recipient is no longer the owner of the stream in the "cancel" function test: when the recipient is no longer the owner of the stream in the "withdraw" function test: when the recipient is no longer the owner of the streams in the "cancelAll" function test: when the recipient is no longer the owner of the streams in the "withdrawAll" function test: when the recipient is no longer the owner of the streams in the "withdrawAllTo" function test: when the recipient is no longer the owner of the stream in the "withrawTo" function test: remove the local "withdrawAmount" variable from the test functions test: change the new owner of the streams to "eve" when using "safeTransferFrom" function test: "isApprovedOrOwner" function in the "SablierV2Linear" contract test: "isApprovedOrOwner" function in the "SablierV2Pro" contract test: correct comment about return value for the "isCancelable" function
perf: add type conversions in the "getWithdrawableAmount" function test: add "assert" function for the "uint64" type test: add "createDynamicArray" for the "uint64" type
feat: add an "Events" library refactor: add a "check" function for the "amount" argument from withdraw refactor: move custom errors from the interfaces to the library refactor: move events from the interfaces to the library
feat: add a validation function for the linear "create" function feat: add a validation function for the pro "create" function refactor: move check functions from the core contracts to the library
refactor: move the "Stream" structs from the core contracts to the library chore: rename the "Stream" structs test: use the "DataTypes" library structs
perf: calculate "depositAmount" in pro using a for loop test: use the constant variable instead of the struct member
style: separate the libraries and interfaces imports
test: correct comment
refactor: change the "_checkBasicCreateArguments" function name refactor: remove unnecessary check in the "_checkCommonCreateArguments" function test: correct revert errors
test: delete the unit folder
chore: correct explanatory comments in the "Validations" library
As discussed privately with @PaulRBerg, I am going to close this PR in favor of multiple PRs with less changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #131 and #132 being merged first
Gas changes
Slightly increase in the
create
function (it's worth it)In the linear contract: 54 gas increase
In the pro contract: 63 gas increase
Before
From this commit: 6e49504


After
From this commit: c4fe032


Just a bit more in the
withdraw
function: 298 increase (I find it useful but I have no problem giving up onValidations.withdrawAmount
function)Before
From this commit: 700bef7

After
From this commit: c4fe032

Stream
structs from the interfaces to theDataTypes
library (no gas changes)depositAmount
from the Pro struct, solves this issue Remove thedepositAmount
variable from the Pro Stream struct #137The libraries I created intend to make our interfaces look cleaner, containing only the functions the linear and pro contracts need.
You can test the gas changes yourself. To make it easier to navigate through I created different branches for each change: