From 02c0ca4f1827d23034c9bfa0f43d806a6409042c Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 9 Dec 2024 14:23:02 +1000 Subject: [PATCH 1/5] Simplify code using typed parameters --- crates/subspace-gateway/src/commands/http.rs | 2 +- .../src/commands/http/server.rs | 46 ++++++------------- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/crates/subspace-gateway/src/commands/http.rs b/crates/subspace-gateway/src/commands/http.rs index 43ef3d2e74..fc617f0465 100644 --- a/crates/subspace-gateway/src/commands/http.rs +++ b/crates/subspace-gateway/src/commands/http.rs @@ -1,5 +1,5 @@ //! Gateway http command. -//! This command start an HTTP server to serve object requests. +//! This command starts an HTTP server to serve object requests. pub(crate) mod server; diff --git a/crates/subspace-gateway/src/commands/http/server.rs b/crates/subspace-gateway/src/commands/http/server.rs index e9d23c33cd..b54a564a79 100644 --- a/crates/subspace-gateway/src/commands/http/server.rs +++ b/crates/subspace-gateway/src/commands/http/server.rs @@ -36,21 +36,22 @@ where s.parse::().map_err(serde::de::Error::custom) } -async fn request_object_mappings(endpoint: String, key: String) -> anyhow::Result { +/// Requests an object mapping with `key` from the indexer service. +async fn request_object_mapping(endpoint: &str, key: Blake3Hash) -> anyhow::Result { let client = reqwest::Client::new(); - let object_mappings_url = format!("http://{}/objects/{}", endpoint, key,); + let object_mappings_url = format!("http://{}/objects/{}", endpoint, hex::encode(key)); debug!(?key, ?object_mappings_url, "Requesting object mapping..."); let response = client - .get(object_mappings_url.clone()) + .get(&object_mappings_url) .send() .await? .json::() .await; match &response { Ok(json) => { - trace!(?key, ?json, "Requested object mapping."); + trace!(?key, ?json, "Received object mapping"); } Err(err) => { error!(?key, ?err, ?object_mappings_url, "Request failed"); @@ -60,8 +61,9 @@ async fn request_object_mappings(endpoint: String, key: String) -> anyhow::Resul response.map_err(|err| err.into()) } +/// Fetches a DSN object with `key`, using the mapping indexer service. async fn serve_object( - key: web::Path, + key: web::Path, additional_data: web::Data>>, ) -> impl Responder where @@ -70,34 +72,16 @@ where let server_params = additional_data.into_inner(); let key = key.into_inner(); - // Validate object hash - let decode_result = hex::decode(key.clone()); - let object_hash = match decode_result { - Ok(hash) => { - if hash.len() != Blake3Hash::SIZE { - error!(?key, ?hash, "Invalid hash provided."); - return HttpResponse::BadRequest().finish(); - } - - Blake3Hash::try_from(hash.as_slice()).expect("Hash size was confirmed.") - } - Err(err) => { - error!(?key, ?err, "Invalid hash provided."); - return HttpResponse::BadRequest().finish(); - } - }; - - let Ok(object_mapping) = - request_object_mappings(server_params.indexer_endpoint.clone(), key.clone()).await + let Ok(object_mapping) = request_object_mapping(&server_params.indexer_endpoint, key).await else { return HttpResponse::BadRequest().finish(); }; - if object_mapping.hash != object_hash { + if object_mapping.hash != key { error!( + ?object_mapping, ?key, - object_mapping_hash=?object_mapping.hash, - "Requested hash doesn't match object mapping." + "Returned object mapping doesn't match requested hash" ); return HttpResponse::ServiceUnavailable().finish(); } @@ -112,11 +96,11 @@ where trace!(?key, size=%object.len(), "Object fetched successfully"); let data_hash = blake3_hash(&object); - if data_hash != object_hash { + if data_hash != key { error!( ?data_hash, - ?object_hash, - "Retrieved data did not match mapping hash" + ?key, + "Retrieved data doesn't match requested mapping hash" ); return HttpResponse::ServiceUnavailable().finish(); } @@ -124,7 +108,7 @@ where object } Err(err) => { - error!(?key, ?err, "Failed to fetch object."); + error!(?key, ?err, "Failed to fetch object"); return HttpResponse::ServiceUnavailable().finish(); } }; From 976279527c1ef31467db7d40e1acb24ec4624e79 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 9 Dec 2024 14:26:22 +1000 Subject: [PATCH 2/5] Use 'hash' name consistently for hash parameter --- .../src/commands/http/server.rs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/subspace-gateway/src/commands/http/server.rs b/crates/subspace-gateway/src/commands/http/server.rs index b54a564a79..5d39d64964 100644 --- a/crates/subspace-gateway/src/commands/http/server.rs +++ b/crates/subspace-gateway/src/commands/http/server.rs @@ -36,12 +36,12 @@ where s.parse::().map_err(serde::de::Error::custom) } -/// Requests an object mapping with `key` from the indexer service. -async fn request_object_mapping(endpoint: &str, key: Blake3Hash) -> anyhow::Result { +/// Requests an object mapping with `hash` from the indexer service. +async fn request_object_mapping(endpoint: &str, hash: Blake3Hash) -> anyhow::Result { let client = reqwest::Client::new(); - let object_mappings_url = format!("http://{}/objects/{}", endpoint, hex::encode(key)); + let object_mappings_url = format!("http://{}/objects/{}", endpoint, hex::encode(hash)); - debug!(?key, ?object_mappings_url, "Requesting object mapping..."); + debug!(?hash, ?object_mappings_url, "Requesting object mapping..."); let response = client .get(&object_mappings_url) @@ -51,36 +51,36 @@ async fn request_object_mapping(endpoint: &str, key: Blake3Hash) -> anyhow::Resu .await; match &response { Ok(json) => { - trace!(?key, ?json, "Received object mapping"); + trace!(?hash, ?json, "Received object mapping"); } Err(err) => { - error!(?key, ?err, ?object_mappings_url, "Request failed"); + error!(?hash, ?err, ?object_mappings_url, "Request failed"); } } response.map_err(|err| err.into()) } -/// Fetches a DSN object with `key`, using the mapping indexer service. +/// Fetches a DSN object with `hash`, using the mapping indexer service. async fn serve_object( - key: web::Path, + hash: web::Path, additional_data: web::Data>>, ) -> impl Responder where PG: PieceGetter + Send + Sync + 'static, { let server_params = additional_data.into_inner(); - let key = key.into_inner(); + let hash = hash.into_inner(); - let Ok(object_mapping) = request_object_mapping(&server_params.indexer_endpoint, key).await + let Ok(object_mapping) = request_object_mapping(&server_params.indexer_endpoint, hash).await else { return HttpResponse::BadRequest().finish(); }; - if object_mapping.hash != key { + if object_mapping.hash != hash { error!( ?object_mapping, - ?key, + ?hash, "Returned object mapping doesn't match requested hash" ); return HttpResponse::ServiceUnavailable().finish(); @@ -93,13 +93,13 @@ where let object = match object_fetcher_result { Ok(object) => { - trace!(?key, size=%object.len(), "Object fetched successfully"); + trace!(?hash, size=%object.len(), "Object fetched successfully"); let data_hash = blake3_hash(&object); - if data_hash != key { + if data_hash != hash { error!( ?data_hash, - ?key, + ?hash, "Retrieved data doesn't match requested mapping hash" ); return HttpResponse::ServiceUnavailable().finish(); @@ -108,7 +108,7 @@ where object } Err(err) => { - error!(?key, ?err, "Failed to fetch object"); + error!(?hash, ?err, "Failed to fetch object"); return HttpResponse::ServiceUnavailable().finish(); } }; From f2f12327bff058aad49a292e4afcba00c2a0c074 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 9 Dec 2024 14:26:36 +1000 Subject: [PATCH 3/5] Expand doc comments --- crates/subspace-gateway/src/commands/http.rs | 2 +- crates/subspace-gateway/src/commands/http/server.rs | 6 ++++++ crates/subspace-gateway/src/commands/rpc.rs | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/subspace-gateway/src/commands/http.rs b/crates/subspace-gateway/src/commands/http.rs index fc617f0465..f759d32770 100644 --- a/crates/subspace-gateway/src/commands/http.rs +++ b/crates/subspace-gateway/src/commands/http.rs @@ -22,7 +22,7 @@ pub(crate) struct HttpCommandOptions { http_listen_on: String, } -/// Runs an HTTP server +/// Runs an HTTP server which fetches DSN objects based on object hashes. pub async fn run(run_options: HttpCommandOptions) -> anyhow::Result<()> { let signal = shutdown_signal(); diff --git a/crates/subspace-gateway/src/commands/http/server.rs b/crates/subspace-gateway/src/commands/http/server.rs index 5d39d64964..177f03799d 100644 --- a/crates/subspace-gateway/src/commands/http/server.rs +++ b/crates/subspace-gateway/src/commands/http/server.rs @@ -1,3 +1,5 @@ +//! HTTP server which fetches objects from the DSN based on a hash, using a mapping indexer service. + use actix_web::{web, App, HttpResponse, HttpServer, Responder}; use serde::{Deserialize, Deserializer, Serialize}; use std::default::Default; @@ -9,6 +11,7 @@ use subspace_data_retrieval::object_fetcher::ObjectFetcher; use subspace_data_retrieval::piece_getter::PieceGetter; use tracing::{debug, error, trace}; +/// Parameters for the DSN object HTTP server. pub(crate) struct ServerParameters where PG: PieceGetter + Send + Sync + 'static, @@ -18,6 +21,7 @@ where pub(crate) http_endpoint: String, } +/// Object mapping format from the indexer service. #[derive(Serialize, Deserialize, Debug, Default)] #[serde(rename_all = "camelCase")] struct ObjectMapping { @@ -28,6 +32,7 @@ struct ObjectMapping { block_number: BlockNumber, } +/// Utility function to deserialize a JSON string into a u32. fn string_to_u32<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -118,6 +123,7 @@ where .body(object) } +/// Starts the DSN object HTTP server. pub async fn start_server(server_params: ServerParameters) -> std::io::Result<()> where PG: PieceGetter + Send + Sync + 'static, diff --git a/crates/subspace-gateway/src/commands/rpc.rs b/crates/subspace-gateway/src/commands/rpc.rs index 0edd2c1a3d..9c5de1cad5 100644 --- a/crates/subspace-gateway/src/commands/rpc.rs +++ b/crates/subspace-gateway/src/commands/rpc.rs @@ -1,5 +1,5 @@ //! Gateway rpc command. -//! This command start an RPC server to serve object requests. +//! This command starts an RPC server to serve object requests from the DSN. pub(crate) mod server; use crate::commands::rpc::server::{launch_rpc_server, RpcOptions, RPC_DEFAULT_PORT}; @@ -21,7 +21,7 @@ pub(crate) struct RpcCommandOptions { rpc_options: RpcOptions, } -/// Runs an RPC server +/// Runs an RPC server which fetches DSN objects based on mappings. pub async fn run(run_options: RpcCommandOptions) -> anyhow::Result<()> { let signal = shutdown_signal(); From 7af513cd3cf14807b027c430b029e2267af8e190 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 10 Dec 2024 08:11:58 +1000 Subject: [PATCH 4/5] Add retrieved object size to hash error message --- crates/subspace-gateway/src/commands/http/server.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/subspace-gateway/src/commands/http/server.rs b/crates/subspace-gateway/src/commands/http/server.rs index 177f03799d..1cb0b31651 100644 --- a/crates/subspace-gateway/src/commands/http/server.rs +++ b/crates/subspace-gateway/src/commands/http/server.rs @@ -98,12 +98,13 @@ where let object = match object_fetcher_result { Ok(object) => { - trace!(?hash, size=%object.len(), "Object fetched successfully"); + trace!(?hash, size = %object.len(), "Object fetched successfully"); let data_hash = blake3_hash(&object); if data_hash != hash { error!( ?data_hash, + data_size = %object.len(), ?hash, "Retrieved data doesn't match requested mapping hash" ); From 21f997d1ddfe4d768931a2ee37c338b4a5511d4c Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 10 Dec 2024 08:15:10 +1000 Subject: [PATCH 5/5] Add trace-level dump of incorrect object data --- crates/subspace-gateway/src/commands/http/server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/subspace-gateway/src/commands/http/server.rs b/crates/subspace-gateway/src/commands/http/server.rs index 1cb0b31651..6ebbd76f06 100644 --- a/crates/subspace-gateway/src/commands/http/server.rs +++ b/crates/subspace-gateway/src/commands/http/server.rs @@ -108,6 +108,7 @@ where ?hash, "Retrieved data doesn't match requested mapping hash" ); + trace!(data = %hex::encode(object), "Retrieved data"); return HttpResponse::ServiceUnavailable().finish(); }