From ca4633aa966f8fe53e827ee854ccf71e868ae75f Mon Sep 17 00:00:00 2001 From: Marco Tabasco Date: Tue, 14 May 2024 11:03:10 +0200 Subject: [PATCH] remove privacy setting when whitelisting and removing opertors --- contracts/libraries/OperatorLib.sol | 30 ++++++++++----------- contracts/modules/SSVOperators.sol | 1 - contracts/modules/SSVOperatorsWhitelist.sol | 6 ++--- contracts/modules/SSVViews.sol | 2 +- test/helpers/gas-usage.ts | 6 ++--- test/validators/whitelist-register.ts | 9 +++++++ 6 files changed, 29 insertions(+), 25 deletions(-) diff --git a/contracts/libraries/OperatorLib.sol b/contracts/libraries/OperatorLib.sol index 20440b13..8356b606 100644 --- a/contracts/libraries/OperatorLib.sol +++ b/contracts/libraries/OperatorLib.sol @@ -147,20 +147,10 @@ library OperatorLib { uint256 addressesLength = whitelistAddresses.length; if (addressesLength == 0) revert ISSVNetworkCore.InvalidWhitelistAddressesLength(); - uint256 operatorsLength = getOperatorsLength(operatorIds); - - ISSVNetworkCore.Operator storage operator; - for (uint256 i; i < operatorsLength; ++i) { - operator = s.operators[operatorIds[i]]; - - checkOwner(operator); - if (registerAddresses && !operator.whitelisted) { - operator.whitelisted = true; - } - } - + checkOperatorsLength(operatorIds); + // create the max number of masks that will be updated - uint256[] memory masks = generateBlockMasks(operatorIds); + uint256[] memory masks = generateBlockMasks(operatorIds, true, s); for (uint256 i; i < addressesLength; ++i) { address whitelistAddress = whitelistAddresses[i]; @@ -183,7 +173,11 @@ library OperatorLib { } } - function generateBlockMasks(uint64[] calldata operatorIds) internal pure returns (uint256[] memory masks) { + function generateBlockMasks( + uint64[] calldata operatorIds, + bool checkOperatorsOwnership, + StorageData storage s + ) internal view returns (uint256[] memory masks) { uint256 blockIndex; uint256 bitPosition; uint64 currentOperatorId; @@ -196,6 +190,10 @@ library OperatorLib { for (uint256 i; i < operatorsLength; ++i) { currentOperatorId = operatorIds[i]; + if (checkOperatorsOwnership) { + checkOwner(s.operators[currentOperatorId]); + } + if (i > 0 && currentOperatorId <= operatorIds[i - 1]) { if (currentOperatorId == operatorIds[i - 1]) { revert ISSVNetworkCore.OperatorsListNotUnique(); @@ -210,7 +208,7 @@ library OperatorLib { } function updatePrivacyStatus(uint64[] calldata operatorIds, bool setPrivate, StorageData storage s) internal { - uint256 operatorsLength = getOperatorsLength(operatorIds); + uint256 operatorsLength = checkOperatorsLength(operatorIds); ISSVNetworkCore.Operator storage operator; for (uint256 i; i < operatorsLength; ++i) { @@ -231,7 +229,7 @@ library OperatorLib { if (whitelistAddress == address(0)) revert ISSVNetworkCore.ZeroAddressNotAllowed(); } - function getOperatorsLength(uint64[] calldata operatorIds) internal pure returns (uint256 operatorsLength) { + function checkOperatorsLength(uint64[] calldata operatorIds) internal pure returns (uint256 operatorsLength) { operatorsLength = operatorIds.length; if (operatorsLength == 0) revert ISSVNetworkCore.InvalidOperatorIdsLength(); } diff --git a/contracts/modules/SSVOperators.sol b/contracts/modules/SSVOperators.sol index 71cff683..d804c03e 100644 --- a/contracts/modules/SSVOperators.sol +++ b/contracts/modules/SSVOperators.sol @@ -63,7 +63,6 @@ contract SSVOperators is ISSVOperators { operator.snapshot.balance = 0; operator.validatorCount = 0; operator.fee = 0; - operator.whitelisted = false; s.operators[operatorId] = operator; diff --git a/contracts/modules/SSVOperatorsWhitelist.sol b/contracts/modules/SSVOperatorsWhitelist.sol index fa2228af..0dafeec3 100644 --- a/contracts/modules/SSVOperatorsWhitelist.sol +++ b/contracts/modules/SSVOperatorsWhitelist.sol @@ -25,8 +25,6 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist { StorageData storage s = SSVStorage.load(); s.operators[operatorId].checkOwner(); - if (!s.operators[operatorId].whitelisted) s.operators[operatorId].whitelisted = true; - // Set the bit at bitPosition for the operatorId in the corresponding uint256 blockIndex (uint256 blockIndex, uint256 bitPosition) = OperatorLib.getBitmapIndexes(operatorId); @@ -59,7 +57,7 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist { if (!OperatorLib.isWhitelistingContract(address(whitelistingContract))) revert InvalidWhitelistingContract(address(whitelistingContract)); - uint256 operatorsLength = OperatorLib.getOperatorsLength(operatorIds); + uint256 operatorsLength = OperatorLib.checkOperatorsLength(operatorIds); StorageData storage s = SSVStorage.load(); Operator storage operator; @@ -86,7 +84,7 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist { } function removeOperatorsWhitelistingContract(uint64[] calldata operatorIds) external { - uint256 operatorsLength = OperatorLib.getOperatorsLength(operatorIds); + uint256 operatorsLength = OperatorLib.checkOperatorsLength(operatorIds); StorageData storage s = SSVStorage.load(); Operator storage operator; diff --git a/contracts/modules/SSVViews.sol b/contracts/modules/SSVViews.sol index 57090706..3ef1de1d 100644 --- a/contracts/modules/SSVViews.sol +++ b/contracts/modules/SSVViews.sol @@ -84,7 +84,7 @@ contract SSVViews is ISSVViews { StorageData storage s = SSVStorage.load(); // create the max number of masks that will be updated - uint256[] memory masks = OperatorLib.generateBlockMasks(operatorIds); + uint256[] memory masks = OperatorLib.generateBlockMasks(operatorIds, false, s); uint256 count; whitelistedOperatorIds = new uint64[](operatorsLength); diff --git a/test/helpers/gas-usage.ts b/test/helpers/gas-usage.ts index c0562f68..9d4029f1 100644 --- a/test/helpers/gas-usage.ts +++ b/test/helpers/gas-usage.ts @@ -99,14 +99,14 @@ const MAX_GAS_PER_GROUP: any = { [GasGroup.REGISTER_OPERATOR]: 134500, [GasGroup.REMOVE_OPERATOR]: 70500, [GasGroup.REMOVE_OPERATOR_WITH_WITHDRAW]: 70500, - [GasGroup.SET_OPERATOR_WHITELIST]: 87000, + [GasGroup.SET_OPERATOR_WHITELIST]: 64000, [GasGroup.SET_OPERATOR_WHITELISTING_CONTRACT]: 95500, [GasGroup.UPDATE_OPERATOR_WHITELISTING_CONTRACT]: 70000, [GasGroup.SET_OPERATOR_WHITELISTING_CONTRACT_10]: 375000, [GasGroup.REMOVE_OPERATOR_WHITELISTING_CONTRACT]: 43000, [GasGroup.REMOVE_OPERATOR_WHITELISTING_CONTRACT_10]: 130000, - [GasGroup.SET_MULTIPLE_OPERATOR_WHITELIST_10_10]: 590000, - [GasGroup.REMOVE_MULTIPLE_OPERATOR_WHITELIST_10_10]: 173500, + [GasGroup.SET_MULTIPLE_OPERATOR_WHITELIST_10_10]: 382000, + [GasGroup.REMOVE_MULTIPLE_OPERATOR_WHITELIST_10_10]: 169000, [GasGroup.SET_OPERATORS_PRIVATE_10]: 313000, [GasGroup.SET_OPERATORS_PUBLIC_10]: 114000, diff --git a/test/validators/whitelist-register.ts b/test/validators/whitelist-register.ts index 0ec3c508..727b1f61 100644 --- a/test/validators/whitelist-register.ts +++ b/test/validators/whitelist-register.ts @@ -48,6 +48,9 @@ describe('Register Validator Tests', () => { await ssvNetwork.write.setOperatorWhitelist([operatorId, owners[3].account.address], { account: owners[1].account, }); + await ssvNetwork.write.setOperatorsPrivateUnchecked([[operatorId]], { + account: owners[1].account, + }); await ssvToken.write.approve([ssvNetwork.address, minDepositAmount], { account: owners[3].account }); @@ -203,6 +206,10 @@ describe('Register Validator Tests', () => { account: owners[0].account, }); + await ssvNetwork.write.setOperatorsPrivateUnchecked([[3]], { + account: owners[0].account, + }); + await expect( ssvNetwork.write.registerValidator( [ @@ -567,6 +574,8 @@ describe('Register Validator Tests', () => { await ssvNetwork.write.setOperatorMultipleWhitelists([[6], [owners[3].account.address]]); + await ssvNetwork.write.setOperatorsPrivateUnchecked([[6]]); + await ssvNetwork.write.registerValidator( [ DataGenerator.publicKey(1),