Merge pull request #6394 from fdefelici/refactor/btc-rpc-rollout-step2-6387

refactor: BitcoinRegtestController RPC roll out (STEP 1)

Author: fdefelici

stacks-node/src/burnchains/bitcoin_regtest_controller.rs

@@ -80,6 +80,7 @@ use url::Url;
 use super::super::operations::BurnchainOpSigner;
 use super::super::Config;
 use super::{BurnchainController, BurnchainTip, Error as BurnchainControllerError};
+use crate::burnchains::rpc::bitcoin_rpc_client::BitcoinRpcClient;
 
 /// The number of bitcoin blocks that can have
 ///  passed since the UTXO cache was last refreshed before
@@ -101,6 +102,7 @@ pub struct BitcoinRegtestController {
     burnchain_config: Option<Burnchain>,
     ongoing_block_commit: Option<OngoingBlockCommit>,
     should_keep_running: Option<Arc<AtomicBool>>,
+    rpc_client: BitcoinRpcClient,
 }
 
 #[derive(Clone)]
@@ -348,6 +350,9 @@ impl BitcoinRegtestController {
             should_keep_running: should_keep_running.clone(),
         };
 
+        let rpc_client = BitcoinRpcClient::from_stx_config(&config)
+            .expect("unable to instantiate the RPC client!");
+
         Self {
             use_coordinator: coordinator_channel,
             config,
@@ -358,6 +363,7 @@ impl BitcoinRegtestController {
             burnchain_config: burnchain,
             ongoing_block_commit: None,
             should_keep_running,
+            rpc_client,
         }
     }
 
@@ -394,6 +400,9 @@ impl BitcoinRegtestController {
             should_keep_running: None,
         };
 
+        let rpc_client = BitcoinRpcClient::from_stx_config(&config)
+            .expect("unable to instantiate the RPC client!");
+
         Self {
             use_coordinator: None,
             config,
@@ -404,6 +413,7 @@ impl BitcoinRegtestController {
             burnchain_config: None,
             ongoing_block_commit: None,
             should_keep_running: None,
+            rpc_client,
         }
     }
 
@@ -1520,11 +1530,7 @@ impl BitcoinRegtestController {
         for txid in ongoing_op.txids.iter() {
             // check if ongoing_op is in the burnchain_db *or* has been confirmed via the bitcoin RPC
             let mined_op = burnchain_db.find_burnchain_op(&self.indexer, txid);
-            let ongoing_tx_confirmed = mined_op.is_some()
-                || matches!(
-                    BitcoinRPCRequest::check_transaction_confirmed(&self.config, txid),
-                    Ok(true)
-                );
+            let ongoing_tx_confirmed = mined_op.is_some() || self.is_transaction_confirmed(txid);
 
             test_debug!("Ongoing Tx confirmed: {ongoing_tx_confirmed} - TXID: {txid}");
             if ongoing_tx_confirmed {
@@ -2147,6 +2153,26 @@ impl BitcoinRegtestController {
             }
         }
     }
+
+    /// Checks whether a transaction has been confirmed by the burnchain
+    ///
+    /// # Arguments
+    ///
+    /// * `txid` - The transaction ID to check (in big-endian order)
+    ///
+    /// # Returns
+    ///
+    /// * `true` if the transaction is confirmed (has at least one confirmation).
+    /// * `false` if the transaction is unconfirmed or could not be found.
+    pub fn is_transaction_confirmed(&self, txid: &Txid) -> bool {
+        match self.rpc_client.get_transaction(txid) {
+            Ok(info) => info.confirmations > 0,
+            Err(e) => {
+                error!("Bitcoin RPC failure: checking tx confirmation {e:?}");
+                false
+            }
+        }
+    }
 }
 
 impl BurnchainController for BitcoinRegtestController {
@@ -2496,32 +2522,6 @@ impl BitcoinRPCRequest {
         Ok(res.get("result").unwrap().as_str().unwrap().to_string())
     }
 
-    /// Was a given transaction ID confirmed by the burnchain?
-    pub fn check_transaction_confirmed(config: &Config, txid: &Txid) -> RPCResult<bool> {
-        let payload = BitcoinRPCRequest {
-            method: "gettransaction".to_string(),
-            params: vec![format!("{txid}").into()],
-            id: "stacks".to_string(),
-            jsonrpc: "2.0".to_string(),
-        };
-        let res = BitcoinRPCRequest::send(config, payload)?;
-        let confirmations = res
-            .get("result")
-            .ok_or_else(|| RPCError::Parsing("No 'result' field in bitcoind RPC response".into()))?
-            .get("confirmations")
-            .ok_or_else(|| {
-                RPCError::Parsing("No 'confirmations' field in bitcoind RPC response".into())
-            })?
-            .as_i64()
-            .ok_or_else(|| {
-                RPCError::Parsing(
-                    "Expected 'confirmations' field to be numeric in bitcoind RPC response".into(),
-                )
-            })?;
-
-        Ok(confirmations >= 1)
-    }
-
     pub fn generate_to_address(config: &Config, num_blocks: u64, address: String) -> RPCResult<()> {
         debug!("Generate {num_blocks} blocks to {address}");
         let payload = BitcoinRPCRequest {
@@ -3118,8 +3118,7 @@ mod tests {
         ];
 
         // test serialize_tx()
-        let mut config = Config::default();
-        config.burnchain.magic_bytes = "T3".as_bytes().into();
+        let config = utils::create_config();
 
         let mut btc_controller = BitcoinRegtestController::new(config, None);
         let mut utxo_set = UTXOSet {
@@ -3496,6 +3495,36 @@ mod tests {
         );
     }
 
+    #[test]
+    #[ignore]
+    fn test_tx_confirmed_from_utxo_ok() {
+        if env::var("BITCOIND_TEST") != Ok("1".into()) {
+            return;
+        }
+
+        let miner_pubkey = utils::create_miner1_pubkey();
+
+        let mut config = utils::create_config();
+        config.burnchain.local_mining_public_key = Some(miner_pubkey.to_hex());
+
+        let mut btcd_controller = BitcoinCoreController::from_stx_config(&config);
+        btcd_controller
+            .start_bitcoind()
+            .expect("bitcoind should be started!");
+
+        let btc_controller = BitcoinRegtestController::new(config.clone(), None);
+
+        btc_controller.bootstrap_chain(101);
+        let utxos = btc_controller.get_all_utxos(&miner_pubkey);
+        assert_eq!(1, utxos.len(), "One UTXO should be confirmed!");
+
+        let txid = Txid::from_bitcoin_tx_hash(&utxos[0].txid);
+        assert!(
+            btc_controller.is_transaction_confirmed(&txid),
+            "UTXO tx should be confirmed!"
+        );
+    }
+
     /// Tests related to Leader Block Commit operation
     mod leader_commit_op {
         use super::*;

stacks-node/src/burnchains/rpc/bitcoin_rpc_client/mod.rs

@@ -161,14 +161,13 @@ pub struct ListUnspentResponse {
     pub confirmations: u32,
 }
 
-/// Deserializes a JSON string (hex-encoded in big-endian order) into [`Txid`],
-/// storing bytes in little-endian order
+/// Deserializes a JSON string (hex-encoded in big-endian order) into [`Txid`].
 fn deserialize_string_to_txid<'de, D>(deserializer: D) -> Result<Txid, D::Error>
 where
     D: Deserializer<'de>,
 {
     let hex_str: String = Deserialize::deserialize(deserializer)?;
-    let txid = Txid::from_bitcoin_hex(&hex_str).map_err(serde::de::Error::custom)?;
+    let txid = Txid::from_hex(&hex_str).map_err(serde::de::Error::custom)?;
     Ok(txid)
 }
 
@@ -305,14 +304,14 @@ impl<'de> Deserialize<'de> for GenerateToAddressResponse {
 /// Response mainly used as deserialization wrapper for [`Txid`]
 struct TxidWrapperResponse(pub Txid);
 
-/// Deserializes a JSON string (hex-encoded, big-endian) into [`Txid`] and wrap it into [`TxidWrapperResponse`]
+/// Deserializes a JSON string (hex-encoded in big-endian order) into [`Txid`] and wrap it into [`TxidWrapperResponse`]
 impl<'de> Deserialize<'de> for TxidWrapperResponse {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
         let hex_str: String = Deserialize::deserialize(deserializer)?;
-        let txid = Txid::from_bitcoin_hex(&hex_str).map_err(serde::de::Error::custom)?;
+        let txid = Txid::from_hex(&hex_str).map_err(serde::de::Error::custom)?;
         Ok(TxidWrapperResponse(txid))
     }
 }
@@ -558,21 +557,19 @@ impl BitcoinRpcClient {
     /// hex-encoded transaction, and other metadata for a transaction tracked by the wallet.
     ///
     /// # Arguments
-    /// * `txid` - The transaction ID (as [`Txid`]) to query,
-    ///            which is intended to be created with [`Txid::from_bitcoin_hex`],
-    ///            or an analogous process.
+    /// * `txid` - The transaction ID (as [`Txid`]) to query (in big-endian order).
     ///
     /// # Returns
     /// A [`GetTransactionResponse`] containing detailed metadata for the specified transaction.
     ///
     /// # Availability
     /// - **Since**: Bitcoin Core **v0.10.0**.
     pub fn get_transaction(&self, txid: &Txid) -> BitcoinRpcClientResult<GetTransactionResponse> {
-        let btc_txid = txid.to_bitcoin_hex();
-
-        Ok(self
-            .wallet_ep
-            .send(&self.client_id, "gettransaction", vec![btc_txid.into()])?)
+        Ok(self.wallet_ep.send(
+            &self.client_id,
+            "gettransaction",
+            vec![txid.to_hex().into()],
+        )?)
     }
 
     /// Broadcasts a raw transaction to the Bitcoin network.
@@ -591,7 +588,7 @@ impl BitcoinRpcClient {
     ///     - If `None`, defaults to `0`, meaning burning is not allowed.
     ///
     /// # Returns
-    /// A [`Txid`] as a transaction ID (storing internally bytes in **little-endian** order)
+    /// A [`Txid`] as a transaction ID (in big-endian order)
     ///
     /// # Availability
     /// - **Since**: Bitcoin Core **v0.7.0**.

stacks-node/src/burnchains/rpc/bitcoin_rpc_client/test_utils.rs

@@ -143,21 +143,18 @@ impl BitcoinRpcClient {
     /// Retrieves and deserializes a raw Bitcoin transaction by its ID.
     ///
     /// # Arguments
-    /// * `txid` - Transaction ID to fetch, which is intended to be created with [`Txid::from_bitcoin_hex`],
-    ///            or an analogous process.
+    /// * `txid` - The transaction ID (as [`Txid`]) to query (in big-endian order).
     ///
     /// # Returns
     /// A [`Transaction`] struct representing the decoded transaction.
     ///
     /// # Availability
     /// - **Since**: Bitcoin Core **v0.7.0**.
     pub fn get_raw_transaction(&self, txid: &Txid) -> BitcoinRpcClientResult<Transaction> {
-        let btc_txid = txid.to_bitcoin_hex();
-
         let raw_hex = self.global_ep.send::<String>(
             &self.client_id,
             "getrawtransaction",
-            vec![btc_txid.to_string().into()],
+            vec![txid.to_hex().into()],
         )?;
         Ok(deserialize_hex(&raw_hex)?)
     }
@@ -249,7 +246,7 @@ impl BitcoinRpcClient {
     /// * `amount` - Amount to send in BTC (not in satoshis).
     ///
     /// # Returns
-    /// A [`Txid`] struct representing the transaction ID (storing internally bytes in **little-endian** order)
+    /// A [`Txid`] as a transaction ID (in big-endian order)
     ///
     /// # Availability
     /// - **Since**: Bitcoin Core **v0.1.0**.

stacks-node/src/burnchains/rpc/bitcoin_rpc_client/tests.rs

@@ -351,7 +351,7 @@ fn test_list_unspent_ok() {
     assert_eq!(0, utxo.vout);
     assert_eq!(expected_address, utxo.address.to_string());
     assert_eq!(6, utxo.confirmations);
-    assert_eq!(expected_txid_str, utxo.txid.to_bitcoin_hex(),);
+    assert_eq!(expected_txid_str, utxo.txid.to_hex(),);
     assert_eq!(expected_script_hex, format!("{:x}", utxo.script_pub_key),);
 }
 
@@ -469,7 +469,7 @@ fn test_get_transaction_ok() {
 
     let client = utils::setup_client(&server);
 
-    let txid = Txid::from_bitcoin_hex(&txid_hex).unwrap();
+    let txid = Txid::from_hex(&txid_hex).unwrap();
     let info = client.get_transaction(&txid).expect("Should be ok!");
     assert_eq!(6, info.confirmations);
 }
@@ -503,7 +503,7 @@ fn test_get_raw_transaction_ok() {
 
     let client = utils::setup_client(&server);
 
-    let txid = Txid::from_bitcoin_hex(txid_hex).unwrap();
+    let txid = Txid::from_hex(txid_hex).unwrap();
     let raw_tx = client.get_raw_transaction(&txid).expect("Should be ok!");
     assert_eq!(txid_hex, raw_tx.txid().to_string());
     assert_eq!(expected_tx_hex, serialize_hex(&raw_tx).unwrap());
@@ -625,7 +625,7 @@ fn test_send_raw_transaction_ok_with_defaults() {
     let txid = client
         .send_raw_transaction(&raw_tx, None, None)
         .expect("Should work!");
-    assert_eq!(expected_txid, txid.to_bitcoin_hex());
+    assert_eq!(expected_txid, txid.to_hex());
 }
 
 #[test]
@@ -661,7 +661,7 @@ fn test_send_raw_transaction_ok_with_custom_params() {
     let txid = client
         .send_raw_transaction(&raw_tx, Some(0.0), Some(5_000))
         .expect("Should work!");
-    assert_eq!(expected_txid, txid.to_bitcoin_hex());
+    assert_eq!(expected_txid, txid.to_hex());
 }
 
 #[test]
@@ -882,7 +882,7 @@ fn test_send_to_address_ok() {
     let txid = client
         .send_to_address(&address, amount)
         .expect("Should be ok!");
-    assert_eq!(expected_txid_str, txid.to_bitcoin_hex());
+    assert_eq!(expected_txid_str, txid.to_hex());
 }
 
 #[test]

stacks-node/src/tests/bitcoin_rpc_integrations.rs

@@ -564,7 +564,7 @@ fn test_get_raw_transaction_ok() {
         .get_raw_transaction(&txid)
         .expect("get raw transaction ok!");
 
-    assert_eq!(txid.to_bitcoin_hex(), raw_tx.txid().to_string());
+    assert_eq!(txid.to_hex(), raw_tx.txid().to_string());
 }
 
 #[ignore]
@@ -795,5 +795,5 @@ fn test_send_raw_transaction_rebroadcast_ok() {
         .send_raw_transaction(&raw_tx, None, None)
         .expect("send raw transaction (rebroadcast) ok!");
 
-    assert_eq!(txid.to_bitcoin_hex(), raw_tx.txid().to_string());
+    assert_eq!(txid.to_hex(), raw_tx.txid().to_string());
 }

stacks-node/src/tests/signer/v0.rs

@@ -104,7 +104,6 @@ use tracing_subscriber::prelude::*;
 use tracing_subscriber::{fmt, EnvFilter};
 
 use super::SignerTest;
-use crate::burnchains::bitcoin_regtest_controller::BitcoinRPCRequest;
 use crate::event_dispatcher::{
     EventObserver, MinedNakamotoBlockEvent, TEST_SKIP_BLOCK_ANNOUNCEMENT,
 };
@@ -4032,8 +4031,7 @@ fn tx_replay_failsafe() {
 
     // Wait for the block commit re-broadcast to be confirmed
     wait_for(10, || {
-        let is_confirmed =
-            BitcoinRPCRequest::check_transaction_confirmed(&conf, &commit_txid.unwrap()).unwrap();
+        let is_confirmed = btc_controller.is_transaction_confirmed(&commit_txid.unwrap());
         Ok(is_confirmed)
     })
     .expect("Timed out waiting for transaction to be confirmed");
@@ -4317,8 +4315,7 @@ fn tx_replay_disagreement() {
 
     // Wait for the block commit re-broadcast to be confirmed
     wait_for(10, || {
-        let is_confirmed =
-            BitcoinRPCRequest::check_transaction_confirmed(&conf, &commit_txid.unwrap()).unwrap();
+        let is_confirmed = btc_controller.is_transaction_confirmed(&commit_txid.unwrap());
         Ok(is_confirmed)
     })
     .expect("Timed out waiting for transaction to be confirmed");