refactor: setup bitcoin client use in validation (#573)

* Add get_tx_info function for the BitcoinInteract trait.
* Remove the inner Context type.
* Make sure the BitcoinCoreClient is Clone.
* Implement TryFrom<&Url> for BitcoinCoreClient.
* Have the AsContractCall validation functions take something that implements Context.

signer/src/api/new_block.rs

@@ -161,9 +161,8 @@ mod tests {
     {
         let db = Store::new_shared();
 
-        let ctx =
-            NoopSignerContext::init(&Settings::new_from_default_config().unwrap(), db.clone())
-                .expect("failed to init context");
+        let ctx = NoopSignerContext::init(Settings::new_from_default_config().unwrap(), db.clone())
+            .expect("failed to init context");
 
         let api = ApiState { ctx: ctx.clone() };
 
@@ -191,9 +190,8 @@ mod tests {
     {
         let db = Store::new_shared();
 
-        let ctx =
-            NoopSignerContext::init(&Settings::new_from_default_config().unwrap(), db.clone())
-                .expect("failed to init context");
+        let ctx = NoopSignerContext::init(Settings::new_from_default_config().unwrap(), db.clone())
+            .expect("failed to init context");
 
         let api = ApiState { ctx: ctx.clone() };
 

signer/src/bitcoin/client.rs

@@ -14,7 +14,8 @@
 //! - Example when trying to get a block that doesn't exist:
 //!   JsonRpc(Rpc(RpcError { code: -5, message: "Block not found", data: None }))
 
-use bitcoincore_rpc::jsonrpc::error::RpcError;
+use bitcoin::BlockHash;
+use bitcoin::Txid;
 use bitcoincore_rpc::RpcApi as _;
 use url::Url;
 
@@ -26,6 +27,7 @@ use crate::keys::PublicKey;
 use crate::util::ApiFallbackClient;
 
 use super::rpc::BitcoinCoreClient;
+use super::rpc::BitcoinTxInfo;
 use super::rpc::GetTxResponse;
 
 /// Implement the [`TryFrom`] trait for a slice of [`Url`]s to allow for a
@@ -35,7 +37,7 @@ impl TryFrom<&[Url]> for ApiFallbackClient<BitcoinCoreClient> {
     fn try_from(urls: &[Url]) -> Result<Self, Self::Error> {
         let clients = urls
             .iter()
-            .map(|url| BitcoinCoreClient::try_from(url.clone()))
+            .map(BitcoinCoreClient::try_from)
             .collect::<Result<Vec<_>, _>>()?;
 
         Self::new(clients).map_err(Into::into)
@@ -47,22 +49,22 @@ impl BitcoinInteract for ApiFallbackClient<BitcoinCoreClient> {
         &self,
         block_hash: &bitcoin::BlockHash,
     ) -> Result<Option<bitcoin::Block>, Error> {
-        self.exec(|client| async {
-            match client.inner_client().get_block(block_hash) {
-                Ok(block) => Ok(Some(block)),
-                Err(bitcoincore_rpc::Error::JsonRpc(bitcoincore_rpc::jsonrpc::Error::Rpc(
-                    RpcError { code: -5, .. },
-                ))) => Ok(None),
-                Err(error) => Err(Error::BitcoinCoreRpc(error)),
-            }
-        })
-        .await
+        self.exec(|client| async { client.get_block(block_hash) })
+            .await
     }
 
-    fn get_tx(&self, txid: &bitcoin::Txid) -> Result<GetTxResponse, Error> {
+    fn get_tx(&self, txid: &Txid) -> Result<Option<GetTxResponse>, Error> {
         self.get_client().get_tx(txid)
     }
 
+    fn get_tx_info(
+        &self,
+        txid: &Txid,
+        block_hash: &BlockHash,
+    ) -> Result<Option<BitcoinTxInfo>, Error> {
+        self.get_client().get_tx_info(txid, block_hash)
+    }
+
     async fn estimate_fee_rate(&self) -> Result<f64, Error> {
         todo!() // TODO(542)
     }

signer/src/bitcoin/mod.rs

@@ -2,6 +2,10 @@
 
 use std::future::Future;
 
+use bitcoin::BlockHash;
+use bitcoin::Txid;
+
+use rpc::BitcoinTxInfo;
 use rpc::GetTxResponse;
 
 use crate::error::Error;
@@ -20,11 +24,18 @@ pub trait BitcoinInteract {
     /// Get block
     fn get_block(
         &self,
-        block_hash: &bitcoin::BlockHash,
+        block_hash: &BlockHash,
     ) -> impl Future<Output = Result<Option<bitcoin::Block>, Error>> + Send;
 
     /// get tx
-    fn get_tx(&self, txid: &bitcoin::Txid) -> Result<GetTxResponse, Error>;
+    fn get_tx(&self, txid: &Txid) -> Result<Option<GetTxResponse>, Error>;
+
+    /// get tx info
+    fn get_tx_info(
+        &self,
+        txid: &Txid,
+        block_hash: &BlockHash,
+    ) -> Result<Option<BitcoinTxInfo>, Error>;
 
     /// Estimate fee rate
     // This should be implemented with the help of the `fees::EstimateFees` trait

signer/src/bitcoin/rpc.rs

@@ -1,21 +1,30 @@
 //! Contains client wrappers for bitcoin core and electrum.
 
+use std::sync::Arc;
+
 use bitcoin::Amount;
+use bitcoin::Block;
 use bitcoin::BlockHash;
 use bitcoin::Denomination;
+use bitcoin::OutPoint;
 use bitcoin::Transaction;
 use bitcoin::Txid;
 use bitcoin::Wtxid;
 use bitcoincore_rpc::json::EstimateMode;
+use bitcoincore_rpc::jsonrpc::error::Error as JsonRpcError;
+use bitcoincore_rpc::jsonrpc::error::RpcError;
 use bitcoincore_rpc::Auth;
+use bitcoincore_rpc::Error as BtcRpcError;
 use bitcoincore_rpc::RpcApi as _;
 use bitcoincore_rpc_json::GetRawTransactionResultVin;
 use bitcoincore_rpc_json::GetRawTransactionResultVout as BitcoinTxInfoVout;
 use bitcoincore_rpc_json::GetRawTransactionResultVoutScriptPubKey as BitcoinTxInfoScriptPubKey;
 use serde::Deserialize;
 use url::Url;
 
+use crate::bitcoin::BitcoinInteract;
 use crate::error::Error;
+use crate::keys::PublicKey;
 
 /// A slimmed down type representing a response from bitcoin-core's
 /// getrawtransaction RPC.
@@ -151,17 +160,18 @@ pub struct FeeEstimate {
 }
 
 /// A client for interacting with bitcoin-core
+#[derive(Debug, Clone)]
 pub struct BitcoinCoreClient {
     /// The underlying bitcoin-core client
-    inner: bitcoincore_rpc::Client,
+    inner: Arc<bitcoincore_rpc::Client>,
 }
 
 /// Implement TryFrom for Url to allow for easy conversion from a URL to a
 /// BitcoinCoreClient.
-impl TryFrom<Url> for BitcoinCoreClient {
+impl TryFrom<&Url> for BitcoinCoreClient {
     type Error = Error;
 
-    fn try_from(url: Url) -> Result<Self, Self::Error> {
+    fn try_from(url: &Url) -> Result<Self, Self::Error> {
         let username = url.username().to_string();
         let password = url.password().unwrap_or_default().to_string();
         let host = url
@@ -184,6 +194,7 @@ impl BitcoinCoreClient {
     pub fn new(url: &str, username: String, password: String) -> Result<Self, Error> {
         let auth = Auth::UserPass(username, password);
         let client = bitcoincore_rpc::Client::new(url, auth)
+            .map(Arc::new)
             .map_err(|err| Error::BitcoinCoreRpcClient(err, url.to_string()))?;
 
         Ok(Self { inner: client })
@@ -194,16 +205,31 @@ impl BitcoinCoreClient {
         &self.inner
     }
 
+    /// Fetch the block identified by the given block hash.
+    pub fn get_block(&self, block_hash: &BlockHash) -> Result<Option<Block>, Error> {
+        match self.inner.get_block(block_hash) {
+            Ok(block) => Ok(Some(block)),
+            Err(BtcRpcError::JsonRpc(JsonRpcError::Rpc(RpcError { code: -5, .. }))) => Ok(None),
+            Err(error) => Err(Error::BitcoinCoreGetBlock(error, *block_hash)),
+        }
+    }
+
     /// Fetch and decode raw transaction from bitcoin-core using the
-    /// getrawtransaction RPC with a verbosity of 1.
+    /// getrawtransaction RPC with a verbosity of 1. None is returned if
+    /// the node cannot find the transaction in a bitcoin block or the
+    /// mempool.
     ///
     /// # Notes
     ///
     /// By default, this call only returns a transaction if it is in the
     /// mempool. If -txindex is enabled on bitcoin-core and no blockhash
     /// argument is passed, it will return the transaction if it is in the
-    /// mempool or any block.
-    pub fn get_tx(&self, txid: &Txid) -> Result<GetTxResponse, Error> {
+    /// mempool or any block. We require -txindex to be enabled (same with
+    /// stacks-core[^1]) so this should work with transactions in either
+    /// the mempool and a bitcoin block.
+    ///
+    /// [^1]: <https://docs.stacks.co/guides-and-tutorials/run-a-miner/mine-mainnet-stacks-tokens>
+    pub fn get_tx(&self, txid: &Txid) -> Result<Option<GetTxResponse>, Error> {
         let args = [
             serde_json::to_value(txid).map_err(Error::JsonSerialize)?,
             // This is the verbosity level. The acceptable values are 0, 1,
@@ -213,9 +239,15 @@ impl BitcoinCoreClient {
             serde_json::Value::Null,
         ];
 
-        self.inner
-            .call("getrawtransaction", &args)
-            .map_err(|err| Error::GetTransactionBitcoinCore(err, *txid))
+        match self.inner.call::<GetTxResponse>("getrawtransaction", &args) {
+            Ok(tx_info) => Ok(Some(tx_info)),
+            // If the transaction is not found in an
+            // actual block then the message is "No such transaction found
+            // in the provided block. Use gettransaction for wallet
+            // transactions." In both cases the code is the same.
+            Err(BtcRpcError::JsonRpc(JsonRpcError::Rpc(RpcError { code: -5, .. }))) => Ok(None),
+            Err(err) => Err(Error::BitcoinCoreGetTransaction(err, *txid)),
+        }
     }
 
     /// Fetch and decode raw transaction from bitcoin-core using the
@@ -225,7 +257,11 @@ impl BitcoinCoreClient {
     ///
     /// We require bitcoin-core v25 or later. For bitcoin-core v24 and
     /// earlier, this function will return an error.
-    pub fn get_tx_info(&self, txid: &Txid, block_hash: &BlockHash) -> Result<BitcoinTxInfo, Error> {
+    pub fn get_tx_info(
+        &self,
+        txid: &Txid,
+        block_hash: &BlockHash,
+    ) -> Result<Option<BitcoinTxInfo>, Error> {
         let args = [
             serde_json::to_value(txid).map_err(Error::JsonSerialize)?,
             // This is the verbosity level. The acceptable values are 0, 1,
@@ -235,9 +271,16 @@ impl BitcoinCoreClient {
             serde_json::to_value(block_hash).map_err(Error::JsonSerialize)?,
         ];
 
-        self.inner
-            .call("getrawtransaction", &args)
-            .map_err(|err| Error::GetTransactionBitcoinCore(err, *txid))
+        match self.inner.call::<BitcoinTxInfo>("getrawtransaction", &args) {
+            Ok(tx_info) => Ok(Some(tx_info)),
+            // If the `block_hash` is not found then the message is "Block
+            // hash not found", while if the transaction is not found in an
+            // actual block then the message is "No such transaction found
+            // in the provided block. Use gettransaction for wallet
+            // transactions." In both cases the code is the same.
+            Err(BtcRpcError::JsonRpc(JsonRpcError::Rpc(RpcError { code: -5, .. }))) => Ok(None),
+            Err(err) => Err(Error::BitcoinCoreGetTransaction(err, *txid)),
+        }
     }
 
     /// Estimates the approximate fee in sats per vbyte needed for a
@@ -275,3 +318,40 @@ impl BitcoinCoreClient {
         Ok(FeeEstimate { sats_per_vbyte })
     }
 }
+
+impl BitcoinInteract for BitcoinCoreClient {
+    async fn broadcast_transaction(&self, _: &Transaction) -> Result<(), Error> {
+        unimplemented!()
+    }
+
+    async fn get_block(&self, block_hash: &BlockHash) -> Result<Option<Block>, Error> {
+        self.get_block(block_hash)
+    }
+
+    fn get_tx(&self, txid: &Txid) -> Result<Option<GetTxResponse>, Error> {
+        self.get_tx(txid)
+    }
+
+    fn get_tx_info(
+        &self,
+        txid: &Txid,
+        block_hash: &BlockHash,
+    ) -> Result<Option<BitcoinTxInfo>, Error> {
+        self.get_tx_info(txid, block_hash)
+    }
+
+    async fn estimate_fee_rate(&self) -> Result<f64, Error> {
+        todo!()
+    }
+
+    async fn get_signer_utxo(
+        &self,
+        _: &PublicKey,
+    ) -> Result<Option<super::utxo::SignerUtxo>, Error> {
+        todo!()
+    }
+
+    async fn get_last_fee(&self, _: OutPoint) -> Result<Option<super::utxo::Fees>, Error> {
+        todo!()
+    }
+}

signer/src/block_observer.rs

@@ -79,7 +79,9 @@ impl DepositRequestValidator for CreateDepositRequest {
         C: BitcoinInteract,
     {
         // Fetch the transaction from either a block or from the mempool
-        let response = client.get_tx(&self.outpoint.txid)?;
+        let Some(response) = client.get_tx(&self.outpoint.txid)? else {
+            return Err(Error::BitcoinTxMissing(self.outpoint.txid));
+        };
 
         Ok(Deposit {
             info: self.validate_tx(&response.tx)?,
@@ -331,6 +333,7 @@ mod tests {
     use rand::seq::IteratorRandom;
     use rand::SeedableRng;
 
+    use crate::bitcoin::rpc::BitcoinTxInfo;
     use crate::bitcoin::rpc::GetTxResponse;
     use crate::bitcoin::utxo;
     use crate::config::Settings;
@@ -362,11 +365,10 @@ mod tests {
         let storage = storage::in_memory::Store::new_shared();
         let test_harness = TestHarness::generate(&mut rng, 20, 0..5);
         let ctx = SignerContext::new(
-            &Settings::new_from_default_config().unwrap(),
+            Settings::new_from_default_config().unwrap(),
             storage.clone(),
             test_harness.clone(),
-        )
-        .unwrap();
+        );
         let block_hash_stream = test_harness.spawn_block_hash_stream();
         let (subscribers, subscriber_rx) = tokio::sync::watch::channel(());
 
@@ -469,11 +471,10 @@ mod tests {
         let block_hash_stream = test_harness.spawn_block_hash_stream();
         let (subscribers, _subscriber_rx) = tokio::sync::watch::channel(());
         let ctx = SignerContext::new(
-            &Settings::new_from_default_config().unwrap(),
+            Settings::new_from_default_config().unwrap(),
             storage.clone(),
             test_harness.clone(),
-        )
-        .unwrap();
+        );
 
         let mut block_observer = BlockObserver {
             stacks_client: test_harness.clone(),
@@ -546,11 +547,10 @@ mod tests {
         let block_hash_stream = test_harness.spawn_block_hash_stream();
         let (subscribers, _subscriber_rx) = tokio::sync::watch::channel(());
         let ctx = SignerContext::new(
-            &Settings::new_from_default_config().unwrap(),
+            Settings::new_from_default_config().unwrap(),
             storage.clone(),
             test_harness.clone(),
-        )
-        .unwrap();
+        );
 
         let mut block_observer = BlockObserver {
             stacks_client: test_harness.clone(),
@@ -774,9 +774,14 @@ mod tests {
     }
 
     impl BitcoinInteract for TestHarness {
-        fn get_tx(&self, txid: &bitcoin::Txid) -> Result<GetTxResponse, Error> {
-            self.deposits.get(txid).cloned().ok_or(Error::Encryption)
+        fn get_tx(&self, txid: &bitcoin::Txid) -> Result<Option<GetTxResponse>, Error> {
+            Ok(self.deposits.get(txid).cloned())
         }
+
+        fn get_tx_info(&self, _: &Txid, _: &BlockHash) -> Result<Option<BitcoinTxInfo>, Error> {
+            unimplemented!()
+        }
+
         async fn get_block(
             &self,
             block_hash: &bitcoin::BlockHash,