diff --git a/config/devnet_config.json b/config/devnet_config.json index d22a4c1..0bcadcd 100644 --- a/config/devnet_config.json +++ b/config/devnet_config.json @@ -1,13 +1,13 @@ { "allow_root_override": true, "vault_verifier": "0x3498311FC4D1D1AEa7c2BE9996fa9660e88B1EbE", + "vault_root_provider": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "operators": { "pauser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "unpauser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "disburser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "defaultAdmin": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "accountRootManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", - "vaultRootManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "tokenMappingManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa" }, "lookup_tables": [ diff --git a/config/tenderly_eth_mainnet_config.json b/config/tenderly_eth_mainnet_config.json index 9aed541..ded4f18 100644 --- a/config/tenderly_eth_mainnet_config.json +++ b/config/tenderly_eth_mainnet_config.json @@ -1,13 +1,13 @@ { "allow_root_override": true, "vault_verifier": "0x0000000000000000000000000000000000000000", + "vault_root_provider": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "operators": { "pauser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "unpauser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "disburser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "defaultAdmin": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "accountRootManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", - "vaultRootManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "tokenMappingManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa" }, "lookup_tables": [ diff --git a/config/tenderly_zkevm_mainnet_config.json b/config/tenderly_zkevm_mainnet_config.json index 5eddf7e..53ba51a 100644 --- a/config/tenderly_zkevm_mainnet_config.json +++ b/config/tenderly_zkevm_mainnet_config.json @@ -1,13 +1,13 @@ { "allow_root_override": true, "vault_verifier": "0x0000000000000000000000000000000000000000", + "vault_root_provider": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "operators": { "pauser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "unpauser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "disburser": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "defaultAdmin": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "accountRootManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", - "vaultRootManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa", "tokenMappingManager": "0xBc6E5103D599FaEC8f4D2911efAEb98814cb5Aaa" }, "lookup_tables": [ diff --git a/script/DeployL2Contracts.s.sol b/script/DeployL2Contracts.s.sol index 1716733..637f4ed 100644 --- a/script/DeployL2Contracts.s.sol +++ b/script/DeployL2Contracts.s.sol @@ -13,6 +13,7 @@ import {ProcessorAccessControl} from "@src/withdrawals/ProcessorAccessControl.so contract DeployL2Contracts is Script { bool private allowRootOverride; address private vaultVerifier; + address private vaultRootProvider; address[63] private lookupTables; VaultWithdrawalProcessor private withdrawalProcessor; VaultWithdrawalProcessor.RoleOperators private operators; @@ -23,6 +24,7 @@ contract DeployL2Contracts is Script { allowRootOverride = vm.parseJsonBool(config, "$.allow_root_override"); vaultVerifier = vm.parseJsonAddress(config, "$.vault_verifier"); + vaultRootProvider = vm.parseJsonAddress(config, "$.vault_root_provider"); operators = abi.decode(vm.parseJson(config, "$.operators"), (ProcessorAccessControl.RoleOperators)); @@ -48,7 +50,8 @@ contract DeployL2Contracts is Script { vaultVerifier = address(new VaultEscapeProofVerifier(lookupTables)); } - withdrawalProcessor = new VaultWithdrawalProcessor(vaultVerifier, operators, allowRootOverride); + withdrawalProcessor = + new VaultWithdrawalProcessor(vaultVerifier, vaultRootProvider, operators, allowRootOverride); withdrawalProcessor.registerTokenMappings(assetMappings); _logDeploymentDetails(); diff --git a/src/bridge/starkex/StarkExchangeMigration.sol b/src/bridge/starkex/StarkExchangeMigration.sol index 6555fcf..dea269a 100644 --- a/src/bridge/starkex/StarkExchangeMigration.sol +++ b/src/bridge/starkex/StarkExchangeMigration.sol @@ -34,6 +34,10 @@ contract StarkExchangeMigration is IStarkExchangeMigration, LegacyStarkExchangeB /// @dev Reference to native ETH, based on the value used to represent ETH on the zkEVM bridge. address public constant NATIVE_ETH = address(0xeee); + constructor() { + _disableInitializers(); + } + /** * @notice Initializes the contract with migration configuration * @param data Encoded initialization data containing addresses diff --git a/src/verifiers/vaults/VaultEscapeProofVerifier.sol b/src/verifiers/vaults/VaultEscapeProofVerifier.sol index 7d755f5..09ffc1d 100644 --- a/src/verifiers/vaults/VaultEscapeProofVerifier.sol +++ b/src/verifiers/vaults/VaultEscapeProofVerifier.sol @@ -337,22 +337,11 @@ contract VaultEscapeProofVerifier is IVaultProofVerifier { return escapeProof[escapeProof.length - 2] >> 4; } - function _validateProofStructure(uint256[] calldata escapeProof) private pure returns (bool) { - uint256 proofLength = escapeProof.length; - - // The minimal supported proof length is for a tree height of 31 in a 68 word representation as follows: + function _validateProofStructure(uint256[] calldata escapeProof) private pure { + // The only supported proof length is for a tree height of 31 in a 68 word representation as follows: // 1. 2 word pairs representing the vault contents + one hash of the 1st pair. - // 2. 31 word pairs representing the authentication path. + // 2. 31 word pairs representing the authentication path. // 3. 1 word pair representing the root and the leaf index. - require(proofLength >= VAULT_PROOF_LENGTH, InvalidVaultProof("Proof too short.")); - - // The contract supports verification paths of lengths up to 97 in a 200 word representation as described above. - // This limitation is imposed in order to avoid potential attacks. - require(proofLength < 200, InvalidVaultProof("Proof too long.")); - - // Ensure proofs are always a series of word pairs. - require((proofLength & 1) == 0, InvalidVaultProof("Proof length must be even.")); - - return true; + require(escapeProof.length == VAULT_PROOF_LENGTH, InvalidVaultProof("Invalid proof length.")); } } diff --git a/src/withdrawals/ProcessorAccessControl.sol b/src/withdrawals/ProcessorAccessControl.sol index 29ecdd9..b051f69 100644 --- a/src/withdrawals/ProcessorAccessControl.sol +++ b/src/withdrawals/ProcessorAccessControl.sol @@ -17,15 +17,12 @@ abstract contract ProcessorAccessControl is AccessControlEnumerable, Pausable { bytes32 public constant DISBURSER_ROLE = keccak256("DISBURSER_ROLE"); /// @notice Role for setting the account root bytes32 public constant ACCOUNT_ROOT_PROVIDER_ROLE = keccak256("ACCOUNT_ROOT_PROVIDER_ROLE"); - /// @notice Role for setting the vault root - bytes32 public constant VAULT_ROOT_PROVIDER_ROLE = keccak256("VAULT_ROOT_PROVIDER_ROLE"); /// @notice Role for managing token mappings bytes32 public constant TOKEN_MAPPING_MANAGER = keccak256("TOKEN_MAPPING_MANAGER"); /// @notice Struct encapsulating addresses that can perform privileged operations struct RoleOperators { address accountRootProvider; - address vaultRootProvider; address tokenMappingManager; address disburser; address pauser; @@ -51,7 +48,6 @@ abstract contract ProcessorAccessControl is AccessControlEnumerable, Pausable { function _validateOperators(RoleOperators memory operators) internal pure { require(operators.accountRootProvider != address(0), InvalidOperatorAddress()); - require(operators.vaultRootProvider != address(0), InvalidOperatorAddress()); require(operators.tokenMappingManager != address(0), InvalidOperatorAddress()); require(operators.disburser != address(0), InvalidOperatorAddress()); require(operators.pauser != address(0), InvalidOperatorAddress()); @@ -61,7 +57,6 @@ abstract contract ProcessorAccessControl is AccessControlEnumerable, Pausable { function _grantRoleOperators(RoleOperators memory operators) internal { _grantRole(ACCOUNT_ROOT_PROVIDER_ROLE, operators.accountRootProvider); - _grantRole(VAULT_ROOT_PROVIDER_ROLE, operators.vaultRootProvider); _grantRole(TOKEN_MAPPING_MANAGER, operators.tokenMappingManager); _grantRole(DISBURSER_ROLE, operators.disburser); _grantRole(PAUSER_ROLE, operators.pauser); diff --git a/src/withdrawals/VaultWithdrawalProcessor.sol b/src/withdrawals/VaultWithdrawalProcessor.sol index 8e34a92..3c0ee58 100644 --- a/src/withdrawals/VaultWithdrawalProcessor.sol +++ b/src/withdrawals/VaultWithdrawalProcessor.sol @@ -30,6 +30,9 @@ contract VaultWithdrawalProcessor is /// @notice Thrown if attempting to set the root override value to the existing value error NoChangeInOverrideValue(); + /// @notice Thrown when a caller other than the authorised vault root provider attempts to set the vault root + error UnauthorizedVaultRootProvider(); + /// @notice Thrown if dequantization would cause an overflow /// @param quantizedBalance The quantized balance that caused the overflow /// @param quantum The quantum value that caused the overflow @@ -40,26 +43,36 @@ contract VaultWithdrawalProcessor is /// @dev Upper bound for valid Stark keys (2^251 + 17 * 2^192 + 1) uint256 internal constant STARK_KEY_UPPER_BOUND = 0x800000000000011000000000000000000000000000000000000000000000001; - uint256 public constant VAULT_PROOF_LENGTH = 68; uint256 public constant ACCOUNT_PROOF_LENGTH = 27; /// @notice The vault proof verifier contract IVaultProofVerifier public immutable VAULT_PROOF_VERIFIER; + /// @notice The address authorised to set the vault root (expected to be a VaultRootReceiverAdapter instance) + address public immutable VAULT_ROOT_PROVIDER; + /// @notice Flag indicating whether the vault root can be overridden after initial setting bool public rootOverrideAllowed = false; /** * @notice Constructs the VaultWithdrawalProcessor contract * @param _vaultProofVerifier The address of the vault proof verifier contract + * @param _vaultRootProvider The address authorised to set the vault root (e.g. a VaultRootReceiverAdapter instance) * @param _operators The list of addresses to be granted specific roles * @param _rootOverrideAllowed Whether the vault and account roots can be overridden after initial setting */ - constructor(address _vaultProofVerifier, RoleOperators memory _operators, bool _rootOverrideAllowed) { + constructor( + address _vaultProofVerifier, + address _vaultRootProvider, + RoleOperators memory _operators, + bool _rootOverrideAllowed + ) { require(_vaultProofVerifier != address(0), ZeroAddress()); + require(_vaultRootProvider != address(0), ZeroAddress()); _validateOperators(_operators); VAULT_PROOF_VERIFIER = IVaultProofVerifier(_vaultProofVerifier); + VAULT_ROOT_PROVIDER = _vaultRootProvider; _grantRoleOperators(_operators); rootOverrideAllowed = _rootOverrideAllowed; } @@ -78,11 +91,9 @@ contract VaultWithdrawalProcessor is require(receiver != address(0), ZeroAddress()); require(accountProof.length == ACCOUNT_PROOF_LENGTH, InvalidAccountProof("Invalid account proof length")); - require( - vaultProof.length == VAULT_PROOF_LENGTH, IVaultProofVerifier.InvalidVaultProof("Invalid vault proof length") - ); // Extract the vault and vault root information from the submitted proof + // Note: the verifier validates the proof length strictly (must be exactly 68) (IVaultProofVerifier.Vault memory vault, uint256 _vaultRoot) = VAULT_PROOF_VERIFIER.extractVaultAndRootFromProof(vaultProof); @@ -118,11 +129,12 @@ contract VaultWithdrawalProcessor is /** * @notice Sets the vault root hash for proof verification - * @dev Only the vault root provider can call this function + * @dev Only the immutable vault root provider address can call this function * @dev The vault root can only be set once unless rootOverrideAllowed is true * @param newRoot The new vault root hash */ - function setVaultRoot(uint256 newRoot) external override onlyRole(VAULT_ROOT_PROVIDER_ROLE) { + function setVaultRoot(uint256 newRoot) external override { + require(msg.sender == VAULT_ROOT_PROVIDER, UnauthorizedVaultRootProvider()); _setVaultRoot(newRoot, rootOverrideAllowed); } diff --git a/test/integration/proofs/withdrawals/VaultWithdrawalProcessor.t.sol b/test/integration/proofs/withdrawals/VaultWithdrawalProcessor.t.sol index b0396f5..c118083 100644 --- a/test/integration/proofs/withdrawals/VaultWithdrawalProcessor.t.sol +++ b/test/integration/proofs/withdrawals/VaultWithdrawalProcessor.t.sol @@ -63,11 +63,10 @@ contract VaultWithdrawalProcessorIntegrationTest is disburser: address(this), defaultAdmin: address(this), accountRootProvider: address(this), - vaultRootProvider: address(rootReceiver), tokenMappingManager: address(this) }); - vaultProcessor = new VaultWithdrawalProcessor(address(vaultVerifier), operators, true); + vaultProcessor = new VaultWithdrawalProcessor(address(vaultVerifier), address(rootReceiver), operators, true); vaultProcessor.setAccountRoot(accountsRoot); vaultProcessor.registerTokenMappings(fixAssets); diff --git a/test/unit/bridge/starkex/StarkExchangeMigration.t.sol b/test/unit/bridge/starkex/StarkExchangeMigration.t.sol index 45a6e2d..402227d 100644 --- a/test/unit/bridge/starkex/StarkExchangeMigration.t.sol +++ b/test/unit/bridge/starkex/StarkExchangeMigration.t.sol @@ -6,6 +6,8 @@ import "@src/bridge/starkex/StarkExchangeMigration.sol"; import "@src/bridge/starkex/IStarkExchangeMigration.sol"; import "@src/bridge/zkEVM/IRootERC20Bridge.sol"; import "@src/bridge/messaging/VaultRootSenderAdapter.sol"; +import "@openzeppelin/contracts/proxy/utils/Initializable.sol"; +import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; @@ -103,37 +105,43 @@ contract StarkExchangeMigrationTest is Test { uint256 private constant BRIDGE_FEE = 0.001 ether; function setUp() public { - starkExBridge = new StarkExchangeMigration(); + StarkExchangeMigration implementation = new StarkExchangeMigration(); mockBridge = new MockRootERC20Bridge(); mockSenderAdapter = new MockVaultRootSenderAdapter(); testToken = new MockERC20("Test Token", "TEST", 18); bytes memory initData = abi.encode(MIGRATION_MANAGER, address(mockBridge), address(mockSenderAdapter), WITHDRAWAL_PROCESSOR); - starkExBridge.initialize(initData); + bytes memory initCallData = abi.encodeWithSelector(StarkExchangeMigration.initialize.selector, initData); + ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), initCallData); + starkExBridge = StarkExchangeMigration(address(proxy)); vm.store(address(starkExBridge), bytes32(uint256(13)), bytes32(TEST_VAULT_ROOT)); vm.deal(MIGRATION_MANAGER, 10 ether); } - function test_Initialize_ValidParameters() public { - StarkExchangeMigration newMigration = new StarkExchangeMigration(); - + function test_Constructor_DisablesInitializers_InitializeRevertsWhenCalledOnImplementation() public { + StarkExchangeMigration implementation = new StarkExchangeMigration(); bytes memory initData = abi.encode(MIGRATION_MANAGER, address(mockBridge), address(mockSenderAdapter), WITHDRAWAL_PROCESSOR); - newMigration.initialize(initData); + vm.expectRevert(Initializable.InvalidInitialization.selector); + implementation.initialize(initData); + } - assertEq(newMigration.migrationManager(), MIGRATION_MANAGER, "Migration manager should be set"); - assertEq(newMigration.zkEVMBridge(), address(mockBridge), "zkEVM bridge should be set"); + function test_Initialize_ValidParameters() public view { + assertEq(starkExBridge.migrationManager(), MIGRATION_MANAGER, "Migration manager should be set"); + assertEq(starkExBridge.zkEVMBridge(), address(mockBridge), "zkEVM bridge should be set"); assertEq( - address(newMigration.rootSenderAdapter()), address(mockSenderAdapter), "Root sender adapter should be set" + address(starkExBridge.rootSenderAdapter()), address(mockSenderAdapter), "Root sender adapter should be set" ); - assertEq(newMigration.zkEVMWithdrawalProcessor(), WITHDRAWAL_PROCESSOR, "Withdrawal processor should be set"); + assertEq(starkExBridge.zkEVMWithdrawalProcessor(), WITHDRAWAL_PROCESSOR, "Withdrawal processor should be set"); } function test_RevertIf_Initialize_InvalidMigrationManager() public { - StarkExchangeMigration newMigration = new StarkExchangeMigration(); + StarkExchangeMigration implementation = new StarkExchangeMigration(); + ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), ""); + StarkExchangeMigration newMigration = StarkExchangeMigration(address(proxy)); bytes memory initData = abi.encode( address(0), // Invalid migration manager @@ -147,7 +155,9 @@ contract StarkExchangeMigrationTest is Test { } function test_RevertIf_Initialize_InvalidZkEVMBridge() public { - StarkExchangeMigration newMigration = new StarkExchangeMigration(); + StarkExchangeMigration implementation = new StarkExchangeMigration(); + ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), ""); + StarkExchangeMigration newMigration = StarkExchangeMigration(address(proxy)); bytes memory initData = abi.encode( MIGRATION_MANAGER, @@ -161,7 +171,9 @@ contract StarkExchangeMigrationTest is Test { } function test_RevertIf_Initialize_InvalidRootSenderAdapter() public { - StarkExchangeMigration newMigration = new StarkExchangeMigration(); + StarkExchangeMigration implementation = new StarkExchangeMigration(); + ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), ""); + StarkExchangeMigration newMigration = StarkExchangeMigration(address(proxy)); bytes memory initData = abi.encode( MIGRATION_MANAGER, @@ -175,7 +187,9 @@ contract StarkExchangeMigrationTest is Test { } function test_RevertIf_Initialize_InvalidWithdrawalProcessor() public { - StarkExchangeMigration newMigration = new StarkExchangeMigration(); + StarkExchangeMigration implementation = new StarkExchangeMigration(); + ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), ""); + StarkExchangeMigration newMigration = StarkExchangeMigration(address(proxy)); bytes memory initData = abi.encode( MIGRATION_MANAGER, @@ -189,15 +203,11 @@ contract StarkExchangeMigrationTest is Test { } function test_RevertIf_Initialize_Twice() public { - StarkExchangeMigration newMigration = new StarkExchangeMigration(); - bytes memory initData = abi.encode(MIGRATION_MANAGER, address(mockBridge), address(mockSenderAdapter), WITHDRAWAL_PROCESSOR); - newMigration.initialize(initData); - - vm.expectRevert(); - newMigration.initialize(initData); + vm.expectRevert(Initializable.InvalidInitialization.selector); + starkExBridge.initialize(initData); } function test_MigrateVaultRoot() public { diff --git a/test/unit/verifiers/vaults/VaultEscapeProofVerifier.t.sol b/test/unit/verifiers/vaults/VaultEscapeProofVerifier.t.sol index 642fc9a..0659609 100644 --- a/test/unit/verifiers/vaults/VaultEscapeProofVerifier.t.sol +++ b/test/unit/verifiers/vaults/VaultEscapeProofVerifier.t.sol @@ -68,37 +68,30 @@ contract VaultEscapeProofVerifierTest is Test, FixtureVaultEscapes, FixtureLooku verifier.verifyVaultProof(invalidProofBadPath); } - function test_RevertIf_VerifyProof_WithInvalidLength_Short() public { - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too short.")); + function test_RevertIf_VerifyProof_WithInvalidLength_TooShort() public { + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); verifier.verifyVaultProof(new uint256[](67)); } - function test_RevertIf_VerifyProof_WithInvalidLength_Long() public { - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too long.")); - verifier.verifyVaultProof(new uint256[](200)); + function test_RevertIf_VerifyProof_WithInvalidLength_TooLong() public { + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); + verifier.verifyVaultProof(new uint256[](69)); } - function test_RevertIf_VerifyProof_WithInvalidLength_Odd() public { - vm.expectRevert( - abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof length must be even.") - ); - verifier.verifyVaultProof(new uint256[](69)); + function test_RevertIf_VerifyProof_WithInvalidLength_Empty() public { + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); + verifier.verifyVaultProof(new uint256[](0)); } - function test_VerifyProof_MinimumValidLength() public view { - // Create a proof with minimum valid length (68) - uint256[] memory minProof = new uint256[](68); + function test_VerifyProof_ExactValidLength() public view { + // Create a proof with exact valid length (68) + uint256[] memory validLengthProof = new uint256[](68); // Copy a valid proof structure for (uint256 i = 0; i < 68; i++) { - minProof[i] = fixVaultEscapes[0].proof[i]; + validLengthProof[i] = fixVaultEscapes[0].proof[i]; } - bool result = verifier.verifyVaultProof(minProof); - assertTrue(result, "Minimum length proof should be valid"); - } - - function test_RevertIf_VerifyProof_ExactlyAtLongLimit() public { - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too long.")); - verifier.verifyVaultProof(new uint256[](200)); + bool result = verifier.verifyVaultProof(validLengthProof); + assertTrue(result, "Proof with exact valid length should pass"); } function test_ExtractLeafFromProof() public view { @@ -183,49 +176,30 @@ contract VaultEscapeProofVerifierTest is Test, FixtureVaultEscapes, FixtureLooku } function test_RevertIf_ExtractLeafFromInvalidProof() public { - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too short.")); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); verifier.extractLeafFromProof(new uint256[](67)); } function test_RevertIf_ExtractRootFromInvalidProof() public { - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too short.")); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); verifier.extractRootFromProof(new uint256[](67)); } function test_RevertIf_ExtractLeafAndRootFromInvalidProof() public { - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too short.")); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); verifier.extractVaultAndRootFromProof(new uint256[](67)); } - function test_RevertIf_ExtractFunctions_WithOddLength() public { - uint256[] memory oddLengthProof = new uint256[](69); - - vm.expectRevert( - abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof length must be even.") - ); - verifier.extractLeafFromProof(oddLengthProof); - - vm.expectRevert( - abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof length must be even.") - ); - verifier.extractRootFromProof(oddLengthProof); - - vm.expectRevert( - abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof length must be even.") - ); - verifier.extractVaultAndRootFromProof(oddLengthProof); - } - - function test_RevertIf_ExtractFunctions_WithLongProof() public { - uint256[] memory longProof = new uint256[](200); + function test_RevertIf_ExtractFunctions_WithWrongLength() public { + uint256[] memory wrongLengthProof = new uint256[](69); - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too long.")); - verifier.extractLeafFromProof(longProof); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); + verifier.extractLeafFromProof(wrongLengthProof); - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too long.")); - verifier.extractRootFromProof(longProof); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); + verifier.extractRootFromProof(wrongLengthProof); - vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Proof too long.")); - verifier.extractVaultAndRootFromProof(longProof); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); + verifier.extractVaultAndRootFromProof(wrongLengthProof); } } diff --git a/test/unit/withdrawals/VaultWithdrawalProcessor.t.sol b/test/unit/withdrawals/VaultWithdrawalProcessor.t.sol index 672e330..cbd1eb1 100644 --- a/test/unit/withdrawals/VaultWithdrawalProcessor.t.sol +++ b/test/unit/withdrawals/VaultWithdrawalProcessor.t.sol @@ -42,7 +42,6 @@ contract VaultWithdrawalProcessorTest is unpauser: address(this), disburser: address(this), defaultAdmin: address(this), - vaultRootProvider: address(this), accountRootProvider: address(this), tokenMappingManager: address(this) }); @@ -58,7 +57,7 @@ contract VaultWithdrawalProcessorTest is vaultVerifier = new MockVaultVerifier(ZKEVM_MAINNET_LOOKUP_TABLES); - vaultWithdrawalProcessor = new VaultWithdrawalProcessor(address(vaultVerifier), initRoles, true); + vaultWithdrawalProcessor = new VaultWithdrawalProcessor(address(vaultVerifier), address(this), initRoles, true); vaultWithdrawalProcessor.setVaultRoot(fixVaultEscapes[0].root); vaultWithdrawalProcessor.setAccountRoot(accountsRoot); vaultWithdrawalProcessor.registerTokenMappings(fixAssets); @@ -68,7 +67,9 @@ contract VaultWithdrawalProcessorTest is function test_Constructor() public view { assertEq(address(vaultWithdrawalProcessor.VAULT_PROOF_VERIFIER()), address(vaultVerifier)); + assertEq(vaultWithdrawalProcessor.VAULT_ROOT_PROVIDER(), address(this)); assertEq(vaultWithdrawalProcessor.vaultRoot(), fixVaultEscapes[0].root); + assertEq(vaultWithdrawalProcessor.rootOverrideAllowed(), true, "rootOverrideAllowed should be true initially"); for (uint256 i = 0; i < fixAssets.length; i++) { uint256 id = fixAssets[i].tokenOnIMX.id; @@ -77,15 +78,10 @@ contract VaultWithdrawalProcessorTest is } } - function test_Constants() public view { - assertEq(vaultWithdrawalProcessor.VAULT_PROOF_LENGTH(), 68, "VAULT_PROOF_LENGTH should be 68"); - assertEq(vaultWithdrawalProcessor.rootOverrideAllowed(), true, "rootOverrideAllowed should be true initially"); - } - function test_Constructor_RootOverrideAllowed() public { // Test with rootOverrideAllowed = false VaultWithdrawalProcessor processorWithOverrideDisabled = - new VaultWithdrawalProcessor(address(vaultVerifier), initRoles, false); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), initRoles, false); assertEq( processorWithOverrideDisabled.rootOverrideAllowed(), false, @@ -239,9 +235,7 @@ contract VaultWithdrawalProcessorTest is function test_RevertIf_EmptyVaultProof() public { uint256[] memory emptyProof = new uint256[](0); - vm.expectRevert( - abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid vault proof length") - ); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); vaultWithdrawalProcessor.verifyAndProcessWithdrawal(recipient, sampleAccount.proof, emptyProof); } @@ -252,9 +246,7 @@ contract VaultWithdrawalProcessorTest is wrongLengthProof[i] = i; } - vm.expectRevert( - abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid vault proof length") - ); + vm.expectRevert(abi.encodeWithSelector(IVaultProofVerifier.InvalidVaultProof.selector, "Invalid proof length.")); vaultWithdrawalProcessor.verifyAndProcessWithdrawal(recipient, sampleAccount.proof, wrongLengthProof); } @@ -321,7 +313,12 @@ contract VaultWithdrawalProcessorTest is function test_RevertIf_Constructor_ZeroVaultVerifier() public { vm.expectRevert(BaseVaultWithdrawalProcessor.ZeroAddress.selector); - new VaultWithdrawalProcessor(address(0), initRoles, true); + new VaultWithdrawalProcessor(address(0), address(this), initRoles, true); + } + + function test_RevertIf_Constructor_ZeroVaultRootProvider() public { + vm.expectRevert(BaseVaultWithdrawalProcessor.ZeroAddress.selector); + new VaultWithdrawalProcessor(address(vaultVerifier), address(0), initRoles, true); } function test_RevertIf_Constructor_InvalidPauserOperator() public { @@ -330,13 +327,12 @@ contract VaultWithdrawalProcessorTest is unpauser: address(this), disburser: address(this), defaultAdmin: address(this), - vaultRootProvider: address(this), accountRootProvider: address(this), tokenMappingManager: address(this) }); vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), invalidRoles, true); } function test_RevertIf_Constructor_InvalidUnpauserOperator() public { @@ -345,13 +341,12 @@ contract VaultWithdrawalProcessorTest is unpauser: address(0), // Invalid: zero address disburser: address(this), defaultAdmin: address(this), - vaultRootProvider: address(this), accountRootProvider: address(this), tokenMappingManager: address(this) }); vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), invalidRoles, true); } function test_RevertIf_Constructor_InvalidDisburserOperator() public { @@ -360,13 +355,12 @@ contract VaultWithdrawalProcessorTest is unpauser: address(this), disburser: address(0), // Invalid: zero address defaultAdmin: address(this), - vaultRootProvider: address(this), accountRootProvider: address(this), tokenMappingManager: address(this) }); vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), invalidRoles, true); } function test_RevertIf_Constructor_InvalidDefaultAdminOperator() public { @@ -375,28 +369,12 @@ contract VaultWithdrawalProcessorTest is unpauser: address(this), disburser: address(this), defaultAdmin: address(0), // Invalid: zero address - vaultRootProvider: address(this), accountRootProvider: address(this), tokenMappingManager: address(this) }); vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); - } - - function test_RevertIf_Constructor_InvalidVaultRootProviderOperator() public { - ProcessorAccessControl.RoleOperators memory invalidRoles = ProcessorAccessControl.RoleOperators({ - pauser: address(this), - unpauser: address(this), - disburser: address(this), - defaultAdmin: address(this), - vaultRootProvider: address(0), // Invalid: zero address - accountRootProvider: address(this), - tokenMappingManager: address(this) - }); - - vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), invalidRoles, true); } function test_RevertIf_Constructor_InvalidAccountRootProviderOperator() public { @@ -405,13 +383,12 @@ contract VaultWithdrawalProcessorTest is unpauser: address(this), disburser: address(this), defaultAdmin: address(this), - vaultRootProvider: address(this), accountRootProvider: address(0), // Invalid: zero address tokenMappingManager: address(this) }); vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), invalidRoles, true); } function test_RevertIf_Constructor_InvalidTokenMappingManagerOperator() public { @@ -420,13 +397,12 @@ contract VaultWithdrawalProcessorTest is unpauser: address(this), disburser: address(this), defaultAdmin: address(this), - vaultRootProvider: address(this), accountRootProvider: address(this), tokenMappingManager: address(0) // Invalid: zero address }); vm.expectRevert(ProcessorAccessControl.InvalidOperatorAddress.selector); - new VaultWithdrawalProcessor(address(vaultVerifier), invalidRoles, true); + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), invalidRoles, true); } function test_SetVaultRoot() public view { @@ -436,13 +412,7 @@ contract VaultWithdrawalProcessorTest is function test_RevertIf_SetVaultRoot_Unauthorized() public { uint256 newRoot = 0x123; vm.startPrank(address(0x123)); - vm.expectRevert( - abi.encodeWithSelector( - IAccessControl.AccessControlUnauthorizedAccount.selector, - address(0x123), - vaultWithdrawalProcessor.VAULT_ROOT_PROVIDER_ROLE() - ) - ); + vm.expectRevert(VaultWithdrawalProcessor.UnauthorizedVaultRootProvider.selector); vaultWithdrawalProcessor.setVaultRoot(newRoot); } @@ -536,7 +506,8 @@ contract VaultWithdrawalProcessorTest is function test_RevertIf_VaultRootNotSet() public { // Create a new processor without setting vault root - VaultWithdrawalProcessor newProcessor = new VaultWithdrawalProcessor(address(vaultVerifier), initRoles, true); + VaultWithdrawalProcessor newProcessor = + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), initRoles, true); newProcessor.setAccountRoot(accountsRoot); newProcessor.registerTokenMappings(fixAssets); @@ -549,7 +520,8 @@ contract VaultWithdrawalProcessorTest is function test_RevertIf_AccountRootNotSet() public { // Create a new processor without setting account root - VaultWithdrawalProcessor newProcessor = new VaultWithdrawalProcessor(address(vaultVerifier), initRoles, true); + VaultWithdrawalProcessor newProcessor = + new VaultWithdrawalProcessor(address(vaultVerifier), address(this), initRoles, true); newProcessor.setVaultRoot(fixVaultEscapes[0].root); newProcessor.registerTokenMappings(fixAssets);