Skip to content

refactor: splits the gas limit used in the RevenueShareUgradePath#28

Merged
0xDiscotech merged 3 commits into
fix/l2-target-proxyadminfrom
refactor/split-gaslimit-revshare
Oct 9, 2025
Merged

refactor: splits the gas limit used in the RevenueShareUgradePath#28
0xDiscotech merged 3 commits into
fix/l2-target-proxyadminfrom
refactor/split-gaslimit-revshare

Conversation

@0xiamflux
Copy link
Copy Markdown

Refactor: Split gas limits in RevenueShareUpgradePath template

Previously used a single deploymentGasLimit for both contract deployments and upgrades. This refactor introduces separate gas limits for better control:

  • deploymentGasLimit (1.1M gas) - for deploying new contracts
  • upgradeGasLimit (500k gas) - for upgrade operations

Changes:

  • Updated RevenueShareUpgradePath.sol to use separate gas limit variables
  • Modified config example
  • Updated test expectations

@0xiamflux 0xiamflux self-assigned this Oct 8, 2025
uint64 internal constant FEE_VAULTS_DEPLOYMENT_GAS_LIMIT = 910_000;
/// @notice The gas limit for the Fee Splitter deployment.
uint64 internal constant FEE_SPLITTER_DEPLOYMENT_GAS_LIMIT = 1_235_000;
/// @notice The gas limit for the Fee Splitter upgrade.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this for all upgrades rather tan only the fee splitter one?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch! Addressed here.

Copy link
Copy Markdown

@0xDiscotech 0xDiscotech left a comment

Choose a reason for hiding this comment

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

It looks good so far! Will approve once is synced with the branch and the unit tests are updated based on these changes

address internal constant PROXY_ADMIN = 0x4200000000000000000000000000000000000018;

/// @notice Based on deployment tests, these are the average gas costs for each of the deployments:
/// - L1 Withdrawer: TBD
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/// @notice The gas limit for the Fee Splitter deployment.
uint64 internal constant FEE_SPLITTER_DEPLOYMENT_GAS_LIMIT = 1_235_000;
/// @notice The gas limit for the upgrade calls on L2.
uint64 internal constant UPGRADE_GAS_LIMIT = 500_000;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TODO: Update this number when we have more precise information, probably can be the half or less. https://linear.app/defi-wonderland/issue/OPT-1210/update-l1withdrawer-deployment-cost-when-we-know-the-value

@0xDiscotech 0xDiscotech merged commit 551f13d into fix/l2-target-proxyadmin Oct 9, 2025
1 check passed
@0xDiscotech 0xDiscotech deleted the refactor/split-gaslimit-revshare branch October 9, 2025 16:11
@0xDiscotech
Copy link
Copy Markdown

It looks good so far! Will approve once is synced with the branch and the unit tests are updated based on these changes

Will be finally addressed here #27

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