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

Add libraries for the custom errors, data types, events and validations, optimize pro contract, rename testing contracts #140

Closed
wants to merge 23 commits into from

Conversation

andreivladbrg
Copy link
Member

@andreivladbrg andreivladbrg commented Oct 22, 2022

Depends on #131 and #132 being merged first

  • Move the custom errors and events from the interfaces to its unique library (no gas changes)
  • Move the function requirements valaditions from the linear and pro contracts to a unique library
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
Screen Shot 2022-10-22 at 16 02 50
Screen Shot 2022-10-22 at 16 02 42

After

From this commit: c4fe032
Screen Shot 2022-10-22 at 15 58 09
Screen Shot 2022-10-22 at 15 58 15

Just a bit more in the withdraw function: 298 increase (I find it useful but I have no problem giving up on Validations.withdrawAmount function)

Before

From this commit: 700bef7
Screen Shot 2022-10-22 at 16 14 25

After

From this commit: c4fe032
Screen Shot 2022-10-22 at 16 02 59

The 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:

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
refactor: change the "_checkBasicCreateArguments" function name
refactor: remove unnecessary check in the "_checkCommonCreateArguments"
function
test: correct revert errors
chore: correct explanatory comments in the "Validations" library
@andreivladbrg
Copy link
Member Author

As discussed privately with @PaulRBerg, I am going to close this PR in favor of multiple PRs with less changes.

@andreivladbrg andreivladbrg deleted the andrei/refactor-rename-contracts branch November 13, 2022 10:20
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.

1 participant