diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 99f96ebcaa2..26a6693011f 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -283,17 +283,16 @@ namespace loggingapp .set_require_client_identity(false); install(Procs::LOG_RECORD_RAW_TEXT, log_record_text, Write); - nwt.signatures.set_global_hook([this, ¬ifier]( - kv::Version version, - const ccf::Signatures::State& s, - const ccf::Signatures::Write& w) { - if (w.size() > 0) - { - nlohmann::json notify_j; - notify_j["commit"] = version; - notifier.notify(jsonrpc::pack(notify_j, jsonrpc::Pack::Text)); - } - }); + nwt.signatures.set_global_hook( + [this, + ¬ifier](kv::Version version, const ccf::Signatures::Write& w) { + if (w.size() > 0) + { + nlohmann::json notify_j; + notify_j["commit"] = version; + notifier.notify(jsonrpc::pack(notify_j, jsonrpc::Pack::Text)); + } + }); } }; diff --git a/src/consensus/pbft/pbft.h b/src/consensus/pbft/pbft.h index eb798b49e75..5b6d216f8e6 100644 --- a/src/consensus/pbft/pbft.h +++ b/src/consensus/pbft/pbft.h @@ -573,7 +573,7 @@ namespace pbft void add_configuration( SeqNo seqno, - std::unordered_set config, + const std::unordered_set& config, const NodeConf& node_conf) override { if (node_conf.node_id == local_id) @@ -594,6 +594,11 @@ namespace pbft nodes[node_conf.node_id] = 0; } + std::unordered_set get_latest_configuration() const override + { + throw std::logic_error("Unimplemented"); + } + void periodic(std::chrono::milliseconds elapsed) override { client_proxy->periodic(elapsed); diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 459dbdd522b..a57fac413a0 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -302,13 +302,23 @@ namespace raft return get_term_internal(idx); } - void add_configuration(Index idx, std::unordered_set conf) + void add_configuration(Index idx, const std::unordered_set& conf) { // This should only be called when the spin lock is held. configurations.push_back({idx, move(conf)}); create_and_remove_node_state(); } + std::unordered_set get_latest_configuration() const + { + if (configurations.empty()) + { + return {}; + } + + return configurations.back().nodes; + } + template size_t write_to_ledger(const T& data) { diff --git a/src/consensus/raft/raft_consensus.h b/src/consensus/raft/raft_consensus.h index 58b5e308910..1d44cc2d1e2 100644 --- a/src/consensus/raft/raft_consensus.h +++ b/src/consensus/raft/raft_consensus.h @@ -81,12 +81,17 @@ namespace raft void add_configuration( SeqNo seqno, - std::unordered_set conf, + const std::unordered_set& conf, const NodeConf& node_conf = {}) override { raft->add_configuration(seqno, conf); } + std::unordered_set get_latest_configuration() const override + { + return raft->get_latest_configuration(); + } + void periodic(std::chrono::milliseconds elapsed) override { raft->periodic(elapsed); diff --git a/src/kv/experimental.h b/src/kv/experimental.h new file mode 100644 index 00000000000..36b38acc0f8 --- /dev/null +++ b/src/kv/experimental.h @@ -0,0 +1,325 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "ds/hash.h" +#include "kv_types.h" +#include "map.h" +#include "tx_view.h" + +#include + +namespace kv +{ + namespace experimental + { + using SerialisedRep = std::vector; + + using RepHasher = std::hash; + + using UntypedMap = kv::Map; + + using UntypedOperationsView = + kv::TxView; + + using UntypedCommitter = + kv::TxViewCommitter; + + using UntypedState = kv::State; + + template + struct MsgPackSerialiser + { + static SerialisedRep to_serialised(const T& t) + { + msgpack::sbuffer sb; + msgpack::pack(sb, t); + auto sb_data = reinterpret_cast(sb.data()); + return SerialisedRep(sb_data, sb_data + sb.size()); + } + + static T from_serialised(const SerialisedRep& rep) + { + msgpack::object_handle oh = msgpack::unpack( + reinterpret_cast(rep.data()), rep.size()); + auto object = oh.get(); + return object.as(); + } + }; + + template + struct JsonSerialiser + { + static SerialisedRep to_serialised(const T& t) + { + const nlohmann::json j = t; + const auto dumped = j.dump(); + return SerialisedRep(dumped.begin(), dumped.end()); + } + + static T from_serialised(const SerialisedRep& rep) + { + const auto j = nlohmann::json::parse(rep); + return j.get(); + } + }; + + template < + typename K, + typename V, + typename KSerialiser = MsgPackSerialiser, + typename VSerialiser = MsgPackSerialiser> + class TxView : public UntypedCommitter + { + protected: + // This _has_ a (non-visible, untyped) view, whereas the standard impl + // _is_ a typed view + UntypedOperationsView untyped_view; + + public: + using KeyType = K; + using ValueType = V; + + TxView( + UntypedMap& m, + size_t rollbacks, + UntypedState& current_state, + UntypedState& committed_state, + Version v) : + UntypedCommitter(m, rollbacks, current_state, committed_state, v), + untyped_view(UntypedCommitter::change_set) + {} + + std::optional get(const K& key) + { + const auto k_rep = KSerialiser::to_serialised(key); + const auto opt_v_rep = untyped_view.get(k_rep); + + if (opt_v_rep.has_value()) + { + return VSerialiser::from_serialised(*opt_v_rep); + } + + return std::nullopt; + } + + std::optional get_globally_committed(const K& key) + { + const auto k_rep = KSerialiser::to_serialised(key); + const auto opt_v_rep = untyped_view.get_globally_committed(k_rep); + + if (opt_v_rep.has_value()) + { + return VSerialiser::from_serialised(*opt_v_rep); + } + + return std::nullopt; + } + + bool put(const K& key, const V& value) + { + const auto k_rep = KSerialiser::to_serialised(key); + const auto v_rep = VSerialiser::to_serialised(value); + + return untyped_view.put(k_rep, v_rep); + } + + bool remove(const K& key) + { + const auto k_rep = KSerialiser::to_serialised(key); + + return untyped_view.remove(k_rep); + } + + template + bool foreach(F&& f) + { + auto g = [&](const SerialisedRep& k_rep, const SerialisedRep& v_rep) { + return f( + KSerialiser::from_serialised(k_rep), + VSerialiser::from_serialised(v_rep)); + }; + return untyped_view.foreach(g); + } + }; + + template < + typename K, + typename V, + typename KSerialiser = MsgPackSerialiser, + typename VSerialiser = MsgPackSerialiser> + class Map : public AbstractMap + { + protected: + using This = Map; + + UntypedMap untyped_map; + + public: + // Expose correct public aliases of types + using VersionV = VersionV; + + using Write = Write; + + using CommitHook = CommitHook; + + using TxView = kv::experimental::TxView; + + template + Map(Ts&&... ts) : untyped_map(std::forward(ts)...) + {} + + bool operator==(const AbstractMap& that) const override + { + auto p = dynamic_cast(&that); + if (p == nullptr) + { + return false; + } + + return untyped_map == p->untyped_map; + } + + bool operator!=(const AbstractMap& that) const override + { + return !(*this == that); + } + + AbstractStore* get_store() override + { + return untyped_map.get_store(); + } + + void serialise( + const AbstractTxView* view, + KvStoreSerialiser& s, + bool include_reads) override + { + untyped_map.serialise(view, s, include_reads); + } + + AbstractTxView* deserialise( + KvStoreDeserialiser& d, Version version) override + { + return untyped_map.deserialise(d, version); + } + + const std::string& get_name() const override + { + return untyped_map.get_name(); + } + + void compact(Version v) override + { + return untyped_map.compact(v); + } + + void post_compact() override + { + return untyped_map.post_compact(); + } + + void rollback(Version v) override + { + untyped_map.rollback(v); + } + + void lock() override + { + untyped_map.lock(); + } + + void unlock() override + { + untyped_map.unlock(); + } + + SecurityDomain get_security_domain() override + { + return untyped_map.get_security_domain(); + } + + bool is_replicated() override + { + return untyped_map.is_replicated(); + } + + void clear() override + { + untyped_map.clear(); + } + + AbstractMap* clone(AbstractStore* store) override + { + return new Map( + store, + untyped_map.get_name(), + untyped_map.get_security_domain(), + untyped_map.is_replicated()); + } + + void swap(AbstractMap* map) override + { + auto p = dynamic_cast(map); + if (p == nullptr) + throw std::logic_error( + "Attempted to swap maps with incompatible types"); + + untyped_map.swap(&p->untyped_map); + } + + template + TView* create_view(Version v) + { + return untyped_map.create_view(v); + } + + static UntypedMap::CommitHook wrap_commit_hook(const CommitHook& hook) + { + return [hook](Version v, const UntypedMap::Write& w) { + Write typed_w; + for (const auto& [uk, version_uv] : w) + { + if (version_uv.version == NoVersion) + { + // Deletions are indicated by {NoVersion, {}}. The second + // element in the serialised representation is an empty vector, + // which may not be safely serialisable! So we duplicate the old + // behaviour and use default-constructed V + typed_w[KSerialiser::from_serialised(uk)] = + VersionV{NoVersion, V{}}; + } + else + { + typed_w[KSerialiser::from_serialised(uk)] = + VersionV{version_uv.version, + VSerialiser::from_serialised(version_uv.value)}; + } + } + + hook(v, typed_w); + }; + } + + void set_local_hook(const CommitHook& hook) + { + untyped_map.set_local_hook(wrap_commit_hook(hook)); + } + + void unset_local_hook() + { + untyped_map.unset_local_hook(); + } + + void set_global_hook(const CommitHook& hook) + { + untyped_map.set_global_hook(wrap_commit_hook(hook)); + } + + void unset_global_hook() + { + untyped_map.unset_global_hook(); + } + }; + } +} \ No newline at end of file diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 81a35718512..12ab5749c11 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -227,8 +227,9 @@ namespace kv virtual void recv_message(OArray&& oa) = 0; virtual void add_configuration( SeqNo seqno, - std::unordered_set conf, + const std::unordered_set& conf, const NodeConf& node_conf = {}) = 0; + virtual std::unordered_set get_latest_configuration() const = 0; virtual bool on_request(const kv::TxHistory::RequestCallbackArgs& args) { @@ -358,17 +359,11 @@ namespace kv public: virtual ~AbstractTxView() = default; - // Commit-related methods virtual bool has_writes() = 0; virtual bool has_changes() = 0; virtual bool prepare() = 0; virtual void commit(Version v) = 0; virtual void post_commit() = 0; - - // Serialisation-related methods - virtual void serialise(KvStoreSerialiser& s, bool include_reads) = 0; - virtual bool deserialise(KvStoreDeserialiser& d, Version version) = 0; - virtual bool is_replicated() = 0; }; class AbstractMap @@ -379,7 +374,10 @@ namespace kv virtual bool operator!=(const AbstractMap& that) const = 0; virtual AbstractStore* get_store() = 0; - virtual AbstractTxView* create_view(Version version) = 0; + virtual void serialise( + const AbstractTxView* view, KvStoreSerialiser& s, bool include_reads) = 0; + virtual AbstractTxView* deserialise( + KvStoreDeserialiser& d, Version version) = 0; virtual const std::string& get_name() const = 0; virtual void compact(Version v) = 0; virtual void post_compact() = 0; diff --git a/src/kv/map.h b/src/kv/map.h index 568cde4b29f..30e8afd9c42 100644 --- a/src/kv/map.h +++ b/src/kv/map.h @@ -52,44 +52,33 @@ namespace kv class Map; template - class ConcreteTxView : public TxView, public AbstractTxView + class TxViewCommitter : public AbstractTxView { protected: - using Base = TxView; - using State = typename Base::State; - using MyMap = Map; - using Base::read_version; - using Base::reads; - using Base::start_version; - using Base::writes; + ChangeSet change_set; MyMap& map; size_t rollback_counter; Version commit_version = NoVersion; + bool changes = false; bool committed_writes = false; public: - ConcreteTxView( - State& current_state, - State& committed_state, - Version v, - MyMap& m, - size_t rollbacks) : - Base(current_state, committed_state, v), + template + TxViewCommitter(MyMap& m, size_t rollbacks, Ts&&... ts) : map(m), - rollback_counter(rollbacks) + rollback_counter(rollbacks), + change_set(std::forward(ts)...) {} - ConcreteTxView(ConcreteTxView&) = delete; - // Commit-related methods bool has_writes() override { - return committed_writes || !writes.empty(); + return committed_writes || !change_set.writes.empty(); } bool has_changes() override @@ -99,7 +88,7 @@ namespace kv bool prepare() override { - if (writes.empty()) + if (change_set.writes.empty()) return true; // If the parent map has rolled back since this transaction began, this @@ -110,14 +99,17 @@ namespace kv // If we have iterated over the map, check for a global version match. auto current = map.roll->get_tail(); - if ((read_version != NoVersion) && (read_version != current->version)) + if ( + (change_set.read_version != NoVersion) && + (change_set.read_version != current->version)) { - LOG_DEBUG_FMT("Read version {} is invalid", read_version); + LOG_DEBUG_FMT("Read version {} is invalid", change_set.read_version); return false; } // Check each key in our read set. - for (auto it = reads.begin(); it != reads.end(); ++it) + for (auto it = change_set.reads.begin(); it != change_set.reads.end(); + ++it) { // Get the value from the current state. auto search = current->state.get(it->first); @@ -148,9 +140,9 @@ namespace kv void commit(Version v) override { - if (writes.empty()) + if (change_set.writes.empty()) { - commit_version = start_version; + commit_version = change_set.start_version; return; } @@ -158,11 +150,12 @@ namespace kv commit_version = v; committed_writes = true; - if (!writes.empty()) + if (!change_set.writes.empty()) { auto state = map.roll->get_tail()->state; - for (auto it = writes.begin(); it != writes.end(); ++it) + for (auto it = change_set.writes.begin(); it != change_set.writes.end(); + ++it) { if (it->second.version >= 0) { @@ -185,7 +178,8 @@ namespace kv if (changes) { - map.roll->insert_back(map.create_new_local_commit(v, state, writes)); + map.roll->insert_back( + map.create_new_local_commit(v, state, change_set.writes)); } } } @@ -195,115 +189,52 @@ namespace kv // This is run separately from commit so that all commits in the Tx // have been applied before local hooks are run. The maps in the Tx // are still locked when post_commit is run. - if (writes.empty()) + if (change_set.writes.empty()) return; if (map.local_hook) { auto roll = map.roll->get_tail(); - map.local_hook(roll->version, roll->state, roll->writes); + map.local_hook(roll->version, roll->writes); } } - // Serialisation-related methods - void serialise(KvStoreSerialiser& s, bool include_reads) override + // Used by owning map during serialise and deserialise + ChangeSet& get_change_set() { - if (!changes) - return; - - s.start_map(map.name, map.get_security_domain()); - - if (include_reads) - { - s.serialise_read_version(read_version); - - s.serialise_count_header(reads.size()); - for (auto it = reads.begin(); it != reads.end(); ++it) - s.serialise_read(it->first, it->second); - } - else - { - s.serialise_read_version(NoVersion); - s.serialise_count_header(0); - } - - uint64_t write_ctr = 0; - uint64_t remove_ctr = 0; - for (auto it = writes.begin(); it != writes.end(); ++it) - { - if (!is_deleted(it->second.version)) - { - ++write_ctr; - } - else - { - auto search = map.roll->get_tail()->state.get(it->first); - if (search.has_value()) - ++remove_ctr; - } - } - s.serialise_count_header(write_ctr); - for (auto it = writes.begin(); it != writes.end(); ++it) - { - if (!is_deleted(it->second.version)) - { - s.serialise_write(it->first, it->second.value); - } - } - - s.serialise_count_header(remove_ctr); - for (auto it = writes.begin(); it != writes.end(); ++it) - { - if (is_deleted(it->second.version)) - { - s.serialise_remove(it->first); - } - } + return change_set; } - bool deserialise(KvStoreDeserialiser& d, Version version) override + const ChangeSet& get_change_set() const { - commit_version = version; - uint64_t ctr; - - auto rv = d.template deserialise_read_version(); - if (rv != NoVersion) - read_version = rv; - - ctr = d.deserialise_read_header(); - for (size_t i = 0; i < ctr; ++i) - { - auto r = d.template deserialise_read(); - reads[std::get<0>(r)] = std::get<1>(r); - } - - ctr = d.deserialise_write_header(); - for (size_t i = 0; i < ctr; ++i) - { - auto w = d.template deserialise_write(); - writes[std::get<0>(w)] = {0, std::get<1>(w)}; - } - - ctr = d.deserialise_remove_header(); - for (size_t i = 0; i < ctr; ++i) - { - auto r = d.template deserialise_remove(); - writes[r] = {NoVersion, V()}; - } - - return true; + return change_set; } - bool is_replicated() override + void set_commit_version(Version v) { - return map.is_replicated(); + commit_version = v; } }; - /// Signature for transaction commit handlers template - using CommitHook = - std::function&, const Write&)>; + struct ConcreteTxView : public TxViewCommitter, + public TxView + { + public: + ConcreteTxView( + Map& m, + size_t rollbacks, + State& current_state, + State& committed_state, + Version v) : + TxViewCommitter(m, rollbacks, current_state, committed_state, v), + TxView(TxViewCommitter::change_set) + {} + }; + + /// Signature for transaction commit handlers + template + using CommitHook = std::function; template class Map : public AbstractMap @@ -312,7 +243,7 @@ namespace kv using VersionV = VersionV; using State = State; using Write = Write; - using CommitHook = CommitHook; + using CommitHook = CommitHook; private: using This = Map; @@ -369,6 +300,9 @@ namespace kv // Public typedef for external consumption using TxView = ConcreteTxView; + // Provide access to hidden rollback_counter, roll, create_new_local_commit + friend TxViewCommitter; + Map( AbstractStore* store_, std::string name_, @@ -391,6 +325,121 @@ namespace kv return new Map(other, name, security_domain, replicated); } + void serialise( + const AbstractTxView* view, + KvStoreSerialiser& s, + bool include_reads) override + { + const auto committer = + dynamic_cast*>(view); + if (committer == nullptr) + { + LOG_FAIL_FMT("Unable to serialise map due to type mismatch"); + return; + } + + const auto& change_set = committer->get_change_set(); + + s.start_map(name, security_domain); + + if (include_reads) + { + s.serialise_read_version(change_set.read_version); + + s.serialise_count_header(change_set.reads.size()); + for (auto it = change_set.reads.begin(); it != change_set.reads.end(); + ++it) + { + s.serialise_read(it->first, it->second); + } + } + else + { + s.serialise_read_version(NoVersion); + s.serialise_count_header(0); + } + + uint64_t write_ctr = 0; + uint64_t remove_ctr = 0; + for (auto it = change_set.writes.begin(); it != change_set.writes.end(); + ++it) + { + if (!is_deleted(it->second.version)) + { + ++write_ctr; + } + else + { + auto search = roll->get_tail()->state.get(it->first); + if (search.has_value()) + { + ++remove_ctr; + } + } + } + + s.serialise_count_header(write_ctr); + for (auto it = change_set.writes.begin(); it != change_set.writes.end(); + ++it) + { + if (!is_deleted(it->second.version)) + { + s.serialise_write(it->first, it->second.value); + } + } + + s.serialise_count_header(remove_ctr); + for (auto it = change_set.writes.begin(); it != change_set.writes.end(); + ++it) + { + if (is_deleted(it->second.version)) + { + s.serialise_remove(it->first); + } + } + } + + AbstractTxView* deserialise( + KvStoreDeserialiser& d, Version version) override + { + // Create a new change set, and deserialise d's contents into it. + auto view = create_view(version); + view->set_commit_version(version); + + auto& change_set = view->get_change_set(); + + uint64_t ctr; + + auto rv = d.template deserialise_read_version(); + if (rv != NoVersion) + { + change_set.read_version = rv; + } + + ctr = d.deserialise_read_header(); + for (size_t i = 0; i < ctr; ++i) + { + auto r = d.template deserialise_read(); + change_set.reads[std::get<0>(r)] = std::get<1>(r); + } + + ctr = d.deserialise_write_header(); + for (size_t i = 0; i < ctr; ++i) + { + auto w = d.template deserialise_write(); + change_set.writes[std::get<0>(w)] = {0, std::get<1>(w)}; + } + + ctr = d.deserialise_remove_header(); + for (size_t i = 0; i < ctr; ++i) + { + auto r = d.template deserialise_remove(); + change_set.writes[r] = {NoVersion, V()}; + } + + return view; + } + /** Get the name of the map * * @return const std::string& @@ -413,7 +462,7 @@ namespace kv * * @param hook function to be called on local transaction commit */ - void set_local_hook(CommitHook hook) + void set_local_hook(const CommitHook& hook) { std::lock_guard guard(sl); local_hook = hook; @@ -431,7 +480,7 @@ namespace kv * * @param hook function to be called on global transaction commit */ - void set_global_hook(CommitHook hook) + void set_global_hook(const CommitHook& hook) { std::lock_guard guard(sl); global_hook = hook; @@ -521,46 +570,6 @@ namespace kv return !(*this == that); } - AbstractTxView* create_view(Version version) override - { - lock(); - - // Find the last entry committed at or before this version. - AbstractTxView* view = nullptr; - - for (auto current = roll->get_tail(); current != nullptr; - current = current->prev) - { - if (current->version <= version) - { - view = new ConcreteTxView( - current->state, - roll->get_head()->state, - current->version, - *this, - rollback_counter); - break; - } - } - - if (view == nullptr) - { - view = new ConcreteTxView( - roll->get_head()->state, - roll->get_head()->state, - roll->get_head()->version, - *this, - rollback_counter); - } - - unlock(); - return view; - } - - private: - // Provides access to private rollback_counter and roll - friend ConcreteTxView; - void compact(Version v) override { // This discards available rollback state before version v, and populates @@ -614,7 +623,7 @@ namespace kv { for (auto r = commit_deltas.get_head(); r != nullptr; r = r->next) { - global_hook(r->version, r->state, r->writes); + global_hook(r->version, r->writes); } } @@ -674,5 +683,42 @@ namespace kv std::swap(rollback_counter, map->rollback_counter); std::swap(roll, map->roll); } + + template + TView* create_view(Version version) + { + lock(); + + // Find the last entry committed at or before this version. + TView* view = nullptr; + + for (auto current = roll->get_tail(); current != nullptr; + current = current->prev) + { + if (current->version <= version) + { + view = new TView( + *this, + rollback_counter, + current->state, + roll->get_head()->state, + current->version); + break; + } + } + + if (view == nullptr) + { + view = new TView( + *this, + rollback_counter, + roll->get_head()->state, + roll->get_head()->state, + roll->get_head()->version); + } + + unlock(); + return view; + } }; } \ No newline at end of file diff --git a/src/kv/store.h b/src/kv/store.h index 94c10063498..be539669216 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -50,14 +50,14 @@ namespace kv } public: - void clone_schema(Store& target) + void clone_schema(Store& from) { std::lock_guard mguard(maps_lock); if ((maps.size() != 0) || (version != 0)) throw std::logic_error("Cannot clone schema on a non-empty store"); - for (auto& [name, map] : target.maps) + for (auto& [name, map] : from.maps) { maps[name] = std::unique_ptr(map->clone(this)); } @@ -336,12 +336,13 @@ namespace kv return DeserialiseSuccess::FAILED; } - auto view = search->second->create_view(v); // if we are not committing now then use NoVersion to deserialise // otherwise the view will be considered as having a committed // version auto deserialise_version = (commit ? v : NoVersion); - if (!view->deserialise(*d, deserialise_version)) + auto deserialised_write_set = + search->second->deserialise(*d, deserialise_version); + if (deserialised_write_set == nullptr) { LOG_FAIL_FMT( "Could not deserialise Tx for map {} at version {}", @@ -350,8 +351,11 @@ namespace kv return DeserialiseSuccess::FAILED; } - views[map_name] = {search->second.get(), - std::unique_ptr(view)}; + // Take ownership of the produced write set, store it to be committed + // later + views[map_name] = { + search->second.get(), + std::unique_ptr(deserialised_write_set)}; } if (!d->end()) diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index dadb34893be..6ffbc304694 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -2,6 +2,7 @@ // Licensed under the Apache 2.0 License. #include "ds/logger.h" #include "kv/encryptor.h" +#include "kv/experimental.h" #include "kv/kv_serialiser.h" #include "kv/store.h" #include "kv/test/null_encryptor.h" @@ -13,70 +14,42 @@ #include #include -struct CustomClass +struct RawMapTypes { - int m_i; - - CustomClass() : CustomClass(-1) {} - CustomClass(int i) : m_i(i) {} - - int get() const - { - return m_i; - } - void set(std::string val) - { - m_i = std::stoi(val); - } - - CustomClass operator()() - { - CustomClass ret; - return ret; - } - - bool operator<(const CustomClass& other) const - { - return m_i < other.m_i; - } - - bool operator==(const CustomClass& other) const - { - return !(other < *this) && !(*this < other); - } - - MSGPACK_DEFINE(m_i); + using StringString = kv::Map; + using NumNum = kv::Map; + using NumString = kv::Map; + using StringNum = kv::Map; }; -namespace std +struct ExperimentalMapTypes { - template <> - struct hash - { - std::size_t operator()(const CustomClass& inst) const - { - return inst.get(); - } - }; -} - -DECLARE_JSON_TYPE(CustomClass) -DECLARE_JSON_REQUIRED_FIELDS(CustomClass, m_i) + using StringString = kv::experimental::Map; + using NumNum = kv::experimental::Map; + using NumString = kv::experimental::Map; + using StringNum = kv::experimental::Map; +}; -TEST_CASE( +TEST_CASE_TEMPLATE( "Serialise/deserialise public map only" * - doctest::test_suite("serialisation")) + doctest::test_suite("serialisation"), + MapImpl, + RawMapTypes, + ExperimentalMapTypes) { // No need for an encryptor here as all maps are public. Both serialisation // and deserialisation should succeed. auto consensus = std::make_shared(); kv::Store kv_store(consensus); - kv::Store kv_store_target; - auto& pub_map = kv_store.create( + auto& pub_map = kv_store.create( "pub_map", kv::SecurityDomain::PUBLIC); + + kv::Store kv_store_target; kv_store_target.clone_schema(kv_store); + auto* target_map = kv_store.get("pub_map"); + REQUIRE(target_map != nullptr); INFO("Commit to public map in source store"); { @@ -88,30 +61,36 @@ TEST_CASE( INFO("Deserialise transaction in target store"); { + REQUIRE(consensus->get_latest_data().second); + REQUIRE(!consensus->get_latest_data().first.empty()); REQUIRE( kv_store_target.deserialise(consensus->get_latest_data().first) == kv::DeserialiseSuccess::PASS); kv::Tx tx_target; - auto view_target = tx_target.get_view( - *kv_store_target.get("pub_map")); + auto view_target = tx_target.get_view(*target_map); REQUIRE(view_target->get("pubk1") == "pubv1"); } } -TEST_CASE( +TEST_CASE_TEMPLATE( "Serialise/deserialise private map only" * - doctest::test_suite("serialisation")) + doctest::test_suite("serialisation"), + MapImpl, + RawMapTypes, + ExperimentalMapTypes) { auto consensus = std::make_shared(); auto encryptor = std::make_shared(); kv::Store kv_store(consensus); + auto& priv_map = kv_store.create("priv_map"); + kv::Store kv_store_target; kv_store_target.set_encryptor(encryptor); - - auto& priv_map = kv_store.create("priv_map"); kv_store_target.clone_schema(kv_store); + auto* target_map = kv_store.get("priv_map"); + REQUIRE(target_map != nullptr); INFO("Commit a private transaction without an encryptor throws an exception"); { @@ -142,28 +121,36 @@ TEST_CASE( kv::DeserialiseSuccess::PASS); kv::Tx tx_target; - auto view_target = tx_target.get_view( - *kv_store_target.get("priv_map")); + auto view_target = tx_target.get_view(*target_map); REQUIRE(view_target->get("privk1") == "privv1"); } } -TEST_CASE( - "Serialise/deserialise private and public maps" * - doctest::test_suite("serialisation")) +TEST_CASE_TEMPLATE( + "Serialise/deserialise private map and public maps" * + doctest::test_suite("serialisation"), + MapImpl, + RawMapTypes, + ExperimentalMapTypes) { auto consensus = std::make_shared(); auto encryptor = std::make_shared(); kv::Store kv_store(consensus); - kv::Store kv_store_target; kv_store.set_encryptor(encryptor); - kv_store_target.set_encryptor(encryptor); - - auto& priv_map = kv_store.create("priv_map"); - auto& pub_map = kv_store.create( + auto& priv_map = kv_store.create("priv_map"); + auto& pub_map = kv_store.create( "pub_map", kv::SecurityDomain::PUBLIC); + + kv::Store kv_store_target; + kv_store_target.set_encryptor(encryptor); kv_store_target.clone_schema(kv_store); + auto* target_priv_map = + kv_store.get("priv_map"); + auto* target_pub_map = + kv_store.get("pub_map"); + REQUIRE(target_priv_map != nullptr); + REQUIRE(target_pub_map != nullptr); INFO("Commit to public and private map in source store"); { @@ -183,28 +170,32 @@ TEST_CASE( kv::DeserialiseSuccess::FAILED); kv::Tx tx; - auto [view_priv, view_pub] = tx.get_view( - *kv_store_target.get("priv_map"), - *kv_store_target.get("pub_map")); + auto [view_priv, view_pub] = tx.get_view(*target_priv_map, *target_pub_map); REQUIRE(view_priv->get("privk1") == "privv1"); REQUIRE(view_pub->get("pubk1") == "pubv1"); } } -TEST_CASE( - "Serialise/deserialise removed keys" * doctest::test_suite("serialisation")) +TEST_CASE_TEMPLATE( + "Serialise/deserialise removed keys" * doctest::test_suite("serialisation"), + MapImpl, + RawMapTypes, + ExperimentalMapTypes) { auto consensus = std::make_shared(); auto encryptor = std::make_shared(); kv::Store kv_store(consensus); - kv::Store kv_store_target; kv_store.set_encryptor(encryptor); - kv_store_target.set_encryptor(encryptor); + auto& priv_map = kv_store.create("priv_map"); - auto& priv_map = kv_store.create("priv_map"); + kv::Store kv_store_target; + kv_store_target.set_encryptor(encryptor); kv_store_target.clone_schema(kv_store); + auto* target_priv_map = + kv_store.get("priv_map"); + REQUIRE(target_priv_map != nullptr); INFO("Commit a new key in source store and deserialise in target store"); { @@ -218,8 +209,7 @@ TEST_CASE( kv::DeserialiseSuccess::FAILED); kv::Tx tx_target; - auto view_priv_target = tx_target.get_view( - *kv_store_target.get("priv_map")); + auto view_priv_target = tx_target.get_view(*target_priv_map); REQUIRE(view_priv_target->get("privk1") == "privv1"); } @@ -240,14 +230,64 @@ TEST_CASE( kv::DeserialiseSuccess::FAILED); kv::Tx tx_target; - auto view_priv_target = tx_target.get_view( - *kv_store_target.get("priv_map")); + auto view_priv_target = tx_target.get_view(*target_priv_map); REQUIRE(view_priv_target->get("privk1").has_value() == false); } } +struct CustomClass +{ + int m_i; + + CustomClass() : CustomClass(-1) {} + CustomClass(int i) : m_i(i) {} + + int get() const + { + return m_i; + } + void set(std::string val) + { + m_i = std::stoi(val); + } + + CustomClass operator()() + { + CustomClass ret; + return ret; + } + + bool operator<(const CustomClass& other) const + { + return m_i < other.m_i; + } + + bool operator==(const CustomClass& other) const + { + return !(other < *this) && !(*this < other); + } + + MSGPACK_DEFINE(m_i); +}; + +namespace std +{ + template <> + struct hash + { + std::size_t operator()(const CustomClass& inst) const + { + return inst.get(); + } + }; +} + +DECLARE_JSON_TYPE(CustomClass) +DECLARE_JSON_REQUIRED_FIELDS(CustomClass, m_i) + TEST_CASE( - "Custom type serialisation test" * doctest::test_suite("serialisation")) + "Custom type serialisation test (original scheme)" * + doctest::test_suite("serialisation")) { kv::Store kv_store; @@ -291,6 +331,137 @@ TEST_CASE( } } +struct CustomClass2 +{ + std::string s; + size_t n; +}; + +struct CustomJsonSerialiser +{ + using Bytes = kv::experimental::SerialisedRep; + + static Bytes to_serialised(const CustomClass2& c) + { + nlohmann::json j = nlohmann::json::object(); + j["s"] = c.s; + j["n"] = c.n; + const auto s = j.dump(); + return Bytes(s.begin(), s.end()); + } + + static CustomClass2 from_serialised(const Bytes& b) + { + const auto j = nlohmann::json::parse(b); + CustomClass2 c; + c.s = j["s"]; + c.n = j["n"]; + return c; + } +}; + +struct KPrefix +{ + static constexpr auto prefix = "This is a key:"; +}; + +struct VPrefix +{ + static constexpr auto prefix = "Here follows a value:"; +}; + +template +struct CustomVerboseDumbSerialiser +{ + using Bytes = kv::experimental::SerialisedRep; + + static Bytes to_serialised(const CustomClass2& c) + { + const auto verbose = fmt::format("{}\ns={}\nn={}", T::prefix, c.s, c.n); + return Bytes(verbose.begin(), verbose.end()); + } + + static CustomClass2 from_serialised(const Bytes& b) + { + std::string s(b.begin(), b.end()); + const auto prefix_start = s.find(T::prefix); + if (prefix_start != 0) + { + throw std::logic_error("Missing expected prefix"); + } + + CustomClass2 c; + const auto first_linebreak = s.find('\n'); + const auto last_linebreak = s.rfind('\n'); + const auto seg_a = s.substr(0, first_linebreak); + const auto seg_b = + s.substr(first_linebreak + 1, last_linebreak - first_linebreak - 1); + const auto seg_c = s.substr(last_linebreak + 1); + + c.s = seg_b.substr(strlen("s=")); + const auto n_str = seg_c.substr(strlen("n=")); + c.n = strtoul(n_str.c_str(), nullptr, 10); + return c; + } +}; + +using MapA = kv::experimental:: + Map; +using MapB = kv::experimental::Map< + CustomClass2, + CustomClass2, + CustomVerboseDumbSerialiser, + CustomVerboseDumbSerialiser>; + +TEST_CASE_TEMPLATE( + "Custom type serialisation test (experimental scheme)" * + doctest::test_suite("serialisation"), + MapType, + MapA, + MapB) +{ + kv::Store kv_store; + + auto& map = kv_store.create("map", kv::SecurityDomain::PUBLIC); + + CustomClass2 k1{"hello", 42}; + CustomClass2 v1{"world", 43}; + + CustomClass2 k2{"saluton", 100}; + CustomClass2 v2{"mondo", 1024}; + + INFO("Serialise/Deserialise 2 kv stores"); + { + kv::Store kv_store2; + auto& map2 = kv_store2.create("map", kv::SecurityDomain::PUBLIC); + + kv::Tx tx(kv_store.next_version()); + auto view = tx.get_view(map); + view->put(k1, v1); + view->put(k2, v2); + + auto [success, reqid, data] = tx.commit_reserved(); + REQUIRE(success == kv::CommitSuccess::OK); + kv_store.compact(kv_store.current_version()); + + REQUIRE(kv_store2.deserialise(data) == kv::DeserialiseSuccess::PASS); + kv::Tx tx2; + auto view2 = tx2.get_view(map2); + + // operator== does not need to be defined for custom types. In this case it + // is not, and we check each member manually + auto va = view2->get(k1); + REQUIRE(va.has_value()); + REQUIRE(va->s == v1.s); + REQUIRE(va->n == v1.n); + + auto vb = view2->get(k2); + REQUIRE(vb.has_value()); + REQUIRE(vb->s == v2.s); + REQUIRE(vb->n == v2.n); + } +} + bool corrupt_serialised_tx( std::vector& serialised_tx, std::vector& value_to_corrupt) { @@ -317,7 +488,11 @@ bool corrupt_serialised_tx( return false; } -TEST_CASE("Integrity" * doctest::test_suite("serialisation")) +TEST_CASE_TEMPLATE( + "Integrity" * doctest::test_suite("serialisation"), + MapImpl, + RawMapTypes, + ExperimentalMapTypes) { SUBCASE("Public and Private") { @@ -340,10 +515,10 @@ TEST_CASE("Integrity" * doctest::test_suite("serialisation")) kv_store.set_encryptor(encryptor); kv_store_target.set_encryptor(encryptor); - auto& public_map = kv_store.create( + auto& public_map = kv_store.create( "public_map", kv::SecurityDomain::PUBLIC); auto& private_map = - kv_store.create("private_map"); + kv_store.create("private_map"); kv_store_target.clone_schema(kv_store); @@ -409,33 +584,42 @@ TEST_CASE("nlohmann (de)serialisation" * doctest::test_suite("serialisation")) } } -TEST_CASE("replicated and derived table serialisation") +TEST_CASE_TEMPLATE( + "Replicated and derived table serialisation" * + doctest::test_suite("serialisation"), + MapImpl, + RawMapTypes, + ExperimentalMapTypes) { + using T = typename MapImpl::NumNum; + auto encryptor = std::make_shared(); std::unordered_set replicated_tables = { "data_replicated", "data_replicated_private"}; + kv::Store store(kv::ReplicateType::SOME, replicated_tables); store.set_encryptor(encryptor); - - kv::Store second_store(kv::ReplicateType::SOME, replicated_tables); - second_store.set_encryptor(encryptor); - auto& data_replicated = - store.create("data_replicated", kv::SecurityDomain::PUBLIC); - auto& second_data_replicated = second_store.create( - "data_replicated", kv::SecurityDomain::PUBLIC); + store.create("data_replicated", kv::SecurityDomain::PUBLIC); auto& data_derived = - store.create("data_derived", kv::SecurityDomain::PUBLIC); - auto& second_data_derived = second_store.create( - "data_derived", kv::SecurityDomain::PUBLIC); - auto& data_replicated_private = - store.create("data_replicated_private"); - auto& second_data_replicated_private = - second_store.create("data_replicated_private"); - auto& data_derived_private = - store.create("data_derived_private"); - auto& second_data_derived_private = - second_store.create("data_derived_private"); + store.create("data_derived", kv::SecurityDomain::PUBLIC); + auto& data_replicated_private = store.create("data_replicated_private"); + auto& data_derived_private = store.create("data_derived_private"); + + kv::Store kv_store_target(kv::ReplicateType::SOME, replicated_tables); + kv_store_target.set_encryptor(encryptor); + kv_store_target.clone_schema(store); + auto* second_data_replicated = + kv_store_target.get(data_replicated.get_name()); + auto* second_data_derived = kv_store_target.get(data_derived.get_name()); + auto* second_data_replicated_private = + kv_store_target.get(data_replicated_private.get_name()); + auto* second_data_derived_private = + kv_store_target.get(data_derived_private.get_name()); + REQUIRE(second_data_replicated != nullptr); + REQUIRE(second_data_derived != nullptr); + REQUIRE(second_data_replicated_private != nullptr); + REQUIRE(second_data_derived_private != nullptr); { kv::Tx tx(store.next_version()); @@ -456,14 +640,15 @@ TEST_CASE("replicated and derived table serialisation") INFO("check that second store derived data is not populated"); { - REQUIRE(second_store.deserialise(data) == kv::DeserialiseSuccess::PASS); + REQUIRE( + kv_store_target.deserialise(data) == kv::DeserialiseSuccess::PASS); kv::Tx tx; auto [data_view_r, data_view_r_p, data_view_d, data_view_d_p] = tx.get_view( - second_data_replicated, - second_data_replicated_private, - second_data_derived, - second_data_derived_private); + *second_data_replicated, + *second_data_replicated_private, + *second_data_derived, + *second_data_derived_private); auto dvr = data_view_r->get(44); REQUIRE(dvr.has_value()); REQUIRE(dvr.value() == 44); @@ -514,7 +699,8 @@ namespace msgpack } } -TEST_CASE("Exceptional serdes" * doctest::test_suite("serialisation")) +TEST_CASE( + "Exceptional serdes (old scheme)" * doctest::test_suite("serialisation")) { auto encryptor = std::make_shared(); auto consensus = std::make_shared(); @@ -527,13 +713,73 @@ TEST_CASE("Exceptional serdes" * doctest::test_suite("serialisation")) { kv::Tx tx; - auto good_view = tx.get_view(good_map); good_view->put(1, 2); + REQUIRE(tx.commit() == kv::CommitSuccess::OK); + } + { + kv::Tx tx; auto bad_view = tx.get_view(bad_map); bad_view->put(0, {}); + REQUIRE_THROWS_AS(tx.commit(), kv::KvSerialiserException); + } + { + kv::Tx tx; + auto good_view = tx.get_view(good_map); + good_view->put(1, 2); + auto bad_view = tx.get_view(bad_map); + bad_view->put(0, {}); REQUIRE_THROWS_AS(tx.commit(), kv::KvSerialiserException); } +} + +struct NonSerialiser +{ + using Bytes = kv::experimental::SerialisedRep; + + static Bytes to_serialised(const NonSerialisable& ns) + { + throw std::runtime_error("Serialise failure"); + } + + static NonSerialisable from_serialised(const Bytes& b) + { + throw std::runtime_error("Deserialise failure"); + } +}; + +TEST_CASE( + "Exceptional serdes (experimental scheme)" * + doctest::test_suite("serialisation")) +{ + auto encryptor = std::make_shared(); + auto consensus = std::make_shared(); + + kv::Store store(consensus); + store.set_encryptor(encryptor); + + auto& bad_map_k = store.create>>("bad_map_k"); + auto& bad_map_v = store.create, + NonSerialiser>>("bad_map_v"); + + { + kv::Tx tx; + auto bad_view = tx.get_view(bad_map_k); + REQUIRE_THROWS(bad_view->put({}, 0)); + } + + { + kv::Tx tx; + auto bad_view = tx.get_view(bad_map_v); + REQUIRE_THROWS(bad_view->put(0, {})); + } } \ No newline at end of file diff --git a/src/kv/test/kv_test.cpp b/src/kv/test/kv_test.cpp index cb9409c3cc0..567270d4434 100644 --- a/src/kv/test/kv_test.cpp +++ b/src/kv/test/kv_test.cpp @@ -2,6 +2,7 @@ // Licensed under the Apache 2.0 License. #include "ds/logger.h" #include "enclave/app_interface.h" +#include "kv/experimental.h" #include "kv/kv_serialiser.h" #include "kv/store.h" #include "kv/test/null_encryptor.h" @@ -14,51 +15,72 @@ #include #include -TEST_CASE("Map creation") +struct RawMapTypes +{ + using StringString = kv::Map; + using NumNum = kv::Map; + using NumString = kv::Map; + using StringNum = kv::Map; +}; + +struct ExperimentalMapTypes +{ + using StringString = kv::experimental::Map; + using NumNum = kv::experimental::Map; + using NumString = kv::experimental::Map; + using StringNum = kv::experimental::Map; +}; + +TEST_CASE_TEMPLATE("Map creation", MapImpl, RawMapTypes, ExperimentalMapTypes) { kv::Store kv_store; - auto& map = kv_store.create("map"); + const auto map_name = "map"; + auto& map = kv_store.create(map_name); INFO("Get a map that does not exist"); { - // Macros can't handle commas, so we need a single named template argument - using StringString = kv::Map; - REQUIRE(kv_store.get("invalid_map") == nullptr); + REQUIRE( + kv_store.get("invalid_map") == nullptr); + } + + INFO("Get a map that does exist"); + { + auto* p_map = kv_store.get(map_name); + REQUIRE(*p_map == map); + REQUIRE(p_map == &map); // They're the _same instance_, not just equal } INFO("Compare different maps"); { - auto& map2 = kv_store.create("map2"); + auto& map2 = kv_store.create("map2"); REQUIRE(map != map2); } INFO("Can't create map that already exists"); { - using StringString = kv::Map; - REQUIRE_THROWS_AS(kv_store.create("map"), std::logic_error); + REQUIRE_THROWS_AS( + kv_store.create(map_name), + std::logic_error); } INFO("Can't get a map with the wrong type"); { - using IntInt = kv::Map; - REQUIRE(kv_store.get("map") == nullptr); - using IntString = kv::Map; - REQUIRE(kv_store.get("map") == nullptr); - using StringInt = kv::Map; - REQUIRE(kv_store.get("map") == nullptr); + REQUIRE(kv_store.get(map_name) == nullptr); + REQUIRE(kv_store.get(map_name) == nullptr); + REQUIRE(kv_store.get(map_name) == nullptr); } INFO("Can create a map with a previously invalid name"); { - using StringString = kv::Map; - CHECK_NOTHROW(kv_store.create("version")); + CHECK_NOTHROW(kv_store.create("version")); } } -TEST_CASE("Reads/writes and deletions") +TEST_CASE_TEMPLATE( + "Reads/writes and deletions", MapImpl, RawMapTypes, ExperimentalMapTypes) { kv::Store kv_store; - auto& map = kv_store.create( + auto& map = kv_store.create( "map", kv::SecurityDomain::PUBLIC); constexpr auto k = "key"; @@ -131,26 +153,154 @@ TEST_CASE("Reads/writes and deletions") auto vc = view3->get(k); REQUIRE(!vc.has_value()); } +} + +TEST_CASE_TEMPLATE("foreach", MapImpl, RawMapTypes, ExperimentalMapTypes) +{ + kv::Store kv_store; + auto& map = kv_store.create( + "map", kv::SecurityDomain::PUBLIC); + + std::map iterated_entries; + + auto store_iterated = + [&iterated_entries](const auto& key, const auto& value) { + auto it = iterated_entries.find(key); + REQUIRE(it == iterated_entries.end()); + iterated_entries[key] = value; + return true; + }; + + SUBCASE("Empty map") + { + kv::Tx tx; + auto view = tx.get_view(map); + view->foreach(store_iterated); + REQUIRE(iterated_entries.empty()); + } + + SUBCASE("Reading own writes") + { + kv::Tx tx; + auto view = tx.get_view(map); + view->put("key1", "value1"); + view->put("key2", "value2"); + view->foreach(store_iterated); + REQUIRE(iterated_entries.size() == 2); + REQUIRE(iterated_entries["key1"] == "value1"); + REQUIRE(iterated_entries["key2"] == "value2"); + + iterated_entries.clear(); + + INFO("Uncommitted writes from other txs are not visible"); + kv::Tx tx2; + auto view2 = tx2.get_view(map); + view2->foreach(store_iterated); + REQUIRE(iterated_entries.empty()); + } + + SUBCASE("Reading committed writes") + { + kv::Tx tx; + auto view = tx.get_view(map); + view->put("key1", "value1"); + view->put("key2", "value2"); + REQUIRE(tx.commit() == kv::CommitSuccess::OK); + + kv::Tx tx2; + auto view2 = tx2.get_view(map); + view2->foreach(store_iterated); + REQUIRE(iterated_entries.size() == 2); + REQUIRE(iterated_entries["key1"] == "value1"); + REQUIRE(iterated_entries["key2"] == "value2"); + } + + SUBCASE("Mix of committed and own writes") + { + kv::Tx tx; + auto view = tx.get_view(map); + view->put("key1", "value1"); + view->put("key2", "value2"); + REQUIRE(tx.commit() == kv::CommitSuccess::OK); + + kv::Tx tx2; + auto view2 = tx2.get_view(map); + view2->put("key2", "replaced2"); + view2->put("key3", "value3"); + view2->foreach(store_iterated); + REQUIRE(iterated_entries.size() == 3); + REQUIRE(iterated_entries["key1"] == "value1"); + REQUIRE(iterated_entries["key2"] == "replaced2"); + REQUIRE(iterated_entries["key3"] == "value3"); + } + + SUBCASE("Deletions") + { + { + kv::Tx tx; + auto view = tx.get_view(map); + view->put("key1", "value1"); + view->put("key2", "value2"); + view->put("key3", "value3"); + REQUIRE(tx.commit() == kv::CommitSuccess::OK); + } + + { + kv::Tx tx; + auto view = tx.get_view(map); + view->remove("key1"); + REQUIRE(tx.commit() == kv::CommitSuccess::OK); + } + + { + kv::Tx tx; + auto view = tx.get_view(map); + view->foreach(store_iterated); + REQUIRE(iterated_entries.size() == 2); + REQUIRE(iterated_entries["key2"] == "value2"); + REQUIRE(iterated_entries["key3"] == "value3"); + + iterated_entries.clear(); + + view->remove("key2"); + view->foreach(store_iterated); + REQUIRE(iterated_entries.size() == 1); + REQUIRE(iterated_entries["key3"] == "value3"); + + iterated_entries.clear(); + + view->put("key1", "value1"); + view->put("key2", "value2"); + view->foreach(store_iterated); + REQUIRE(iterated_entries.size() == 3); + REQUIRE(iterated_entries["key1"] == "value1"); + REQUIRE(iterated_entries["key2"] == "value2"); + REQUIRE(iterated_entries["key3"] == "value3"); + } + } - INFO("Test early temination of KV foreach"); + SUBCASE("Early termination") { kv::Tx tx; auto view = tx.get_view(map); view->put("key1", "value1"); view->put("key2", "value2"); + view->put("key3", "value3"); size_t ctr = 0; view->foreach([&ctr](const auto& key, const auto& value) { ++ctr; - return false; + return ctr <= 1; // Continue after the first, but not the second (so never + // see the third) }); - REQUIRE(ctr == 1); + REQUIRE(ctr == 2); } } -TEST_CASE("Rollback and compact") +TEST_CASE_TEMPLATE( + "Rollback and compact", MapImpl, RawMapTypes, ExperimentalMapTypes) { kv::Store kv_store; - auto& map = kv_store.create( + auto& map = kv_store.create( "map", kv::SecurityDomain::PUBLIC); constexpr auto k = "key"; @@ -201,12 +351,13 @@ TEST_CASE("Rollback and compact") } } -TEST_CASE("Clear entire store") +TEST_CASE_TEMPLATE( + "Clear entire store", MapImpl, RawMapTypes, ExperimentalMapTypes) { kv::Store kv_store; - auto& map1 = kv_store.create( + auto& map1 = kv_store.create( "map1", kv::SecurityDomain::PUBLIC); - auto& map2 = kv_store.create( + auto& map2 = kv_store.create( "map2", kv::SecurityDomain::PUBLIC); INFO("Commit a transaction over two maps"); @@ -240,22 +391,22 @@ TEST_CASE("Clear entire store") } } -TEST_CASE("Local commit hooks") +TEST_CASE_TEMPLATE( + "Local commit hooks", MapImpl, RawMapTypes, ExperimentalMapTypes) { - using State = kv::Map::State; - using Write = kv::Map::Write; + using Write = typename MapImpl::StringString::Write; std::vector local_writes; std::vector global_writes; - auto local_hook = [&](kv::Version v, const State& s, const Write& w) { + auto local_hook = [&](kv::Version v, const Write& w) { local_writes.push_back(w); }; - auto global_hook = [&](kv::Version v, const State& s, const Write& w) { + auto global_hook = [&](kv::Version v, const Write& w) { global_writes.push_back(w); }; kv::Store kv_store; - auto& map = kv_store.create( + auto& map = kv_store.create( "map", kv::SecurityDomain::PUBLIC); map.set_local_hook(local_hook); map.set_global_hook(global_hook); @@ -264,12 +415,18 @@ TEST_CASE("Local commit hooks") { kv::Tx tx; auto view = tx.get_view(map); - view->put("key", "value1"); + view->put("key1", "value1"); + view->put("key2", "value2"); + view->remove("key2"); REQUIRE(tx.commit() == kv::CommitSuccess::OK); REQUIRE(global_writes.size() == 0); REQUIRE(local_writes.size() == 1); - REQUIRE(local_writes.front().at("key").value == "value1"); + const auto& latest_writes = local_writes.front(); + REQUIRE(latest_writes.at("key1").value == "value1"); + INFO("Local removals are not seen"); + REQUIRE(latest_writes.find("key2") == latest_writes.end()); + REQUIRE(latest_writes.size() == 1); local_writes.clear(); } @@ -281,7 +438,7 @@ TEST_CASE("Local commit hooks") kv::Tx tx; auto view = tx.get_view(map); - view->put("key", "value2"); + view->put("key2", "value2"); REQUIRE(tx.commit() == kv::CommitSuccess::OK); REQUIRE(local_writes.size() == 0); @@ -295,21 +452,29 @@ TEST_CASE("Local commit hooks") kv::Tx tx; auto view = tx.get_view(map); - view->put("key", "value3"); + view->remove("key2"); + view->put("key3", "value3"); REQUIRE(tx.commit() == kv::CommitSuccess::OK); REQUIRE(global_writes.size() == 0); REQUIRE(local_writes.size() == 1); - REQUIRE(local_writes.front().at("key").value == "value3"); + const auto& latest_writes = local_writes.front(); + INFO("Old writes are not included"); + REQUIRE(latest_writes.find("key1") == latest_writes.end()); + INFO("Visible removals are included"); + REQUIRE(latest_writes.at("key2").version == kv::NoVersion); + REQUIRE(latest_writes.at("key3").value == "value3"); + REQUIRE(latest_writes.size() == 2); local_writes.clear(); } } -TEST_CASE("Global commit hooks") +TEST_CASE_TEMPLATE( + "Global commit hooks", MapImpl, RawMapTypes, ExperimentalMapTypes) { - using State = kv::Map::State; - using Write = kv::Map::Write; + using Write = typename MapImpl::StringString::Write; + struct GlobalHookInput { kv::Version version; @@ -318,7 +483,7 @@ TEST_CASE("Global commit hooks") std::vector global_writes; - auto global_hook = [&](kv::Version v, const State& s, const Write& w) { + auto global_hook = [&](kv::Version v, const Write& w) { global_writes.emplace_back(GlobalHookInput({v, w})); }; @@ -364,13 +529,15 @@ TEST_CASE("Global commit hooks") view_hook->put("key2", "value2"); REQUIRE(tx2.commit() == kv::CommitSuccess::OK); + const auto compact_version = kv_store.current_version(); + // This does not affect map_with_hook but still increments the current // version of the store auto view_no_hook = tx3.get_view(map_no_hook); view_no_hook->put("key3", "value3"); REQUIRE(tx3.commit() == kv::CommitSuccess::OK); - kv_store.compact(3); + kv_store.compact(compact_version); // Only the changes made to map_with_hook should be passed to the global // hook @@ -397,11 +564,13 @@ TEST_CASE("Global commit hooks") view_no_hook->put("key2", "value2"); REQUIRE(tx2.commit() == kv::CommitSuccess::OK); + const auto compact_version = kv_store.current_version(); + view_hook = tx3.get_view(map_with_hook); view_hook->put("key3", "value3"); REQUIRE(tx3.commit() == kv::CommitSuccess::OK); - kv_store.compact(2); + kv_store.compact(compact_version); // Only the changes made to map_with_hook should be passed to the global // hook @@ -420,14 +589,14 @@ TEST_CASE("Global commit hooks") view_hook->put("key1", "value1"); REQUIRE(tx1.commit() == kv::CommitSuccess::OK); - kv_store.compact(1); + kv_store.compact(kv_store.current_version()); global_writes.clear(); view_hook = tx2.get_view(map_with_hook); view_hook->put("key2", "value2"); REQUIRE(tx2.commit() == kv::CommitSuccess::OK); - kv_store.compact(2); + kv_store.compact(kv_store.current_version()); // Only writes since the last compact are passed to the global hook REQUIRE(global_writes.size() == 1); @@ -439,15 +608,15 @@ TEST_CASE("Global commit hooks") } } -TEST_CASE("Clone schema") +TEST_CASE_TEMPLATE("Clone schema", MapImpl, RawMapTypes, ExperimentalMapTypes) { auto encryptor = std::make_shared(); kv::Store store; store.set_encryptor(encryptor); - auto& public_map = - store.create("public", kv::SecurityDomain::PUBLIC); - auto& private_map = store.create("private"); + auto& public_map = store.create( + "public", kv::SecurityDomain::PUBLIC); + auto& private_map = store.create("private"); kv::Tx tx1(store.next_version()); auto [view1, view2] = tx1.get_view(public_map, private_map); view1->put(42, "aardvark"); @@ -462,7 +631,8 @@ TEST_CASE("Clone schema") REQUIRE(clone.deserialise(data) == kv::DeserialiseSuccess::PASS); } -TEST_CASE("Deserialise return status") +TEST_CASE_TEMPLATE( + "Deserialise return status", MapImpl, RawMapTypes, ExperimentalMapTypes) { kv::Store store; @@ -470,7 +640,8 @@ TEST_CASE("Deserialise return status") ccf::Tables::SIGNATURES, kv::SecurityDomain::PUBLIC); auto& nodes = store.create(ccf::Tables::NODES, kv::SecurityDomain::PUBLIC); - auto& data = store.create("data", kv::SecurityDomain::PUBLIC); + auto& data = + store.create("data", kv::SecurityDomain::PUBLIC); auto kp = tls::make_key_pair(); @@ -513,21 +684,22 @@ TEST_CASE("Deserialise return status") } } -TEST_CASE("map swap between stores") +TEST_CASE_TEMPLATE( + "Map swap between stores", MapImpl, RawMapTypes, ExperimentalMapTypes) { auto encryptor = std::make_shared(); kv::Store s1; s1.set_encryptor(encryptor); - auto& d1 = s1.create("data"); - auto& pd1 = - s1.create("public_data", kv::SecurityDomain::PUBLIC); + auto& d1 = s1.create("data"); + auto& pd1 = s1.create( + "public_data", kv::SecurityDomain::PUBLIC); kv::Store s2; s2.set_encryptor(encryptor); - auto& d2 = s2.create("data"); - auto& pd2 = - s2.create("public_data", kv::SecurityDomain::PUBLIC); + auto& d2 = s2.create("data"); + auto& pd2 = s2.create( + "public_data", kv::SecurityDomain::PUBLIC); { kv::Tx tx; @@ -585,15 +757,16 @@ TEST_CASE("map swap between stores") } } -TEST_CASE("invalid map swaps") +TEST_CASE_TEMPLATE( + "Invalid map swaps", MapImpl, RawMapTypes, ExperimentalMapTypes) { { kv::Store s1; - s1.create("one"); + s1.create("one"); kv::Store s2; - s2.create("one"); - s2.create("two"); + s2.create("one"); + s2.create("two"); REQUIRE_THROWS_WITH( s2.swap_private_maps(s1), @@ -602,11 +775,11 @@ TEST_CASE("invalid map swaps") { kv::Store s1; - s1.create("one"); - s1.create("two"); + s1.create("one"); + s1.create("two"); kv::Store s2; - s2.create("one"); + s2.create("one"); REQUIRE_THROWS_WITH( s2.swap_private_maps(s1), @@ -614,20 +787,21 @@ TEST_CASE("invalid map swaps") } } -TEST_CASE("private recovery map swap") +TEST_CASE_TEMPLATE( + "Private recovery map swap", MapImpl, RawMapTypes, ExperimentalMapTypes) { auto encryptor = std::make_shared(); kv::Store s1; s1.set_encryptor(encryptor); - auto& priv1 = s1.create("private"); - auto& pub1 = - s1.create("public", kv::SecurityDomain::PUBLIC); + auto& priv1 = s1.create("private"); + auto& pub1 = s1.create( + "public", kv::SecurityDomain::PUBLIC); kv::Store s2; s2.set_encryptor(encryptor); - auto& priv2 = s2.create("private"); - auto& pub2 = - s2.create("public", kv::SecurityDomain::PUBLIC); + auto& priv2 = s2.create("private"); + auto& pub2 = s2.create( + "public", kv::SecurityDomain::PUBLIC); INFO("Populate s1 with public entries"); // We compact twice, deliberately. A public KV during recovery @@ -678,7 +852,7 @@ TEST_CASE("private recovery map swap") s2.compact(s2.current_version()); INFO("Swap in private maps"); - s1.swap_private_maps(s2); + REQUIRE_NOTHROW(s1.swap_private_maps(s2)); INFO("Check state looks as expected in s1"); { @@ -744,16 +918,17 @@ TEST_CASE("private recovery map swap") } } -TEST_CASE("Conflict resolution") +TEST_CASE_TEMPLATE( + "Conflict resolution", MapImpl, RawMapTypes, ExperimentalMapTypes) { kv::Store kv_store; - auto& map = kv_store.create( + auto& map = kv_store.create( "map", kv::SecurityDomain::PUBLIC); auto try_write = [&](kv::Tx& tx, const std::string& s) { auto view = tx.get_view(map); - // Introduce read-dependency + // Numroduce read-dependency view->get("foo"); view->put("foo", s); diff --git a/src/kv/test/stub_consensus.h b/src/kv/test/stub_consensus.h index af176a56ff2..9d262041ac0 100644 --- a/src/kv/test/stub_consensus.h +++ b/src/kv/test/stub_consensus.h @@ -93,10 +93,15 @@ namespace kv void add_configuration( SeqNo seqno, - std::unordered_set conf, + const std::unordered_set& conf, const NodeConf& node_conf) override {} + std::unordered_set get_latest_configuration() const override + { + return {}; + } + void set_f(size_t) override { return; diff --git a/src/kv/tx.h b/src/kv/tx.h index 6179cfc5d6c..c70003ad39e 100644 --- a/src/kv/tx.h +++ b/src/kv/tx.h @@ -9,18 +9,6 @@ namespace kv { - static inline std::map> - get_views_grouped_by_domain(const OrderedViews& maps) - { - std::map> grouped_views; - for (auto it = maps.cbegin(); it != maps.cend(); ++it) - { - grouped_views[it->second.map->get_security_domain()].push_back( - it->second.view.get()); - } - return grouped_views; - } - class Tx : public ViewContainer { private: @@ -35,11 +23,24 @@ namespace kv template std::tuple get_tuple(M& m) { - // If the M is present, its AbtractTxView must be an M::TxView. + using MapView = typename M::TxView; + + // If the M is present, its AbtractTxView should be an M::TxView. This + // invariant could be broken by set_view_list, which will produce an error + // here auto search = view_list.find(m.get_name()); if (search != view_list.end()) - return std::make_tuple( - dynamic_cast(search->second.view.get())); + { + auto view = dynamic_cast(search->second.view.get()); + + if (view == nullptr) + { + throw std::logic_error(fmt::format( + "View over map {} is not of expected type", m.get_name())); + } + + return std::make_tuple(view); + } auto it = view_list.begin(); if (it != view_list.end()) @@ -56,9 +57,15 @@ namespace kv read_version = m.get_store()->current_version(); } - AbstractTxView* view = m.create_view(read_version); - auto typed_view = dynamic_cast(view); - view_list[m.get_name()] = {&m, std::unique_ptr(view)}; + MapView* typed_view = m.template create_view(read_version); + auto abstract_view = dynamic_cast(typed_view); + if (abstract_view == nullptr) + { + throw std::logic_error(fmt::format( + "View over map {} is not an AbstractTxView", m.get_name())); + } + view_list[m.get_name()] = { + &m, std::unique_ptr(abstract_view)}; return std::make_tuple(typed_view); } @@ -241,37 +248,34 @@ namespace kv throw std::logic_error("Transaction aborted"); // If no transactions made changes, return a zero length vector. - bool changes = false; + const bool any_changes = + std::any_of(view_list.begin(), view_list.end(), [](const auto& it) { + return it.second.view->has_changes(); + }); - for (auto it = view_list.begin(); it != view_list.end(); ++it) - { - if (it->second.view->has_changes()) - { - changes = true; - break; - } - } - - if (!changes) + if (!any_changes) { return {}; } + // Retrieve encryptor. auto map = view_list.begin()->second.map; auto e = map->get_store()->get_encryptor(); KvStoreSerialiser replicated_serialiser(e, version); - // flags that indicate if we have actually written any data in the - // serializers - auto grouped_views = get_views_grouped_by_domain(view_list); - for (auto domain_it : grouped_views) + // Process in security domain order + for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) { - for (auto curr_view : domain_it.second) + for (const auto& it : view_list) { - if (curr_view->is_replicated()) + const auto map = it.second.map; + if ( + map->get_security_domain() == domain && map->is_replicated() && + it.second.view->has_changes()) { - curr_view->serialise(replicated_serialiser, include_reads); + map->serialise( + it.second.view.get(), replicated_serialiser, include_reads); } } } diff --git a/src/kv/tx_view.h b/src/kv/tx_view.h index bdf6940f5c0..5ab7c26b328 100644 --- a/src/kv/tx_view.h +++ b/src/kv/tx_view.h @@ -30,14 +30,15 @@ namespace kv template using Read = std::unordered_map; - template + template > using Write = std::unordered_map, H>; - template - class TxView + // This is a container for a write-set + dependencies. It can be applied to a + // given state, or used to track a set of operations on a state + template > + struct ChangeSet { - protected: - using VersionV = VersionV; + public: using State = State; using Read = Read; using Write = Write; @@ -46,9 +47,28 @@ namespace kv State committed; Version start_version; + Version read_version = NoVersion; Read reads = {}; Write writes = {}; - Version read_version = NoVersion; + + ChangeSet( + State& current_state, State& committed_state, Version current_version) : + state(current_state), + committed(committed_state), + start_version(current_version) + {} + + ChangeSet(ChangeSet&) = delete; + }; + + template > + class TxView + { + protected: + using State = State; + + using ChangeSet = ChangeSet; + ChangeSet& tx_changes; public: // Expose these types so that other code can use them as MyTx::KeyType or @@ -57,11 +77,7 @@ namespace kv using KeyType = K; using ValueType = V; - TxView(State& current_state, State& committed_state, Version v) : - state(current_state), - committed(committed_state), - start_version(v) - {} + TxView(ChangeSet& cs) : tx_changes(cs) {} /** Get value for key * @@ -77,8 +93,8 @@ namespace kv { // A write followed by a read doesn't introduce a read dependency. // If we have written, return the value without updating the read set. - auto write = writes.find(key); - if (write != writes.end()) + auto write = tx_changes.writes.find(key); + if (write != tx_changes.writes.end()) { // Return empty for a key that has been removed. if (is_deleted(write->second.version)) @@ -91,16 +107,16 @@ namespace kv // If the key doesn't exist, return empty and record that we depend on // the key not existing. - auto search = state.get(key); + auto search = tx_changes.state.get(key); if (!search.has_value()) { - reads.insert(std::make_pair(key, NoVersion)); + tx_changes.reads.insert(std::make_pair(key, NoVersion)); return std::nullopt; } // Record the version that we depend on. auto& found = search.value(); - reads.insert(std::make_pair(key, found.version)); + tx_changes.reads.insert(std::make_pair(key, found.version)); // If the key has been deleted, return empty. if (is_deleted(found.version)) @@ -128,7 +144,7 @@ namespace kv std::optional get_globally_committed(const K& key) { // If there is no committed value, return empty. - auto search = committed.get(key); + auto search = tx_changes.committed.get(key); if (!search.has_value()) { return std::nullopt; @@ -158,7 +174,7 @@ namespace kv bool put(const K& key, const V& value) { // Record in the write set. - writes[key] = {0, value}; + tx_changes.writes[key] = {0, value}; return true; } @@ -173,16 +189,16 @@ namespace kv */ bool remove(const K& key) { - auto write = writes.find(key); - auto search = state.get(key).has_value(); + auto write = tx_changes.writes.find(key); + auto search = tx_changes.state.get(key).has_value(); - if (write != writes.end()) + if (write != tx_changes.writes.end()) { if (!search) { // this key only exists locally, there is no reason to maintain and // serialise it - writes.erase(key); + tx_changes.writes.erase(key); } else { @@ -200,7 +216,7 @@ namespace kv } // Record in the write set. - writes.emplace( + tx_changes.writes.emplace( std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple(NoVersion, V())); @@ -216,10 +232,10 @@ namespace kv bool foreach(F&& f) { // Record a global read dependency. - read_version = start_version; - auto& w = writes; + tx_changes.read_version = tx_changes.start_version; + auto& w = tx_changes.writes; - state.foreach([&w, &f](const K& k, const VersionV& v) { + tx_changes.state.foreach([&w, &f](const K& k, const VersionV& v) { auto write = w.find(k); if ((write == w.end()) && !is_deleted(v.version)) @@ -227,7 +243,9 @@ namespace kv return true; }); - for (auto write = writes.begin(); write != writes.end(); ++write) + for (auto write = tx_changes.writes.begin(); + write != tx_changes.writes.end(); + ++write) { if (!is_deleted(write->second.version)) if (!f(write->first, write->second.value)) diff --git a/src/node/node_state.h b/src/node/node_state.h index 7e536104f9b..f7eea73022c 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1316,8 +1316,7 @@ namespace ccf // When a transaction that changes the configuration commits globally, // inform the host of any nodes that no longer need to be tracked. network.nodes.set_global_hook( - [this]( - kv::Version version, const Nodes::State& s, const Nodes::Write& w) { + [this](kv::Version version, const Nodes::Write& w) { for (auto& [node_id, ni] : w) { if (ni.value.status == NodeStatus::RETIRED) @@ -1325,92 +1324,89 @@ namespace ccf } }); - network.service.set_global_hook([this]( - kv::Version version, - const Service::State& s, - const Service::Write& w) { - if (w.at(0).value.status == ServiceStatus::OPEN) - { - this->consensus->set_f(1); - open_user_frontend(); - LOG_INFO_FMT("Network is OPEN, now accepting user transactions"); - } - }); + network.service.set_global_hook( + [this](kv::Version version, const Service::Write& w) { + if (w.at(0).value.status == ServiceStatus::OPEN) + { + this->consensus->set_f(1); + open_user_frontend(); + LOG_INFO_FMT("Network is OPEN, now accepting user transactions"); + } + }); - network.secrets.set_local_hook([this]( - kv::Version version, - const Secrets::State& s, - const Secrets::Write& w) { - bool has_secrets = false; - std::list restored_secrets; + network.secrets.set_local_hook( + [this](kv::Version version, const Secrets::Write& w) { + bool has_secrets = false; + std::list restored_secrets; - for (auto& [v, secret_set] : w) - { - for (auto& encrypted_secret_for_node : secret_set.value.secrets) + for (auto& [v, secret_set] : w) { - if (encrypted_secret_for_node.node_id == self) + for (auto& encrypted_secret_for_node : secret_set.value.secrets) { - crypto::GcmCipher gcmcipher; - gcmcipher.deserialise(encrypted_secret_for_node.encrypted_secret); - std::vector plain_secret(gcmcipher.cipher.size()); - - auto primary_pubk = tls::make_public_key( - secret_set.value.primary_public_encryption_key); - - crypto::KeyAesGcm primary_shared_key( - tls::KeyExchangeContext(node_encrypt_kp, primary_pubk) - .compute_shared_secret()); - - if (!primary_shared_key.decrypt( - gcmcipher.hdr.get_iv(), - gcmcipher.hdr.tag, - gcmcipher.cipher, - nullb, - plain_secret.data())) - { - throw std::logic_error( - "Decryption of past network secrets failed"); - } - - has_secrets = true; - - // If the version key is NoVersion, we are rekeying. Use the - // version passed to the hook instead. For recovery, the version - // of the past secrets is passed as the key. - kv::Version secret_version = (v == kv::NoVersion) ? version : v; - - if (is_part_of_public_network()) - { - restored_secrets.push_back( - {secret_version, LedgerSecret(plain_secret)}); - } - else + if (encrypted_secret_for_node.node_id == self) { - // When rekeying, set the encryption key for the next version - // onward (for the backups to deserialise this transaction - // with the old key). The encryptor is in charge of updating - // the ledger secrets on global commit. - encryptor->update_encryption_key( - secret_version + 1, plain_secret); + crypto::GcmCipher gcmcipher; + gcmcipher.deserialise( + encrypted_secret_for_node.encrypted_secret); + std::vector plain_secret(gcmcipher.cipher.size()); + + auto primary_pubk = tls::make_public_key( + secret_set.value.primary_public_encryption_key); + + crypto::KeyAesGcm primary_shared_key( + tls::KeyExchangeContext(node_encrypt_kp, primary_pubk) + .compute_shared_secret()); + + if (!primary_shared_key.decrypt( + gcmcipher.hdr.get_iv(), + gcmcipher.hdr.tag, + gcmcipher.cipher, + nullb, + plain_secret.data())) + { + throw std::logic_error( + "Decryption of past network secrets failed"); + } + + has_secrets = true; + + // If the version key is NoVersion, we are rekeying. Use the + // version passed to the hook instead. For recovery, the version + // of the past secrets is passed as the key. + kv::Version secret_version = (v == kv::NoVersion) ? version : v; + + if (is_part_of_public_network()) + { + restored_secrets.push_back( + {secret_version, LedgerSecret(plain_secret)}); + } + else + { + // When rekeying, set the encryption key for the next version + // onward (for the backups to deserialise this transaction + // with the old key). The encryptor is in charge of updating + // the ledger secrets on global commit. + encryptor->update_encryption_key( + secret_version + 1, plain_secret); + } } } } - } - // When recovering, trigger end of recovery protocol - if (has_secrets && is_part_of_public_network()) - { - restored_secrets.sort( - []( - const LedgerSecrets::VersionedLedgerSecret& a, - const LedgerSecrets::VersionedLedgerSecret& b) { - return a.version < b.version; - }); - - network.ledger_secrets->restore(std::move(restored_secrets)); - backup_finish_recovery(); - } - }); + // When recovering, trigger end of recovery protocol + if (has_secrets && is_part_of_public_network()) + { + restored_secrets.sort( + []( + const LedgerSecrets::VersionedLedgerSecret& a, + const LedgerSecrets::VersionedLedgerSecret& b) { + return a.version < b.version; + }); + + network.ledger_secrets->restore(std::move(restored_secrets)); + backup_finish_recovery(); + } + }); } kv::Version get_last_recovered_commit_idx() override @@ -1427,8 +1423,7 @@ namespace ccf void setup_recovery_hook() { network.shares.set_local_hook( - [this]( - kv::Version version, const Shares::State& s, const Shares::Write& w) { + [this](kv::Version version, const Shares::Write& w) { for (auto& [k, v] : w) { kv::Version ledger_secret_version; @@ -1507,45 +1502,49 @@ namespace ccf // map the node id to a hostname and service and inform raft so that it // can add a new active configuration. network.nodes.set_local_hook( - [this]( - kv::Version version, const Nodes::State& s, const Nodes::Write& w) { + [this](kv::Version version, const Nodes::Write& w) { auto configure = false; - std::unordered_set configuration; + std::unordered_set configuration = + consensus->get_latest_configuration(); for (auto& [node_id, ni] : w) { - switch (ni.value.status) + if (kv::is_deleted(ni.version)) { - case NodeStatus::PENDING: - { - // Pending nodes are not added to consensus until they are - // trusted - break; - } - case NodeStatus::TRUSTED: - { - add_node(node_id, ni.value.nodehost, ni.value.nodeport); - configure = true; - break; - } - case NodeStatus::RETIRED: + configuration.erase(node_id); + } + else + { + switch (ni.value.status) { - configure = true; - break; + case NodeStatus::PENDING: + { + // Pending nodes are not added to consensus until they are + // trusted + break; + } + case NodeStatus::TRUSTED: + { + add_node(node_id, ni.value.nodehost, ni.value.nodeport); + configuration.insert(node_id); + configure = true; + break; + } + case NodeStatus::RETIRED: + { + configuration.erase(node_id); + configure = true; + break; + } + default: + {} } - default: - {} } } if (configure) { - s.foreach([&](NodeId node_id, const Nodes::VersionV& v) { - if (v.value.status == NodeStatus::TRUSTED) - configuration.insert(node_id); - return true; - }); - consensus->add_configuration(version, move(configuration)); + consensus->add_configuration(version, configuration); } }); @@ -1669,16 +1668,14 @@ namespace ccf // When a node is added, even locally, inform the host so that it can // map the node id to a hostname and service and inform pbft network.nodes.set_local_hook( - [this]( - kv::Version version, const Nodes::State& s, const Nodes::Write& w) { - std::unordered_set configuration; + [this](kv::Version version, const Nodes::Write& w) { for (auto& [node_id, ni] : w) { add_node(node_id, ni.value.nodehost, ni.value.nodeport); consensus->add_configuration( version, - configuration, + {}, {node_id, ni.value.nodehost, ni.value.nodeport, ni.value.cert}); } }); diff --git a/src/node/scripts.h b/src/node/scripts.h index 461f03c99a5..8efbfbb79b7 100644 --- a/src/node/scripts.h +++ b/src/node/scripts.h @@ -2,12 +2,16 @@ // Licensed under the Apache 2.0 License. #pragma once -#include "kv/map.h" +#include "kv/experimental.h" #include "script.h" namespace ccf { - using Scripts = kv::Map; + using Scripts = kv::experimental::Map< + std::string, + Script, + kv::experimental::JsonSerialiser, + kv::experimental::JsonSerialiser