diff --git a/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol b/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol index 219ebfb45e..9a407261e0 100644 --- a/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol +++ b/contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol @@ -48,6 +48,7 @@ abstract contract ValidatorRegistrator is Governable, Pausable { uint256[47] private __gap; enum VALIDATOR_STATE { + NON_REGISTERED, // validator is not registered on the SSV network REGISTERED, // validator is registered on the SSV network STAKED, // validator has funds staked EXITING, // exit message has been posted and validator is in the process of exiting @@ -207,6 +208,7 @@ abstract contract ValidatorRegistrator is Governable, Pausable { /// @notice Registers a new validator in the SSV Cluster. /// Only the registrator can call this function. + // slither-disable-start reentrancy-no-eth function registerSsvValidator( bytes calldata publicKey, uint64[] calldata operatorIds, @@ -214,6 +216,11 @@ abstract contract ValidatorRegistrator is Governable, Pausable { uint256 amount, Cluster calldata cluster ) external onlyRegistrator whenNotPaused { + require( + validatorsStates[keccak256(publicKey)] == + VALIDATOR_STATE.NON_REGISTERED, + "Validator already registered" + ); ISSVNetwork(SSV_NETWORK_ADDRESS).registerValidator( publicKey, operatorIds, @@ -225,6 +232,8 @@ abstract contract ValidatorRegistrator is Governable, Pausable { emit SSVValidatorRegistered(publicKey, operatorIds); } + // slither-disable-end reentrancy-no-eth + /// @notice Exit a validator from the Beacon chain. /// The staked ETH will eventually swept to this native staking strategy. /// Only the registrator can call this function. diff --git a/contracts/deploy/holesky/010_upgrade_strategy.js b/contracts/deploy/holesky/010_upgrade_strategy.js index a073a9d2b1..89f152e13e 100644 --- a/contracts/deploy/holesky/010_upgrade_strategy.js +++ b/contracts/deploy/holesky/010_upgrade_strategy.js @@ -3,7 +3,6 @@ const { withConfirmation } = require("../../utils/deploy"); const { resolveContract } = require("../../utils/resolvers"); const addresses = require("../../utils/addresses"); const { parseEther } = require("ethers/lib/utils"); -// const { impersonateAndFund } = require("../../utils/signers.js"); const mainExport = async () => { console.log("Running 010 deployment on Holesky..."); diff --git a/contracts/deploy/holesky/011_upgrade_strategy.js b/contracts/deploy/holesky/011_upgrade_strategy.js new file mode 100644 index 0000000000..1dc95a7268 --- /dev/null +++ b/contracts/deploy/holesky/011_upgrade_strategy.js @@ -0,0 +1,18 @@ +const { upgradeNativeStakingSSVStrategy } = require("../deployActions"); + +const mainExport = async () => { + console.log("Running 011 deployment on Holesky..."); + + console.log("Upgrading native staking strategy"); + await upgradeNativeStakingSSVStrategy(); + + console.log("Running 011 deployment done"); + return true; +}; + +mainExport.id = "011_upgrade_strategy"; +mainExport.tags = []; +mainExport.dependencies = []; +mainExport.skip = () => false; + +module.exports = mainExport; diff --git a/contracts/test/behaviour/ssvStrategy.js b/contracts/test/behaviour/ssvStrategy.js index afd756915e..0a86594136 100644 --- a/contracts/test/behaviour/ssvStrategy.js +++ b/contracts/test/behaviour/ssvStrategy.js @@ -9,7 +9,7 @@ const hre = require("hardhat"); const { oethUnits } = require("../helpers"); const { impersonateAndFund } = require("../../utils/signers"); const { getClusterInfo } = require("../../utils/ssv"); -const { parseEther } = require("ethers/lib/utils"); +const { parseEther, keccak256 } = require("ethers/lib/utils"); const { setERC20TokenBalance } = require("../_fund"); /** @@ -181,6 +181,12 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { const stakeAmount = oethUnits("32"); + expect( + await nativeStakingSSVStrategy.validatorsStates( + keccak256(testValidator.publicKey) + ) + ).to.equal(0, "Validator state not 0 (NON_REGISTERED)"); + // Register a new validator with the SSV Network const regTx = await nativeStakingSSVStrategy .connect(validatorRegistrator) @@ -195,6 +201,12 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { .to.emit(nativeStakingSSVStrategy, "SSVValidatorRegistered") .withArgs(testValidator.publicKey, testValidator.operatorIds); + expect( + await nativeStakingSSVStrategy.validatorsStates( + keccak256(testValidator.publicKey) + ) + ).to.equal(1, "Validator state not 1 (REGISTERED)"); + // Stake stakeAmount ETH to the new validator const stakeTx = await nativeStakingSSVStrategy .connect(validatorRegistrator) @@ -213,6 +225,12 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { amount: oethUnits("32"), }); + expect( + await nativeStakingSSVStrategy.validatorsStates( + keccak256(testValidator.publicKey) + ) + ).to.equal(2, "Validator state not 2 (STAKED)"); + expect(await weth.balanceOf(nativeStakingSSVStrategy.address)).to.equal( strategyWethBalanceBefore.sub( stakeAmount, @@ -283,10 +301,7 @@ const shouldBehaveLikeAnSsvStrategy = (context) => { emptyCluster ); - // Waffle's custom error matcher is not working here. - // Checking the trace, the error thrown is ValidatorAlreadyExistsWithData - // which is what we expect. - await expect(tx2).to.be.reverted; + await expect(tx2).to.be.revertedWith("Validator already registered"); }); it("Should emit correct values in deposit event", async () => { diff --git a/contracts/test/strategies/nativeSSVStaking.js b/contracts/test/strategies/nativeSSVStaking.js index 26ac2fa84a..9a116a1fe4 100644 --- a/contracts/test/strategies/nativeSSVStaking.js +++ b/contracts/test/strategies/nativeSSVStaking.js @@ -1,6 +1,6 @@ const { expect } = require("chai"); const { BigNumber } = require("ethers"); -const { parseEther } = require("ethers").utils; +const { keccak256, parseEther } = require("ethers").utils; const { setBalance, setStorageAt, @@ -37,6 +37,14 @@ const testValidator = { depositDataRoot: "0xdbe778a625c68446f3cc8b2009753a5e7dd7c37b8721ee98a796bb9179dfe8ac", }; +const testPublicKeys = [ + "0xaba6acd335d524a89fb89b9977584afdb23f34a6742547fa9ec1c656fbd2bfc0e7a234460328c2731828c9a43be06e25", + "0xa8adaec39a6738b09053a3ed9d44e481d5b2dfafefe0059da48756db951adf4f2956c1149f3bd0634e4cde009a770afb", + "0xaa8cdeb9efe0cb2f703332a46051214464796e7de7b882abd243c175b2d96250ad227846f713876445f864b2e2f695c1", + "0xb22b68e2a4f524e96c7818dbfca3de0f7fb4e87449fe8166fd310bea3e3e4295db41b21e65612d1d4bd8a14f2d47e49a", + "0x92fe1f554b8110fa5c74af8181ca2afaad12f6d22cad933ef1978b5d4d099d75045e4d6d15066c290aee29990858cb90", + "0xb27b34f6931ba70a11c2ba82f194e9b98093a5a482bb035a836df9aa4b5f57542354da453538b651c18eefc0ea3a7689", +]; const emptyCluster = [ 0, // validatorCount @@ -1064,30 +1072,46 @@ describe("Unit test: Native SSV Staking Strategy", function () { .setStakeETHThreshold(stakeThreshold); }); - const stakeValidator = async (validators, stakeTresholdErrorTriggered) => { + const stakeValidator = async ( + validators, + stakeTresholdErrorTriggered, + startingIndex = 0 + ) => { const { nativeStakingSSVStrategy, validatorRegistrator } = fixture; // there is a limitation to this function as it will only check for // a failure transaction with the last stake call - for (let i = 0; i < validators; i++) { + for (let i = startingIndex; i < validators; i++) { + expect( + await nativeStakingSSVStrategy.validatorsStates( + keccak256(testPublicKeys[i]) + ) + ).to.equal(0, "Validator state not 0 (NON_REGISTERED)"); + const stakeAmount = ethUnits("32"); // Register a new validator with the SSV Network await nativeStakingSSVStrategy .connect(validatorRegistrator) .registerSsvValidator( - testValidator.publicKey, + testPublicKeys[i], testValidator.operatorIds, testValidator.sharesData, stakeAmount, emptyCluster ); + expect( + await nativeStakingSSVStrategy.validatorsStates( + keccak256(testPublicKeys[i]) + ) + ).to.equal(1, "Validator state not 1 (REGISTERED)"); + // Stake ETH to the new validator const tx = nativeStakingSSVStrategy .connect(validatorRegistrator) .stakeEth([ { - pubkey: testValidator.publicKey, + pubkey: testPublicKeys[i], signature: testValidator.signature, depositDataRoot: testValidator.depositDataRoot, }, @@ -1097,6 +1121,12 @@ describe("Unit test: Native SSV Staking Strategy", function () { await expect(tx).to.be.revertedWith("Staking ETH over threshold"); } else { await tx; + + expect( + await nativeStakingSSVStrategy.validatorsStates( + keccak256(testPublicKeys[i]) + ) + ).to.equal(2, "Validator state not 2 (STAKED)"); } } }; @@ -1113,6 +1143,23 @@ describe("Unit test: Native SSV Staking Strategy", function () { await stakeValidator(3, true); }); + it("Fail to stake a validator that hasn't been registered", async () => { + const { nativeStakingSSVStrategy, validatorRegistrator } = fixture; + + // Stake ETH to the unregistered validator + const tx = nativeStakingSSVStrategy + .connect(validatorRegistrator) + .stakeEth([ + { + pubkey: testValidator.publicKey, + signature: testValidator.signature, + depositDataRoot: testValidator.depositDataRoot, + }, + ]); + + await expect(tx).to.be.revertedWith("Validator not registered"); + }); + it("Should stake to 2 validators continually when threshold is reset", async () => { const { anna, nativeStakingSSVStrategy } = fixture; @@ -1120,11 +1167,11 @@ describe("Unit test: Native SSV Staking Strategy", function () { await nativeStakingSSVStrategy.connect(anna).resetStakeETHTally(); }; - await stakeValidator(2, false); + await stakeValidator(2, false, 0); await resetThreshold(); - await stakeValidator(2, false); + await stakeValidator(2, false, 2); await resetThreshold(); - await stakeValidator(2, false); + await stakeValidator(2, false, 4); await resetThreshold(); });