datastore: improve DomainGetLatestQuery wrappers (#2661)

Bind a fixed domain name to each query.

.clang-tidy

@@ -86,6 +86,7 @@ Checks: >
   -readability-suspicious-call-argument,
   -readability-uppercase-literal-suffix
 CheckOptions:
+  performance-move-const-arg.CheckTriviallyCopyableMove: false
   performance-unnecessary-value-param.AllowedTypes: std::exception_ptr
   readability-identifier-naming.ClassCase: CamelCase
   readability-identifier-naming.ClassIgnoredRegexp: buildinfo|glaze

silkworm/db/cli/snapshots.cpp

@@ -544,16 +544,15 @@ void open_existence_index(const SnapshotSubcommandSettings& settings) {
                 ByteView nonexistent_key = {full_be + kSizeDiff, sizeof(intx::uint256) - kSizeDiff};
                 SILK_TRACE << "KV: previous_key=" << to_hex(previous_key) << " key=" << to_hex(key)
                            << " nonexistent_key=" << to_hex(nonexistent_key);
-                if (const bool key_found = existence_index.contains(nonexistent_key); key_found) {
+                if (existence_index.contains(nonexistent_key)) {
                     ++nonexistent_found_count;
                 }
             }
             ++key_count;
         } else {
             value = *kv_iterator;
-            const bool key_found = existence_index.contains(key);
             SILK_DEBUG << "KV: key=" << to_hex(key) << " value=" << to_hex(value);
-            ensure(key_found,
+            ensure(existence_index.contains(key),
                    [&]() { return "open_existence_index: unexpected not found key=" + to_hex(key) +
                                   " position=" + std::to_string(key_count); });
             ++found_count;

silkworm/db/datastore/kvdb/cursor_iterator.hpp

@@ -61,7 +61,7 @@ class CursorIterator {
   private:
     void decode(const CursorResult& result);
     std::shared_ptr<ROCursor> cursor_;
-    MoveOperation move_op_{MoveOperation::first};
+    MoveOperation move_op_{MoveOperation::next};
     value_type decoders_;
 };
 

silkworm/db/datastore/kvdb/domain_delete_query.hpp

@@ -32,10 +32,18 @@ struct DomainDeleteQuery {
         const Key& key,
         Timestamp timestamp,
         const std::optional<Value>& prev_value,
-        Step prev_step) {
+        Step current_step) {
         if (prev_value) {
+            TValueEncoder prev_value_encoder;
+            prev_value_encoder.value = std::move(*prev_value);
+            Slice prev_value_slice = prev_value_encoder.encode();
+
+            RawDecoder<ByteView> prev_value_slice_decoder;
+            prev_value_slice_decoder.decode(prev_value_slice);
+            ByteView prev_value_data = prev_value_slice_decoder.value;
+
             DomainPutQuery<TKeyEncoder, RawEncoder<ByteView>> query{tx, entity};
-            query.exec(key, ByteView{}, timestamp, prev_value, prev_step);
+            query.exec(key, ByteView{}, timestamp, prev_value_data, current_step);
         }
     }
 };

silkworm/db/datastore/kvdb/domain_put_latest_query.hpp

@@ -51,7 +51,7 @@ struct DomainPutLatestQuery {
             same_step_value_encoder.value.timestamp.value = step;
             Slice same_step_value = same_step_value_encoder.encode();
 
-            CursorResult result = cursor->find_multivalue(key_data, same_step_value, false);
+            CursorResult result = cursor->lower_bound_multivalue(key_data, same_step_value, false);
             if (result) {
                 // the found value will have the same key, but the step part can be different,
                 // let's decode it ignoring the data part

silkworm/db/datastore/kvdb/domain_put_latest_query_test.cpp

@@ -0,0 +1,114 @@
+/*
+   Copyright 2024 The Silkworm Authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+#include "domain_put_latest_query.hpp"
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <silkworm/infra/common/directories.hpp>
+
+#include "big_endian_codec.hpp"
+#include "database.hpp"
+#include "domain_get_latest_query.hpp"
+
+namespace silkworm::datastore::kvdb {
+
+struct DomainPutEntry {
+    uint64_t key{0};
+    uint64_t value{0};
+    Step step{0};
+};
+
+using DomainGetQuery = DomainGetLatestQuery<BigEndianU64Codec, BigEndianU64Codec>;
+using Result = DomainGetQuery::Result;
+
+bool operator==(const Result& lhs, const Result& rhs) {
+    return (lhs.value == rhs.value) && (lhs.step == rhs.step);
+};
+
+TEST_CASE("DomainPutLatestQuery") {
+    const TemporaryDirectory tmp_dir;
+    ::mdbx::env_managed env = open_env(EnvConfig{.path = tmp_dir.path().string(), .create = true, .in_memory = true});
+
+    EntityName name{"Test"};
+    Schema::DatabaseDef schema;
+    schema.domain(name);
+
+    Database db{std::move(env), schema};
+    db.create_tables();
+    Domain entity = db.domain(name);
+    RWAccess db_access = db.access_rw();
+
+    auto find_in = [&db_access, &entity](std::vector<DomainPutEntry>&& data, uint64_t key) -> std::optional<Result> {
+        {
+            RWTxnManaged tx = db_access.start_rw_tx();
+            DomainPutLatestQuery<BigEndianU64Codec, BigEndianU64Codec> query{tx, entity};
+            for (auto& entry : data) {
+                query.exec(entry.key, entry.value, entry.step);
+            }
+            tx.commit_and_stop();
+        }
+
+        ROTxnManaged tx = db_access.start_ro_tx();
+        DomainGetQuery query{tx, entity};
+        return query.exec(key);
+    };
+
+    auto count = [&db_access, &entity]() -> uint64_t {
+        ROTxnManaged tx = db_access.start_ro_tx();
+        PooledCursor cursor{tx, entity.values_table};
+        return cursor.get_map_stat().ms_entries;
+    };
+
+    SECTION("single entry - correct key") {
+        CHECK(find_in({DomainPutEntry{1, 2, Step{3}}}, 1) == Result{2, Step{3}});
+        CHECK(count() == 1);
+    }
+    SECTION("single entry - wrong key") {
+        CHECK_FALSE(find_in({DomainPutEntry{1, 2, Step{3}}}, 4).has_value());
+        CHECK(count() == 1);
+    }
+    SECTION("different steps - different keys") {
+        CHECK(find_in({DomainPutEntry{1, 11, Step{101}}, DomainPutEntry{2, 22, Step{102}}, DomainPutEntry{3, 33, Step{103}}}, 2) == Result{22, Step{102}});
+        CHECK(count() == 3);
+    }
+    SECTION("ascending steps - same key") {
+        CHECK(find_in({DomainPutEntry{1, 11, Step{101}}, DomainPutEntry{1, 22, Step{102}}, DomainPutEntry{1, 33, Step{103}}}, 1) == Result{33, Step{103}});
+        CHECK(count() == 3);
+    }
+    SECTION("descending steps - same key") {
+        CHECK(find_in({DomainPutEntry{1, 33, Step{103}}, DomainPutEntry{1, 22, Step{102}}, DomainPutEntry{1, 11, Step{101}}}, 1) == Result{33, Step{103}});
+        CHECK(count() == 3);
+    }
+    SECTION("same step - different key") {
+        CHECK(find_in({DomainPutEntry{1, 11, Step{100}}, DomainPutEntry{2, 22, Step{100}}, DomainPutEntry{3, 33, Step{100}}}, 2) == Result{22, Step{100}});
+        CHECK(count() == 3);
+    }
+    SECTION("same step - same key") {
+        CHECK(find_in({DomainPutEntry{1, 11, Step{100}}, DomainPutEntry{1, 22, Step{100}}, DomainPutEntry{1, 33, Step{100}}}, 1) == Result{33, Step{100}});
+        CHECK(count() == 1);
+    }
+    SECTION("ascending and same steps - same key") {
+        CHECK(find_in({DomainPutEntry{1, 11, Step{101}}, DomainPutEntry{1, 22, Step{102}}, DomainPutEntry{1, 33, Step{103}}, DomainPutEntry{1, 331, Step{103}}}, 1) == Result{331, Step{103}});
+        CHECK(count() == 3);
+    }
+    SECTION("descending and same steps - same key") {
+        CHECK(find_in({DomainPutEntry{1, 33, Step{103}}, DomainPutEntry{1, 331, Step{103}}, DomainPutEntry{1, 22, Step{102}}, DomainPutEntry{1, 11, Step{101}}}, 1) == Result{331, Step{103}});
+        CHECK(count() == 3);
+    }
+}
+
+}  // namespace silkworm::datastore::kvdb

silkworm/db/datastore/kvdb/domain_put_query.hpp

@@ -35,9 +35,9 @@ struct DomainPutQuery {
         const Value& value,
         Timestamp timestamp,
         const std::optional<Value>& prev_value,
-        Step prev_step) {
+        Step current_step) {
         DomainPutLatestQuery<TKeyEncoder, TValueEncoder> value_query{tx, entity};
-        value_query.exec(key, value, prev_step);
+        value_query.exec(key, value, current_step);
 
         if (entity.history) {
             if (prev_value) {

silkworm/db/datastore/kvdb/inverted_index_range_by_key_query_test.cpp

@@ -22,10 +22,11 @@
 
 #include <catch2/catch_test_macros.hpp>
 
+#include <silkworm/infra/common/directories.hpp>
+
 #include "big_endian_codec.hpp"
 #include "database.hpp"
 #include "inverted_index_put_query.hpp"
-#include "silkworm/infra/common/directories.hpp"
 
 namespace silkworm::datastore::kvdb {
 

silkworm/db/datastore/snapshots/bloom_filter/bloom_filter.cpp

@@ -64,7 +64,7 @@ BloomFilter::BloomFilter(
     file_stream >> *this;
 
     path_ = std::move(path);
-    data_key_hasher_ = data_key_hasher;
+    data_key_hasher_ = std::move(data_key_hasher);
 }
 
 BloomFilter::BloomFilter()

silkworm/db/state/account_codecs.hpp

@@ -60,7 +60,7 @@ struct AccountSnapshotsCodec : public snapshots::Codec {
         auto account = AccountCodec::from_encoded_storage_v3(input_word);
         if (!account)
             throw DecodingException{account.error(), "AccountSnapshotsCodec failed to decode Account"};
-        value = *account;
+        value = std::move(*account);
     }
 };
 

silkworm/db/state/accounts_domain.hpp

@@ -22,12 +22,27 @@
 
 #include "account_codecs.hpp"
 #include "address_codecs.hpp"
+#include "schema_config.hpp"
 
 namespace silkworm::db::state {
 
-using AccountsDomainGetLatestQuery = datastore::DomainGetLatestQuery<
+using AccountsDomainGetLatestQueryBase = datastore::DomainGetLatestQuery<
     AddressKVDBEncoder, AddressSnapshotsEncoder,
     AccountKVDBCodec, AccountSnapshotsCodec>;
+
+struct AccountsDomainGetLatestQuery : public AccountsDomainGetLatestQueryBase {
+    AccountsDomainGetLatestQuery(
+        datastore::kvdb::DatabaseRef database,
+        datastore::kvdb::ROTxn& tx,
+        const snapshots::SnapshotRepositoryROAccess& repository)
+        : AccountsDomainGetLatestQueryBase{
+              db::state::kDomainNameAccounts,
+              std::move(database),
+              tx,
+              repository,
+          } {}
+};
+
 using AccountsDomainPutQuery = datastore::kvdb::DomainPutQuery<AddressKVDBEncoder, AccountKVDBCodec>;
 using AccountsDomainDeleteQuery = datastore::kvdb::DomainDeleteQuery<AddressKVDBEncoder, AccountKVDBCodec>;
 

silkworm/db/state/code_domain.hpp

@@ -22,12 +22,27 @@
 #include <silkworm/db/datastore/snapshots/segment/kv_segment_reader.hpp>
 
 #include "address_codecs.hpp"
+#include "schema_config.hpp"
 
 namespace silkworm::db::state {
 
-using CodeDomainGetLatestQuery = datastore::DomainGetLatestQuery<
+using CodeDomainGetLatestQueryBase = datastore::DomainGetLatestQuery<
     AddressKVDBEncoder, AddressSnapshotsEncoder,
     datastore::kvdb::RawDecoder<ByteView>, snapshots::RawDecoder<ByteView>>;
+
+struct CodeDomainGetLatestQuery : public CodeDomainGetLatestQueryBase {
+    CodeDomainGetLatestQuery(
+        datastore::kvdb::DatabaseRef database,
+        datastore::kvdb::ROTxn& tx,
+        const snapshots::SnapshotRepositoryROAccess& repository)
+        : CodeDomainGetLatestQueryBase{
+              db::state::kDomainNameCode,
+              std::move(database),
+              tx,
+              repository,
+          } {}
+};
+
 using CodeDomainPutQuery = datastore::kvdb::DomainPutQuery<AddressKVDBEncoder, datastore::kvdb::RawEncoder<ByteView>>;
 using CodeDomainDeleteQuery = datastore::kvdb::DomainDeleteQuery<AddressKVDBEncoder, datastore::kvdb::RawEncoder<ByteView>>;
 

silkworm/db/state/commitment_domain.hpp

@@ -21,11 +21,27 @@
 #include <silkworm/db/datastore/snapshots/common/raw_codec.hpp>
 #include <silkworm/db/datastore/snapshots/segment/kv_segment_reader.hpp>
 
+#include "schema_config.hpp"
+
 namespace silkworm::db::state {
 
-using CommitmentDomainGetLatestQuery = datastore::DomainGetLatestQuery<
+using CommitmentDomainGetLatestQueryBase = datastore::DomainGetLatestQuery<
     datastore::kvdb::RawEncoder<ByteView>, snapshots::RawEncoder<ByteView>,
     datastore::kvdb::RawDecoder<Bytes>, snapshots::RawDecoder<Bytes>>;
+
+struct CommitmentDomainGetLatestQuery : public CommitmentDomainGetLatestQueryBase {
+    CommitmentDomainGetLatestQuery(
+        datastore::kvdb::DatabaseRef database,
+        datastore::kvdb::ROTxn& tx,
+        const snapshots::SnapshotRepositoryROAccess& repository)
+        : CommitmentDomainGetLatestQueryBase{
+              db::state::kDomainNameCommitment,
+              std::move(database),
+              tx,
+              repository,
+          } {}
+};
+
 using CommitmentDomainPutQuery = datastore::kvdb::DomainPutQuery<datastore::kvdb::RawEncoder<ByteView>, datastore::kvdb::RawEncoder<ByteView>>;
 using CommitmentDomainDeleteQuery = datastore::kvdb::DomainDeleteQuery<datastore::kvdb::RawEncoder<ByteView>, datastore::kvdb::RawEncoder<ByteView>>;
 

silkworm/db/state/receipts_domain.hpp

@@ -23,11 +23,27 @@
 #include <silkworm/db/datastore/snapshots/segment/kv_segment_reader.hpp>
 #include <silkworm/db/datastore/snapshots/segment/seg/common/varint.hpp>
 
+#include "schema_config.hpp"
+
 namespace silkworm::db::state {
 
-using ReceiptsDomainGetLatestQuery = datastore::DomainGetLatestQuery<
+using ReceiptsDomainGetLatestQueryBase = datastore::DomainGetLatestQuery<
     datastore::kvdb::RawEncoder<ByteView>, snapshots::RawEncoder<ByteView>,
     datastore::kvdb::RawDecoder<Bytes>, snapshots::RawDecoder<Bytes>>;
+
+struct ReceiptsDomainGetLatestQuery : public ReceiptsDomainGetLatestQueryBase {
+    ReceiptsDomainGetLatestQuery(
+        datastore::kvdb::DatabaseRef database,
+        datastore::kvdb::ROTxn& tx,
+        const snapshots::SnapshotRepositoryROAccess& repository)
+        : ReceiptsDomainGetLatestQueryBase{
+              db::state::kDomainNameReceipts,
+              std::move(database),
+              tx,
+              repository,
+          } {}
+};
+
 using ReceiptsDomainPutQuery = datastore::kvdb::DomainPutQuery<datastore::kvdb::RawEncoder<ByteView>, datastore::kvdb::RawEncoder<ByteView>>;
 using ReceiptsDomainDeleteQuery = datastore::kvdb::DomainDeleteQuery<datastore::kvdb::RawEncoder<ByteView>, datastore::kvdb::RawEncoder<ByteView>>;
 

silkworm/db/state/storage_domain.hpp

@@ -20,13 +20,28 @@
 #include <silkworm/db/datastore/kvdb/domain_queries.hpp>
 #include <silkworm/db/datastore/snapshots/segment/kv_segment_reader.hpp>
 
+#include "schema_config.hpp"
 #include "storage_codecs.hpp"
 
 namespace silkworm::db::state {
 
-using StorageDomainGetLatestQuery = datastore::DomainGetLatestQuery<
+using StorageDomainGetLatestQueryBase = datastore::DomainGetLatestQuery<
     StorageAddressAndLocationKVDBEncoder, StorageAddressAndLocationSnapshotsCodec,
     Bytes32KVDBCodec, Bytes32SnapshotsCodec>;
+
+struct StorageDomainGetLatestQuery : public StorageDomainGetLatestQueryBase {
+    StorageDomainGetLatestQuery(
+        datastore::kvdb::DatabaseRef database,
+        datastore::kvdb::ROTxn& tx,
+        const snapshots::SnapshotRepositoryROAccess& repository)
+        : StorageDomainGetLatestQueryBase{
+              db::state::kDomainNameStorage,
+              std::move(database),
+              tx,
+              repository,
+          } {}
+};
+
 using StorageDomainPutQuery = datastore::kvdb::DomainPutQuery<StorageAddressAndLocationKVDBEncoder, Bytes32KVDBCodec>;
 using StorageDomainDeleteQuery = datastore::kvdb::DomainDeleteQuery<StorageAddressAndLocationKVDBEncoder, Bytes32KVDBCodec>;