From b71c9cf992cd875ce041e11a99d155e8f4591b2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renaud?= Date: Tue, 21 May 2024 19:03:06 +0200 Subject: [PATCH] Oz upgrade to v5 (#13874) --- smart-contracts/contracts/CardPurchaser.sol | 3 +-- .../contracts/hooks/CaptchaHook.sol | 5 +++-- .../contracts/hooks/DiscountCodeHook.sol | 3 ++- .../contracts/hooks/GitcoinHook.sol | 5 +++-- smart-contracts/contracts/hooks/GuildHook.sol | 5 +++-- .../contracts/hooks/PasswordRequiredHook.sol | 3 ++- .../contracts/test-artifacts/TestERC1155.sol | 1 - .../contracts/test-artifacts/TestERC721.sol | 8 +++---- .../test-artifacts/TestProxyAdmin.sol | 4 +++- .../UDT}/UnlockDiscountTokenV3.sol | 0 .../UDT}/UnlockProtocolGovernor.sol | 0 .../UDT}/UnlockProtocolTimelock.sol | 0 smart-contracts/package.json | 4 +++- .../test/Lock/hooks/CaptchaHook.js | 2 +- .../test/Lock/hooks/GitcoinHook.js | 5 +++-- smart-contracts/test/Lock/hooks/GuildHook.js | 4 ++-- .../test/Lock/renewMembershipFor.js | 4 ++-- yarn.lock | 22 +++++++++++++++++-- 18 files changed, 51 insertions(+), 27 deletions(-) rename smart-contracts/contracts/{ => tokens/UDT}/UnlockDiscountTokenV3.sol (100%) rename smart-contracts/contracts/{ => tokens/UDT}/UnlockProtocolGovernor.sol (100%) rename smart-contracts/contracts/{ => tokens/UDT}/UnlockProtocolTimelock.sol (100%) diff --git a/smart-contracts/contracts/CardPurchaser.sol b/smart-contracts/contracts/CardPurchaser.sol index e352fbaa09c..f0acc6715a9 100644 --- a/smart-contracts/contracts/CardPurchaser.sol +++ b/smart-contracts/contracts/CardPurchaser.sol @@ -54,12 +54,11 @@ contract CardPurchaser is Ownable, EIP712 { address _owner, address _unlockAddress, address _usdc - ) EIP712("Card Purchaser", "1") Ownable() { + ) EIP712("Card Purchaser", "1") Ownable(_owner) { name = "Card Purchaser"; version = "1"; unlockAddress = _unlockAddress; usdc = _usdc; - transferOwnership(_owner); } /** diff --git a/smart-contracts/contracts/hooks/CaptchaHook.sol b/smart-contracts/contracts/hooks/CaptchaHook.sol index ce52eb63557..7911b87740a 100644 --- a/smart-contracts/contracts/hooks/CaptchaHook.sol +++ b/smart-contracts/contracts/hooks/CaptchaHook.sol @@ -1,6 +1,7 @@ //SPDX-License-Identifier: Unlicense pragma solidity ^0.8.0; +import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@unlock-protocol/contracts/dist/PublicLock/IPublicLockV12.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; @@ -8,7 +9,7 @@ import "@openzeppelin/contracts/access/Ownable.sol"; contract CaptchaHook is Ownable { mapping(address => bool) public signers; - constructor() {} + constructor() Ownable(msg.sender) {} function addSigner(address signer) public onlyOwner { signers[signer] = true; @@ -44,7 +45,7 @@ contract CaptchaHook is Ownable { ) public view returns (bool isSigner) { bytes memory encoded = abi.encodePacked(message); bytes32 messageHash = keccak256(encoded); - bytes32 hash = ECDSA.toEthSignedMessageHash(messageHash); + bytes32 hash = MessageHashUtils.toEthSignedMessageHash(messageHash); address recoveredAddress = ECDSA.recover(hash, signature); return signers[recoveredAddress]; } diff --git a/smart-contracts/contracts/hooks/DiscountCodeHook.sol b/smart-contracts/contracts/hooks/DiscountCodeHook.sol index 4e1ca1939b6..045bdbc4acf 100644 --- a/smart-contracts/contracts/hooks/DiscountCodeHook.sol +++ b/smart-contracts/contracts/hooks/DiscountCodeHook.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.9; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@unlock-protocol/contracts/dist/PublicLock/IPublicLockV12.sol"; +import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; error TOO_BIG(); error NOT_AUTHORIZED(); @@ -86,7 +87,7 @@ contract DiscountHook { bytes calldata signature ) public pure returns (address recoveredAddress) { bytes32 hash = keccak256(abi.encodePacked(message)); - bytes32 signedMessageHash = ECDSA.toEthSignedMessageHash(hash); + bytes32 signedMessageHash = MessageHashUtils.toEthSignedMessageHash(hash); return ECDSA.recover(signedMessageHash, signature); } diff --git a/smart-contracts/contracts/hooks/GitcoinHook.sol b/smart-contracts/contracts/hooks/GitcoinHook.sol index daec0ed1242..b7f0be80f39 100644 --- a/smart-contracts/contracts/hooks/GitcoinHook.sol +++ b/smart-contracts/contracts/hooks/GitcoinHook.sol @@ -1,13 +1,14 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import "@unlock-protocol/contracts/dist/PublicLock/IPublicLockV13.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; contract GitcoinHook is Ownable { mapping(address => bool) public signers; - constructor() {} + constructor() Ownable(msg.sender) {} function addSigner(address signer) public onlyOwner { signers[signer] = true; @@ -43,7 +44,7 @@ contract GitcoinHook is Ownable { ) private view returns (bool isSigner) { bytes memory encoded = abi.encodePacked(message); bytes32 messageHash = keccak256(encoded); - bytes32 hash = ECDSA.toEthSignedMessageHash(messageHash); + bytes32 hash = MessageHashUtils.toEthSignedMessageHash(messageHash); address recoveredAddress = ECDSA.recover(hash, signature); return signers[recoveredAddress]; } diff --git a/smart-contracts/contracts/hooks/GuildHook.sol b/smart-contracts/contracts/hooks/GuildHook.sol index 389e4b7d95e..45def4c5cbb 100644 --- a/smart-contracts/contracts/hooks/GuildHook.sol +++ b/smart-contracts/contracts/hooks/GuildHook.sol @@ -1,13 +1,14 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import "@unlock-protocol/contracts/dist/PublicLock/IPublicLockV13.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; contract GuildHook is Ownable { mapping(address => bool) public signers; - constructor() {} + constructor() Ownable(msg.sender) {} function addSigner(address signer) public onlyOwner { signers[signer] = true; @@ -44,7 +45,7 @@ contract GuildHook is Ownable { ) private view returns (bool isSigner) { bytes memory encoded = abi.encodePacked(message); bytes32 messageHash = keccak256(encoded); - bytes32 hash = ECDSA.toEthSignedMessageHash(messageHash); + bytes32 hash = MessageHashUtils.toEthSignedMessageHash(messageHash); address recoveredAddress = ECDSA.recover(hash, signature); return signers[recoveredAddress]; } diff --git a/smart-contracts/contracts/hooks/PasswordRequiredHook.sol b/smart-contracts/contracts/hooks/PasswordRequiredHook.sol index bc85f0905bd..43a438723f8 100644 --- a/smart-contracts/contracts/hooks/PasswordRequiredHook.sol +++ b/smart-contracts/contracts/hooks/PasswordRequiredHook.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@unlock-protocol/contracts/dist/PublicLock/IPublicLockV12.sol"; +import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; error WRONG_PASSWORD(); error NOT_AUTHORIZED(); @@ -52,7 +53,7 @@ contract PasswordRequiredHook { bytes calldata signature ) public pure returns (address recoveredAddress) { bytes32 hash = keccak256(abi.encodePacked(message)); - bytes32 signedMessageHash = ECDSA.toEthSignedMessageHash(hash); + bytes32 signedMessageHash = MessageHashUtils.toEthSignedMessageHash(hash); return ECDSA.recover(signedMessageHash, signature); } diff --git a/smart-contracts/contracts/test-artifacts/TestERC1155.sol b/smart-contracts/contracts/test-artifacts/TestERC1155.sol index 2da8f741f40..48a59c94f91 100644 --- a/smart-contracts/contracts/test-artifacts/TestERC1155.sol +++ b/smart-contracts/contracts/test-artifacts/TestERC1155.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC1155/ERC1155.sol"; -import "@openzeppelin/contracts/utils/Counters.sol"; contract TestERC1155 is ERC1155 { constructor() ERC1155("tokenURIExample") {} diff --git a/smart-contracts/contracts/test-artifacts/TestERC721.sol b/smart-contracts/contracts/test-artifacts/TestERC721.sol index 38b608f9d65..51f0d25eb0d 100644 --- a/smart-contracts/contracts/test-artifacts/TestERC721.sol +++ b/smart-contracts/contracts/test-artifacts/TestERC721.sol @@ -2,18 +2,16 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC721/ERC721.sol"; -import "@openzeppelin/contracts/utils/Counters.sol"; contract TestERC721 is ERC721 { - using Counters for Counters.Counter; - Counters.Counter private _tokenIds; + uint private _lastTokenId = 1; constructor() ERC721("BasicToken", "BASIC") {} function mint(address holder) public returns (uint256) { - _tokenIds.increment(); + _lastTokenId++; - uint256 newItemId = _tokenIds.current(); + uint256 newItemId = _lastTokenId; _mint(holder, newItemId); return newItemId; diff --git a/smart-contracts/contracts/test-artifacts/TestProxyAdmin.sol b/smart-contracts/contracts/test-artifacts/TestProxyAdmin.sol index f98da9b7b23..6250451b367 100644 --- a/smart-contracts/contracts/test-artifacts/TestProxyAdmin.sol +++ b/smart-contracts/contracts/test-artifacts/TestProxyAdmin.sol @@ -3,4 +3,6 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; -contract TestProxyAdmin is ProxyAdmin {} +contract TestProxyAdmin is ProxyAdmin { + constructor() ProxyAdmin(msg.sender) {} +} diff --git a/smart-contracts/contracts/UnlockDiscountTokenV3.sol b/smart-contracts/contracts/tokens/UDT/UnlockDiscountTokenV3.sol similarity index 100% rename from smart-contracts/contracts/UnlockDiscountTokenV3.sol rename to smart-contracts/contracts/tokens/UDT/UnlockDiscountTokenV3.sol diff --git a/smart-contracts/contracts/UnlockProtocolGovernor.sol b/smart-contracts/contracts/tokens/UDT/UnlockProtocolGovernor.sol similarity index 100% rename from smart-contracts/contracts/UnlockProtocolGovernor.sol rename to smart-contracts/contracts/tokens/UDT/UnlockProtocolGovernor.sol diff --git a/smart-contracts/contracts/UnlockProtocolTimelock.sol b/smart-contracts/contracts/tokens/UDT/UnlockProtocolTimelock.sol similarity index 100% rename from smart-contracts/contracts/UnlockProtocolTimelock.sol rename to smart-contracts/contracts/tokens/UDT/UnlockProtocolTimelock.sol diff --git a/smart-contracts/package.json b/smart-contracts/package.json index 4446ce55e31..67fcdbe735e 100644 --- a/smart-contracts/package.json +++ b/smart-contracts/package.json @@ -12,8 +12,10 @@ "@nomicfoundation/hardhat-ethers": "3.0.5", "@nomicfoundation/hardhat-network-helpers": "1.0.10", "@nomicfoundation/hardhat-verify": "2.0.6", - "@openzeppelin/contracts": "4.9.6", + "@openzeppelin/contracts": "5.0.2", "@openzeppelin/contracts-upgradeable": "4.9.6", + "@openzeppelin/contracts-upgradeable5": "npm:@openzeppelin/contracts-upgradeable@5.0.2", + "@openzeppelin/contracts5": "npm:@openzeppelin/contracts@5.0.2", "@openzeppelin/hardhat-upgrades": "3.1.0", "@openzeppelin/upgrades-core": "1.33.1", "@safe-global/safe-core-sdk": "3.3.5", diff --git a/smart-contracts/test/Lock/hooks/CaptchaHook.js b/smart-contracts/test/Lock/hooks/CaptchaHook.js index 034fb823919..2c47515bc71 100644 --- a/smart-contracts/test/Lock/hooks/CaptchaHook.js +++ b/smart-contracts/test/Lock/hooks/CaptchaHook.js @@ -129,7 +129,7 @@ describe('CaptchaHook', function () { [await aThird.getAddress()], ['0x'] ), - 'ECDSA: invalid signature length' + 'ECDSAInvalidSignatureLength' ) }) }) diff --git a/smart-contracts/test/Lock/hooks/GitcoinHook.js b/smart-contracts/test/Lock/hooks/GitcoinHook.js index 838af8b5de2..41766371922 100644 --- a/smart-contracts/test/Lock/hooks/GitcoinHook.js +++ b/smart-contracts/test/Lock/hooks/GitcoinHook.js @@ -82,7 +82,7 @@ describe('GitcoinHook', function () { [await aThird.getAddress()], ['0x'] ), - 'ECDSA: invalid signature length' + 'ECDSAInvalidSignatureLength' ) }) @@ -96,6 +96,7 @@ describe('GitcoinHook', function () { await hook.addSigner(await signer.getAddress()) expect(await hook.signers(await signer.getAddress())).to.equal(true) expect(await hook.owner()).to.equal(await user.getAddress()) + const previousOwner = await hook.owner() // Transfer ownership await hook.transferOwnership(await anotherUser.getAddress()) @@ -105,7 +106,7 @@ describe('GitcoinHook', function () { const anotherSigner = ethers.Wallet.createRandom() await reverts( hook.addSigner(await anotherSigner.getAddress()), - 'Ownable: caller is not the owner' + `OwnableUnauthorizedAccount("${previousOwner}")` ) expect(await hook.signers(await anotherSigner.getAddress())).to.equal(false) diff --git a/smart-contracts/test/Lock/hooks/GuildHook.js b/smart-contracts/test/Lock/hooks/GuildHook.js index 73c4ace4eff..ff40e0f103f 100644 --- a/smart-contracts/test/Lock/hooks/GuildHook.js +++ b/smart-contracts/test/Lock/hooks/GuildHook.js @@ -82,7 +82,7 @@ describe('GuildHook', function () { [await aThird.getAddress()], ['0x'] ), - 'ECDSA: invalid signature length' + 'ECDSAInvalidSignatureLength' ) }) @@ -105,7 +105,7 @@ describe('GuildHook', function () { const anotherSigner = ethers.Wallet.createRandom() await reverts( hook.addSigner(await anotherSigner.getAddress()), - 'Ownable: caller is not the owner' + 'OwnableUnauthorizedAccount' ) expect(await hook.signers(await anotherSigner.getAddress())).to.equal(false) diff --git a/smart-contracts/test/Lock/renewMembershipFor.js b/smart-contracts/test/Lock/renewMembershipFor.js index 77157ac949b..82bbd6cd29d 100644 --- a/smart-contracts/test/Lock/renewMembershipFor.js +++ b/smart-contracts/test/Lock/renewMembershipFor.js @@ -221,7 +221,7 @@ describe('Lock / Recurring memberships', () => { // now reverts await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'ERC20: insufficient allowance' + 'ERC20InsufficientAllowance' ) }) @@ -250,7 +250,7 @@ describe('Lock / Recurring memberships', () => { // now funds are not enough await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'ERC20: transfer amount exceeds balance' + 'ERC20InsufficientBalance' ) }) }) diff --git a/yarn.lock b/yarn.lock index 282cd886b71..65aa89ae142 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12684,6 +12684,15 @@ __metadata: languageName: node linkType: hard +"@openzeppelin/contracts-upgradeable5@npm:@openzeppelin/contracts-upgradeable@5.0.2": + version: 5.0.2 + resolution: "@openzeppelin/contracts-upgradeable@npm:5.0.2" + peerDependencies: + "@openzeppelin/contracts": 5.0.2 + checksum: 10/71847c6bbd7a859a2f02f496215b9664e41375589010e66da32f080d9af9215accf558da63134926e0eb3eb87ee7ab952462bc877ec5c5e1ac077b44cac9c363 + languageName: node + linkType: hard + "@openzeppelin/contracts-upgradeable@npm:4.5.2": version: 4.5.2 resolution: "@openzeppelin/contracts-upgradeable@npm:4.5.2" @@ -12705,6 +12714,13 @@ __metadata: languageName: node linkType: hard +"@openzeppelin/contracts5@npm:@openzeppelin/contracts@5.0.2, @openzeppelin/contracts@npm:5.0.2": + version: 5.0.2 + resolution: "@openzeppelin/contracts@npm:5.0.2" + checksum: 10/938ebffbdade7dc59ea3df5b562c0e457bbefde9d82be8fa2acfd11da887df11653ac07922f41746b80cdbc106430e1e6978ce244fe99b00a7d9dc1418fc7670 + languageName: node + linkType: hard + "@openzeppelin/contracts@npm:3.4.1-solc-0.7-2": version: 3.4.1-solc-0.7-2 resolution: "@openzeppelin/contracts@npm:3.4.1-solc-0.7-2" @@ -12740,7 +12756,7 @@ __metadata: languageName: node linkType: hard -"@openzeppelin/contracts@npm:4.9.6, @openzeppelin/contracts@npm:^4.9.2": +"@openzeppelin/contracts@npm:^4.9.2": version: 4.9.6 resolution: "@openzeppelin/contracts@npm:4.9.6" checksum: 10/71f45ad42e68c0559be4ba502115462a01c76fc805c08d3005c10b5550a093f1a2b00b2d7e9d6d1f331e147c50fd4ad832f71c4470ec5b34f5a2d0751cd19a47 @@ -19838,8 +19854,10 @@ __metadata: "@nomicfoundation/hardhat-ethers": "npm:3.0.5" "@nomicfoundation/hardhat-network-helpers": "npm:1.0.10" "@nomicfoundation/hardhat-verify": "npm:2.0.6" - "@openzeppelin/contracts": "npm:4.9.6" + "@openzeppelin/contracts": "npm:5.0.2" "@openzeppelin/contracts-upgradeable": "npm:4.9.6" + "@openzeppelin/contracts-upgradeable5": "npm:@openzeppelin/contracts-upgradeable@5.0.2" + "@openzeppelin/contracts5": "npm:@openzeppelin/contracts@5.0.2" "@openzeppelin/hardhat-upgrades": "npm:3.1.0" "@openzeppelin/upgrades-core": "npm:1.33.1" "@safe-global/safe-core-sdk": "npm:3.3.5"