refactor(devimint): add get_block_count to bitcoind

[bradleystachurski]

Followup for #3893 (comment)

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (747585f) 56.94% compared to head (022a8a6) 56.99%.

Files	Patch %	Lines
devimint/src/external.rs	0.00%	3 Missing ⚠️
devimint/src/federation.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3898      +/-   ##
==========================================
+ Coverage   56.94%   56.99%   +0.05%     
==========================================
  Files         193      193              
  Lines       42740    42743       +3     
==========================================
+ Hits        24337    24361      +24     
+ Misses      18403    18382      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[elsirion]

We already account for that afaik:

fedimint/fedimint-bitcoind/src/bitcoincore.rs

Lines 56 to 61 in 747585f

	async fn get_block_count(&self) -> anyhow::Result<u64> {
	// The RPC function is confusingly named and actually returns the block height
	block_in_place(|| self.0.get_block_count())
	.map(|height| height + 1)
	.map_err(anyhow::Error::from)
	}

Maybe something else is wrong?

[bradleystachurski]

I also found this a bit confusing. The client for Bitcoind in devimint external uses bitcoincore_rpc::Client instead of IBitcoindRpc, so we're essentially duplicating impl.

Should we consider changing the devimint external client to IBitcoindRpc? If so, I can create an issue.

[bradleystachurski]

Hmm, on further thought I'm not sure it's a good idea to switch to IBitcoindRpc in devimint external. We rely on accessing several methods from bitcoind's rpc that would make unifying with IBitcoindRpc pretty messy.

[elsirion]

Oh, then nevermind.