Skip to content

Conversation

@Amxx
Copy link
Contributor

@Amxx Amxx commented Mar 17, 2021

Changelog

Core - Internal

  • Add a fetchOrDeployProxy that stored new proxies address + type in the manifest
  • Update layout to version 3.2: add a list of proxy deployments (address + txHash + kind) to the manifest
  • Add a mechanism to extract signatures out of FunctionDefinition (from the AST)
  • Add a new error type 'no-public-upgrade-fn'
  • Add a list of all public and external function signatures to the validation objects
  • Add check for the presence of an upgradeTo(address) function
    • this test is disabled using unsafeAllow: ['no-public-upgrade-fn'] which is automatically added by plugins when operating on a transparent proxy

Plugin - Public

  • Add a new field to the DeploymentOptions interface kind which allows users to specify the proxy flavor they want to use. Accepted values are auto (default), uups and transparent.
    • When deploying a new contract, auto fallback to transparent for backward compatibility
    • when upgrading a contract, auto will autodetect the proxy type from the manifest and apply the corresponding checks
  • Add warning message when:
    • Deploying a UUPS proxy (that doesn't uses the proxyadmin pattern) while a proxyadmin instance is registered in the manifest.
    • Some proxies are not affected by a proxyAdmin being transfered

Plugin - Internal

  • Detect the presence of a proxyAdmin when doing upgrades. This adds support to transparent proxy that are not owned through a proxyAdmin.

@Amxx Amxx force-pushed the feature/uup-support-#3 branch from 7424e41 to f2a5b12 Compare March 17, 2021 18:13
@Amxx Amxx force-pushed the feature/uup-support-#3 branch from f2a5b12 to 8e56c18 Compare March 17, 2021 18:20
@Amxx Amxx force-pushed the feature/uup-support-#3 branch from 97e29bf to 5bdad43 Compare March 24, 2021 17:20
@Amxx Amxx marked this pull request as ready for review March 24, 2021 17:25
@Amxx Amxx force-pushed the feature/uup-support-#3 branch from 49e28f7 to 8dfeab8 Compare March 24, 2021 17:46
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thank you!

Started reviewing the code. Leaving a couple of initial comments, will continue tomorrow.

Copy link
Contributor Author

@Amxx Amxx left a comment

Choose a reason for hiding this comment

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

This used to be the case, but caused issues during manual tests. I'll have to check that again.

Details: cache of old implementation doesn't contain this field, which caused issues.

@Amxx
Copy link
Contributor Author

Amxx commented Apr 19, 2021

Manual tests look good

@frangio
Copy link
Contributor

frangio commented Apr 19, 2021

cache of old implementation doesn't contain this field

This is solved now because I bumped the validation data version so the cache will be flushed.

@frangio frangio changed the title Add UUPS support (WIP) Add UUPS support Apr 20, 2021
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

I've finished reviewing everything. It looks great.

We need to think about how we want to document this. We can do it in a separate PR.

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.

4 participants