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

Contract size improvements #1046

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

prateek105
Copy link
Contributor

@prateek105 prateek105 commented Dec 20, 2023

Description

  • Remove the _determineInflatorState method and add the method logic into the Pool contract.
  • Remove the _getCollateralDustPricePrecisionAdjustment method and add the method logic into the ERC20Pool contract.
  • Update the getExchangeRate method visibility in the Buckets library to external as that was only called inside the bucketExchangeRate method in the Pool contract which is a view method.

Purpose

  • PR aims to reduce contract size to accommodate contract changes that may occur in the future.

Impact

  • ERC721 contract size reduced from 24,540B (99.85%) to 24,329B (98.99%).
  • ERC20 contract size reduced from 23,597B (96.01%) to 23,375B (95.11%).

Tasks

  • Changes to protocol contracts are covered by unit tests executed by CI.
  • Protocol contract size limits have not been exceeded.
  • Gas consumption for impacted transactions have been compared with the target branch, and nontrivial changes cited in the Impact section above.
  • Scope labels have been assigned as appropriate.
  • Invariant tests have been manually executed as appropriate for the nature of the change.

@@ -250,7 +250,7 @@ library Buckets {
uint256 bucketLP_,
uint256 bucketDeposit_,
uint256 bucketPrice_
) internal pure returns (uint256) {
) external pure returns (uint256) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating getExchangeRate method visibility to external created inconsistency in Buckets Library but is only called in view method in Pool contract.

@ith-harvey
Copy link
Contributor

Even though this really just looks like simple copy paste of moving different parts of the program around I am reluctant to make this change as close as we are to deployment.

If we had another 4 days of invariant runs ahead of us before deploying to mainnet I would say fine but I think its too big a risk to make src changes like this without the invariant testing to back it up.

I'm of the opinion that we leave it in it's PR form now then if we ever need the extra contract space we can add it.

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

Successfully merging this pull request may close these issues.

2 participants