Do data validation after should_store() not before

Author: njgheorghita

ethportal-peertest/src/jsonrpc.rs

@@ -363,10 +363,7 @@ pub fn make_ipc_request(ipc_path: &str, request: &JsonRpcRequest) -> anyhow::Res
     stream.flush().unwrap();
     let deser = serde_json::Deserializer::from_reader(stream);
     let next_obj = deser.into_iter::<Value>().next();
-    let response_obj = match next_obj {
-        Some(val) => val,
-        None => return Err(anyhow!("Empty JsonRpc response")),
-    };
+    let response_obj = next_obj.ok_or_else(|| anyhow!("Empty JsonRpc response"))?;
     get_response_result(response_obj)
 }
 

src/lib.rs

@@ -88,7 +88,6 @@ pub async fn run_trin(
         history_event_tx,
         history_utp_tx,
         history_jsonrpc_tx,
-        _header_oracle,
     ) = if trin_config
         .networks
         .iter()

trin-core/src/portalnet/overlay_service.rs

@@ -1007,21 +1007,24 @@ where
         let content_key = match TContentKey::try_from(request.content_key) {
             Ok(val) => val,
             Err(msg) => {
-                error!("Unable to process received content: Invalid content key: {msg:?}");
+                error!("Unable to process received content: We sent a request with an invalid content key: {msg:?}");
                 return;
             }
         };
-        if let Err(err) = self
-            .validator
-            .validate_content(&content_key, &content)
-            .await
-        {
-            error!("Unable to validate received content: {err:?}");
-            return;
-        };
-        match self.storage.read().should_store(&content_key) {
-            Ok(should_store) => {
-                if should_store {
+        let should_store = self.storage.read().should_store(&content_key);
+        match should_store {
+            Ok(val) => {
+                if val {
+                    // validate content before storing
+                    if let Err(err) = self
+                        .validator
+                        .validate_content(&content_key, &content)
+                        .await
+                    {
+                        error!("Unable to validate received content: {err:?}");
+                        return;
+                    };
+
                     if let Err(err) = self.storage.write().store(&content_key, &content.into()) {
                         error!("Content received, but not stored: {err}")
                     }

trin-core/src/types/validation.rs

@@ -40,7 +40,8 @@ impl Default for HeaderOracle {
 
 impl HeaderOracle {
     // Currently falls back to infura, to be updated to use canonical block indices network.
-    pub fn get_hash_at_height(&mut self, hex_number: String) -> anyhow::Result<String> {
+    pub fn get_hash_at_height(&mut self, block_number: u64) -> anyhow::Result<String> {
+        let hex_number = format!("0x:{:02X}", block_number);
         let request = JsonRequest {
             jsonrpc: "2.0".to_string(),
             params: Params::Array(vec![json!(hex_number), json!(false)]),

trin-history/src/validation.rs

@@ -2,7 +2,6 @@ use std::sync::{Arc, RwLock};
 
 use anyhow::anyhow;
 use async_trait::async_trait;
-use rlp::Rlp;
 
 use trin_core::{
     portalnet::types::{content_key::HistoryContentKey, messages::ByteList},
@@ -28,14 +27,12 @@ impl Validator<HistoryContentKey> for ChainHistoryValidator {
     {
         match content_key {
             HistoryContentKey::BlockHeader(key) => {
-                let rlp = Rlp::new(content);
-                let header = Header::decode_rlp(&rlp)?;
-                let number = format!("0x{:02X}", header.number);
+                let header: Header = rlp::decode(content)?;
                 let expected_hash = &self
                     .header_oracle
                     .write()
                     .unwrap()
-                    .get_hash_at_height(number)?;
+                    .get_hash_at_height(header.number)?;
                 let actual_hash = &hex::encode(key.block_hash);
                 if actual_hash == expected_hash {
                     Ok(())
@@ -73,7 +70,6 @@ mod tests {
     use ethereum_types::U256;
     use hex;
     use httpmock::prelude::*;
-    use rlp::{self, Rlp};
     use serde_json::json;
 
     use trin_core::portalnet::types::content_key::BlockHeader;
@@ -125,9 +121,8 @@ mod tests {
         let server = setup_mock_infura_server();
         let header_rlp = get_header_rlp();
         let header_bytelist = ByteList::try_from(header_rlp.clone()).unwrap();
-        let rlp = Rlp::new(&header_rlp);
 
-        let header: Header = Header::decode_rlp(&rlp).expect("error decoding header");
+        let header: Header = rlp::decode(&header_rlp).expect("error decoding header");
         let infura_url = server.url("/get_header");
         let header_oracle = Arc::new(RwLock::new(HeaderOracle {
             infura_url,
@@ -151,8 +146,7 @@ mod tests {
         // RLP encoded block header #669051
         let header_rlp = get_header_rlp();
         let header_bytelist = ByteList::try_from(header_rlp.clone()).unwrap();
-        let rlp = Rlp::new(&header_rlp);
-        let mut header: Header = Header::decode_rlp(&rlp).expect("error decoding header");
+        let mut header: Header = rlp::decode(&header_rlp).expect("error decoding header");
 
         // set invalid block height
         header.number = 669052;
@@ -180,8 +174,7 @@ mod tests {
         // RLP encoded block header #669051
         let header_rlp = get_header_rlp();
         let header_bytelist = ByteList::try_from(header_rlp.clone()).unwrap();
-        let rlp = Rlp::new(&header_rlp);
-        let mut header: Header = Header::decode_rlp(&rlp).expect("error decoding header");
+        let mut header: Header = rlp::decode(&header_rlp).expect("error decoding header");
 
         // set invalid block gaslimit
         // valid gaslimit = 3141592