From f30749cdfc9664859328dbcea8c7fd791fd90c95 Mon Sep 17 00:00:00 2001 From: Marco Tabasco Date: Mon, 20 May 2024 14:21:51 +0200 Subject: [PATCH] add integration tests, fix CI --- .../mocks/BadOperatorWhitelistingContract.sol | 4 +- contracts/test/mocks/BeneficiaryContract.sol | 1 - .../test/mocks/GenericWhitelistContract.sol | 27 +++ hardhat.config.ts | 4 +- test-forked/operators-whitelist.ts | 154 +++++++++++++++--- test/validators/whitelist-register.ts | 116 ++++++++++++- 6 files changed, 274 insertions(+), 32 deletions(-) create mode 100644 contracts/test/mocks/GenericWhitelistContract.sol diff --git a/contracts/test/mocks/BadOperatorWhitelistingContract.sol b/contracts/test/mocks/BadOperatorWhitelistingContract.sol index a018a497..28a90835 100644 --- a/contracts/test/mocks/BadOperatorWhitelistingContract.sol +++ b/contracts/test/mocks/BadOperatorWhitelistingContract.sol @@ -5,7 +5,6 @@ import "../../interfaces/external/ISSVWhitelistingContract.sol"; import "../../interfaces/ISSVClusters.sol"; import "./BeneficiaryContract.sol"; import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import "hardhat/console.sol"; /// @notice Whitelisted contract that passes the validatity check of supporting ISSVWhitelistingContract /// and tries to re-enter SSVNetwork.registerValidator function. @@ -25,7 +24,8 @@ contract BadOperatorWhitelistingContract is ERC165 { // only proceed if the function being called is isWhitelisted if (selector == ISSVWhitelistingContract.isWhitelisted.selector) { // decode the operator Id - (uint256 operatorId) = abi.decode(msg.data[36:], (uint256)); + // we can save the target operatorId and try the withdrawal only if it matches + // (uint256 operatorId) = abi.decode(msg.data[36:], (uint256)); // call BeneficiaryContract to withdraw operator earnings beneficiaryContract.withdrawOperatorEarnings(10000000); } diff --git a/contracts/test/mocks/BeneficiaryContract.sol b/contracts/test/mocks/BeneficiaryContract.sol index bb28086a..8efcbc68 100644 --- a/contracts/test/mocks/BeneficiaryContract.sol +++ b/contracts/test/mocks/BeneficiaryContract.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.24; import "../../interfaces/external/ISSVWhitelistingContract.sol"; import "../../interfaces/ISSVOperators.sol"; import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import "hardhat/console.sol"; contract BeneficiaryContract { ISSVOperators private ssvContract; diff --git a/contracts/test/mocks/GenericWhitelistContract.sol b/contracts/test/mocks/GenericWhitelistContract.sol new file mode 100644 index 00000000..987b0cfc --- /dev/null +++ b/contracts/test/mocks/GenericWhitelistContract.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.24; + +import "../../interfaces/ISSVClusters.sol"; +import "../../interfaces/ISSVNetworkCore.sol"; +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +contract GenericWhitelistContract { + ISSVClusters private ssvContract; + IERC20 private ssvToken; + + constructor(ISSVClusters _ssvContract, IERC20 _ssvToken) { + ssvContract = _ssvContract; + ssvToken = _ssvToken; + } + + function registerValidatorSSV( + bytes calldata _publicKey, + uint64[] memory _operatorIds, + bytes calldata _sharesData, + uint256 _amount, + ISSVNetworkCore.Cluster memory _cluserData + ) external { + ssvToken.approve(address(ssvContract), _amount); + ssvContract.registerValidator(_publicKey, _operatorIds, _sharesData, _amount, _cluserData); + } +} diff --git a/hardhat.config.ts b/hardhat.config.ts index bf5a4b62..558180ff 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -82,7 +82,7 @@ const config: HardhatUserConfig = { }, }; -if (process.env.HOLESKY_ETH_NODE_URL) { +if (process.env.HOLESKY_ETH_NODE_URL && process.env.HOLESKY_OWNER_PRIVATE_KEY) { const sharedConfig = { url: `${process.env.HOLESKY_ETH_NODE_URL}${process.env.NODE_PROVIDER_KEY}`, accounts: [`0x${process.env.HOLESKY_OWNER_PRIVATE_KEY}`], @@ -103,7 +103,7 @@ if (process.env.HOLESKY_ETH_NODE_URL) { }; } -if (process.env.MAINNET_ETH_NODE_URL) { +if (process.env.MAINNET_ETH_NODE_URL && process.env.MAINNET_OWNER_PRIVATE_KEY) { //@ts-ignore config.networks = { ...config.networks, diff --git a/test-forked/operators-whitelist.ts b/test-forked/operators-whitelist.ts index baca33ec..30c85d65 100644 --- a/test-forked/operators-whitelist.ts +++ b/test-forked/operators-whitelist.ts @@ -6,10 +6,10 @@ import { setBalance } from '@nomicfoundation/hardhat-toolbox-viem/network-helper import { expect } from 'chai'; import { ethers } from 'hardhat'; -import { DataGenerator, MOCK_SHARES } from '../test/helpers/contract-helpers'; +import { DataGenerator, MOCK_SHARES, publicClient } from '../test/helpers/contract-helpers'; import { assertPostTxEvent } from '../test/helpers/utils/test'; -import { Address, TestClient } from 'viem'; +import { Address, TestClient, encodeFunctionData, walletActions } from 'viem'; // Declare globals let ssvNetwork: any, ssvViews: any, ssvToken: any, owners: any[], client: TestClient; @@ -22,7 +22,7 @@ describe('Whitelisting Tests (fork)', () => { beforeEach(async () => { owners = await hre.viem.getWalletClients(); - client = await hre.viem.getTestClient(); + client = (await hre.viem.getTestClient()).extend(walletActions); await client.impersonateAccount({ address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }); await setBalance('0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6', 2000000000000000000n); @@ -32,30 +32,9 @@ describe('Whitelisting Tests (fork)', () => { ssvToken = await hre.viem.getContractAt('SSVToken', ssvTokenAddress as Address); }); - describe('After Upgrade SSV Core Contracts Tests', () => { + describe('Pre-upgrade SSV Core Contracts Tests', () => { beforeEach(async () => { - const ssvNetworkUpgrade = await hre.viem.deployContract('SSVNetwork', [], { - account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, - }); - await ssvNetwork.write.upgradeTo([await ssvNetworkUpgrade.address], { - account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, - }); - - const ssvViewsUpgrade = await hre.viem.deployContract('SSVNetworkViews', [], { - account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, - }); - await ssvViews.write.upgradeTo([await ssvViewsUpgrade.address], { - account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, - }); - - await upgradeModule('SSVOperators', 0); - await upgradeModule('SSVClusters', 1); - await upgradeModule('SSVViews', 3); - await upgradeModule('SSVOperatorsWhitelist', 4); - - await client.stopImpersonatingAccount({ - address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6', - }); + await upgradeAllContracts(); }); it('Check an existing whitelisted operator is whitelisted but not using an external contract', async () => { const operatorData = await ssvViews.read.getOperatorById([314]); @@ -213,6 +192,102 @@ describe('Whitelisting Tests (fork)', () => { expect((await ssvViews.read.getOperatorById([314]))[3]).to.deep.equal(prevWhitelistedAddress); }); }); + + describe('Ongoing SSV Core Contracts upgrade Tests', () => { + it('WT-3 - Check backward compatibility with existing generic contracts', async () => { + // owner of the operator 314 + await client.impersonateAccount({ address: '0xB4084F25DfCb2c1bf6636b420b59eda807953769' }); + await setBalance('0xB4084F25DfCb2c1bf6636b420b59eda807953769', 1200000000000000000n); + + // deploy a generic contract + const genericWhitelistContract = await hre.viem.deployContract( + 'GenericWhitelistContract', + [await ssvNetwork.address, await ssvToken.address], + { + account: { address: '0xB4084F25DfCb2c1bf6636b420b59eda807953769' }, + }, + ); + + const generiWhitelistContractAddress = await genericWhitelistContract.address; + + const abiItem = { + inputs: [ + { + name: 'operatorId', + type: 'uint64', + }, + { + name: 'whitelisted', + type: 'address', + }, + ], + name: 'setOperatorWhitelist', + outputs: [], + stateMutability: 'nonpayable', + type: 'function', + }; + + // whitelist the generic contract for operator 314 + await client.sendTransaction({ + account: '0xB4084F25DfCb2c1bf6636b420b59eda807953769', + data: encodeFunctionData({ + abi: [abiItem], + functionName: 'setOperatorWhitelist', + args: [314n, generiWhitelistContractAddress], + }), + to: await ssvNetwork.address, + }); + + const validatorCount = (await ssvViews.read.getOperatorById([314n]))[2]; + + await upgradeAllContracts(); + + // whitelist a different operator using SSV whitelisting module + await ssvNetwork.write.setOperatorMultipleWhitelists([[315n], ['0xB4084F25DfCb2c1bf6636b420b59eda807953769']], { + account: { address: '0xB4084F25DfCb2c1bf6636b420b59eda807953769' }, + }); + + const minDepositAmount = 1000000000000000000000n; + + await client.impersonateAccount({ address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }); + + // give the generic contract enough SSV tokens + await ssvToken.write.mint([generiWhitelistContractAddress, minDepositAmount], { + account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, + }); + + // use a new account owners[4] to register a validator using + // the operator 314 through the generic contract + await genericWhitelistContract.write.registerValidatorSSV( + [ + DataGenerator.publicKey(1), + [30, 31, 32, 314], + MOCK_SHARES, + minDepositAmount, + { + validatorCount: 0, + networkFeeIndex: 0, + index: 0, + balance: 0n, + active: true, + }, + ], + { account: owners[4].account }, + ); + + // event confirms full execution + await assertPostTxEvent([ + { + contract: ssvNetwork, + eventName: 'ValidatorAdded', + argNames: ['owner'], + argValuesList: [[generiWhitelistContractAddress]], + }, + ]); + + expect((await ssvViews.read.getOperatorById([314n]))[2]).to.equal(validatorCount + 1); + }); + }); }); //* HELPERS */ @@ -225,3 +300,30 @@ const upgradeModule = async function (contractName: string, id: number) { account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, }); }; + +const upgradeAllContracts = async function () { + await client.impersonateAccount({ address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }); + + const ssvNetworkUpgrade = await hre.viem.deployContract('SSVNetwork', [], { + account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, + }); + await ssvNetwork.write.upgradeTo([await ssvNetworkUpgrade.address], { + account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, + }); + + const ssvViewsUpgrade = await hre.viem.deployContract('SSVNetworkViews', [], { + account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, + }); + await ssvViews.write.upgradeTo([await ssvViewsUpgrade.address], { + account: { address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6' }, + }); + + await upgradeModule('SSVOperators', 0); + await upgradeModule('SSVClusters', 1); + await upgradeModule('SSVViews', 3); + await upgradeModule('SSVOperatorsWhitelist', 4); + + await client.stopImpersonatingAccount({ + address: '0xb35096b074fdb9bBac63E3AdaE0Bbde512B2E6b6', + }); +}; diff --git a/test/validators/whitelist-register.ts b/test/validators/whitelist-register.ts index 55f61e55..a09c2697 100644 --- a/test/validators/whitelist-register.ts +++ b/test/validators/whitelist-register.ts @@ -14,7 +14,7 @@ import { MOCK_SHARES, publicClient, } from '../helpers/contract-helpers'; -import { assertPostTxEvent } from '../helpers/utils/test'; +import { assertPostTxEvent, assertEvent } from '../helpers/utils/test'; import { trackGas, GasGroup, trackGasFromReceipt } from '../helpers/gas-usage'; import { mine } from '@nomicfoundation/hardhat-toolbox-viem/network-helpers'; @@ -426,6 +426,7 @@ describe('Register Validator Tests', () => { describe('Register using whitelisting contract', () => { let mockWhitelistingContractAddress: any; + beforeEach(async () => { // Whitelist whitelistedCaller using an external contract const mockWhitelistingContract = await hre.viem.deployContract( @@ -449,6 +450,119 @@ describe('Register Validator Tests', () => { }); }); + it('Register using whitelisting contract and SSV whitelisting module for 2 operators', async () => { + // Account A whitelists account B on SSV whitelisting module + // Account A adds a whitelisting contract + // Account A adds account C to that whitelist contract + // Register validator with account B and C both work + + // Account A = owners[0] + // Account B = owners[3] + // Account C = owners[4] + + // Account A whitelists account B on SSV whitelisting module (operator 5) + await ssvNetwork.write.setOperatorWhitelist([5, owners[3].account.address]); + + // Account A adds account C to that whitelist contract + const whitelistingContract = await hre.viem.deployContract( + 'MockWhitelistingContract', + [[owners[4].account.address]], + { + client: owners[0].client, + }, + ); + const whitelistingContractAddress = await whitelistingContract.address; + + // Account A adds a whitelisting contract (operator 6) + await ssvNetwork.write.setOperatorsWhitelistingContract([[6], whitelistingContractAddress], { + account: owners[0].account, + }); + + await ssvNetwork.write.setOperatorsPrivateUnchecked([[5, 6]], { + account: owners[0].account, + }); + + await ssvToken.write.approve([ssvNetwork.address, minDepositAmount], { account: owners[3].account }); + + // Register validator with account B works + await assertEvent( + ssvNetwork.write.registerValidator( + [ + DataGenerator.publicKey(1), + [2, 3, 4, 5], + MOCK_SHARES, + minDepositAmount, + { + validatorCount: 0, + networkFeeIndex: 0, + index: 0, + balance: 0n, + active: true, + }, + ], + { account: owners[3].account }, + ), + [ + { + contract: ssvNetwork, + eventName: 'ValidatorAdded', + argNames: ['owner', 'operatorIds'], + argValuesList: [[owners[3].account.address, [2, 3, 4, 5]]], + }, + ], + ); + + // Check the operator 5 increased validatorCount + expect(await ssvViews.read.getOperatorById([5])).to.deep.equal([ + owners[0].account.address, // owner + CONFIG.minimalOperatorFee, // fee + 1, + ethers.ZeroAddress, + true, // isPrivate + true, // active + ]); + + await ssvToken.write.approve([ssvNetwork.address, minDepositAmount], { account: owners[4].account }); + + // Register validator with account C works + await assertEvent( + ssvNetwork.write.registerValidator( + [ + DataGenerator.publicKey(1), + [6, 7, 8, 9], + MOCK_SHARES, + minDepositAmount, + { + validatorCount: 0, + networkFeeIndex: 0, + index: 0, + balance: 0n, + active: true, + }, + ], + { account: owners[4].account }, + ), + [ + { + contract: ssvNetwork, + eventName: 'ValidatorAdded', + argNames: ['owner', 'operatorIds'], + argValuesList: [[owners[4].account.address, [6, 7, 8, 9]]], + }, + ], + ); + + // Check the operator 6 increased validatorCount + expect(await ssvViews.read.getOperatorById([6])).to.deep.equal([ + owners[0].account.address, // owner + CONFIG.minimalOperatorFee, // fee + 1, + whitelistingContractAddress, + true, // isPrivate + true, // active + ]); + }); + it('Register using whitelisting contract for 1 operator in 4 operators cluster gas limits/events/logic', async () => { await ssvToken.write.approve([ssvNetwork.address, minDepositAmount], { account: owners[3].account });