From c2ee6ba31d32e59497d0f03805444479d6e01d46 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 18 Nov 2022 09:35:19 +0100 Subject: [PATCH 1/4] Fix the light client protocol protobuf schema --- .../src/light_client_requests/handler.rs | 57 ++++++++++--------- client/network/light/src/schema.rs | 32 +++++++++++ .../network/light/src/schema/light.v1.proto | 28 ++++----- 3 files changed, 76 insertions(+), 41 deletions(-) diff --git a/client/network/light/src/light_client_requests/handler.rs b/client/network/light/src/light_client_requests/handler.rs index 77904c7256295..a95f4f40aee7c 100644 --- a/client/network/light/src/light_client_requests/handler.rs +++ b/client/network/light/src/light_client_requests/handler.rs @@ -147,14 +147,18 @@ where let request = schema::v1::light::Request::decode(&payload[..])?; let response = match &request.request { - Some(schema::v1::light::request::Request::RemoteCallRequest(r)) => - self.on_remote_call_request(&peer, r)?, - Some(schema::v1::light::request::Request::RemoteReadRequest(r)) => - self.on_remote_read_request(&peer, r)?, - Some(schema::v1::light::request::Request::RemoteReadChildRequest(r)) => - self.on_remote_read_child_request(&peer, r)?, - None => - return Err(HandleRequestError::BadRequest("Remote request without request data.")), + Some(schema::v1::light::request::Request::RemoteCallRequest(r)) => { + self.on_remote_call_request(&peer, r)? + }, + Some(schema::v1::light::request::Request::RemoteReadRequest(r)) => { + self.on_remote_read_request(&peer, r)? + }, + Some(schema::v1::light::request::Request::RemoteReadChildRequest(r)) => { + self.on_remote_read_child_request(&peer, r)? + }, + None => { + return Err(HandleRequestError::BadRequest("Remote request without request data.")) + }, }; let mut data = Vec::new(); @@ -173,10 +177,7 @@ where let block = Decode::decode(&mut request.block.as_ref())?; let response = match self.client.execution_proof(block, &request.method, &request.data) { - Ok((_, proof)) => { - let r = schema::v1::light::RemoteCallResponse { proof: proof.encode() }; - Some(schema::v1::light::response::Response::RemoteCallResponse(r)) - }, + Ok((_, proof)) => schema::v1::light::RemoteCallResponse { proof: Some(proof.encode()) }, Err(e) => { trace!( "remote call request from {} ({} at {:?}) failed with: {}", @@ -185,11 +186,13 @@ where request.block, e, ); - None + schema::v1::light::RemoteCallResponse { proof: None } }, }; - Ok(schema::v1::light::Response { response }) + Ok(schema::v1::light::Response { + response: Some(schema::v1::light::response::Response::RemoteCallResponse(response)), + }) } fn on_remote_read_request( @@ -199,7 +202,7 @@ where ) -> Result { if request.keys.is_empty() { debug!("Invalid remote read request sent by {}.", peer); - return Err(HandleRequestError::BadRequest("Remote read request without keys.")) + return Err(HandleRequestError::BadRequest("Remote read request without keys.")); } trace!( @@ -213,10 +216,7 @@ where let response = match self.client.read_proof(block, &mut request.keys.iter().map(AsRef::as_ref)) { - Ok(proof) => { - let r = schema::v1::light::RemoteReadResponse { proof: proof.encode() }; - Some(schema::v1::light::response::Response::RemoteReadResponse(r)) - }, + Ok(proof) => schema::v1::light::RemoteReadResponse { proof: Some(proof.encode()) }, Err(error) => { trace!( "remote read request from {} ({} at {:?}) failed with: {}", @@ -225,11 +225,13 @@ where request.block, error, ); - None + schema::v1::light::RemoteReadResponse { proof: None } }, }; - Ok(schema::v1::light::Response { response }) + Ok(schema::v1::light::Response { + response: Some(schema::v1::light::response::Response::RemoteReadResponse(response)), + }) } fn on_remote_read_child_request( @@ -239,7 +241,7 @@ where ) -> Result { if request.keys.is_empty() { debug!("Invalid remote child read request sent by {}.", peer); - return Err(HandleRequestError::BadRequest("Remove read child request without keys.")) + return Err(HandleRequestError::BadRequest("Remove read child request without keys.")); } trace!( @@ -264,10 +266,7 @@ where &mut request.keys.iter().map(AsRef::as_ref), ) }) { - Ok(proof) => { - let r = schema::v1::light::RemoteReadResponse { proof: proof.encode() }; - Some(schema::v1::light::response::Response::RemoteReadResponse(r)) - }, + Ok(proof) => schema::v1::light::RemoteReadResponse { proof: Some(proof.encode()) }, Err(error) => { trace!( "remote read child request from {} ({} {} at {:?}) failed with: {}", @@ -277,11 +276,13 @@ where request.block, error, ); - None + schema::v1::light::RemoteReadResponse { proof: None } }, }; - Ok(schema::v1::light::Response { response }) + Ok(schema::v1::light::Response { + response: Some(schema::v1::light::response::Response::RemoteReadResponse(response)), + }) } } diff --git a/client/network/light/src/schema.rs b/client/network/light/src/schema.rs index 09cc82cc2811a..8bff7e5311f71 100644 --- a/client/network/light/src/schema.rs +++ b/client/network/light/src/schema.rs @@ -23,3 +23,35 @@ pub(crate) mod v1 { include!(concat!(env!("OUT_DIR"), "/api.v1.light.rs")); } } + +#[cfg(test)] +mod tests { + use prost::Message as _; + + #[test] + fn with_proof_encodes_correctly() { + let encoded = super::v1::light::Response { + response: Some(super::v1::light::response::Response::RemoteReadResponse( + super::v1::light::RemoteReadResponse { proof: Some(Vec::new()) }, + )), + } + .encode_to_vec(); + + // Make sure that the response contains one field of number 2 and wire type 2 (message), + // then another field of number 2 and wire type 2 (bytes), then a length of 0. + assert_eq!(encoded, vec![(2 << 3) | 2, 2, (2 << 3) | 2, 0]); + } + + #[test] + fn no_proof_encodes_correctly() { + let encoded = super::v1::light::Response { + response: Some(super::v1::light::response::Response::RemoteReadResponse( + super::v1::light::RemoteReadResponse { proof: None }, + )), + } + .encode_to_vec(); + + // Make sure that the response contains one field of number 2 and wire type 2 (message). + assert_eq!(encoded, vec![(2 << 3) | 2, 0]); + } +} diff --git a/client/network/light/src/schema/light.v1.proto b/client/network/light/src/schema/light.v1.proto index 1df5466e21988..1aa84a039452b 100644 --- a/client/network/light/src/schema/light.v1.proto +++ b/client/network/light/src/schema/light.v1.proto @@ -1,15 +1,15 @@ // Schema definition for light client messages. -syntax = "proto3"; +syntax = "proto2"; package api.v1.light; // A pair of arbitrary bytes. message Pair { // The first element of the pair. - bytes fst = 1; + required bytes fst = 1; // The second element of the pair. - bytes snd = 2; + required bytes snd = 2; } // Enumerate all possible light client request messages. @@ -34,40 +34,42 @@ message Response { // Remote call request. message RemoteCallRequest { // Block at which to perform call. - bytes block = 2; + required bytes block = 2; // Method name. - string method = 3; + required string method = 3; // Call data. - bytes data = 4; + required bytes data = 4; } // Remote call response. message RemoteCallResponse { - // Execution proof. - bytes proof = 2; + // Execution proof. If missing, indicates that the remote couldn't answer, for example because + // the block is pruned. + optional bytes proof = 2; } // Remote storage read request. message RemoteReadRequest { // Block at which to perform call. - bytes block = 2; + required bytes block = 2; // Storage keys. repeated bytes keys = 3; } // Remote read response. message RemoteReadResponse { - // Read proof. - bytes proof = 2; + // Read proof. If missing, indicates that the remote couldn't answer, for example because + // the block is pruned. + optional bytes proof = 2; } // Remote storage read child request. message RemoteReadChildRequest { // Block at which to perform call. - bytes block = 2; + required bytes block = 2; // Child Storage key, this is relative // to the child type storage location. - bytes storage_key = 3; + required bytes storage_key = 3; // Storage keys. repeated bytes keys = 6; } From 96e5e2f25fa15fed1bb292633d393a64b0f67f0e Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 18 Nov 2022 09:44:52 +0100 Subject: [PATCH 2/4] Add another test --- client/network/light/src/schema.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/client/network/light/src/schema.rs b/client/network/light/src/schema.rs index 8bff7e5311f71..1fc91659a5bbb 100644 --- a/client/network/light/src/schema.rs +++ b/client/network/light/src/schema.rs @@ -29,7 +29,7 @@ mod tests { use prost::Message as _; #[test] - fn with_proof_encodes_correctly() { + fn empty_proof_encodes_correctly() { let encoded = super::v1::light::Response { response: Some(super::v1::light::response::Response::RemoteReadResponse( super::v1::light::RemoteReadResponse { proof: Some(Vec::new()) }, @@ -54,4 +54,16 @@ mod tests { // Make sure that the response contains one field of number 2 and wire type 2 (message). assert_eq!(encoded, vec![(2 << 3) | 2, 0]); } + + #[test] + fn proof_encodes_correctly() { + let encoded = super::v1::light::Response { + response: Some(super::v1::light::response::Response::RemoteReadResponse( + super::v1::light::RemoteReadResponse { proof: Some(vec![1, 2, 3, 4]) }, + )), + } + .encode_to_vec(); + + assert_eq!(encoded, vec![(2 << 3) | 2, 6, (2 << 3) | 2, 4, 1, 2, 3, 4]); + } } From 858a91109358fef48fc9b67be21ce32fb36717d9 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 18 Nov 2022 09:45:03 +0100 Subject: [PATCH 3/4] Remove unused protobuf struct --- client/network/light/src/schema/light.v1.proto | 8 -------- 1 file changed, 8 deletions(-) diff --git a/client/network/light/src/schema/light.v1.proto b/client/network/light/src/schema/light.v1.proto index 1aa84a039452b..a269ea73c2074 100644 --- a/client/network/light/src/schema/light.v1.proto +++ b/client/network/light/src/schema/light.v1.proto @@ -4,14 +4,6 @@ syntax = "proto2"; package api.v1.light; -// A pair of arbitrary bytes. -message Pair { - // The first element of the pair. - required bytes fst = 1; - // The second element of the pair. - required bytes snd = 2; -} - // Enumerate all possible light client request messages. message Request { oneof request { From ccc1393d2375bdd1b440ea174b8d123c5e6e53e6 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 18 Nov 2022 11:19:40 +0100 Subject: [PATCH 4/4] Ok you have to use the nightly rustfmt apparently --- .../src/light_client_requests/handler.rs | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/client/network/light/src/light_client_requests/handler.rs b/client/network/light/src/light_client_requests/handler.rs index a95f4f40aee7c..abf012b82f9db 100644 --- a/client/network/light/src/light_client_requests/handler.rs +++ b/client/network/light/src/light_client_requests/handler.rs @@ -147,18 +147,14 @@ where let request = schema::v1::light::Request::decode(&payload[..])?; let response = match &request.request { - Some(schema::v1::light::request::Request::RemoteCallRequest(r)) => { - self.on_remote_call_request(&peer, r)? - }, - Some(schema::v1::light::request::Request::RemoteReadRequest(r)) => { - self.on_remote_read_request(&peer, r)? - }, - Some(schema::v1::light::request::Request::RemoteReadChildRequest(r)) => { - self.on_remote_read_child_request(&peer, r)? - }, - None => { - return Err(HandleRequestError::BadRequest("Remote request without request data.")) - }, + Some(schema::v1::light::request::Request::RemoteCallRequest(r)) => + self.on_remote_call_request(&peer, r)?, + Some(schema::v1::light::request::Request::RemoteReadRequest(r)) => + self.on_remote_read_request(&peer, r)?, + Some(schema::v1::light::request::Request::RemoteReadChildRequest(r)) => + self.on_remote_read_child_request(&peer, r)?, + None => + return Err(HandleRequestError::BadRequest("Remote request without request data.")), }; let mut data = Vec::new(); @@ -202,7 +198,7 @@ where ) -> Result { if request.keys.is_empty() { debug!("Invalid remote read request sent by {}.", peer); - return Err(HandleRequestError::BadRequest("Remote read request without keys.")); + return Err(HandleRequestError::BadRequest("Remote read request without keys.")) } trace!( @@ -241,7 +237,7 @@ where ) -> Result { if request.keys.is_empty() { debug!("Invalid remote child read request sent by {}.", peer); - return Err(HandleRequestError::BadRequest("Remove read child request without keys.")); + return Err(HandleRequestError::BadRequest("Remove read child request without keys.")) } trace!(