Skip to content

Commit 848856e

Browse files
Merge bitcoindevkit/rust-esplora-client#90: Don't return 1.0 feerate if none is found by convert_fee_rate
ec5ee8233011269b2d0bbdf8a1647e27e61b9050 test: improve test `feerate_parsing` (valued mammal) ed219e29455a9d38487f50d3862888b73cdb211c 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 ec5ee8233011269b2d0bbdf8a1647e27e61b9050 Tree-SHA512: 32fd2a8a57bcc1a6fb8569d779aca8a273c843d38afe6f092f0c70c7dad0ff7b37595284985ca4d3c5e253810b70857600043817fd9f928ee0c706108f8a9bcb
2 parents 4f9f3f7 + a5b07e9 commit 848856e

File tree

1 file changed

+14
-12
lines changed

1 file changed

+14
-12
lines changed

src/lib.rs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,14 @@ pub use r#async::AsyncClient;
8888

8989
/// Get a fee value in sats/vbytes from the estimates
9090
/// that matches the confirmation target set as parameter.
91-
pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Result<f32, Error> {
92-
let fee_val = {
93-
let mut pairs = estimates.into_iter().collect::<Vec<(u16, f64)>>();
94-
pairs.sort_unstable_by_key(|(k, _)| std::cmp::Reverse(*k));
95-
pairs
96-
.into_iter()
97-
.find(|(k, _)| *k as usize <= target)
98-
.map(|(_, v)| v)
99-
.unwrap_or(1.0)
100-
};
101-
Ok(fee_val as f32)
91+
///
92+
/// Returns `None` if no feerate estimate is found at or below `target` confirmations.
93+
pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Option<f32> {
94+
estimates
95+
.into_iter()
96+
.filter(|(k, _)| *k as usize <= target)
97+
.max_by_key(|(k, _)| *k)
98+
.map(|(_, v)| v as f32)
10299
}
103100

104101
#[derive(Debug, Clone)]
@@ -391,12 +388,17 @@ mod test {
391388
"#,
392389
)
393390
.unwrap();
391+
assert!(convert_fee_rate(1, HashMap::new()).is_none());
394392
assert_eq!(convert_fee_rate(6, esplora_fees.clone()).unwrap(), 2.236);
395393
assert_eq!(
396-
convert_fee_rate(26, esplora_fees).unwrap(),
394+
convert_fee_rate(26, esplora_fees.clone()).unwrap(),
397395
1.015,
398396
"should inherit from value for 25"
399397
);
398+
assert!(
399+
convert_fee_rate(0, esplora_fees).is_none(),
400+
"should not return feerate for 0 target"
401+
);
400402
}
401403

402404
#[cfg(all(feature = "blocking", feature = "async"))]

0 commit comments

Comments
 (0)