From 9cf2f46fca6f66027fe36eb39f63c23716510ecc Mon Sep 17 00:00:00 2001
From: Max Tropets <maxtropets@microsoft.com>
Date: Tue, 2 Jul 2024 20:45:52 +0000
Subject: [PATCH 1/4] Add customisable error handling to hist. queries

---
 CHANGELOG.md                               |   3 +-
 include/ccf/historical_queries_adapter.h   |  39 ++++-
 samples/apps/logging/logging.cpp           |   6 +-
 src/apps/js_generic/js_generic_base.cpp    |   2 +-
 src/endpoints/common_endpoint_registry.cpp |   2 +-
 src/js/registry.cpp                        |   2 +-
 src/node/historical_queries_adapter.cpp    | 179 +++++++++++++++++++++
 7 files changed, 223 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 77cff1fd9b2e..93e9430575cc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,9 +27,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Introduce `DynamicJSEndpointRegistry::record_action_for_audit_v1` and `DynamicJSEndpointRegistry::check_action_not_replayed_v1` to allow an application making use of the programmability feature to easily implement auditability, and protect users allowed to update the application against replay attacks (#6285).
 - Endpoints now support a `ToBackup` redirection strategy, for requests which should never be executed on a primary. These must also be read-only. These are configured similar to `ToPrimary` endpoints, with a `to_backup` object (specifying by-role or statically-addressed targets) in each node's configuration.
 
-## Changed
+### Changed
 
 - Updated Open Enclave to [0.19.7](https://github.com/openenclave/openenclave/releases/tag/v0.19.7).
+- `ccf::historical::adapter_v3` becomes deprecated and gets replaced by `ccf::historical::read_only_adapter_v4` and `ccf::historical::read_write_adapter_v4`. Users are now capable of passing a custom error handler to the adapter to customise RPC responses for internal historical queries errors, which are listed in `ccf::historical::HistoricalQueryErrorCode` enum.
 
 ### Removed
 
diff --git a/include/ccf/historical_queries_adapter.h b/include/ccf/historical_queries_adapter.h
index d51308cdd178..37082412eaf0 100644
--- a/include/ccf/historical_queries_adapter.h
+++ b/include/ccf/historical_queries_adapter.h
@@ -39,6 +39,25 @@ namespace ccf::historical
   std::optional<ccf::TxID> txid_from_header(
     endpoints::CommandEndpointContext& args);
 
+  enum class HistoricalQueryErrorCode
+  {
+    InternalError,
+    TransactionPending,
+    TransactionInvalid,
+    TransactionIdMissing,
+    TransactionPartiallyReady,
+  };
+
+  using ErrorHandler = std::function<void(
+    HistoricalQueryErrorCode err,
+    std::string reason,
+    endpoints::CommandEndpointContext& args)>;
+
+  void default_error_handler(
+    HistoricalQueryErrorCode err,
+    std::string reason,
+    endpoints::CommandEndpointContext& args);
+
   enum class HistoricalTxStatus
   {
     Error,
@@ -56,21 +75,35 @@ namespace ccf::historical
     ccf::SeqNo seqno,
     std::string& error_reason);
 
-  ccf::endpoints::EndpointFunction adapter_v3(
+  FMT_DEPRECATED ccf::endpoints::EndpointFunction adapter_v3(
     const HandleHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
     const TxIDExtractor& extractor = txid_from_header);
 
-  ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v3(
+  FMT_DEPRECATED ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v3(
     const HandleReadOnlyHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
     const ReadOnlyTxIDExtractor& extractor = txid_from_header);
 
-  ccf::endpoints::EndpointFunction read_write_adapter_v3(
+  FMT_DEPRECATED ccf::endpoints::EndpointFunction read_write_adapter_v3(
     const HandleReadWriteHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
     const TxIDExtractor& extractor = txid_from_header);
+
+  ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
+    const HandleReadOnlyHistoricalQuery& f,
+    ccf::AbstractNodeContext& node_context,
+    const CheckHistoricalTxStatus& available,
+    const CommandTxIDExtractor& extractor = txid_from_header,
+    const ErrorHandler& ehandler = default_error_handler);
+
+  ccf::endpoints::EndpointFunction read_write_adapter_v4(
+    const HandleReadWriteHistoricalQuery& f,
+    ccf::AbstractNodeContext& node_context,
+    const CheckHistoricalTxStatus& available,
+    const CommandTxIDExtractor& extractor = txid_from_header,
+    const ErrorHandler& ehandler = default_error_handler);
 }
\ No newline at end of file
diff --git a/samples/apps/logging/logging.cpp b/samples/apps/logging/logging.cpp
index 9ab2807b79b0..0f167f99d76d 100644
--- a/samples/apps/logging/logging.cpp
+++ b/samples/apps/logging/logging.cpp
@@ -1279,7 +1279,7 @@ namespace loggingapp
       make_read_only_endpoint(
         "/log/private/historical",
         HTTP_GET,
-        ccf::historical::read_only_adapter_v3(
+        ccf::historical::read_only_adapter_v4(
           get_historical, context, is_tx_committed),
         auth_policies)
         .set_auto_schema<void, LoggingGetHistorical::Out>()
@@ -1329,7 +1329,7 @@ namespace loggingapp
       make_read_only_endpoint(
         "/log/private/historical_receipt",
         HTTP_GET,
-        ccf::historical::read_only_adapter_v3(
+        ccf::historical::read_only_adapter_v4(
           get_historical_with_receipt, context, is_tx_committed),
         auth_policies)
         .set_auto_schema<void, LoggingGetReceipt::Out>()
@@ -1385,7 +1385,7 @@ namespace loggingapp
       make_read_only_endpoint(
         "/log/public/historical_receipt",
         HTTP_GET,
-        ccf::historical::read_only_adapter_v3(
+        ccf::historical::read_only_adapter_v4(
           get_historical_with_receipt_and_claims, context, is_tx_committed),
         auth_policies)
         .set_auto_schema<void, LoggingGetReceipt::Out>()
diff --git a/src/apps/js_generic/js_generic_base.cpp b/src/apps/js_generic/js_generic_base.cpp
index 073ed433048e..1ddb4c9a09bf 100644
--- a/src/apps/js_generic/js_generic_base.cpp
+++ b/src/apps/js_generic/js_generic_base.cpp
@@ -58,7 +58,7 @@ namespace ccf
               consensus, view, seqno, error_reason);
           };
 
-        ccf::historical::adapter_v3(
+        ccf::historical::read_write_adapter_v4(
           [this, endpoint](
             ccf::endpoints::EndpointContext& endpoint_ctx,
             ccf::historical::StatePtr state) {
diff --git a/src/endpoints/common_endpoint_registry.cpp b/src/endpoints/common_endpoint_registry.cpp
index cc87a9e610fc..907a714728e7 100644
--- a/src/endpoints/common_endpoint_registry.cpp
+++ b/src/endpoints/common_endpoint_registry.cpp
@@ -296,7 +296,7 @@ namespace ccf
     make_read_only_endpoint(
       "/receipt",
       HTTP_GET,
-      ccf::historical::read_only_adapter_v3(
+      ccf::historical::read_only_adapter_v4(
         get_receipt, context, is_tx_committed, txid_from_query_string),
       no_auth_required)
       .set_auto_schema<void, nlohmann::json>()
diff --git a/src/js/registry.cpp b/src/js/registry.cpp
index 1675c870c43d..a886c7eea219 100644
--- a/src/js/registry.cpp
+++ b/src/js/registry.cpp
@@ -412,7 +412,7 @@ namespace ccf::js
             consensus, view, seqno, error_reason);
         };
 
-      ccf::historical::adapter_v3(
+      ccf::historical::read_write_adapter_v4(
         [this, endpoint](
           ccf::endpoints::EndpointContext& endpoint_ctx,
           ccf::historical::StatePtr state) {
diff --git a/src/node/historical_queries_adapter.cpp b/src/node/historical_queries_adapter.cpp
index 3ca36ca16376..d99b6a6d5a93 100644
--- a/src/node/historical_queries_adapter.cpp
+++ b/src/node/historical_queries_adapter.cpp
@@ -190,6 +190,57 @@ namespace ccf::historical
     return tx_id_opt;
   }
 
+  void default_error_handler(
+    HistoricalQueryErrorCode err,
+    std::string reason,
+    endpoints::CommandEndpointContext& args)
+  {
+    if (err == HistoricalQueryErrorCode::InternalError)
+    {
+      args.rpc_ctx->set_error(
+        HTTP_STATUS_INTERNAL_SERVER_ERROR,
+        ccf::errors::TransactionPendingOrUnknown,
+        std::move(reason));
+    }
+    else if (err == HistoricalQueryErrorCode::TransactionPending)
+    {
+      args.rpc_ctx->set_response_header(
+        http::headers::CACHE_CONTROL, "no-cache");
+      args.rpc_ctx->set_error(
+        HTTP_STATUS_NOT_FOUND,
+        ccf::errors::TransactionPendingOrUnknown,
+        std::move(reason));
+    }
+    else if (err == HistoricalQueryErrorCode::TransactionInvalid)
+    {
+      args.rpc_ctx->set_error(
+        HTTP_STATUS_NOT_FOUND,
+        ccf::errors::TransactionInvalid,
+        std::move(reason));
+    }
+    else if (err == HistoricalQueryErrorCode::TransactionIdMissing)
+    {
+      args.rpc_ctx->set_error(
+        HTTP_STATUS_NOT_FOUND,
+        ccf::errors::TransactionInvalid,
+        std::move(reason));
+    }
+    else if (err == HistoricalQueryErrorCode::TransactionPartiallyReady)
+    {
+      args.rpc_ctx->set_response_status(HTTP_STATUS_ACCEPTED);
+      constexpr size_t retry_after_seconds = 3;
+      args.rpc_ctx->set_response_header(
+        http::headers::RETRY_AFTER, retry_after_seconds);
+      args.rpc_ctx->set_response_header(
+        http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT);
+      args.rpc_ctx->set_response_body(std::move(reason));
+    }
+    else
+    {
+      LOG_FAIL_FMT("Unexpected historical query error {}", err);
+    }
+  }
+
   HistoricalTxStatus is_tx_committed_v2(
     ccf::kv::Consensus* consensus,
     ccf::View view,
@@ -374,4 +425,132 @@ namespace ccf::historical
       ccf::endpoints::EndpointFunction,
       ccf::endpoints::EndpointContext>(f, node_context, available, extractor);
   }
+
+  template <
+    class TQueryHandler,
+    class TEndpointFunction,
+    class TEndpointContext>
+  TEndpointFunction _adapter_v4(
+    const TQueryHandler& f,
+    ccf::AbstractNodeContext& node_context,
+    const CheckHistoricalTxStatus& available,
+    const CommandTxIDExtractor& extractor,
+    const ErrorHandler& ehandler)
+  {
+    auto& state_cache = node_context.get_historical_state();
+    auto network_identity_subsystem =
+      node_context.get_subsystem<NetworkIdentitySubsystemInterface>();
+
+    return [f,
+            &state_cache,
+            network_identity_subsystem,
+            available,
+            extractor,
+            ehandler](TEndpointContext& args) {
+      // Extract the requested transaction ID
+      ccf::TxID target_tx_id;
+      {
+        const auto tx_id_opt = extractor(args);
+        if (tx_id_opt.has_value())
+        {
+          target_tx_id = tx_id_opt.value();
+        }
+        else
+        {
+          ehandler(
+            HistoricalQueryErrorCode::TransactionIdMissing,
+            "Could not extract TX ID",
+            args);
+          return;
+        }
+      }
+
+      // Check that the requested transaction ID is available
+      {
+        auto error_reason = fmt::format(
+          "Transaction {} is not available.", target_tx_id.to_str());
+        auto is_available =
+          available(target_tx_id.view, target_tx_id.seqno, error_reason);
+
+        switch (is_available)
+        {
+          case HistoricalTxStatus::Error:
+            ehandler(
+              HistoricalQueryErrorCode::InternalError,
+              std::move(error_reason),
+              args);
+            return;
+          case HistoricalTxStatus::PendingOrUnknown:
+            ehandler(
+              HistoricalQueryErrorCode::TransactionPending,
+              std::move(error_reason),
+              args);
+            return;
+          case HistoricalTxStatus::Invalid:
+            ehandler(
+              HistoricalQueryErrorCode::TransactionInvalid,
+              std::move(error_reason),
+              args);
+            return;
+          case HistoricalTxStatus::Valid:
+            break;
+        }
+      }
+
+      // We need a handle to determine whether this request is the 'same' as a
+      // previous one. For simplicity we use target_tx_id.seqno. This means we
+      // keep a lot of state around for old requests! It should be cleaned up
+      // manually
+      const auto historic_request_handle = target_tx_id.seqno;
+
+      // Get a state at the target version from the cache, if it is present
+      auto historical_state =
+        state_cache.get_state_at(historic_request_handle, target_tx_id.seqno);
+      if (
+        historical_state == nullptr ||
+        (!populate_service_endorsements(
+          args.tx, historical_state, state_cache, network_identity_subsystem)))
+      {
+        auto reason = fmt::format(
+          "Historical transaction {} is not currently available.",
+          target_tx_id.to_str());
+        ehandler(
+          HistoricalQueryErrorCode::TransactionPartiallyReady,
+          std::move(reason),
+          args);
+        return;
+      }
+
+      // Call the provided handler
+      f(args, historical_state);
+    };
+  }
+
+  ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
+    const HandleReadOnlyHistoricalQuery& f,
+    ccf::AbstractNodeContext& node_context,
+    const CheckHistoricalTxStatus& available,
+    const CommandTxIDExtractor& extractor,
+    const ErrorHandler& ehandler)
+  {
+    return _adapter_v4<
+      HandleReadOnlyHistoricalQuery,
+      ccf::endpoints::ReadOnlyEndpointFunction,
+      ccf::endpoints::ReadOnlyEndpointContext>(
+      f, node_context, available, extractor, ehandler);
+  }
+
+  ccf::endpoints::EndpointFunction read_write_adapter_v4(
+    const HandleReadWriteHistoricalQuery& f,
+    ccf::AbstractNodeContext& node_context,
+    const CheckHistoricalTxStatus& available,
+    const CommandTxIDExtractor& extractor,
+    const ErrorHandler& ehandler)
+  {
+    return _adapter_v4<
+      HandleReadWriteHistoricalQuery,
+      ccf::endpoints::EndpointFunction,
+      ccf::endpoints::EndpointContext>(
+      f, node_context, available, extractor, ehandler);
+  }
 }
\ No newline at end of file

From 7b1f391e9e7aae065e766da2667a1a0acea868c5 Mon Sep 17 00:00:00 2001
From: Max Tropets <maxtropets@microsoft.com>
Date: Wed, 3 Jul 2024 12:10:18 +0000
Subject: [PATCH 2/4] Code review changes

---
 CHANGELOG.md                             |   6 +-
 include/ccf/historical_queries_adapter.h |  22 +++--
 src/node/historical_queries_adapter.cpp  | 105 ++++++++++++-----------
 3 files changed, 74 insertions(+), 59 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93e9430575cc..45ab64980395 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,11 +26,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - The `programmability` sample app now demonstrates how applications can define their own extensions, creating bindings between C++ and JS state, and allowing JS endpoints to call functions implemented in C++.
 - Introduce `DynamicJSEndpointRegistry::record_action_for_audit_v1` and `DynamicJSEndpointRegistry::check_action_not_replayed_v1` to allow an application making use of the programmability feature to easily implement auditability, and protect users allowed to update the application against replay attacks (#6285).
 - Endpoints now support a `ToBackup` redirection strategy, for requests which should never be executed on a primary. These must also be read-only. These are configured similar to `ToPrimary` endpoints, with a `to_backup` object (specifying by-role or statically-addressed targets) in each node's configuration.
+- Introduced `ccf::historical::read_only_adapter_v4` and `ccf::historical::read_write_adapter_v4`. Users are now capable of passing a custom error handler to the adapter to customise RPC responses for internal historical queries errors, which are listed in `ccf::historical::HistoricalQueryErrorCode` enum.
 
 ### Changed
 
 - Updated Open Enclave to [0.19.7](https://github.com/openenclave/openenclave/releases/tag/v0.19.7).
-- `ccf::historical::adapter_v3` becomes deprecated and gets replaced by `ccf::historical::read_only_adapter_v4` and `ccf::historical::read_write_adapter_v4`. Users are now capable of passing a custom error handler to the adapter to customise RPC responses for internal historical queries errors, which are listed in `ccf::historical::HistoricalQueryErrorCode` enum.
+
+### Deprecated
+
+- `ccf::historical::adapter_v3` becomes deprecated in favour of `_v4` version.
 
 ### Removed
 
diff --git a/include/ccf/historical_queries_adapter.h b/include/ccf/historical_queries_adapter.h
index 37082412eaf0..01abd9720227 100644
--- a/include/ccf/historical_queries_adapter.h
+++ b/include/ccf/historical_queries_adapter.h
@@ -51,7 +51,12 @@ namespace ccf::historical
   using ErrorHandler = std::function<void(
     HistoricalQueryErrorCode err,
     std::string reason,
-    endpoints::CommandEndpointContext& args)>;
+    endpoints::EndpointContext& args)>;
+
+  using ReadOnlyErrorHandler = std::function<void(
+    HistoricalQueryErrorCode err,
+    std::string reason,
+    endpoints::ReadOnlyEndpointContext& args)>;
 
   void default_error_handler(
     HistoricalQueryErrorCode err,
@@ -75,19 +80,22 @@ namespace ccf::historical
     ccf::SeqNo seqno,
     std::string& error_reason);
 
-  FMT_DEPRECATED ccf::endpoints::EndpointFunction adapter_v3(
+  CCF_DEPRECATED("Replaced by _v4")
+  ccf::endpoints::EndpointFunction adapter_v3(
     const HandleHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
     const TxIDExtractor& extractor = txid_from_header);
 
-  FMT_DEPRECATED ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v3(
+  CCF_DEPRECATED("Replaced by _v4")
+  ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v3(
     const HandleReadOnlyHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
     const ReadOnlyTxIDExtractor& extractor = txid_from_header);
 
-  FMT_DEPRECATED ccf::endpoints::EndpointFunction read_write_adapter_v3(
+  CCF_DEPRECATED("Replaced by _v4")
+  ccf::endpoints::EndpointFunction read_write_adapter_v3(
     const HandleReadWriteHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
@@ -97,13 +105,13 @@ namespace ccf::historical
     const HandleReadOnlyHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
-    const CommandTxIDExtractor& extractor = txid_from_header,
-    const ErrorHandler& ehandler = default_error_handler);
+    const ReadOnlyTxIDExtractor& extractor = txid_from_header,
+    const ReadOnlyErrorHandler& ehandler = default_error_handler);
 
   ccf::endpoints::EndpointFunction read_write_adapter_v4(
     const HandleReadWriteHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
-    const CommandTxIDExtractor& extractor = txid_from_header,
+    const TxIDExtractor& extractor = txid_from_header,
     const ErrorHandler& ehandler = default_error_handler);
 }
\ No newline at end of file
diff --git a/src/node/historical_queries_adapter.cpp b/src/node/historical_queries_adapter.cpp
index d99b6a6d5a93..c00176ce5d1e 100644
--- a/src/node/historical_queries_adapter.cpp
+++ b/src/node/historical_queries_adapter.cpp
@@ -195,49 +195,50 @@ namespace ccf::historical
     std::string reason,
     endpoints::CommandEndpointContext& args)
   {
-    if (err == HistoricalQueryErrorCode::InternalError)
+    switch (err)
     {
-      args.rpc_ctx->set_error(
-        HTTP_STATUS_INTERNAL_SERVER_ERROR,
-        ccf::errors::TransactionPendingOrUnknown,
-        std::move(reason));
-    }
-    else if (err == HistoricalQueryErrorCode::TransactionPending)
-    {
-      args.rpc_ctx->set_response_header(
-        http::headers::CACHE_CONTROL, "no-cache");
-      args.rpc_ctx->set_error(
-        HTTP_STATUS_NOT_FOUND,
-        ccf::errors::TransactionPendingOrUnknown,
-        std::move(reason));
-    }
-    else if (err == HistoricalQueryErrorCode::TransactionInvalid)
-    {
-      args.rpc_ctx->set_error(
-        HTTP_STATUS_NOT_FOUND,
-        ccf::errors::TransactionInvalid,
-        std::move(reason));
-    }
-    else if (err == HistoricalQueryErrorCode::TransactionIdMissing)
-    {
-      args.rpc_ctx->set_error(
-        HTTP_STATUS_NOT_FOUND,
-        ccf::errors::TransactionInvalid,
-        std::move(reason));
-    }
-    else if (err == HistoricalQueryErrorCode::TransactionPartiallyReady)
-    {
-      args.rpc_ctx->set_response_status(HTTP_STATUS_ACCEPTED);
-      constexpr size_t retry_after_seconds = 3;
-      args.rpc_ctx->set_response_header(
-        http::headers::RETRY_AFTER, retry_after_seconds);
-      args.rpc_ctx->set_response_header(
-        http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT);
-      args.rpc_ctx->set_response_body(std::move(reason));
-    }
-    else
-    {
-      LOG_FAIL_FMT("Unexpected historical query error {}", err);
+      case HistoricalQueryErrorCode::InternalError:
+      {
+        args.rpc_ctx->set_error(
+          HTTP_STATUS_INTERNAL_SERVER_ERROR,
+          ccf::errors::TransactionPendingOrUnknown,
+          std::move(reason));
+        break;
+      }
+      case HistoricalQueryErrorCode::TransactionPending:
+      {
+        args.rpc_ctx->set_response_header(
+          http::headers::CACHE_CONTROL, "no-cache");
+        args.rpc_ctx->set_error(
+          HTTP_STATUS_NOT_FOUND,
+          ccf::errors::TransactionPendingOrUnknown,
+          std::move(reason));
+        break;
+      }
+      case HistoricalQueryErrorCode::TransactionInvalid:
+      case HistoricalQueryErrorCode::TransactionIdMissing:
+      {
+        args.rpc_ctx->set_error(
+          HTTP_STATUS_NOT_FOUND,
+          ccf::errors::TransactionInvalid,
+          std::move(reason));
+        break;
+      }
+      case HistoricalQueryErrorCode::TransactionPartiallyReady:
+      {
+        args.rpc_ctx->set_response_status(HTTP_STATUS_ACCEPTED);
+        constexpr size_t retry_after_seconds = 3;
+        args.rpc_ctx->set_response_header(
+          http::headers::RETRY_AFTER, retry_after_seconds);
+        args.rpc_ctx->set_response_header(
+          http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT);
+        args.rpc_ctx->set_response_body(std::move(reason));
+        break;
+      }
+      default:
+      {
+        LOG_FAIL_FMT("Unexpected historical query error {}", err);
+      }
     }
   }
 
@@ -429,13 +430,15 @@ namespace ccf::historical
   template <
     class TQueryHandler,
     class TEndpointFunction,
-    class TEndpointContext>
+    class TEndpointContext,
+    class TTxIDExtractor,
+    class TErrorHandler>
   TEndpointFunction _adapter_v4(
     const TQueryHandler& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
-    const CommandTxIDExtractor& extractor,
-    const ErrorHandler& ehandler)
+    const TTxIDExtractor& extractor,
+    const TErrorHandler& ehandler)
   {
     auto& state_cache = node_context.get_historical_state();
     auto network_identity_subsystem =
@@ -526,12 +529,12 @@ namespace ccf::historical
     };
   }
 
-  ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
+ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
     const HandleReadOnlyHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
-    const CommandTxIDExtractor& extractor,
-    const ErrorHandler& ehandler)
+    const ReadOnlyTxIDExtractor& extractor,
+    const ReadOnlyErrorHandler& ehandler)
   {
     return _adapter_v4<
       HandleReadOnlyHistoricalQuery,
@@ -540,11 +543,11 @@ namespace ccf::historical
       f, node_context, available, extractor, ehandler);
   }
 
-  ccf::endpoints::EndpointFunction read_write_adapter_v4(
+ ccf::endpoints::EndpointFunction read_write_adapter_v4(
     const HandleReadWriteHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
-    const CommandTxIDExtractor& extractor,
+    const TxIDExtractor& extractor,
     const ErrorHandler& ehandler)
   {
     return _adapter_v4<
@@ -553,4 +556,4 @@ namespace ccf::historical
       ccf::endpoints::EndpointContext>(
       f, node_context, available, extractor, ehandler);
   }
-}
\ No newline at end of file
+}

From 859de528d3d8b40ecac1f92a4f0c309c5d0983d0 Mon Sep 17 00:00:00 2001
From: Max Tropets <maxtropets@microsoft.com>
Date: Wed, 3 Jul 2024 12:14:15 +0000
Subject: [PATCH 3/4] CR: reuser adater_v4 in adapter_v3

---
 src/node/historical_queries_adapter.cpp | 92 +------------------------
 1 file changed, 2 insertions(+), 90 deletions(-)

diff --git a/src/node/historical_queries_adapter.cpp b/src/node/historical_queries_adapter.cpp
index c00176ce5d1e..f5c3d5dc9c91 100644
--- a/src/node/historical_queries_adapter.cpp
+++ b/src/node/historical_queries_adapter.cpp
@@ -298,96 +298,8 @@ namespace ccf::historical
     const CheckHistoricalTxStatus& available,
     const TTxIDExtractor& extractor)
   {
-    auto& state_cache = node_context.get_historical_state();
-    auto network_identity_subsystem =
-      node_context.get_subsystem<NetworkIdentitySubsystemInterface>();
-
-    return [f, &state_cache, network_identity_subsystem, available, extractor](
-             TEndpointContext& args) {
-      // Extract the requested transaction ID
-      ccf::TxID target_tx_id;
-      {
-        const auto tx_id_opt = extractor(args);
-        if (tx_id_opt.has_value())
-        {
-          target_tx_id = tx_id_opt.value();
-        }
-        else
-        {
-          return;
-        }
-      }
-
-      // Check that the requested transaction ID is available
-      {
-        auto error_reason = fmt::format(
-          "Transaction {} is not available.", target_tx_id.to_str());
-        auto is_available =
-          available(target_tx_id.view, target_tx_id.seqno, error_reason);
-        switch (is_available)
-        {
-          case HistoricalTxStatus::Error:
-          {
-            args.rpc_ctx->set_error(
-              HTTP_STATUS_INTERNAL_SERVER_ERROR,
-              ccf::errors::TransactionPendingOrUnknown,
-              std::move(error_reason));
-            return;
-          }
-          case HistoricalTxStatus::PendingOrUnknown:
-          {
-            // Set header No-Cache
-            args.rpc_ctx->set_response_header(
-              http::headers::CACHE_CONTROL, "no-cache");
-            args.rpc_ctx->set_error(
-              HTTP_STATUS_NOT_FOUND,
-              ccf::errors::TransactionPendingOrUnknown,
-              std::move(error_reason));
-            return;
-          }
-          case HistoricalTxStatus::Invalid:
-          {
-            args.rpc_ctx->set_error(
-              HTTP_STATUS_NOT_FOUND,
-              ccf::errors::TransactionInvalid,
-              std::move(error_reason));
-            return;
-          }
-          case HistoricalTxStatus::Valid:
-          {
-          }
-        }
-      }
-
-      // We need a handle to determine whether this request is the 'same' as a
-      // previous one. For simplicity we use target_tx_id.seqno. This means we
-      // keep a lot of state around for old requests! It should be cleaned up
-      // manually
-      const auto historic_request_handle = target_tx_id.seqno;
-
-      // Get a state at the target version from the cache, if it is present
-      auto historical_state =
-        state_cache.get_state_at(historic_request_handle, target_tx_id.seqno);
-      if (
-        historical_state == nullptr ||
-        (!populate_service_endorsements(
-          args.tx, historical_state, state_cache, network_identity_subsystem)))
-      {
-        args.rpc_ctx->set_response_status(HTTP_STATUS_ACCEPTED);
-        constexpr size_t retry_after_seconds = 3;
-        args.rpc_ctx->set_response_header(
-          http::headers::RETRY_AFTER, retry_after_seconds);
-        args.rpc_ctx->set_response_header(
-          http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT);
-        args.rpc_ctx->set_response_body(fmt::format(
-          "Historical transaction {} is not currently available.",
-          target_tx_id.to_str()));
-        return;
-      }
-
-      // Call the provided handler
-      f(args, historical_state);
-    };
+    return _adapter_v4<TQueryHandler, TEndpointFunction, TEndpointContext>(
+      f, node_context, available, extractor, default_error_handler);
   }
 
   ccf::endpoints::EndpointFunction adapter_v3(

From aeecb0d6f611b41f312f10e38d608e173d65c8bb Mon Sep 17 00:00:00 2001
From: Max Tropets <maxtropets@microsoft.com>
Date: Wed, 3 Jul 2024 12:24:45 +0000
Subject: [PATCH 4/4] Whitespace

---
 src/node/historical_queries_adapter.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/node/historical_queries_adapter.cpp b/src/node/historical_queries_adapter.cpp
index f5c3d5dc9c91..260aaabcaa9d 100644
--- a/src/node/historical_queries_adapter.cpp
+++ b/src/node/historical_queries_adapter.cpp
@@ -441,7 +441,7 @@ namespace ccf::historical
     };
   }
 
-ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
+  ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
     const HandleReadOnlyHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,
@@ -455,7 +455,7 @@ ccf::endpoints::ReadOnlyEndpointFunction read_only_adapter_v4(
       f, node_context, available, extractor, ehandler);
   }
 
- ccf::endpoints::EndpointFunction read_write_adapter_v4(
+  ccf::endpoints::EndpointFunction read_write_adapter_v4(
     const HandleReadWriteHistoricalQuery& f,
     ccf::AbstractNodeContext& node_context,
     const CheckHistoricalTxStatus& available,