Skip to content
This repository has been archived by the owner on Apr 27, 2022. It is now read-only.

Make Authentication Contract Upgradable #167

Closed
bh2smith opened this issue Nov 20, 2020 · 1 comment · Fixed by #351
Closed

Make Authentication Contract Upgradable #167

bh2smith opened this issue Nov 20, 2020 · 1 comment · Fixed by #351
Milestone

Comments

@bh2smith
Copy link
Contributor

The GPv2Settlement contract points to GPv2Authenticator which should be made upgradable via a proxy pattern.

@bh2smith bh2smith mentioned this issue Nov 20, 2020
2 tasks
@nlordell nlordell added this to the MVP milestone Nov 23, 2020
@bh2smith
Copy link
Contributor Author

bh2smith commented Jan 4, 2021

Lucky for us, it looks like OpenZeppelin has already migrated their contract upgradability SDK to be compatible with hardhat!

https://github.com/OpenZeppelin/openzeppelin-upgrades

@mergify mergify bot closed this as completed in #351 Feb 3, 2021
mergify bot pushed a commit that referenced this issue Feb 3, 2021
Closes #167 

This PR introduces minor changes to the `AllowListAuthenticator` making it upgradable and includes tests verifying upgradability. In particular:

- removing `customInitiallyOwnable` as it is no longer used: Not yet sure how this will affect the `deployments` directory which still includes it.
- Updating authenticator deployment script to deploy as proxy.
- rename authenticator `owner` to `manager` because of name collision with proxy owner
- introduce proxy.ts with helper methods to fetch implementation and owner address.
- Include two unit tests: one verifying that upgrade is possible and the other to ensure preservation of storage.

Note that the unit tests involving Authenticator's non-upgrade related functionality do not use the proxy deployment as specified in the migration scripts, so that manager must be set immediately after deployment.

We had originally planned/hoped to use the `hardhat-upgrades` plugin and opened the following two PRs to `openzeppelin-upgrades`, but this plugin turned out to be unnecessary.

- Custom Deployments: OpenZeppelin/openzeppelin-upgrades#273
- Manual Safety Override: OpenZeppelin/openzeppelin-upgrades#280

### Test Plan

Old + new unit and e2e tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants