node: fix block body txn counters in access layer (#1328)

silkworm/node/db/access_layer.cpp

@@ -393,24 +393,25 @@ size_t process_blocks_at_height(ROTxn& txn, BlockNum height, std::function<void(
 
     auto count = db::cursor_for_prefix(
         *bodies_cursor, key_prefix,
-        [&process_func, &txn, &read_senders](ByteView key, ByteView raw_body) {
+        [&process_func, &txn, &height, &read_senders](ByteView key, ByteView raw_body) {
             if (raw_body.empty()) throw std::logic_error("empty header in table Headers");
             // read block...
             Block block;
             // ...ommers
-            auto body_for_storage = detail::decode_stored_block_body(raw_body);
-            std::swap(block.ommers, body_for_storage.ommers);
+            auto body = detail::decode_stored_block_body(raw_body);
+            std::swap(block.ommers, body.ommers);
             // ...transactions
-            read_transactions(txn, body_for_storage.base_txn_id, body_for_storage.txn_count, block.transactions);
+            ensure(body.txn_count > 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) {

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()};

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);

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{};

silkworm/node/stagedsync/stages/stage_tx_lookup_test.cpp

@@ -17,6 +17,7 @@
 #include <catch2/catch.hpp>
 
 #include <silkworm/core/common/test_util.hpp>
+#include <silkworm/infra/test_util/log.hpp>
 #include <silkworm/node/db/access_layer.hpp>
 #include <silkworm/node/stagedsync/stages/stage_tx_lookup.hpp>
 #include <silkworm/node/test/context.hpp>
@@ -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,28 +46,28 @@ 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
     // ---------------------------------------
     Bytes tx_rlp{};
     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));