Add recommended_fees to BDK electrum client

[matthiasdebernardini]

Describe the enhancement
Add a new function to BdkElectrumClient that returns a vector of FeeRates, so you can easily get the value you need to use with a transaction.

Use case
Using the estimate_fee route from the electrum API is unituitive because it uses units. There should be a method that returns a vector or FeeRates that you can then use to make a transaction with.

Additional context
Something like the mempool recommended fees could be good example but if you try to do it this way, a user would pull in the reqwest library and end up having to depend on OpenSSL. While they should know better than to avoid this, it would be great if they could get an easy way to know which feerate to use.

I'm trying this for now, but something simpler would be better I think.

pub fn estimate_electrum_fee(network: Network) -> anyhow::Result<MempoolSpaceFeeEstimation> {
    let electrum_url = match network {
        // todo point to mempool
        Bitcoin =>
        // "ssl://mempool.space:50002",
            "ssl://electrum.blockstream.info:50002",
        _ => "ssl://mempool.space:60602",
    };

    let client = BdkElectrumClient::new(electrum_client::Client::new(electrum_url).unwrap());
    let frs: Vec<_> = (1..=6).map(|n| {
        match client.inner.estimate_fee(n) {
            Ok(fee) => {
                // Convert BTC/kB to satoshis/kb, truncating
                let sats_per_kb = Amount::from_btc((fee* 100_000_000.0).round() / 100_000_000.0).unwrap().to_sat();

                // Convert kB to weight units (1 kB = 4000 weight units)
                let weight_kb = Weight::from_vb(250).unwrap();

                // Calculate satoshis per 1000 weight units
                let sats_per_kwu = sats_per_kb / weight_kb.to_wu();

                FeeRate::from_sat_per_kwu(sats_per_kwu)
            }
            Err(e) => {FeeRate::ZERO}
        }

    }).collect();

    let fastest_fee = frs.first().cloned().unwrap();
    let half_hour_fee = frs[3];
    let hour_fee = frs.last().cloned().unwrap();
    let msfe = MempoolSpaceFeeEstimation{
        fastest_fee,
        half_hour_fee,
        hour_fee,
    };
    Ok(msfe)
}

[matthiasdebernardini]

Thinking through this, perhaps a better idea would be to add more methods to FeeRate so it can accept the values from electrum and provide a fee rate to users that they can use.

[storopoli]

We probably have the machinery already at rust-electrum see https://github.com/bitcoindevkit/rust-electrum-client/blob/64c77ee1bc90c5e19ec0d401b97e420d495d7c6d/src/batch.rs#L64-L69

    /// Add one `blockchain.estimatefee` request to the batch queue
    pub fn estimate_fee(&mut self, number: usize) {
        let params = vec![Param::Usize(number)];
        self.calls
            .push((String::from("blockchain.estimatefee"), params));
    }

So we just need to double check the math and implement something very similar to rust-esplora-client: https://github.com/bitcoindevkit/rust-esplora-client/blob/6002aeaeea220ab3aa80f88b7d8a1f9306a292f6/src/lib.rs#L89-L102

/// 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)
}

with a set of tests and a docstring.
Cc @oleonardolima. Let's try to hack this together this week before Bitcoin conf.

[storopoli]

@notmandatory can you assign me?

[storopoli]

Actually the estimatefee is deprecated in ElectrumX: https://electrumx.readthedocs.io/en/latest/protocol-methods.html#blockchain.estimatefee (same with relayfee, also get_fee_histogram).

So I don't think we should rely in bdk_electrum for anything fee related.

[storopoli]

Maybe we should close this :/

[notmandatory]

@matthiasdebernardini what do you think? since you opened this one I'll let you do the honors of closing it since it seems to be deprecated and support will depend on who's server you connect to.

[LLFourn]

So there's no way at all to do this with electrum?

[tnull]

Actually the estimatefee is deprecated in ElectrumX: https://electrumx.readthedocs.io/en/latest/protocol-methods.html#blockchain.estimatefee (same with relayfee, also get_fee_histogram).

So I don't think we should rely in bdk_electrum for anything fee related.

I think you're looking at the wrong docs (all the Electrum forks are very confusing). IIUC, the relevant fork of ElectrumX that didn't drop Bitcoin is https://github.com/spesmilo/electrumx, so the relevant protocol docs are at https://electrumx-spesmilo.readthedocs.io/en/latest/protocol-methods.html which doesn't mention anything about deprecation.

[storopoli]

Thanks @tnull, so we can implement this in the same way as the rust-esplora-client convert_fee_rate: https://github.com/bitcoindevkit/rust-esplora-client/blob/6002aeaeea220ab3aa80f88b7d8a1f9306a292f6/src/lib.rs#L89-L102

/// 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)
}

Does that sound good you @tnull and @matthiasdebernardini?

I am thinking on something similar to the esplora implementation above, that you pass a target as a parameter.

[tnull]

Does that sound good you @tnull and @matthiasdebernardini?

I am thinking on something similar to the esplora implementation above, that you pass a target as a parameter.

I don't think so? Electrum's estimatefee calls through to Bitcoin Core's estimatesmartfee RPC API which takes a single target and returns the fee estimation for that target. IMO, it makes sense that electrum-client's estimate_fee method could/should return a FeeRate, but logic like the one above doesn't seem necessary as Electrum won't return a HashMap of results anyways?

[ValuedMammal]

@matthiasdebernardini I think this would work

const ELECTRUM_URL: &str = "tcp://electrum.blockstream.info:50001";

fn main() -> Result<(), anyhow::Error> {
    let client = electrum_client::Client::new(ELECTRUM_URL)?;

    for n in 1..=6 {
        let btc_kvb = client.estimate_fee(n)?;
        // 0.00001000 btc/kvb * 1e8 sat/btc * 1vb/4wu = 250 sat/kwu
        let sat_kwu = (btc_kvb * 1e8 * 0.25) as u64;
        let feerate = bitcoin::FeeRate::from_sat_per_kwu(sat_kwu);
        println!("{}.00 sat/vb", feerate.to_sat_per_vb_floor());
    }

    Ok(())
}

[notmandatory]

@matthiasdebernardini I'm removing this from the 1.0.0-beta milestone. Let me know if @ValuedMammal's suggestion above works for you and if so I'll close this one.