From 36233a0bc194ab90ae52451208000531bdb7e48a Mon Sep 17 00:00:00 2001 From: Hansie Odendaal Date: Tue, 19 Sep 2023 12:51:26 +0200 Subject: [PATCH 1/2] Fix exception in RequestContext Fixed possible exception in 'pub struct RequestContext' that could occur when splitting the request and reply channel in 'pub fn split' --- base_layer/mmr/src/sparse_merkle_tree/proofs.rs | 6 +++--- base_layer/service_framework/src/lib.rs | 2 +- .../service_framework/src/reply_channel.rs | 16 +++++----------- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/base_layer/mmr/src/sparse_merkle_tree/proofs.rs b/base_layer/mmr/src/sparse_merkle_tree/proofs.rs index 43760ad980..cf7b3405fe 100644 --- a/base_layer/mmr/src/sparse_merkle_tree/proofs.rs +++ b/base_layer/mmr/src/sparse_merkle_tree/proofs.rs @@ -335,7 +335,7 @@ mod test { // feature?) assert!(proof.validate(&NodeKey::from([67u8; 32]), tree.hash())); // Use an exclusion proof to try and give an invalid validation for a key that does exist - assert_eq!(proof.validate(&NodeKey::from([64u8; 32]), tree.hash()), false); + assert!(!proof.validate(&NodeKey::from([64u8; 32]), tree.hash())); // A tree with branches tree.upsert(NodeKey::from([96u8; 32]), ValueHash::from([1u8; 32])) @@ -352,11 +352,11 @@ mod test { // Does not validate against invalid root hash assert!(!proof.validate(&NodeKey::from([99u8; 32]), &NodeHash::default())); // Not all non-existent keys will validate. (bug or feature?) - assert_eq!(proof.validate(&NodeKey::from([65; 32]), tree.hash()), false); + assert!(!proof.validate(&NodeKey::from([65; 32]), tree.hash())); // But any keys with prefix 011 (that is not 96) will validate assert!(proof.validate(&NodeKey::from([0b0110_0011; 32]), tree.hash())); assert!(proof.validate(&NodeKey::from([0b0111_0001; 32]), tree.hash())); // The key 96 does exist.. - assert_eq!(proof.validate(&NodeKey::from([0b0110_0000; 32]), tree.hash()), false); + assert!(!proof.validate(&NodeKey::from([0b0110_0000; 32]), tree.hash())); } } diff --git a/base_layer/service_framework/src/lib.rs b/base_layer/service_framework/src/lib.rs index 1b6143d3ed..f2642c15f8 100644 --- a/base_layer/service_framework/src/lib.rs +++ b/base_layer/service_framework/src/lib.rs @@ -52,7 +52,7 @@ //! // At the same time receive the request and reply //! async move { //! let req_context = receiver.next().await.unwrap(); -//! let msg = req_context.request().unwrap().clone(); +//! let msg = req_context.request().clone(); //! req_context.reply(msg.to_uppercase()); //! } //! ); diff --git a/base_layer/service_framework/src/reply_channel.rs b/base_layer/service_framework/src/reply_channel.rs index f69374405b..a56cadffe3 100644 --- a/base_layer/service_framework/src/reply_channel.rs +++ b/base_layer/service_framework/src/reply_channel.rs @@ -131,35 +131,29 @@ impl Future for TransportResponseFuture { /// request. pub struct RequestContext { reply_tx: oneshot::Sender, - request: Option, + request: TReq, } impl RequestContext { /// Create a new RequestContect pub fn new(request: TReq, reply_tx: oneshot::Sender) -> Self { Self { - request: Some(request), + request, reply_tx, } } /// Return a reference to the request object. None is returned after take_request has /// been called. - pub fn request(&self) -> Option<&TReq> { - self.request.as_ref() - } - - /// Take ownership of the request object, if ownership has not already been taken, - /// otherwise None is returned. - pub fn take_request(&mut self) -> Option { - self.request.take() + pub fn request(&self) -> &TReq { + &self.request } /// Consume this object and return it's parts. Namely, the request object and /// the reply oneshot channel. pub fn split(self) -> (TReq, oneshot::Sender) { ( - self.request.expect("RequestContext must be initialized with a request"), + self.request, self.reply_tx, ) } From 6475ef72e4c3565fc6df44f4798ad2bfcd03b958 Mon Sep 17 00:00:00 2001 From: Hansie Odendaal Date: Thu, 21 Sep 2023 12:00:22 +0200 Subject: [PATCH 2/2] cargo fmt --- base_layer/service_framework/src/reply_channel.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/base_layer/service_framework/src/reply_channel.rs b/base_layer/service_framework/src/reply_channel.rs index a56cadffe3..983fd0afe3 100644 --- a/base_layer/service_framework/src/reply_channel.rs +++ b/base_layer/service_framework/src/reply_channel.rs @@ -137,10 +137,7 @@ pub struct RequestContext { impl RequestContext { /// Create a new RequestContect pub fn new(request: TReq, reply_tx: oneshot::Sender) -> Self { - Self { - request, - reply_tx, - } + Self { request, reply_tx } } /// Return a reference to the request object. None is returned after take_request has @@ -152,10 +149,7 @@ impl RequestContext { /// Consume this object and return it's parts. Namely, the request object and /// the reply oneshot channel. pub fn split(self) -> (TReq, oneshot::Sender) { - ( - self.request, - self.reply_tx, - ) + (self.request, self.reply_tx) } /// Sends a reply to the caller