Remove duplicate ccf::kv::TxID type (#7477)

Co-authored-by: cjen1-msft <chrisjensen@microsoft.com>

Author: eddyashton

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Changed
 
 - Start nodes now confirm that read-only ledger directories are empty on startup (#7355).
+- In the C++ API, the method `get_txid()` on `ccf::kv::ReadOnlyStore` has been renamed to `current_txid()`. This may affect historical query code which works directly with the returned `StorePtr` (#7477).
 
 ## [7.0.0-dev5]
 

include/ccf/kv/read_only_store.h

@@ -14,7 +14,7 @@ namespace ccf::kv
   public:
     virtual ~ReadOnlyStore() = default;
 
-    virtual ccf::TxID get_txid() = 0;
+    virtual ccf::TxID current_txid() = 0;
     virtual ccf::kv::ReadOnlyTx create_read_only_tx() = 0;
     virtual std::unique_ptr<ccf::kv::ReadOnlyTx> create_read_only_tx_ptr() = 0;
     virtual ccf::kv::TxDiff create_tx_diff() = 0;

samples/apps/logging/logging.cpp

@@ -1721,7 +1721,7 @@ namespace loggingapp
           if (v.has_value())
           {
             LoggingGetHistoricalRange::Entry e;
-            e.seqno = store->get_txid().seqno;
+            e.seqno = store->current_txid().seqno;
             e.id = id;
             e.msg = v.value();
             response.entries.push_back(e);
@@ -1905,7 +1905,7 @@ namespace loggingapp
           if (v.has_value())
           {
             LoggingGetHistoricalRange::Entry e;
-            e.seqno = store->get_txid().seqno;
+            e.seqno = store->current_txid().seqno;
             e.id = id;
             e.msg = v.value();
             response.entries.push_back(e);

src/consensus/aft/raft.h

@@ -1325,7 +1325,7 @@ namespace aft
           return;
         }
 
-        ccf::kv::TxID expected{r.term_of_idx, i};
+        ccf::TxID expected{r.term_of_idx, i};
         auto ds = store->deserialize(entry, public_only, expected);
         if (ds == nullptr)
         {

src/consensus/aft/raft_types.h

@@ -35,12 +35,12 @@ namespace aft
     virtual ~Store() {}
     virtual void compact(Index v) = 0;
     virtual void rollback(
-      const ccf::kv::TxID& tx_id, Term term_of_next_version) = 0;
+      const ccf::TxID& tx_id, Term term_of_next_version) = 0;
     virtual void initialise_term(Term t) = 0;
     virtual std::unique_ptr<ccf::kv::AbstractExecutionWrapper> deserialize(
       const std::vector<uint8_t> data,
       bool public_only = false,
-      const std::optional<ccf::kv::TxID>& expected_txid = std::nullopt) = 0;
+      const std::optional<ccf::TxID>& expected_txid = std::nullopt) = 0;
   };
 
   template <typename T>
@@ -61,8 +61,7 @@ namespace aft
       }
     }
 
-    void rollback(
-      const ccf::kv::TxID& tx_id, Term term_of_next_version) override
+    void rollback(const ccf::TxID& tx_id, Term term_of_next_version) override
     {
       auto p = x.lock();
       if (p)
@@ -83,7 +82,7 @@ namespace aft
     std::unique_ptr<ccf::kv::AbstractExecutionWrapper> deserialize(
       const std::vector<uint8_t> data,
       bool public_only = false,
-      const std::optional<ccf::kv::TxID>& expected_txid = std::nullopt) override
+      const std::optional<ccf::TxID>& expected_txid = std::nullopt) override
     {
       auto p = x.lock();
       if (p)

src/consensus/aft/test/driver.h

@@ -79,14 +79,14 @@ struct LoggingStubStore_Mermaid : public aft::LoggingStubStoreConfig
     aft::LoggingStubStoreConfig::compact(idx);
   }
 
-  void rollback(const ccf::kv::TxID& tx_id, aft::Term t) override
+  void rollback(const ccf::TxID& tx_id, aft::Term t) override
   {
     RAFT_DRIVER_PRINT(
       "{}->>{}: [KV] rolling back to {}.{}, in term {}",
       _id,
       _id,
-      tx_id.term,
-      tx_id.version,
+      tx_id.view,
+      tx_id.seqno,
       t);
     aft::LoggingStubStoreConfig::rollback(tx_id, t);
   }

src/consensus/aft/test/logging_stub.h

@@ -349,7 +349,7 @@ namespace aft
 
     virtual void compact(Index i) {}
 
-    virtual void rollback(const ccf::kv::TxID& tx_id, Term t) {}
+    virtual void rollback(const ccf::TxID& tx_id, Term t) {}
 
     virtual void initialise_term(Term t) {}
 
@@ -373,7 +373,7 @@ namespace aft
     public:
       ExecutionWrapper(
         const std::vector<uint8_t>& data_,
-        const std::optional<ccf::kv::TxID>& expected_txid,
+        const std::optional<ccf::TxID>& expected_txid,
         ccf::kv::ConsensusHookPtrs&& hooks_) :
         hooks(std::move(hooks_))
       {
@@ -390,7 +390,7 @@ namespace aft
 
         if (expected_txid.has_value())
         {
-          if (term != expected_txid->term || index != expected_txid->version)
+          if (term != expected_txid->view || index != expected_txid->seqno)
           {
             result = ccf::kv::ApplyResult::FAIL;
           }
@@ -452,7 +452,7 @@ namespace aft
     virtual std::unique_ptr<ccf::kv::AbstractExecutionWrapper> deserialize(
       const std::vector<uint8_t>& data,
       bool public_only = false,
-      const std::optional<ccf::kv::TxID>& expected_txid = std::nullopt)
+      const std::optional<ccf::TxID>& expected_txid = std::nullopt)
     {
       ccf::kv::ConsensusHookPtrs hooks = {};
       return std::make_unique<ExecutionWrapper>(
@@ -504,20 +504,20 @@ namespace aft
         retired_committed_entries.end());
     }
 
-    virtual void rollback(const ccf::kv::TxID& tx_id, Term t) override
+    virtual void rollback(const ccf::TxID& tx_id, Term t) override
     {
       retired_committed_entries.erase(
         std::remove_if(
           retired_committed_entries.begin(),
           retired_committed_entries.end(),
-          [tx_id](const auto& entry) { return entry.first > tx_id.version; }),
+          [tx_id](const auto& entry) { return entry.first > tx_id.seqno; }),
         retired_committed_entries.end());
     }
 
     virtual std::unique_ptr<ccf::kv::AbstractExecutionWrapper> deserialize(
       const std::vector<uint8_t>& data,
       bool public_only = false,
-      const std::optional<ccf::kv::TxID>& expected_txid = std::nullopt) override
+      const std::optional<ccf::TxID>& expected_txid = std::nullopt) override
     {
       // Set reconfiguration hook if there are any new nodes
       // Read wrapping term and version

src/indexing/indexer.h

@@ -110,7 +110,7 @@ namespace ccf::indexing
           auto stores = transaction_fetcher->fetch_transactions(seqnos);
           for (auto& store : stores)
           {
-            const ccf::TxID tx_id = store->get_txid();
+            const ccf::TxID tx_id = store->current_txid();
 
             for (auto& strategy : strategies)
             {

src/indexing/test/common.h

@@ -191,7 +191,7 @@ static inline bool create_transactions(
       return false;
     }
 
-    const auto seqno = tx.get_txid()->version;
+    const auto seqno = tx.get_txid()->seqno;
 
     for (auto p_exp : modified)
     {

src/indexing/test/indexing.cpp

@@ -227,8 +227,7 @@ TEST_CASE("basic indexing" * doctest::test_suite("indexing"))
   REQUIRE(indexer.install_strategy(index_b));
   REQUIRE_FALSE(indexer.install_strategy(index_b));
 
-  auto current_ = kv_store.current_txid();
-  ccf::TxID current{current_.term, current_.version};
+  ccf::TxID current = kv_store.current_txid();
   REQUIRE(index_a->get_indexed_watermark() == current);
   REQUIRE(index_b->get_indexed_watermark() == ccf::TxID());
 
@@ -832,7 +831,7 @@ TEST_CASE(
 
     while (true)
     {
-      const auto end_seqno = kv_store.get_txid().seqno;
+      const auto end_seqno = kv_store.current_txid().seqno;
       const auto range_end = std::min(end_seqno, range_start + max_range);
 
       auto results =