From 3cbfd0efc98c695d2b3c184e678aa098a2ac96e5 Mon Sep 17 00:00:00 2001 From: canepat <16927169+canepat@users.noreply.github.com> Date: Sun, 16 Jul 2023 09:03:30 +0200 Subject: [PATCH] node: fix block body txn counters in access layer (#1328) --- silkworm/node/db/access_layer.cpp | 29 ++++++++------- silkworm/node/db/access_layer_test.cpp | 36 +++++++++---------- .../node/stagedsync/execution_engine_test.cpp | 9 +++-- silkworm/node/stagedsync/stages/_test.cpp | 12 +++++-- .../stages/stage_tx_lookup_test.cpp | 13 ++++--- 5 files changed, 59 insertions(+), 40 deletions(-) diff --git a/silkworm/node/db/access_layer.cpp b/silkworm/node/db/access_layer.cpp index f2fa399f6a..8a88b826ef 100644 --- a/silkworm/node/db/access_layer.cpp +++ b/silkworm/node/db/access_layer.cpp @@ -393,24 +393,25 @@ size_t process_blocks_at_height(ROTxn& txn, BlockNum height, std::function 1, "unexpected txn_count=" + std::to_string(body.txn_count) + " for number=" + std::to_string(height)); + read_transactions(txn, body.base_txn_id + 1, body.txn_count - 2, block.transactions); // ...senders if (!block.transactions.empty() && read_senders) { - Bytes kkey{key.data(), key.length()}; - db::parse_senders(txn, kkey, block.transactions); + Bytes key_bytes{key.data(), key.length()}; // TODO(canepat) avoid unnecessary copy by changing read_senders API + db::parse_senders(txn, key_bytes, block.transactions); } // ...header auto [block_num, hash] = split_block_key(key); - bool present = read_header(txn, hash, block_num, block.header); - if (!present) throw std::logic_error("header not found for body number= " + std::to_string(block_num) + ", hash= " + to_hex(hash)); + const bool present = read_header(txn, hash, block_num, block.header); + ensure(present, "header not found for body number= " + std::to_string(block_num) + ", hash= " + to_hex(hash)); // invoke handler process_func(block); }, @@ -439,7 +440,9 @@ bool read_body(ROTxn& txn, const Bytes& key, bool read_senders, BlockBody& out) auto body{detail::decode_stored_block_body(data_view)}; std::swap(out.ommers, body.ommers); - read_transactions(txn, body.base_txn_id, body.txn_count, out.transactions); + std::swap(out.withdrawals, body.withdrawals); + ensure(body.txn_count > 1, "unexpected txn_count=" + std::to_string(body.txn_count) + " for key=" + to_hex(key)); + read_transactions(txn, body.base_txn_id + 1, body.txn_count - 2, out.transactions); if (!out.transactions.empty() && read_senders) { parse_senders(txn, key, out.transactions); } @@ -454,8 +457,8 @@ bool read_rlp_transactions(ROTxn& txn, BlockNum block_number, const evmc::bytes3 ByteView data_view{from_slice(data.value)}; const auto body{detail::decode_stored_block_body(data_view)}; - - read_rlp_transactions(txn, body.base_txn_id, body.txn_count, rlp_txs); + ensure(body.txn_count > 1, "unexpected txn_count=" + std::to_string(body.txn_count) + " for key=" + std::to_string(block_number)); + read_rlp_transactions(txn, body.base_txn_id + 1, body.txn_count - 2, rlp_txs); return true; } @@ -495,7 +498,7 @@ void write_body(RWTxn& txn, const BlockBody& body, const evmc::bytes32& hash, Bl void write_body(RWTxn& txn, const BlockBody& body, const uint8_t (&hash)[kHashLength], const BlockNum number) { detail::BlockBodyForStorage body_for_storage{}; body_for_storage.ommers = body.ommers; - body_for_storage.txn_count = body.transactions.size(); + body_for_storage.txn_count = body.transactions.size() + 2; body_for_storage.base_txn_id = increment_map_sequence(txn, table::kBlockTransactions.name, body_for_storage.txn_count); Bytes value{body_for_storage.encode()}; @@ -504,7 +507,7 @@ void write_body(RWTxn& txn, const BlockBody& body, const uint8_t (&hash)[kHashLe auto target = txn.rw_cursor(table::kBlockBodies); target->upsert(to_slice(key), to_slice(value)); - write_transactions(txn, body.transactions, body_for_storage.base_txn_id); + write_transactions(txn, body.transactions, body_for_storage.base_txn_id + 1); } static ByteView read_senders_raw(ROTxn& txn, const Bytes& key) { diff --git a/silkworm/node/db/access_layer_test.cpp b/silkworm/node/db/access_layer_test.cpp index a64d03fd17..1432e3a553 100644 --- a/silkworm/node/db/access_layer_test.cpp +++ b/silkworm/node/db/access_layer_test.cpp @@ -88,7 +88,7 @@ static BlockBody sample_block_body() { namespace silkworm::db { -TEST_CASE("Db Opening") { +TEST_CASE("Db Opening", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; // Empty dir std::string empty{}; @@ -133,7 +133,7 @@ TEST_CASE("Db Opening") { env.close(); } -TEST_CASE("Methods cursor_for_each/cursor_for_count") { +TEST_CASE("Methods cursor_for_each/cursor_for_count", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -159,7 +159,7 @@ TEST_CASE("Methods cursor_for_each/cursor_for_count") { CHECK(table_names.size() == max_count); } -TEST_CASE("VersionBase primitives") { +TEST_CASE("VersionBase primitives", "[silkworm][node][db][access_layer]") { VersionBase v1{0, 0, 0}; VersionBase v2{0, 0, 1}; VersionBase v3{0, 0, 1}; @@ -170,7 +170,7 @@ TEST_CASE("VersionBase primitives") { CHECK(v2 == v3); } -TEST_CASE("Sequences") { +TEST_CASE("Sequences", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -217,7 +217,7 @@ TEST_CASE("Sequences") { CHECK(thrown); } -TEST_CASE("Schema Version") { +TEST_CASE("Schema Version", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context(/*with_create_tables=*/false); @@ -255,7 +255,7 @@ TEST_CASE("Schema Version") { } } -TEST_CASE("Storage and Prune Modes") { +TEST_CASE("Storage and Prune Modes", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.txn()}; @@ -382,7 +382,7 @@ TEST_CASE("Storage and Prune Modes") { } } -TEST_CASE("Stages") { +TEST_CASE("Stages", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -421,7 +421,7 @@ TEST_CASE("Stages") { CHECK(stages::read_stage_prune_progress(txn, stages::kBlockBodiesKey) == 0); } -TEST_CASE("Snapshots") { +TEST_CASE("Snapshots", "[silkworm][node][db][access_layer]") { test::Context context; auto& txn{context.rw_txn()}; @@ -435,7 +435,7 @@ TEST_CASE("Snapshots") { CHECK(read_snapshots(txn) == snapshot_list); } -TEST_CASE("Difficulty") { +TEST_CASE("Difficulty", "[silkworm][node][db][access_layer]") { test::Context context; auto& txn{context.rw_txn()}; @@ -449,7 +449,7 @@ TEST_CASE("Difficulty") { CHECK(model.read_total_difficulty(block_num, hash) == difficulty); } -TEST_CASE("Headers and bodies") { +TEST_CASE("Headers and bodies", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -570,7 +570,7 @@ TEST_CASE("Headers and bodies") { } } -TEST_CASE("Account") { +TEST_CASE("Account", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; db::RWTxn& txn{context.rw_txn()}; @@ -614,7 +614,7 @@ TEST_CASE("Account") { CHECK(intx::to_string(historical_account->balance) == std::to_string(protocol::kBlockRewardFrontier)); } -TEST_CASE("Storage") { +TEST_CASE("Storage", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -643,7 +643,7 @@ TEST_CASE("Storage") { CHECK(db::read_storage(txn, addr, kDefaultIncarnation, loc4) == evmc::bytes32{}); } -TEST_CASE("Account_changes") { +TEST_CASE("Account_changes", "[silkworm][node][db][access_layer]") { test::Context context; auto& txn{context.rw_txn()}; @@ -702,7 +702,7 @@ TEST_CASE("Account_changes") { CHECK(changes.empty()); } -TEST_CASE("Storage changes") { +TEST_CASE("Storage changes", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -782,7 +782,7 @@ TEST_CASE("Storage changes") { CHECK(db_changes == expected_changes3); } -TEST_CASE("Chain config") { +TEST_CASE("Chain config", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -805,7 +805,7 @@ TEST_CASE("Chain config") { CHECK(chain_config3 == kSepoliaConfig); } -TEST_CASE("Head header") { +TEST_CASE("Head header", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -815,7 +815,7 @@ TEST_CASE("Head header") { REQUIRE(db::read_head_header_hash(txn).value() == kSepoliaGenesisHash); } -TEST_CASE("Last Fork Choice") { +TEST_CASE("Last Fork Choice", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; @@ -833,7 +833,7 @@ TEST_CASE("Last Fork Choice") { CHECK(db::read_last_finalized_block(txn) == hash3); } -TEST_CASE("read rlp encoded transactions") { +TEST_CASE("read rlp encoded transactions", "[silkworm][node][db][access_layer]") { test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; test::Context context; auto& txn{context.rw_txn()}; diff --git a/silkworm/node/stagedsync/execution_engine_test.cpp b/silkworm/node/stagedsync/execution_engine_test.cpp index 61b216e8fb..29fea2b9ae 100644 --- a/silkworm/node/stagedsync/execution_engine_test.cpp +++ b/silkworm/node/stagedsync/execution_engine_test.cpp @@ -130,6 +130,11 @@ TEST_CASE("ExecutionEngine") { BlockId block0_id{0, *header0_hash}; + // check db + BlockBody block0_body; + const bool block0_present = db::read_body(tx, *header0_hash, block0_id.number, block0_body); + CHECK(block0_present); + /* status: * h0 * input: @@ -156,8 +161,8 @@ TEST_CASE("ExecutionEngine") { // check db BlockBody saved_body; - bool present = db::read_body(tx, header1_hash, block1->header.number, saved_body); - CHECK(present); + const bool block1_present = db::read_body(tx, header1_hash, block1->header.number, saved_body); + CHECK(block1_present); auto progress = exec_engine.block_progress(); CHECK(progress == 1); diff --git a/silkworm/node/stagedsync/stages/_test.cpp b/silkworm/node/stagedsync/stages/_test.cpp index 4d36dc8c4c..b2526d44fd 100644 --- a/silkworm/node/stagedsync/stages/_test.cpp +++ b/silkworm/node/stagedsync/stages/_test.cpp @@ -57,12 +57,16 @@ TEST_CASE("Sync Stages") { db::RWTxnManaged txn(chaindata_env); db::table::check_or_create_chaindata_tables(txn); txn.commit_and_renew(); + const auto initial_tx_sequence{db::read_map_sequence(txn, db::table::kBlockTransactions.name)}; + REQUIRE(initial_tx_sequence == 0); // no txs at start auto source_data{read_genesis_data(node_settings.network_id)}; auto genesis_json = nlohmann::json::parse(source_data, nullptr, /* allow_exceptions = */ false); db::initialize_genesis(txn, genesis_json, /*allow_exceptions=*/true); txn.commit_and_renew(); node_settings.chain_config = db::read_chain_config(txn); + const auto tx_sequence_after_genesis{db::read_map_sequence(txn, db::table::kBlockTransactions.name)}; + REQUIRE(tx_sequence_after_genesis == 2); // 2 system txs for genesis SECTION("BlockHashes") { SECTION("Forward/Unwind/Prune args validation") { @@ -151,9 +155,13 @@ TEST_CASE("Sync Stages") { // First block - 1 transaction block_body.transactions.push_back(sample_transactions[0]); REQUIRE_NOTHROW(db::write_body(txn, block_body, block_hashes[0].bytes, 1)); + const auto tx_sequence_after_block1{db::read_map_sequence(txn, db::table::kBlockTransactions.name)}; + REQUIRE(tx_sequence_after_block1 == 5); // 1 tx + 2 system txs for block 1 // Second block - 1 transactions REQUIRE_NOTHROW(db::write_body(txn, block_body, block_hashes[1].bytes, 2)); + const auto tx_sequence_after_block2{db::read_map_sequence(txn, db::table::kBlockTransactions.name)}; + REQUIRE(tx_sequence_after_block2 == 8); // 1 tx + 2 system txs for block 2 // Third block - 0 transactions block_body.transactions.clear(); @@ -172,8 +180,8 @@ TEST_CASE("Sync Stages") { REQUIRE_NOTHROW(txn.commit_and_renew()); // Verify sequence for transactions has been incremented properly - auto last_tx_sequence{db::read_map_sequence(txn, db::table::kBlockTransactions.name)}; - REQUIRE(last_tx_sequence == 2); + const auto last_tx_sequence{db::read_map_sequence(txn, db::table::kBlockTransactions.name)}; + REQUIRE(last_tx_sequence == 10); // 2 system txs for block 3 // Prepare stage stagedsync::SyncContext sync_context{}; diff --git a/silkworm/node/stagedsync/stages/stage_tx_lookup_test.cpp b/silkworm/node/stagedsync/stages/stage_tx_lookup_test.cpp index 12677a8f0e..6ef71a376c 100644 --- a/silkworm/node/stagedsync/stages/stage_tx_lookup_test.cpp +++ b/silkworm/node/stagedsync/stages/stage_tx_lookup_test.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -26,6 +27,8 @@ using namespace evmc::literals; namespace silkworm { TEST_CASE("Stage Transaction Lookups") { + test_util::SetLogVerbosityGuard log_guard{log::Level::kNone}; + static constexpr evmc::bytes32 hash_0{0x3ac225168df54212a25c1c01fd35bebfea408fdac2e31ddd6f80a4bbf9a5f1cb_bytes32}; static constexpr evmc::bytes32 hash_1{0xb5553de315e0edf504d9150af82dafa5c4667fa618ed0a6f19c69b41166c5510_bytes32}; @@ -43,7 +46,8 @@ TEST_CASE("Stage Transaction Lookups") { db::detail::BlockBodyForStorage block{}; auto transactions{test::sample_transactions()}; block.base_txn_id = 1; - block.txn_count = 1; + block.txn_count = 1 + 2; // + 2: 2 system txs (1 at the beginning and 1 at the end) + // --------------------------------------- // Push first block // --------------------------------------- @@ -51,20 +55,19 @@ TEST_CASE("Stage Transaction Lookups") { rlp::encode(tx_rlp, transactions[0]); auto tx_hash_1{keccak256(tx_rlp)}; - transactions_table.upsert(db::to_slice(db::block_key(1)), db::to_slice(tx_rlp)); + transactions_table.upsert(db::to_slice(db::block_key(block.base_txn_id + 1)), db::to_slice(tx_rlp)); bodies_table.upsert(db::to_slice(db::block_key(1, hash_0.bytes)), db::to_slice(block.encode())); REQUIRE_NOTHROW(db::write_canonical_header_hash(txn, hash_0.bytes, 1)); // --------------------------------------- // Push second block // --------------------------------------- - - block.base_txn_id = 2; + block.base_txn_id += block.txn_count; rlp::encode(tx_rlp, transactions[1]); auto tx_hash_2{keccak256(tx_rlp)}; - transactions_table.upsert(db::to_slice(db::block_key(2)), db::to_slice(tx_rlp)); + transactions_table.upsert(db::to_slice(db::block_key(block.base_txn_id + 1)), db::to_slice(tx_rlp)); bodies_table.upsert(db::to_slice(db::block_key(2, hash_1.bytes)), db::to_slice(block.encode())); REQUIRE_NOTHROW(db::write_canonical_header_hash(txn, hash_1.bytes, 2));