From b74a0f09c8634ebc9f55d319c90c6da42cf2a94c Mon Sep 17 00:00:00 2001 From: Todd <148772493+toddfil@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:13:09 +0800 Subject: [PATCH 01/18] chore: update docs (#465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ - update docs ## Why ❔ - incorrect link reference in zk_intuition.md ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: Igor Aleksanov --- docs/advanced/zk_intuition.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/advanced/zk_intuition.md b/docs/advanced/zk_intuition.md index 7ea7dad0a449..4a8996ff8dbe 100644 --- a/docs/advanced/zk_intuition.md +++ b/docs/advanced/zk_intuition.md @@ -139,8 +139,7 @@ version 1.4.0. [witness_example]: https://github.com/matter-labs/era-zkevm_test_harness/tree/main/src/witness/individual_circuits/decommit_code.rs#L24 -[verifier]: - https://github.com/matter-labs/zksync-2-contracts/blob/d9785355518edc7f686fb2c91ff7d1caced9f9b8/ethereum/contracts/zksync/Plonk4VerifierWithAccessToDNext.sol#L284 +[verifier]: https://github.com/matter-labs/era-contracts/blob/main/ethereum/contracts/zksync/Verifier.sol [bellman repo]: https://github.com/matter-labs/bellman [bellman cuda repo]: https://github.com/matter-labs/era-bellman-cuda [example ecrecover circuit]: From 0d55d6df980b70a060712ca4beb80d72d704b9d4 Mon Sep 17 00:00:00 2001 From: Jean <148654781+oxJean@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:14:45 +0800 Subject: [PATCH 02/18] chore: fixed typo in code notes (#497) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ fixed typo in code notes ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: Igor Aleksanov --- infrastructure/protocol-upgrade/src/transaction.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructure/protocol-upgrade/src/transaction.ts b/infrastructure/protocol-upgrade/src/transaction.ts index 9162da1c46f8..f4fd30dcde1b 100644 --- a/infrastructure/protocol-upgrade/src/transaction.ts +++ b/infrastructure/protocol-upgrade/src/transaction.ts @@ -60,7 +60,7 @@ export interface L2CanonicalTransaction { // is to be passed to account and any changes to its structure // would mean a breaking change to these accounts. In order to prevent this, // we should keep some fields as "reserved". - // It is also recommneded that their length is fixed, since + // It is also recommended that their length is fixed, since // it would allow easier proof integration (in case we will need // some special circuit for preprocessing transactions). reserved: [BigNumberish, BigNumberish, BigNumberish, BigNumberish]; From c067007a3e9bc41db35d25f1beb79eb5d2dc5bb2 Mon Sep 17 00:00:00 2001 From: Karma <148863819+0xKarm@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:15:15 +0800 Subject: [PATCH 03/18] chore(docs): fixed typos in documentation (#479) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ - fix typo in README.md ## Why ❔ - fix typo ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: Igor Aleksanov --- core/tests/loadnext/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/tests/loadnext/README.md b/core/tests/loadnext/README.md index 99c0a47d5b7c..397226f66896 100644 --- a/core/tests/loadnext/README.md +++ b/core/tests/loadnext/README.md @@ -8,7 +8,7 @@ The general flow is as follows: - The master account performs an initial deposit to L2 - Paymaster on L2 is funded if necessary - The L2 master account distributes funds to the participating accounts (`accounts_amount` configuration option) -- Each account continiously sends L2 transactions as configured in `contract_execution_params` configuration option. At +- Each account continuously sends L2 transactions as configured in `contract_execution_params` configuration option. At any given time there are no more than `max_inflight_txs` transactions in flight for each account. - Once each account is done with the initial deposit, the test is run for `duration_sec` seconds. - After the test is finished, the master account withdraws all the remaining funds from L2. From f4f322ae03f08abe8c2ae391798297f345ee8da7 Mon Sep 17 00:00:00 2001 From: Doll <148654386+Dollyerls@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:16:35 +0800 Subject: [PATCH 04/18] chore: fixed typo (#486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Fixed typo ## Why ❔ Affect reading ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: Igor Aleksanov --- core/tests/ts-integration/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/tests/ts-integration/README.md b/core/tests/ts-integration/README.md index b3707cac664f..5b34bd7ac02e 100644 --- a/core/tests/ts-integration/README.md +++ b/core/tests/ts-integration/README.md @@ -134,7 +134,7 @@ finalization: it make take several hours to generate a proof and send it onchain Because of that, framework supports "fast" and "long" modes. `TestMaster` objects have `isFastMode` method to determine which mode is currently being used. -If you're going to write a test that can make test run duration longer, it is adviced to guard the "long" part with the +If you're going to write a test that can make test run duration longer, it is advised to guard the "long" part with the corresponding check. By default, "long" mode is assumed, and to enable the "fast" mode one must set the `ZK_INTEGRATION_TESTS_FAST_MODE` From dbe89e2dd802dcf4a4bbe23b05770a24d27b390a Mon Sep 17 00:00:00 2001 From: Salad <148864073+Saladerl@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:17:35 +0800 Subject: [PATCH 05/18] chore(docs): fix docs (#487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ fixed docs ## Why ❔ fixed typo ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: Igor Aleksanov --- docs/advanced/02_deposits.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/advanced/02_deposits.md b/docs/advanced/02_deposits.md index 9d9ac4d526b8..a7b729495932 100644 --- a/docs/advanced/02_deposits.md +++ b/docs/advanced/02_deposits.md @@ -162,7 +162,7 @@ The zk server (that you started with `zk server` command) is listening on events ) and adds them to the postgres database (into `transactions` table). You can actually check it - by running the psql and looking at the contents of the table - then you'll notice that -transaction was succesfully inserted, and it was also marked as 'priority' (as it came from L1) - as regular +transaction was successfully inserted, and it was also marked as 'priority' (as it came from L1) - as regular transactions that are received by the server directly are not marked as priority. You can verify that this is your transaction, by looking at the `l1_block_number` column (it should match the From 4b00ee01b5d07f829ff78d7ab05231f8854fee08 Mon Sep 17 00:00:00 2001 From: momodaka <463435681@qq.com> Date: Thu, 30 Nov 2023 14:17:37 +0800 Subject: [PATCH 06/18] docs: fix typo (#488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ fix typo issue `succesfully` -> `successfully` `occurence` -> `occurence` ## Why ❔ ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: jiachen Co-authored-by: Igor Aleksanov --- docs/advanced/bytecode_compression.md | 2 +- docs/advanced/zk_intuition.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/advanced/bytecode_compression.md b/docs/advanced/bytecode_compression.md index 6f94277f8017..2c9e42acd5d6 100644 --- a/docs/advanced/bytecode_compression.md +++ b/docs/advanced/bytecode_compression.md @@ -31,7 +31,7 @@ Dictionary would be: 3 -> 0xC (count: 1) ``` -Note that '1' maps to '0xD', as it occurs twice, and first occurrence is earlier than first occurence of 0xB, that also +Note that '1' maps to '0xD', as it occurs twice, and first occurrence is earlier than first occurrence of 0xB, that also occurs twice. Compressed bytecode: diff --git a/docs/advanced/zk_intuition.md b/docs/advanced/zk_intuition.md index 4a8996ff8dbe..58777b264fc1 100644 --- a/docs/advanced/zk_intuition.md +++ b/docs/advanced/zk_intuition.md @@ -7,7 +7,7 @@ understanding. We're leaving out a lot of details to keep things brief. In our case, the prover takes public input and witness (which is huge - you'll see below), and produces a proof, but the verifier takes (public input, proof) only, without witness. This means that the huge witness doesn't have to be -submitted to L1. This property can be used for many things, like privacy, but here we use it to ipmlement an efficient +submitted to L1. This property can be used for many things, like privacy, but here we use it to implement an efficient rollup that publishes the least required amount of data to L1. ## Basic overview @@ -85,7 +85,7 @@ located in a module [zksync core witness]. However, for the new proof system, th new location called [separate witness binary]. Inside this new location, after the necessary data is fetched from storage, the witness generator calls another piece of -code from [zkevm_test_harness witness] named `run_with_fixed_params`. This code is responsible for createing the +code from [zkevm_test_harness witness] named `run_with_fixed_params`. This code is responsible for creating the witnesses themselves (which can get really HUGE). ## Generating the Proof From dddb797818661ba966e1a6202d340bc28ccaa971 Mon Sep 17 00:00:00 2001 From: buldazer <93915704+buldazer23@users.noreply.github.com> Date: Thu, 30 Nov 2023 09:19:14 +0300 Subject: [PATCH 07/18] chore: fix minor typo (#518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ fix minor typo ## Why ❔ minor typo succesfully---->successfully were----->where occurence---->occurrence separte---->separate constrain system---->constraint system constraing system---->constraint system Co-authored-by: Igor Aleksanov --- docs/advanced/03_withdrawals.md | 2 +- docs/advanced/contracts.md | 2 +- docs/advanced/prover.md | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/advanced/03_withdrawals.md b/docs/advanced/03_withdrawals.md index 003121d86467..46833809e0ac 100644 --- a/docs/advanced/03_withdrawals.md +++ b/docs/advanced/03_withdrawals.md @@ -81,7 +81,7 @@ This is a good opportunity to talk about system contracts that are automatically list here [in github](https://github.com/matter-labs/era-system-contracts/blob/436d57da2fb35c40e38bcb6637c3a090ddf60701/scripts/constants.ts#L29) -This is the place were we specify that `bootloader` is at address 0x8001, `NonceHolder` at 0x8003 etc. +This is the place where we specify that `bootloader` is at address 0x8001, `NonceHolder` at 0x8003 etc. This brings us to [L2EthToken.sol](https://github.com/matter-labs/era-system-contracts/blob/main/contracts/L2EthToken.sol) that has the diff --git a/docs/advanced/contracts.md b/docs/advanced/contracts.md index 9b44268827c0..e32992fb79bd 100644 --- a/docs/advanced/contracts.md +++ b/docs/advanced/contracts.md @@ -32,7 +32,7 @@ a bunch of registers. More details on this will be written in the future article Having a different VM means that we must have a separate compiler [zk-solc](https://github.com/matter-labs/zksolc-bin) - as the bytecode that is produced by this compiler has to use the zkEVM specific opcodes. -While having a separte compiler introduces a bunch of challenges (for example, we need a custom +While having a separate compiler introduces a bunch of challenges (for example, we need a custom [hardhat plugins](https://github.com/matter-labs/hardhat-zksync) ), it brings a bunch of benefits too: for example it allows us to move some of the VM logic (like new contract deployment) into System contracts - which allows faster & cheaper modifications and increased flexibility. diff --git a/docs/advanced/prover.md b/docs/advanced/prover.md index 02e69c4d38e2..6211f00dea78 100644 --- a/docs/advanced/prover.md +++ b/docs/advanced/prover.md @@ -86,7 +86,7 @@ pub fn select>( ``` And then there is a block of code for witness evaluation (let's skip it for now), and the final block that adds the gate -to the constrain system `cs`: +to the constraint system `cs`: ```rust if ::SetupConfig::KEEP_SETUP { @@ -204,7 +204,7 @@ filled with concrete values. ### CsAllocatable -Implements CsAllocatable - which allows you to directly 'allocate' this struct within constraing system (similarly to +Implements CsAllocatable - which allows you to directly 'allocate' this struct within constraint system (similarly to how we were operating on regular 'Variables' above). ### CSSelectable From 6095d690154cd16d6e6688ec4830f5e8f226e2ac Mon Sep 17 00:00:00 2001 From: Mc Kenna <150222622+McKenna8989@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:26:21 +0800 Subject: [PATCH 08/18] chore(docs): the typos have been corrected (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ - the typos have been corrected ## Why ❔ - the typos have been corrected ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --------- Co-authored-by: Igor Aleksanov --- .../contracts/custom-account/interfaces/IPaymaster.sol | 2 +- core/tests/ts-integration/src/test-master.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/tests/ts-integration/contracts/custom-account/interfaces/IPaymaster.sol b/core/tests/ts-integration/contracts/custom-account/interfaces/IPaymaster.sol index cf5ced948782..1bd5b81f32b4 100644 --- a/core/tests/ts-integration/contracts/custom-account/interfaces/IPaymaster.sol +++ b/core/tests/ts-integration/contracts/custom-account/interfaces/IPaymaster.sol @@ -37,7 +37,7 @@ interface IPaymaster { /// @param _context, the context of the execution, returned by the "validateAndPayForPaymasterTransaction" method. /// @param _transaction, the users' transaction. /// @param _txResult, the result of the transaction execution (success or failure). - /// @param _maxRefundedGas, the upper bound on the amout of gas that could be refunded to the paymaster. + /// @param _maxRefundedGas, the upper bound on the amount of gas that could be refunded to the paymaster. /// @dev The exact amount refunded depends on the gas spent by the "postOp" itself and so the developers should /// take that into account. function postTransaction( diff --git a/core/tests/ts-integration/src/test-master.ts b/core/tests/ts-integration/src/test-master.ts index 8f59288ba5c3..8919bbffd1e9 100644 --- a/core/tests/ts-integration/src/test-master.ts +++ b/core/tests/ts-integration/src/test-master.ts @@ -30,7 +30,7 @@ export class TestMaster { const contextStr = process.env.ZKSYNC_JEST_TEST_CONTEXT; if (!contextStr) { - throw new Error('Test context was not initalized; unable to load context environment variable'); + throw new Error('Test context was not initialized; unable to load context environment variable'); } const context = JSON.parse(contextStr) as TestContext; From f15885e4850a70f321da0c8d4b4d2b48df686df4 Mon Sep 17 00:00:00 2001 From: 0xmbcode <152050562+0xmbcode@users.noreply.github.com> Date: Thu, 30 Nov 2023 09:35:28 +0300 Subject: [PATCH 09/18] chore: Fix broken link in repositories.md (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hello,
 ## Description This change is made to ensure that developers, contributors can access the correct documentation.
 ## Changes Made This pull request fixes a broken link. 

 ## Why The broken link was causing confusion and potentially preventing developers, contributors from accessing the zksync-cli. Correct link:
https://github.com/matter-labs/zksync-cli ## Testing Done - Manually reviewed the documentation and verified the correct link.
 - No additional dependencies or changes are required. - This fix is straightforward and does not impact any other functionalities. Thank you. ## Checklist - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zk fmt` and `zk lint`. - [ ] Spellcheck has been run via `cargo spellcheck --cfg=./spellcheck/era.cfg --code 1`. --- docs/repositories.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/repositories.md b/docs/repositories.md index 7250c5aef221..0902f38dcd80 100644 --- a/docs/repositories.md +++ b/docs/repositories.md @@ -51,7 +51,7 @@ | [local-setup](https://github.com/matter-labs/local-setup) | Docker-based zk server (together with L1), that can be used for local testing | | [zksolc-bin](https://github.com/matter-labs/zksolc-bin) | repository with solc compiler binaries | | [zkvyper-bin](https://github.com/matter-labs/zkvyper-bin) | repository with vyper compiler binaries | -| [zksync-cli](<(https://github.com/matter-labs/zksync-cli)>) | Command line tool to interact with zksync | +| [zksync-cli](https://github.com/matter-labs/zksync-cli) | Command line tool to interact with zksync | | [hardhat-zksync](https://github.com/matter-labs/hardhat-zksync) | Plugins for hardhat | ### Examples & documentation From cae51479570060fe9e78816714f738cd6dff22d0 Mon Sep 17 00:00:00 2001 From: RakeshXBT <85406290+Rakesh-lab-stack@users.noreply.github.com> Date: Thu, 30 Nov 2023 12:06:27 +0530 Subject: [PATCH 10/18] chore: fix minor typo in rust code in docs (#566) Co-authored-by: Igor Aleksanov --- docs/advanced/gas_and_fees.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/advanced/gas_and_fees.md b/docs/advanced/gas_and_fees.md index 800d27299c2a..b8f0e531e98f 100644 --- a/docs/advanced/gas_and_fees.md +++ b/docs/advanced/gas_and_fees.md @@ -86,7 +86,7 @@ transaction. ```rust let gas_remaining_before = vm.gas_remaining(); execute_tx(); -let gas_used = gas_remainig_before = vm.gas_remaining(); +let gas_used = gas_remaining_before - vm.gas_remaining(); ``` ## Gas estimation From 26767b6952c3588e7bc1c9dfe6c3261931cf78d2 Mon Sep 17 00:00:00 2001 From: Tudor <32748771+RedaOps@users.noreply.github.com> Date: Thu, 30 Nov 2023 09:40:24 +0200 Subject: [PATCH 11/18] refactor(eth_client): Use `BlockId` for `block_id` instead of String in `eth_client` (#499) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ The `block` function within `eth_client` now uses the type `BlockId` instead of `String` for its `block_id` parameter. ## Why ❔ TODO comment: https://github.com/matter-labs/zksync-era/blob/cb873bd0da6b421160ce96b8d578f1351861f376/core/lib/eth_client/src/clients/http/query.rs#L289-L308 This PR fixes the TODO. The `web3` crate being used inside the project was updated to version `0.19.0` inside `core/lib/basic_types/Cargo.toml` in commit 829ef5085f938ddda1f2a695930c6b4308e1643a. The `web3` crate now supports `BlockNumber::Finalized`. Source: https://docs.rs/web3/latest/web3/types/enum.BlockNumber.html Source: https://github.com/tomusdrw/rust-web3/releases/tag/v0.19.0 ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Code has been formatted via `zk fmt` and `zk lint`. Co-authored-by: Igor Aleksanov --- core/lib/eth_client/src/clients/http/query.rs | 16 +++------------- core/lib/eth_client/src/clients/http/signing.rs | 2 +- core/lib/eth_client/src/clients/mock.rs | 4 ++-- core/lib/eth_client/src/lib.rs | 2 +- .../zksync_core/src/eth_sender/eth_tx_manager.rs | 8 ++++++-- core/lib/zksync_core/src/eth_watch/client.rs | 4 ++-- 6 files changed, 15 insertions(+), 21 deletions(-) diff --git a/core/lib/eth_client/src/clients/http/query.rs b/core/lib/eth_client/src/clients/http/query.rs index 198f5fc45af0..0094c76f88a6 100644 --- a/core/lib/eth_client/src/clients/http/query.rs +++ b/core/lib/eth_client/src/clients/http/query.rs @@ -14,13 +14,12 @@ use zksync_types::web3::{ Contract, Options, }, ethabi, - helpers::CallFuture, transports::Http, types::{ Address, Block, BlockId, BlockNumber, Bytes, Filter, Log, Transaction, TransactionId, TransactionReceipt, H256, U256, U64, }, - Transport, Web3, + Web3, }; /// An "anonymous" Ethereum client that can invoke read-only methods that aren't @@ -286,23 +285,14 @@ impl EthInterface for QueryClient { Ok(logs) } - // TODO (PLA-333): at the moment the latest version of `web3` crate doesn't have `Finalized` variant in `BlockNumber`. - // However, it's already added in github repo and probably will be included in the next released version. - // Scope of PLA-333 includes forking/using crate directly from github, after that we will be able to change - // type of `block_id` from `String` to `BlockId` and use `self.web3.eth().block(block_id)`. async fn block( &self, - block_id: String, + block_id: BlockId, component: &'static str, ) -> Result>, Error> { COUNTERS.call[&(Method::Block, component)].inc(); let latency = LATENCIES.direct[&Method::Block].start(); - let block = CallFuture::new( - self.web3 - .transport() - .execute("eth_getBlockByNumber", vec![block_id.into(), false.into()]), - ) - .await?; + let block = self.web3.eth().block(block_id).await?; latency.observe(); Ok(block) } diff --git a/core/lib/eth_client/src/clients/http/signing.rs b/core/lib/eth_client/src/clients/http/signing.rs index fcc38efb4cc8..a0a6647db5fd 100644 --- a/core/lib/eth_client/src/clients/http/signing.rs +++ b/core/lib/eth_client/src/clients/http/signing.rs @@ -213,7 +213,7 @@ impl EthInterface for SigningClient { async fn block( &self, - block_id: String, + block_id: BlockId, component: &'static str, ) -> Result>, Error> { self.query_client.block(block_id, component).await diff --git a/core/lib/eth_client/src/clients/mock.rs b/core/lib/eth_client/src/clients/mock.rs index 07297a3645fb..576fbac21a70 100644 --- a/core/lib/eth_client/src/clients/mock.rs +++ b/core/lib/eth_client/src/clients/mock.rs @@ -342,7 +342,7 @@ impl EthInterface for MockEthereum { async fn block( &self, - _block_id: String, + _block_id: BlockId, _component: &'static str, ) -> Result>, Error> { unimplemented!("Not needed right now") @@ -524,7 +524,7 @@ impl + Send + Sync> EthInterface for T { async fn block( &self, - block_id: String, + block_id: BlockId, component: &'static str, ) -> Result>, Error> { self.as_ref().block(block_id, component).await diff --git a/core/lib/eth_client/src/lib.rs b/core/lib/eth_client/src/lib.rs index a03503683255..f61814893bb4 100644 --- a/core/lib/eth_client/src/lib.rs +++ b/core/lib/eth_client/src/lib.rs @@ -131,7 +131,7 @@ pub trait EthInterface: Sync + Send { /// Returns the block header for the specified block number or hash. async fn block( &self, - block_id: String, + block_id: BlockId, component: &'static str, ) -> Result>, Error>; } diff --git a/core/lib/zksync_core/src/eth_sender/eth_tx_manager.rs b/core/lib/zksync_core/src/eth_sender/eth_tx_manager.rs index 98e75702a4c8..5aab4a2903c8 100644 --- a/core/lib/zksync_core/src/eth_sender/eth_tx_manager.rs +++ b/core/lib/zksync_core/src/eth_sender/eth_tx_manager.rs @@ -12,7 +12,11 @@ use zksync_eth_client::{ }; use zksync_types::{ eth_sender::EthTx, - web3::{contract::Options, error::Error as Web3Error}, + web3::{ + contract::Options, + error::Error as Web3Error, + types::{BlockId, BlockNumber}, + }, L1BlockNumber, Nonce, H256, U256, }; use zksync_utils::time::seconds_since_epoch; @@ -285,7 +289,7 @@ where (latest_block_number.saturating_sub(confirmations) as u32).into() } else { self.ethereum_gateway - .block("finalized".to_string(), "eth_tx_manager") + .block(BlockId::Number(BlockNumber::Finalized), "eth_tx_manager") .await? .expect("Finalized block must be present on L1") .number diff --git a/core/lib/zksync_core/src/eth_watch/client.rs b/core/lib/zksync_core/src/eth_watch/client.rs index af38ac79ae7d..cbd3785640e2 100644 --- a/core/lib/zksync_core/src/eth_watch/client.rs +++ b/core/lib/zksync_core/src/eth_watch/client.rs @@ -5,7 +5,7 @@ use zksync_types::{ vk_transform::l1_vk_commitment, web3::{ self, - types::{BlockNumber, FilterBuilder, Log}, + types::{BlockId, BlockNumber, FilterBuilder, Log}, }, Address, H256, }; @@ -225,7 +225,7 @@ impl EthClient for EthHttpQueryClient Date: Thu, 30 Nov 2023 11:11:21 +0100 Subject: [PATCH 12/18] ci: Remove leftovers of explicit publishing to public Docker registries (#496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Subj ## Why ❔ All DockerHub registries of FOSS repos are now public ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --- .github/workflows/build-core-template.yml | 2 +- infrastructure/zk/src/docker.ts | 24 ++++++++++------------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build-core-template.yml b/.github/workflows/build-core-template.yml index c4ae27faf9c3..95bb28947953 100644 --- a/.github/workflows/build-core-template.yml +++ b/.github/workflows/build-core-template.yml @@ -83,7 +83,7 @@ jobs: COMPONENT: ${{ matrix.component }} run: | ci_run rustup default nightly-2023-08-21 - ci_run zk docker $DOCKER_ACTION $COMPONENT -- --public + ci_run zk docker $DOCKER_ACTION $COMPONENT - name: Show sccache stats if: always() run: | diff --git a/infrastructure/zk/src/docker.ts b/infrastructure/zk/src/docker.ts index 3db9a69313d5..9a4bf6e291a4 100644 --- a/infrastructure/zk/src/docker.ts +++ b/infrastructure/zk/src/docker.ts @@ -25,19 +25,19 @@ async function dockerCommand( command: 'push' | 'build', image: string, customTag?: string, - publishPublic: boolean = false, dockerOrg: string = 'matterlabs' ) { // Generating all tags for containers. We need 2 tags here: SHA and SHA+TS const { stdout: COMMIT_SHORT_SHA }: { stdout: string } = await utils.exec('git rev-parse --short HEAD'); + // COMMIT_SHORT_SHA returns with newline, so we need to trim it const imageTagShaTS: string = process.env.IMAGE_TAG_SUFFIX ? process.env.IMAGE_TAG_SUFFIX : `${COMMIT_SHORT_SHA.trim()}-${UNIX_TIMESTAMP}`; - // we want alternative flow for rust image + // We want an alternative flow for Rust image if (image == 'rust') { - await dockerCommand(command, 'server-v2', customTag, publishPublic); - await dockerCommand(command, 'prover', customTag, publishPublic); + await dockerCommand(command, 'server-v2', customTag, dockerOrg); + await dockerCommand(command, 'prover', customTag, dockerOrg); return; } if (!IMAGES.includes(image)) { @@ -49,14 +49,14 @@ async function dockerCommand( } const tagList = customTag ? [customTag] : defaultTagList(image, COMMIT_SHORT_SHA.trim(), imageTagShaTS); + // Main build\push flow - // COMMIT_SHORT_SHA returns with newline, so we need to trim it switch (command) { case 'build': await _build(image, tagList, dockerOrg); break; case 'push': - await _push(image, tagList, publishPublic); + await _push(image, tagList); break; default: console.log(`Unknown command for docker ${command}.`); @@ -114,7 +114,7 @@ async function _build(image: string, tagList: string[], dockerOrg: string) { await utils.spawn(buildCommand); } -async function _push(image: string, tagList: string[], publishPublic: boolean = false) { +async function _push(image: string, tagList: string[]) { // For development purposes, we want to use `2.0` tags for 2.0 images, just to not interfere with 1.x for (const tag of tagList) { @@ -134,9 +134,6 @@ async function _push(image: string, tagList: string[], publishPublic: boolean = await utils.spawn(`docker push asia-docker.pkg.dev/matterlabs-infra/matterlabs-docker/${image}:${tag}`); await utils.spawn(`docker push europe-docker.pkg.dev/matterlabs-infra/matterlabs-docker/${image}:${tag}`); } - if (image == 'external-node' && publishPublic) { - await utils.spawn(`docker push matterlabs/${image}-public:${tag}`); - } } } @@ -145,12 +142,12 @@ export async function build(image: string, cmd: Command) { } export async function customBuildForHyperchain(image: string, dockerOrg: string) { - await dockerCommand('build', image, '', false, dockerOrg); + await dockerCommand('build', image, '', dockerOrg); } export async function push(image: string, cmd: Command) { - await dockerCommand('build', image, cmd.customTag, cmd.public); - await dockerCommand('push', image, cmd.customTag, cmd.public); + await dockerCommand('build', image, cmd.customTag); + await dockerCommand('push', image, cmd.customTag); } export async function restart(container: string) { @@ -171,7 +168,6 @@ command command .command('push ') .option('--custom-tag ', 'Custom tag for image') - .option('--public', 'Publish image to the public repo') .description('build and push docker image') .action(push); command.command('pull').description('pull all containers').action(pull); From 83791aa6704221755674dd5b1eb428e286f791da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Grze=C5=9Bkiewicz?= Date: Thu, 30 Nov 2023 11:49:23 +0100 Subject: [PATCH 13/18] feat: zk db check-sqlx-check on pre-push (#548) --- .githooks/pre-push | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.githooks/pre-push b/.githooks/pre-push index eb1acbb693c1..94cc937c9b1d 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # # Pre-push hook verifying that inappropriate code will not be pushed. @@ -8,7 +8,13 @@ NC='\033[0m' # No Color # Check that prettier formatting rules are not violated. if ! zk fmt --check; then - echo -e "${RED}Commit error!${NC}" + echo -e "${RED}Push error!${NC}" echo "Please format the code via 'zk fmt', cannot push unformatted code" exit 1 fi + +if ! zk db check-sqlx-data; then + echo -e "${RED}Push error!${NC}" + echo "Please update sqlx-data.json via 'zk db setup', cannot push invalid sqlx-data.json file" + exit 1 +fi From e2c1b20e361e6ee2f5ac69cefe75d9c5575eb2f7 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 30 Nov 2023 14:34:55 +0200 Subject: [PATCH 14/18] feat(merkle tree): Remove enumeration index assignment from Merkle tree (#551) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Since enumeration indices are now fully stored in Postgres, it makes sense to not duplicate their assignment in the Merkle tree. Instead, the tree could take enum indices as inputs. ## Why ❔ This allows simplifying tree logic and unify "normal" L1 batch processing and tree recovery. (This unification is not a part of this PR; it'll be implemented separately.) ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --- .../lib/merkle_tree/examples/loadtest/main.rs | 24 +- core/lib/merkle_tree/examples/recovery.rs | 10 +- core/lib/merkle_tree/src/consistency.rs | 35 ++- core/lib/merkle_tree/src/domain.rs | 145 ++++++------ core/lib/merkle_tree/src/getters.rs | 17 +- core/lib/merkle_tree/src/hasher/mod.rs | 24 +- core/lib/merkle_tree/src/hasher/proofs.rs | 62 +++-- core/lib/merkle_tree/src/lib.rs | 13 +- core/lib/merkle_tree/src/pruning.rs | 28 +-- core/lib/merkle_tree/src/recovery.rs | 35 +-- core/lib/merkle_tree/src/storage/mod.rs | 62 ++--- core/lib/merkle_tree/src/storage/patch.rs | 21 +- core/lib/merkle_tree/src/storage/proofs.rs | 224 ++---------------- .../merkle_tree/src/storage/serialization.rs | 11 +- core/lib/merkle_tree/src/storage/tests.rs | 128 +++++----- core/lib/merkle_tree/src/types/internal.rs | 22 +- core/lib/merkle_tree/src/types/mod.rs | 93 ++++++-- core/lib/merkle_tree/src/utils.rs | 5 - .../merkle_tree/tests/integration/common.rs | 50 ++-- .../tests/integration/consistency.rs | 6 +- .../merkle_tree/tests/integration/domain.rs | 53 +++-- .../tests/integration/merkle_tree.rs | 116 +++++---- .../merkle_tree/tests/integration/recovery.rs | 43 +--- .../zksync_core/src/api_server/tree/mod.rs | 2 +- .../src/metadata_calculator/helpers.rs | 91 +++---- .../src/metadata_calculator/metrics.rs | 2 +- 26 files changed, 591 insertions(+), 731 deletions(-) diff --git a/core/lib/merkle_tree/examples/loadtest/main.rs b/core/lib/merkle_tree/examples/loadtest/main.rs index b598a579f6b4..527daa87b37a 100644 --- a/core/lib/merkle_tree/examples/loadtest/main.rs +++ b/core/lib/merkle_tree/examples/loadtest/main.rs @@ -15,7 +15,8 @@ use std::{ use zksync_crypto::hasher::blake2::Blake2Hasher; use zksync_merkle_tree::{ - Database, HashTree, MerkleTree, MerkleTreePruner, PatchSet, RocksDBWrapper, TreeInstruction, + Database, HashTree, MerkleTree, MerkleTreePruner, PatchSet, RocksDBWrapper, TreeEntry, + TreeInstruction, }; use zksync_storage::{RocksDB, RocksDBOptions}; use zksync_types::{AccountTreeId, Address, StorageKey, H256, U256}; @@ -135,19 +136,22 @@ impl Cli { next_key_idx += new_keys.len() as u64; next_value_idx += (new_keys.len() + updated_indices.len()) as u64; - let values = (next_value_idx..).map(H256::from_low_u64_be); let updated_keys = Self::generate_keys(updated_indices.into_iter()); - let kvs = new_keys.into_iter().chain(updated_keys).zip(values); + let kvs = new_keys + .into_iter() + .chain(updated_keys) + .zip(next_value_idx..); + let kvs = kvs.map(|(key, idx)| { + // The assigned leaf indices here are not always correct, but it's OK for load test purposes. + TreeEntry::new(key, idx, H256::from_low_u64_be(idx)) + }); tracing::info!("Processing block #{version}"); let start = Instant::now(); let root_hash = if self.proofs { - let reads = Self::generate_keys(read_indices.into_iter()) - .map(|key| (key, TreeInstruction::Read)); - let instructions = kvs - .map(|(key, hash)| (key, TreeInstruction::Write(hash))) - .chain(reads) - .collect(); + let reads = + Self::generate_keys(read_indices.into_iter()).map(TreeInstruction::Read); + let instructions = kvs.map(TreeInstruction::Write).chain(reads).collect(); let output = tree.extend_with_proofs(instructions); output.root_hash().unwrap() } else { @@ -160,7 +164,7 @@ impl Cli { tracing::info!("Verifying tree consistency..."); let start = Instant::now(); - tree.verify_consistency(self.commit_count - 1) + tree.verify_consistency(self.commit_count - 1, false) .expect("tree consistency check failed"); let elapsed = start.elapsed(); tracing::info!("Verified tree consistency in {elapsed:?}"); diff --git a/core/lib/merkle_tree/examples/recovery.rs b/core/lib/merkle_tree/examples/recovery.rs index af16ed05baf3..1a2aae236ea5 100644 --- a/core/lib/merkle_tree/examples/recovery.rs +++ b/core/lib/merkle_tree/examples/recovery.rs @@ -9,8 +9,8 @@ use std::time::Instant; use zksync_crypto::hasher::blake2::Blake2Hasher; use zksync_merkle_tree::{ - recovery::{MerkleTreeRecovery, RecoveryEntry}, - HashTree, Key, PatchSet, PruneDatabase, RocksDBWrapper, ValueHash, + recovery::MerkleTreeRecovery, HashTree, Key, PatchSet, PruneDatabase, RocksDBWrapper, + TreeEntry, ValueHash, }; use zksync_storage::{RocksDB, RocksDBOptions}; @@ -94,7 +94,7 @@ impl Cli { .map(|_| { last_leaf_index += 1; if self.random { - RecoveryEntry { + TreeEntry { key: Key::from(rng.gen::<[u8; 32]>()), value: ValueHash::zero(), leaf_index: last_leaf_index, @@ -102,7 +102,7 @@ impl Cli { } else { last_key += key_step - Key::from(rng.gen::()); // ^ Increases the key by a random increment close to `key` step with some randomness. - RecoveryEntry { + TreeEntry { key: last_key, value: ValueHash::zero(), leaf_index: last_leaf_index, @@ -127,7 +127,7 @@ impl Cli { recovery_started_at.elapsed() ); let started_at = Instant::now(); - tree.verify_consistency(recovered_version).unwrap(); + tree.verify_consistency(recovered_version, true).unwrap(); tracing::info!("Verified consistency in {:?}", started_at.elapsed()); } } diff --git a/core/lib/merkle_tree/src/consistency.rs b/core/lib/merkle_tree/src/consistency.rs index 85896bad1ae1..2cc8996e64e9 100644 --- a/core/lib/merkle_tree/src/consistency.rs +++ b/core/lib/merkle_tree/src/consistency.rs @@ -69,10 +69,17 @@ pub enum ConsistencyError { impl MerkleTree { /// Verifies the internal tree consistency as stored in the database. /// + /// If `validate_indices` flag is set, it will be checked that indices for all tree leaves are unique + /// and are sequentially assigned starting from 1. + /// /// # Errors /// /// Returns an error (the first encountered one if there are multiple). - pub fn verify_consistency(&self, version: u64) -> Result<(), ConsistencyError> { + pub fn verify_consistency( + &self, + version: u64, + validate_indices: bool, + ) -> Result<(), ConsistencyError> { let manifest = self.db.try_manifest()?; let manifest = manifest.ok_or(ConsistencyError::MissingVersion(version))?; if version >= manifest.version_count { @@ -91,16 +98,19 @@ impl MerkleTree { // We want to perform a depth-first walk of the tree in order to not keep // much in memory. let root_key = Nibbles::EMPTY.with_version(version); - let leaf_data = LeafConsistencyData::new(leaf_count); - self.validate_node(&root_node, root_key, &leaf_data)?; - leaf_data.validate_count() + let leaf_data = validate_indices.then(|| LeafConsistencyData::new(leaf_count)); + self.validate_node(&root_node, root_key, leaf_data.as_ref())?; + if let Some(leaf_data) = leaf_data { + leaf_data.validate_count()?; + } + Ok(()) } fn validate_node( &self, node: &Node, key: NodeKey, - leaf_data: &LeafConsistencyData, + leaf_data: Option<&LeafConsistencyData>, ) -> Result { match node { Node::Leaf(leaf) => { @@ -111,7 +121,9 @@ impl MerkleTree { full_key: leaf.full_key, }); } - leaf_data.insert_leaf(leaf)?; + if let Some(leaf_data) = leaf_data { + leaf_data.insert_leaf(leaf)?; + } } Node::Internal(node) => { @@ -261,7 +273,10 @@ mod tests { use std::num::NonZeroU64; use super::*; - use crate::{types::InternalNode, PatchSet}; + use crate::{ + types::{InternalNode, TreeEntry}, + PatchSet, + }; use zksync_types::{H256, U256}; const FIRST_KEY: Key = U256([0, 0, 0, 0x_dead_beef_0000_0000]); @@ -270,8 +285,8 @@ mod tests { fn prepare_database() -> PatchSet { let mut tree = MerkleTree::new(PatchSet::default()); tree.extend(vec![ - (FIRST_KEY, H256([1; 32])), - (SECOND_KEY, H256([2; 32])), + TreeEntry::new(FIRST_KEY, 1, H256([1; 32])), + TreeEntry::new(SECOND_KEY, 2, H256([2; 32])), ]); tree.db } @@ -300,7 +315,7 @@ mod tests { .num_threads(1) .build() .expect("failed initializing `rayon` thread pool"); - thread_pool.install(|| MerkleTree::new(db).verify_consistency(0)) + thread_pool.install(|| MerkleTree::new(db).verify_consistency(0, true)) } #[test] diff --git a/core/lib/merkle_tree/src/domain.rs b/core/lib/merkle_tree/src/domain.rs index bb82233aec28..0cd9a56a4866 100644 --- a/core/lib/merkle_tree/src/domain.rs +++ b/core/lib/merkle_tree/src/domain.rs @@ -5,7 +5,10 @@ use zksync_utils::h256_to_u256; use crate::{ storage::{MerkleTreeColumnFamily, PatchSet, Patched, RocksDBWrapper}, - types::{Key, Root, TreeEntryWithProof, TreeInstruction, TreeLogEntry, ValueHash, TREE_DEPTH}, + types::{ + Key, Root, TreeEntry, TreeEntryWithProof, TreeInstruction, TreeLogEntry, ValueHash, + TREE_DEPTH, + }, BlockOutput, HashTree, MerkleTree, NoVersionError, }; use zksync_crypto::hasher::blake2::Blake2Hasher; @@ -13,7 +16,7 @@ use zksync_storage::RocksDB; use zksync_types::{ proofs::{PrepareBasicCircuitsJob, StorageLogMetadata}, writes::{InitialStorageWrite, RepeatedStorageWrite, StateDiffRecord}, - L1BatchNumber, StorageKey, StorageLog, StorageLogKind, U256, + L1BatchNumber, StorageKey, U256, }; /// Metadata for the current tree state. @@ -65,17 +68,17 @@ impl ZkSyncTree { /// Returns metadata based on `storage_logs` generated by the genesis L1 batch. This does not /// create a persistent tree. - pub fn process_genesis_batch(storage_logs: &[StorageLog]) -> BlockOutput { - let kvs = Self::filter_write_logs(storage_logs); + pub fn process_genesis_batch(storage_logs: &[TreeInstruction]) -> BlockOutput { + let kvs = Self::filter_write_instructions(storage_logs); tracing::info!( "Creating Merkle tree for genesis batch with {instr_count} writes", instr_count = kvs.len() ); - let kvs = kvs + let kvs: Vec<_> = kvs .iter() - .map(|(k, v)| (k.hashed_key_u256(), *v)) - .collect::>(); + .map(|instr| instr.map_key(StorageKey::hashed_key_u256)) + .collect(); let mut in_memory_tree = MerkleTree::new(PatchSet::default()); let output = in_memory_tree.extend(kvs); @@ -170,29 +173,36 @@ impl ZkSyncTree { /// Panics if an inconsistency is detected. pub fn verify_consistency(&self, l1_batch_number: L1BatchNumber) { let version = u64::from(l1_batch_number.0); - self.tree.verify_consistency(version).unwrap_or_else(|err| { - panic!("Tree at version {version} is inconsistent: {err}"); - }); + self.tree + .verify_consistency(version, true) + .unwrap_or_else(|err| { + panic!("Tree at version {version} is inconsistent: {err}"); + }); } /// Processes an iterator of storage logs comprising a single L1 batch. - pub fn process_l1_batch(&mut self, storage_logs: &[StorageLog]) -> TreeMetadata { + pub fn process_l1_batch( + &mut self, + storage_logs: &[TreeInstruction], + ) -> TreeMetadata { match self.mode { TreeMode::Full => self.process_l1_batch_full(storage_logs), TreeMode::Lightweight => self.process_l1_batch_lightweight(storage_logs), } } - fn process_l1_batch_full(&mut self, storage_logs: &[StorageLog]) -> TreeMetadata { + fn process_l1_batch_full( + &mut self, + instructions: &[TreeInstruction], + ) -> TreeMetadata { let l1_batch_number = self.next_l1_batch_number(); - let instructions = Self::transform_logs(storage_logs); let starting_leaf_count = self.tree.latest_root().leaf_count(); let starting_root_hash = self.tree.latest_root_hash(); - let instructions_with_hashed_keys = instructions + let instructions_with_hashed_keys: Vec<_> = instructions .iter() - .map(|(k, instr)| (k.hashed_key_u256(), *instr)) - .collect::>(); + .map(|instr| instr.map_key(StorageKey::hashed_key_u256)) + .collect(); tracing::info!( "Extending Merkle tree with batch #{l1_batch_number} with {instr_count} ops in full mode", @@ -207,7 +217,7 @@ impl ZkSyncTree { let mut witness = PrepareBasicCircuitsJob::new(starting_leaf_count + 1); witness.reserve(output.logs.len()); - for (log, (key, instruction)) in output.logs.iter().zip(&instructions) { + for (log, instruction) in output.logs.iter().zip(instructions) { let empty_levels_end = TREE_DEPTH - log.merkle_path.len(); let empty_subtree_hashes = (0..empty_levels_end).map(|i| Blake2Hasher.empty_subtree_hash(i)); @@ -218,20 +228,22 @@ impl ZkSyncTree { .collect(); let value_written = match instruction { - TreeInstruction::Write(value) => value.0, - TreeInstruction::Read => [0_u8; 32], + TreeInstruction::Write(entry) => entry.value.0, + TreeInstruction::Read(_) => [0_u8; 32], }; let log = StorageLogMetadata { root_hash: log.root_hash.0, is_write: !log.base.is_read(), - first_write: matches!(log.base, TreeLogEntry::Inserted { .. }), + first_write: matches!(log.base, TreeLogEntry::Inserted), merkle_paths, - leaf_hashed_key: key.hashed_key_u256(), - leaf_enumeration_index: match log.base { - TreeLogEntry::Updated { leaf_index, .. } - | TreeLogEntry::Inserted { leaf_index } - | TreeLogEntry::Read { leaf_index, .. } => leaf_index, - TreeLogEntry::ReadMissingKey => 0, + leaf_hashed_key: instruction.key().hashed_key_u256(), + leaf_enumeration_index: match instruction { + TreeInstruction::Write(entry) => entry.leaf_index, + TreeInstruction::Read(_) => match log.base { + TreeLogEntry::Read { leaf_index, .. } => leaf_index, + TreeLogEntry::ReadMissingKey => 0, + _ => unreachable!("Read instructions always transform to Read / ReadMissingKey log entries"), + } }, value_written, value_read: match log.base { @@ -243,7 +255,7 @@ impl ZkSyncTree { previous_value.0 } TreeLogEntry::Read { value, .. } => value.0, - TreeLogEntry::Inserted { .. } | TreeLogEntry::ReadMissingKey => [0_u8; 32], + TreeLogEntry::Inserted | TreeLogEntry::ReadMissingKey => [0_u8; 32], }, }; witness.push_merkle_path(log); @@ -254,12 +266,12 @@ impl ZkSyncTree { .logs .into_iter() .filter_map(|log| (!log.base.is_read()).then_some(log.base)); - let kvs = instructions.into_iter().filter_map(|(key, instruction)| { - let TreeInstruction::Write(value) = instruction else { - return None; - }; - Some((key, value)) - }); + let kvs = instructions + .iter() + .filter_map(|instruction| match instruction { + TreeInstruction::Write(entry) => Some(*entry), + TreeInstruction::Read(_) => None, + }); let (initial_writes, repeated_writes, state_diffs) = Self::extract_writes(logs, kvs); tracing::info!( @@ -281,21 +293,9 @@ impl ZkSyncTree { } } - fn transform_logs(storage_logs: &[StorageLog]) -> Vec<(StorageKey, TreeInstruction)> { - let instructions = storage_logs.iter().map(|log| { - let key = log.key; - let instruction = match log.kind { - StorageLogKind::Write => TreeInstruction::Write(log.value), - StorageLogKind::Read => TreeInstruction::Read, - }; - (key, instruction) - }); - instructions.collect() - } - fn extract_writes( logs: impl Iterator, - kvs: impl Iterator, + entries: impl Iterator>, ) -> ( Vec, Vec, @@ -304,13 +304,14 @@ impl ZkSyncTree { let mut initial_writes = vec![]; let mut repeated_writes = vec![]; let mut state_diffs = vec![]; - for (log_entry, (key, value)) in logs.zip(kvs) { + for (log_entry, input_entry) in logs.zip(entries) { + let key = &input_entry.key; match log_entry { - TreeLogEntry::Inserted { leaf_index } => { + TreeLogEntry::Inserted => { initial_writes.push(InitialStorageWrite { - index: leaf_index, + index: input_entry.leaf_index, key: key.hashed_key_u256(), - value, + value: input_entry.value, }); state_diffs.push(StateDiffRecord { address: *key.address(), @@ -318,25 +319,25 @@ impl ZkSyncTree { derived_key: StorageKey::raw_hashed_key(key.address(), key.key()), enumeration_index: 0u64, initial_value: U256::default(), - final_value: h256_to_u256(value), + final_value: h256_to_u256(input_entry.value), }); } TreeLogEntry::Updated { + previous_value: prev_value_hash, leaf_index, - previous_value, } => { - if previous_value != value { + if prev_value_hash != input_entry.value { repeated_writes.push(RepeatedStorageWrite { - index: leaf_index, - value, + index: input_entry.leaf_index, + value: input_entry.value, }); state_diffs.push(StateDiffRecord { address: *key.address(), key: h256_to_u256(*key.key()), derived_key: StorageKey::raw_hashed_key(key.address(), key.key()), enumeration_index: leaf_index, - initial_value: h256_to_u256(previous_value), - final_value: h256_to_u256(value), + initial_value: h256_to_u256(prev_value_hash), + final_value: h256_to_u256(input_entry.value), }); } // Else we have a no-op update that must be omitted from `repeated_writes`. @@ -348,8 +349,11 @@ impl ZkSyncTree { (initial_writes, repeated_writes, state_diffs) } - fn process_l1_batch_lightweight(&mut self, storage_logs: &[StorageLog]) -> TreeMetadata { - let kvs = Self::filter_write_logs(storage_logs); + fn process_l1_batch_lightweight( + &mut self, + instructions: &[TreeInstruction], + ) -> TreeMetadata { + let kvs = Self::filter_write_instructions(instructions); let l1_batch_number = self.next_l1_batch_number(); tracing::info!( "Extending Merkle tree with batch #{l1_batch_number} with {kv_count} writes \ @@ -357,10 +361,10 @@ impl ZkSyncTree { kv_count = kvs.len() ); - let kvs_with_derived_key = kvs + let kvs_with_derived_key: Vec<_> = kvs .iter() - .map(|(k, v)| (k.hashed_key_u256(), *v)) - .collect::>(); + .map(|entry| entry.map_key(StorageKey::hashed_key_u256)) + .collect(); let output = if let Some(thread_pool) = &self.thread_pool { thread_pool.install(|| self.tree.extend(kvs_with_derived_key.clone())) @@ -390,14 +394,15 @@ impl ZkSyncTree { } } - fn filter_write_logs(storage_logs: &[StorageLog]) -> Vec<(StorageKey, ValueHash)> { - let kvs = storage_logs.iter().filter_map(|log| match log.kind { - StorageLogKind::Write => { - let key = log.key; - Some((key, log.value)) - } - StorageLogKind::Read => None, - }); + fn filter_write_instructions( + instructions: &[TreeInstruction], + ) -> Vec> { + let kvs = instructions + .iter() + .filter_map(|instruction| match instruction { + TreeInstruction::Write(entry) => Some(*entry), + TreeInstruction::Read(_) => None, + }); kvs.collect() } diff --git a/core/lib/merkle_tree/src/getters.rs b/core/lib/merkle_tree/src/getters.rs index 67ce2aa98773..7fd6bfc96ed0 100644 --- a/core/lib/merkle_tree/src/getters.rs +++ b/core/lib/merkle_tree/src/getters.rs @@ -26,7 +26,7 @@ impl MerkleTree { let node = patch_set.get(longest_prefix); match node { Some(Node::Leaf(leaf)) if &leaf.full_key == leaf_key => (*leaf).into(), - _ => TreeEntry::empty(), + _ => TreeEntry::empty(*leaf_key), } }, ) @@ -76,11 +76,12 @@ impl MerkleTree { |patch_set, &leaf_key, longest_prefix| { let (leaf, merkle_path) = patch_set.create_proof(&mut hasher, leaf_key, longest_prefix, 0); - let value_hash = leaf + let value = leaf .as_ref() .map_or_else(ValueHash::zero, |leaf| leaf.value_hash); TreeEntry { - value_hash, + key: leaf_key, + value, leaf_index: leaf.map_or(0, |leaf| leaf.leaf_index), } .with_merkle_path(merkle_path.into_inner()) @@ -107,26 +108,26 @@ mod tests { let entries = tree.entries_with_proofs(0, &[missing_key]).unwrap(); assert_eq!(entries.len(), 1); assert!(entries[0].base.is_empty()); - entries[0].verify(&tree.hasher, missing_key, tree.hasher.empty_tree_hash()); + entries[0].verify(&tree.hasher, tree.hasher.empty_tree_hash()); } #[test] fn entries_in_single_node_tree() { let mut tree = MerkleTree::new(PatchSet::default()); let key = Key::from(987_654); - let output = tree.extend(vec![(key, ValueHash::repeat_byte(1))]); + let output = tree.extend(vec![TreeEntry::new(key, 1, ValueHash::repeat_byte(1))]); let missing_key = Key::from(123); let entries = tree.entries(0, &[key, missing_key]).unwrap(); assert_eq!(entries.len(), 2); - assert_eq!(entries[0].value_hash, ValueHash::repeat_byte(1)); + assert_eq!(entries[0].value, ValueHash::repeat_byte(1)); assert_eq!(entries[0].leaf_index, 1); let entries = tree.entries_with_proofs(0, &[key, missing_key]).unwrap(); assert_eq!(entries.len(), 2); assert!(!entries[0].base.is_empty()); - entries[0].verify(&tree.hasher, key, output.root_hash); + entries[0].verify(&tree.hasher, output.root_hash); assert!(entries[1].base.is_empty()); - entries[1].verify(&tree.hasher, missing_key, output.root_hash); + entries[1].verify(&tree.hasher, output.root_hash); } } diff --git a/core/lib/merkle_tree/src/hasher/mod.rs b/core/lib/merkle_tree/src/hasher/mod.rs index 8b2478c43d34..9425a5836f02 100644 --- a/core/lib/merkle_tree/src/hasher/mod.rs +++ b/core/lib/merkle_tree/src/hasher/mod.rs @@ -11,7 +11,7 @@ pub(crate) use self::nodes::{InternalNodeCache, MerklePath}; pub use self::proofs::TreeRangeDigest; use crate::{ metrics::HashingStats, - types::{Key, ValueHash, TREE_DEPTH}, + types::{TreeEntry, ValueHash, TREE_DEPTH}, }; use zksync_crypto::hasher::{blake2::Blake2Hasher, Hasher}; @@ -65,17 +65,11 @@ impl dyn HashTree + '_ { empty_hashes.chain(path.iter().copied()) } - fn fold_merkle_path( - &self, - path: &[ValueHash], - key: Key, - value_hash: ValueHash, - leaf_index: u64, - ) -> ValueHash { - let mut hash = self.hash_leaf(&value_hash, leaf_index); + fn fold_merkle_path(&self, path: &[ValueHash], entry: TreeEntry) -> ValueHash { + let mut hash = self.hash_leaf(&entry.value, entry.leaf_index); let full_path = self.extend_merkle_path(path); for (depth, adjacent_hash) in full_path.enumerate() { - hash = if key.bit(depth) { + hash = if entry.key.bit(depth) { self.hash_branch(&adjacent_hash, &hash) } else { self.hash_branch(&hash, &adjacent_hash) @@ -254,7 +248,7 @@ mod tests { let address: Address = "4b3af74f66ab1f0da3f2e4ec7a3cb99baf1af7b2".parse().unwrap(); let key = StorageKey::new(AccountTreeId::new(address), H256::zero()); let key = key.hashed_key_u256(); - let leaf = LeafNode::new(key, H256([1; 32]), 1); + let leaf = LeafNode::new(TreeEntry::new(key, 1, H256([1; 32]))); let stats = HashingStats::default(); let mut hasher = (&Blake2Hasher as &dyn HashTree).with_stats(&stats); @@ -265,7 +259,7 @@ mod tests { assert!(stats.hashed_bytes.into_inner() > 100); let hasher: &dyn HashTree = &Blake2Hasher; - let folded_hash = hasher.fold_merkle_path(&[], key, H256([1; 32]), 1); + let folded_hash = hasher.fold_merkle_path(&[], leaf.into()); assert_eq!(folded_hash, EXPECTED_HASH); } @@ -274,7 +268,7 @@ mod tests { let address: Address = "4b3af74f66ab1f0da3f2e4ec7a3cb99baf1af7b2".parse().unwrap(); let key = StorageKey::new(AccountTreeId::new(address), H256::zero()); let key = key.hashed_key_u256(); - let leaf = LeafNode::new(key, H256([1; 32]), 1); + let leaf = LeafNode::new(TreeEntry::new(key, 1, H256([1; 32]))); let mut hasher = HasherWithStats::new(&Blake2Hasher); let leaf_hash = leaf.hash(&mut hasher, 2); @@ -283,9 +277,7 @@ mod tests { let expected_hash = hasher.hash_branch(&merkle_path[0], &leaf_hash); let expected_hash = hasher.hash_branch(&expected_hash, &merkle_path[1]); - let folded_hash = hasher - .inner - .fold_merkle_path(&merkle_path, key, H256([1; 32]), 1); + let folded_hash = hasher.inner.fold_merkle_path(&merkle_path, leaf.into()); assert_eq!(folded_hash, expected_hash); } } diff --git a/core/lib/merkle_tree/src/hasher/proofs.rs b/core/lib/merkle_tree/src/hasher/proofs.rs index d97df0ad97d0..49d4bfe92958 100644 --- a/core/lib/merkle_tree/src/hasher/proofs.rs +++ b/core/lib/merkle_tree/src/hasher/proofs.rs @@ -22,36 +22,37 @@ impl BlockOutputWithProofs { &self, hasher: &dyn HashTree, old_root_hash: ValueHash, - instructions: &[(Key, TreeInstruction)], + instructions: &[TreeInstruction], ) { assert_eq!(instructions.len(), self.logs.len()); let mut root_hash = old_root_hash; - for (op, &(key, instruction)) in self.logs.iter().zip(instructions) { + for (op, &instruction) in self.logs.iter().zip(instructions) { assert!(op.merkle_path.len() <= TREE_DEPTH); - if matches!(instruction, TreeInstruction::Read) { + if matches!(instruction, TreeInstruction::Read(_)) { assert_eq!(op.root_hash, root_hash); assert!(op.base.is_read()); } else { assert!(!op.base.is_read()); } - let (prev_leaf_index, leaf_index, prev_value) = match op.base { - TreeLogEntry::Inserted { leaf_index } => (0, leaf_index, ValueHash::zero()), + let prev_entry = match op.base { + TreeLogEntry::Inserted | TreeLogEntry::ReadMissingKey => { + TreeEntry::empty(instruction.key()) + } TreeLogEntry::Updated { leaf_index, - previous_value, - } => (leaf_index, leaf_index, previous_value), - - TreeLogEntry::Read { leaf_index, value } => (leaf_index, leaf_index, value), - TreeLogEntry::ReadMissingKey => (0, 0, ValueHash::zero()), + previous_value: value, + } + | TreeLogEntry::Read { leaf_index, value } => { + TreeEntry::new(instruction.key(), leaf_index, value) + } }; - let prev_hash = - hasher.fold_merkle_path(&op.merkle_path, key, prev_value, prev_leaf_index); + let prev_hash = hasher.fold_merkle_path(&op.merkle_path, prev_entry); assert_eq!(prev_hash, root_hash); - if let TreeInstruction::Write(value) = instruction { - let next_hash = hasher.fold_merkle_path(&op.merkle_path, key, value, leaf_index); + if let TreeInstruction::Write(new_entry) = instruction { + let next_hash = hasher.fold_merkle_path(&op.merkle_path, new_entry); assert_eq!(next_hash, op.root_hash); } root_hash = op.root_hash; @@ -65,19 +66,14 @@ impl TreeEntryWithProof { /// # Panics /// /// Panics if the proof doesn't verify. - pub fn verify(&self, hasher: &dyn HashTree, key: Key, trusted_root_hash: ValueHash) { + pub fn verify(&self, hasher: &dyn HashTree, trusted_root_hash: ValueHash) { if self.base.leaf_index == 0 { assert!( - self.base.value_hash.is_zero(), + self.base.value.is_zero(), "Invalid missing value specification: leaf index is zero, but value is non-default" ); } - let root_hash = hasher.fold_merkle_path( - &self.merkle_path, - key, - self.base.value_hash, - self.base.leaf_index, - ); + let root_hash = hasher.fold_merkle_path(&self.merkle_path, self.base); assert_eq!(root_hash, trusted_root_hash, "Root hash mismatch"); } } @@ -146,11 +142,7 @@ impl<'a> TreeRangeDigest<'a> { let left_contour: Vec<_> = left_contour.collect(); Self { hasher: HasherWithStats::new(hasher), - current_leaf: LeafNode::new( - start_key, - start_entry.base.value_hash, - start_entry.base.leaf_index, - ), + current_leaf: LeafNode::new(start_entry.base), left_contour: left_contour.try_into().unwrap(), // ^ `unwrap()` is safe by construction; `left_contour` will always have necessary length } @@ -161,13 +153,13 @@ impl<'a> TreeRangeDigest<'a> { /// # Panics /// /// Panics if the provided `key` is not greater than the previous key provided to this digest. - pub fn update(&mut self, key: Key, entry: TreeEntry) { + pub fn update(&mut self, entry: TreeEntry) { assert!( - key > self.current_leaf.full_key, + entry.key > self.current_leaf.full_key, "Keys provided to a digest must be monotonically increasing" ); - let diverging_level = utils::find_diverging_bit(self.current_leaf.full_key, key) + 1; + let diverging_level = utils::find_diverging_bit(self.current_leaf.full_key, entry.key) + 1; // Hash the current leaf up to the `diverging_level`, taking current `left_contour` into account. let mut hash = self @@ -188,7 +180,7 @@ impl<'a> TreeRangeDigest<'a> { } // Record the computed hash. self.left_contour[TREE_DEPTH - diverging_level] = hash; - self.current_leaf = LeafNode::new(key, entry.value_hash, entry.leaf_index); + self.current_leaf = LeafNode::new(entry); } /// Finalizes this digest and returns the root hash of the tree. @@ -196,8 +188,8 @@ impl<'a> TreeRangeDigest<'a> { /// # Panics /// /// Panics if the provided `final_key` is not greater than the previous key provided to this digest. - pub fn finalize(mut self, final_key: Key, final_entry: &TreeEntryWithProof) -> ValueHash { - self.update(final_key, final_entry.base); + pub fn finalize(mut self, final_entry: &TreeEntryWithProof) -> ValueHash { + self.update(final_entry.base); let full_path = self .hasher @@ -206,9 +198,9 @@ impl<'a> TreeRangeDigest<'a> { let zipped_paths = self.left_contour.into_iter().zip(full_path); let mut hash = self .hasher - .hash_leaf(&final_entry.base.value_hash, final_entry.base.leaf_index); + .hash_leaf(&final_entry.base.value, final_entry.base.leaf_index); for (depth, (left, right)) in zipped_paths.enumerate() { - hash = if final_key.bit(depth) { + hash = if final_entry.base.key.bit(depth) { self.hasher.hash_branch(&left, &hash) } else { self.hasher.hash_branch(&hash, &right) diff --git a/core/lib/merkle_tree/src/lib.rs b/core/lib/merkle_tree/src/lib.rs index 166400cbb640..85ace50aada5 100644 --- a/core/lib/merkle_tree/src/lib.rs +++ b/core/lib/merkle_tree/src/lib.rs @@ -26,10 +26,15 @@ //! - Hash of a vacant leaf is `hash([0_u8; 40])`, where `hash` is the hash function used //! (Blake2s-256). //! - Hash of an occupied leaf is `hash(u64::to_be_bytes(leaf_index) ++ value_hash)`, -//! where `leaf_index` is the 1-based index of the leaf key in the order of insertion, +//! where `leaf_index` is a 1-based index of the leaf key provided when the leaf is inserted / updated, //! `++` is byte concatenation. //! - Hash of an internal node is `hash(left_child_hash ++ right_child_hash)`. //! +//! Currently in zksync, leaf indices enumerate leaves in the order of their insertion into the tree. +//! Indices are computed externally and are provided to the tree as inputs; the tree doesn't verify +//! index assignment and doesn't rely on particular index assignment assumptions (other than when +//! [verifying tree consistency](MerkleTree::verify_consistency())). +//! //! [Jellyfish Merkle tree]: https://developers.diem.com/papers/jellyfish-merkle-tree/2021-01-14.pdf // Linter settings. @@ -209,10 +214,10 @@ impl MerkleTree { /// # Return value /// /// Returns information about the update such as the final tree hash. - pub fn extend(&mut self, key_value_pairs: Vec<(Key, ValueHash)>) -> BlockOutput { + pub fn extend(&mut self, entries: Vec) -> BlockOutput { let next_version = self.db.manifest().unwrap_or_default().version_count; let storage = Storage::new(&self.db, &self.hasher, next_version, true); - let (output, patch) = storage.extend(key_value_pairs); + let (output, patch) = storage.extend(entries); self.db.apply_patch(patch); output } @@ -226,7 +231,7 @@ impl MerkleTree { /// instruction. pub fn extend_with_proofs( &mut self, - instructions: Vec<(Key, TreeInstruction)>, + instructions: Vec, ) -> BlockOutputWithProofs { let next_version = self.db.manifest().unwrap_or_default().version_count; let storage = Storage::new(&self.db, &self.hasher, next_version, true); diff --git a/core/lib/merkle_tree/src/pruning.rs b/core/lib/merkle_tree/src/pruning.rs index 21a3e8712fd7..5b1911ca6005 100644 --- a/core/lib/merkle_tree/src/pruning.rs +++ b/core/lib/merkle_tree/src/pruning.rs @@ -187,7 +187,7 @@ mod tests { use super::*; use crate::{ types::{Node, NodeKey}, - Database, Key, MerkleTree, PatchSet, ValueHash, + Database, Key, MerkleTree, PatchSet, TreeEntry, ValueHash, }; fn create_db() -> PatchSet { @@ -195,7 +195,7 @@ mod tests { for i in 0..5 { let key = Key::from(i); let value = ValueHash::from_low_u64_be(i); - MerkleTree::new(&mut db).extend(vec![(key, value)]); + MerkleTree::new(&mut db).extend(vec![TreeEntry::new(key, i + 1, value)]); } db } @@ -245,9 +245,9 @@ mod tests { assert!(start.elapsed() < Duration::from_secs(10)); } - fn generate_key_value_pairs(indexes: impl Iterator) -> Vec<(Key, ValueHash)> { + fn generate_key_value_pairs(indexes: impl Iterator) -> Vec { indexes - .map(|i| (Key::from(i), ValueHash::from_low_u64_be(i))) + .map(|i| TreeEntry::new(Key::from(i), i + 1, ValueHash::from_low_u64_be(i))) .collect() } @@ -273,7 +273,7 @@ mod tests { let mut tree = MerkleTree::new(&mut db); for version in first_retained_version..=latest_version { - tree.verify_consistency(version).unwrap(); + tree.verify_consistency(version, true).unwrap(); } let kvs = generate_key_value_pairs(100..200); @@ -290,7 +290,7 @@ mod tests { let tree = MerkleTree::new(&mut db); for version in first_retained_version..=latest_version { - tree.verify_consistency(version).unwrap(); + tree.verify_consistency(version, true).unwrap(); } assert_no_stale_keys(&db, first_retained_version); } @@ -318,8 +318,8 @@ mod tests { const ITERATIVE_BATCH_COUNT: usize = 10; let mut db = PatchSet::default(); - let kvs: Vec<_> = (0_u32..100) - .map(|i| (Key::from(i), ValueHash::zero())) + let kvs: Vec<_> = (0_u64..100) + .map(|i| TreeEntry::new(Key::from(i), i + 1, ValueHash::zero())) .collect(); let batch_count = if initialize_iteratively { @@ -335,8 +335,8 @@ mod tests { // Completely overwrite all keys. let new_value_hash = ValueHash::from_low_u64_be(1_000); - let new_kvs = (0_u32..100) - .map(|i| (Key::from(i), new_value_hash)) + let new_kvs = (0_u64..100) + .map(|i| TreeEntry::new(Key::from(i), i + 1, new_value_hash)) .collect(); MerkleTree::new(&mut db).extend(new_kvs); @@ -364,16 +364,16 @@ mod tests { prune_iteratively: bool, ) { let mut db = PatchSet::default(); - let kvs: Vec<_> = (0_u32..100) - .map(|i| (Key::from(i), ValueHash::zero())) + let kvs: Vec<_> = (0_u64..100) + .map(|i| TreeEntry::new(Key::from(i), i + 1, ValueHash::zero())) .collect(); MerkleTree::new(&mut db).extend(kvs); let leaf_keys_in_db = leaf_keys(&mut db); // Completely overwrite all keys in several batches. let new_value_hash = ValueHash::from_low_u64_be(1_000); - let new_kvs: Vec<_> = (0_u32..100) - .map(|i| (Key::from(i), new_value_hash)) + let new_kvs: Vec<_> = (0_u64..100) + .map(|i| TreeEntry::new(Key::from(i), i + 1, new_value_hash)) .collect(); for chunk in new_kvs.chunks(20) { MerkleTree::new(&mut db).extend(chunk.to_vec()); diff --git a/core/lib/merkle_tree/src/recovery.rs b/core/lib/merkle_tree/src/recovery.rs index 85ac578cc0a1..d1f2618a5cdd 100644 --- a/core/lib/merkle_tree/src/recovery.rs +++ b/core/lib/merkle_tree/src/recovery.rs @@ -40,23 +40,11 @@ use std::time::Instant; use crate::{ hasher::{HashTree, HasherWithStats}, storage::{PatchSet, PruneDatabase, PrunePatchSet, Storage}, - types::{Key, Manifest, Root, TreeTags, ValueHash}, + types::{Key, Manifest, Root, TreeEntry, TreeTags, ValueHash}, MerkleTree, }; use zksync_crypto::hasher::blake2::Blake2Hasher; -/// Entry in a Merkle tree used during recovery. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct RecoveryEntry { - /// Entry key. - pub key: Key, - /// Entry value. - pub value: ValueHash, - /// Leaf index associated with the entry. It is **not** checked whether leaf indices are well-formed - /// during recovery (e.g., that they are unique). - pub leaf_index: u64, -} - /// Handle to a Merkle tree during its recovery. #[derive(Debug)] pub struct MerkleTreeRecovery { @@ -154,7 +142,7 @@ impl MerkleTreeRecovery { %entries.key_range = entries_key_range(&entries), ), )] - pub fn extend_linear(&mut self, entries: Vec) { + pub fn extend_linear(&mut self, entries: Vec) { tracing::debug!("Started extending tree"); let started_at = Instant::now(); @@ -177,7 +165,7 @@ impl MerkleTreeRecovery { entries.len = entries.len(), ), )] - pub fn extend_random(&mut self, entries: Vec) { + pub fn extend_random(&mut self, entries: Vec) { tracing::debug!("Started extending tree"); let started_at = Instant::now(); @@ -242,7 +230,7 @@ impl MerkleTreeRecovery { } } -fn entries_key_range(entries: &[RecoveryEntry]) -> String { +fn entries_key_range(entries: &[TreeEntry]) -> String { let (Some(first), Some(last)) = (entries.first(), entries.last()) else { return "(empty)".to_owned(); }; @@ -280,11 +268,7 @@ mod tests { #[test] fn recovering_tree_with_single_node() { let mut recovery = MerkleTreeRecovery::new(PatchSet::default(), 42); - let recovery_entry = RecoveryEntry { - key: Key::from(123), - value: ValueHash::repeat_byte(1), - leaf_index: 1, - }; + let recovery_entry = TreeEntry::new(Key::from(123), 1, ValueHash::repeat_byte(1)); recovery.extend_linear(vec![recovery_entry]); let tree = recovery.finalize(); @@ -292,13 +276,8 @@ mod tests { let mut hasher = HasherWithStats::new(&Blake2Hasher); assert_eq!( tree.latest_root_hash(), - LeafNode::new( - recovery_entry.key, - recovery_entry.value, - recovery_entry.leaf_index - ) - .hash(&mut hasher, 0) + LeafNode::new(recovery_entry).hash(&mut hasher, 0) ); - tree.verify_consistency(42).unwrap(); + tree.verify_consistency(42, true).unwrap(); } } diff --git a/core/lib/merkle_tree/src/storage/mod.rs b/core/lib/merkle_tree/src/storage/mod.rs index c5a56abfca90..ae273d22f323 100644 --- a/core/lib/merkle_tree/src/storage/mod.rs +++ b/core/lib/merkle_tree/src/storage/mod.rs @@ -18,12 +18,10 @@ pub use self::{ use crate::{ hasher::HashTree, metrics::{TreeUpdaterStats, BLOCK_TIMINGS, GENERAL_METRICS}, - recovery::RecoveryEntry, types::{ BlockOutput, ChildRef, InternalNode, Key, LeafNode, Manifest, Nibbles, Node, Root, - TreeLogEntry, TreeTags, ValueHash, + TreeEntry, TreeLogEntry, TreeTags, ValueHash, }, - utils::increment_counter, }; /// Tree operation: either inserting a new version or updating an existing one (the latter is only @@ -132,17 +130,17 @@ impl TreeUpdater { /// hashes for all updated nodes in [`Self::finalize()`]. fn insert( &mut self, - key: Key, - value_hash: ValueHash, + entry: TreeEntry, parent_nibbles: &Nibbles, - leaf_index_fn: impl FnOnce() -> u64, ) -> (TreeLogEntry, NewLeafData) { let version = self.patch_set.root_version(); + let key = entry.key; + let traverse_outcome = self.patch_set.traverse(key, parent_nibbles); let (log, leaf_data) = match traverse_outcome { TraverseOutcome::LeafMatch(nibbles, mut leaf) => { - let log = TreeLogEntry::update(leaf.value_hash, leaf.leaf_index); - leaf.value_hash = value_hash; + let log = TreeLogEntry::update(leaf.leaf_index, leaf.value_hash); + leaf.update_from(entry); self.patch_set.insert(nibbles, leaf.into()); self.metrics.updated_leaves += 1; (log, NewLeafData::new(nibbles, leaf)) @@ -173,23 +171,20 @@ impl TreeUpdater { nibble_idx += 1; } - let leaf_index = leaf_index_fn(); - let new_leaf = LeafNode::new(key, value_hash, leaf_index); + let new_leaf = LeafNode::new(entry); let new_leaf_nibbles = Nibbles::new(&key, nibble_idx + 1); let leaf_data = NewLeafData::new(new_leaf_nibbles, new_leaf); let moved_leaf_nibbles = Nibbles::new(&leaf.full_key, nibble_idx + 1); let leaf_data = leaf_data.with_adjacent_leaf(moved_leaf_nibbles, leaf); - (TreeLogEntry::insert(leaf_index), leaf_data) + (TreeLogEntry::Inserted, leaf_data) } TraverseOutcome::MissingChild(nibbles) if nibbles.nibble_count() == 0 => { // The root is currently empty; we replace it with a leaf. - let leaf_index = leaf_index_fn(); - debug_assert_eq!(leaf_index, 1); - let root_leaf = LeafNode::new(key, value_hash, leaf_index); + let root_leaf = LeafNode::new(entry); self.set_root_node(root_leaf.into()); let leaf_data = NewLeafData::new(Nibbles::EMPTY, root_leaf); - (TreeLogEntry::insert(1), leaf_data) + (TreeLogEntry::Inserted, leaf_data) } TraverseOutcome::MissingChild(nibbles) => { @@ -198,10 +193,9 @@ impl TreeUpdater { unreachable!("Node parent must be an internal node"); }; parent.insert_child_ref(last_nibble, ChildRef::leaf(version)); - let leaf_index = leaf_index_fn(); - let new_leaf = LeafNode::new(key, value_hash, leaf_index); + let new_leaf = LeafNode::new(entry); let leaf_data = NewLeafData::new(nibbles, new_leaf); - (TreeLogEntry::insert(leaf_index), leaf_data) + (TreeLogEntry::Inserted, leaf_data) } }; @@ -289,19 +283,20 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { /// Extends the Merkle tree in the lightweight operation mode, without intermediate hash /// computations. - pub fn extend(mut self, key_value_pairs: Vec<(Key, ValueHash)>) -> (BlockOutput, PatchSet) { + pub fn extend(mut self, entries: Vec) -> (BlockOutput, PatchSet) { let load_nodes_latency = BLOCK_TIMINGS.load_nodes.start(); - let sorted_keys = SortedKeys::new(key_value_pairs.iter().map(|(key, _)| *key)); + let sorted_keys = SortedKeys::new(entries.iter().map(|entry| entry.key)); let parent_nibbles = self.updater.load_ancestors(&sorted_keys, self.db); let load_nodes_latency = load_nodes_latency.observe(); tracing::debug!("Load stage took {load_nodes_latency:?}"); let extend_patch_latency = BLOCK_TIMINGS.extend_patch.start(); - let mut logs = Vec::with_capacity(key_value_pairs.len()); - for ((key, value_hash), parent_nibbles) in key_value_pairs.into_iter().zip(parent_nibbles) { - let (log, _) = self.updater.insert(key, value_hash, &parent_nibbles, || { - increment_counter(&mut self.leaf_count) - }); + let mut logs = Vec::with_capacity(entries.len()); + for (entry, parent_nibbles) in entries.into_iter().zip(parent_nibbles) { + let (log, _) = self.updater.insert(entry, &parent_nibbles); + if matches!(log, TreeLogEntry::Inserted) { + self.leaf_count += 1; + } logs.push(log); } let extend_patch_latency = extend_patch_latency.observe(); @@ -321,10 +316,7 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { Some(self.updater.load_greatest_key(self.db)?.0.full_key) } - pub fn extend_during_linear_recovery( - mut self, - recovery_entries: Vec, - ) -> PatchSet { + pub fn extend_during_linear_recovery(mut self, recovery_entries: Vec) -> PatchSet { let (mut prev_key, mut prev_nibbles) = match self.updater.load_greatest_key(self.db) { Some((leaf, nibbles)) => (Some(leaf.full_key), nibbles), None => (None, Nibbles::EMPTY), @@ -343,9 +335,7 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { let key_nibbles = Nibbles::new(&entry.key, prev_nibbles.nibble_count()); let parent_nibbles = prev_nibbles.common_prefix(&key_nibbles); - let (_, new_leaf) = - self.updater - .insert(entry.key, entry.value, &parent_nibbles, || entry.leaf_index); + let (_, new_leaf) = self.updater.insert(entry, &parent_nibbles); prev_nibbles = new_leaf.nibbles; self.leaf_count += 1; } @@ -356,10 +346,7 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { patch } - pub fn extend_during_random_recovery( - mut self, - recovery_entries: Vec, - ) -> PatchSet { + pub fn extend_during_random_recovery(mut self, recovery_entries: Vec) -> PatchSet { let load_nodes_latency = BLOCK_TIMINGS.load_nodes.start(); let sorted_keys = SortedKeys::new(recovery_entries.iter().map(|entry| entry.key)); let parent_nibbles = self.updater.load_ancestors(&sorted_keys, self.db); @@ -368,8 +355,7 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { let extend_patch_latency = BLOCK_TIMINGS.extend_patch.start(); for (entry, parent_nibbles) in recovery_entries.into_iter().zip(parent_nibbles) { - self.updater - .insert(entry.key, entry.value, &parent_nibbles, || entry.leaf_index); + self.updater.insert(entry, &parent_nibbles); self.leaf_count += 1; } let extend_patch_latency = extend_patch_latency.observe(); diff --git a/core/lib/merkle_tree/src/storage/patch.rs b/core/lib/merkle_tree/src/storage/patch.rs index 6d0c38d6c9fb..ff41fb2f6bf3 100644 --- a/core/lib/merkle_tree/src/storage/patch.rs +++ b/core/lib/merkle_tree/src/storage/patch.rs @@ -680,7 +680,7 @@ mod tests { use super::*; use crate::{ storage::Storage, - types::{Key, LeafNode}, + types::{Key, LeafNode, TreeEntry}, }; fn patch_len(patch: &WorkingPatchSet) -> usize { @@ -697,7 +697,7 @@ mod tests { let key = Key::from_little_endian(&[i; 32]); let nibbles = Nibbles::new(&key, 2 + usize::from(i) % 4); // ^ We need nibble count at least 2 for all `nibbles` to be distinct. - let leaf = LeafNode::new(key, ValueHash::zero(), i.into()); + let leaf = LeafNode::new(TreeEntry::new(key, i.into(), ValueHash::zero())); patch.insert(nibbles, leaf.into()); nibbles }); @@ -742,7 +742,8 @@ mod tests { // Test DB with a single entry. let mut db = PatchSet::default(); let key = Key::from(1234_u64); - let (_, patch) = Storage::new(&db, &(), 0, true).extend(vec![(key, ValueHash::zero())]); + let (_, patch) = + Storage::new(&db, &(), 0, true).extend(vec![TreeEntry::new(key, 1, ValueHash::zero())]); db.apply_patch(patch); let mut patch = WorkingPatchSet::new(1, db.root(0).unwrap()); @@ -754,8 +755,11 @@ mod tests { // Test DB with multiple entries. let other_key = Key::from_little_endian(&[0xa0; 32]); - let (_, patch) = - Storage::new(&db, &(), 1, true).extend(vec![(other_key, ValueHash::zero())]); + let (_, patch) = Storage::new(&db, &(), 1, true).extend(vec![TreeEntry::new( + other_key, + 2, + ValueHash::zero(), + )]); db.apply_patch(patch); let mut patch = WorkingPatchSet::new(2, db.root(1).unwrap()); @@ -766,8 +770,11 @@ mod tests { assert_eq!(load_result.db_reads, 1); let greater_key = Key::from_little_endian(&[0xaf; 32]); - let (_, patch) = - Storage::new(&db, &(), 2, true).extend(vec![(greater_key, ValueHash::zero())]); + let (_, patch) = Storage::new(&db, &(), 2, true).extend(vec![TreeEntry::new( + greater_key, + 3, + ValueHash::zero(), + )]); db.apply_patch(patch); let mut patch = WorkingPatchSet::new(3, db.root(2).unwrap()); diff --git a/core/lib/merkle_tree/src/storage/proofs.rs b/core/lib/merkle_tree/src/storage/proofs.rs index 9e2d172bd6bd..81f140088d37 100644 --- a/core/lib/merkle_tree/src/storage/proofs.rs +++ b/core/lib/merkle_tree/src/storage/proofs.rs @@ -15,26 +15,6 @@ //! with root at level 4 (= 1 nibble). Thus, the patch sets and Merkle proofs //! produced by each group are mostly disjoint; they intersect only at the root node level. //! -//! ## Computing leaf indices -//! -//! We need to determine leaf indices for all write instructions. Indices potentially depend -//! on the entire list of `instructions`, so we should determine leaf indices before -//! parallelization. Otherwise, we'd need to sync between parallelized tasks, which defeats -//! the purpose of parallelization. -//! -//! We precompute indices as a separate step using the following observations: -//! -//! - If a leaf is present in the tree *before* `instructions` are applied, its index -//! can be obtained from the node ancestors loaded on the first step of the process. -//! - Otherwise, a leaf may have been added by a previous instruction for the same key. -//! Since we already need [`SortedKeys`] to efficiently load ancestors, it's easy -//! to determine such pairs of instructions. -//! - Otherwise, we have a first write, and the leaf index is defined as the current leaf -//! count. -//! -//! In summary, we can determine leaf indices for all write `instructions` in linear time -//! and without synchronization required during the parallel steps of the process. -//! //! ## Merging Merkle proofs //! //! The proofs produced by different groups only intersect at levels 0..4. This can be dealt with @@ -68,7 +48,7 @@ use crate::{ BlockOutputWithProofs, InternalNode, Key, Nibbles, Node, TreeInstruction, TreeLogEntry, TreeLogEntryWithProof, ValueHash, }, - utils::{increment_counter, merge_by_index}, + utils::merge_by_index, }; /// Number of subtrees used for parallel computations. @@ -93,16 +73,13 @@ impl TreeUpdater { for instruction in instructions { let InstructionWithPrecomputes { index, - key, instruction, parent_nibbles, - leaf_index, } = instruction; let log = match instruction { - TreeInstruction::Write(value_hash) => { - let (log, leaf_data) = - self.insert(key, value_hash, &parent_nibbles, || leaf_index); + TreeInstruction::Write(entry) => { + let (log, leaf_data) = self.insert(entry, &parent_nibbles); let (new_root_hash, merkle_path) = self.update_node_hashes(hasher, &leaf_data); root_hash = new_root_hash; TreeLogEntryWithProof { @@ -111,7 +88,7 @@ impl TreeUpdater { root_hash, } } - TreeInstruction::Read => { + TreeInstruction::Read(key) => { let (log, merkle_path) = self.prove(hasher, key, &parent_nibbles); TreeLogEntryWithProof { base: log, @@ -183,7 +160,7 @@ impl TreeUpdater { self.patch_set .create_proof(hasher, key, parent_nibbles, SUBTREE_ROOT_LEVEL / 4); let operation = leaf.map_or(TreeLogEntry::ReadMissingKey, |leaf| { - TreeLogEntry::read(leaf.value_hash, leaf.leaf_index) + TreeLogEntry::read(leaf.leaf_index, leaf.value_hash) }); if matches!(operation, TreeLogEntry::ReadMissingKey) { @@ -259,16 +236,14 @@ impl TreeUpdater { impl<'a, DB: Database + ?Sized> Storage<'a, DB> { pub fn extend_with_proofs( mut self, - instructions: Vec<(Key, TreeInstruction)>, + instructions: Vec, ) -> (BlockOutputWithProofs, PatchSet) { let load_nodes_latency = BLOCK_TIMINGS.load_nodes.start(); - let sorted_keys = SortedKeys::new(instructions.iter().map(|(key, _)| *key)); + let sorted_keys = SortedKeys::new(instructions.iter().map(TreeInstruction::key)); let parent_nibbles = self.updater.load_ancestors(&sorted_keys, self.db); load_nodes_latency.observe(); - let leaf_indices = self.compute_leaf_indices(&instructions, sorted_keys, &parent_nibbles); - let instruction_parts = - InstructionWithPrecomputes::split(instructions, parent_nibbles, leaf_indices); + let instruction_parts = InstructionWithPrecomputes::split(instructions, parent_nibbles); let initial_root = self.updater.patch_set.ensure_internal_root_node(); let initial_metrics = self.updater.metrics; let storage_parts = self.updater.split(); @@ -310,44 +285,13 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { output_with_proofs } - /// Computes leaf indices for all writes in `instructions`. Leaf indices are not used for reads; - /// thus, the corresponding entries are always 0. - fn compute_leaf_indices( - &mut self, - instructions: &[(Key, TreeInstruction)], - mut sorted_keys: SortedKeys, - parent_nibbles: &[Nibbles], - ) -> Vec { - sorted_keys.remove_read_instructions(instructions); - let key_mentions = sorted_keys.key_mentions(instructions.len()); - let patch_set = &self.updater.patch_set; - - let mut leaf_indices = Vec::with_capacity(instructions.len()); - let it = instructions.iter().zip(parent_nibbles).enumerate(); - for (idx, ((key, instruction), nibbles)) in it { - let leaf_index = match (instruction, key_mentions[idx]) { - (TreeInstruction::Read, _) => 0, - // ^ Leaf indices are not used for read instructions. - (TreeInstruction::Write(_), KeyMention::First) => { - let leaf_index = match patch_set.get(nibbles) { - Some(Node::Leaf(leaf)) if leaf.full_key == *key => Some(leaf.leaf_index), - _ => None, - }; - leaf_index.unwrap_or_else(|| increment_counter(&mut self.leaf_count)) - } - (TreeInstruction::Write(_), KeyMention::SameAs(prev_idx)) => leaf_indices[prev_idx], - }; - leaf_indices.push(leaf_index); - } - leaf_indices - } - fn finalize_with_proofs( mut self, hasher: &mut HasherWithStats<'_>, root: InternalNode, logs: Vec<(usize, TreeLogEntryWithProof)>, ) -> (BlockOutputWithProofs, PatchSet) { + self.leaf_count += self.updater.metrics.new_leaves; tracing::debug!( "Finished updating tree; total leaf count: {}, stats: {:?}", self.leaf_count, @@ -370,95 +314,35 @@ impl<'a, DB: Database + ?Sized> Storage<'a, DB> { } } -/// Mention of a key in a block: either the first mention, or the same mention as the specified -/// 0-based index in the block. -#[derive(Debug, Clone, Copy)] -enum KeyMention { - First, - SameAs(usize), -} - -impl SortedKeys { - fn remove_read_instructions(&mut self, instructions: &[(Key, TreeInstruction)]) { - debug_assert_eq!(instructions.len(), self.0.len()); - - self.0.retain(|(idx, key)| { - let (key_for_instruction, instruction) = &instructions[*idx]; - debug_assert_eq!(key_for_instruction, key); - matches!(instruction, TreeInstruction::Write(_)) - }); - } - - /// Determines for the original sequence of `Key`s whether a particular key mention - /// is the first one, or it follows after another mention. - fn key_mentions(&self, original_len: usize) -> Vec { - debug_assert!(original_len >= self.0.len()); - - let mut flags = vec![KeyMention::First; original_len]; - let [(mut first_key_mention, mut prev_key), tail @ ..] = self.0.as_slice() else { - return flags; - }; - - // Note that `SameAs(_)` doesn't necessarily reference the first mention of a key, - // just one with a lesser index. This is OK for our purposes. - for &(idx, key) in tail { - if prev_key == key { - if idx > first_key_mention { - flags[idx] = KeyMention::SameAs(first_key_mention); - } else { - debug_assert!(idx < first_key_mention); // all indices should be unique - flags[first_key_mention] = KeyMention::SameAs(idx); - first_key_mention = idx; - } - } else { - prev_key = key; - first_key_mention = idx; - } - } - flags - } -} - /// [`TreeInstruction`] together with precomputed data necessary to efficiently parallelize /// Merkle tree traversal. #[derive(Debug)] struct InstructionWithPrecomputes { /// 0-based index of the instruction. index: usize, - /// Key read / written by the instruction. - key: Key, instruction: TreeInstruction, /// Nibbles for the parent node computed by [`Storage::load_ancestors()`]. parent_nibbles: Nibbles, - /// Leaf index for the operation computed by [`Storage::compute_leaf_indices()`]. - /// Always 0 for reads. - leaf_index: u64, } impl InstructionWithPrecomputes { /// Creates groups of instructions to be used during parallelized tree traversal. fn split( - instructions: Vec<(Key, TreeInstruction)>, + instructions: Vec, parent_nibbles: Vec, - leaf_indices: Vec, ) -> [Vec; SUBTREE_COUNT] { const EMPTY_VEC: Vec = Vec::new(); // ^ Need to extract this to a constant to be usable as an array initializer. let mut parts = [EMPTY_VEC; SUBTREE_COUNT]; - let it = instructions - .into_iter() - .zip(parent_nibbles) - .zip(leaf_indices); - for (index, (((key, instruction), parent_nibbles), leaf_index)) in it.enumerate() { - let first_nibble = Nibbles::nibble(&key, 0); + let it = instructions.into_iter().zip(parent_nibbles); + for (index, (instruction, parent_nibbles)) in it.enumerate() { + let first_nibble = Nibbles::nibble(&instruction.key(), 0); let part = &mut parts[first_nibble as usize]; part.push(Self { index, - key, instruction, parent_nibbles, - leaf_index, }); } parts @@ -472,8 +356,6 @@ mod tests { use super::*; use crate::types::Root; - const HASH: ValueHash = ValueHash::zero(); - fn byte_key(byte: u8) -> Key { Key::from_little_endian(&[byte; 32]) } @@ -485,88 +367,14 @@ mod tests { assert_eq!(sorted_keys.0, [1, 3, 4, 0, 2].map(|i| (i, keys[i]))); } - #[test] - fn computing_key_mentions() { - let keys = [4, 1, 3, 4, 3, 3].map(byte_key); - let sorted_keys = SortedKeys::new(keys.into_iter()); - let mentions = sorted_keys.key_mentions(6); - - assert_matches!( - mentions.as_slice(), - [ - KeyMention::First, KeyMention::First, KeyMention::First, - KeyMention::SameAs(0), KeyMention::SameAs(2), KeyMention::SameAs(i) - ] if *i == 2 || *i == 4 - ); - } - - #[test] - fn computing_leaf_indices() { - let db = prepare_db(); - let (instructions, expected_indices) = get_instructions_and_leaf_indices(); - let mut storage = Storage::new(&db, &(), 1, true); - let sorted_keys = SortedKeys::new(instructions.iter().map(|(key, _)| *key)); - let parent_nibbles = storage.updater.load_ancestors(&sorted_keys, &db); - - let leaf_indices = - storage.compute_leaf_indices(&instructions, sorted_keys, &parent_nibbles); - assert_eq!(leaf_indices, expected_indices); - } - - fn prepare_db() -> PatchSet { - let mut db = PatchSet::default(); - let (_, patch) = - Storage::new(&db, &(), 0, true).extend(vec![(byte_key(2), HASH), (byte_key(1), HASH)]); - db.apply_patch(patch); - db - } - - fn get_instructions_and_leaf_indices() -> (Vec<(Key, TreeInstruction)>, Vec) { - let instructions_and_indices = vec![ - (byte_key(3), TreeInstruction::Read, 0), - (byte_key(1), TreeInstruction::Write(HASH), 2), - (byte_key(2), TreeInstruction::Read, 0), - (byte_key(3), TreeInstruction::Write(HASH), 3), - (byte_key(1), TreeInstruction::Read, 0), - (byte_key(3), TreeInstruction::Write(HASH), 3), - (byte_key(2), TreeInstruction::Write(HASH), 1), - (byte_key(0xc0), TreeInstruction::Write(HASH), 4), - (byte_key(2), TreeInstruction::Write(HASH), 1), - ]; - instructions_and_indices - .into_iter() - .map(|(key, instr, idx)| ((key, instr), idx)) - .unzip() - } - - #[test] - fn extending_storage_with_proofs() { - let db = prepare_db(); - let (instructions, expected_indices) = get_instructions_and_leaf_indices(); - let storage = Storage::new(&db, &(), 1, true); - let (block_output, _) = storage.extend_with_proofs(instructions); - assert_eq!(block_output.leaf_count, 4); - - assert_eq!(block_output.logs.len(), expected_indices.len()); - for (expected_idx, log) in expected_indices.into_iter().zip(&block_output.logs) { - match log.base { - TreeLogEntry::Inserted { leaf_index } - | TreeLogEntry::Updated { leaf_index, .. } => { - assert_eq!(leaf_index, expected_idx); - } - _ => {} - } - } - } - #[test] fn proofs_for_empty_storage() { let db = PatchSet::default(); let storage = Storage::new(&db, &(), 0, true); let instructions = vec![ - (byte_key(1), TreeInstruction::Read), - (byte_key(2), TreeInstruction::Read), - (byte_key(0xff), TreeInstruction::Read), + TreeInstruction::Read(byte_key(1)), + TreeInstruction::Read(byte_key(2)), + TreeInstruction::Read(byte_key(0xff)), ]; let (block_output, patch) = storage.extend_with_proofs(instructions); assert_eq!(block_output.leaf_count, 0); diff --git a/core/lib/merkle_tree/src/storage/serialization.rs b/core/lib/merkle_tree/src/storage/serialization.rs index 15d67604cc04..6a9216fa104a 100644 --- a/core/lib/merkle_tree/src/storage/serialization.rs +++ b/core/lib/merkle_tree/src/storage/serialization.rs @@ -26,7 +26,11 @@ impl LeafNode { let leaf_index = leb128::read::unsigned(&mut bytes).map_err(|err| { DeserializeErrorKind::Leb128(err).with_context(ErrorContext::LeafIndex) })?; - Ok(Self::new(full_key, value_hash, leaf_index)) + Ok(Self { + full_key, + value_hash, + leaf_index, + }) } pub(super) fn serialize(&self, buffer: &mut Vec) { @@ -297,6 +301,7 @@ impl Manifest { #[cfg(test)] mod tests { use super::*; + use crate::types::TreeEntry; use zksync_types::H256; #[test] @@ -369,7 +374,7 @@ mod tests { #[test] fn serializing_leaf_node() { - let leaf = LeafNode::new(513.into(), H256([4; 32]), 42); + let leaf = LeafNode::new(TreeEntry::new(513.into(), 42, H256([4; 32]))); let mut buffer = vec![]; leaf.serialize(&mut buffer); assert_eq!(buffer[..30], [0; 30]); // padding for the key @@ -426,7 +431,7 @@ mod tests { #[test] fn serializing_root_with_leaf() { - let leaf = LeafNode::new(513.into(), H256([4; 32]), 42); + let leaf = LeafNode::new(TreeEntry::new(513.into(), 42, H256([4; 32]))); let root = Root::new(1, leaf.into()); let mut buffer = vec![]; root.serialize(&mut buffer); diff --git a/core/lib/merkle_tree/src/storage/tests.rs b/core/lib/merkle_tree/src/storage/tests.rs index 958c906289ea..e70cb057280e 100644 --- a/core/lib/merkle_tree/src/storage/tests.rs +++ b/core/lib/merkle_tree/src/storage/tests.rs @@ -25,7 +25,7 @@ pub(super) fn generate_nodes(version: u64, nibble_counts: &[usize]) -> HashMap) -> V fn reading_keys_does_not_change_child_version() { let mut db = PatchSet::default(); let storage = Storage::new(&db, &(), 0, true); - let kvs = vec![(FIRST_KEY, H256([0; 32])), (SECOND_KEY, H256([1; 32]))]; + let kvs = vec![ + TreeEntry::new(FIRST_KEY, 1, H256([0; 32])), + TreeEntry::new(SECOND_KEY, 2, H256([1; 32])), + ]; let (_, patch) = storage.extend(kvs); db.apply_patch(patch); let storage = Storage::new(&db, &(), 1, true); let instructions = vec![ - (FIRST_KEY, TreeInstruction::Read), - (E_KEY, TreeInstruction::Write(H256([2; 32]))), + TreeInstruction::Read(FIRST_KEY), + TreeInstruction::Write(TreeEntry::new(E_KEY, 3, H256([2; 32]))), ]; let (_, patch) = storage.extend_with_proofs(instructions); @@ -327,12 +339,15 @@ fn reading_keys_does_not_change_child_version() { fn read_ops_are_not_reflected_in_patch() { let mut db = PatchSet::default(); let storage = Storage::new(&db, &(), 0, true); - let kvs = vec![(FIRST_KEY, H256([0; 32])), (SECOND_KEY, H256([1; 32]))]; + let kvs = vec![ + TreeEntry::new(FIRST_KEY, 1, H256([0; 32])), + TreeEntry::new(SECOND_KEY, 2, H256([1; 32])), + ]; let (_, patch) = storage.extend(kvs); db.apply_patch(patch); let storage = Storage::new(&db, &(), 1, true); - let instructions = vec![(FIRST_KEY, TreeInstruction::Read)]; + let instructions = vec![TreeInstruction::Read(FIRST_KEY)]; let (_, patch) = storage.extend_with_proofs(instructions); assert!(patch.patches_by_version[&1].nodes.is_empty()); } @@ -351,7 +366,7 @@ fn read_instructions_do_not_lead_to_copied_nodes(writes_per_block: u64) { let mut database = PatchSet::default(); let storage = Storage::new(&database, &(), 0, true); let kvs = (0..key_count) - .map(|i| (big_endian_key(i), H256::zero())) + .map(|i| TreeEntry::new(big_endian_key(i), i + 1, H256::zero())) .collect(); let (_, patch) = storage.extend(kvs); database.apply_patch(patch); @@ -361,10 +376,11 @@ fn read_instructions_do_not_lead_to_copied_nodes(writes_per_block: u64) { // Select some existing keys to read. Keys may be repeated, this is fine for our purpose. let reads = (0..writes_per_block).map(|_| { let key = big_endian_key(rng.gen_range(0..key_count)); - (key, TreeInstruction::Read) + TreeInstruction::Read(key) + }); + let writes = (key_count..key_count + writes_per_block).map(|i| { + TreeInstruction::Write(TreeEntry::new(big_endian_key(i), i + 1, H256::zero())) }); - let writes = (key_count..key_count + writes_per_block) - .map(|i| (big_endian_key(i), TreeInstruction::Write(H256::zero()))); let mut instructions: Vec<_> = reads.chain(writes).collect(); instructions.shuffle(&mut rng); @@ -400,7 +416,7 @@ fn replaced_keys_are_correctly_tracked(writes_per_block: usize, with_proofs: boo let mut database = PatchSet::default(); let storage = Storage::new(&database, &(), 0, true); let kvs = (0..100) - .map(|i| (big_endian_key(i), H256::zero())) + .map(|i| TreeEntry::new(big_endian_key(i), i + 1, H256::zero())) .collect(); let (_, patch) = storage.extend(kvs); @@ -412,11 +428,11 @@ fn replaced_keys_are_correctly_tracked(writes_per_block: usize, with_proofs: boo let updates = (0..100) .choose_multiple(&mut rng, writes_per_block) .into_iter() - .map(|i| (big_endian_key(i), H256::zero())); + .map(|i| TreeEntry::new(big_endian_key(i), i + 1, H256::zero())); let storage = Storage::new(&database, &(), new_version, true); let patch = if with_proofs { - let instructions = updates.map(|(key, value)| (key, TreeInstruction::Write(value))); + let instructions = updates.map(TreeInstruction::Write); storage.extend_with_proofs(instructions.collect()).1 } else { storage.extend(updates.collect()).1 @@ -454,14 +470,18 @@ fn assert_replaced_keys(db: &PatchSet, patch: &PatchSet) { #[test] fn tree_handles_keys_at_terminal_level() { let mut db = PatchSet::default(); - let kvs = (0_u32..100) - .map(|i| (Key::from(i), ValueHash::zero())) + let kvs = (0_u64..100) + .map(|i| TreeEntry::new(Key::from(i), i + 1, ValueHash::zero())) .collect(); let (_, patch) = Storage::new(&db, &(), 0, true).extend(kvs); db.apply_patch(patch); // Overwrite a key and check that we don't panic. - let new_kvs = vec![(Key::from(0), ValueHash::from_low_u64_be(1))]; + let new_kvs = vec![TreeEntry::new( + Key::from(0), + 1, + ValueHash::from_low_u64_be(1), + )]; let (_, patch) = Storage::new(&db, &(), 1, true).extend(new_kvs); assert_eq!( @@ -483,7 +503,7 @@ fn tree_handles_keys_at_terminal_level() { #[test] fn recovery_flattens_node_versions() { let recovery_version = 100; - let recovery_entries = (0_u64..10).map(|i| RecoveryEntry { + let recovery_entries = (0_u64..10).map(|i| TreeEntry { key: Key::from(i) << 252, // the first key nibbles are distinct value: ValueHash::zero(), leaf_index: i + 1, @@ -516,7 +536,7 @@ fn recovery_flattens_node_versions() { #[test_casing(7, [256, 4, 5, 20, 69, 127, 128])] fn recovery_with_node_hierarchy(chunk_size: usize) { let recovery_version = 100; - let recovery_entries = (0_u64..256).map(|i| RecoveryEntry { + let recovery_entries = (0_u64..256).map(|i| TreeEntry { key: Key::from(i) << 248, // the first two key nibbles are distinct value: ValueHash::zero(), leaf_index: i + 1, @@ -567,7 +587,7 @@ fn recovery_with_node_hierarchy(chunk_size: usize) { #[test_casing(7, [256, 5, 7, 20, 59, 127, 128])] fn recovery_with_deep_node_hierarchy(chunk_size: usize) { let recovery_version = 1_000; - let recovery_entries = (0_u64..256).map(|i| RecoveryEntry { + let recovery_entries = (0_u64..256).map(|i| TreeEntry { key: Key::from(i), // the last two key nibbles are distinct value: ValueHash::zero(), leaf_index: i + 1, @@ -630,7 +650,7 @@ fn recovery_with_deep_node_hierarchy(chunk_size: usize) { fn recovery_workflow_with_multiple_stages() { let mut db = PatchSet::default(); let recovery_version = 100; - let recovery_entries = (0_u64..100).map(|i| RecoveryEntry { + let recovery_entries = (0_u64..100).map(|i| TreeEntry { key: Key::from(i), value: ValueHash::zero(), leaf_index: i, @@ -640,7 +660,7 @@ fn recovery_workflow_with_multiple_stages() { assert_eq!(patch.root(recovery_version).unwrap().leaf_count(), 100); db.apply_patch(patch); - let more_recovery_entries = (100_u64..200).map(|i| RecoveryEntry { + let more_recovery_entries = (100_u64..200).map(|i| TreeEntry { key: Key::from(i), value: ValueHash::zero(), leaf_index: i, @@ -653,7 +673,7 @@ fn recovery_workflow_with_multiple_stages() { // Check that all entries can be accessed let storage = Storage::new(&db, &(), recovery_version + 1, true); - let instructions = (0_u32..200).map(|i| (Key::from(i), TreeInstruction::Read)); + let instructions = (0_u32..200).map(|i| TreeInstruction::Read(Key::from(i))); let (output, _) = storage.extend_with_proofs(instructions.collect()); assert_eq!(output.leaf_count, 200); assert_eq!(output.logs.len(), 200); @@ -687,17 +707,15 @@ fn test_recovery_pruning_equivalence( ); let mut rng = StdRng::seed_from_u64(RNG_SEED); - let kvs = (0..100).map(|i| { - ( - U256([rng.gen(), rng.gen(), rng.gen(), rng.gen()]), - ValueHash::repeat_byte(i), - ) + let entries = (0..100).map(|i| { + let key = U256([rng.gen(), rng.gen(), rng.gen(), rng.gen()]); + TreeEntry::new(key, u64::from(i) + 1, ValueHash::repeat_byte(i)) }); - let kvs: Vec<_> = kvs.collect(); + let entries: Vec<_> = entries.collect(); // Add `kvs` into the tree in several commits. let mut db = PatchSet::default(); - for (version, chunk) in kvs.chunks(chunk_size).enumerate() { + for (version, chunk) in entries.chunks(chunk_size).enumerate() { let (_, patch) = Storage::new(&db, hasher, version as u64, true).extend(chunk.to_vec()); db.apply_patch(patch); } @@ -716,11 +734,7 @@ fn test_recovery_pruning_equivalence( // Generate recovery entries. let recovery_entries = all_nodes.values().filter_map(|node| { if let Node::Leaf(leaf) = node { - return Some(RecoveryEntry { - key: leaf.full_key, - value: leaf.value_hash, - leaf_index: leaf.leaf_index, - }); + return Some(TreeEntry::from(*leaf)); } None }); diff --git a/core/lib/merkle_tree/src/types/internal.rs b/core/lib/merkle_tree/src/types/internal.rs index 5e875f6e28ac..cb35b0281c2b 100644 --- a/core/lib/merkle_tree/src/types/internal.rs +++ b/core/lib/merkle_tree/src/types/internal.rs @@ -4,10 +4,9 @@ use std::{fmt, num::NonZeroU64}; -use zksync_types::{H256, U256}; - use crate::{ hasher::{HashTree, InternalNodeCache}, + types::{Key, TreeEntry, ValueHash}, utils::SmallMap, }; @@ -323,11 +322,6 @@ impl fmt::Display for NodeKey { } } -/// Key stored in the tree. -pub type Key = U256; -/// Hashed value stored in the tree. -pub type ValueHash = H256; - /// Leaf node of the tree. #[derive(Debug, Clone, Copy)] #[cfg_attr(test, derive(PartialEq, Eq))] @@ -338,13 +332,18 @@ pub struct LeafNode { } impl LeafNode { - pub(crate) fn new(full_key: Key, value_hash: ValueHash, leaf_index: u64) -> Self { + pub(crate) fn new(entry: TreeEntry) -> Self { Self { - full_key, - value_hash, - leaf_index, + full_key: entry.key, + value_hash: entry.value, + leaf_index: entry.leaf_index, } } + + pub(crate) fn update_from(&mut self, entry: TreeEntry) { + self.value_hash = entry.value; + self.leaf_index = entry.leaf_index; + } } /// Reference to a child in an [`InternalNode`]. @@ -556,6 +555,7 @@ impl StaleNodeKey { #[cfg(test)] mod tests { use super::*; + use zksync_types::U256; // `U256` uses little-endian `u64` ordering; i.e., this is // 0x_dead_beef_0000_0000_.._0000. diff --git a/core/lib/merkle_tree/src/types/mod.rs b/core/lib/merkle_tree/src/types/mod.rs index de35d9024b7b..15ab72b6911d 100644 --- a/core/lib/merkle_tree/src/types/mod.rs +++ b/core/lib/merkle_tree/src/types/mod.rs @@ -5,22 +5,53 @@ mod internal; pub(crate) use self::internal::{ ChildRef, Nibbles, NibblesBytes, StaleNodeKey, TreeTags, HASH_SIZE, KEY_SIZE, TREE_DEPTH, }; -pub use self::internal::{InternalNode, Key, LeafNode, Manifest, Node, NodeKey, Root, ValueHash}; +pub use self::internal::{InternalNode, LeafNode, Manifest, Node, NodeKey, Root}; + +use zksync_types::{H256, U256}; + +/// Key stored in the tree. +pub type Key = U256; +/// Hash type of values and intermediate nodes in the tree. +pub type ValueHash = H256; /// Instruction to read or write a tree value at a certain key. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum TreeInstruction { - /// Read the current tree value. - Read, - /// Write the specified value. - Write(ValueHash), +pub enum TreeInstruction { + /// Read the current tree value at the specified key. + Read(K), + /// Write the specified entry. + Write(TreeEntry), +} + +impl TreeInstruction { + /// Creates a write instruction. + pub fn write(key: K, leaf_index: u64, value: ValueHash) -> Self { + Self::Write(TreeEntry::new(key, leaf_index, value)) + } + + /// Returns the tree key this instruction is related to. + pub fn key(&self) -> K { + match self { + Self::Read(key) => *key, + Self::Write(entry) => entry.key, + } + } + + pub(crate) fn map_key(&self, map_fn: impl FnOnce(&K) -> U) -> TreeInstruction { + match self { + Self::Read(key) => TreeInstruction::Read(map_fn(key)), + Self::Write(entry) => TreeInstruction::Write(entry.map_key(map_fn)), + } + } } /// Entry in a Merkle tree associated with a key. -#[derive(Debug, Clone, Copy)] -pub struct TreeEntry { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct TreeEntry { + /// Tree key. + pub key: K, /// Value associated with the key. - pub value_hash: ValueHash, + pub value: ValueHash, /// Enumeration index of the key. pub leaf_index: u64, } @@ -28,23 +59,40 @@ pub struct TreeEntry { impl From for TreeEntry { fn from(leaf: LeafNode) -> Self { Self { - value_hash: leaf.value_hash, + key: leaf.full_key, + value: leaf.value_hash, leaf_index: leaf.leaf_index, } } } +impl TreeEntry { + /// Creates a new entry with the specified fields. + pub fn new(key: K, leaf_index: u64, value: ValueHash) -> Self { + Self { + key, + value, + leaf_index, + } + } + + pub(crate) fn map_key(&self, map_fn: impl FnOnce(&K) -> U) -> TreeEntry { + TreeEntry::new(map_fn(&self.key), self.leaf_index, self.value) + } +} + impl TreeEntry { - pub(crate) fn empty() -> Self { + pub(crate) fn empty(key: Key) -> Self { Self { - value_hash: ValueHash::zero(), + key, + value: ValueHash::zero(), leaf_index: 0, } } /// Returns `true` if and only if this entry encodes lack of a value. pub fn is_empty(&self) -> bool { - self.leaf_index == 0 && self.value_hash.is_zero() + self.leaf_index == 0 && self.value.is_zero() } pub(crate) fn with_merkle_path(self, merkle_path: Vec) -> TreeEntryWithProof { @@ -53,6 +101,12 @@ impl TreeEntry { merkle_path, } } + + /// Replaces the value in this entry and returns the modified entry. + #[must_use] + pub fn with_value(self, value: H256) -> Self { + Self { value, ..self } + } } /// Entry in a Merkle tree together with a proof of authenticity. @@ -86,10 +140,7 @@ pub struct BlockOutput { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum TreeLogEntry { /// A node was inserted into the tree. - Inserted { - /// Index of the inserted node. - leaf_index: u64, - }, + Inserted, /// A node with the specified index was updated. Updated { /// Index of the updated node. @@ -109,18 +160,14 @@ pub enum TreeLogEntry { } impl TreeLogEntry { - pub(crate) fn insert(leaf_index: u64) -> Self { - Self::Inserted { leaf_index } - } - - pub(crate) fn update(previous_value: ValueHash, leaf_index: u64) -> Self { + pub(crate) fn update(leaf_index: u64, previous_value: ValueHash) -> Self { Self::Updated { leaf_index, previous_value, } } - pub(crate) fn read(value: ValueHash, leaf_index: u64) -> Self { + pub(crate) fn read(leaf_index: u64, value: ValueHash) -> Self { Self::Read { leaf_index, value } } diff --git a/core/lib/merkle_tree/src/utils.rs b/core/lib/merkle_tree/src/utils.rs index 9542b24bbd3c..4771a940f2c8 100644 --- a/core/lib/merkle_tree/src/utils.rs +++ b/core/lib/merkle_tree/src/utils.rs @@ -114,11 +114,6 @@ impl SmallMap { } } -pub(crate) fn increment_counter(counter: &mut u64) -> u64 { - *counter += 1; - *counter -} - pub(crate) fn find_diverging_bit(lhs: Key, rhs: Key) -> usize { let diff = lhs ^ rhs; diff.leading_zeros() as usize diff --git a/core/lib/merkle_tree/tests/integration/common.rs b/core/lib/merkle_tree/tests/integration/common.rs index fd9e00855c20..096a54ce7111 100644 --- a/core/lib/merkle_tree/tests/integration/common.rs +++ b/core/lib/merkle_tree/tests/integration/common.rs @@ -5,23 +5,22 @@ use once_cell::sync::Lazy; use std::collections::HashMap; use zksync_crypto::hasher::{blake2::Blake2Hasher, Hasher}; -use zksync_merkle_tree::{HashTree, TreeInstruction}; +use zksync_merkle_tree::{HashTree, TreeEntry, TreeInstruction}; use zksync_types::{AccountTreeId, Address, StorageKey, H256, U256}; -pub fn generate_key_value_pairs(indexes: impl Iterator) -> Vec<(U256, H256)> { +pub fn generate_key_value_pairs(indexes: impl Iterator) -> Vec { let address: Address = "4b3af74f66ab1f0da3f2e4ec7a3cb99baf1af7b2".parse().unwrap(); let kvs = indexes.map(|idx| { let key = H256::from_low_u64_be(idx); let key = StorageKey::new(AccountTreeId::new(address), key); - (key.hashed_key_u256(), H256::from_low_u64_be(idx + 1)) + let value = H256::from_low_u64_be(idx + 1); + TreeEntry::new(key.hashed_key_u256(), idx + 1, value) }); kvs.collect() } -pub fn compute_tree_hash(kvs: impl Iterator) -> H256 { - let kvs_with_indices = kvs - .enumerate() - .map(|(i, (key, value))| (key, value, i as u64 + 1)); +pub fn compute_tree_hash(kvs: impl Iterator) -> H256 { + let kvs_with_indices = kvs.map(|entry| (entry.key, entry.value, entry.leaf_index)); compute_tree_hash_with_indices(kvs_with_indices) } @@ -70,17 +69,18 @@ fn compute_tree_hash_with_indices(kvs: impl Iterator) } // Computing the expected hash takes some time in the debug mode, so we memoize it. -pub static KVS_AND_HASH: Lazy<(Vec<(U256, H256)>, H256)> = Lazy::new(|| { - let kvs = generate_key_value_pairs(0..100); - let expected_hash = compute_tree_hash(kvs.iter().copied()); - (kvs, expected_hash) +pub static ENTRIES_AND_HASH: Lazy<(Vec, H256)> = Lazy::new(|| { + let entries = generate_key_value_pairs(0..100); + let expected_hash = compute_tree_hash(entries.iter().copied()); + (entries, expected_hash) }); -pub fn convert_to_writes(kvs: &[(U256, H256)]) -> Vec<(U256, TreeInstruction)> { - let kvs = kvs +pub fn convert_to_writes(entries: &[TreeEntry]) -> Vec { + entries .iter() - .map(|&(key, hash)| (key, TreeInstruction::Write(hash))); - kvs.collect() + .copied() + .map(TreeInstruction::Write) + .collect() } /// Emulates leaf index assignment in a real Merkle tree. @@ -88,22 +88,22 @@ pub fn convert_to_writes(kvs: &[(U256, H256)]) -> Vec<(U256, TreeInstruction)> { pub struct TreeMap(HashMap); impl TreeMap { - pub fn new(initial_entries: &[(U256, H256)]) -> Self { + pub fn new(initial_entries: &[TreeEntry]) -> Self { let map = initial_entries .iter() - .enumerate() - .map(|(i, (key, value))| (*key, (*value, i as u64 + 1))) + .map(|entry| (entry.key, (entry.value, entry.leaf_index))) .collect(); Self(map) } - pub fn extend(&mut self, kvs: &[(U256, H256)]) { - for &(key, new_value) in kvs { - if let Some((value, _)) = self.0.get_mut(&key) { - *value = new_value; + pub fn extend(&mut self, kvs: &[TreeEntry]) { + for &new_entry in kvs { + if let Some((value, leaf_index)) = self.0.get_mut(&new_entry.key) { + assert_eq!(*leaf_index, new_entry.leaf_index); // sanity check + *value = new_entry.value; } else { - let leaf_index = self.0.len() as u64 + 1; - self.0.insert(key, (new_value, leaf_index)); + self.0 + .insert(new_entry.key, (new_entry.value, new_entry.leaf_index)); } } } @@ -112,7 +112,7 @@ impl TreeMap { let entries = self .0 .iter() - .map(|(key, (value, idx))| (*key, *value, *idx)); + .map(|(key, (value, leaf_index))| (*key, *value, *leaf_index)); compute_tree_hash_with_indices(entries) } } diff --git a/core/lib/merkle_tree/tests/integration/consistency.rs b/core/lib/merkle_tree/tests/integration/consistency.rs index 7c1d69657bff..da3312d2002d 100644 --- a/core/lib/merkle_tree/tests/integration/consistency.rs +++ b/core/lib/merkle_tree/tests/integration/consistency.rs @@ -26,7 +26,7 @@ fn five_thousand_angry_monkeys_vs_merkle_tree() { let kvs = generate_key_value_pairs(0..100); tree.extend(kvs); - tree.verify_consistency(0).unwrap(); + tree.verify_consistency(0, true).unwrap(); let mut raw_db = db.into_inner(); let cf = MerkleTreeColumnFamily::Tree; @@ -53,7 +53,9 @@ fn five_thousand_angry_monkeys_vs_merkle_tree() { raw_db.write(batch).unwrap(); let mut db = RocksDBWrapper::from(raw_db); - let err = MerkleTree::new(&mut db).verify_consistency(0).unwrap_err(); + let err = MerkleTree::new(&mut db) + .verify_consistency(0, true) + .unwrap_err(); println!("{err}"); // Restore the value back so that it doesn't influence the following cases. diff --git a/core/lib/merkle_tree/tests/integration/domain.rs b/core/lib/merkle_tree/tests/integration/domain.rs index d3b666c88492..f3febda5f06a 100644 --- a/core/lib/merkle_tree/tests/integration/domain.rs +++ b/core/lib/merkle_tree/tests/integration/domain.rs @@ -7,14 +7,14 @@ use tempfile::TempDir; use std::slice; use zksync_crypto::hasher::blake2::Blake2Hasher; -use zksync_merkle_tree::{domain::ZkSyncTree, HashTree}; +use zksync_merkle_tree::{domain::ZkSyncTree, HashTree, TreeEntry, TreeInstruction}; use zksync_storage::RocksDB; use zksync_system_constants::ACCOUNT_CODE_STORAGE_ADDRESS; use zksync_types::{ - proofs::StorageLogMetadata, AccountTreeId, Address, L1BatchNumber, StorageKey, StorageLog, H256, + proofs::StorageLogMetadata, AccountTreeId, Address, L1BatchNumber, StorageKey, H256, }; -fn gen_storage_logs() -> Vec { +fn gen_storage_logs() -> Vec> { let addrs = vec![ "4b3af74f66ab1f0da3f2e4ec7a3cb99baf1af7b2", "ef4bb7b21c5fe7432a7d63876cc59ecc23b46636", @@ -32,7 +32,11 @@ fn gen_storage_logs() -> Vec { proof_keys .zip(proof_values) - .map(|(proof_key, proof_value)| StorageLog::new_write_log(proof_key, proof_value)) + .enumerate() + .map(|(i, (proof_key, proof_value))| { + let entry = TreeEntry::new(proof_key, i as u64 + 1, proof_value); + TreeInstruction::Write(entry) + }) .collect() } @@ -54,7 +58,11 @@ fn basic_workflow() { assert_eq!(metadata.rollup_last_leaf_index, 101); assert_eq!(metadata.initial_writes.len(), logs.len()); for (write, log) in metadata.initial_writes.iter().zip(&logs) { - assert_eq!(write.value, log.value); + let expected_value = match log { + TreeInstruction::Write(entry) => entry.value, + TreeInstruction::Read(_) => unreachable!(), + }; + assert_eq!(write.value, expected_value); } assert!(metadata.repeated_writes.is_empty()); @@ -124,7 +132,10 @@ fn filtering_out_no_op_writes() { // Add some actual repeated writes. let mut expected_writes_count = 0; for log in logs.iter_mut().step_by(3) { - log.value = H256::repeat_byte(0xff); + let TreeInstruction::Write(entry) = log else { + unreachable!("Unexpected instruction: {log:?}"); + }; + entry.value = H256::repeat_byte(0xff); expected_writes_count += 1; } let new_metadata = tree.process_l1_batch(&logs); @@ -155,14 +166,16 @@ fn revert_blocks() { // Add couple of blocks of distinct keys/values let mut logs: Vec<_> = proof_keys .zip(proof_values) - .map(|(proof_key, proof_value)| StorageLog::new_write_log(proof_key, proof_value)) + .map(|(proof_key, proof_value)| { + let entry = TreeEntry::new(proof_key, proof_value.to_low_u64_be() + 1, proof_value); + TreeInstruction::Write(entry) + }) .collect(); // Add a block with repeated keys let extra_logs = (0..block_size).map(move |i| { - StorageLog::new_write_log( - StorageKey::new(AccountTreeId::new(address), H256::from_low_u64_be(i as u64)), - H256::from_low_u64_be((i + 1) as u64), - ) + let key = StorageKey::new(AccountTreeId::new(address), H256::from_low_u64_be(i as u64)); + let entry = TreeEntry::new(key, i as u64 + 1, H256::from_low_u64_be(i as u64 + 1)); + TreeInstruction::Write(entry) }); logs.extend(extra_logs); @@ -277,7 +290,7 @@ fn read_logs() { let mut tree = ZkSyncTree::new_lightweight(db); let read_logs: Vec<_> = logs .into_iter() - .map(|log| StorageLog::new_read_log(log.key, log.value)) + .map(|instr| TreeInstruction::Read(instr.key())) .collect(); let read_metadata = tree.process_l1_batch(&read_logs); @@ -285,14 +298,13 @@ fn read_logs() { } fn create_write_log( + leaf_index: u64, address: Address, address_storage_key: [u8; 32], value: [u8; 32], -) -> StorageLog { - StorageLog::new_write_log( - StorageKey::new(AccountTreeId::new(address), H256(address_storage_key)), - H256(value), - ) +) -> TreeInstruction { + let key = StorageKey::new(AccountTreeId::new(address), H256(address_storage_key)); + TreeInstruction::Write(TreeEntry::new(key, leaf_index, H256(value))) } fn subtract_from_max_value(diff: u8) -> [u8; 32] { @@ -315,28 +327,33 @@ fn root_hash_compatibility() { ); let storage_logs = vec![ - create_write_log(ACCOUNT_CODE_STORAGE_ADDRESS, [0; 32], [1; 32]), + create_write_log(1, ACCOUNT_CODE_STORAGE_ADDRESS, [0; 32], [1; 32]), create_write_log( + 2, Address::from_low_u64_be(9223372036854775808), [254; 32], subtract_from_max_value(1), ), create_write_log( + 3, Address::from_low_u64_be(9223372036854775809), [253; 32], subtract_from_max_value(2), ), create_write_log( + 4, Address::from_low_u64_be(9223372036854775810), [252; 32], subtract_from_max_value(3), ), create_write_log( + 5, Address::from_low_u64_be(9223372036854775811), [251; 32], subtract_from_max_value(4), ), create_write_log( + 6, Address::from_low_u64_be(9223372036854775812), [250; 32], subtract_from_max_value(5), diff --git a/core/lib/merkle_tree/tests/integration/merkle_tree.rs b/core/lib/merkle_tree/tests/integration/merkle_tree.rs index 9f3eb970cd38..e4f052bb03c4 100644 --- a/core/lib/merkle_tree/tests/integration/merkle_tree.rs +++ b/core/lib/merkle_tree/tests/integration/merkle_tree.rs @@ -7,12 +7,14 @@ use std::{cmp, mem}; use zksync_crypto::hasher::blake2::Blake2Hasher; use zksync_merkle_tree::{ - Database, HashTree, MerkleTree, PatchSet, Patched, TreeInstruction, TreeLogEntry, + Database, HashTree, MerkleTree, PatchSet, Patched, TreeEntry, TreeInstruction, TreeLogEntry, TreeRangeDigest, }; use zksync_types::{AccountTreeId, Address, StorageKey, H256, U256}; -use crate::common::{compute_tree_hash, convert_to_writes, generate_key_value_pairs, KVS_AND_HASH}; +use crate::common::{ + compute_tree_hash, convert_to_writes, generate_key_value_pairs, ENTRIES_AND_HASH, +}; #[test] fn compute_tree_hash_works_correctly() { @@ -25,7 +27,7 @@ fn compute_tree_hash_works_correctly() { let address: Address = "4b3af74f66ab1f0da3f2e4ec7a3cb99baf1af7b2".parse().unwrap(); let key = StorageKey::new(AccountTreeId::new(address), H256::zero()); let key = key.hashed_key_u256(); - let hash = compute_tree_hash([(key, H256([1; 32]))].into_iter()); + let hash = compute_tree_hash([TreeEntry::new(key, 1, H256([1; 32]))].into_iter()); assert_eq!(hash, EXPECTED_HASH); } @@ -59,7 +61,7 @@ fn output_proofs_are_computed_correctly_on_empty_tree(kv_count: u64) { let reads = instructions .iter() - .map(|(key, _)| (*key, TreeInstruction::Read)); + .map(|instr| TreeInstruction::Read(instr.key())); let mut reads: Vec<_> = reads.collect(); reads.shuffle(&mut rng); let output = tree.extend_with_proofs(reads.clone()); @@ -77,25 +79,26 @@ fn entry_proofs_are_computed_correctly_on_empty_tree(kv_count: u64) { let expected_hash = compute_tree_hash(kvs.iter().copied()); tree.extend(kvs.clone()); - let existing_keys: Vec<_> = kvs.iter().map(|(key, _)| *key).collect(); + let existing_keys: Vec<_> = kvs.iter().map(|entry| entry.key).collect(); let entries = tree.entries_with_proofs(0, &existing_keys).unwrap(); assert_eq!(entries.len(), existing_keys.len()); - for ((key, value), entry) in kvs.iter().zip(entries) { - entry.verify(&Blake2Hasher, *key, expected_hash); - assert_eq!(entry.base.value_hash, *value); + for (input_entry, entry) in kvs.iter().zip(entries) { + entry.verify(&Blake2Hasher, expected_hash); + assert_eq!(entry.base, *input_entry); } // Test some keys adjacent to existing ones. - let adjacent_keys = kvs.iter().flat_map(|(key, _)| { + let adjacent_keys = kvs.iter().flat_map(|entry| { + let key = entry.key; [ - *key ^ (U256::one() << rng.gen_range(0..256)), - *key ^ (U256::one() << rng.gen_range(0..256)), - *key ^ (U256::one() << rng.gen_range(0..256)), + key ^ (U256::one() << rng.gen_range(0..256)), + key ^ (U256::one() << rng.gen_range(0..256)), + key ^ (U256::one() << rng.gen_range(0..256)), ] }); let random_keys = generate_key_value_pairs(kv_count..(kv_count * 2)) .into_iter() - .map(|(key, _)| key); + .map(|entry| entry.key); let mut missing_keys: Vec<_> = adjacent_keys.chain(random_keys).collect(); missing_keys.shuffle(&mut rng); @@ -103,7 +106,8 @@ fn entry_proofs_are_computed_correctly_on_empty_tree(kv_count: u64) { assert_eq!(entries.len(), missing_keys.len()); for (key, entry) in missing_keys.iter().zip(entries) { assert!(entry.base.is_empty()); - entry.verify(&Blake2Hasher, *key, expected_hash); + assert_eq!(entry.base.key, *key); + entry.verify(&Blake2Hasher, expected_hash); } } @@ -117,10 +121,13 @@ fn proofs_are_computed_correctly_for_mixed_instructions() { let output = tree.extend(kvs.clone()); let old_root_hash = output.root_hash; - let reads = kvs.iter().map(|(key, _)| (*key, TreeInstruction::Read)); + let reads = kvs.iter().map(|entry| TreeInstruction::Read(entry.key)); let mut instructions: Vec<_> = reads.collect(); // Overwrite all keys in the tree. - let writes: Vec<_> = kvs.iter().map(|(key, _)| (*key, H256::zero())).collect(); + let writes: Vec<_> = kvs + .iter() + .map(|entry| entry.with_value(H256::zero())) + .collect(); let expected_hash = compute_tree_hash(writes.iter().copied()); instructions.extend(convert_to_writes(&writes)); instructions.shuffle(&mut rng); @@ -145,7 +152,7 @@ fn proofs_are_computed_correctly_for_missing_keys() { let mut instructions = convert_to_writes(&kvs); let missing_reads = generate_key_value_pairs(20..50) .into_iter() - .map(|(key, _)| (key, TreeInstruction::Read)); + .map(|entry| TreeInstruction::Read(entry.key)); instructions.extend(missing_reads); instructions.shuffle(&mut rng); @@ -161,7 +168,7 @@ fn proofs_are_computed_correctly_for_missing_keys() { } fn test_intermediate_commits(db: &mut impl Database, chunk_size: usize) { - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; let mut final_hash = H256::zero(); let mut tree = MerkleTree::new(db); for chunk in kvs.chunks(chunk_size) { @@ -172,7 +179,7 @@ fn test_intermediate_commits(db: &mut impl Database, chunk_size: usize) { let latest_version = tree.latest_version().unwrap(); for version in 0..=latest_version { - tree.verify_consistency(version).unwrap(); + tree.verify_consistency(version, true).unwrap(); } } @@ -183,7 +190,7 @@ fn root_hash_is_computed_correctly_with_intermediate_commits(chunk_size: usize) #[test_casing(6, [3, 5, 10, 17, 28, 42])] fn output_proofs_are_computed_correctly_with_intermediate_commits(chunk_size: usize) { - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; let mut tree = MerkleTree::new(PatchSet::default()); let mut root_hash = Blake2Hasher.empty_subtree_hash(256); @@ -198,8 +205,8 @@ fn output_proofs_are_computed_correctly_with_intermediate_commits(chunk_size: us #[test_casing(4, [10, 17, 28, 42])] fn entry_proofs_are_computed_correctly_with_intermediate_commits(chunk_size: usize) { - let (kvs, _) = &*KVS_AND_HASH; - let all_keys: Vec<_> = kvs.iter().map(|(key, _)| *key).collect(); + let (kvs, _) = &*ENTRIES_AND_HASH; + let all_keys: Vec<_> = kvs.iter().map(|entry| entry.key).collect(); let mut tree = MerkleTree::new(PatchSet::default()); let mut root_hashes = vec![]; for chunk in kvs.chunks(chunk_size) { @@ -210,8 +217,9 @@ fn entry_proofs_are_computed_correctly_with_intermediate_commits(chunk_size: usi let entries = tree.entries_with_proofs(version as u64, &all_keys).unwrap(); assert_eq!(entries.len(), all_keys.len()); for (i, (key, entry)) in all_keys.iter().zip(entries).enumerate() { + assert_eq!(entry.base.key, *key); assert_eq!(entry.base.is_empty(), i >= (version + 1) * chunk_size); - entry.verify(&Blake2Hasher, *key, output.root_hash); + entry.verify(&Blake2Hasher, output.root_hash); } } @@ -220,14 +228,15 @@ fn entry_proofs_are_computed_correctly_with_intermediate_commits(chunk_size: usi let entries = tree.entries_with_proofs(version as u64, &all_keys).unwrap(); assert_eq!(entries.len(), all_keys.len()); for (i, (key, entry)) in all_keys.iter().zip(entries).enumerate() { + assert_eq!(entry.base.key, *key); assert_eq!(entry.base.is_empty(), i >= (version + 1) * chunk_size); - entry.verify(&Blake2Hasher, *key, root_hash); + entry.verify(&Blake2Hasher, root_hash); } } } fn test_accumulated_commits(db: DB, chunk_size: usize) -> DB { - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; let mut db = Patched::new(db); let mut final_hash = H256::zero(); for chunk in kvs.chunks(chunk_size) { @@ -242,7 +251,7 @@ fn test_accumulated_commits(db: DB, chunk_size: usize) -> DB { let tree = MerkleTree::new(&mut db); let latest_version = tree.latest_version().unwrap(); for version in 0..=latest_version { - tree.verify_consistency(version).unwrap(); + tree.verify_consistency(version, true).unwrap(); } db } @@ -253,9 +262,12 @@ fn accumulating_commits(chunk_size: usize) { } fn test_root_hash_computing_with_reverts(db: &mut impl Database) { - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; let (initial_update, final_update) = kvs.split_at(75); - let key_updates: Vec<_> = kvs.iter().map(|(key, _)| (*key, H256([255; 32]))).collect(); + let key_updates: Vec<_> = kvs + .iter() + .map(|entry| entry.with_value(H256([255; 32]))) + .collect(); let key_inserts = generate_key_value_pairs(100..200); let mut tree = MerkleTree::new(db); @@ -300,7 +312,7 @@ fn test_root_hash_computing_with_key_updates(db: impl Database) { // Overwrite some `kvs` entries and add some new ones. let changed_kvs = kvs.iter_mut().enumerate().filter_map(|(i, kv)| { if i % 3 == 1 { - kv.1 = H256::from_low_u64_be((i + 100) as u64); + *kv = kv.with_value(H256::from_low_u64_be((i + 100) as u64)); return Some(*kv); } None @@ -361,12 +373,12 @@ fn root_hash_is_computed_correctly_with_key_updates() { fn proofs_are_computed_correctly_with_key_updates(updated_keys: usize) { const RNG_SEED: u64 = 1_234; - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; let mut rng = StdRng::seed_from_u64(RNG_SEED); let old_instructions: Vec<_> = kvs[..updated_keys] .iter() - .map(|(key, _)| (*key, TreeInstruction::Write(H256([255; 32])))) + .map(|entry| TreeInstruction::Write(entry.with_value(H256([255; 32])))) .collect(); // Move the updated keys to the random places in the `kvs` vector. let mut writes = convert_to_writes(kvs); @@ -386,11 +398,11 @@ fn proofs_are_computed_correctly_with_key_updates(updated_keys: usize) { assert_eq!(output.root_hash(), Some(*expected_hash)); output.verify_proofs(&Blake2Hasher, root_hash, &instructions); - let keys: Vec<_> = kvs.iter().map(|(key, _)| *key).collect(); + let keys: Vec<_> = kvs.iter().map(|entry| entry.key).collect(); let proofs = tree.entries_with_proofs(1, &keys).unwrap(); - for ((key, value), proof) in kvs.iter().zip(proofs) { - assert_eq!(proof.base.value_hash, *value); - proof.verify(&Blake2Hasher, *key, *expected_hash); + for (entry, proof) in kvs.iter().zip(proofs) { + assert_eq!(proof.base, *entry); + proof.verify(&Blake2Hasher, *expected_hash); } } @@ -417,7 +429,11 @@ fn test_root_hash_equals_to_previous_implementation(db: &mut impl Database) { }) }); let values = (0..100).map(H256::from_low_u64_be); - let kvs: Vec<_> = keys.zip(values).collect(); + let kvs: Vec<_> = keys + .zip(values) + .enumerate() + .map(|(idx, (key, value))| TreeEntry::new(key, idx as u64 + 1, value)) + .collect(); let expected_hash = compute_tree_hash(kvs.iter().copied()); assert_eq!(expected_hash, PREV_IMPL_HASH); @@ -437,13 +453,13 @@ fn root_hash_equals_to_previous_implementation() { #[test_casing(7, [2, 3, 5, 10, 17, 28, 42])] fn range_proofs_with_multiple_existing_items(range_size: usize) { - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; assert!(range_size >= 2 && range_size <= kvs.len()); let mut tree = MerkleTree::new(PatchSet::default()); tree.extend(kvs.clone()); - let mut sorted_keys: Vec<_> = kvs.iter().map(|(key, _)| *key).collect(); + let mut sorted_keys: Vec<_> = kvs.iter().map(|entry| entry.key).collect(); sorted_keys.sort_unstable(); for start_idx in 0..(sorted_keys.len() - range_size) { @@ -460,10 +476,10 @@ fn range_proofs_with_multiple_existing_items(range_size: usize) { let other_entries = tree.entries(0, other_keys).unwrap(); let mut range = TreeRangeDigest::new(&Blake2Hasher, *first_key, &first_entry); - for (key, entry) in other_keys.iter().zip(other_entries) { - range.update(*key, entry); + for entry in other_entries { + range.update(entry); } - let range_hash = range.finalize(*last_key, &last_entry); + let range_hash = range.finalize(&last_entry); assert_eq!(range_hash, *expected_hash); } } @@ -479,7 +495,7 @@ fn range_proofs_with_random_ranges() { const RNG_SEED: u64 = 321; let mut rng = StdRng::seed_from_u64(RNG_SEED); - let (kvs, expected_hash) = &*KVS_AND_HASH; + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; let mut tree = MerkleTree::new(PatchSet::default()); tree.extend(kvs.clone()); @@ -493,9 +509,9 @@ fn range_proofs_with_random_ranges() { } // Find out keys falling into the range. - let keys_in_range = kvs - .iter() - .filter_map(|&(key, _)| (key > start_key && key < end_key).then_some(key)); + let keys_in_range = kvs.iter().filter_map(|entry| { + (entry.key > start_key && entry.key < end_key).then_some(entry.key) + }); let mut keys_in_range: Vec<_> = keys_in_range.collect(); keys_in_range.sort_unstable(); println!("Proving range with {} keys", keys_in_range.len()); @@ -506,10 +522,10 @@ fn range_proofs_with_random_ranges() { let other_entries = tree.entries(0, &keys_in_range).unwrap(); let mut range = TreeRangeDigest::new(&Blake2Hasher, start_key, &first_entry); - for (key, entry) in keys_in_range.iter().zip(other_entries) { - range.update(*key, entry); + for entry in other_entries { + range.update(entry); } - let range_hash = range.finalize(end_key, &last_entry); + let range_hash = range.finalize(&last_entry); assert_eq!(range_hash, *expected_hash); } } @@ -633,7 +649,7 @@ mod rocksdb { fn tree_tags_mismatch() { let Harness { mut db, dir: _dir } = Harness::new(); let mut tree = MerkleTree::new(&mut db); - tree.extend(vec![(U256::zero(), H256::zero())]); + tree.extend(vec![TreeEntry::new(U256::zero(), 1, H256::zero())]); MerkleTree::with_hasher(&mut db, ()); } @@ -643,7 +659,7 @@ mod rocksdb { fn tree_tags_mismatch_with_cold_restart() { let Harness { db, dir } = Harness::new(); let mut tree = MerkleTree::new(db); - tree.extend(vec![(U256::zero(), H256::zero())]); + tree.extend(vec![TreeEntry::new(U256::zero(), 1, H256::zero())]); drop(tree); let db = RocksDBWrapper::new(dir.path()); diff --git a/core/lib/merkle_tree/tests/integration/recovery.rs b/core/lib/merkle_tree/tests/integration/recovery.rs index fda57f788514..6739e4ffe023 100644 --- a/core/lib/merkle_tree/tests/integration/recovery.rs +++ b/core/lib/merkle_tree/tests/integration/recovery.rs @@ -5,11 +5,10 @@ use test_casing::test_casing; use zksync_crypto::hasher::blake2::Blake2Hasher; use zksync_merkle_tree::{ - recovery::{MerkleTreeRecovery, RecoveryEntry}, - Database, MerkleTree, PatchSet, PruneDatabase, ValueHash, + recovery::MerkleTreeRecovery, Database, MerkleTree, PatchSet, PruneDatabase, ValueHash, }; -use crate::common::{convert_to_writes, generate_key_value_pairs, TreeMap, KVS_AND_HASH}; +use crate::common::{convert_to_writes, generate_key_value_pairs, TreeMap, ENTRIES_AND_HASH}; #[derive(Debug, Clone, Copy)] enum RecoveryKind { @@ -23,16 +22,8 @@ impl RecoveryKind { #[test] fn recovery_basics() { - let (kvs, expected_hash) = &*KVS_AND_HASH; - let recovery_entries = kvs - .iter() - .enumerate() - .map(|(i, &(key, value))| RecoveryEntry { - key, - value, - leaf_index: i as u64 + 1, - }); - let mut recovery_entries: Vec<_> = recovery_entries.collect(); + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; + let mut recovery_entries: Vec<_> = kvs.clone(); recovery_entries.sort_unstable_by_key(|entry| entry.key); let greatest_key = recovery_entries[99].key; @@ -44,20 +35,12 @@ fn recovery_basics() { assert_eq!(recovery.root_hash(), *expected_hash); let tree = recovery.finalize(); - tree.verify_consistency(recovered_version).unwrap(); + tree.verify_consistency(recovered_version, true).unwrap(); } fn test_recovery_in_chunks(mut db: impl PruneDatabase, kind: RecoveryKind, chunk_size: usize) { - let (kvs, expected_hash) = &*KVS_AND_HASH; - let recovery_entries = kvs - .iter() - .enumerate() - .map(|(i, &(key, value))| RecoveryEntry { - key, - value, - leaf_index: i as u64 + 1, - }); - let mut recovery_entries: Vec<_> = recovery_entries.collect(); + let (kvs, expected_hash) = &*ENTRIES_AND_HASH; + let mut recovery_entries = kvs.clone(); if matches!(kind, RecoveryKind::Linear) { recovery_entries.sort_unstable_by_key(|entry| entry.key); } @@ -84,7 +67,7 @@ fn test_recovery_in_chunks(mut db: impl PruneDatabase, kind: RecoveryKind, chunk assert_eq!(recovery.root_hash(), *expected_hash); let mut tree = recovery.finalize(); - tree.verify_consistency(recovered_version).unwrap(); + tree.verify_consistency(recovered_version, true).unwrap(); // Check that new tree versions can be built and function as expected. test_tree_after_recovery(&mut tree, recovered_version, *expected_hash); } @@ -107,13 +90,13 @@ fn test_tree_after_recovery( let mut rng = StdRng::seed_from_u64(RNG_SEED); let mut kvs = generate_key_value_pairs(100..=150); let mut modified_kvs = generate_key_value_pairs(50..=100); - for (_, value) in &mut modified_kvs { - *value = ValueHash::repeat_byte(1); + for entry in &mut modified_kvs { + entry.value = ValueHash::repeat_byte(1); } + modified_kvs.shuffle(&mut rng); kvs.extend(modified_kvs); - kvs.shuffle(&mut rng); - let mut tree_map = TreeMap::new(&KVS_AND_HASH.0); + let mut tree_map = TreeMap::new(&ENTRIES_AND_HASH.0); let mut prev_root_hash = root_hash; for (i, chunk) in kvs.chunks(CHUNK_SIZE).enumerate() { tree_map.extend(chunk); @@ -129,7 +112,7 @@ fn test_tree_after_recovery( }; assert_eq!(new_root_hash, tree_map.root_hash()); - tree.verify_consistency(recovered_version + i as u64) + tree.verify_consistency(recovered_version + i as u64, true) .unwrap(); prev_root_hash = new_root_hash; } diff --git a/core/lib/zksync_core/src/api_server/tree/mod.rs b/core/lib/zksync_core/src/api_server/tree/mod.rs index 74dd3e5b70c1..7b4c9086ac68 100644 --- a/core/lib/zksync_core/src/api_server/tree/mod.rs +++ b/core/lib/zksync_core/src/api_server/tree/mod.rs @@ -54,7 +54,7 @@ impl TreeEntryWithProof { let mut merkle_path = src.merkle_path; merkle_path.reverse(); // Use root-to-leaf enumeration direction as in Ethereum Self { - value: src.base.value_hash, + value: src.base.value, index: src.base.leaf_index, merkle_path, } diff --git a/core/lib/zksync_core/src/metadata_calculator/helpers.rs b/core/lib/zksync_core/src/metadata_calculator/helpers.rs index 32f39276a1e5..9ae936febfe6 100644 --- a/core/lib/zksync_core/src/metadata_calculator/helpers.rs +++ b/core/lib/zksync_core/src/metadata_calculator/helpers.rs @@ -16,10 +16,10 @@ use zksync_dal::StorageProcessor; use zksync_health_check::{Health, HealthStatus}; use zksync_merkle_tree::{ domain::{TreeMetadata, ZkSyncTree, ZkSyncTreeReader}, - Key, MerkleTreeColumnFamily, NoVersionError, TreeEntryWithProof, + Key, MerkleTreeColumnFamily, NoVersionError, TreeEntryWithProof, TreeInstruction, }; use zksync_storage::{RocksDB, RocksDBOptions, StalledWritesRetries}; -use zksync_types::{block::L1BatchHeader, L1BatchNumber, StorageLog, H256}; +use zksync_types::{block::L1BatchHeader, L1BatchNumber, StorageKey, H256}; use super::metrics::{LoadChangesStage, TreeUpdateStage, METRICS}; @@ -147,7 +147,10 @@ impl AsyncTree { self.as_ref().root_hash() } - pub async fn process_l1_batch(&mut self, storage_logs: Vec) -> TreeMetadata { + pub async fn process_l1_batch( + &mut self, + storage_logs: Vec>, + ) -> TreeMetadata { let mut tree = self.inner.take().expect(Self::INCONSISTENT_MSG); let (tree, metadata) = tokio::task::spawn_blocking(move || { let metadata = tree.process_l1_batch(&storage_logs); @@ -242,7 +245,7 @@ impl Delayer { #[cfg_attr(test, derive(PartialEq))] pub(crate) struct L1BatchWithLogs { pub header: L1BatchHeader, - pub storage_logs: Vec, + pub storage_logs: Vec>, } impl L1BatchWithLogs { @@ -276,15 +279,22 @@ impl L1BatchWithLogs { .await; touched_slots_latency.observe_with_count(touched_slots.len()); + let leaf_indices_latency = METRICS.start_load_stage(LoadChangesStage::LoadLeafIndices); + let hashed_keys_for_writes: Vec<_> = + touched_slots.keys().map(StorageKey::hashed_key).collect(); + let l1_batches_for_initial_writes = storage + .storage_logs_dal() + .get_l1_batches_and_indices_for_initial_writes(&hashed_keys_for_writes) + .await; + leaf_indices_latency.observe_with_count(hashed_keys_for_writes.len()); + let mut storage_logs = BTreeMap::new(); for storage_key in protective_reads { touched_slots.remove(&storage_key); // ^ As per deduplication rules, all keys in `protective_reads` haven't *really* changed // in the considered L1 batch. Thus, we can remove them from `touched_slots` in order to simplify // their further processing. - - let log = StorageLog::new_read_log(storage_key, H256::zero()); - // ^ The tree doesn't use the read value, so we set it to zero. + let log = TreeInstruction::Read(storage_key); storage_logs.insert(storage_key, log); } tracing::debug!( @@ -292,45 +302,17 @@ impl L1BatchWithLogs { touched_slots.len() ); - // We don't want to update the tree with zero values which were never written to per storage log - // deduplication rules. If we write such values to the tree, it'd result in bogus tree hashes because - // new (bogus) leaf indices would be allocated for them. To filter out those values, it's sufficient - // to check when a `storage_key` was first written per `initial_writes` table. If this never occurred - // or occurred after the considered `l1_batch_number`, this means that the write must be ignored. - // - // Note that this approach doesn't filter out no-op writes of the same value, but this is fine; - // since no new leaf indices are allocated in the tree for them, such writes are no-op on the tree side as well. - let hashed_keys_for_zero_values: Vec<_> = touched_slots - .iter() - .filter(|(_, value)| { - // Only zero values are worth checking for initial writes; non-zero values are always - // written per deduplication rules. - value.is_zero() - }) - .map(|(key, _)| key.hashed_key()) - .collect(); - METRICS - .load_changes_zero_values - .observe(hashed_keys_for_zero_values.len()); - - let latency = METRICS.start_load_stage(LoadChangesStage::LoadInitialWritesForZeroValues); - let l1_batches_for_initial_writes = storage - .storage_logs_dal() - .get_l1_batches_and_indices_for_initial_writes(&hashed_keys_for_zero_values) - .await; - latency.observe_with_count(hashed_keys_for_zero_values.len()); - for (storage_key, value) in touched_slots { - let write_matters = if value.is_zero() { - let initial_write_batch_for_key = - l1_batches_for_initial_writes.get(&storage_key.hashed_key()); - initial_write_batch_for_key.map_or(false, |&(number, _)| number <= l1_batch_number) - } else { - true - }; - - if write_matters { - storage_logs.insert(storage_key, StorageLog::new_write_log(storage_key, value)); + if let Some(&(initial_write_batch_for_key, leaf_index)) = + l1_batches_for_initial_writes.get(&storage_key.hashed_key()) + { + // Filter out logs that correspond to deduplicated writes. + if initial_write_batch_for_key <= l1_batch_number { + storage_logs.insert( + storage_key, + TreeInstruction::write(storage_key, leaf_index, value), + ); + } } } @@ -347,7 +329,7 @@ mod tests { use tempfile::TempDir; use zksync_dal::ConnectionPool; - use zksync_types::{proofs::PrepareBasicCircuitsJob, L2ChainId, StorageKey, StorageLogKind}; + use zksync_types::{proofs::PrepareBasicCircuitsJob, L2ChainId, StorageKey, StorageLog}; use super::*; use crate::{ @@ -386,6 +368,10 @@ mod tests { .storage_logs_dal() .get_previous_storage_values(&hashed_keys, l1_batch_number) .await; + let l1_batches_for_initial_writes = storage + .storage_logs_dal() + .get_l1_batches_and_indices_for_initial_writes(&hashed_keys) + .await; for storage_key in protective_reads { let previous_value = previous_values[&storage_key.hashed_key()].unwrap_or_default(); @@ -397,16 +383,17 @@ mod tests { ); } - storage_logs.insert( - storage_key, - StorageLog::new_read_log(storage_key, previous_value), - ); + storage_logs.insert(storage_key, TreeInstruction::Read(storage_key)); } for (storage_key, value) in touched_slots { let previous_value = previous_values[&storage_key.hashed_key()].unwrap_or_default(); if previous_value != value { - storage_logs.insert(storage_key, StorageLog::new_write_log(storage_key, value)); + let (_, leaf_index) = l1_batches_for_initial_writes[&storage_key.hashed_key()]; + storage_logs.insert( + storage_key, + TreeInstruction::write(storage_key, leaf_index, value), + ); } } @@ -608,7 +595,7 @@ mod tests { let read_logs_count = l1_batch_with_logs .storage_logs .iter() - .filter(|log| log.kind == StorageLogKind::Read) + .filter(|log| matches!(log, TreeInstruction::Read(_))) .count(); assert_eq!(read_logs_count, 7); diff --git a/core/lib/zksync_core/src/metadata_calculator/metrics.rs b/core/lib/zksync_core/src/metadata_calculator/metrics.rs index f2bedf47229d..f8ef8f85b641 100644 --- a/core/lib/zksync_core/src/metadata_calculator/metrics.rs +++ b/core/lib/zksync_core/src/metadata_calculator/metrics.rs @@ -35,7 +35,7 @@ pub(super) enum LoadChangesStage { LoadL1BatchHeader, LoadProtectiveReads, LoadTouchedSlots, - LoadInitialWritesForZeroValues, + LoadLeafIndices, } /// Latency metric for a certain stage of the tree update. From 15d7eaf872e222338810243865cec9dff7f6e799 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 30 Nov 2023 17:17:05 +0200 Subject: [PATCH 15/18] feat(en): Support arbitrary genesis block for external nodes (#537) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Support non-zero genesis block specified in executor configuration. Check whether this block exists on initialization; validate its correspondence if it does, and persist consensus fields if it doesn't. ## Why ❔ This is necessary to support gossip-based syncing in practice; we likely won't back-sign all blocks in all envs. ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. --- core/lib/types/src/block.rs | 10 +- core/lib/zksync_core/src/consensus/payload.rs | 2 +- .../zksync_core/src/sync_layer/external_io.rs | 31 ++- .../lib/zksync_core/src/sync_layer/fetcher.rs | 16 +- .../src/sync_layer/gossip/buffered/tests.rs | 2 + .../src/sync_layer/gossip/conversions.rs | 1 - .../zksync_core/src/sync_layer/gossip/mod.rs | 4 +- .../src/sync_layer/gossip/storage/mod.rs | 165 ++++++++++-- .../src/sync_layer/gossip/storage/tests.rs | 224 +++++++++++++++- .../src/sync_layer/gossip/tests.rs | 249 +++++++++++++++--- .../zksync_core/src/sync_layer/sync_action.rs | 4 +- core/lib/zksync_core/src/sync_layer/tests.rs | 13 +- 12 files changed, 611 insertions(+), 110 deletions(-) diff --git a/core/lib/types/src/block.rs b/core/lib/types/src/block.rs index 80a4d131e21b..b40264688684 100644 --- a/core/lib/types/src/block.rs +++ b/core/lib/types/src/block.rs @@ -89,7 +89,7 @@ pub struct MiniblockHeader { } /// Consensus-related L2 block (= miniblock) fields. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct ConsensusBlockFields { /// Hash of the previous consensus block. pub parent: validator::BlockHeaderHash, @@ -99,12 +99,14 @@ pub struct ConsensusBlockFields { impl ProtoFmt for ConsensusBlockFields { type Proto = crate::proto::ConsensusBlockFields; - fn read(r: &Self::Proto) -> anyhow::Result { + + fn read(proto: &Self::Proto) -> anyhow::Result { Ok(Self { - parent: read_required(&r.parent).context("parent")?, - justification: read_required(&r.justification).context("justification")?, + parent: read_required(&proto.parent).context("parent")?, + justification: read_required(&proto.justification).context("justification")?, }) } + fn build(&self) -> Self::Proto { Self::Proto { parent: Some(self.parent.build()), diff --git a/core/lib/zksync_core/src/consensus/payload.rs b/core/lib/zksync_core/src/consensus/payload.rs index 8d53fdf21f31..dbe276196b0f 100644 --- a/core/lib/zksync_core/src/consensus/payload.rs +++ b/core/lib/zksync_core/src/consensus/payload.rs @@ -6,7 +6,7 @@ use zksync_types::api::en::SyncBlock; use zksync_types::{Address, L1BatchNumber, Transaction, H256}; /// L2 block (= miniblock) payload. -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub(crate) struct Payload { pub hash: H256, pub l1_batch_number: L1BatchNumber, diff --git a/core/lib/zksync_core/src/sync_layer/external_io.rs b/core/lib/zksync_core/src/sync_layer/external_io.rs index dcc38334a998..8e3ca863072a 100644 --- a/core/lib/zksync_core/src/sync_layer/external_io.rs +++ b/core/lib/zksync_core/src/sync_layer/external_io.rs @@ -1,4 +1,5 @@ use async_trait::async_trait; +use futures::future; use std::{ collections::HashMap, @@ -108,7 +109,6 @@ impl ExternalIO { async fn load_previous_l1_batch_hash(&self) -> U256 { let mut storage = self.pool.access_storage_tagged("sync_layer").await.unwrap(); - let wait_latency = KEEPER_METRICS.wait_for_prev_hash_time.start(); let (hash, _) = extractors::wait_for_prev_l1_batch_params(&mut storage, self.current_l1_batch_number) @@ -117,6 +117,18 @@ impl ExternalIO { hash } + async fn load_previous_miniblock_hash(&self) -> H256 { + let prev_miniblock_number = self.current_miniblock_number - 1; + let mut storage = self.pool.access_storage_tagged("sync_layer").await.unwrap(); + let header = storage + .blocks_dal() + .get_miniblock_header(prev_miniblock_number) + .await + .unwrap() + .unwrap_or_else(|| panic!("Miniblock #{prev_miniblock_number} is missing")); + header.hash + } + async fn load_base_system_contracts_by_version_id( &self, id: ProtocolVersionId, @@ -307,15 +319,20 @@ impl StateKeeperIO for ExternalIO { operator_address, protocol_version, first_miniblock_info: (miniblock_number, virtual_blocks), - prev_miniblock_hash, }) => { assert_eq!( number, self.current_l1_batch_number, "Batch number mismatch" ); - tracing::info!("Getting previous L1 batch hash"); - let previous_l1_batch_hash = self.load_previous_l1_batch_hash().await; - tracing::info!("Previous L1 batch hash: {previous_l1_batch_hash}"); + tracing::info!("Getting previous L1 batch hash and miniblock hash"); + let (previous_l1_batch_hash, previous_miniblock_hash) = future::join( + self.load_previous_l1_batch_hash(), + self.load_previous_miniblock_hash(), + ) + .await; + tracing::info!( + "Previous L1 batch hash: {previous_l1_batch_hash}, previous miniblock hash: {previous_miniblock_hash}" + ); let base_system_contracts = self .load_base_system_contracts_by_version_id(protocol_version) @@ -328,7 +345,7 @@ impl StateKeeperIO for ExternalIO { l1_gas_price, l2_fair_gas_price, miniblock_number, - prev_miniblock_hash, + previous_miniblock_hash, base_system_contracts, self.validation_computational_gas_limit, protocol_version, @@ -539,6 +556,8 @@ impl StateKeeperIO for ExternalIO { // Mimic the metric emitted by the main node to reuse existing Grafana charts. APP_METRICS.block_number[&BlockStage::Sealed].set(self.current_l1_batch_number.0.into()); + self.sync_state + .set_local_block(self.current_miniblock_number); self.current_miniblock_number += 1; // Due to fictive miniblock being sealed. self.current_l1_batch_number += 1; Ok(()) diff --git a/core/lib/zksync_core/src/sync_layer/fetcher.rs b/core/lib/zksync_core/src/sync_layer/fetcher.rs index 4aabd163f21e..9cdd7e64fd1e 100644 --- a/core/lib/zksync_core/src/sync_layer/fetcher.rs +++ b/core/lib/zksync_core/src/sync_layer/fetcher.rs @@ -6,7 +6,7 @@ use std::time::Duration; use zksync_dal::StorageProcessor; use zksync_types::{ api::en::SyncBlock, block::ConsensusBlockFields, Address, L1BatchNumber, MiniblockNumber, - ProtocolVersionId, H256, + ProtocolVersionId, }; use zksync_web3_decl::jsonrpsee::core::Error as RpcError; @@ -29,7 +29,6 @@ pub(super) struct FetchedBlock { pub last_in_batch: bool, pub protocol_version: ProtocolVersionId, pub timestamp: u64, - pub hash: H256, pub l1_gas_price: u64, pub l2_fair_gas_price: u64, pub virtual_blocks: u32, @@ -38,15 +37,14 @@ pub(super) struct FetchedBlock { pub consensus: Option, } -impl FetchedBlock { - fn from_sync_block(block: SyncBlock) -> Self { +impl From for FetchedBlock { + fn from(block: SyncBlock) -> Self { Self { number: block.number, l1_batch_number: block.l1_batch_number, last_in_batch: block.last_in_batch, protocol_version: block.protocol_version, timestamp: block.timestamp, - hash: block.hash.unwrap_or_default(), l1_gas_price: block.l1_gas_price, l2_fair_gas_price: block.l2_fair_gas_price, virtual_blocks: block.virtual_blocks.unwrap_or(0), @@ -64,7 +62,6 @@ impl FetchedBlock { pub struct FetcherCursor { // Fields are public for testing purposes. pub(super) next_miniblock: MiniblockNumber, - pub(super) prev_miniblock_hash: H256, pub(super) l1_batch: L1BatchNumber, } @@ -93,7 +90,6 @@ impl FetcherCursor { // Miniblocks are always fully processed. let next_miniblock = last_miniblock_header.number + 1; - let prev_miniblock_hash = last_miniblock_header.hash; // Decide whether the next batch should be explicitly opened or not. let l1_batch = if was_new_batch_open { // No `OpenBatch` action needed. @@ -106,7 +102,6 @@ impl FetcherCursor { Ok(Self { next_miniblock, l1_batch, - prev_miniblock_hash, }) } @@ -136,7 +131,6 @@ impl FetcherCursor { protocol_version: block.protocol_version, // `block.virtual_blocks` can be `None` only for old VM versions where it's not used, so it's fine to provide any number. first_miniblock_info: (block.number, block.virtual_blocks), - prev_miniblock_hash: self.prev_miniblock_hash, }); FETCHER_METRICS.l1_batch[&L1BatchStage::Open].set(block.l1_batch_number.0.into()); self.l1_batch += 1; @@ -168,7 +162,6 @@ impl FetcherCursor { new_actions.push(SyncAction::SealMiniblock(block.consensus)); } self.next_miniblock += 1; - self.prev_miniblock_hash = block.hash; new_actions } @@ -280,8 +273,7 @@ impl MainNodeFetcher { request_latency.observe(); let block_number = block.number; - let fetched_block = FetchedBlock::from_sync_block(block); - let new_actions = self.cursor.advance(fetched_block); + let new_actions = self.cursor.advance(block.into()); tracing::info!( "New miniblock: {block_number} / {}", diff --git a/core/lib/zksync_core/src/sync_layer/gossip/buffered/tests.rs b/core/lib/zksync_core/src/sync_layer/gossip/buffered/tests.rs index de5ef8a88cb0..62c81bca7ca6 100644 --- a/core/lib/zksync_core/src/sync_layer/gossip/buffered/tests.rs +++ b/core/lib/zksync_core/src/sync_layer/gossip/buffered/tests.rs @@ -11,6 +11,7 @@ use zksync_concurrency::{ ctx::{self, channel}, scope, sync::{self, watch}, + testonly::abort_on_panic, time, }; use zksync_consensus_roles::validator::{BlockHeader, BlockNumber, FinalBlock, Payload}; @@ -131,6 +132,7 @@ async fn test_buffered_storage( block_interval: time::Duration, shuffle_blocks: impl FnOnce(&mut StdRng, &mut [FinalBlock]), ) { + abort_on_panic(); let ctx = &ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); diff --git a/core/lib/zksync_core/src/sync_layer/gossip/conversions.rs b/core/lib/zksync_core/src/sync_layer/gossip/conversions.rs index 8face4e69426..410c2bfe2046 100644 --- a/core/lib/zksync_core/src/sync_layer/gossip/conversions.rs +++ b/core/lib/zksync_core/src/sync_layer/gossip/conversions.rs @@ -42,7 +42,6 @@ impl FetchedBlock { last_in_batch, protocol_version: ProtocolVersionId::latest(), // FIXME timestamp: payload.timestamp, - hash: payload.hash, l1_gas_price: payload.l1_gas_price, l2_fair_gas_price: payload.l2_fair_gas_price, virtual_blocks: payload.virtual_blocks, diff --git a/core/lib/zksync_core/src/sync_layer/gossip/mod.rs b/core/lib/zksync_core/src/sync_layer/gossip/mod.rs index 630ded953453..2fd9f46aabb5 100644 --- a/core/lib/zksync_core/src/sync_layer/gossip/mod.rs +++ b/core/lib/zksync_core/src/sync_layer/gossip/mod.rs @@ -67,7 +67,9 @@ async fn run_gossip_fetcher_inner( let cursor = FetcherCursor::new(&mut storage).await?; drop(storage); - let store = PostgresBlockStorage::new(pool, actions, cursor); + let store = + PostgresBlockStorage::new(ctx, pool, actions, cursor, &executor_config.genesis_block) + .await?; let buffered = Arc::new(Buffered::new(store)); let store = buffered.inner(); let executor = Executor::new(executor_config, node_key, buffered.clone()) diff --git a/core/lib/zksync_core/src/sync_layer/gossip/storage/mod.rs b/core/lib/zksync_core/src/sync_layer/gossip/storage/mod.rs index d4e95c9e2d46..a490147512e4 100644 --- a/core/lib/zksync_core/src/sync_layer/gossip/storage/mod.rs +++ b/core/lib/zksync_core/src/sync_layer/gossip/storage/mod.rs @@ -13,15 +13,18 @@ use zksync_concurrency::{ use zksync_consensus_roles::validator::{BlockNumber, FinalBlock}; use zksync_consensus_storage::{BlockStore, StorageError, StorageResult}; use zksync_dal::{ConnectionPool, StorageProcessor}; -use zksync_types::{Address, MiniblockNumber}; +use zksync_types::{api::en::SyncBlock, block::ConsensusBlockFields, Address, MiniblockNumber}; #[cfg(test)] mod tests; use super::{buffered::ContiguousBlockStore, conversions::sync_block_to_consensus_block}; -use crate::sync_layer::{ - fetcher::{FetchedBlock, FetcherCursor}, - sync_action::{ActionQueueSender, SyncAction}, +use crate::{ + consensus, + sync_layer::{ + fetcher::{FetchedBlock, FetcherCursor}, + sync_action::{ActionQueueSender, SyncAction}, + }, }; #[derive(Debug)] @@ -67,6 +70,7 @@ impl CursorWithCachedBlock { #[derive(Debug)] pub(super) struct PostgresBlockStorage { pool: ConnectionPool, + first_block_number: MiniblockNumber, actions: ActionQueueSender, block_sender: watch::Sender, cursor: Mutex, @@ -74,16 +78,109 @@ pub(super) struct PostgresBlockStorage { impl PostgresBlockStorage { /// Creates a new storage handle. `pool` should have multiple connections to work efficiently. - pub fn new(pool: ConnectionPool, actions: ActionQueueSender, cursor: FetcherCursor) -> Self { + pub async fn new( + ctx: &ctx::Ctx, + pool: ConnectionPool, + actions: ActionQueueSender, + cursor: FetcherCursor, + genesis_block: &FinalBlock, + ) -> StorageResult { + let mut storage = ctx + .wait(pool.access_storage_tagged("sync_layer")) + .await? + .map_err(StorageError::Database)?; + Self::ensure_genesis_block(ctx, &mut storage, genesis_block).await?; + drop(storage); + + let first_block_number = u32::try_from(genesis_block.header.number.0) + .context("Block number overflow for genesis block") + .map_err(StorageError::Database)?; + let first_block_number = MiniblockNumber(first_block_number); + + Ok(Self::new_unchecked( + pool, + first_block_number, + actions, + cursor, + )) + } + + fn new_unchecked( + pool: ConnectionPool, + first_block_number: MiniblockNumber, + actions: ActionQueueSender, + cursor: FetcherCursor, + ) -> Self { let current_block_number = cursor.next_miniblock.0.saturating_sub(1).into(); Self { pool, + first_block_number, actions, block_sender: watch::channel(BlockNumber(current_block_number)).0, cursor: Mutex::new(cursor.into()), } } + async fn ensure_genesis_block( + ctx: &ctx::Ctx, + storage: &mut StorageProcessor<'_>, + genesis_block: &FinalBlock, + ) -> StorageResult<()> { + let block_number = u32::try_from(genesis_block.header.number.0) + .context("Block number overflow for genesis block") + .map_err(StorageError::Database)?; + let block = Self::sync_block(ctx, storage, MiniblockNumber(block_number)).await?; + let block = block + .with_context(|| { + format!("Genesis block #{block_number} (first block with consensus data) is not present in Postgres") + }) + .map_err(StorageError::Database)?; + let actual_consensus_fields = block.consensus.clone(); + + // Some of the following checks are duplicated in `Executor` initialization, but it's necessary + // to run them if the genesis consensus block is not present locally. + let expected_payload = consensus::Payload::decode(&genesis_block.payload) + .context("Cannot decode genesis block payload") + .map_err(StorageError::Database)?; + let actual_payload: consensus::Payload = + block.try_into().map_err(StorageError::Database)?; + if actual_payload != expected_payload { + let err = anyhow::anyhow!( + "Genesis block payload from Postgres {actual_payload:?} does not match the configured one \ + {expected_payload:?}" + ); + return Err(StorageError::Database(err)); + } + + let expected_consensus_fields = ConsensusBlockFields { + parent: genesis_block.header.parent, + justification: genesis_block.justification.clone(), + }; + if let Some(actual_consensus_fields) = &actual_consensus_fields { + // While justifications may differ among nodes for an arbitrary block, we assume that + // the genesis block has a hardcoded justification. + if *actual_consensus_fields != expected_consensus_fields { + let err = anyhow::anyhow!( + "Genesis block consensus fields in Postgres {actual_consensus_fields:?} do not match \ + the configured ones {expected_consensus_fields:?}" + ); + return Err(StorageError::Database(err)); + } + } else { + tracing::info!( + "Postgres doesn't have consensus fields for genesis block; saving {expected_consensus_fields:?}" + ); + ctx.wait(storage.blocks_dal().set_miniblock_consensus_fields( + MiniblockNumber(block_number), + &expected_consensus_fields, + )) + .await? + .context("Failed saving consensus fields for genesis block") + .map_err(StorageError::Database)?; + } + Ok(()) + } + /// Runs background tasks for this store. This method **must** be spawned as a background task /// which should be running as long at the [`PostgresBlockStorage`] is in use; otherwise, /// it will function incorrectly. @@ -116,22 +213,28 @@ impl PostgresBlockStorage { .map_err(StorageError::Database) } + async fn sync_block( + ctx: &ctx::Ctx, + storage: &mut StorageProcessor<'_>, + number: MiniblockNumber, + ) -> StorageResult> { + let operator_address = Address::default(); // FIXME: where to get this address from? + ctx.wait( + storage + .sync_dal() + .sync_block(number, operator_address, true), + ) + .await? + .with_context(|| format!("Failed getting miniblock #{number} from Postgres")) + .map_err(StorageError::Database) + } + async fn block( ctx: &ctx::Ctx, storage: &mut StorageProcessor<'_>, number: MiniblockNumber, ) -> StorageResult> { - let operator_address = Address::default(); // FIXME: where to get this address from? - let Some(block) = ctx - .wait( - storage - .sync_dal() - .sync_block(number, operator_address, true), - ) - .await? - .with_context(|| format!("Failed getting miniblock #{number} from Postgres")) - .map_err(StorageError::Database)? - else { + let Some(block) = Self::sync_block(ctx, storage, number).await? else { return Ok(None); }; let block = sync_block_to_consensus_block(block).map_err(StorageError::Database)?; @@ -167,7 +270,7 @@ impl BlockStore for PostgresBlockStorage { async fn first_block(&self, ctx: &ctx::Ctx) -> StorageResult { let mut storage = self.storage(ctx).await?; - Self::block(ctx, &mut storage, MiniblockNumber(0)) + Self::block(ctx, &mut storage, self.first_block_number) .await? .context("Genesis miniblock not present in Postgres") .map_err(StorageError::Database) @@ -182,19 +285,33 @@ impl BlockStore for PostgresBlockStorage { ctx: &ctx::Ctx, number: BlockNumber, ) -> StorageResult> { - let number = u32::try_from(number.0) - .context("block number is too large") - .map_err(StorageError::Database)?; + let Ok(number) = u32::try_from(number.0) else { + return Ok(None); + }; + let number = MiniblockNumber(number); + if number < self.first_block_number { + return Ok(None); + } let mut storage = self.storage(ctx).await?; - Self::block(ctx, &mut storage, MiniblockNumber(number)).await + Self::block(ctx, &mut storage, number).await } async fn missing_block_numbers( &self, - _ctx: &ctx::Ctx, - _range: ops::Range, + ctx: &ctx::Ctx, + range: ops::Range, ) -> StorageResult> { - Ok(vec![]) // The storage never has missing blocks by construction + let mut output = vec![]; + let first_block_number = u64::from(self.first_block_number.0); + let numbers_before_first_block = (range.start.0..first_block_number).map(BlockNumber); + output.extend(numbers_before_first_block); + + let last_block_number = self.sealed_miniblock_number(ctx).await?; + let numbers_after_last_block = (last_block_number.next().0..range.end.0).map(BlockNumber); + output.extend(numbers_after_last_block); + + // By design, no blocks are missing in the `first_block_number..=last_block_number` range. + Ok(output) } fn subscribe_to_block_writes(&self) -> watch::Receiver { diff --git a/core/lib/zksync_core/src/sync_layer/gossip/storage/tests.rs b/core/lib/zksync_core/src/sync_layer/gossip/storage/tests.rs index 437c51883308..cfd14f784112 100644 --- a/core/lib/zksync_core/src/sync_layer/gossip/storage/tests.rs +++ b/core/lib/zksync_core/src/sync_layer/gossip/storage/tests.rs @@ -2,7 +2,8 @@ use rand::{thread_rng, Rng}; -use zksync_concurrency::scope; +use zksync_concurrency::{scope, testonly::abort_on_panic}; +use zksync_consensus_roles::validator; use zksync_types::L2ChainId; use super::*; @@ -11,7 +12,7 @@ use crate::{ sync_layer::{ gossip::tests::{ add_consensus_fields, assert_first_block_actions, assert_second_block_actions, - load_final_block, + block_payload, create_genesis_block, load_final_block, }, tests::run_state_keeper_with_multiple_miniblocks, ActionQueue, @@ -22,15 +23,21 @@ const TEST_TIMEOUT: time::Duration = time::Duration::seconds(10); #[tokio::test] async fn block_store_basics_for_postgres() { + abort_on_panic(); let pool = ConnectionPool::test_pool().await; run_state_keeper_with_multiple_miniblocks(pool.clone()).await; let mut storage = pool.access_storage().await.unwrap(); - add_consensus_fields(&mut storage, &thread_rng().gen(), 3).await; + add_consensus_fields(&mut storage, &thread_rng().gen(), 0..3).await; let cursor = FetcherCursor::new(&mut storage).await.unwrap(); drop(storage); let (actions_sender, _) = ActionQueue::new(); - let storage = PostgresBlockStorage::new(pool.clone(), actions_sender, cursor); + let storage = PostgresBlockStorage::new_unchecked( + pool.clone(), + MiniblockNumber(0), + actions_sender, + cursor, + ); let ctx = &ctx::test_root(&ctx::RealClock); let genesis_block = BlockStore::first_block(&storage, ctx).await.unwrap(); @@ -52,6 +59,7 @@ async fn block_store_basics_for_postgres() { #[tokio::test] async fn subscribing_to_block_updates_for_postgres() { + abort_on_panic(); let pool = ConnectionPool::test_pool().await; let mut storage = pool.access_storage().await.unwrap(); if storage.blocks_dal().is_genesis_needed().await.unwrap() { @@ -64,7 +72,12 @@ async fn subscribing_to_block_updates_for_postgres() { // `ContiguousBlockStore`), but for testing subscriptions this is fine. drop(storage); let (actions_sender, _) = ActionQueue::new(); - let storage = PostgresBlockStorage::new(pool.clone(), actions_sender, cursor); + let storage = PostgresBlockStorage::new_unchecked( + pool.clone(), + MiniblockNumber(0), + actions_sender, + cursor, + ); let mut subscriber = storage.subscribe_to_block_writes(); let ctx = &ctx::test_root(&ctx::RealClock); @@ -90,11 +103,12 @@ async fn subscribing_to_block_updates_for_postgres() { #[tokio::test] async fn processing_new_blocks() { + abort_on_panic(); let pool = ConnectionPool::test_pool().await; run_state_keeper_with_multiple_miniblocks(pool.clone()).await; let mut storage = pool.access_storage().await.unwrap(); - add_consensus_fields(&mut storage, &thread_rng().gen(), 3).await; + add_consensus_fields(&mut storage, &thread_rng().gen(), 0..3).await; let first_block = load_final_block(&mut storage, 1).await; let second_block = load_final_block(&mut storage, 2).await; storage @@ -110,7 +124,12 @@ async fn processing_new_blocks() { drop(storage); let (actions_sender, mut actions) = ActionQueue::new(); - let storage = PostgresBlockStorage::new(pool.clone(), actions_sender, cursor); + let storage = PostgresBlockStorage::new_unchecked( + pool.clone(), + MiniblockNumber(0), + actions_sender, + cursor, + ); let ctx = &ctx::test_root(&ctx::RealClock); let ctx = &ctx.with_timeout(TEST_TIMEOUT); storage @@ -125,3 +144,194 @@ async fn processing_new_blocks() { .unwrap(); assert_second_block_actions(&mut actions).await; } + +#[tokio::test] +async fn ensuring_consensus_fields_for_genesis_block() { + abort_on_panic(); + let ctx = &ctx::test_root(&ctx::RealClock); + let pool = ConnectionPool::test_pool().await; + let mut storage = pool.access_storage().await.unwrap(); + if storage.blocks_dal().is_genesis_needed().await.unwrap() { + ensure_genesis_state(&mut storage, L2ChainId::default(), &GenesisParams::mock()) + .await + .unwrap(); + } + let cursor = FetcherCursor::new(&mut storage).await.unwrap(); + let block_payload = block_payload(&mut storage, 0).await.encode(); + drop(storage); + + let validator_key = validator::SecretKey::generate(&mut ctx.rng()); + let genesis_block = create_genesis_block(&validator_key, 0, block_payload.clone()); + + let (actions_sender, _) = ActionQueue::new(); + PostgresBlockStorage::new(ctx, pool.clone(), actions_sender, cursor, &genesis_block) + .await + .unwrap(); + + // Check that the consensus fields are persisted for the genesis block. + let mut storage = pool.access_storage().await.unwrap(); + let sync_block = storage + .sync_dal() + .sync_block(MiniblockNumber(0), Address::default(), false) + .await + .unwrap() + .expect("No genesis block"); + assert!(sync_block.consensus.is_some()); + let cursor = FetcherCursor::new(&mut storage).await.unwrap(); + let other_cursor = FetcherCursor::new(&mut storage).await.unwrap(); + drop(storage); + + // Check that the storage can be initialized again. + let (actions_sender, _) = ActionQueue::new(); + PostgresBlockStorage::new(ctx, pool.clone(), actions_sender, cursor, &genesis_block) + .await + .unwrap(); + + // Create a genesis block with another validator. + let validator_key = validator::SecretKey::generate(&mut ctx.rng()); + let other_genesis_block = create_genesis_block(&validator_key, 0, block_payload); + + // Storage should not be able to initialize with other genesis block. + let (actions_sender, _) = ActionQueue::new(); + PostgresBlockStorage::new( + ctx, + pool, + actions_sender, + other_cursor, + &other_genesis_block, + ) + .await + .unwrap_err(); +} + +#[tokio::test] +async fn genesis_block_payload_mismatch() { + abort_on_panic(); + let ctx = &ctx::test_root(&ctx::RealClock); + let pool = ConnectionPool::test_pool().await; + let mut storage = pool.access_storage().await.unwrap(); + if storage.blocks_dal().is_genesis_needed().await.unwrap() { + ensure_genesis_state(&mut storage, L2ChainId::default(), &GenesisParams::mock()) + .await + .unwrap(); + } + let cursor = FetcherCursor::new(&mut storage).await.unwrap(); + let other_cursor = FetcherCursor::new(&mut storage).await.unwrap(); + + let bogus_block_payload = validator::Payload(vec![]); + let validator_key = validator::SecretKey::generate(&mut ctx.rng()); + let genesis_block = create_genesis_block(&validator_key, 0, bogus_block_payload); + + let (actions_sender, _) = ActionQueue::new(); + PostgresBlockStorage::new(ctx, pool.clone(), actions_sender, cursor, &genesis_block) + .await + .unwrap_err(); + + let mut bogus_block_payload = block_payload(&mut storage, 0).await; + bogus_block_payload.timestamp += 1; + let genesis_block = create_genesis_block(&validator_key, 0, bogus_block_payload.encode()); + + let (actions_sender, _) = ActionQueue::new(); + PostgresBlockStorage::new( + ctx, + pool.clone(), + actions_sender, + other_cursor, + &genesis_block, + ) + .await + .unwrap_err(); +} + +#[tokio::test] +async fn missing_genesis_block() { + abort_on_panic(); + let ctx = &ctx::test_root(&ctx::RealClock); + let pool = ConnectionPool::test_pool().await; + let mut storage = pool.access_storage().await.unwrap(); + if storage.blocks_dal().is_genesis_needed().await.unwrap() { + ensure_genesis_state(&mut storage, L2ChainId::default(), &GenesisParams::mock()) + .await + .unwrap(); + } + let cursor = FetcherCursor::new(&mut storage).await.unwrap(); + let block_payload = block_payload(&mut storage, 0).await.encode(); + drop(storage); + + // Create a genesis block for the (non-existing) block #2. + let validator_key = validator::SecretKey::generate(&mut ctx.rng()); + let genesis_block = create_genesis_block(&validator_key, 2, block_payload.clone()); + + let (actions_sender, _) = ActionQueue::new(); + PostgresBlockStorage::new(ctx, pool, actions_sender, cursor, &genesis_block) + .await + .unwrap_err(); +} + +#[tokio::test] +async fn using_non_zero_genesis_block() { + abort_on_panic(); + let ctx = &ctx::test_root(&ctx::RealClock); + let pool = ConnectionPool::test_pool().await; + run_state_keeper_with_multiple_miniblocks(pool.clone()).await; + + let mut storage = pool.access_storage().await.unwrap(); + let cursor = FetcherCursor::new(&mut storage).await.unwrap(); + let block_payload = block_payload(&mut storage, 2).await.encode(); + drop(storage); + + let validator_key = validator::SecretKey::generate(&mut ctx.rng()); + let genesis_block = create_genesis_block(&validator_key, 2, block_payload.clone()); + + let (actions_sender, _) = ActionQueue::new(); + let store = PostgresBlockStorage::new(ctx, pool, actions_sender, cursor, &genesis_block) + .await + .unwrap(); + + let head_block = store.head_block(ctx).await.unwrap(); + assert_eq!(head_block.header.number, BlockNumber(2)); + assert_eq!( + head_block.header.parent, + validator::BlockHeaderHash::from_bytes([0; 32]) + ); + let first_block = store.first_block(ctx).await.unwrap(); + assert_eq!(first_block.header.number, BlockNumber(2)); + let last_contiguous_block_number = store.last_contiguous_block_number(ctx).await.unwrap(); + assert_eq!(last_contiguous_block_number, BlockNumber(2)); + + let block = store.block(ctx, BlockNumber(2)).await.unwrap(); + assert_eq!(block, Some(head_block)); + for number in [0, 1, 3] { + let missing_block = store.block(ctx, BlockNumber(number)).await.unwrap(); + assert!(missing_block.is_none()); + } + + let missing_blocks = store + .missing_block_numbers(ctx, BlockNumber(0)..BlockNumber(5)) + .await + .unwrap(); + assert_eq!( + missing_blocks, + [ + BlockNumber(0), + BlockNumber(1), + BlockNumber(3), + BlockNumber(4) + ] + ); + let missing_blocks = store + .missing_block_numbers(ctx, BlockNumber(0)..BlockNumber(2)) + .await + .unwrap(); + assert_eq!(missing_blocks, [BlockNumber(0), BlockNumber(1)]); + let missing_blocks = store + .missing_block_numbers(ctx, BlockNumber(2)..BlockNumber(5)) + .await + .unwrap(); + assert_eq!(missing_blocks, [BlockNumber(3), BlockNumber(4)]); + let missing_blocks = store + .missing_block_numbers(ctx, BlockNumber(2)..BlockNumber(3)) + .await + .unwrap(); + assert_eq!(missing_blocks, []); +} diff --git a/core/lib/zksync_core/src/sync_layer/gossip/tests.rs b/core/lib/zksync_core/src/sync_layer/gossip/tests.rs index ca3ce29f4d37..30597189f0b3 100644 --- a/core/lib/zksync_core/src/sync_layer/gossip/tests.rs +++ b/core/lib/zksync_core/src/sync_layer/gossip/tests.rs @@ -3,12 +3,16 @@ use assert_matches::assert_matches; use test_casing::{test_casing, Product}; -use zksync_concurrency::{ctx, scope, time}; +use std::ops; + +use zksync_concurrency::{ctx, scope, testonly::abort_on_panic, time}; use zksync_consensus_executor::testonly::FullValidatorConfig; use zksync_consensus_roles::validator::{self, FinalBlock}; use zksync_consensus_storage::{InMemoryStorage, WriteBlockStore}; use zksync_dal::{ConnectionPool, StorageProcessor}; -use zksync_types::{block::ConsensusBlockFields, Address, L1BatchNumber, MiniblockNumber}; +use zksync_types::{ + api::en::SyncBlock, block::ConsensusBlockFields, Address, L1BatchNumber, MiniblockNumber, H256, +}; use super::*; use crate::{ @@ -26,40 +30,49 @@ use crate::{ const CLOCK_SPEEDUP: i64 = 20; const POLL_INTERVAL: time::Duration = time::Duration::milliseconds(50 * CLOCK_SPEEDUP); +async fn load_sync_block(storage: &mut StorageProcessor<'_>, number: u32) -> SyncBlock { + storage + .sync_dal() + .sync_block(MiniblockNumber(number), Address::default(), true) + .await + .unwrap() + .unwrap_or_else(|| panic!("no sync block #{number}")) +} + /// Loads a block from the storage and converts it to a `FinalBlock`. pub(super) async fn load_final_block( storage: &mut StorageProcessor<'_>, number: u32, ) -> FinalBlock { - let sync_block = storage - .sync_dal() - .sync_block(MiniblockNumber(number), Address::repeat_byte(1), true) - .await - .unwrap() - .unwrap_or_else(|| panic!("no sync block #{number}")); + let sync_block = load_sync_block(storage, number).await; conversions::sync_block_to_consensus_block(sync_block).unwrap() } -pub async fn block_payload(storage: &mut StorageProcessor<'_>, number: u32) -> validator::Payload { - let sync_block = storage - .sync_dal() - .sync_block(MiniblockNumber(number), Address::repeat_byte(1), true) - .await - .unwrap() - .unwrap_or_else(|| panic!("no sync block #{number}")); - consensus::Payload::try_from(sync_block).unwrap().encode() +fn convert_sync_blocks(sync_blocks: Vec) -> Vec { + sync_blocks + .into_iter() + .map(|sync_block| conversions::sync_block_to_consensus_block(sync_block).unwrap()) + .collect() +} + +pub(super) async fn block_payload( + storage: &mut StorageProcessor<'_>, + number: u32, +) -> consensus::Payload { + let sync_block = load_sync_block(storage, number).await; + consensus::Payload::try_from(sync_block).unwrap() } /// Adds consensus information for the specified `count` of miniblocks, starting from the genesis. pub(super) async fn add_consensus_fields( storage: &mut StorageProcessor<'_>, validator_key: &validator::SecretKey, - count: u32, + block_numbers: ops::Range, ) { let mut prev_block_hash = validator::BlockHeaderHash::from_bytes([0; 32]); let validator_set = validator::ValidatorSet::new([validator_key.public()]).unwrap(); - for number in 0..count { - let payload = block_payload(storage, number).await; + for number in block_numbers { + let payload = block_payload(storage, number).await.encode(); let block_header = validator::BlockHeader { parent: prev_block_hash, number: validator::BlockNumber(number.into()), @@ -87,6 +100,33 @@ pub(super) async fn add_consensus_fields( } } +/// Creates a genesis block for the consensus with the specified number / payload authored by a single validator. +pub(super) fn create_genesis_block( + validator_key: &validator::SecretKey, + number: u64, + payload: validator::Payload, +) -> FinalBlock { + let block_header = validator::BlockHeader { + parent: validator::BlockHeaderHash::from_bytes([0; 32]), + number: validator::BlockNumber(number), + payload: payload.hash(), + }; + let validator_set = validator::ValidatorSet::new([validator_key.public()]).unwrap(); + let replica_commit = validator::ReplicaCommit { + protocol_version: validator::CURRENT_VERSION, + view: validator::ViewNumber(number), + proposal: block_header, + }; + let replica_commit = validator_key.sign_msg(replica_commit); + let justification = + validator::CommitQC::from(&[replica_commit], &validator_set).expect("Failed creating QC"); + FinalBlock { + header: block_header, + payload, + justification, + } +} + pub(super) async fn assert_first_block_actions(actions: &mut ActionQueue) -> Vec { let mut received_actions = vec![]; while !matches!(received_actions.last(), Some(SyncAction::SealMiniblock(_))) { @@ -137,20 +177,21 @@ pub(super) async fn assert_second_block_actions(actions: &mut ActionQueue) -> Ve #[test_casing(4, Product(([false, true], [false, true])))] #[tokio::test] async fn syncing_via_gossip_fetcher(delay_first_block: bool, delay_second_block: bool) { - zksync_concurrency::testonly::abort_on_panic(); + abort_on_panic(); let pool = ConnectionPool::test_pool().await; let tx_hashes = run_state_keeper_with_multiple_miniblocks(pool.clone()).await; let mut storage = pool.access_storage().await.unwrap(); - let genesis_block_payload = block_payload(&mut storage, 0).await; + let genesis_block_payload = block_payload(&mut storage, 0).await.encode(); let ctx = &ctx::test_root(&ctx::AffineClock::new(CLOCK_SPEEDUP as f64)); let rng = &mut ctx.rng(); let mut validator = FullValidatorConfig::for_single_validator(rng, genesis_block_payload); let validator_set = validator.node_config.validators.clone(); let external_node = validator.connect_full_node(rng); - let (genesis_block, blocks) = - get_blocks_and_reset_storage(storage, &validator.validator_key).await; + let genesis_block = validator.node_config.genesis_block.clone(); + add_consensus_fields(&mut storage, &validator.validator_key, 0..3).await; + let blocks = convert_sync_blocks(reset_storage(storage).await); let [first_block, second_block] = blocks.as_slice() else { unreachable!("Unexpected blocks in storage: {blocks:?}"); }; @@ -228,21 +269,16 @@ async fn syncing_via_gossip_fetcher(delay_first_block: bool, delay_second_block: } } -async fn get_blocks_and_reset_storage( - mut storage: StorageProcessor<'_>, - validator_key: &validator::SecretKey, -) -> (FinalBlock, Vec) { +/// Returns the removed blocks. +async fn reset_storage(mut storage: StorageProcessor<'_>) -> Vec { let sealed_miniblock_number = storage .blocks_dal() .get_sealed_miniblock_number() .await .unwrap(); - add_consensus_fields(&mut storage, validator_key, sealed_miniblock_number.0 + 1).await; - let genesis_block = load_final_block(&mut storage, 0).await; - - let mut blocks = Vec::with_capacity(sealed_miniblock_number.0 as usize); + let mut blocks = vec![]; for number in 1..=sealed_miniblock_number.0 { - blocks.push(load_final_block(&mut storage, number).await); + blocks.push(load_sync_block(&mut storage, number).await); } storage @@ -259,29 +295,30 @@ async fn get_blocks_and_reset_storage( .delete_l1_batches(L1BatchNumber(0)) .await .unwrap(); - (genesis_block, blocks) + blocks } #[test_casing(4, [3, 2, 1, 0])] #[tokio::test] async fn syncing_via_gossip_fetcher_with_multiple_l1_batches(initial_block_count: usize) { assert!(initial_block_count <= 3); - zksync_concurrency::testonly::abort_on_panic(); + abort_on_panic(); let pool = ConnectionPool::test_pool().await; let tx_hashes = run_state_keeper_with_multiple_l1_batches(pool.clone()).await; let tx_hashes: Vec<_> = tx_hashes.iter().map(Vec::as_slice).collect(); let mut storage = pool.access_storage().await.unwrap(); - let genesis_block_payload = block_payload(&mut storage, 0).await; + let genesis_block_payload = block_payload(&mut storage, 0).await.encode(); let ctx = &ctx::test_root(&ctx::AffineClock::new(CLOCK_SPEEDUP as f64)); let rng = &mut ctx.rng(); let mut validator = FullValidatorConfig::for_single_validator(rng, genesis_block_payload); let validator_set = validator.node_config.validators.clone(); let external_node = validator.connect_full_node(rng); - let (genesis_block, blocks) = - get_blocks_and_reset_storage(storage, &validator.validator_key).await; + let genesis_block = validator.node_config.genesis_block.clone(); + add_consensus_fields(&mut storage, &validator.validator_key, 0..4).await; + let blocks = convert_sync_blocks(reset_storage(storage).await); assert_eq!(blocks.len(), 3); // 2 real + 1 fictive blocks tracing::trace!("Node storage reset"); let (initial_blocks, delayed_blocks) = blocks.split_at(initial_block_count); @@ -309,9 +346,8 @@ async fn syncing_via_gossip_fetcher_with_multiple_l1_batches(initial_block_count Ok(()) }); - let cloned_pool = pool.clone(); s.spawn_bg(async { - mock_l1_batch_hash_computation(cloned_pool, 1).await; + mock_l1_batch_hash_computation(pool.clone(), 1).await; Ok(()) }); s.spawn_bg(run_gossip_fetcher_inner( @@ -337,3 +373,138 @@ async fn syncing_via_gossip_fetcher_with_multiple_l1_batches(initial_block_count block.justification.verify(&validator_set, 1).unwrap(); } } + +#[test_casing(2, [1, 2])] +#[tokio::test] +async fn syncing_from_non_zero_block(first_block_number: u32) { + abort_on_panic(); + let pool = ConnectionPool::test_pool().await; + let tx_hashes = run_state_keeper_with_multiple_l1_batches(pool.clone()).await; + let tx_hashes: Vec<_> = tx_hashes.iter().map(Vec::as_slice).collect(); + + let mut storage = pool.access_storage().await.unwrap(); + let genesis_block_payload = block_payload(&mut storage, first_block_number) + .await + .encode(); + let ctx = &ctx::test_root(&ctx::AffineClock::new(CLOCK_SPEEDUP as f64)); + let rng = &mut ctx.rng(); + let mut validator = + FullValidatorConfig::for_single_validator(rng, genesis_block_payload.clone()); + // Override the genesis block since it has an incorrect block number. + let genesis_block = create_genesis_block( + &validator.validator_key, + first_block_number.into(), + genesis_block_payload, + ); + validator.node_config.genesis_block = genesis_block.clone(); + let validator_set = validator.node_config.validators.clone(); + let external_node = validator.connect_full_node(rng); + + add_consensus_fields( + &mut storage, + &validator.validator_key, + first_block_number..4, + ) + .await; + let mut initial_blocks = reset_storage(storage).await; + let delayed_blocks = initial_blocks.split_off(first_block_number as usize); + assert!(!initial_blocks.is_empty()); + assert!(!delayed_blocks.is_empty()); + let delayed_blocks = convert_sync_blocks(delayed_blocks); + + // Re-insert initial blocks to the storage. This allows to more precisely emulate node syncing + // (e.g., missing L1 batch relation for the latest blocks). + insert_sync_blocks(pool.clone(), initial_blocks, &tx_hashes).await; + tracing::trace!("Re-inserted blocks to node storage"); + + let validator_storage = Arc::new(InMemoryStorage::new(genesis_block)); + let validator = Executor::new( + validator.node_config, + validator.node_key, + validator_storage.clone(), + ) + .unwrap(); + + let tx_hashes = if first_block_number >= 2 { + &tx_hashes[1..] // Skip transactions in L1 batch #1, since they won't be executed + } else { + &tx_hashes + }; + let (actions_sender, actions) = ActionQueue::new(); + let state_keeper = StateKeeperHandles::new(pool.clone(), actions, tx_hashes).await; + scope::run!(ctx, |ctx, s| async { + s.spawn_bg(validator.run(ctx)); + s.spawn_bg(async { + for block in &delayed_blocks { + ctx.sleep(POLL_INTERVAL).await?; + validator_storage.put_block(ctx, block).await?; + } + Ok(()) + }); + + if first_block_number < 2 { + // L1 batch #1 will be sealed during the state keeper operation; we need to emulate + // computing metadata for it. + s.spawn_bg(async { + mock_l1_batch_hash_computation(pool.clone(), 1).await; + Ok(()) + }); + } + + s.spawn_bg(run_gossip_fetcher_inner( + ctx, + pool.clone(), + actions_sender, + external_node.node_config, + external_node.node_key, + )); + + state_keeper + .wait(|state| state.get_local_block() == MiniblockNumber(3)) + .await; + Ok(()) + }) + .await + .unwrap(); + + // Check that received blocks have consensus fields persisted. + let mut storage = pool.access_storage().await.unwrap(); + for number in first_block_number..4 { + let block = load_final_block(&mut storage, number).await; + block.justification.verify(&validator_set, 1).unwrap(); + } +} + +async fn insert_sync_blocks(pool: ConnectionPool, blocks: Vec, tx_hashes: &[&[H256]]) { + let expected_block_number = blocks.last().expect("blocks cannot be empty").number; + let sealed_l1_batches = blocks + .iter() + .filter_map(|block| block.last_in_batch.then_some(block.l1_batch_number)); + let sealed_l1_batches: Vec<_> = sealed_l1_batches.collect(); + + let mut fetcher = FetcherCursor::new(&mut pool.access_storage().await.unwrap()) + .await + .unwrap(); + let (actions_sender, actions) = ActionQueue::new(); + let state_keeper = StateKeeperHandles::new(pool.clone(), actions, tx_hashes).await; + for block in blocks { + let block_actions = fetcher.advance(block.into()); + actions_sender.push_actions(block_actions).await; + } + + let hash_tasks: Vec<_> = sealed_l1_batches + .into_iter() + .map(|l1_batch_number| { + tokio::spawn(mock_l1_batch_hash_computation( + pool.clone(), + l1_batch_number.0, + )) + }) + .collect(); + state_keeper + .wait(|state| state.get_local_block() == expected_block_number) + .await; + for hash_task in hash_tasks { + hash_task.await.unwrap(); + } +} diff --git a/core/lib/zksync_core/src/sync_layer/sync_action.rs b/core/lib/zksync_core/src/sync_layer/sync_action.rs index b4f56999d4fd..b278cb1c98e6 100644 --- a/core/lib/zksync_core/src/sync_layer/sync_action.rs +++ b/core/lib/zksync_core/src/sync_layer/sync_action.rs @@ -2,7 +2,7 @@ use tokio::sync::mpsc; use zksync_types::{ block::ConsensusBlockFields, Address, L1BatchNumber, MiniblockNumber, ProtocolVersionId, - Transaction, H256, + Transaction, }; use super::metrics::QUEUE_METRICS; @@ -137,7 +137,6 @@ pub(crate) enum SyncAction { protocol_version: ProtocolVersionId, // Miniblock number and virtual blocks count. first_miniblock_info: (MiniblockNumber, u32), - prev_miniblock_hash: H256, }, Miniblock { number: MiniblockNumber, @@ -180,7 +179,6 @@ mod tests { operator_address: Default::default(), protocol_version: ProtocolVersionId::latest(), first_miniblock_info: (1.into(), 1), - prev_miniblock_hash: H256::default(), } } diff --git a/core/lib/zksync_core/src/sync_layer/tests.rs b/core/lib/zksync_core/src/sync_layer/tests.rs index 4a337bbf5dc3..20bafc51cf67 100644 --- a/core/lib/zksync_core/src/sync_layer/tests.rs +++ b/core/lib/zksync_core/src/sync_layer/tests.rs @@ -137,7 +137,6 @@ fn open_l1_batch(number: u32, timestamp: u64, first_miniblock_number: u32) -> Sy operator_address: Default::default(), protocol_version: ProtocolVersionId::latest(), first_miniblock_info: (MiniblockNumber(first_miniblock_number), 1), - prev_miniblock_hash: H256::default(), } } @@ -404,7 +403,7 @@ pub(super) async fn mock_l1_batch_hash_computation(pool: ConnectionPool, number: let metadata = create_l1_batch_metadata(number); storage .blocks_dal() - .save_l1_batch_metadata(L1BatchNumber(1), &metadata, H256::zero(), false) + .save_l1_batch_metadata(L1BatchNumber(number), &metadata, H256::zero(), false) .await .unwrap(); break; @@ -574,15 +573,6 @@ async fn fetcher_with_real_server() { // Fill in transactions grouped in multiple miniblocks in the storage. let tx_hashes = run_state_keeper_with_multiple_miniblocks(pool.clone()).await; let mut tx_hashes = VecDeque::from(tx_hashes); - let mut connection = pool.access_storage().await.unwrap(); - let genesis_miniblock_hash = connection - .blocks_dal() - .get_miniblock_header(MiniblockNumber(0)) - .await - .unwrap() - .expect("No genesis miniblock") - .hash; - drop(connection); // Start the API server. let network_config = NetworkConfig::for_tests(); @@ -598,7 +588,6 @@ async fn fetcher_with_real_server() { let client = ::json_rpc(&format!("http://{server_addr}/")).unwrap(); let fetcher_cursor = FetcherCursor { next_miniblock: MiniblockNumber(1), - prev_miniblock_hash: genesis_miniblock_hash, l1_batch: L1BatchNumber(0), }; let fetcher = fetcher_cursor.into_fetcher( From e05d955036c76a29f9b6e900872c69e20278e045 Mon Sep 17 00:00:00 2001 From: EmilLuta Date: Fri, 1 Dec 2023 11:10:25 +0100 Subject: [PATCH 16/18] fix(witness_generator): Disable BWIP dependency (#573) This revert is done to facilitate boojum upgrade on mainnet2. Without this, old provers would be halt and boojum upgrade could take longer than anticipated. `waiting_for_artifacts` forced witness to wait on BWIP run. `queued` makes them run instantly. - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. - [ ] Spellcheck has been run via `cargo spellcheck --cfg=./spellcheck/era.cfg --code 1`. --------- Signed-off-by: Danil Co-authored-by: Danil --- core/lib/dal/sqlx-data.json | 30 +++++++++++------------ core/lib/dal/src/witness_generator_dal.rs | 2 +- spellcheck/era.dic | 1 + 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/core/lib/dal/sqlx-data.json b/core/lib/dal/sqlx-data.json index 3776b4f84b34..2c958ce0394c 100644 --- a/core/lib/dal/sqlx-data.json +++ b/core/lib/dal/sqlx-data.json @@ -7470,21 +7470,6 @@ }, "query": "SELECT number, timestamp, is_finished, l1_tx_count, l2_tx_count, fee_account_address, bloom, priority_ops_onchain_data, hash, parent_hash, commitment, compressed_write_logs, compressed_contracts, eth_prove_tx_id, eth_commit_tx_id, eth_execute_tx_id, merkle_root_hash, l2_to_l1_logs, l2_to_l1_messages, used_contract_hashes, compressed_initial_writes, compressed_repeated_writes, l2_l1_compressed_messages, l2_l1_merkle_root, l1_gas_price, l2_fair_gas_price, rollup_last_leaf_index, zkporter_is_available, bootloader_code_hash, default_aa_code_hash, base_fee_per_gas, aux_data_hash, pass_through_data_hash, meta_parameters_hash, protocol_version, compressed_state_diffs, system_logs, events_queue_commitment, bootloader_initial_content_commitment FROM l1_batches LEFT JOIN commitments ON commitments.l1_batch_number = l1_batches.number WHERE eth_prove_tx_id IS NOT NULL AND eth_execute_tx_id IS NULL ORDER BY number LIMIT $1" }, - "8ff9d76b4791af1177231661847b6c8879ad625fd11c15de51a16c81d8712129": { - "describe": { - "columns": [], - "nullable": [], - "parameters": { - "Left": [ - "Int8", - "Bytea", - "Text", - "Int4" - ] - } - }, - "query": "INSERT INTO witness_inputs(l1_batch_number, merkle_tree_paths, merkel_tree_paths_blob_url, status, protocol_version, created_at, updated_at) VALUES ($1, $2, $3, 'waiting_for_artifacts', $4, now(), now()) ON CONFLICT (l1_batch_number) DO NOTHING" - }, "9051cc1a715e152afdd0c19739c76666b1a9b134e17601ef9fdf3dec5d2fc561": { "describe": { "columns": [ @@ -11182,6 +11167,21 @@ }, "query": "UPDATE l1_batches SET predicted_commit_gas_cost = $2, updated_at = now() WHERE number = $1" }, + "ec35fc5128cf59d19e6d65ed6d84fcc50fedce921405c4ce700dd2e08c990642": { + "describe": { + "columns": [], + "nullable": [], + "parameters": { + "Left": [ + "Int8", + "Bytea", + "Text", + "Int4" + ] + } + }, + "query": "INSERT INTO witness_inputs(l1_batch_number, merkle_tree_paths, merkel_tree_paths_blob_url, status, protocol_version, created_at, updated_at) VALUES ($1, $2, $3, 'queued', $4, now(), now()) ON CONFLICT (l1_batch_number) DO NOTHING" + }, "ed50c609371b4588964e29f8757c41973706710090a80eb025ec263ce3d019b4": { "describe": { "columns": [], diff --git a/core/lib/dal/src/witness_generator_dal.rs b/core/lib/dal/src/witness_generator_dal.rs index 983112ab35cc..a8079a9dccee 100644 --- a/core/lib/dal/src/witness_generator_dal.rs +++ b/core/lib/dal/src/witness_generator_dal.rs @@ -728,7 +728,7 @@ impl WitnessGeneratorDal<'_, '_> { { sqlx::query!( "INSERT INTO witness_inputs(l1_batch_number, merkle_tree_paths, merkel_tree_paths_blob_url, status, protocol_version, created_at, updated_at) \ - VALUES ($1, $2, $3, 'waiting_for_artifacts', $4, now(), now()) \ + VALUES ($1, $2, $3, 'queued', $4, now(), now()) \ ON CONFLICT (l1_batch_number) DO NOTHING", block_number.0 as i64, // TODO(SMA-1476): remove the below column once blob is migrated to GCS. diff --git a/spellcheck/era.dic b/spellcheck/era.dic index 214efbcd595f..666ee047fd26 100644 --- a/spellcheck/era.dic +++ b/spellcheck/era.dic @@ -262,6 +262,7 @@ sidechain sidechains tokenomics validator's +validator CHAINID PREVRANDAO ECDSA From 5994aaef62f43910629c5ec916799d6cd6ff0967 Mon Sep 17 00:00:00 2001 From: zksync-era-bot <147085853+zksync-era-bot@users.noreply.github.com> Date: Fri, 1 Dec 2023 12:12:27 +0100 Subject: [PATCH 17/18] chore(main): release core 18.4.0 (#560) :robot: I have created a release *beep* *boop* --- ## [18.4.0](https://github.com/matter-labs/zksync-era/compare/core-v18.3.1...core-v18.4.0) (2023-12-01) ### Features * adds spellchecker workflow, and corrects misspelled words ([#559](https://github.com/matter-labs/zksync-era/issues/559)) ([beac0a8](https://github.com/matter-labs/zksync-era/commit/beac0a85bb1535b05c395057171f197cd976bf82)) * **en:** Support arbitrary genesis block for external nodes ([#537](https://github.com/matter-labs/zksync-era/issues/537)) ([15d7eaf](https://github.com/matter-labs/zksync-era/commit/15d7eaf872e222338810243865cec9dff7f6e799)) * **merkle tree:** Remove enumeration index assignment from Merkle tree ([#551](https://github.com/matter-labs/zksync-era/issues/551)) ([e2c1b20](https://github.com/matter-labs/zksync-era/commit/e2c1b20e361e6ee2f5ac69cefe75d9c5575eb2f7)) * Restore commitment test in Boojum integration ([#539](https://github.com/matter-labs/zksync-era/issues/539)) ([06f510d](https://github.com/matter-labs/zksync-era/commit/06f510d00f855ddafaebb504f7ea799700221072)) ### Bug Fixes * Change no pending batches 404 error into a success response ([#279](https://github.com/matter-labs/zksync-era/issues/279)) ([e8fd805](https://github.com/matter-labs/zksync-era/commit/e8fd805c8be7980de7676bca87cfc2d445aab9e1)) * **vm:** Expose additional types and traits ([#563](https://github.com/matter-labs/zksync-era/issues/563)) ([bd268ac](https://github.com/matter-labs/zksync-era/commit/bd268ac02bc3530c1d3247cb9496c3e13c2e52d9)) * **witness_generator:** Disable BWIP dependency ([#573](https://github.com/matter-labs/zksync-era/issues/573)) ([e05d955](https://github.com/matter-labs/zksync-era/commit/e05d955036c76a29f9b6e900872c69e20278e045)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --- .github/release-please/manifest.json | 2 +- core/CHANGELOG.md | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/release-please/manifest.json b/.github/release-please/manifest.json index f5774af59440..9c7f15805a9b 100644 --- a/.github/release-please/manifest.json +++ b/.github/release-please/manifest.json @@ -1,5 +1,5 @@ { "sdk/zksync-rs": "0.4.0", - "core": "18.3.1", + "core": "18.4.0", "prover": "9.0.0" } diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index d34f9d4faf58..04f3f13e716e 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## [18.4.0](https://github.com/matter-labs/zksync-era/compare/core-v18.3.1...core-v18.4.0) (2023-12-01) + + +### Features + +* adds spellchecker workflow, and corrects misspelled words ([#559](https://github.com/matter-labs/zksync-era/issues/559)) ([beac0a8](https://github.com/matter-labs/zksync-era/commit/beac0a85bb1535b05c395057171f197cd976bf82)) +* **en:** Support arbitrary genesis block for external nodes ([#537](https://github.com/matter-labs/zksync-era/issues/537)) ([15d7eaf](https://github.com/matter-labs/zksync-era/commit/15d7eaf872e222338810243865cec9dff7f6e799)) +* **merkle tree:** Remove enumeration index assignment from Merkle tree ([#551](https://github.com/matter-labs/zksync-era/issues/551)) ([e2c1b20](https://github.com/matter-labs/zksync-era/commit/e2c1b20e361e6ee2f5ac69cefe75d9c5575eb2f7)) +* Restore commitment test in Boojum integration ([#539](https://github.com/matter-labs/zksync-era/issues/539)) ([06f510d](https://github.com/matter-labs/zksync-era/commit/06f510d00f855ddafaebb504f7ea799700221072)) + + +### Bug Fixes + +* Change no pending batches 404 error into a success response ([#279](https://github.com/matter-labs/zksync-era/issues/279)) ([e8fd805](https://github.com/matter-labs/zksync-era/commit/e8fd805c8be7980de7676bca87cfc2d445aab9e1)) +* **vm:** Expose additional types and traits ([#563](https://github.com/matter-labs/zksync-era/issues/563)) ([bd268ac](https://github.com/matter-labs/zksync-era/commit/bd268ac02bc3530c1d3247cb9496c3e13c2e52d9)) +* **witness_generator:** Disable BWIP dependency ([#573](https://github.com/matter-labs/zksync-era/issues/573)) ([e05d955](https://github.com/matter-labs/zksync-era/commit/e05d955036c76a29f9b6e900872c69e20278e045)) + ## [18.3.1](https://github.com/matter-labs/zksync-era/compare/core-v18.3.0...core-v18.3.1) (2023-11-28) From 0cd2c6b5ada568ffe01bce6d2dd8951457369141 Mon Sep 17 00:00:00 2001 From: Yury Akudovich Date: Fri, 1 Dec 2023 12:42:44 +0100 Subject: [PATCH 18/18] ci: Runs spellcheck in merge queue. (#574) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Runs spellcheck in merge queue. ## Why ❔ ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`. - [ ] Spellcheck has been run via `cargo spellcheck --cfg=./spellcheck/era.cfg --code 1`. --- .github/workflows/check-spelling.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-spelling.yml b/.github/workflows/check-spelling.yml index 76fd6352c8e7..9073401eab9e 100644 --- a/.github/workflows/check-spelling.yml +++ b/.github/workflows/check-spelling.yml @@ -5,8 +5,9 @@ on: branches: - main pull_request: + merge_group: -env: +env: CARGO_TERM_COLOR: always jobs: @@ -17,7 +18,7 @@ jobs: uses: taiki-e/install-action@v2 with: tool: cargo-spellcheck - + - uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4 - name: Run cargo-spellcheck