replace Unknown content with error variant

ethportal-api/README.md

@@ -11,7 +11,7 @@ Enable `client` feature of `ethportal-api` crate.
 ```rust,no_run
 use ethportal_api::jsonrpsee::http_client::HttpClientBuilder;
 use ethportal_api::{
-    HistoryContentItem, HistoryContentKey, HistoryNetworkApiClient, Web3ApiClient,
+    HistoryContentValue, HistoryContentKey, HistoryNetworkApiClient, Web3ApiClient,
 };
 
 #[tokio::main]
@@ -31,7 +31,7 @@ async fn main() {
 
     // Deserialise to a portal history content key type from a hex string
     let content_key: HistoryContentKey = serde_json::from_str(content_key_json).unwrap();
-    let content_item: HistoryContentItem = serde_json::from_str(content_item_json).unwrap();
+    let content_item: HistoryContentValue = serde_json::from_str(content_item_json).unwrap();
 
     // Store content to remote node, call portal_historyStore endpoint
     let result: bool = client
@@ -40,8 +40,8 @@ async fn main() {
         .unwrap();
     assert!(result);
 
-    // Call portal_historyLocalContent endpoint and deserialize to `HistoryContentItem::BlockHeaderWithProof` type
-    let result: HistoryContentItem = client.local_content(content_key).await.unwrap();
+    // Call portal_historyLocalContent endpoint and deserialize to `HistoryContentValue::BlockHeaderWithProof` type
+    let result: HistoryContentValue = client.local_content(content_key).await.unwrap();
     assert_eq!(result, content_item);
 }
 ```

ethportal-api/src/history.rs

@@ -8,7 +8,7 @@ use crate::types::{
 };
 use jsonrpsee::{core::RpcResult, proc_macros::rpc};
 use trin_types::content_key::HistoryContentKey;
-use trin_types::content_value::HistoryContentValue;
+use trin_types::content_value::{HistoryContentValue, PossibleHistoryContentValue};
 
 /// Portal History JSON-RPC endpoints
 #[rpc(client, server, namespace = "portal")]
@@ -63,7 +63,7 @@ pub trait HistoryNetworkApi {
     async fn recursive_find_content(
         &self,
         content_key: HistoryContentKey,
-    ) -> RpcResult<HistoryContentValue>;
+    ) -> RpcResult<PossibleHistoryContentValue>;
 
     /// Lookup a target content key in the network. Return tracing info.
     #[method(name = "historyTraceRecursiveFindContent")]
@@ -109,6 +109,8 @@ pub trait HistoryNetworkApi {
 
     /// Get a content from the local database
     #[method(name = "historyLocalContent")]
-    async fn local_content(&self, content_key: HistoryContentKey)
-        -> RpcResult<HistoryContentValue>;
+    async fn local_content(
+        &self,
+        content_key: HistoryContentKey,
+    ) -> RpcResult<PossibleHistoryContentValue>;
 }

ethportal-api/src/types/portal.rs

@@ -2,7 +2,7 @@ use crate::types::enr::Enr;
 use serde::{Deserialize, Serialize};
 use ssz_types::{typenum, BitList};
 use trin_types::content_key::HistoryContentKey;
-use trin_types::content_value::HistoryContentValue;
+use trin_types::content_value::{HistoryContentValue, PossibleHistoryContentValue};
 
 pub type DataRadius = ethereum_types::U256;
 pub type Distance = ethereum_types::U256;
@@ -50,11 +50,14 @@ pub struct AcceptInfo {
     pub content_keys: BitList<typenum::U8>,
 }
 
-/// Response for TraceRecursiveFindContent endpoint
+/// Parsed response for TraceRecursiveFindContent endpoint
+///
+/// The RPC response encodes absent content as "0x". This struct
+/// represents the content info, using None for absent content.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct TraceContentInfo {
-    pub content: HistoryContentValue,
+    pub content: PossibleHistoryContentValue,
     pub route: Vec<NodeInfo>,
 }
 

ethportal-peertest/src/jsonrpc.rs

@@ -8,6 +8,7 @@ use hyper::{self, Body, Client, Method, Request};
 use serde_json::{self, json, Value};
 use ssz::Encode;
 use tracing::{error, info};
+use trin_types::constants::CONTENT_ABSENT;
 
 use crate::{cli::PeertestConfig, Peertest};
 use trin_types::distance::Distance;
@@ -203,7 +204,7 @@ fn validate_discv5_node_info(val: &Value, _peertest: &Peertest) {
 fn validate_discv5_routing_table_info(val: &Value, _peertest: &Peertest) {
     let local_key = val.get("localNodeId").unwrap();
     assert!(local_key.is_string());
-    assert!(local_key.as_str().unwrap().contains("0x"));
+    assert!(local_key.as_str().unwrap().starts_with("0x"));
     assert!(val.get("buckets").unwrap().is_array());
 }
 
@@ -254,7 +255,7 @@ pub fn validate_portal_offer(result: AcceptInfo, _peertest: &Peertest) {
 }
 
 pub fn validate_portal_local_content(result: &Value, _peertest: &Peertest) {
-    assert_eq!(result.as_str().unwrap(), "0x0");
+    assert_eq!(result.as_str().unwrap(), CONTENT_ABSENT);
 }
 
 #[cfg(unix)]

ethportal-peertest/src/scenarios/find.rs

@@ -4,7 +4,7 @@ use crate::{
     jsonrpc::{make_ipc_request, JsonRpcRequest, HISTORY_CONTENT_VALUE},
     Peertest, PeertestConfig,
 };
-use trin_types::jsonrpc::params::Params;
+use trin_types::{constants::CONTENT_ABSENT, jsonrpc::params::Params};
 
 pub fn test_trace_recursive_find_content(_peertest_config: PeertestConfig, peertest: &Peertest) {
     let uniq_content_key = "0x0015b11b918355b1ef9c5db810302ebad0bf2544255b530cdce90674d5887bb286";
@@ -36,6 +36,27 @@ pub fn test_trace_recursive_find_content(_peertest_config: PeertestConfig, peert
     );
 }
 
+// This test ensures that when content is not found the correct response is returned.
+pub fn test_trace_recursive_find_content_for_absent_content(
+    _peertest_config: PeertestConfig,
+    peertest: &Peertest,
+) {
+    // Different key to other test (final character).
+    let uniq_content_key = "0x0015b11b918355b1ef9c5db810302ebad0bf2544255b530cdce90674d5887bb287";
+    // Do not store content to offer in the testnode db
+
+    // Send trace recursive find content request
+    let request = JsonRpcRequest {
+        method: "portal_historyTraceRecursiveFindContent".to_string(),
+        id: 12,
+        params: Params::Array(vec![json!(uniq_content_key)]),
+    };
+
+    let result = make_ipc_request(&peertest.nodes[0].web3_ipc_path, &request).unwrap();
+    assert_eq!(result["content"], json!(CONTENT_ABSENT.to_string()));
+    // Check that at least one route was involved.
+    assert!(!result["route"].as_array().unwrap().is_empty());
+}
 // This test ensures that the jsonrpc channels don't close if there is an invalid recursive find
 // content request
 pub fn test_recursive_find_content_invalid_params(
@@ -50,9 +71,11 @@ pub fn test_recursive_find_content_invalid_params(
         params: Params::Array(vec![json!(invalid_content_key)]),
     };
 
-    // Expect '0x' response for invalid request
-    let response = make_ipc_request(&peertest.nodes[0].web3_ipc_path, &request).unwrap();
-    assert_eq!(response, json!("0x"));
+    // Expect invalid parameter response for invalid request
+    let response = make_ipc_request(&peertest.nodes[0].web3_ipc_path, &request);
+    assert!(response.is_err());
+    let error = response.err().unwrap().to_string();
+    assert_eq!(error, "JsonRpc response contains an error: Object {\"code\": Number(-32602), \"message\": String(\"unable to decode key SSZ bytes 0x00 due to InvalidByteLength { len: 0, expected: 32 }\")}");
 }
 
 pub fn test_trace_recursive_find_content_local_db(

ethportal-peertest/src/scenarios/offer_accept.rs

@@ -1,12 +1,13 @@
-use std::{str::FromStr, thread, time};
+use std::str::FromStr;
 
 use serde_json::json;
 use tracing::{error, info};
 
-use ethportal_api::jsonrpsee::async_client::Client;
-use ethportal_api::HistoryContentKey;
-use ethportal_api::{HistoryContentValue, HistoryNetworkApiClient};
-use trin_types::enr::Enr;
+use ethportal_api::{
+    jsonrpsee::async_client::Client, HistoryContentKey, HistoryContentValue,
+    HistoryNetworkApiClient,
+};
+use trin_types::{content_value::PossibleHistoryContentValue, enr::Enr};
 
 use crate::{
     jsonrpc::{validate_portal_offer, HISTORY_CONTENT_KEY, HISTORY_CONTENT_VALUE},
@@ -48,7 +49,11 @@ pub async fn test_unpopulated_offer(peertest_config: PeertestConfig, peertest: &
     validate_portal_offer(result, peertest);
 
     // Check if the stored content value in bootnode's DB matches the offered
-    let received_content_value = wait_for_content(ipc_client, content_key).await;
+    let response = wait_for_content(ipc_client, content_key).await;
+    let received_content_value = match response {
+        PossibleHistoryContentValue::ContentPresent(c) => c,
+        PossibleHistoryContentValue::ContentAbsent => panic!("Expected content to be found"),
+    };
     assert_eq!(
         content_value, received_content_value,
         "The received content {received_content_value:?}, must match the expected {content_value:?}",
@@ -84,7 +89,11 @@ pub async fn test_populated_offer(peertest_config: PeertestConfig, peertest: &Pe
     validate_portal_offer(result, peertest);
 
     // Check if the stored content value in bootnode's DB matches the offered
-    let received_content_value = wait_for_content(ipc_client, content_key).await;
+    let response = wait_for_content(ipc_client, content_key).await;
+    let received_content_value = match response {
+        PossibleHistoryContentValue::ContentPresent(c) => c,
+        PossibleHistoryContentValue::ContentAbsent => panic!("Expected content to be found"),
+    };
     assert_eq!(
         content_value, received_content_value,
         "The received content {received_content_value:?}, must match the expected {content_value:?}",
@@ -95,20 +104,25 @@ pub async fn test_populated_offer(peertest_config: PeertestConfig, peertest: &Pe
 async fn wait_for_content(
     ipc_client: Client,
     content_key: HistoryContentKey,
-) -> HistoryContentValue {
-    let mut received_content_value = ipc_client.local_content(content_key.clone()).await.unwrap();
+) -> PossibleHistoryContentValue {
+    let mut received_content_value = ipc_client.local_content(content_key.clone()).await;
 
     let mut counter = 0;
-    while received_content_value == HistoryContentValue::Unknown("".to_owned()) && counter < 5 {
-        error!("Retrying after 0.5sec, because content should have been present");
-        thread::sleep(time::Duration::from_millis(500));
-        received_content_value = ipc_client
-            .local_content(serde_json::from_value(json!(content_key)).unwrap())
-            .await
-            .unwrap();
 
+    // If content is absent an error will be returned.
+    while counter < 5 {
+        let message = match received_content_value {
+            x @ Ok(PossibleHistoryContentValue::ContentPresent(_)) => return x.unwrap(),
+            Ok(PossibleHistoryContentValue::ContentAbsent) => {
+                "absent content response received".to_string()
+            }
+            Err(e) => format!("received an error {e}"),
+        };
+        error!("Retrying after 0.5s, because {message}");
+        tokio::time::sleep(tokio::time::Duration::from_millis(500)).await;
+        received_content_value = ipc_client.local_content(content_key.clone()).await;
         counter += 1;
     }
 
-    received_content_value
+    received_content_value.unwrap()
 }

ethportal-peertest/src/scenarios/paginate.rs

@@ -1,7 +1,9 @@
 use ethportal_api::HistoryContentValue;
 use ethportal_api::HistoryNetworkApiClient;
 use ethportal_api::{BlockHeaderKey, HistoryContentKey};
+use serde_json::json;
 
+use crate::jsonrpc::HISTORY_CONTENT_VALUE;
 use crate::{Peertest, PeertestConfig};
 
 pub async fn test_paginate_local_storage(peertest_config: PeertestConfig, _peertest: &Peertest) {
@@ -25,10 +27,12 @@ pub async fn test_paginate_local_storage(peertest_config: PeertestConfig, _peert
 
     for content_key in content_keys.clone().into_iter() {
         // Store content to offer in the testnode db
+        let dummy_content_value: HistoryContentValue =
+            serde_json::from_value(json!(HISTORY_CONTENT_VALUE)).unwrap();
         let store_result = ipc_client
             .store(
                 serde_json::from_str(&content_key).unwrap(),
-                HistoryContentValue::Unknown("0x00".to_owned()),
+                dummy_content_value,
             )
             .await
             .unwrap();

newsfragments/538.changed.md

@@ -0,0 +1 @@
+Change HistoryContentValue to remove Unknown variant.

portalnet/src/overlay_service.rs

@@ -1471,9 +1471,11 @@ where
         let content_values = portal_wire::decode_content_payload(payload)?;
 
         // Accepted content keys len should match content value len
-        if content_keys.len() != content_values.len() {
+        let keys_len = content_keys.len();
+        let vals_len = content_values.len();
+        if keys_len != vals_len {
             return Err(anyhow!(
-                "Content keys len doesn't match content values len."
+                "Content keys len {keys_len} doesn't match content values len {vals_len}."
             ));
         }