From 4ae0b3bf846e42ee4d540d44824d36ad2c47f75a Mon Sep 17 00:00:00 2001
From: Michael Uti <utimichael9@gmail.com>
Date: Sat, 16 Apr 2022 00:38:40 +0100
Subject: [PATCH 1/2] [p2p] cbfmgr: improve error on cbfmgr

Return error early if filter header isn't downloaded for requested filter
---
 p2p/src/protocol/cbfmgr.rs | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/p2p/src/protocol/cbfmgr.rs b/p2p/src/protocol/cbfmgr.rs
index 15dd6590..177da6f9 100644
--- a/p2p/src/protocol/cbfmgr.rs
+++ b/p2p/src/protocol/cbfmgr.rs
@@ -248,8 +248,8 @@ pub trait Events {
 #[derive(Error, Debug)]
 pub enum GetFiltersError {
     /// The specified range is invalid, eg. it is out of bounds.
-    #[error("the specified range is invalid")]
-    InvalidRange,
+    #[error("the specified range is invalid, current tip is {tip}")]
+    InvalidRange { tip: Height },
     /// Not connected to any compact filter peer.
     #[error("not connected to any peer with compact filters support")]
     NotConnected,
@@ -494,13 +494,18 @@ impl<F: Filters, U: SyncFilters + Events + Wakeup + Disconnect, C: Clock> Filter
         range: RangeInclusive<Height>,
         tree: &T,
     ) -> Result<(), GetFiltersError> {
+        let tip = self.filters.height();
+
         if self.peers.is_empty() {
             return Err(GetFiltersError::NotConnected);
         }
         if range.is_empty() {
-            return Err(GetFiltersError::InvalidRange);
+            return Err(GetFiltersError::InvalidRange { tip });
+        }
+
+        if *range.end() > tip {
+            return Err(GetFiltersError::InvalidRange { tip });
         }
-        assert!(*range.end() <= self.filters.height());
 
         // TODO: Only ask peers synced to a certain height.
         // Choose a different peer for each requested range.
@@ -512,7 +517,7 @@ impl<F: Filters, U: SyncFilters + Events + Wakeup + Disconnect, C: Clock> Filter
         {
             let stop_hash = tree
                 .get_block_by_height(*range.end())
-                .ok_or(GetFiltersError::InvalidRange)?
+                .ok_or(GetFiltersError::InvalidRange { tip })?
                 .block_hash();
             let timeout = self.config.request_timeout;
 

From 2ae547ce6b1ae83cd3f8f64937e7b3a1559445d3 Mon Sep 17 00:00:00 2001
From: Michael Uti <utimichael9@gmail.com>
Date: Sun, 17 Apr 2022 20:18:08 +0100
Subject: [PATCH 2/2] [p2p] cbfmgr: test get_filter with no header err

---
 p2p/src/protocol/cbfmgr.rs |  5 +++-
 p2p/src/protocol/tests.rs  | 50 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/p2p/src/protocol/cbfmgr.rs b/p2p/src/protocol/cbfmgr.rs
index 177da6f9..d17c0ce3 100644
--- a/p2p/src/protocol/cbfmgr.rs
+++ b/p2p/src/protocol/cbfmgr.rs
@@ -249,7 +249,10 @@ pub trait Events {
 pub enum GetFiltersError {
     /// The specified range is invalid, eg. it is out of bounds.
     #[error("the specified range is invalid, current tip is {tip}")]
-    InvalidRange { tip: Height },
+    InvalidRange {
+        /// Filter header tip.
+        tip: Height,
+    },
     /// Not connected to any compact filter peer.
     #[error("not connected to any peer with compact filters support")]
     NotConnected,
diff --git a/p2p/src/protocol/tests.rs b/p2p/src/protocol/tests.rs
index 33fcfc71..2e601298 100644
--- a/p2p/src/protocol/tests.rs
+++ b/p2p/src/protocol/tests.rs
@@ -10,6 +10,7 @@ use std::net;
 use std::ops::{Bound, Range};
 use std::sync::Arc;
 
+use cbfmgr::GetFiltersError;
 use log::*;
 use nakamoto_common::bitcoin::network::message_blockdata::GetHeadersMessage;
 
@@ -190,6 +191,55 @@ fn test_inv_getheaders() {
         .expect("a timer should be returned");
 }
 
+#[test]
+fn test_get_filter_with_no_header() {
+    let mut rng = fastrand::Rng::new();
+    let height = 16;
+    let network = Network::Regtest;
+    let remote: PeerId = ([88, 88, 88, 88], 8333).into();
+
+    logger::init(log::Level::Debug);
+
+    let genesis = network.genesis_block();
+    let chain = gen::blockchain(genesis, height, &mut rng);
+    let headers = NonEmpty::from_vec(chain.iter().map(|b| b.header).collect()).unwrap();
+    let cfheader_genesis = FilterHeader::genesis(network);
+    let cfheaders = gen::cfheaders_from_blocks(cfheader_genesis, chain.iter())
+        .into_iter()
+        .skip(1) // Skip genesis
+        .collect::<Vec<_>>();
+
+    let mut peer = Peer::new(
+        "alice",
+        [48, 48, 48, 48],
+        network,
+        headers.tail,
+        cfheaders,
+        vec![],
+        rng,
+    );
+
+    // Connect to a peer with cfilter support.
+    peer.connect(
+        &PeerDummy {
+            addr: remote,
+            height,
+            protocol_version: PROTOCOL_VERSION,
+            services: cbfmgr::REQUIRED_SERVICES | syncmgr::REQUIRED_SERVICES,
+            relay: false,
+            time: peer.local_time(),
+        },
+        Link::Outbound,
+    );
+
+    let (transmit, receiver) = chan::unbounded();
+    peer.command(Command::GetFilters(17..=100, transmit));
+
+    let err = receiver.recv().unwrap().err().unwrap();
+
+    assert!(matches!(err, GetFiltersError::InvalidRange { tip: 16 }));
+}
+
 #[test]
 fn test_bad_magic() {
     let rng = fastrand::Rng::new();