Skip to content

Commit 0998cb5

Browse files
committed
Upgrade OpenZeppelin to the latest 4.x version
The project is on OpenZeppelin 4 and the latest version currently available is 4.9.6. This change will also allow us to use Ownable2StepUpgradeable that was not available in OpenZeppelin 4.6. The upgrade required increasing the version of @openzeppelin/hardhat-upgrades plugin given the bug in the previously used 1.20.0 version not resolving the correct version of dependencies used (OZ contracts vs contracts-upgradeable) and in our case complaining about the delegatecall used in the OZ's AddressUpgradeable code. The bug was fixed in 1.20.4 version of the plugin. This also required updates in the deployment tests as some functions are no longer available in the upgraded version. https://forum.openzeppelin.com/t/interacting-with-uups-upgradeable-contracts-in-test-throwing-contract-is-not-upgrade-safe-use-of-delegatecall-is-not-allowed/32743/5
1 parent 6188812 commit 0998cb5

File tree

3 files changed

+416
-81
lines changed

3 files changed

+416
-81
lines changed

solidity/ecdsa/package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"@nomiclabs/hardhat-ethers": "^2.0.6",
4141
"@nomiclabs/hardhat-etherscan": "^3.1.0",
4242
"@nomiclabs/hardhat-waffle": "^2.0.2",
43-
"@openzeppelin/hardhat-upgrades": "^1.20.0",
43+
"@openzeppelin/hardhat-upgrades": "^1.20.4",
4444
"@tenderly/hardhat-tenderly": ">=1.0.13 <1.2.0",
4545
"@thesis-co/eslint-config": "github:thesis/eslint-config",
4646
"@typechain/ethers-v5": "^8.0.5",
@@ -72,8 +72,8 @@
7272
"dependencies": {
7373
"@keep-network/random-beacon": "development",
7474
"@keep-network/sortition-pools": "^2.0.0-pre.16",
75-
"@openzeppelin/contracts": "^4.6.0",
76-
"@openzeppelin/contracts-upgradeable": "^4.6.0",
75+
"@openzeppelin/contracts": "^4.9.6",
76+
"@openzeppelin/contracts-upgradeable": "^4.9.6",
7777
"@threshold-network/solidity-contracts": "development"
7878
},
7979
"engines": {

solidity/ecdsa/test/WalletRegistry.Deployment.test.ts

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,8 @@ describe("WalletRegistry - Deployment", async () => {
5353
})
5454

5555
it("should set WalletRegistry proxy admin", async () => {
56-
// To let a non-proxy-admin read the admin we have to read it directly from
57-
// the storage slot, see: https://docs.openzeppelin.com/contracts/4.x/api/proxy#TransparentUpgradeableProxy-admin--
5856
expect(
59-
ethers.utils.defaultAbiCoder.decode(
60-
["address"],
61-
await ethers.provider.getStorageAt(
62-
walletRegistry.address,
63-
"0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103"
64-
)
65-
)[0],
66-
"invalid WalletRegistry proxy admin (read from storage slot)"
67-
).to.be.equal(proxyAdmin.address)
68-
69-
expect(
70-
await walletRegistryProxy.connect(proxyAdmin.address).callStatic.admin(),
57+
await upgrades.erc1967.getAdminAddress(walletRegistry.address),
7158
"invalid WalletRegistry proxy admin"
7259
).to.be.equal(proxyAdmin.address)
7360
})
@@ -79,23 +66,8 @@ describe("WalletRegistry - Deployment", async () => {
7966
})
8067

8168
it("should set WalletRegistry implementation", async () => {
82-
// To let a non-proxy-admin read the implementation we have to read it directly from
83-
// the storage slot, see: https://docs.openzeppelin.com/contracts/4.x/api/proxy#TransparentUpgradeableProxy-implementation--
8469
expect(
85-
ethers.utils.defaultAbiCoder.decode(
86-
["address"],
87-
await ethers.provider.getStorageAt(
88-
walletRegistry.address,
89-
"0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc"
90-
)
91-
)[0],
92-
"invalid WalletRegistry implementation (read from storage slot)"
93-
).to.be.equal(walletRegistryImplementationAddress)
94-
95-
expect(
96-
await walletRegistryProxy
97-
.connect(proxyAdmin.address)
98-
.callStatic.implementation(),
70+
await upgrades.erc1967.getImplementationAddress(walletRegistry.address),
9971
"invalid WalletRegistry implementation"
10072
).to.be.equal(walletRegistryImplementationAddress)
10173
})
@@ -107,15 +79,6 @@ describe("WalletRegistry - Deployment", async () => {
10779
).to.be.equal(walletRegistryImplementationAddress)
10880
})
10981

110-
it("should set implementation address different than proxy address", async () => {
111-
expect(
112-
await walletRegistryProxy
113-
.connect(proxyAdmin.address)
114-
.callStatic.implementation(),
115-
"invalid ProxyAdmin owner"
116-
).to.be.not.equal(walletRegistry.address)
117-
})
118-
11982
it("should set WalletRegistry governance", async () => {
12083
expect(
12184
await walletRegistry.governance(),

0 commit comments

Comments
 (0)