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

Check for non-zero admin address when importing transparent proxy #887

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

ericglau
Copy link
Member

@ericglau ericglau commented Sep 27, 2023

Fixes #882

When importing a proxy, the proxy kind can be inferred based on the presence of upgrade function signatures in the implementation.

For transparent proxies, we import the admin address using the admin address slot. During this import, we check whether the admin address has code.

If the proxy kind was inferred incorrectly, or if the user provided the wrong kind option (e.g. passed in 'transparent' but the proxy is actually UUPS), the admin address is the zero address. This causes the error when we check for code at the admin address:

No contract at address 0x0000000000000000000000000000000000000000 (Removed from manifest)

Instead, we should throw an error if the admin address is zero, and tell the user to set the correct proxy kind.

NOTE: This assumes that a zero admin address is never a valid scenario for a transparent proxy.

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

My 2 wei

packages/plugin-hardhat/src/force-import.ts Outdated Show resolved Hide resolved
packages/plugin-hardhat/src/force-import.ts Show resolved Hide resolved
Co-authored-by: Ernesto García <ernestognw@gmail.com>
@ericglau ericglau enabled auto-merge (squash) September 28, 2023 02:29
@ericglau ericglau merged commit ba541bf into OpenZeppelin:master Sep 28, 2023
@ericglau ericglau deleted the adminaddress branch September 28, 2023 02:48
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.

With contracts-upgradeable v5.0.0-rc.0 adminAddress is zero
2 participants