diff --git a/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol b/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol index 5636c8a0a0..977c7764c5 100644 --- a/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol +++ b/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol @@ -156,12 +156,9 @@ abstract contract ValidatorRegistrator is Governable, Pausable { /// The `ValidatorStakeData` struct contains the pubkey, signature and depositDataRoot. /// Only the registrator can call this function. // slither-disable-start reentrancy-eth - function stakeEth(ValidatorStakeData[] calldata validators) - external - onlyRegistrator - whenNotPaused - nonReentrant - { + function stakeEth( + ValidatorStakeData[] calldata validators + ) external onlyRegistrator whenNotPaused nonReentrant { uint256 requiredETH = validators.length * FULL_STAKE; // Check there is enough WETH from the deposits sitting in this strategy contract @@ -267,66 +264,87 @@ abstract contract ValidatorRegistrator is Governable, Pausable { ); } - // slither-disable-end reentrancy-no-eth - - /// @notice Exit a validator from the Beacon chain. + /// @notice Exit validators from the Beacon chain. /// The staked ETH will eventually swept to this native staking strategy. /// Only the registrator can call this function. - /// @param publicKey The public key of the validator + /// @param publicKeys List of SSV validator public keys /// @param operatorIds The operator IDs of the SSV Cluster // slither-disable-start reentrancy-no-eth - function exitSsvValidator( - bytes calldata publicKey, + function exitSsvValidators( + bytes[] calldata publicKeys, uint64[] calldata operatorIds - ) external onlyRegistrator whenNotPaused { - bytes32 pubKeyHash = keccak256(publicKey); - VALIDATOR_STATE currentState = validatorsStates[pubKeyHash]; - require(currentState == VALIDATOR_STATE.STAKED, "Validator not staked"); + ) external onlyRegistrator whenNotPaused nonReentrant { + ISSVNetwork(SSV_NETWORK).bulkExitValidator(publicKeys, operatorIds); - ISSVNetwork(SSV_NETWORK).exitValidator(publicKey, operatorIds); + bytes32 pubKeyHash; + VALIDATOR_STATE currentState; + for (uint256 i = 0; i < publicKeys.length; ++i) { + pubKeyHash = keccak256(publicKeys[i]); + currentState = validatorsStates[pubKeyHash]; - validatorsStates[pubKeyHash] = VALIDATOR_STATE.EXITING; + // Check each validator has not already been staked. + // This would normally be done before the external call but is after + // so only one for loop of the validators is needed. + require( + currentState == VALIDATOR_STATE.STAKED, + "Validator not staked" + ); - emit SSVValidatorExitInitiated(pubKeyHash, publicKey, operatorIds); - } + // Store the new validator state + validatorsStates[pubKeyHash] = VALIDATOR_STATE.EXITING; - // slither-disable-end reentrancy-no-eth + emit SSVValidatorExitInitiated( + pubKeyHash, + publicKeys[i], + operatorIds + ); + } + } - /// @notice Remove a validator from the SSV Cluster. + /// @notice Remove validators from the SSV Cluster. /// Make sure `exitSsvValidator` is called before and the validate has exited the Beacon chain. /// If removed before the validator has exited the beacon chain will result in the validator being slashed. /// Only the registrator can call this function. - /// @param publicKey The public key of the validator + /// @param publicKeys List of SSV validator public keys /// @param operatorIds The operator IDs of the SSV Cluster /// @param cluster The SSV cluster details including the validator count and SSV balance - // slither-disable-start reentrancy-no-eth - function removeSsvValidator( - bytes calldata publicKey, + function removeSsvValidators( + bytes[] calldata publicKeys, uint64[] calldata operatorIds, Cluster calldata cluster - ) external onlyRegistrator whenNotPaused { - bytes32 pubKeyHash = keccak256(publicKey); - VALIDATOR_STATE currentState = validatorsStates[pubKeyHash]; - // Can remove SSV validators that were incorrectly registered and can not be deposited to. - require( - currentState == VALIDATOR_STATE.EXITING || - currentState == VALIDATOR_STATE.REGISTERED, - "Validator not regd or exiting" - ); - - ISSVNetwork(SSV_NETWORK).removeValidator( - publicKey, + ) external onlyRegistrator whenNotPaused nonReentrant { + ISSVNetwork(SSV_NETWORK).bulkRemoveValidator( + publicKeys, operatorIds, cluster ); - validatorsStates[pubKeyHash] = VALIDATOR_STATE.EXIT_COMPLETE; + bytes32 pubKeyHash; + VALIDATOR_STATE currentState; + for (uint256 i = 0; i < publicKeys.length; ++i) { + pubKeyHash = keccak256(publicKeys[i]); + currentState = validatorsStates[pubKeyHash]; + + // Check each validator is either registered or exited. + // This would normally be done before the external call but is after + // so only one for loop of the validators is needed. + require( + currentState == VALIDATOR_STATE.EXITING || + currentState == VALIDATOR_STATE.REGISTERED, + "Validator not regd or exiting" + ); + + // Store the new validator state + validatorsStates[pubKeyHash] = VALIDATOR_STATE.EXIT_COMPLETE; - emit SSVValidatorExitCompleted(pubKeyHash, publicKey, operatorIds); + emit SSVValidatorExitCompleted( + pubKeyHash, + publicKeys[i], + operatorIds + ); + } } - // slither-disable-end reentrancy-no-eth - /// @notice Deposits more SSV Tokens to the SSV Network contract which is used to pay the SSV Operators. /// @dev A SSV cluster is defined by the SSVOwnerAddress and the set of operatorIds. /// uses "onlyStrategist" modifier so continuous front-running can't DOS our maintenance service diff --git a/contracts/deploy/holesky/019_upgrade_strategy.js b/contracts/deploy/holesky/019_upgrade_strategy.js new file mode 100644 index 0000000000..9f514276ac --- /dev/null +++ b/contracts/deploy/holesky/019_upgrade_strategy.js @@ -0,0 +1,17 @@ +const { upgradeNativeStakingSSVStrategy } = require("../deployActions"); + +const mainExport = async () => { + console.log("Running 019 deployment on Holesky..."); + + await upgradeNativeStakingSSVStrategy(); + + console.log("Running 019 deployment done"); + return true; +}; + +mainExport.id = "019_upgrade_strategy"; +mainExport.tags = []; +mainExport.dependencies = []; +mainExport.skip = () => false; + +module.exports = mainExport; diff --git a/contracts/docs/NativeStakingSSVStrategyHierarchy.svg b/contracts/docs/NativeStakingSSVStrategyHierarchy.svg index 68f386df5c..c270349d0f 100644 --- a/contracts/docs/NativeStakingSSVStrategyHierarchy.svg +++ b/contracts/docs/NativeStakingSSVStrategyHierarchy.svg @@ -16,119 +16,119 @@ Governable ../contracts/governance/Governable.sol - + -286 +299 FeeAccumulator ../contracts/strategies/NativeStaking/FeeAccumulator.sol - + -288 +301 NativeStakingSSVStrategy ../contracts/strategies/NativeStaking/NativeStakingSSVStrategy.sol - + -288->286 +301->299 - + -289 +302 <<Abstract>> ValidatorAccountant ../contracts/strategies/NativeStaking/ValidatorAccountant.sol - + -288->289 +301->302 - + -217 +226 <<Abstract>> InitializableAbstractStrategy ../contracts/utils/InitializableAbstractStrategy.sol - + -288->217 +301->226 - + -291 +304 <<Abstract>> ValidatorRegistrator ../contracts/strategies/NativeStaking/ValidatorRegistrator.sol - + -289->291 +302->304 - + -291->20 +304->20 - + -348 +361 <<Abstract>> Pausable ../node_modules/@openzeppelin/contracts/security/Pausable.sol - + -291->348 +304->361 - + -216 +225 <<Abstract>> Initializable ../contracts/utils/Initializable.sol - + -217->20 +226->20 - + -217->216 +226->225 - + -353 +366 <<Abstract>> Context ../node_modules/@openzeppelin/contracts/utils/Context.sol - + -348->353 +361->366 diff --git a/contracts/docs/NativeStakingSSVStrategySquashed.svg b/contracts/docs/NativeStakingSSVStrategySquashed.svg index c4ba4dcb15..35836684bf 100644 --- a/contracts/docs/NativeStakingSSVStrategySquashed.svg +++ b/contracts/docs/NativeStakingSSVStrategySquashed.svg @@ -93,8 +93,8 @@    resetStakeETHTally() <<onlyStakingMonitor>> <<ValidatorRegistrator>>    stakeEth(validators: ValidatorStakeData[]) <<onlyRegistrator, whenNotPaused, nonReentrant>> <<ValidatorRegistrator>>    registerSsvValidators(publicKeys: bytes[], operatorIds: uint64[], sharesData: bytes[], ssvAmount: uint256, cluster: Cluster) <<onlyRegistrator, whenNotPaused>> <<ValidatorRegistrator>> -    exitSsvValidator(publicKey: bytes, operatorIds: uint64[]) <<onlyRegistrator, whenNotPaused>> <<ValidatorRegistrator>> -    removeSsvValidator(publicKey: bytes, operatorIds: uint64[], cluster: Cluster) <<onlyRegistrator, whenNotPaused>> <<ValidatorRegistrator>> +    exitSsvValidators(publicKeys: bytes[], operatorIds: uint64[]) <<onlyRegistrator, whenNotPaused, nonReentrant>> <<ValidatorRegistrator>> +    removeSsvValidators(publicKeys: bytes[], operatorIds: uint64[], cluster: Cluster) <<onlyRegistrator, whenNotPaused, nonReentrant>> <<ValidatorRegistrator>>    depositSSV(operatorIds: uint64[], ssvAmount: uint256, cluster: Cluster) <<onlyStrategist>> <<ValidatorRegistrator>>    setFuseInterval(_fuseIntervalStart: uint256, _fuseIntervalEnd: uint256) <<onlyGovernor>> <<ValidatorAccountant>>    doAccounting(): (accountingValid: bool) <<onlyRegistrator, whenNotPaused, nonReentrant>> <<ValidatorAccountant>> diff --git a/contracts/tasks/ssv.js b/contracts/tasks/ssv.js index e1c69916a8..2e5bb68383 100644 --- a/contracts/tasks/ssv.js +++ b/contracts/tasks/ssv.js @@ -1,4 +1,9 @@ -const { parseUnits, formatUnits, solidityPack } = require("ethers/lib/utils"); +const { + parseUnits, + formatUnits, + solidityPack, + hexlify, +} = require("ethers/lib/utils"); const addresses = require("../utils/addresses"); const { resolveContract } = require("../utils/resolvers"); @@ -7,11 +12,10 @@ const { getClusterInfo } = require("../utils/ssv"); const { networkMap } = require("../utils/hardhat-helpers"); const { logTxDetails } = require("../utils/txLogger"); const { resolveNativeStakingStrategyProxy } = require("./validator"); -const { checkPubkeyFormat } = require("./taskUtils"); const log = require("../utils/logger")("task:ssv"); -async function removeValidator({ index, pubkey, operatorids }) { +async function removeValidators({ index, pubkeys, operatorids }) { const signer = await getSigner(); log(`Splitting operator IDs ${operatorids}`); @@ -29,12 +33,14 @@ async function removeValidator({ index, pubkey, operatorids }) { ownerAddress: strategy.address, }); - log(`About to remove validator`); - pubkey = checkPubkeyFormat(pubkey); + log(`Splitting public keys ${pubkeys}`); + const pubKeys = pubkeys.split(",").map((pubkey) => hexlify(pubkey)); + + log(`About to remove validators: ${pubKeys}`); const tx = await strategy .connect(signer) - .removeSsvValidator(pubkey, operatorIds, cluster); - await logTxDetails(tx, "removeSsvValidator"); + .removeSsvValidators(pubKeys, operatorIds, cluster); + await logTxDetails(tx, "removeSsvValidators"); } const printClusterInfo = async (options) => { @@ -121,5 +127,5 @@ module.exports = { printClusterInfo, depositSSV, calcDepositRoot, - removeValidator, + removeValidators, }; diff --git a/contracts/tasks/taskUtils.js b/contracts/tasks/taskUtils.js deleted file mode 100644 index 5dfa5e519c..0000000000 --- a/contracts/tasks/taskUtils.js +++ /dev/null @@ -1,14 +0,0 @@ -/** - * A separate file for small utils other files can use. The purpose of this being separate is also to not mess up the - * dependency graph too much, since there is a source file limit of 5MB on Defender actions - */ -const checkPubkeyFormat = (pubkey) => { - if (!pubkey.startsWith("0x")) { - pubkey = `0x${pubkey}`; - } - return pubkey; -}; - -module.exports = { - checkPubkeyFormat, -}; diff --git a/contracts/tasks/tasks.js b/contracts/tasks/tasks.js index 2d66cd85d1..caefc814e7 100644 --- a/contracts/tasks/tasks.js +++ b/contracts/tasks/tasks.js @@ -69,7 +69,7 @@ const { calcDepositRoot, depositSSV, printClusterInfo, - removeValidator, + removeValidators, } = require("./ssv"); const { amoStrategyTask, @@ -90,7 +90,7 @@ const { } = require("./strategy"); const { validatorOperationsConfig, - exitValidator, + exitValidators, doAccounting, resetStakeETHTally, setStakeETHThreshold, @@ -1218,10 +1218,10 @@ task("stakeValidators").setAction(async (_, __, runSuper) => { return runSuper(); }); -subtask("exitValidator", "Starts the exit process from a validator") +subtask("exitValidators", "Starts the exit process from validators") .addParam( - "pubkey", - "Public key of the validator to exit", + "pubkeys", + "Comma separated validator public keys", undefined, types.string ) @@ -1239,15 +1239,15 @@ subtask("exitValidator", "Starts the exit process from a validator") ) .setAction(async (taskArgs) => { const signer = await getSigner(); - await exitValidator({ ...taskArgs, signer }); + await exitValidators({ ...taskArgs, signer }); }); -task("exitValidator").setAction(async (_, __, runSuper) => { +task("exitValidators").setAction(async (_, __, runSuper) => { return runSuper(); }); subtask( - "removeValidator", - "Removes a validator from the SSV cluster after it has exited the beacon chain" + "removeValidators", + "Removes validators from the SSV cluster after they have exited the beacon chain" ) .addOptionalParam( "index", @@ -1256,8 +1256,8 @@ subtask( types.int ) .addParam( - "pubkey", - "Public key of the validator to exit", + "pubkeys", + "Comma separated validator public keys", undefined, types.string ) @@ -1269,9 +1269,9 @@ subtask( ) .setAction(async (taskArgs) => { const signer = await getSigner(); - await removeValidator({ ...taskArgs, signer }); + await removeValidators({ ...taskArgs, signer }); }); -task("removeValidator").setAction(async (_, __, runSuper) => { +task("removeValidators").setAction(async (_, __, runSuper) => { return runSuper(); }); diff --git a/contracts/tasks/validator.js b/contracts/tasks/validator.js index cad5bf8312..39fe466e04 100644 --- a/contracts/tasks/validator.js +++ b/contracts/tasks/validator.js @@ -1,10 +1,9 @@ -const { formatUnits, parseEther } = require("ethers").utils; +const { formatUnits, hexlify, parseEther } = require("ethers").utils; const { KeyValueStoreClient, } = require("@openzeppelin/defender-kvstore-client"); const { getBlock } = require("./block"); -const { checkPubkeyFormat } = require("./taskUtils"); const { getValidator, getEpoch } = require("./beaconchain"); const addresses = require("../utils/addresses"); const { resolveContract } = require("../utils/resolvers"); @@ -97,34 +96,40 @@ const validatorOperationsConfig = async (taskArgs) => { // @dev check validator is eligible for exit - // has been active for at least 256 epochs -async function verifyMinActivationTime({ pubkey }) { +async function verifyMinActivationTimes({ pubkeys }) { const latestEpoch = await getEpoch("latest"); - const validator = await getValidator(pubkey); - const epochDiff = latestEpoch.epoch - validator.activationepoch; + log(`Splitting public keys ${pubkeys}`); + const pubKeys = pubkeys.split(",").map((id) => hexlify(id)); - if (epochDiff < 256) { - throw new Error( - `Can not exit validator. Validator needs to be ` + - `active for 256 epoch. Current one active for ${epochDiff}` - ); + for (const pubkey of pubKeys) { + const validator = await getValidator(pubkey); + + const epochDiff = latestEpoch.epoch - validator.activationepoch; + + if (epochDiff < 256) { + throw new Error( + `Can not exit validator. Validator needs to be ` + + `active for 256 epoch. ${pubkey} has only been active for ${epochDiff}` + ); + } } } -async function exitValidator({ index, pubkey, operatorids, signer }) { - await verifyMinActivationTime({ pubkey }); +async function exitValidators({ index, pubkeys, operatorids, signer }) { + await verifyMinActivationTimes({ pubkeys }); log(`Splitting operator IDs ${operatorids}`); const operatorIds = operatorids.split(",").map((id) => parseInt(id)); const strategy = await resolveNativeStakingStrategyProxy(index); - log(`About to exit validator`); - pubkey = checkPubkeyFormat(pubkey); + const pubKeys = pubkeys.split(",").map((pubkey) => hexlify(pubkey)); + log(`About to exit validators: ${pubKeys}`); const tx = await strategy .connect(signer) - .exitSsvValidator(pubkey, operatorIds); + .exitSsvValidators(pubKeys, operatorIds); await logTxDetails(tx, "exitSsvValidator"); } @@ -297,7 +302,7 @@ const resolveFeeAccumulatorProxy = async (index) => { module.exports = { validatorOperationsConfig, - exitValidator, + exitValidators, doAccounting, resetStakeETHTally, setStakeETHThreshold, diff --git a/contracts/test/behaviour/ssvStrategy.js b/contracts/test/behaviour/ssvStrategy.js index 260aad7594..ec8dbcc684 100644 --- a/contracts/test/behaviour/ssvStrategy.js +++ b/contracts/test/behaviour/ssvStrategy.js @@ -434,7 +434,10 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { // exit validator from SSV network const exitTx = await nativeStakingSSVStrategy .connect(validatorRegistrator) - .exitSsvValidator(testValidator.publicKey, testValidator.operatorIds); + .exitSsvValidators( + [testValidator.publicKey], + testValidator.operatorIds + ); await expect(exitTx) .to.emit(nativeStakingSSVStrategy, "SSVValidatorExitInitiated") @@ -446,8 +449,8 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { const removeTx = await nativeStakingSSVStrategy .connect(validatorRegistrator) - .removeSsvValidator( - testValidator.publicKey, + .removeSsvValidators( + [testValidator.publicKey], testValidator.operatorIds, newCluster ); @@ -508,8 +511,8 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { const removeTx = await nativeStakingSSVStrategy .connect(validatorRegistrator) - .removeSsvValidator( - testValidator.publicKey, + .removeSsvValidators( + [testValidator.publicKey], testValidator.operatorIds, newCluster );