-
Notifications
You must be signed in to change notification settings - Fork 109
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
Fix & complete contract ABIs, export Typechain contract types and factories #82
Conversation
Yeah, I really like this idea. |
Sounds great. I also noticed this problem when adding new known contracts. I ended up adding the "raw" ABI here: https://github.com/gnosis/zodiac/tree/master/src/abi and importing them https://github.com/gnosis/zodiac/blob/10b6ca9c450143736395b1ff2e0b769a9e669f8e/src/factory/constants.ts#L274-L276. |
BREAKING CHANGES: renaming public exports - `SUPPORTED_NETWORKS` -> `SupportedNetworks` - `CONTRACT_ADDRESSES` -> `ContractAddresses` - `CONTRACT_ABIS` -> `ContractAbis` - remove interface formerly exported as `ContractAddresses`
b47e10c
to
5d2931d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
There is only one thing; the old factory
file (src/factory/factory.ts
) is added back in. The functionality that was previously in that file is now improved/updated and is in the moduleDeployer
file (src/factory/moduleDeployer.ts
). Perhaps we can just remove the factory
file (src/factory/factory.ts
) again?
Ah, great catch! I've fixed it now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds the correct ABIs for all zodiac module contracts. It also sets up Typechain so the
@gnosis.pm/zodiac
will now export interfaces and factories for all our contracts. This should make this package more useful when building apps on top of our contracts.While doing this wholesale review I couldn't resist clearing out some inconsistencies in the names of some of the existing exports:
ContractAddresses
(which is not used anywhere afaict)SUPPORTED_NETWORKS
->SupportedNetworks
CONTRACT_ADDRESSES
->ContractAddresses
CONTRACT_ABIS
->ContractAbis
All of these are BREAKING CHANGES meaning we must publish this as
v2.0.0
.