Avoid mixing up app/non-app historical state handles

doc/schemas/app_openapi.json

@@ -798,6 +798,55 @@
         }
       }
     },
+    "/app/log/private/historical/handle": {
+      "get": {
+        "operationId": "GetAppLogPrivateHistoricalHandle",
+        "parameters": [
+          {
+            "in": "query",
+            "name": "handle",
+            "required": true,
+            "schema": {
+              "$ref": "#/components/schemas/uint64"
+            }
+          },
+          {
+            "in": "query",
+            "name": "seqno",
+            "required": true,
+            "schema": {
+              "$ref": "#/components/schemas/uint64"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/json"
+                }
+              }
+            },
+            "description": "Default response description"
+          },
+          "default": {
+            "$ref": "#/components/responses/default"
+          }
+        },
+        "security": [
+          {
+            "jwt": []
+          },
+          {
+            "user_cose_sign1": []
+          }
+        ],
+        "x-ccf-forwarding": {
+          "$ref": "#/components/x-ccf-forwarding/never"
+        }
+      }
+    },
     "/app/log/private/historical/sparse": {
       "get": {
         "operationId": "GetAppLogPrivateHistoricalSparse",

samples/apps/logging/logging.cpp

@@ -13,11 +13,13 @@
 #include "ccf/ds/hash.h"
 #include "ccf/endpoints/authentication/all_of_auth.h"
 #include "ccf/historical_queries_adapter.h"
+#include "ccf/historical_queries_utils.h"
 #include "ccf/http_etag.h"
 #include "ccf/http_query.h"
 #include "ccf/indexing/strategies/seqnos_by_key_bucketed.h"
 #include "ccf/indexing/strategy.h"
 #include "ccf/json_handler.h"
+#include "ccf/network_identity_interface.h"
 #include "ccf/version.h"
 
 #include <charconv>
@@ -1315,6 +1317,97 @@ namespace loggingapp
         .install();
       // SNIPPET_END: get_historical
 
+      // SNIPPET_START: get_historical_with_handle
+      auto get_historical_with_handle =
+        [this](ccf::endpoints::ReadOnlyEndpointContext& ctx) {
+          using namespace ccf::historical;
+
+          const auto parsed_query =
+            ccf::http::parse_query(ctx.rpc_ctx->get_request_query());
+
+          std::string error_reason{};
+          size_t handle = 0;
+          if (!ccf::http::get_query_value(
+                parsed_query, "handle", handle, error_reason))
+          {
+            ctx.rpc_ctx->set_error(
+              HTTP_STATUS_BAD_REQUEST,
+              ccf::errors::InvalidQueryParameterValue,
+              std::move(error_reason));
+            return;
+          }
+
+          size_t seqno = 0;
+          error_reason.clear();
+          if (!ccf::http::get_query_value(
+                parsed_query, "seqno", seqno, error_reason))
+          {
+            ctx.rpc_ctx->set_error(
+              HTTP_STATUS_BAD_REQUEST,
+              ccf::errors::InvalidQueryParameterValue,
+              std::move(error_reason));
+            return;
+          }
+
+          error_reason.clear();
+          const auto tx_status = get_tx_status(seqno);
+          if (
+            !tx_status.has_value() ||
+            tx_status.value() != ccf::TxStatus::Committed)
+          {
+            ctx.rpc_ctx->set_error(
+              HTTP_STATUS_BAD_REQUEST,
+              ccf::errors::InvalidHeaderValue,
+              "Requested historical TxID is not committed");
+            return;
+          }
+
+          auto& state_cache = context.get_historical_state();
+          auto state = state_cache.get_state_at(handle, seqno);
+
+          if (!state)
+          {
+            default_error_handler(
+              HistoricalQueryErrorCode::TransactionPartiallyReady,
+              "Pending",
+              ctx);
+            return;
+          }
+          auto network_identity_subsystem =
+            context.get_subsystem<ccf::NetworkIdentitySubsystemInterface>();
+
+          if (!populate_service_endorsements(
+                ctx.tx, state, state_cache, network_identity_subsystem))
+          {
+            default_error_handler(
+              HistoricalQueryErrorCode::TransactionPartiallyReady,
+              "Pending",
+              ctx);
+            return;
+          }
+
+          if (state->receipt)
+          {
+            auto j = describe_receipt_v1(*state->receipt);
+            ccf::jsonhandler::set_response(std::move(j), ctx.rpc_ctx);
+          }
+          else
+          {
+            ctx.rpc_ctx->set_response_status(HTTP_STATUS_NO_CONTENT);
+          }
+        };
+      make_read_only_endpoint(
+        "/log/private/historical/handle",
+        HTTP_GET,
+        get_historical_with_handle,
+        auth_policies)
+        .set_auto_schema<void, nlohmann::json>()
+        .add_query_parameter<size_t>("handle")
+        .add_query_parameter<size_t>("seqno")
+        .set_forwarding_required(ccf::endpoints::ForwardingRequired::Never)
+        .install();
+      // SNIPPET_END: get_historical_with_handle
+
       // SNIPPET_START: get_historical_with_receipt
       auto get_historical_with_receipt =
         [this](

src/node/historical_queries.h

@@ -34,7 +34,6 @@ namespace ccf::historical
   };
 
   using CompoundHandle = std::pair<RequestNamespace, RequestHandle>;
-
 };
 
 FMT_BEGIN_NAMESPACE

src/node/historical_queries_adapter.cpp

@@ -213,7 +213,7 @@ namespace ccf
   std::optional<std::vector<uint8_t>> describe_merkle_proof_v1(
     const TxReceiptImpl& receipt)
   {
-    constexpr size_t buf_size = 2048; // TBD: calculate why this is enough
+    constexpr size_t buf_size = 2048;
     std::vector<uint8_t> underlying_buffer(buf_size);
     UsefulBuf buffer{underlying_buffer.data(), underlying_buffer.size()};
     assert(buffer.len == buf_size);

src/node/historical_queries_utils.cpp

@@ -8,6 +8,7 @@
 #include "ccf/service/tables/service.h"
 #include "consensus/aft/raft_types.h"
 #include "kv/kv_types.h"
+#include "node/historical_queries.h"
 #include "node/identity.h"
 #include "node/tx_receipt_impl.h"
 #include "service/tables/previous_service_identity.h"
@@ -111,6 +112,12 @@ namespace
     }
   }
 
+  ccf::historical::CompoundHandle make_system_handle(ccf::SeqNo seq)
+  {
+    return ccf::historical::CompoundHandle{
+      ccf::historical::RequestNamespace::System, seq};
+  }
+
   bool keep_fetching(ccf::SeqNo target_seq)
   {
     return !is_self_endorsement(cose_endorsements_cache.back()) &&
@@ -136,8 +143,19 @@ namespace
       }
       const auto prev_endorsement_seqno =
         last_cose_endorsement.previous_version.value();
-      const auto hstate = state_cache.get_state_at(
-        prev_endorsement_seqno, prev_endorsement_seqno);
+
+      const auto system_handle = make_system_handle(prev_endorsement_seqno);
+      auto* cache_impl =
+        dynamic_cast<ccf::historical::StateCacheImpl*>(&state_cache);
+      if (cache_impl == nullptr)
+      {
+        throw std::logic_error(
+          "StateCacheImpl required to access cache as "
+          "RequestNamespace::System");
+      }
+
+      const auto hstate =
+        cache_impl->get_state_at(system_handle, prev_endorsement_seqno);
 
       if (!hstate)
       {
@@ -258,7 +276,18 @@ namespace ccf
         }
         i = hservice_info->previous_service_identity_version.value_or(i - 1);
         LOG_TRACE_FMT("historical service identity search at: {}", i);
-        auto hstate = state_cache.get_state_at(i, i);
+
+        const auto system_handle = make_system_handle(i);
+        auto* cache_impl =
+          dynamic_cast<ccf::historical::StateCacheImpl*>(&state_cache);
+        if (cache_impl == nullptr)
+        {
+          throw std::logic_error(
+            "StateCacheImpl required to access cache as "
+            "RequestNamespace::System");
+        }
+
+        auto hstate = cache_impl->get_state_at(system_handle, i);
         if (!hstate)
         {
           return std::nullopt; // Not available yet - retry later.

tests/recovery.py

@@ -475,6 +475,25 @@ def test_recover_service_with_wrong_identity(network, args):
         shifted_tx(curr_tx_id, 0, -3),
     ]
 
+    with primary.client("user0") as client:
+
+        def pull_with_handle():
+            # Receipts for previous service instances require back-endorsement.
+            # In this case it should trigger reading pulling up state
+            # for previous_service_created_tx_id, which will have an overlapping
+            # seqno with the target tx, but this has to work just fine due to
+            # App/Sys handle split.
+            return client.get(
+                f"/log/private/historical/handle?seqno={previous_service_created_tx_id.seqno + 1}&handle={previous_service_created_tx_id.seqno}"
+            ).status_code
+
+        for _ in range(10):
+            if pull_with_handle() == http.HTTPStatus.OK:
+                break
+            time.sleep(0.5)
+        else:
+            assert False, "Could not get a receipt with a custom handle"
+
     for tx in txids:
         receipt = primary.get_receipt(tx.view, tx.seqno).json()