From add2434d682090903dbad5a9424aecc8fcb28b2a Mon Sep 17 00:00:00 2001 From: Marco Tabasco Date: Sat, 8 Jun 2024 08:30:41 +0200 Subject: [PATCH] getWhitelistedOperators legacy whitelist support, fix reduceOperatorFee --- contracts/modules/SSVOperators.sol | 4 +- contracts/modules/SSVViews.sol | 29 ++++---- contracts/test/mocks/BeneficiaryContract.sol | 2 +- test-forked/operators-whitelist.ts | 12 ++-- test/helpers/contract-helpers.ts | 2 +- test/operators/update-fee.ts | 6 ++ test/sanity/balances.ts | 75 ++++++++++++++------ 7 files changed, 89 insertions(+), 41 deletions(-) diff --git a/contracts/modules/SSVOperators.sol b/contracts/modules/SSVOperators.sol index 4cbcf335..029cbc34 100644 --- a/contracts/modules/SSVOperators.sol +++ b/contracts/modules/SSVOperators.sol @@ -11,7 +11,7 @@ import "../libraries/CoreLib.sol"; import {Counters} from "@openzeppelin/contracts/utils/Counters.sol"; contract SSVOperators is ISSVOperators { - uint64 private constant MINIMAL_OPERATOR_FEE = 100_000_000; + uint64 private constant MINIMAL_OPERATOR_FEE = 1_000_000_000; uint64 private constant PRECISION_FACTOR = 10_000; using Types256 for uint256; @@ -155,6 +155,8 @@ contract SSVOperators is ISSVOperators { Operator memory operator = s.operators[operatorId]; operator.checkOwner(); + if (fee != 0 && fee < MINIMAL_OPERATOR_FEE) revert FeeTooLow(); + uint64 shrunkAmount = fee.shrink(); if (shrunkAmount >= operator.fee) revert FeeIncreaseNotAllowed(); diff --git a/contracts/modules/SSVViews.sol b/contracts/modules/SSVViews.sol index 4e585042..3f530f7f 100644 --- a/contracts/modules/SSVViews.sol +++ b/contracts/modules/SSVViews.sol @@ -77,10 +77,10 @@ contract SSVViews is ISSVViews { function getWhitelistedOperators( uint64[] calldata operatorIds, - address whitelistedAddress + address addressToCheck ) external view override returns (uint64[] memory whitelistedOperatorIds) { uint256 operatorsLength = operatorIds.length; - if (operatorsLength == 0) return whitelistedOperatorIds; + if (operatorsLength == 0 || addressToCheck == address(0)) return whitelistedOperatorIds; StorageData storage s = SSVStorage.load(); @@ -97,7 +97,7 @@ contract SSVViews is ISSVViews { for (uint256 blockIndex; blockIndex < masks.length; ++blockIndex) { // Only check blocks that have operator IDs if (masks[blockIndex] != 0) { - whitelistedMask = s.addressWhitelistedForOperators[whitelistedAddress][blockIndex]; + whitelistedMask = s.addressWhitelistedForOperators[addressToCheck][blockIndex]; // This will give the matching whitelisted operators matchedMask = whitelistedMask & masks[blockIndex]; @@ -130,19 +130,22 @@ contract SSVViews is ISSVViews { uint64 operatorId = operatorIds[operatorIndex]; // Check if operatorId is already in internalWhitelistedOperatorIds - if (internalWhitelistIndex < internalCount && operatorId == internalWhitelistedOperatorIds[internalWhitelistIndex]) { + if ( + internalWhitelistIndex < internalCount && + operatorId == internalWhitelistedOperatorIds[internalWhitelistIndex] + ) { whitelistedOperatorIds[count++] = operatorId; ++internalWhitelistIndex; } else { - // Check whitelisting contract - address whitelistingContract = s.operatorsWhitelist[operatorId]; - if (whitelistingContract != address(0)) { - if ( - OperatorLib.isWhitelistingContract(whitelistingContract) && - ISSVWhitelistingContract(whitelistingContract).isWhitelisted(whitelistedAddress, operatorId) - ) { - whitelistedOperatorIds[count++] = operatorId; - } + address whitelistedAddress = s.operatorsWhitelist[operatorId]; + + // Legacy address whitelists (EOAs or generic contracts) + if ( + whitelistedAddress == addressToCheck || + (OperatorLib.isWhitelistingContract(whitelistedAddress) && + ISSVWhitelistingContract(whitelistedAddress).isWhitelisted(addressToCheck, operatorId)) + ) { + whitelistedOperatorIds[count++] = operatorId; } } ++operatorIndex; diff --git a/contracts/test/mocks/BeneficiaryContract.sol b/contracts/test/mocks/BeneficiaryContract.sol index c7e92d0f..c9626cb9 100644 --- a/contracts/test/mocks/BeneficiaryContract.sol +++ b/contracts/test/mocks/BeneficiaryContract.sol @@ -23,6 +23,6 @@ contract BeneficiaryContract { } function registerOperator() external returns (uint64 operatorId) { - return ISSVOperators(ssvContract).registerOperator("0xcafecafe", 100000000, false); + return ISSVOperators(ssvContract).registerOperator("0xcafecafe", 1000000000, false); } } diff --git a/test-forked/operators-whitelist.ts b/test-forked/operators-whitelist.ts index 0dff189f..290d4edf 100644 --- a/test-forked/operators-whitelist.ts +++ b/test-forked/operators-whitelist.ts @@ -181,13 +181,17 @@ describe('Whitelisting Tests (fork) - Pre-upgrade SSV Core Contracts Tests', () // get the current whitelisted address const prevWhitelistedAddress = (await ssvViews.read.getOperatorById([314]))[3]; - await ssvNetwork.write.setOperatosWhitelists([[314, 315, 316, 317], [owners[2].account.address]], { + await ssvNetwork.write.setOperatosWhitelists([[315, 316, 317], [owners[2].account.address]], { account: { address: '0xB4084F25DfCb2c1bf6636b420b59eda807953769' }, }); - expect( - await ssvViews.read.getWhitelistedOperators([[314, 315, 316, 317], owners[2].account.address]), - ).to.deep.equal([314n, 315n, 316n, 317n]); + expect(await ssvViews.read.getWhitelistedOperators([[315, 316, 317], owners[2].account.address])).to.deep.equal([ + 315n, + 316n, + 317n, + ]); + + expect(await ssvViews.read.getWhitelistedOperators([[314], prevWhitelistedAddress])).to.deep.equal([314n]); // the operator uses the previous whitelisting main address expect((await ssvViews.read.getOperatorById([314]))[3]).to.deep.equal(prevWhitelistedAddress); diff --git a/test/helpers/contract-helpers.ts b/test/helpers/contract-helpers.ts index 1351d6fa..cae0fa7d 100644 --- a/test/helpers/contract-helpers.ts +++ b/test/helpers/contract-helpers.ts @@ -25,7 +25,7 @@ export const CONFIG: SSVConfig = { operatorMaxFeeIncrease: 1000, declareOperatorFeePeriod: 3600, // HOUR executeOperatorFeePeriod: 86400, // DAY - minimalOperatorFee: 100000000n, + minimalOperatorFee: 1000000000n, minimalBlocksBeforeLiquidation: 100800, minimumLiquidationCollateral: 200000000, validatorsPerOperatorLimit: 500, diff --git a/test/operators/update-fee.ts b/test/operators/update-fee.ts index 5f4efb0f..556ef0ae 100644 --- a/test/operators/update-fee.ts +++ b/test/operators/update-fee.ts @@ -328,6 +328,12 @@ describe('Operator Fee Tests', () => { ]); }); + it('Reduce fee with a fee thats too low reverts "FeeTooLow"', async () => { + await expect(ssvNetwork.write.reduceOperatorFee([1, 10e6], { account: owners[2].account })).to.be.rejectedWith( + 'FeeTooLow', + ); + }); + it('Reduce fee with an increased value reverts "FeeIncreaseNotAllowed"', async () => { await expect( ssvNetwork.write.reduceOperatorFee([1, initialFee * 2n], { account: owners[2].account }), diff --git a/test/sanity/balances.ts b/test/sanity/balances.ts index cc858d03..8a6c8377 100644 --- a/test/sanity/balances.ts +++ b/test/sanity/balances.ts @@ -293,15 +293,14 @@ describe('Balance Tests', () => { const newNetworkFee = networkFee * 2n; await ssvNetwork.write.updateNetworkFee([newNetworkFee]); - const newBurnPerBlock = (CONFIG.minimalOperatorFee * 4n) + newNetworkFee; + const newBurnPerBlock = CONFIG.minimalOperatorFee * 4n + newNetworkFee; await mine(1); - expect( await ssvViews.read.getBalance([owners[4].account.address, cluster1.operatorIds, cluster1.cluster]), - ).to.equal(minDepositAmount - (burnPerBlock * 2n) - newBurnPerBlock); + ).to.equal(minDepositAmount - burnPerBlock * 2n - newBurnPerBlock); expect((await ssvViews.read.getNetworkEarnings()) - initNetworkFeeBalance).to.equal( - (networkFee * 4n) + (newNetworkFee * 2n), + networkFee * 4n + newNetworkFee * 2n, ); const minDep2 = minDepositAmount * 2n; @@ -314,7 +313,6 @@ describe('Balance Tests', () => { active: true, }); - await mine(2); expect(await ssvViews.read.getOperatorEarnings([1])).to.equal( @@ -327,10 +325,10 @@ describe('Balance Tests', () => { expect(await ssvViews.read.getOperatorEarnings([5])).to.equal(CONFIG.minimalOperatorFee * 2n); expect( await ssvViews.read.getBalance([owners[4].account.address, cluster1.operatorIds, cluster1.cluster]), - ).to.equal(minDepositAmount - (burnPerBlock * 2n) - (newBurnPerBlock * 5n)); + ).to.equal(minDepositAmount - burnPerBlock * 2n - newBurnPerBlock * 5n); expect( await ssvViews.read.getBalance([owners[4].account.address, cluster2.args.operatorIds, cluster2.args.cluster]), - ).to.equal(minDep2 - (newBurnPerBlock * 2n)); + ).to.equal(minDep2 - newBurnPerBlock * 2n); // cold cluster + cluster1 * networkFee (4) + (cold cluster + cluster1 * newNetworkFee (5 + 5)) + cluster2 * newNetworkFee (2) expect((await ssvViews.read.getNetworkEarnings()) - initNetworkFeeBalance).to.equal( @@ -363,20 +361,6 @@ describe('Balance Tests', () => { ); }); - it('Check operator earnings and cluster balance when reducing operator fee"', async () => { - const newFee = CONFIG.minimalOperatorFee / 2n; - await ssvNetwork.write.reduceOperatorFee([1, newFee]); - - await mine(2); - - expect(await ssvViews.read.getOperatorEarnings([1])).to.equal( - CONFIG.minimalOperatorFee * 4n + (CONFIG.minimalOperatorFee + newFee * 2n), - ); - expect( - await ssvViews.read.getBalance([owners[4].account.address, cluster1.operatorIds, cluster1.cluster]), - ).to.equal(minDepositAmount - burnPerBlock - (CONFIG.minimalOperatorFee * 3n + networkFee) * 2n - newFee * 2n); - }); - it('Check cluster balance after withdraw and deposit', async () => { await mine(1); expect( @@ -541,3 +525,52 @@ describe('Balance Tests', () => { ); }); }); + +describe('Balance Tests (reduce fee)', () => { + beforeEach(async () => { + // Initialize contract + const metadata = await initializeContract(); + ssvNetwork = metadata.ssvNetwork; + ssvViews = metadata.ssvNetworkViews; + ssvToken = metadata.ssvToken; + + // Register operators + await registerOperators(0, 14, CONFIG.minimalOperatorFee * 2n); + + networkFee = CONFIG.minimalOperatorFee; + burnPerBlock = CONFIG.minimalOperatorFee * 2n * 4n + networkFee; + minDepositAmount = BigInt(CONFIG.minimalBlocksBeforeLiquidation) * burnPerBlock; + + // Set network fee + await ssvNetwork.write.updateNetworkFee([networkFee]); + + // Register validators + // cold register + await coldRegisterValidator(); + + cluster1 = ( + await bulkRegisterValidators(4, 1, DEFAULT_OPERATOR_IDS[4], minDepositAmount, { + validatorCount: 0, + networkFeeIndex: 0, + index: 0, + balance: 0n, + active: true, + }) + ).args; + }); + + it('Check operator earnings and cluster balance when reducing operator fee"', async () => { + const prevOperatorFee = CONFIG.minimalOperatorFee * 2n; + const newFee = CONFIG.minimalOperatorFee; + await ssvNetwork.write.reduceOperatorFee([1, newFee]); + + await mine(2); + + expect(await ssvViews.read.getOperatorEarnings([1])).to.equal( + prevOperatorFee * 4n + (prevOperatorFee + newFee * 2n), + ); + expect( + await ssvViews.read.getBalance([owners[4].account.address, cluster1.operatorIds, cluster1.cluster]), + ).to.equal(minDepositAmount - burnPerBlock - (prevOperatorFee * 3n + networkFee) * 2n - newFee * 2n); + }); +});