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

Support TypeChain in deploy and upgrade proxy functions - continuation of work done on #535 #1099

Conversation

robertmagier
Copy link
Contributor

@robertmagier robertmagier commented Nov 11, 2024

Continuation of work done on #535 by frimoldi.
See #325 and #1082.

Returns type safe contract types with: deployProxy, deployContract, deployBeaconProxy, and upgradeProxy functions.

This does not apply to deployBeacon, upgradeBeacon, and forceImport since those can return UpgradeableBeacon rather than the implementation contract type.

…onProxy, deployBeacon, upgradeProxy, upgradeBeacon, and forceImport functions
@robertmagier robertmagier force-pushed the feat/deploy-upgrade-proxy-contract-return-type branch from 1e452b5 to 837272c Compare November 11, 2024 22:01
Copy link
Member

@ericglau ericglau left a comment

Choose a reason for hiding this comment

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

Thanks a lot for picking this up and sorry for the late response. This looks great!

I made a few changes as follows:

  • deployBeacon and upgradeBeacon should return an UpgradeableBeacon instance, rather than the implementation contract's type. We don't generate detailed TypeChain information for UpgradeableBeacon for now, so I've reverted those to just return Contract.
  • forceImport could also return an UpgradeableBeacon if the given address is a beacon. For simplicity, I've reverted that as well so it just returns Contract.
  • changed the type definition of ContractTypeOfFactory to be ReturnType<F['attach']> & ReturnType<F['deploy']> since with Ethers v6, attach just returns a BaseContract which does not have the contract's type. But TypeChain provides an override of deploy which gives us the expected type -- this seems to work.

For reference, since our tests in this project uses JS, I created a sample TS project and tested it with these testcases: https://github.com/ericglau/hardhat-upgrades-typechain-tests/blob/main/test/test.ts

@ericglau ericglau merged commit 451d217 into OpenZeppelin:master Dec 20, 2024
11 checks passed
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