From d078a872be4124bdd36a9112eded201875afa571 Mon Sep 17 00:00:00 2001 From: Marco Tabasco Date: Thu, 6 Jun 2024 08:16:38 +0200 Subject: [PATCH] registerOperator can set privacy status --- contracts/SSVNetwork.sol | 2 +- contracts/interfaces/ISSVOperators.sol | 3 +- contracts/modules/SSVOperators.sol | 12 +- contracts/test/SSVNetworkUpgrade.sol | 4 +- contracts/test/mocks/BeneficiaryContract.sol | 2 +- contracts/test/modules/SSVOperatorsUpdate.sol | 12 +- test/deployment/deploy.ts | 2 +- test/helpers/contract-helpers.ts | 2 +- test/helpers/gas-usage.ts | 2 +- test/operators/register.ts | 107 +++++++++++++++--- test/operators/remove.ts | 4 +- test/operators/update-fee.ts | 4 +- test/operators/whitelist.ts | 12 +- test/validators/whitelist-register.ts | 4 +- 14 files changed, 134 insertions(+), 38 deletions(-) diff --git a/contracts/SSVNetwork.sol b/contracts/SSVNetwork.sol index 36454bf3..cb95e9a1 100644 --- a/contracts/SSVNetwork.sol +++ b/contracts/SSVNetwork.sol @@ -121,7 +121,7 @@ contract SSVNetwork is /* Operator External Functions */ /*******************************/ - function registerOperator(bytes calldata publicKey, uint256 fee) external override returns (uint64 id) { + function registerOperator(bytes calldata publicKey, uint256 fee, bool setPrivate) external override returns (uint64 id) { _delegate(SSVStorage.load().ssvContracts[SSVModules.SSV_OPERATORS]); } diff --git a/contracts/interfaces/ISSVOperators.sol b/contracts/interfaces/ISSVOperators.sol index 537e2ba5..b236db69 100644 --- a/contracts/interfaces/ISSVOperators.sol +++ b/contracts/interfaces/ISSVOperators.sol @@ -7,7 +7,8 @@ interface ISSVOperators is ISSVNetworkCore { /// @notice Registers a new operator /// @param publicKey The public key of the operator /// @param fee The operator's fee (SSV) - function registerOperator(bytes calldata publicKey, uint256 fee) external returns (uint64); + /// @param setPrivate Flag indicating whether the operator should be set as private or not + function registerOperator(bytes calldata publicKey, uint256 fee, bool setPrivate) external returns (uint64); /// @notice Removes an existing operator /// @param operatorId The ID of the operator to be removed diff --git a/contracts/modules/SSVOperators.sol b/contracts/modules/SSVOperators.sol index 5bfc0a1e..4cbcf335 100644 --- a/contracts/modules/SSVOperators.sol +++ b/contracts/modules/SSVOperators.sol @@ -23,7 +23,11 @@ contract SSVOperators is ISSVOperators { /* Operator External Functions */ /*******************************/ - function registerOperator(bytes calldata publicKey, uint256 fee) external override returns (uint64 id) { + function registerOperator( + bytes calldata publicKey, + uint256 fee, + bool setPrivate + ) external override returns (uint64 id) { if (fee != 0 && fee < MINIMAL_OPERATOR_FEE) { revert ISSVNetworkCore.FeeTooLow(); } @@ -43,11 +47,15 @@ contract SSVOperators is ISSVOperators { snapshot: ISSVNetworkCore.Snapshot({block: uint32(block.number), index: 0, balance: 0}), validatorCount: 0, fee: fee.shrink(), - whitelisted: false + whitelisted: setPrivate }); s.operatorsPKs[hashedPk] = id; + uint64[] memory operatorIds = new uint64[](1); + operatorIds[0] = id; + emit OperatorAdded(id, msg.sender, publicKey, fee); + emit OperatorPrivacyStatusUpdated(operatorIds, setPrivate); } function removeOperator(uint64 operatorId) external override { diff --git a/contracts/test/SSVNetworkUpgrade.sol b/contracts/test/SSVNetworkUpgrade.sol index 71e346c5..b6742a0d 100644 --- a/contracts/test/SSVNetworkUpgrade.sol +++ b/contracts/test/SSVNetworkUpgrade.sol @@ -117,10 +117,10 @@ contract SSVNetworkUpgrade is /* Operator External Functions */ /*******************************/ - function registerOperator(bytes calldata publicKey, uint256 fee) external override returns (uint64 id) { + function registerOperator(bytes calldata publicKey, uint256 fee, bool setPrivate) external override returns (uint64 id) { bytes memory result = _delegateCall( SSVStorage.load().ssvContracts[SSVModules.SSV_OPERATORS], - abi.encodeWithSignature("registerOperator(bytes,uint256)", publicKey, fee) + abi.encodeWithSignature("registerOperator(bytes,uint256)", publicKey, fee, setPrivate) ); return abi.decode(result, (uint64)); } diff --git a/contracts/test/mocks/BeneficiaryContract.sol b/contracts/test/mocks/BeneficiaryContract.sol index 8efcbc68..c7e92d0f 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); + return ISSVOperators(ssvContract).registerOperator("0xcafecafe", 100000000, false); } } diff --git a/contracts/test/modules/SSVOperatorsUpdate.sol b/contracts/test/modules/SSVOperatorsUpdate.sol index 45ed79c9..d8f3d99f 100644 --- a/contracts/test/modules/SSVOperatorsUpdate.sol +++ b/contracts/test/modules/SSVOperatorsUpdate.sol @@ -23,7 +23,11 @@ contract SSVOperatorsUpdate is ISSVOperators { /* Operator External Functions */ /*******************************/ - function registerOperator(bytes calldata publicKey, uint256 fee) external override returns (uint64 id) { + function registerOperator( + bytes calldata publicKey, + uint256 fee, + bool setPrivate + ) external override returns (uint64 id) { if (fee != 0 && fee < MINIMAL_OPERATOR_FEE) { revert ISSVNetworkCore.FeeTooLow(); } @@ -39,11 +43,15 @@ contract SSVOperatorsUpdate is ISSVOperators { snapshot: ISSVNetworkCore.Snapshot({block: uint32(block.number), index: 0, balance: 0}), validatorCount: 0, fee: fee.shrink(), - whitelisted: false + whitelisted: setPrivate }); s.operatorsPKs[hashedPk] = id; + uint64[] memory operatorIds = new uint64[](1); + operatorIds[0] = id; + emit OperatorAdded(id, msg.sender, publicKey, fee); + emit OperatorPrivacyStatusUpdated(operatorIds, setPrivate); } function removeOperator(uint64 operatorId) external override { diff --git a/test/deployment/deploy.ts b/test/deployment/deploy.ts index febffe1f..569b3e67 100644 --- a/test/deployment/deploy.ts +++ b/test/deployment/deploy.ts @@ -38,7 +38,7 @@ describe('Deployment tests', () => { }); it('Upgrade SSVNetwork contract. Check new function execution', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { account: owners[1].account, }); diff --git a/test/helpers/contract-helpers.ts b/test/helpers/contract-helpers.ts index 2a20b0f0..1351d6fa 100644 --- a/test/helpers/contract-helpers.ts +++ b/test/helpers/contract-helpers.ts @@ -176,7 +176,7 @@ export const registerOperators = async function ( operator.publicKey = keccak256(toBytes(operator.operatorKey)); const { eventsByName } = await trackGas( - ssvNetwork.write.registerOperator([operator.publicKey, fee], { + ssvNetwork.write.registerOperator([operator.publicKey, fee, false], { account: owners[ownerId].account, }), gasGroups, diff --git a/test/helpers/gas-usage.ts b/test/helpers/gas-usage.ts index 32d05d2d..adae4eb6 100644 --- a/test/helpers/gas-usage.ts +++ b/test/helpers/gas-usage.ts @@ -95,7 +95,7 @@ export enum GasGroup { const MAX_GAS_PER_GROUP: any = { /* REAL GAS LIMITS */ - [GasGroup.REGISTER_OPERATOR]: 134500, + [GasGroup.REGISTER_OPERATOR]: 137000, [GasGroup.REMOVE_OPERATOR]: 70500, [GasGroup.REMOVE_OPERATOR_WITH_WITHDRAW]: 70500, [GasGroup.SET_OPERATOR_WHITELISTING_CONTRACT]: 95500, diff --git a/test/operators/register.ts b/test/operators/register.ts index 872ac7ce..99fa82c6 100644 --- a/test/operators/register.ts +++ b/test/operators/register.ts @@ -17,11 +17,11 @@ describe('Register Operator Tests', () => { ssvViews = metadata.ssvNetworkViews; }); - it('Register operator emits "OperatorAdded"', async () => { + it('Register operator emits "OperatorAdded" and "OperatorPrivacyStatusUpdated" if setPrivate is true', async () => { const publicKey = DataGenerator.publicKey(0); await assertEvent( - ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee], { + ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee, true], { account: owners[1].account, }), [ @@ -31,21 +31,58 @@ describe('Register Operator Tests', () => { argNames: ['operatorId', 'owner', 'publicKey', 'fee'], argValuesList: [[1, owners[1].account.address, publicKey, CONFIG.minimalOperatorFee]], }, + { + contract: ssvNetwork, + eventName: 'OperatorPrivacyStatusUpdated', + argNames: ['operatorIds', 'toPrivate'], + argValuesList: [[[1], true]], + }, + ], + ); + }); + + it('Register operator emits "OperatorAdded" and "OperatorPrivacyStatusUpdated" if setPrivate is false', async () => { + const publicKey = DataGenerator.publicKey(0); + + await assertEvent( + ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee, false], { + account: owners[1].account, + }), + [ + { + contract: ssvNetwork, + eventName: 'OperatorAdded', + argNames: ['operatorId', 'owner', 'publicKey', 'fee'], + argValuesList: [[1, owners[1].account.address, publicKey, CONFIG.minimalOperatorFee]], + }, + { + contract: ssvNetwork, + eventName: 'OperatorPrivacyStatusUpdated', + argNames: ['operatorIds', 'toPrivate'], + argValuesList: [[[1], false]], + }, ], ); }); it('Register operator gas limits', async () => { await trackGas( - ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { + account: owners[1].account, + }), + [GasGroup.REGISTER_OPERATOR], + ); + + await trackGas( + ssvNetwork.write.registerOperator([DataGenerator.publicKey(1), CONFIG.minimalOperatorFee, true], { account: owners[1].account, }), [GasGroup.REGISTER_OPERATOR], ); }); - it('Get operator by id', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + it('Get operator by id with setPrivate false', async () => { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { account: owners[1].account, }); @@ -59,8 +96,23 @@ describe('Register Operator Tests', () => { ]); }); + it('Get operator by id with setPrivate true', async () => { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, true], { + account: owners[1].account, + }); + + expect(await ssvViews.read.getOperatorById([1])).to.deep.equal([ + owners[1].account.address, // owner + CONFIG.minimalOperatorFee, // fee + 0, // validatorCount + ethers.ZeroAddress, // whitelisting contract address + true, // isPrivate + true, // active + ]); + }); + it('Get non-existent operator by id', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { account: owners[1].account, }); @@ -75,7 +127,7 @@ describe('Register Operator Tests', () => { }); it('Get operator removed by id', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { account: owners[1].account, }); await ssvNetwork.write.removeOperator([1], { @@ -92,24 +144,51 @@ describe('Register Operator Tests', () => { ]); }); - it('Register an operator with a fee thats too low reverts "FeeTooLow"', async () => { - await expect(ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), '10'])).to.be.rejectedWith('FeeTooLow'); + it('Register an operator with a fee thats too low reverts "FeeTooLow", setPrivate false', async () => { + await expect(ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), '10', false])).to.be.rejectedWith( + 'FeeTooLow', + ); + }); + + it('Register an operator with a fee thats too low reverts "FeeTooLow", setPrivate true', async () => { + await expect(ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), '10', true])).to.be.rejectedWith( + 'FeeTooLow', + ); }); - it('Register an operator with a fee thats too high reverts "FeeTooHigh"', async () => { - await expect(ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), 2e14])).to.be.rejectedWith( + it('Register an operator with a fee thats too high reverts "FeeTooHigh", setPrivate false', async () => { + await expect(ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), 2e14, false])).to.be.rejectedWith( 'FeeTooHigh', ); }); - it('Register same operator twice reverts "OperatorAlreadyExists"', async () => { + it('Register an operator with a fee thats too high reverts "FeeTooHigh", setPrivate false', async () => { + await expect(ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), 2e14, true])).to.be.rejectedWith( + 'FeeTooHigh', + ); + }); + + it('Register same operator twice reverts "OperatorAlreadyExists", setPrivate false', async () => { + const publicKey = DataGenerator.publicKey(1); + await ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee, false], { + account: owners[1].account, + }); + + await expect( + ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee, false], { + account: owners[1].account, + }), + ).to.be.rejectedWith('OperatorAlreadyExists'); + }); + + it('Register same operator twice reverts "OperatorAlreadyExists", setPrivate true', async () => { const publicKey = DataGenerator.publicKey(1); - await ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee, true], { account: owners[1].account, }); await expect( - ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee], { + ssvNetwork.write.registerOperator([publicKey, CONFIG.minimalOperatorFee, true], { account: owners[1].account, }), ).to.be.rejectedWith('OperatorAlreadyExists'); diff --git a/test/operators/remove.ts b/test/operators/remove.ts index 17f2806b..ea9cd709 100644 --- a/test/operators/remove.ts +++ b/test/operators/remove.ts @@ -46,7 +46,7 @@ describe('Remove Operator Tests', () => { it('Remove private operator emits "OperatorRemoved"', async () => { const result = await trackGas( - ssvNetwork.write.registerOperator([DataGenerator.publicKey(22), CONFIG.minimalOperatorFee]), + ssvNetwork.write.registerOperator([DataGenerator.publicKey(22), CONFIG.minimalOperatorFee, true]), ); const { operatorId } = result.eventsByName.OperatorAdded[0].args; @@ -66,7 +66,7 @@ describe('Remove Operator Tests', () => { 0, // fee 0, // validatorCount ethers.ZeroAddress, // whitelisting contract address - false, // isPrivate + true, // isPrivate false, // active ]); }); diff --git a/test/operators/update-fee.ts b/test/operators/update-fee.ts index 3ba1534e..5f4efb0f 100644 --- a/test/operators/update-fee.ts +++ b/test/operators/update-fee.ts @@ -193,7 +193,7 @@ describe('Operator Fee Tests', () => { }); it('Declare fee after registering an operator with zero fee reverts "FeeIncreaseNotAllowed"', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(2), 0], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(2), 0, false], { account: owners[2].account, }); @@ -218,7 +218,7 @@ describe('Operator Fee Tests', () => { const maxOperatorFee = 8e14; await ssvNetwork.write.updateMaximumOperatorFee([maxOperatorFee]); - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(10), maxOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(10), maxOperatorFee, false], { account: owners[3].account, }); diff --git a/test/operators/whitelist.ts b/test/operators/whitelist.ts index 5d8800e2..2d55ad4f 100644 --- a/test/operators/whitelist.ts +++ b/test/operators/whitelist.ts @@ -27,7 +27,7 @@ describe('Whitelisting Operator Tests', () => { /* GAS LIMITS */ it('Set operator whitelisting contract (1 operator) gas limits', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, true], { account: owners[1].account, }); await trackGas( @@ -39,7 +39,7 @@ describe('Whitelisting Operator Tests', () => { }); it('Update operator whitelisting contract (1 operator) gas limits', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, true], { account: owners[1].account, }); @@ -75,7 +75,7 @@ describe('Whitelisting Operator Tests', () => { }); it('Remove operator whitelisting contract (1 operator) gas limits', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, true], { account: owners[1].account, }); @@ -116,7 +116,7 @@ describe('Whitelisting Operator Tests', () => { account: owners[1].account, }), [GasGroup.SET_MULTIPLE_OPERATOR_WHITELIST_10_10], - );ƒ + ); }); it('Remove 10 whitelist addresses (EOAs) for 10 operators gas limits', async () => { @@ -547,7 +547,7 @@ describe('Whitelisting Operator Tests', () => { }); it('Get private operator by id', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { account: owners[1].account, }); @@ -570,7 +570,7 @@ describe('Whitelisting Operator Tests', () => { }); it('Get removed private operator by id', async () => { - await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee], { + await ssvNetwork.write.registerOperator([DataGenerator.publicKey(0), CONFIG.minimalOperatorFee, false], { account: owners[1].account, }); diff --git a/test/validators/whitelist-register.ts b/test/validators/whitelist-register.ts index bd224c70..27c21ad0 100644 --- a/test/validators/whitelist-register.ts +++ b/test/validators/whitelist-register.ts @@ -356,11 +356,11 @@ describe('Register Validator Tests', () => { address: await ssvNetwork.address, abi: ssvNetwork.abi, functionName: 'registerOperator', - args: ['0xabcd', CONFIG.minimalOperatorFee], + args: ['0xabcd', CONFIG.minimalOperatorFee, true], account: owners[0].account, }); - await ssvNetwork.write.registerOperator(['0xabcd', CONFIG.minimalOperatorFee]); + await ssvNetwork.write.registerOperator(['0xabcd', CONFIG.minimalOperatorFee, true]); // Whitelist the new operator with the attacker contract await ssvNetwork.write.setOperatorsWhitelistingContract( [[goodOperatorId], await badOperatorWhitelistingContract.address],