From c80dc3b5ccf8d3e2d770bdebb0d5cf6bfb0086a5 Mon Sep 17 00:00:00 2001 From: Marco Tabasco Date: Tue, 16 Jan 2024 15:45:45 +0100 Subject: [PATCH] add exit validator tests --- contracts/libraries/ValidatorLib.sol | 6 +- contracts/modules/SSVClusters.sol | 8 +- test/sanity/balances.ts | 51 +++++++- test/validators/{others.ts => exit.ts} | 139 ++++++++++++++------ test/validators/remove.ts | 169 ++++++++++++++++++------- 5 files changed, 279 insertions(+), 94 deletions(-) rename test/validators/{others.ts => exit.ts} (72%) diff --git a/contracts/libraries/ValidatorLib.sol b/contracts/libraries/ValidatorLib.sol index ae12e02f..ff85ee90 100644 --- a/contracts/libraries/ValidatorLib.sol +++ b/contracts/libraries/ValidatorLib.sol @@ -56,6 +56,9 @@ library ValidatorLib { uint64[] calldata operatorIds, StorageData storage s ) internal view returns (bytes32[] memory hashedValidator) { + if (publicKeys.length == 0) { + revert ISSVNetworkCore.ValidatorDoesNotExist(); + } hashedValidator = new bytes32[](publicKeys.length); bytes32 hashedOperatorIds = hashOperatorIds(operatorIds); @@ -76,7 +79,8 @@ library ValidatorLib { revert ISSVNetworkCore.ValidatorDoesNotExist(); } - if ((validatorData & ~bytes32(uint256(1))) != hashedOperatorIds) { // All bits set to 1 except LSB + if ((validatorData & ~bytes32(uint256(1))) != hashedOperatorIds) { + // All bits set to 1 except LSB // Clear LSB of stored validator data and compare revert ISSVNetworkCore.IncorrectValidatorState(); } diff --git a/contracts/modules/SSVClusters.sol b/contracts/modules/SSVClusters.sol index c81edaa3..49e0b097 100644 --- a/contracts/modules/SSVClusters.sol +++ b/contracts/modules/SSVClusters.sol @@ -85,11 +85,10 @@ contract SSVClusters is ISSVClusters { ) external override { StorageData storage s = SSVStorage.load(); + bytes32 hashedCluster = cluster.validateHashedCluster(msg.sender, operatorIds, s); bytes32 hashedOperatorIds = ValidatorLib.hashOperatorIds(operatorIds); bytes32 hashedValidator = ValidatorLib.validateState(publicKey, hashedOperatorIds, s); - bytes32 hashedCluster = cluster.validateHashedCluster(msg.sender, operatorIds, s); - if (cluster.active) { StorageProtocol storage sp = SSVStorageProtocol.load(); (uint64 clusterIndex, ) = OperatorLib.updateClusterOperators(operatorIds, false, false, 1, s, sp); @@ -116,7 +115,6 @@ contract SSVClusters is ISSVClusters { StorageData storage s = SSVStorage.load(); bytes32 hashedCluster = cluster.validateHashedCluster(msg.sender, operatorIds, s); - bytes32[] memory hashedValidators = ValidatorLib.validateStates(publicKeys, operatorIds, s); uint32 validatorsLength = uint32(publicKeys.length); @@ -137,12 +135,12 @@ contract SSVClusters is ISSVClusters { sp.updateDAO(false, validatorsLength); } + cluster.validatorCount -= validatorsLength; + for (uint i; i < validatorsLength; ++i) { delete s.validatorPKs[hashedValidators[i]]; } - cluster.validatorCount -= validatorsLength; - s.clusters[hashedCluster] = cluster.hashClusterData(); for (uint i; i < validatorsLength; ++i) { diff --git a/test/sanity/balances.ts b/test/sanity/balances.ts index e19da092..bb4401ce 100644 --- a/test/sanity/balances.ts +++ b/test/sanity/balances.ts @@ -430,13 +430,7 @@ describe('Balance Tests', () => { 10, [5, 6, 7, 8], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await utils.progressBlocks(2); @@ -474,4 +468,47 @@ describe('Balance Tests', () => { expect(await ssvViews.getOperatorEarnings(7)).to.equal((helpers.CONFIG.minimalOperatorFee * 10 * 3) + (helpers.CONFIG.minimalOperatorFee * 5 * 2)); expect(await ssvViews.getOperatorEarnings(8)).to.equal((helpers.CONFIG.minimalOperatorFee * 10 * 3) + (helpers.CONFIG.minimalOperatorFee * 5 * 2)); }); + + it('Remove validators from a liquidated cluster', async () => { + const clusterDeposit = minDepositAmount * 10; + // 3 operators cluster burnPerBlock + const newBurnPerBlock = helpers.CONFIG.minimalOperatorFee * 3 + networkFee; + + // register 10 validators + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + [5, 6, 7, 8], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await utils.progressBlocks(2); + + // remove one operator + await ssvNetworkContract.connect(helpers.DB.owners[0]).removeOperator(8); + + await utils.progressBlocks(2); + + // bulk remove 10 validators + const result = await trackGas(ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks, args.operatorIds, args.cluster) + ); + const removed = result.eventsByName.ValidatorRemoved[0].args; + + await utils.progressBlocks(2); + + // check operators' balances + expect(await ssvViews.getOperatorEarnings(5)).to.equal((helpers.CONFIG.minimalOperatorFee * 10 * 6)); + expect(await ssvViews.getOperatorEarnings(6)).to.equal((helpers.CONFIG.minimalOperatorFee * 10 * 6)); + expect(await ssvViews.getOperatorEarnings(7)).to.equal((helpers.CONFIG.minimalOperatorFee * 10 * 6)); + expect(await ssvViews.getOperatorEarnings(8)).to.equal(0); + + // check cluster balance + expect( + await ssvViews.getBalance(helpers.DB.owners[2].address, removed.operatorIds, removed.cluster), + ).to.equal(clusterDeposit - (burnPerBlock * 10 * 3) - (newBurnPerBlock * 10 * 3)); + + }); }); diff --git a/test/validators/others.ts b/test/validators/exit.ts similarity index 72% rename from test/validators/others.ts rename to test/validators/exit.ts index 6d751866..dd5eff6a 100644 --- a/test/validators/others.ts +++ b/test/validators/exit.ts @@ -6,7 +6,7 @@ import { trackGas, GasGroup } from '../helpers/gas-usage'; // Declare globals let ssvNetworkContract: any, minDepositAmount: any, firstCluster: any; -describe('Other Validator Tests', () => { +describe('Exit Validator Tests', () => { beforeEach(async () => { // Initialize contract const metadata = await helpers.initializeContract(); @@ -176,13 +176,7 @@ describe('Other Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[4], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await expect( @@ -199,13 +193,7 @@ describe('Other Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[4], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -222,13 +210,7 @@ describe('Other Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[7], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -245,13 +227,7 @@ describe('Other Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[10], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -268,13 +244,7 @@ describe('Other Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[13], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -284,4 +254,101 @@ describe('Other Validator Tests', () => { [GasGroup.BULK_EXIT_10_VALIDATOR_13], ); }); + + it('Bulk exiting removed validators reverts "ValidatorDoesNotExist"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await trackGas(ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks.slice(0, 5), args.operatorIds, args.cluster) + ); + + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkExitValidator(pks.slice(0, 5), args.operatorIds), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + }); + + it('Bulk exiting non-existing validators reverts "ValidatorDoesNotExist"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + pks[4] = "0xabcd1234"; + + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkExitValidator(pks, args.operatorIds), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + }); + + it('Bulk exiting validators with empty operator list reverts "IncorrectValidatorState"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await expect( + ssvNetworkContract.connect(helpers.DB.owners[2]).bulkExitValidator(pks, []), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'IncorrectValidatorState'); + }); + + it('Bulk exiting validators with empty public key reverts "ValidatorDoesNotExist', async () => { + const { args } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await expect( + ssvNetworkContract.connect(helpers.DB.owners[2]).bulkExitValidator([], args.operatorIds), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + }); + + it('Bulk exiting validators using the wrong account reverts "ValidatorDoesNotExist"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[3]) + .bulkExitValidator(pks, args.operatorIds), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + }); + + it('Bulk exiting validators with incorrect operators (unsorted list) reverts with "IncorrectValidatorState"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await expect( + ssvNetworkContract.connect(helpers.DB.owners[1]).bulkExitValidator(pks, [4, 3, 2, 1]), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'IncorrectValidatorState'); + }); }); diff --git a/test/validators/remove.ts b/test/validators/remove.ts index e5b8bf35..ecbdc3bd 100644 --- a/test/validators/remove.ts +++ b/test/validators/remove.ts @@ -49,13 +49,7 @@ describe('Remove Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[4], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await expect( @@ -68,10 +62,9 @@ describe('Remove Validator Tests', () => { it('Remove validator after cluster liquidation period emits "ValidatorRemoved"', async () => { await utils.progressBlocks(helpers.CONFIG.minimalBlocksBeforeLiquidation + 10); - await expect( - ssvNetworkContract - .connect(helpers.DB.owners[1]) - .removeValidator(helpers.DataGenerator.publicKey(1), firstCluster.operatorIds, firstCluster.cluster), + await expect(ssvNetworkContract + .connect(helpers.DB.owners[1]) + .removeValidator(helpers.DataGenerator.publicKey(1), firstCluster.operatorIds, firstCluster.cluster), ).to.emit(ssvNetworkContract, 'ValidatorRemoved'); }); @@ -90,13 +83,7 @@ describe('Remove Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[4], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -132,13 +119,7 @@ describe('Remove Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[7], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -174,13 +155,7 @@ describe('Remove Validator Tests', () => { 10, helpers.DEFAULT_OPERATOR_IDS[10], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -196,7 +171,7 @@ describe('Remove Validator Tests', () => { 1, (minDepositAmount * 4).toString(), [2], - [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13], + helpers.DEFAULT_OPERATOR_IDS[13], helpers.getClusterForValidator(0, 0, 0, 0, true), [GasGroup.REGISTER_VALIDATOR_NEW_STATE_13], ); @@ -214,15 +189,9 @@ describe('Remove Validator Tests', () => { const { args, pks } = await helpers.bulkRegisterValidators( 2, 10, - [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13], + helpers.DEFAULT_OPERATOR_IDS[13], minDepositAmount, - { - validatorCount: 0, - networkFeeIndex: 0, - index: 0, - balance: 0, - active: true, - }, + helpers.DB.initialClusterState, ); await trackGas( @@ -297,23 +266,42 @@ describe('Remove Validator Tests', () => { ); }); - it('Remove validator with an invalid owner reverts "ValidatorDoesNotExist"', async () => { + it('Remove validator with an invalid owner reverts "ClusterDoesNotExists"', async () => { await expect( ssvNetworkContract .connect(helpers.DB.owners[2]) .removeValidator(helpers.DataGenerator.publicKey(1), firstCluster.operatorIds, firstCluster.cluster), - ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ClusterDoesNotExists'); }); - it('Remove validator with an invalid operator setup reverts "IncorrectValidatorState"', async () => { + it('Remove validator with an invalid operator setup reverts "ClusterDoesNotExists"', async () => { await expect( ssvNetworkContract .connect(helpers.DB.owners[1]) .removeValidator(helpers.DataGenerator.publicKey(1), [1, 2, 3, 5], firstCluster.cluster), - ).to.be.revertedWithCustomError(ssvNetworkContract, 'IncorrectValidatorState'); + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ClusterDoesNotExists'); }); it('Remove the same validator twice reverts "ValidatorDoesNotExist"', async () => { + // Remove validator + const result = await trackGas( + ssvNetworkContract + .connect(helpers.DB.owners[1]) + .removeValidator(helpers.DataGenerator.publicKey(1), firstCluster.operatorIds, firstCluster.cluster), + [GasGroup.REMOVE_VALIDATOR], + ); + + const removed = result.eventsByName.ValidatorRemoved[0].args; + + // Remove validator again + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[1]) + .removeValidator(helpers.DataGenerator.publicKey(1), removed.operatorIds, removed.cluster), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + }); + + it('Remove the same validator with wrong input parameters reverts "IncorrectClusterState"', async () => { // Remove validator await trackGas( ssvNetworkContract @@ -327,6 +315,97 @@ describe('Remove Validator Tests', () => { ssvNetworkContract .connect(helpers.DB.owners[1]) .removeValidator(helpers.DataGenerator.publicKey(1), firstCluster.operatorIds, firstCluster.cluster), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'IncorrectClusterState'); + }); + + it('Bulk Remove validator that does not exist in a valid cluster reverts "ValidatorDoesNotExist"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + pks[2] = "0xabcd1234"; + + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks, args.operatorIds, args.cluster), ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); }); + + it('Bulk remove validator with an invalid operator setup reverts "ClusterDoesNotExists"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks, [1, 2, 3, 5], args.cluster), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ClusterDoesNotExists'); + }); + + it('Bulk Remove the same validator twice reverts "ValidatorDoesNotExist"', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + const result = await trackGas( + ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks, args.operatorIds, args.cluster) + ); + + const removed = result.eventsByName.ValidatorRemoved[0].args; + + // Remove validator again + await expect( + ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks, removed.operatorIds, removed.cluster), + ).to.be.revertedWithCustomError(ssvNetworkContract, 'ValidatorDoesNotExist'); + }); + + it('Remove validators from a liquidated cluster', async () => { + const { args, pks } = await helpers.bulkRegisterValidators( + 2, + 10, + helpers.DEFAULT_OPERATOR_IDS[4], + minDepositAmount, + helpers.DB.initialClusterState, + ); + + await utils.progressBlocks(helpers.CONFIG.minimalBlocksBeforeLiquidation - 2); + + let result = await trackGas(ssvNetworkContract + .connect(helpers.DB.owners[1]) + .liquidate(args.owner, args.operatorIds, args.cluster) + ); + + const liquidated = result.eventsByName.ClusterLiquidated[0].args; + + result = await trackGas(ssvNetworkContract + .connect(helpers.DB.owners[2]) + .bulkRemoveValidator(pks.slice(0, 5), liquidated.operatorIds, liquidated.cluster) + ); + + const removed = result.eventsByName.ValidatorRemoved[0].args; + + expect(removed.cluster.validatorCount).to.equal(5); + expect(removed.cluster.networkFeeIndex.toNumber()).to.equal(0); + expect(removed.cluster.index.toNumber()).to.equal(0); + expect(removed.cluster.active).to.equal(false); + expect(removed.cluster.balance.toNumber()).to.equal(0); + }); });