From 3c6f74731030d18d7a5b445e15065e2b08aeffe3 Mon Sep 17 00:00:00 2001 From: Ryan Sauge Date: Mon, 21 Aug 2023 15:35:20 +0200 Subject: [PATCH] Proxy test: adaptation to Hardhat and support of custom errors --- contracts/test/proxy/CMTAT_PROXY.sol | 23 ++++++ package.json | 3 +- test/deploymentUtils.js | 15 +++- test/proxy/general/KillImplementation.test.js | 18 ++--- test/proxy/general/Proxy.test.js | 40 +++++------ test/proxy/general/UpgradeProxy.test.js | 72 +++++++++++++------ 6 files changed, 113 insertions(+), 58 deletions(-) create mode 100644 contracts/test/proxy/CMTAT_PROXY.sol diff --git a/contracts/test/proxy/CMTAT_PROXY.sol b/contracts/test/proxy/CMTAT_PROXY.sol new file mode 100644 index 00000000..05592234 --- /dev/null +++ b/contracts/test/proxy/CMTAT_PROXY.sol @@ -0,0 +1,23 @@ +//SPDX-License-Identifier: MPL-2.0 + +pragma solidity ^0.8.20; + +import "../../CMTAT_PROXY.sol"; + +/** +* @title a contrat used to test the proxy upgrade functionality +*/ +contract CMTAT_PROXY_TEST is CMTAT_PROXY { + /** + * @notice Contract version for the deployment with a proxy + * @param forwarderIrrevocable address of the forwarder, required for the gasless support + */ + /// @custom:oz-upgrades-unsafe-allow constructor + constructor( + address forwarderIrrevocable + ) CMTAT_PROXY(forwarderIrrevocable) { + // Nothing to do + } + + uint256[50] private __gap; +} diff --git a/package.json b/package.json index 6860d4c7..52c54b11 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,8 @@ "test:hardhat:creditEvents": "npx hardhat test test/standard/modules/CreditEventsModule.test.js test/proxy/modules/CreditEventsModule.test.js", "test:hardhat:validation": "npx hardhat test test/standard/modules/ValidationModule/ValidationModule.test.js test/proxy/modules/ValidationModule/ValidationModule.test.js test/standard/modules/ValidationModule/ValidationModuleConstructor.test.js test/proxy/modules/ValidationModule/ValidationModuleConstructor.test.js test/standard/modules/ValidationModule/ValidationModuleSetRuleEngine.test.js test/proxy/modules/ValidationModule/ValidationModuleSetRuleEngine.test.js", "test:hardhat:authorization": "npx hardhat test test/standard/modules/AuthorizationModule/AuthorizationModule.test.js test/proxy/modules/AuthorizationModule/AuthorizationModule.test.js", - "test:hardhat:snapshot": "npx hardhat test test/standard/modules/SnapshotModule.test.js test/proxy/modules/SnapshotModule.test.js" + "test:hardhat:snapshot": "npx hardhat test test/standard/modules/SnapshotModule.test.js test/proxy/modules/SnapshotModule.test.js", + "test:hardhat:proxy": "npx hardhat test test/proxy/general/KillImplementation.test.js test/proxy/general/Proxy.test.js test/proxy/general/UpgradeProxy.test.js" }, "repository": { "type": "git", diff --git a/test/deploymentUtils.js b/test/deploymentUtils.js index 16c9aff3..39228330 100644 --- a/test/deploymentUtils.js +++ b/test/deploymentUtils.js @@ -52,6 +52,19 @@ async function deployCMTATProxyWithSnapshot (_, admin, deployerAddress) { return TRUFFLE_CMTAT_PROXY_ADDRESS } +async function deployCMTATProxyWithKillTest (_, admin, deployerAddress) { + // Ref: https://forum.openzeppelin.com/t/upgrades-hardhat-truffle5/30883/3 + const ETHERS_CMTAT_PROXY_FACTORY = await ethers.getContractFactory('CMTAT_KILL_TEST'); + const ETHERS_CMTAT_PROXY = await upgrades.deployProxy(ETHERS_CMTAT_PROXY_FACTORY, [admin, 'CMTA Token', 'CMTAT', DEPLOYMENT_DECIMAL, 'CMTAT_ISIN', 'https://cmta.ch', ZERO_ADDRESS, 'CMTAT_info', DEPLOYMENT_FLAG], { + initializer: 'initialize', + constructorArgs: [_], + from: deployerAddress + }); + const TRUFFLE_CMTAT_PROXY = artifacts.require('CMTAT_KILL_TEST') + const TRUFFLE_CMTAT_PROXY_ADDRESS = await TRUFFLE_CMTAT_PROXY.at(await ETHERS_CMTAT_PROXY.getAddress()); + return TRUFFLE_CMTAT_PROXY_ADDRESS +} + async function deployCMTATProxyWithParameter (deployerAddress, forwarderIrrevocable, admin, nameIrrevocable, symbolIrrevocable, decimalsIrrevocable, @@ -69,4 +82,4 @@ async function deployCMTATProxyWithParameter (deployerAddress, forwarderIrrevoca return TRUFFLE_CMTAT_PROXY_ADDRESS } -module.exports = { deployCMTATStandalone, deployCMTATProxy, deployCMTATProxyWithSnapshot, deployCMTATProxyWithParameter, deployCMTATStandaloneWithParameter, DEPLOYMENT_FLAG, DEPLOYMENT_DECIMAL } +module.exports = { deployCMTATStandalone, deployCMTATProxy, deployCMTATProxyWithSnapshot, deployCMTATProxyWithKillTest, deployCMTATProxyWithParameter, deployCMTATStandaloneWithParameter, DEPLOYMENT_FLAG, DEPLOYMENT_DECIMAL } diff --git a/test/proxy/general/KillImplementation.test.js b/test/proxy/general/KillImplementation.test.js index e96a8435..53380eff 100644 --- a/test/proxy/general/KillImplementation.test.js +++ b/test/proxy/general/KillImplementation.test.js @@ -4,6 +4,7 @@ * */ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers') +const {deployCMTATProxyWithKillTest} = require('../../deploymentUtils') const { should } = require('chai').should() const DECIMAL = 0 const { ZERO_ADDRESS } = require('../../utils') @@ -11,25 +12,16 @@ const { deployProxy, erc1967 } = require('@openzeppelin/truffle-upgrades') - +const { ethers, upgrades } = require("hardhat"); const CMTAT_KILL_TEST = artifacts.require('CMTAT_KILL_TEST') -contract('Proxy - Security Test', function ([_, admin]) { +contract('Proxy - Security Test', function ([_, admin, deployerAddress]) { beforeEach(async function () { this.flag = 5 // Contract to deploy: CMTAT_KILL_TEST - this.CMTAT_PROXY = await deployProxy( - CMTAT_KILL_TEST, - [admin, 'CMTA Token', 'CMTAT', DECIMAL, 'CMTAT_ISIN', 'https://cmta.ch', ZERO_ADDRESS, 'CMTAT_info', this.flag], - { - initializer: 'initialize', - constructorArgs: [ - _ - ] - } - ) + this.CMTAT_PROXY = await deployCMTATProxyWithKillTest(_, admin, deployerAddress) const implementationContractAddress = - await erc1967.getImplementationAddress(this.CMTAT_PROXY.address, { + await upgrades.erc1967.getImplementationAddress(this.CMTAT_PROXY.address, { from: admin }) this.implementationContract = await CMTAT_KILL_TEST.at( diff --git a/test/proxy/general/Proxy.test.js b/test/proxy/general/Proxy.test.js index d8ea00c2..ddfbd860 100644 --- a/test/proxy/general/Proxy.test.js +++ b/test/proxy/general/Proxy.test.js @@ -2,12 +2,13 @@ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers') const { should } = require('chai').should() const { deployProxy, upgradeProxy, erc1967 } = require('@openzeppelin/truffle-upgrades') +const { expectRevertCustomError } = require('../../../openzeppelin-contracts-upgradeable/test/helpers/customError.js') const CMTAT = artifacts.require('CMTAT_PROXY') const { DEFAULT_ADMIN_ROLE } = require('../../utils') const { ZERO_ADDRESS } = require('../../utils') const DECIMAL = 0 const { deployCMTATProxy, DEPLOYMENT_FLAG } = require('../../deploymentUtils') - +const { ethers, upgrades } = require("hardhat"); contract( 'Proxy - Security Test', function ([_, admin, attacker, deployerAddress]) { @@ -15,48 +16,43 @@ contract( this.flag = 5 // Contract to deploy: CMTAT this.CMTAT_PROXY = await deployCMTATProxy(_, admin, deployerAddress) - const implementationContractAddress = await erc1967.getImplementationAddress(this.CMTAT_PROXY.address, { from: admin }) + const implementationContractAddress = await upgrades.erc1967.getImplementationAddress(this.CMTAT_PROXY.address, { from: admin }) this.implementationContract = await CMTAT.at(implementationContractAddress) }) context('Attacker', function () { // Here the argument to indicate if it is deployed with a proxy, set at false by the attacker it('testCannotBeTakenControlByAttacker1', async function () { - await expectRevert( + await expectRevertCustomError( this.implementationContract.initialize(attacker, 'CMTA Token', 'CMTAT', DECIMAL, 'CMTAT_ISIN', 'https://cmta.ch', ZERO_ADDRESS, 'CMTAT_info', DEPLOYMENT_FLAG, { from: attacker }), - 'Initializable: contract is already initialized' + 'AlreadyInitialized', + [] ) - await expectRevert( + await expectRevertCustomError( this.implementationContract.kill({ from: attacker }), - 'AccessControl: account ' + - attacker.toLowerCase() + - ' is missing role ' + - DEFAULT_ADMIN_ROLE + 'AccessControlUnauthorizedAccount', + [attacker, DEFAULT_ADMIN_ROLE] ) }) // Here the argument to indicate if it is deployed with a proxy, set at true by the attacker it('testCannotBeTakenControlByAttacker2', async function () { - await expectRevert( + await expectRevertCustomError( this.implementationContract.initialize(attacker, 'CMTA Token', 'CMTAT', DECIMAL, 'CMTAT_ISIN', 'https://cmta.ch', ZERO_ADDRESS, 'CMTAT_info', DEPLOYMENT_FLAG, { from: attacker }), - 'Initializable: contract is already initialized' - ) - await expectRevert( + 'AlreadyInitialized', + []) + await expectRevertCustomError( this.implementationContract.kill({ from: attacker }), - 'AccessControl: account ' + - attacker.toLowerCase() + - ' is missing role ' + - DEFAULT_ADMIN_ROLE + 'AccessControlUnauthorizedAccount', + [attacker, DEFAULT_ADMIN_ROLE] ) }) }) context('Admin', function () { it('testCannotKillTheImplementationContractByAdmin', async function () { - await expectRevert( + await expectRevertCustomError( this.implementationContract.kill({ from: admin }), - 'AccessControl: account ' + - admin.toLowerCase() + - ' is missing role ' + - DEFAULT_ADMIN_ROLE + 'AccessControlUnauthorizedAccount', + [admin, DEFAULT_ADMIN_ROLE] ); (await this.implementationContract.terms()).should.equal('') }) diff --git a/test/proxy/general/UpgradeProxy.test.js b/test/proxy/general/UpgradeProxy.test.js index b800020e..5cde55d3 100644 --- a/test/proxy/general/UpgradeProxy.test.js +++ b/test/proxy/general/UpgradeProxy.test.js @@ -1,53 +1,83 @@ -const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers') +const { expectEvent, expectRevert, BN } = require('@openzeppelin/test-helpers') const { should } = require('chai').should() const { deployProxy, upgradeProxy, erc1967 } = require('@openzeppelin/truffle-upgrades') const CMTAT1 = artifacts.require('CMTAT_PROXY') -const CMTAT2 = artifacts.require('CMTAT_PROXY') +const CMTAT2 = artifacts.require('CMTAT_PROXY_TEST') const { ZERO_ADDRESS } = require('../../utils') const DECIMAL = 0 - -contract('UpgradeableCMTAT - Proxy', function ([_, admin, address1]) { +const { deployCMTATProxy, DEPLOYMENT_FLAG, DEPLOYMENT_DECIMAL } = require('../../deploymentUtils') +const { ethers, upgrades } = require("hardhat"); +contract('UpgradeableCMTAT - Proxy', function ([_, admin, address1, deployerAddress]) { /* Functions used: balanceOf, totalSupply, mint */ it('testKeepStorageForTokens', async function () { - this.flag = 5 - // With the first version of CMTAT - this.CMTAT_PROXY = await deployProxy(CMTAT1, [admin, 'CMTA Token', 'CMTAT', DECIMAL, 'CMTAT_ISIN', 'https://cmta.ch', ZERO_ADDRESS, 'CMTAT_info', this.flag], { + ///// ADAPT TRUFFLE TEST TO HARDHAT + /* Factory & Artefact*/ + const ETHERS_CMTAT_PROXY_FACTORY = await ethers.getContractFactory('CMTAT_PROXY'); + const ETHERS_CMTAT_PROXY_FACTORY_2 = await ethers.getContractFactory('CMTAT_PROXY_TEST'); + const TRUFFLE_CMTAT_PROXY = artifacts.require('CMTAT_PROXY') + const TRUFFLE_CMTAT_PROXY_V2 = artifacts.require('CMTAT_PROXY_TEST') + // Deployment + const ETHERS_CMTAT_PROXY = await upgrades.deployProxy(ETHERS_CMTAT_PROXY_FACTORY, [admin, 'CMTA Token', 'CMTAT', DEPLOYMENT_DECIMAL, 'CMTAT_ISIN', 'https://cmta.ch', ZERO_ADDRESS, 'CMTAT_info', DEPLOYMENT_FLAG], { initializer: 'initialize', - constructorArgs: [_] - }) - const implementationContractAddress1 = erc1967.getImplementationAddress(this.CMTAT_PROXY.address, { + constructorArgs: [_], + from: deployerAddress + }); + + // Get Proxy Address + const TRUFFLE_CMTAT_PROXY_ADDRESS = await TRUFFLE_CMTAT_PROXY.at(await ETHERS_CMTAT_PROXY.getAddress()); + const ETHER_PROXY_ADDRESS = await ETHERS_CMTAT_PROXY.getAddress() + const IMPLEMENTATION_CONTRACT_ADDRESS_V1 = await upgrades.erc1967.getImplementationAddress(ETHER_PROXY_ADDRESS, { from: admin }); - (await this.CMTAT_PROXY.balanceOf(admin)).should.be.bignumber.equal('0'); + + // With the first version of CMTAT + (await TRUFFLE_CMTAT_PROXY_ADDRESS.balanceOf(admin)).should.be.bignumber.equal('0'); // Issue 20 and check balances and total supply - ({ logs: this.logs1 } = await this.CMTAT_PROXY.mint(address1, 20, { + ({ logs: this.logs1 } = await TRUFFLE_CMTAT_PROXY_ADDRESS.mint(address1, 20, { from: admin })); - (await this.CMTAT_PROXY.balanceOf(address1)).should.be.bignumber.equal('20'); - (await this.CMTAT_PROXY.totalSupply()).should.be.bignumber.equal('20') + + (await TRUFFLE_CMTAT_PROXY_ADDRESS.balanceOf(address1)).should.be.bignumber.equal('20'); + (await TRUFFLE_CMTAT_PROXY_ADDRESS.totalSupply()).should.be.bignumber.equal('20') // Upgrade the proxy with a new implementation contract - this.upgradeableCMTATV2Instance = await upgradeProxy(this.CMTAT_PROXY.address, CMTAT2, { + const ETHERS_CMTAT_PROXY_V2 = await upgrades.upgradeProxy(ETHER_PROXY_ADDRESS, ETHERS_CMTAT_PROXY_FACTORY_2, { constructorArgs: [_] }) + const PROXY_ADDRESS_V2_INSTANCE = await ETHERS_CMTAT_PROXY_V2.getAddress() + // Get the new implementation contract address - const implementationContractAddress2 = erc1967.getImplementationAddress(this.CMTAT_PROXY.address, { + const IMPLEMENTATION_CONTRACT_ADDRESS_V2 = await upgrades.erc1967.getImplementationAddress(PROXY_ADDRESS_V2_INSTANCE, { from: admin }) + + // Just to be sure that the proxy address remains unchanged + ETHER_PROXY_ADDRESS.should.be.equal(PROXY_ADDRESS_V2_INSTANCE); + // The address of the implementation contract has changed - implementationContractAddress1.should.not.be.equal(implementationContractAddress2); + IMPLEMENTATION_CONTRACT_ADDRESS_V1.should.not.be.equal(IMPLEMENTATION_CONTRACT_ADDRESS_V2); + + // Just to be sure that the proxy address remains unchanged + const TRUFFLE_CMTAT_PROXY_ADDRESS_2 = await TRUFFLE_CMTAT_PROXY_V2.at(PROXY_ADDRESS_V2_INSTANCE); + //TRUFFLE_CMTAT_PROXY_ADDRESS.should.be.equal(TRUFFLE_CMTAT_PROXY_ADDRESS_2); + + ({ logs: this.logs1 } = await TRUFFLE_CMTAT_PROXY_ADDRESS_2.balanceOf(address1)); + + (await TRUFFLE_CMTAT_PROXY_ADDRESS_2.balanceOf(address1)).should.be.bignumber.equal(BN(20)); + // keep the storage - (await this.upgradeableCMTATV2Instance.balanceOf(address1)).should.be.bignumber.equal('20'); + + // Issue 20 and check balances and total supply - ({ logs: this.logs1 } = await this.upgradeableCMTATV2Instance.mint(address1, 20, { + ({ logs: this.logs1 } = await TRUFFLE_CMTAT_PROXY_ADDRESS_2.mint(address1, 20, { from: admin })); - (await this.upgradeableCMTATV2Instance.balanceOf(address1)).should.be.bignumber.equal('40'); - (await this.upgradeableCMTATV2Instance.totalSupply()).should.be.bignumber.equal('40') + (await TRUFFLE_CMTAT_PROXY_ADDRESS_2.balanceOf(address1)).should.be.bignumber.equal('40'); + (await TRUFFLE_CMTAT_PROXY_ADDRESS_2.totalSupply()).should.be.bignumber.equal('40') }) })