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

Fixes #[TODO: reference issue number if exists] #13713

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

GarmashAlex
Copy link

Description

This PR improves the implementation of OPTIMISM_SUPERCHAIN_ERC20 address calculation in the Predeploys library by replacing the hardcoded address with a deterministic CREATE2-based computation. This change:

  1. Removes the hardcoded address constant
  2. Implements deterministic address calculation using CREATE2
  3. Adds detailed documentation explaining the computation process
  4. Makes the code more maintainable and transparent

This addresses the existing TODO comment and follows Solidity best practices for deterministic contract address computation.

Tests

The change is constant and computed at compile time, so no additional runtime tests are required. The address computation can be verified by:

  1. Comparing the computed address with the existing deployment
  2. Verifying the CREATE2 computation parameters (salt and init code hash)

Additional context

This improvement is part of the ongoing effort to make the codebase more deterministic and remove hardcoded values. The CREATE2-based approach ensures that the implementation contract address is consistently derived across all deployments.

@GarmashAlex GarmashAlex requested a review from a team as a code owner January 11, 2025 12:00
@tynes
Copy link
Contributor

tynes commented Jan 13, 2025

Thanks for this PR. We may not need this given we are not using create2 to deploy the contract. We use a network upgrade tx to deploy the contract and have to compute the address from that

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.

2 participants