From 88b09ff9da9dada3e9c066243f0e303cd00f838c Mon Sep 17 00:00:00 2001 From: Jacek Glen Date: Tue, 9 Jul 2024 08:02:58 +0200 Subject: [PATCH] db: add extra tests for Buffer (#2171) --- silkworm/db/buffer_test.cpp | 213 ++++++++++++++++++++++++++++++------ 1 file changed, 179 insertions(+), 34 deletions(-) diff --git a/silkworm/db/buffer_test.cpp b/silkworm/db/buffer_test.cpp index 4539738132..5e299a7722 100644 --- a/silkworm/db/buffer_test.cpp +++ b/silkworm/db/buffer_test.cpp @@ -30,7 +30,7 @@ namespace silkworm::db { using silkworm::test_util::SetLogVerbosityGuard; -TEST_CASE("Storage update") { +TEST_CASE("Buffer storage", "[silkworm][db][buffer]") { SetLogVerbosityGuard log_guard{log::Level::kNone}; db::test_util::TempChainData context; auto& txn{context.rw_txn()}; @@ -42,10 +42,13 @@ TEST_CASE("Storage update") { const auto value_a1{0x000000000000000000000000000000000000000000000000000000000000006b_bytes32}; const auto value_a2{0x0000000000000000000000000000000000000000000000000000000000000085_bytes32}; const auto value_a3{0x0000000000000000000000000000000000000000000000000000000000000095_bytes32}; + const auto value_nil{0x0000000000000000000000000000000000000000000000000000000000000000_bytes32}; const auto location_b{0x0000000000000000000000000000000000000000000000000000000000000002_bytes32}; const auto value_b{0x0000000000000000000000000000000000000000000000000000000000000132_bytes32}; + const auto location_c{0x0000000000000000000000000000000000000000000000000000000000000003_bytes32}; + auto state = txn.rw_cursor_dup_sort(table::kPlainState); upsert_storage_value(*state, key, location_a.bytes, value_a1.bytes); @@ -53,50 +56,192 @@ TEST_CASE("Storage update") { Buffer buffer{txn}; - CHECK(buffer.read_storage(address, kDefaultIncarnation, location_a) == value_a1); + SECTION("Reads storage by address and location") { + CHECK(buffer.read_storage(address, kDefaultIncarnation, location_a) == value_a1); + CHECK(buffer.read_storage(address, kDefaultIncarnation, location_b) == value_b); + } - // Update only location A - buffer.update_storage(address, kDefaultIncarnation, location_a, - /*initial=*/value_a1, /*current=*/value_a2); + SECTION("Updates storage by address and location") { + // Update only location A + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/value_a2); - REQUIRE(buffer.storage_changes().empty() == false); + REQUIRE(buffer.storage_changes().empty() == false); - buffer.write_to_db(); + buffer.write_to_db(); - // Location A should have the new value - const std::optional db_value_a{find_value_suffix(*state, key, location_a.bytes)}; - REQUIRE(db_value_a.has_value()); - CHECK(db_value_a == zeroless_view(value_a2.bytes)); + // Location A should have the new value + const std::optional db_value_a{find_value_suffix(*state, key, location_a.bytes)}; + REQUIRE(db_value_a.has_value()); + CHECK(db_value_a == zeroless_view(value_a2.bytes)); - // Location B should not change - const std::optional db_value_b{find_value_suffix(*state, key, location_b.bytes)}; - REQUIRE(db_value_b.has_value()); - CHECK(db_value_b == zeroless_view(value_b.bytes)); + // Location B should not change + const std::optional db_value_b{find_value_suffix(*state, key, location_b.bytes)}; + REQUIRE(db_value_b.has_value()); + CHECK(db_value_b == zeroless_view(value_b.bytes)); + } - // Update again only location A - buffer.update_storage(address, kDefaultIncarnation, location_a, - /*initial=*/value_a2, /*current=*/value_a3); + SECTION("Keeps track of storage changes") { + // Update location A + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/value_a2); + buffer.write_to_db(); + + // Update again location A + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a2, /*current=*/value_a3); + + REQUIRE(buffer.storage_changes().empty() == false); + + // Ask state buffer to not write change sets + buffer.write_to_db(/*write_change_sets=*/false); + + // Location A should have the previous value of old value in state changes, i.e. value_a1 + const auto storage_changes{db::read_storage_changes(txn, 0)}; + REQUIRE(storage_changes.size() == 1); + const auto& [changed_address, changed_map] = *storage_changes.begin(); + CHECK(changed_address == address); + REQUIRE(changed_map.size() == 1); + const auto& [changed_incarnation, changed_storage] = *changed_map.begin(); + CHECK(changed_incarnation == kDefaultIncarnation); + REQUIRE(changed_storage.size() == 1); + const auto& [changed_location, changed_value] = *changed_storage.begin(); + CHECK(changed_location == location_a); + CHECK(changed_value == zeroless_view(value_a1.bytes)); + } - REQUIRE(buffer.storage_changes().empty() == false); + SECTION("Multiple storage changes in a single block saves one storage change entry") { + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/value_a2); + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a2, /*current=*/value_a3); + + REQUIRE(buffer.storage_changes().empty() == false); + + buffer.write_to_db(); + + const auto storage_changes{db::read_storage_changes(txn, 0)}; + REQUIRE(storage_changes.size() == 1); + const auto& [changed_address, changed_map] = *storage_changes.begin(); + CHECK(changed_address == address); + REQUIRE(changed_map.size() == 1); + const auto& [changed_incarnation, changed_storage] = *changed_map.begin(); + CHECK(changed_incarnation == kDefaultIncarnation); + REQUIRE(changed_storage.size() == 1); + const auto& [changed_location_a, changed_value_a] = *changed_storage.find(location_a); + CHECK(changed_location_a == location_a); + CHECK(changed_value_a == zeroless_view(value_a2.bytes)); + } - // Ask state buffer to not write change sets - buffer.write_to_db(/*write_change_sets=*/false); + SECTION("Multiple storage changes in different blocks cause multiple storage changes") { + buffer.begin_block(1, 1); + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/value_nil); + buffer.write_to_db(); + + buffer.begin_block(2, 1); + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_nil, /*current=*/value_a3); + buffer.write_to_db(); + + buffer.begin_block(3, 1); + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a3, /*current=*/value_a2); + buffer.write_to_db(); + + const auto storage_changes1{db::read_storage_changes(txn, 1)}; + REQUIRE(storage_changes1.size() == 1); + const auto storage_changes2{db::read_storage_changes(txn, 2)}; + REQUIRE(storage_changes2.size() == 1); + const auto storage_changes3{db::read_storage_changes(txn, 3)}; + REQUIRE(storage_changes3.size() == 1); + + const std::optional db_value_a2{find_value_suffix(*state, key, location_a.bytes)}; + REQUIRE(db_value_a2.has_value()); + CHECK(db_value_a2 == zeroless_view(value_a2.bytes)); + } + + SECTION("Deletes storage by address and location") { + // Delete location A + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/value_nil); + + // Buffer value set to nil + auto current_value_a1{buffer.read_storage(address, kDefaultIncarnation, location_a)}; + CHECK(current_value_a1 == value_nil); + + // Not deleted from the db yet + const std::optional db_value_a1{find_value_suffix(*state, key, location_a.bytes)}; + CHECK(db_value_a1.has_value()); + CHECK(db_value_a1 == zeroless_view(value_a1.bytes)); + + buffer.write_to_db(); + + // Buffer reads the value from the db + auto current_value_a2{buffer.read_storage(address, kDefaultIncarnation, location_a)}; + CHECK(current_value_a2 == value_nil); + + // Location A should be deleted + const std::optional db_value_a2{find_value_suffix(*state, key, location_a.bytes)}; + CHECK(!db_value_a2.has_value()); + + // Location B should not change + const std::optional db_value_b{find_value_suffix(*state, key, location_b.bytes)}; + REQUIRE(db_value_b.has_value()); + CHECK(db_value_b == zeroless_view(value_b.bytes)); + } - // Location A should have the previous value of old value in state changes, i.e. value_a1 - const auto storage_changes{db::read_storage_changes(txn, 0)}; - REQUIRE(storage_changes.size() == 1); - const auto& [changed_address, changed_map] = *storage_changes.begin(); - CHECK(changed_address == address); - REQUIRE(changed_map.size() == 1); - const auto& [changed_incarnation, changed_storage] = *changed_map.begin(); - CHECK(changed_incarnation == kDefaultIncarnation); - REQUIRE(changed_storage.size() == 1); - const auto& [changed_location, changed_value] = *changed_storage.begin(); - CHECK(changed_location == location_a); - CHECK(changed_value == zeroless_view(value_a1.bytes)); + SECTION("Can re-set value after deletion") { + // Buffer only + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/value_nil); + + // Buffer value set to nil + auto current_value_a1{buffer.read_storage(address, kDefaultIncarnation, location_a)}; + CHECK(current_value_a1 == value_nil); + + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_nil, /*current=*/value_a2); + + auto current_value_a2{buffer.read_storage(address, kDefaultIncarnation, location_a)}; + CHECK(current_value_a2 == value_a2); + } + + SECTION("Sets new value") { + buffer.update_storage(address, kDefaultIncarnation, location_c, + /*initial=*/{}, /*current=*/value_a1); + + auto current_value_a1{buffer.read_storage(address, kDefaultIncarnation, location_c)}; + CHECK(current_value_a1 == value_a1); + + buffer.write_to_db(); + + const std::optional db_value_c1{find_value_suffix(*state, key, location_c.bytes)}; + REQUIRE(db_value_c1.has_value()); + CHECK(db_value_c1 == zeroless_view(value_a1.bytes)); + + auto current_value_a2{buffer.read_storage(address, kDefaultIncarnation, location_c)}; + CHECK(current_value_a2 == value_a1); + } + + SECTION("Setting to nil deletes the value") { + buffer.update_storage(address, kDefaultIncarnation, location_a, + /*initial=*/value_a1, /*current=*/{}); + + auto current_value_a1{buffer.read_storage(address, kDefaultIncarnation, location_a)}; + CHECK(current_value_a1 == value_nil); + + buffer.write_to_db(); + + const std::optional db_value_a1{find_value_suffix(*state, key, location_a.bytes)}; + CHECK(!db_value_a1.has_value()); + + auto current_value_a2{buffer.read_storage(address, kDefaultIncarnation, location_a)}; + CHECK(current_value_a2 == value_nil); + } } -TEST_CASE("Account update") { +TEST_CASE("Buffer account", "[silkworm][db][buffer]") { SetLogVerbosityGuard log_guard{log::Level::kNone}; db::test_util::TempChainData context; auto& txn{context.rw_txn()};