diff --git a/abi/IValidatorsChecker.json b/abi/IValidatorsChecker.json index 523abc44..4e02a50a 100644 --- a/abi/IValidatorsChecker.json +++ b/abi/IValidatorsChecker.json @@ -36,8 +36,13 @@ "outputs": [ { "internalType": "uint256", - "name": "", + "name": "blockNumber", "type": "uint256" + }, + { + "internalType": "enum IValidatorsChecker.Status", + "name": "status", + "type": "uint8" } ], "stateMutability": "view", @@ -70,8 +75,13 @@ "outputs": [ { "internalType": "uint256", - "name": "", + "name": "blockNumber", "type": "uint256" + }, + { + "internalType": "enum IValidatorsChecker.Status", + "name": "status", + "type": "uint8" } ], "stateMutability": "view", diff --git a/contracts/interfaces/IValidatorsChecker.sol b/contracts/interfaces/IValidatorsChecker.sol index 19b7625a..4921ffb3 100644 --- a/contracts/interfaces/IValidatorsChecker.sol +++ b/contracts/interfaces/IValidatorsChecker.sol @@ -8,20 +8,33 @@ pragma solidity ^0.8.22; * @notice Defines the interface for ValidatorsChecker */ interface IValidatorsChecker { + enum Status { + SUCCEEDED, + INVALID_VALIDATORS_REGISTRY_ROOT, + INVALID_VAULT, + INSUFFICIENT_ASSETS, + INVALID_SIGNATURE, + INVALID_VALIDATORS_MANAGER, + INVALID_VALIDATORS_COUNT, + INVALID_VALIDATORS_LENGTH, + INVALID_PROOF + } + /** * @notice Function for checking validators manager signature * @param vault The address of the vault * @param validatorsRegistryRoot The validators registry root * @param validators The concatenation of the validators' public key, deposit signature, deposit root and optionally withdrawal address * @param signature The validators manager signature - * @return Current block number + * @return blockNumber Current block number + * @return status The status of the verification */ function checkValidatorsManagerSignature( address vault, bytes32 validatorsRegistryRoot, bytes calldata validators, bytes calldata signature - ) external view returns (uint256); + ) external view returns (uint256 blockNumber, Status status); /** * @notice Function for checking deposit data root @@ -31,7 +44,8 @@ interface IValidatorsChecker { * @param proof The proof used for the merkle tree verification * @param proofFlags The multi proof flags for the merkle tree verification * @param proofIndexes The indexes of the leaves for the merkle tree multi proof verification - * @return Current block number + * @return blockNumber Current block number + * @return status The status of the verification */ function checkDepositDataRoot( address vault, @@ -40,5 +54,5 @@ interface IValidatorsChecker { bytes32[] calldata proof, bool[] calldata proofFlags, uint256[] calldata proofIndexes - ) external view returns (uint256); + ) external view returns (uint256 blockNumber, Status status); } diff --git a/contracts/validators/ValidatorsChecker.sol b/contracts/validators/ValidatorsChecker.sol index 684a611d..e4c12841 100644 --- a/contracts/validators/ValidatorsChecker.sol +++ b/contracts/validators/ValidatorsChecker.sol @@ -61,19 +61,19 @@ abstract contract ValidatorsChecker is IValidatorsChecker { bytes32 validatorsRegistryRoot, bytes calldata validators, bytes calldata signature - ) external view override returns (uint256) { + ) external view override returns (uint256 blockNumber, Status status) { if (_validatorsRegistry.get_deposit_root() != validatorsRegistryRoot) { - revert Errors.InvalidValidatorsRegistryRoot(); + return (block.number, Status.INVALID_VALIDATORS_REGISTRY_ROOT); } if (!_vaultsRegistry.vaults(vault) || IVaultVersion(vault).version() < 2) { - revert Errors.InvalidVault(); + return (block.number, Status.INVALID_VAULT); } // verify vault has enough assets if ( !_keeper.isCollateralized(vault) && IVaultState(vault).withdrawableAssets() < _depositAmount() ) { - revert Errors.InsufficientAssets(); + return (block.number, Status.INSUFFICIENT_ASSETS); } // compose signing message @@ -87,10 +87,10 @@ abstract contract ValidatorsChecker is IValidatorsChecker { signature ) ) { - revert Errors.AccessDenied(); + return (block.number, Status.INVALID_SIGNATURE); } - return block.number; + return (block.number, Status.SUCCEEDED); } /// @inheritdoc IValidatorsChecker @@ -101,17 +101,19 @@ abstract contract ValidatorsChecker is IValidatorsChecker { bytes32[] calldata proof, bool[] calldata proofFlags, uint256[] calldata proofIndexes - ) external view override returns (uint256) { + ) external view override returns (uint256 blockNumber, Status status) { if (_validatorsRegistry.get_deposit_root() != validatorsRegistryRoot) { - revert Errors.InvalidValidatorsRegistryRoot(); + return (block.number, Status.INVALID_VALIDATORS_REGISTRY_ROOT); + } + if (!_vaultsRegistry.vaults(vault)) { + return (block.number, Status.INVALID_VAULT); } - if (!_vaultsRegistry.vaults(vault)) revert Errors.InvalidVault(); // verify vault has enough assets if ( !_keeper.isCollateralized(vault) && IVaultState(vault).withdrawableAssets() < _depositAmount() ) { - revert Errors.InsufficientAssets(); + return (block.number, Status.INSUFFICIENT_ASSETS); } uint8 vaultVersion = IVaultVersion(vault).version(); @@ -119,7 +121,9 @@ abstract contract ValidatorsChecker is IValidatorsChecker { address validatorsManager = IVaultValidators(vault).validatorsManager(); // verify vault did not set custom validators manager - if (validatorsManager != address(_depositDataRegistry)) revert Errors.AccessDenied(); + if (validatorsManager != address(_depositDataRegistry)) { + return (block.number, Status.INVALID_VALIDATORS_MANAGER); + } } uint256 currentIndex; @@ -135,12 +139,16 @@ abstract contract ValidatorsChecker is IValidatorsChecker { // define leaves for multiproof uint256 validatorsCount = proofIndexes.length; - if (validatorsCount == 0) revert Errors.InvalidValidators(); + if (validatorsCount == 0) { + return (block.number, Status.INVALID_VALIDATORS_COUNT); + } bytes32[] memory leaves = new bytes32[](validatorsCount); // calculate validator length uint256 validatorLength = validators.length / validatorsCount; - if (validatorLength == 0) revert Errors.InvalidValidators(); + if (validatorLength == 0 || validatorsCount * validatorLength != validators.length) { + return (block.number, Status.INVALID_VALIDATORS_LENGTH); + } // calculate leaves { @@ -163,10 +171,10 @@ abstract contract ValidatorsChecker is IValidatorsChecker { // check matches merkle root and next validator index if (!MerkleProof.multiProofVerifyCalldata(proof, proofFlags, depositDataRoot, leaves)) { - revert Errors.InvalidProof(); + return (block.number, Status.INVALID_PROOF); } - return block.number; + return (block.number, Status.SUCCEEDED); } /** diff --git a/test/ValidatorsChecker.spec.ts b/test/ValidatorsChecker.spec.ts index 17d4c7d0..8ceda8fc 100644 --- a/test/ValidatorsChecker.spec.ts +++ b/test/ValidatorsChecker.spec.ts @@ -29,6 +29,18 @@ import { createGnoValidatorsChecker } from './shared/gnoFixtures' const networks = ['ETHEREUM', 'GNOSIS'] +enum Status { + SUCCEEDED, + INVALID_VALIDATORS_REGISTRY_ROOT, + INVALID_VAULT, + INSUFFICIENT_ASSETS, + INVALID_SIGNATURE, + INVALID_VALIDATORS_MANAGER, + INVALID_VALIDATORS_COUNT, + INVALID_VALIDATORS_LENGTH, + INVALID_PROOF, +} + networks.forEach((network) => { describe(`ValidatorsChecker [${network}]`, () => { let validatorDeposit = ethers.parseEther('32') @@ -155,45 +167,43 @@ networks.forEach((network) => { it('fails for invalid validators registry root', async () => { const fakeRoot = Buffer.alloc(32).fill(1) - await expect( - validatorsChecker - .connect(admin) - .checkValidatorsManagerSignature(await vault.getAddress(), fakeRoot, '0x', '0x') - ).to.be.revertedWithCustomError(validatorsChecker, 'InvalidValidatorsRegistryRoot') + const response = await validatorsChecker.checkValidatorsManagerSignature( + await vault.getAddress(), + fakeRoot, + '0x', + '0x' + ) + expect(response.status).to.eq(Status.INVALID_VALIDATORS_REGISTRY_ROOT) }) it('fails for non-vault', async () => { - await expect( - validatorsChecker - .connect(admin) - .checkValidatorsManagerSignature(other.address, validatorsRegistryRoot, '0x', '0x') - ).to.be.revertedWithCustomError(validatorsChecker, 'InvalidVault') + const response = await validatorsChecker.checkValidatorsManagerSignature( + other.address, + validatorsRegistryRoot, + '0x', + '0x' + ) + expect(response.status).to.eq(Status.INVALID_VAULT) }) it('fails for vault v1', async () => { - await expect( - validatorsChecker - .connect(admin) - .checkValidatorsManagerSignature( - await vaultV1.getAddress(), - validatorsRegistryRoot, - '0x', - '0x' - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'InvalidVault') + const response = await validatorsChecker.checkValidatorsManagerSignature( + await vaultV1.getAddress(), + validatorsRegistryRoot, + '0x', + '0x' + ) + expect(response.status).to.eq(Status.INVALID_VAULT) }) it('fails for vault not collateralized not deposited', async () => { - await expect( - validatorsChecker - .connect(admin) - .checkValidatorsManagerSignature( - await vaultNotDeposited.getAddress(), - validatorsRegistryRoot, - '0x', - '0x' - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'InsufficientAssets') + const response = await validatorsChecker.checkValidatorsManagerSignature( + await vaultNotDeposited.getAddress(), + validatorsRegistryRoot, + '0x', + '0x' + ) + expect(response.status).to.eq(Status.INSUFFICIENT_ASSETS) }) it('fails for signer who is not validators manager', async () => { @@ -208,16 +218,13 @@ networks.forEach((network) => { data: typedData, version: SignTypedDataVersion.V4, }) - await expect( - validatorsChecker - .connect(admin) - .checkValidatorsManagerSignature( - vaultAddress, - validatorsRegistryRoot, - Buffer.concat(validators), - ethers.getBytes(signature) - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'AccessDenied') + const response = await validatorsChecker.checkValidatorsManagerSignature( + vaultAddress, + validatorsRegistryRoot, + Buffer.concat(validators), + ethers.getBytes(signature) + ) + expect(response.status).to.eq(Status.INVALID_SIGNATURE) }) it('succeeds', async () => { @@ -233,17 +240,14 @@ networks.forEach((network) => { version: SignTypedDataVersion.V4, }) const blockNumber = await ethers.provider.getBlockNumber() - - expect( - await validatorsChecker - .connect(admin) - .checkValidatorsManagerSignature( - vaultAddress, - validatorsRegistryRoot, - Buffer.concat(validators), - ethers.getBytes(signature) - ) - ).to.eq(blockNumber) + const response = await validatorsChecker.checkValidatorsManagerSignature( + vaultAddress, + validatorsRegistryRoot, + Buffer.concat(validators), + ethers.getBytes(signature) + ) + expect(response.blockNumber).to.eq(blockNumber) + expect(response.status).to.eq(Status.SUCCEEDED) }) }) @@ -268,116 +272,117 @@ networks.forEach((network) => { it('fails for invalid validators registry root', async () => { const fakeRoot = Buffer.alloc(32).fill(1) - await expect( - validatorsChecker - .connect(admin) - .checkDepositDataRoot( - await vault.getAddress(), - fakeRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'InvalidValidatorsRegistryRoot') + const response = await validatorsChecker.checkDepositDataRoot( + await vault.getAddress(), + fakeRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.status).to.eq(Status.INVALID_VALIDATORS_REGISTRY_ROOT) }) it('fails for non-vault', async () => { - await expect( - validatorsChecker - .connect(admin) - .checkDepositDataRoot( - other.address, - validatorsRegistryRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'InvalidVault') + const response = await validatorsChecker.checkDepositDataRoot( + other.address, + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.status).to.eq(Status.INVALID_VAULT) }) it('fails for vault not collateralized not deposited', async () => { - await expect( - validatorsChecker - .connect(admin) - .checkDepositDataRoot( - await vaultNotDeposited.getAddress(), - validatorsRegistryRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'InsufficientAssets') + const response = await validatorsChecker.checkDepositDataRoot( + await vaultNotDeposited.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.status).to.eq(Status.INSUFFICIENT_ASSETS) }) it('fails for validators manager not equal to deposit data registry', async () => { await vault.connect(admin).setValidatorsManager(other.address) - - await expect( - validatorsChecker - .connect(admin) - .checkDepositDataRoot( - await vault.getAddress(), - validatorsRegistryRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'AccessDenied') + const response = await validatorsChecker.checkDepositDataRoot( + await vault.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.status).to.eq(Status.INVALID_VALIDATORS_MANAGER) }) it('fails for invalid proof', async () => { proof[0] = '0x' + '1'.repeat(64) + const response = await validatorsChecker.checkDepositDataRoot( + await vault.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.status).to.eq(Status.INVALID_PROOF) + }) + + it('fails for invalid proof indexes', async () => { + const response = await validatorsChecker.checkDepositDataRoot( + await vault.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + [] + ) + expect(response.status).to.eq(Status.INVALID_VALIDATORS_COUNT) + }) - await expect( - validatorsChecker - .connect(admin) - .checkDepositDataRoot( - await vault.getAddress(), - validatorsRegistryRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.be.revertedWithCustomError(validatorsChecker, 'InvalidProof') + it('fails for invalid validators', async () => { + const response = await validatorsChecker.checkDepositDataRoot( + await vault.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators.slice(0, -1)), + proof, + proofFlags, + proofIndexes + ) + expect(response.status).to.eq(Status.INVALID_VALIDATORS_LENGTH) }) it('succeeds for vault v1', async () => { const blockNumber = await ethers.provider.getBlockNumber() - - expect( - await validatorsChecker - .connect(admin) - .checkDepositDataRoot( - await vaultV1.getAddress(), - validatorsRegistryRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.eq(blockNumber) + const response = await validatorsChecker.checkDepositDataRoot( + await vaultV1.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.blockNumber).to.eq(blockNumber) + expect(response.status).to.eq(Status.SUCCEEDED) }) it('succeeds for vault v2', async () => { const blockNumber = await ethers.provider.getBlockNumber() - - expect( - await validatorsChecker - .connect(admin) - .checkDepositDataRoot( - await vault.getAddress(), - validatorsRegistryRoot, - Buffer.concat(validators), - proof, - proofFlags, - proofIndexes - ) - ).to.eq(blockNumber) + const response = await validatorsChecker.checkDepositDataRoot( + await vault.getAddress(), + validatorsRegistryRoot, + Buffer.concat(validators), + proof, + proofFlags, + proofIndexes + ) + expect(response.blockNumber).to.eq(blockNumber) + expect(response.status).to.eq(Status.SUCCEEDED) }) }) })