Merge #90: Don't return 1.0 feerate if none is found by `convert_fee_…

…rate`

ec5ee82 test: improve test `feerate_parsing` (valued mammal)
ed219e2 fix: don't return default 1.0 feerate if not found by `convert_fee_rate` (valued mammal)

Pull request description:

  Change `convert_fee_rate` to return `Option<f32>` instead of Result.

  PR #65 made this function effectively infallible by removing the parse int error while falling back to a bogus 1.0 feerate if a value isn't found in fee estimates. We could make it return an error if for example the given fee estimates map is empty without changing the function signature but in that case we would need a new `Error` variant making it a breaking change anyway, therefore just change the function to return `Option` which should only be `None` if given a target of 0 or an empty map assuming esplora always has a fee estimate for a target of 1 confirmation.

  fixes #80

ACKs for top commit:
  notmandatory:
    ACK ec5ee82

Tree-SHA512: 32fd2a8a57bcc1a6fb8569d779aca8a273c843d38afe6f092f0c70c7dad0ff7b37595284985ca4d3c5e253810b70857600043817fd9f928ee0c706108f8a9bcb

src/lib.rs

@@ -88,17 +88,14 @@ pub use r#async::AsyncClient;
 
 /// Get a fee value in sats/vbytes from the estimates
 /// that matches the confirmation target set as parameter.
-pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Result<f32, Error> {
-    let fee_val = {
-        let mut pairs = estimates.into_iter().collect::<Vec<(u16, f64)>>();
-        pairs.sort_unstable_by_key(|(k, _)| std::cmp::Reverse(*k));
-        pairs
-            .into_iter()
-            .find(|(k, _)| *k as usize <= target)
-            .map(|(_, v)| v)
-            .unwrap_or(1.0)
-    };
-    Ok(fee_val as f32)
+///
+/// Returns `None` if no feerate estimate is found at or below `target` confirmations.
+pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Option<f32> {
+    estimates
+        .into_iter()
+        .filter(|(k, _)| *k as usize <= target)
+        .max_by_key(|(k, _)| *k)
+        .map(|(_, v)| v as f32)
 }
 
 #[derive(Debug, Clone)]
@@ -391,12 +388,17 @@ mod test {
 "#,
         )
         .unwrap();
+        assert!(convert_fee_rate(1, HashMap::new()).is_none());
         assert_eq!(convert_fee_rate(6, esplora_fees.clone()).unwrap(), 2.236);
         assert_eq!(
-            convert_fee_rate(26, esplora_fees).unwrap(),
+            convert_fee_rate(26, esplora_fees.clone()).unwrap(),
             1.015,
             "should inherit from value for 25"
         );
+        assert!(
+            convert_fee_rate(0, esplora_fees).is_none(),
+            "should not return feerate for 0 target"
+        );
     }
 
     #[cfg(all(feature = "blocking", feature = "async"))]