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.

Sign up for GitHub

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

Jump to bottom

Update OZ lib to 4.8.2 #711

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Update OZ lib to 4.8.2 #711

merged 1 commit into from
Mar 30, 2023

Conversation

grandizzy
Copy link
Contributor

@grandizzy grandizzy commented Mar 28, 2023 •
edited
Loading

Contract size

## Pre Change
============ Deployment Bytecode Sizes ============
  ERC721Pool               -  24,356B  (99.10%)
  ERC20Pool                -  23,802B  (96.85%)
  Auctions                 -  21,067B  (85.72%)

Post Change

============ Deployment Bytecode Sizes ============
  ERC721Pool               -  24,356B  (99.10%)
  ERC20Pool                -  23,802B  (96.85%)
  Auctions                 -  21,125B  (85.95%)

Gas usage

Pre Change

| Function Name                        | min             | avg    | median | max    | # calls |
| addQuoteToken                        | 119315          | 175668 | 161115 | 694632 | 60004   |
| bucketTake                           | 327628          | 348558 | 348570 | 369464 | 4       |
| drawDebt                             | 248277          | 292957 | 268224 | 785846 | 136003  |
| kick                                 | 195615          | 221635 | 220800 | 986346 | 48000   |
| kickWithDeposit                      | 240830          | 275372 | 270174 | 983400 | 8000    |
| moveQuoteToken                       | 266942          | 266942 | 266942 | 266942 | 1       |
| removeQuoteToken                     | 133868          | 155338 | 158301 | 180802 | 4000    |
| repayDebt                            | 556944          | 589087 | 578983 | 737254 | 16002   |
| settle                               | 212455          | 212455 | 212455 | 212455 | 1       |
| take                                 | 97715           | 102846 | 102502 | 315779 | 15998   |

Post Change

| Function Name                        | min             | avg    | median | max    | # calls |
| addQuoteToken                        | 119315          | 175668 | 161115 | 694632 | 60004   |
| bucketTake                           | 327628          | 348558 | 348570 | 369464 | 4       |
| drawDebt                             | 248277          | 292957 | 268224 | 785846 | 136003  |
| kick                                 | 195615          | 221635 | 220800 | 986346 | 48000   |
| kickWithDeposit                      | 240830          | 275372 | 270174 | 983400 | 8000    |
| moveQuoteToken                       | 266942          | 266942 | 266942 | 266942 | 1       |
| removeQuoteToken                     | 133868          | 155338 | 158301 | 180802 | 4000    |
| repayDebt                            | 556944          | 589087 | 578983 | 737254 | 16002   |
| settle                               | 212420          | 212420 | 212420 | 212420 | 1       |
| take                                 | 97715           | 102846 | 102502 | 315779 | 15998   |

EdNoepel
EdNoepel reviewed Mar 28, 2023
View reviewed changes
Copy link
Contributor

@EdNoepel EdNoepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing PR template. Useful to see how the library change impacts contract size and gas consumption.

@grandizzy grandizzy changed the title Update oz lib to 4.8.2 Update OZ lib to 4.8.2 Mar 28, 2023
@grandizzy
Copy link
Contributor Author

Missing PR template. Useful to see how the library change impacts contract size and gas consumption.

makes sense, updated

prateek105
prateek105 approved these changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@prateek105 prateek105 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. There are gas optimizations in ERC20Votes which is inherited in Ajna token contract on ecosystem coordination repo. If we are going to redeploy Ajna token this upgrade should be made on ecosystem repo as well. @MikeHathaway

MikeHathaway
MikeHathaway approved these changes Mar 29, 2023
View reviewed changes
Copy link
Collaborator

@MikeHathaway MikeHathaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@grandizzy grandizzy merged commit 0889a4c into develop Mar 30, 2023
@grandizzy grandizzy deleted the update-oz-lib-4.8.2 branch March 30, 2023 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EdNoepel EdNoepel EdNoepel left review comments

@MikeHathaway MikeHathaway MikeHathaway approved these changes

@prateek105 prateek105 prateek105 approved these changes

@ith-harvey ith-harvey Awaiting requested review from ith-harvey

Assignees
No one assigned
Labels
Priority: High Status: Ready for Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@grandizzy @prateek105 @MikeHathaway @EdNoepel