From 0f40268a3c2085dee9cfa803c70fcecf51f9f3d3 Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Sun, 22 Sep 2024 22:47:25 -0400
Subject: [PATCH 01/15] Add the types for deserializing getrawtransaction
 responses

---
 Cargo.lock      |  1 +
 Cargo.toml      |  1 +
 sbtc/Cargo.toml |  1 +
 sbtc/src/rpc.rs | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+)

diff --git a/Cargo.lock b/Cargo.lock
index 2b6708aa..d92486ac 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -5465,6 +5465,7 @@ version = "0.1.0"
 dependencies = [
  "bitcoin",
  "bitcoincore-rpc",
+ "bitcoincore-rpc-json",
  "clarity",
  "rand 0.8.5",
  "secp256k1 0.29.0",
diff --git a/Cargo.toml b/Cargo.toml
index 7f5f7955..c8b6ce8f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -33,6 +33,7 @@ base64 = "0.22.1"
 bincode = "1.3.3"
 bitcoin = { version = "0.32", features = ["serde"] }
 bitcoincore-rpc = { version = "0.19" }
+bitcoincore-rpc-json = { version = "0.19" }
 bitvec = { version = "1.0", default-features = false, features = ["serde"] }
 clarity = { git = "https://github.com/stacks-network/stacks-core", rev = "cbf0c52fb7a0b8ac55badadcf3773ca0848a25cf" }
 clap = { version = "4.5.4", features = ["derive", "env"] }
diff --git a/sbtc/Cargo.toml b/sbtc/Cargo.toml
index 1476e11a..bb7c877d 100644
--- a/sbtc/Cargo.toml
+++ b/sbtc/Cargo.toml
@@ -13,6 +13,7 @@ testing = []
 [dependencies]
 bitcoin = { workspace = true, features = ["rand-std"] }
 bitcoincore-rpc.workspace = true
+bitcoincore-rpc-json.workspace = true
 clarity.workspace = true
 rand.workspace = true
 serde.workspace = true
diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 870568b0..d70524af 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -4,13 +4,18 @@ use std::future::Future;
 
 use bitcoin::consensus;
 use bitcoin::consensus::Decodable as _;
+use bitcoin::Amount;
 use bitcoin::BlockHash;
 use bitcoin::Denomination;
 use bitcoin::Transaction;
 use bitcoin::Txid;
+use bitcoin::Wtxid;
 use bitcoincore_rpc::json::EstimateMode;
 use bitcoincore_rpc::Auth;
 use bitcoincore_rpc::RpcApi as _;
+use bitcoincore_rpc_json::GetRawTransactionResultVin;
+use bitcoincore_rpc_json::GetRawTransactionResultVout as GetRawTxResponseVout;
+use bitcoincore_rpc_json::GetRawTransactionResultVoutScriptPubKey as GetRawTxResponseScriptPubKey;
 use serde::Deserialize;
 use url::Url;
 
@@ -43,6 +48,93 @@ pub struct GetTxResponse {
     pub in_active_chain: Option<bool>,
 }
 
+/// A description of an input into a transaction.
+#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
+pub struct GetRawTxResponseVin {
+    /// Most of the details to the input into the transaction
+    #[serde(flatten)]
+    pub vin: GetRawTransactionResultVin,
+    /// The previous output, omitted if block undo data is not available.
+    pub prevout: Option<GetRawTxResponseVinPrevout>,
+}
+
+/// The previous output, omitted if block undo data is not available.
+#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
+pub struct GetRawTxResponseVinPrevout {
+    /// Whether this is a Coinbase or not.
+    pub generated: bool,
+    /// The height of the prevout.
+    pub height: u64,
+    /// The value of the prevout in BTC.
+    #[serde(with = "bitcoin::amount::serde::as_btc")]
+    pub value: Amount,
+    /// The scriptPubKey of the prevout.
+    #[serde(rename = "scriptPubKey")]
+    pub script_pub_key: GetRawTxResponseScriptPubKey,
+}
+
+/// A struct containing the response from bitcoin-core for a
+/// getrawtransaction RPC where verbose is set to 2.
+///
+/// # Notes
+///
+/// Although there is
+/// [`GetRawTransactionResult`](bitcoincore_rpc_json::GetRawTransactionResult)
+/// type, that type is missing some information that we want. It is
+/// populated with a `getrawtransaction` RPC where verbose is set to 1, and
+/// it seems the follow the developer.bitcoin.org docs [^1] instead of the
+/// bitcoincore.org docs.
+///
+/// The docs here are taken from the bitcoin-core docs, which can be found
+/// here:
+/// <https://bitcoincore.org/en/doc/27.0.0/rpc/rawtransactions/getrawtransaction/>
+///
+/// [^1]: <https://developer.bitcoin.org/reference/rpc/getrawtransaction.html>
+#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
+pub struct GetRawTxResponse {
+    /// Whether the specified block (in the getrawtransaction RPC) is in
+    /// the active chain or not. It is only present when the "blockhash"
+    /// argument is present in the RPC.
+    pub in_active_chain: Option<bool>,
+    /// The transaction fee paid to the bitcoin miners.
+    #[serde(with = "bitcoin::amount::serde::as_btc::opt")]
+    pub fee: Option<Amount>,
+    /// The raw bitcoin transaction.
+    #[serde(with = "bitcoin::consensus::serde::With::<bitcoin::consensus::serde::Hex>")]
+    #[serde(rename = "hex")]
+    pub tx: Transaction,
+    /// The transaction id (the same value provided in the RPC).
+    pub txid: Txid,
+    /// The transaction hash (differs from txid for witness transactions).
+    pub hash: Wtxid,
+    /// The serialized transaction size.
+    pub size: usize,
+    /// The virtual transaction size (differs from size for witness
+    /// transactions).
+    pub vsize: usize,
+    /// The version.
+    pub version: u32,
+    /// The transaction lock time.
+    #[serde(rename = "locktime")]
+    pub lock_time: u32,
+    /// The inputs into the transaction.
+    pub vin: Vec<GetRawTxResponseVin>,
+    /// A description of the transactions outputs. This object is missing
+    /// the `desc` field in the `scriptPubKey` object. That field is the
+    /// "Inferred descriptor for the output".
+    pub vout: Vec<GetRawTxResponseVout>,
+    /// The block hash of the Bitcoin block that includes this transaction.
+    #[serde(rename = "blockhash")]
+    pub block_hash: Option<BlockHash>,
+    /// The number of confirmations deep from that chain tip of the bitcoin
+    /// block that includes this transaction.
+    pub confirmations: Option<u32>,
+    /// The Unix epoch time when the block was mined. It reflects the
+    /// timestamp as recorded by the miner of the block.
+    #[serde(rename = "blocktime")]
+    pub block_time: Option<u64>,
+}
+
 /// A struct representing the recommended fee, in sats per vbyte, from a
 /// particular source.
 #[derive(Debug, Clone, Copy, PartialEq)]

From 0b100fd5ac38fa0e7a4ef219da34e99aa763a331 Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Sun, 22 Sep 2024 23:11:01 -0400
Subject: [PATCH 02/15] Add implementation of function that fetches
 transactions from bitcoin-core

---
 sbtc/src/error.rs |  3 +++
 sbtc/src/rpc.rs   | 24 +++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/sbtc/src/error.rs b/sbtc/src/error.rs
index 001eb160..cc5c76c3 100644
--- a/sbtc/src/error.rs
+++ b/sbtc/src/error.rs
@@ -54,6 +54,9 @@ pub enum Error {
     /// The reclaim script was invalid.
     #[error("the reclaim script format was invalid")]
     InvalidReclaimScript,
+    /// This should never happen.
+    #[error("could not serialize the type into JSON")]
+    JsonSerialize(#[source] serde_json::Error),
     /// Failed to convert response into an Amount, which is unsigned and
     /// bounded.
     #[error("Could not convert float {1} into bitcoin::Amount: {0}")]
diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index d70524af..26fa748c 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -97,7 +97,7 @@ pub struct GetRawTxResponse {
     /// argument is present in the RPC.
     pub in_active_chain: Option<bool>,
     /// The transaction fee paid to the bitcoin miners.
-    #[serde(with = "bitcoin::amount::serde::as_btc::opt")]
+    #[serde(default, with = "bitcoin::amount::serde::as_btc::opt")]
     pub fee: Option<Amount>,
     /// The raw bitcoin transaction.
     #[serde(with = "bitcoin::consensus::serde::With::<bitcoin::consensus::serde::Hex>")]
@@ -224,7 +224,29 @@ impl BitcoinCoreClient {
             in_active_chain: response.in_active_chain,
         })
     }
+    /// Fetch and decode raw transaction from bitcoin-core using the
+    /// getrawtransaction RPC with a verbosity of 2.
+    ///
+    /// # 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_raw_tx(&self, txid: &Txid, block_hash: Option<&BlockHash>) -> Result<GetRawTxResponse, Error> {
+        let args = [
+            serde_json::to_value(txid).map_err(Error::JsonSerialize)?,
+            // This is the verbosity level. The acceptable values are 0, 1,
+            // and 2, and we want the 2 because it will include the fee
+            // information.
+            serde_json::Value::Number(serde_json::value::Number::from(2u32)),
+            serde_json::to_value(block_hash).map_err(Error::JsonSerialize)?,
+        ];
 
+        self.inner
+            .call("getrawtransaction", &args)
+            .map_err(|err| Error::GetTransactionBitcoinCore(err, *txid))
+    }
     /// Estimates the approximate fee in sats per vbyte needed for a
     /// transaction to be confirmed within `num_blocks`.
     ///

From 5fb3d5e6786fe64843f6a25ed0d0d20c3588068f Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 01:14:42 -0400
Subject: [PATCH 03/15] Polish the docs [ci skip]

---
 sbtc/src/rpc.rs | 85 ++++++++++++++++++++++++-------------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 26fa748c..84f26593 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -14,8 +14,8 @@ use bitcoincore_rpc::json::EstimateMode;
 use bitcoincore_rpc::Auth;
 use bitcoincore_rpc::RpcApi as _;
 use bitcoincore_rpc_json::GetRawTransactionResultVin;
-use bitcoincore_rpc_json::GetRawTransactionResultVout as GetRawTxResponseVout;
-use bitcoincore_rpc_json::GetRawTransactionResultVoutScriptPubKey as GetRawTxResponseScriptPubKey;
+use bitcoincore_rpc_json::GetRawTransactionResultVout as BitcoinTxInfoVout;
+use bitcoincore_rpc_json::GetRawTransactionResultVoutScriptPubKey as BitcoinTxInfoScriptPubKey;
 use serde::Deserialize;
 use url::Url;
 
@@ -50,17 +50,17 @@ pub struct GetTxResponse {
 
 /// A description of an input into a transaction.
 #[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
-pub struct GetRawTxResponseVin {
+pub struct BitcoinTxInfoVin {
     /// Most of the details to the input into the transaction
     #[serde(flatten)]
-    pub vin: GetRawTransactionResultVin,
+    pub details: GetRawTransactionResultVin,
     /// The previous output, omitted if block undo data is not available.
-    pub prevout: Option<GetRawTxResponseVinPrevout>,
+    pub prevout: Option<BitcoinTxInfoVinPrevout>,
 }
 
 /// The previous output, omitted if block undo data is not available.
 #[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
-pub struct GetRawTxResponseVinPrevout {
+pub struct BitcoinTxInfoVinPrevout {
     /// Whether this is a Coinbase or not.
     pub generated: bool,
     /// The height of the prevout.
@@ -70,35 +70,39 @@ pub struct GetRawTxResponseVinPrevout {
     pub value: Amount,
     /// The scriptPubKey of the prevout.
     #[serde(rename = "scriptPubKey")]
-    pub script_pub_key: GetRawTxResponseScriptPubKey,
+    pub script_pub_key: BitcoinTxInfoScriptPubKey,
 }
 
 /// A struct containing the response from bitcoin-core for a
-/// getrawtransaction RPC where verbose is set to 2.
+/// `getrawtransaction` RPC where verbose is set to 2 where the block hash
+/// is supplied as an RPC argument.
 ///
 /// # Notes
 ///
-/// Although there is
-/// [`GetRawTransactionResult`](bitcoincore_rpc_json::GetRawTransactionResult)
-/// type, that type is missing some information that we want. It is
-/// populated with a `getrawtransaction` RPC where verbose is set to 1, and
-/// it seems the follow the developer.bitcoin.org docs [^1] instead of the
-/// bitcoincore.org docs.
-///
-/// The docs here are taken from the bitcoin-core docs, which can be found
-/// here:
-/// <https://bitcoincore.org/en/doc/27.0.0/rpc/rawtransactions/getrawtransaction/>
-///
-/// [^1]: <https://developer.bitcoin.org/reference/rpc/getrawtransaction.html>
+/// * This struct is a slightly modified version of the
+///   [`GetRawTransactionResult`](bitcoincore_rpc_json::GetRawTransactionResult)
+///   type, which is what the bitcoincore-rpc crate returns for the
+///   `getrawtransaction` RPC with verbosity set to 1. That type is missing
+///   some information that we may want.
+/// * The `block_hash`, `block_time`, `confirmations`, `fee`, and
+///   `is_active_chain` fields are always populated from bitcoin-core for a
+///   `getrawtransaction` RPC with verbosity 2 when the `block_hash` is
+///   supplied. That is why they are not `Option`s here.
+/// * This struct omits some fields returned from bitcoin-core, most
+///   notably the `vin.prevout.script_pub_key.desc` field.
+/// * Since we require bitcoin-core v25 or later these docs were taken from
+///   <https://bitcoincore.org/en/doc/25.0.0/rpc/rawtransactions/getrawtransaction/>
+///   and not from the more generic bitcoin.org docs
+///   <https://developer.bitcoin.org/reference/rpc/getrawtransaction.html>
 #[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
-pub struct GetRawTxResponse {
+pub struct BitcoinTxInfo {
     /// Whether the specified block (in the getrawtransaction RPC) is in
     /// the active chain or not. It is only present when the "blockhash"
     /// argument is present in the RPC.
-    pub in_active_chain: Option<bool>,
+    pub in_active_chain: bool,
     /// The transaction fee paid to the bitcoin miners.
-    #[serde(default, with = "bitcoin::amount::serde::as_btc::opt")]
-    pub fee: Option<Amount>,
+    #[serde(default, with = "bitcoin::amount::serde::as_btc")]
+    pub fee: Amount,
     /// The raw bitcoin transaction.
     #[serde(with = "bitcoin::consensus::serde::With::<bitcoin::consensus::serde::Hex>")]
     #[serde(rename = "hex")]
@@ -108,31 +112,26 @@ pub struct GetRawTxResponse {
     /// The transaction hash (differs from txid for witness transactions).
     pub hash: Wtxid,
     /// The serialized transaction size.
-    pub size: usize,
+    pub size: u64,
     /// The virtual transaction size (differs from size for witness
     /// transactions).
-    pub vsize: usize,
-    /// The version.
-    pub version: u32,
-    /// The transaction lock time.
-    #[serde(rename = "locktime")]
-    pub lock_time: u32,
+    pub vsize: u64,
     /// The inputs into the transaction.
-    pub vin: Vec<GetRawTxResponseVin>,
+    pub vin: Vec<BitcoinTxInfoVin>,
     /// A description of the transactions outputs. This object is missing
     /// the `desc` field in the `scriptPubKey` object. That field is the
     /// "Inferred descriptor for the output".
-    pub vout: Vec<GetRawTxResponseVout>,
+    pub vout: Vec<BitcoinTxInfoVout>,
     /// The block hash of the Bitcoin block that includes this transaction.
     #[serde(rename = "blockhash")]
-    pub block_hash: Option<BlockHash>,
+    pub block_hash: BlockHash,
     /// The number of confirmations deep from that chain tip of the bitcoin
     /// block that includes this transaction.
-    pub confirmations: Option<u32>,
+    pub confirmations: u32,
     /// The Unix epoch time when the block was mined. It reflects the
     /// timestamp as recorded by the miner of the block.
     #[serde(rename = "blocktime")]
-    pub block_time: Option<u64>,
+    pub block_time: u64,
 }
 
 /// A struct representing the recommended fee, in sats per vbyte, from a
@@ -225,20 +224,18 @@ impl BitcoinCoreClient {
         })
     }
     /// Fetch and decode raw transaction from bitcoin-core using the
-    /// getrawtransaction RPC with a verbosity of 2.
+    /// `getrawtransaction` RPC with a verbosity of 2.
     ///
     /// # 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_raw_tx(&self, txid: &Txid, block_hash: Option<&BlockHash>) -> Result<GetRawTxResponse, Error> {
+    /// 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> {
         let args = [
             serde_json::to_value(txid).map_err(Error::JsonSerialize)?,
             // This is the verbosity level. The acceptable values are 0, 1,
-            // and 2, and we want the 2 because it will include the fee
-            // information.
+            // and 2, and we want the 2 because it will include all of the
+            // required fields of the type.
             serde_json::Value::Number(serde_json::value::Number::from(2u32)),
             serde_json::to_value(block_hash).map_err(Error::JsonSerialize)?,
         ];

From ece1b6357ff406ccd306d15c4ed809c06f27306b Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 09:00:12 -0400
Subject: [PATCH 04/15] reorder stuff

---
 sbtc/src/rpc.rs | 50 ++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 84f26593..b39675ec 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -48,31 +48,6 @@ pub struct GetTxResponse {
     pub in_active_chain: Option<bool>,
 }
 
-/// A description of an input into a transaction.
-#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
-pub struct BitcoinTxInfoVin {
-    /// Most of the details to the input into the transaction
-    #[serde(flatten)]
-    pub details: GetRawTransactionResultVin,
-    /// The previous output, omitted if block undo data is not available.
-    pub prevout: Option<BitcoinTxInfoVinPrevout>,
-}
-
-/// The previous output, omitted if block undo data is not available.
-#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
-pub struct BitcoinTxInfoVinPrevout {
-    /// Whether this is a Coinbase or not.
-    pub generated: bool,
-    /// The height of the prevout.
-    pub height: u64,
-    /// The value of the prevout in BTC.
-    #[serde(with = "bitcoin::amount::serde::as_btc")]
-    pub value: Amount,
-    /// The scriptPubKey of the prevout.
-    #[serde(rename = "scriptPubKey")]
-    pub script_pub_key: BitcoinTxInfoScriptPubKey,
-}
-
 /// A struct containing the response from bitcoin-core for a
 /// `getrawtransaction` RPC where verbose is set to 2 where the block hash
 /// is supplied as an RPC argument.
@@ -134,6 +109,31 @@ pub struct BitcoinTxInfo {
     pub block_time: u64,
 }
 
+/// A description of an input into a transaction.
+#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
+pub struct BitcoinTxInfoVin {
+    /// Most of the details to the input into the transaction
+    #[serde(flatten)]
+    pub details: GetRawTransactionResultVin,
+    /// The previous output, omitted if block undo data is not available.
+    pub prevout: Option<BitcoinTxInfoVinPrevout>,
+}
+
+/// The previous output, omitted if block undo data is not available.
+#[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
+pub struct BitcoinTxInfoVinPrevout {
+    /// Whether this is a Coinbase or not.
+    pub generated: bool,
+    /// The height of the prevout.
+    pub height: u64,
+    /// The value of the prevout in BTC.
+    #[serde(with = "bitcoin::amount::serde::as_btc")]
+    pub value: Amount,
+    /// The scriptPubKey of the prevout.
+    #[serde(rename = "scriptPubKey")]
+    pub script_pub_key: BitcoinTxInfoScriptPubKey,
+}
+
 /// A struct representing the recommended fee, in sats per vbyte, from a
 /// particular source.
 #[derive(Debug, Clone, Copy, PartialEq)]

From 3b41221470d7e1a41cf4391da24a559b6d351a5f Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 09:00:27 -0400
Subject: [PATCH 05/15] use bitcoin-core v25 in CI

---
 docker-compose.test.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docker-compose.test.yml b/docker-compose.test.yml
index 59362c3b..cd95650e 100644
--- a/docker-compose.test.yml
+++ b/docker-compose.test.yml
@@ -2,7 +2,7 @@ services:
 
   bitcoind:
     container_name: bitcoind
-    image: lncm/bitcoind:v27.0
+    image: lncm/bitcoind:v25.0
     volumes:
       - ./signer/tests/service-configs/bitcoin.conf:/data/.bitcoin/bitcoin.conf:ro
     restart: on-failure

From f056b254f290634d85dc5906bf988c546cf7eeea Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 10:09:33 -0400
Subject: [PATCH 06/15] Implement functions for apportioning fees for a bitcoin
 transaction

---
 sbtc/src/rpc.rs | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index b39675ec..37528560 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -7,8 +7,10 @@ use bitcoin::consensus::Decodable as _;
 use bitcoin::Amount;
 use bitcoin::BlockHash;
 use bitcoin::Denomination;
+use bitcoin::OutPoint;
 use bitcoin::Transaction;
 use bitcoin::Txid;
+use bitcoin::Weight;
 use bitcoin::Wtxid;
 use bitcoincore_rpc::json::EstimateMode;
 use bitcoincore_rpc::Auth;
@@ -109,6 +111,78 @@ pub struct BitcoinTxInfo {
     pub block_time: u64,
 }
 
+impl BitcoinTxInfo {
+    /// Assess how much of the bitcoin miner fee should be apportioned to
+    /// the input associated with the given `outpoint`.
+    ///
+    /// # Notes
+    ///
+    /// Each input and output is assessed a fee that is proportional to
+    /// their weight amount all of the requests serviced by this
+    /// transaction.
+    ///
+    /// This function assumes that this transaction is an sBTC transaction,
+    /// which implies that the first input and the first two outputs are
+    /// always the signers'. So `None` is returned if there is no input,
+    /// after the first input, with the given `outpoint`.
+    pub fn assess_input_fee(&self, outpoint: OutPoint) -> Option<Amount> {
+        let request_weight_vbytes = self.request_weight().to_vbytes_ceil();
+        // We skip the first input because that is always the signers'
+        // input UTXO.
+        let input_weight_vbytes = self
+            .tx
+            .input
+            .iter()
+            .find(|tx_in| tx_in.previous_output == outpoint)?
+            .segwit_weight()
+            .to_vbytes_ceil();
+
+        let fee_sats = (input_weight_vbytes * self.fee.to_sat()).div_ceil(request_weight_vbytes);
+        Some(Amount::from_sat(fee_sats))
+    }
+
+    /// Assess how much of the bitcoin miner fee should be apportioned to
+    /// the output at the given output index `vout`.
+    ///
+    /// # Notes
+    ///
+    /// Each input and output is assessed a fee that is proportional to
+    /// their weight amount all of the requests serviced by this
+    /// transaction.
+    ///
+    /// This function assumes that this transaction is an sBTC transaction,
+    /// which implies that the first input and the first two outputs are
+    /// always the signers'. So `None` is returned if the given `vout` is 0
+    /// or 1 or if there is no output in the transaction at `vout`.
+    pub fn assess_output_fee(&self, vout: usize) -> Option<Amount> {
+        // We skip the first input because that is always the signers'
+        // input UTXO.
+        if vout < 2 {
+            return None;
+        }
+        let request_weight_vbytes = self.request_weight().to_vbytes_ceil();
+        let input_weight_vbytes = self.tx.output.get(vout)?.weight().to_vbytes_ceil();
+
+        let fee_sats = (input_weight_vbytes * self.fee.to_sat()).div_ceil(request_weight_vbytes);
+        Some(Amount::from_sat(fee_sats))
+    }
+
+    /// Computes the total weight of the inputs and the outputs, excluding
+    /// the ones related to the signers.
+    fn request_weight(&self) -> Weight {
+        // We skip the first input and output because those are always the
+        // signers' UTXO input and output. We skip the second output
+        // because that is always the OP_RETURN output for sBTC data.
+        self.tx
+            .input
+            .iter()
+            .skip(1)
+            .map(|x| x.segwit_weight())
+            .chain(self.tx.output.iter().skip(2).map(|x| x.weight()))
+            .sum()
+    }
+}
+
 /// A description of an input into a transaction.
 #[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize, serde::Serialize)]
 pub struct BitcoinTxInfoVin {

From 58b03c99ba0cf405ff411df2a842b8de21ed3d4e Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 10:56:24 -0400
Subject: [PATCH 07/15] Update the comments for the fee

---
 sbtc/src/rpc.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 37528560..6bacf3f4 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -78,6 +78,18 @@ pub struct BitcoinTxInfo {
     /// argument is present in the RPC.
     pub in_active_chain: bool,
     /// The transaction fee paid to the bitcoin miners.
+    /// 
+    /// This field is returned whenever the "block undo data" is present
+    /// for a block. The block undo data is always present for validated
+    /// blocks, and block validation is always done for blocks on the
+    /// currently active chain [1]. So if this field is missing then this
+    /// block has not been validated and so is not on the active
+    /// blockchain.
+    /// 
+    /// [1]: https://bitcoincore.reviews/23319#l-133,
+    ///     https://bitcoincore.reviews/23319#l-141,
+    ///     https://bitcoincore.reviews/23319#l-147,
+    ///     https://bitcoincore.reviews/23319#l-153
     #[serde(default, with = "bitcoin::amount::serde::as_btc")]
     pub fee: Amount,
     /// The raw bitcoin transaction.
@@ -297,6 +309,7 @@ impl BitcoinCoreClient {
             in_active_chain: response.in_active_chain,
         })
     }
+
     /// Fetch and decode raw transaction from bitcoin-core using the
     /// `getrawtransaction` RPC with a verbosity of 2.
     ///
@@ -318,6 +331,7 @@ impl BitcoinCoreClient {
             .call("getrawtransaction", &args)
             .map_err(|err| Error::GetTransactionBitcoinCore(err, *txid))
     }
+
     /// Estimates the approximate fee in sats per vbyte needed for a
     /// transaction to be confirmed within `num_blocks`.
     ///

From 1013dd7a6abf1baf15e22f7f4fb21cdf5fea7955 Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 16:09:34 -0400
Subject: [PATCH 08/15] Update after removing an unnecessary field

---
 sbtc/src/deposits.rs          |  1 -
 sbtc/src/error.rs             |  6 +++---
 sbtc/src/rpc.rs               | 40 +++++++++++++----------------------
 sbtc/tests/integration/rpc.rs | 11 ----------
 signer/src/block_observer.rs  |  3 ---
 5 files changed, 18 insertions(+), 43 deletions(-)

diff --git a/sbtc/src/deposits.rs b/sbtc/src/deposits.rs
index 187b296a..a772e1c4 100644
--- a/sbtc/src/deposits.rs
+++ b/sbtc/src/deposits.rs
@@ -596,7 +596,6 @@ mod tests {
                 block_hash: None,
                 confirmations: None,
                 block_time: None,
-                in_active_chain: None,
             })
         }
     }
diff --git a/sbtc/src/error.rs b/sbtc/src/error.rs
index cc5c76c3..2898a93a 100644
--- a/sbtc/src/error.rs
+++ b/sbtc/src/error.rs
@@ -16,9 +16,9 @@ pub enum Error {
     /// Error when using a BitcoinClient trait function
     #[error("could not execute bitcoin client RPC call {0}")]
     BitcoinClient(#[source] Box<dyn std::error::Error + Sync + Send + 'static>),
-    // /// Error when creating an RPC client to bitcoin-core
-    // #[error("could not create RPC client to {1}: {0}")]
-    // BitcoinCoreRpcClient(#[source] bitcoincore_rpc::Error, String),
+    /// Error when creating an RPC client to bitcoin-core
+    #[error("could not create RPC client to {1}: {0}")]
+    BitcoinCoreRpcClient(#[source] bitcoincore_rpc::Error, String),
     /// Returned when we could not decode the hex into a
     /// bitcoin::Transaction.
     #[error("failed to decode the provided hex into a transaction. txid: {1}. {0}")]
diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 6bacf3f4..23144b6b 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -2,8 +2,6 @@
 
 use std::future::Future;
 
-use bitcoin::consensus;
-use bitcoin::consensus::Decodable as _;
 use bitcoin::Amount;
 use bitcoin::BlockHash;
 use bitcoin::Denomination;
@@ -27,11 +25,11 @@ use crate::error::Error;
 /// getrawtransaction RPC.
 ///
 /// The docs for the getrawtransaction RPC call can be found here:
-/// https://bitcoincore.org/en/doc/27.0.0/rpc/rawtransactions/getrawtransaction/.
+/// https://bitcoincore.org/en/doc/25.0.0/rpc/rawtransactions/getrawtransaction/.
 #[derive(Debug, Clone, Deserialize)]
 pub struct GetTxResponse {
     /// The raw bitcoin transaction.
-    #[serde(with = "consensus::serde::With::<consensus::serde::Hex>")]
+    #[serde(with = "bitcoin::consensus::serde::With::<bitcoin::consensus::serde::Hex>")]
     #[serde(rename = "hex")]
     pub tx: Transaction,
     /// The block hash of the Bitcoin block that includes this transaction.
@@ -44,10 +42,6 @@ pub struct GetTxResponse {
     /// timestamp as recorded by the miner of the block.
     #[serde(rename = "blocktime")]
     pub block_time: Option<u64>,
-    /// Whether the specified block (in the getrawtransaction RPC) is in
-    /// the active chain or not. It is only present when the "blockhash"
-    /// argument is present in the RPC.
-    pub in_active_chain: Option<bool>,
 }
 
 /// A struct containing the response from bitcoin-core for a
@@ -272,7 +266,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_err(|err| Error::BitcoinClient(Box::new(err)))?;
+            .map_err(|err| Error::BitcoinCoreRpcClient(err, url.to_string()))?;
 
         Ok(Self { inner: client })
     }
@@ -283,7 +277,7 @@ impl BitcoinCoreClient {
     }
 
     /// Fetch and decode raw transaction from bitcoin-core using the
-    /// getrawtransaction RPC.
+    /// getrawtransaction RPC with a verbosity of 1.
     ///
     /// # Notes
     ///
@@ -292,22 +286,18 @@ impl BitcoinCoreClient {
     /// 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> {
-        let response = self
-            .inner
-            .get_raw_transaction_info(txid, None)
-            .map_err(|err| Error::GetTransactionBitcoinCore(err, *txid))?;
-        let tx = Transaction::consensus_decode(&mut response.hex.as_slice())
-            .map_err(|err| Error::DecodeTx(err, *txid))?;
-
-        debug_assert_eq!(txid, &response.txid);
+        let args = [
+            serde_json::to_value(txid).map_err(Error::JsonSerialize)?,
+            // This is the verbosity level. The acceptable values are 0, 1,
+            // and 2, and we want the 1 for some additional information
+            // over just the raw transaction.
+            serde_json::Value::Number(serde_json::value::Number::from(1u32)),
+            serde_json::Value::Null,
+        ];
 
-        Ok(GetTxResponse {
-            tx,
-            block_hash: response.blockhash,
-            confirmations: response.confirmations,
-            block_time: response.blocktime.map(|time| time as u64),
-            in_active_chain: response.in_active_chain,
-        })
+        self.inner
+            .call("getrawtransaction", &args)
+            .map_err(|err| Error::GetTransactionBitcoinCore(err, *txid))
     }
 
     /// Fetch and decode raw transaction from bitcoin-core using the
diff --git a/sbtc/tests/integration/rpc.rs b/sbtc/tests/integration/rpc.rs
index 401c14cd..86c3b360 100644
--- a/sbtc/tests/integration/rpc.rs
+++ b/sbtc/tests/integration/rpc.rs
@@ -37,7 +37,6 @@ fn btc_client_gets_transactions() {
     assert!(response.block_hash.is_none());
     assert!(response.block_time.is_none());
     assert!(response.confirmations.is_none());
-    assert!(response.in_active_chain.is_none());
 
     // Now let's confirm it and try again
     faucet.generate_blocks(1);
@@ -50,16 +49,6 @@ fn btc_client_gets_transactions() {
     assert!(response.block_hash.is_some());
     assert!(response.block_time.is_some());
     assert_eq!(response.confirmations, Some(1));
-    // The `in_active_chain` field is tricky, it needs more confirmations
-    // before it is set. Moreover, it only gets set with the electrum
-    // client. Under the hood, electrum looks up the blockhash of the given
-    // txid and makes a getrawtransaction call to bitcoin-core with this
-    // optional blockhash input, and bitcoin-core will only set the
-    // `in_active_chain` field in the response if it has the blockhash
-    // input in the request. So we stop here and just check that it is
-    // still None. If this was the ElectrumClient then after one more
-    // `faucet.generate_blocks(1)` call it would be set to `Some(true)`.
-    assert!(response.in_active_chain.is_none());
 }
 
 #[cfg_attr(not(feature = "integration-tests"), ignore)]
diff --git a/signer/src/block_observer.rs b/signer/src/block_observer.rs
index c157e864..ad99955e 100644
--- a/signer/src/block_observer.rs
+++ b/signer/src/block_observer.rs
@@ -394,7 +394,6 @@ mod tests {
             block_hash: None,
             confirmations: None,
             block_time: None,
-            in_active_chain: None,
         };
 
         let tx_setup1 = sbtc::testing::deposits::tx_setup(300, 2000, amount);
@@ -415,7 +414,6 @@ mod tests {
             block_hash: None,
             confirmations: None,
             block_time: None,
-            in_active_chain: None,
         };
 
         // Let's add the "responses" to the field that feeds the
@@ -496,7 +494,6 @@ mod tests {
             block_hash: None,
             confirmations: None,
             block_time: None,
-            in_active_chain: None,
         };
 
         // Let's add the "responses" to the field that feeds the

From fd272f366aa3f434a3baedf10169aa06e610566b Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 16:15:36 -0400
Subject: [PATCH 09/15] Add an integration test for the new get_tx_info
 function. Update an old test after bitcoin-core version change

---
 sbtc/tests/integration/rpc.rs        | 42 +++++++++++++++++++++++++++-
 sbtc/tests/integration/validation.rs |  8 ++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/sbtc/tests/integration/rpc.rs b/sbtc/tests/integration/rpc.rs
index 86c3b360..26024bae 100644
--- a/sbtc/tests/integration/rpc.rs
+++ b/sbtc/tests/integration/rpc.rs
@@ -9,7 +9,7 @@ use sbtc::testing::regtest::Recipient;
 
 #[cfg_attr(not(feature = "integration-tests"), ignore)]
 #[test]
-fn btc_client_gets_transactions() {
+fn btc_client_getstransaction() {
     let client = BitcoinCoreClient::new(
         "http://localhost:18443",
         regtest::BITCOIN_CORE_RPC_USERNAME.to_string(),
@@ -51,6 +51,46 @@ fn btc_client_gets_transactions() {
     assert_eq!(response.confirmations, Some(1));
 }
 
+#[cfg_attr(not(feature = "integration-tests"), ignore)]
+#[test]
+fn btc_client_gets_transaction_info() {
+    let client = BitcoinCoreClient::new(
+        "http://localhost:18443",
+        regtest::BITCOIN_CORE_RPC_USERNAME.to_string(),
+        regtest::BITCOIN_CORE_RPC_PASSWORD.to_string(),
+    )
+    .unwrap();
+    let (rpc, faucet) = regtest::initialize_blockchain();
+    let signer = Recipient::new(AddressType::P2tr);
+
+    // Newly created "recipients" do not have any UTXOs associated with
+    // their address.
+    let balance = signer.get_balance(rpc);
+    assert_eq!(balance.to_sat(), 0);
+
+    // Okay now we send coins to an address from the one address that
+    // coins have been mined to.
+    let outpoint = faucet.send_to(500_000, &signer.address);
+    let vout = outpoint.vout as usize;
+
+    let response = client.get_tx(&outpoint.txid).unwrap();
+    // Let's make sure we got the right transaction
+    assert_eq!(response.tx.compute_txid(), outpoint.txid);
+    assert_eq!(response.tx.output[vout].value.to_sat(), 500_000);
+    // The transaction has not been confirmed, so these should be None.
+    assert!(response.block_hash.is_none());
+    assert!(response.block_time.is_none());
+    assert!(response.confirmations.is_none());
+
+    // Now let's confirm it and try again
+    let block_hash = faucet.generate_blocks(1).pop().unwrap();
+
+    let response = client.get_tx_info(&outpoint.txid, &block_hash).unwrap();
+    // Let's make sure we got the right transaction
+    assert_eq!(response.tx.compute_txid(), outpoint.txid);
+    assert_eq!(response.tx.output[vout].value.to_sat(), 500_000);
+}
+
 #[cfg_attr(not(feature = "integration-tests"), ignore)]
 #[test]
 fn btc_client_unsubmitted_tx() {
diff --git a/sbtc/tests/integration/validation.rs b/sbtc/tests/integration/validation.rs
index 2ac3e62d..d23745a9 100644
--- a/sbtc/tests/integration/validation.rs
+++ b/sbtc/tests/integration/validation.rs
@@ -380,10 +380,14 @@ fn op_csv_disabled() {
         }],
     };
 
-    let expected = "mandatory-script-verify-flag-failed (Locktime requirement not satisfied)";
+    // In bitcoin-core v25 the message is "non-mandatory-script-verify-flag
+    // (Locktime requirement not satisfied)", but in bitcoin-core v27 the
+    // message is "mandatory-script-verify-flag-failed (Locktime
+    // requirement not satisfied)". We match on the part that is probably
+    // consistent across versions.
     match rpc.send_raw_transaction(&tx3).unwrap_err() {
         BtcRpcError::JsonRpc(JsonRpcError::Rpc(RpcError { code: -26, message, .. }))
-            if message == expected => {}
+            if message.ends_with("(Locktime requirement not satisfied)") => {}
         err => panic!("{err}"),
     };
 }

From c95f8a1861c3acf5157152cf856c3f865f130130 Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Mon, 23 Sep 2024 16:15:48 -0400
Subject: [PATCH 10/15] Make sure sbtc integration tests get run in CI

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 6022c497..c5af73a9 100644
--- a/Makefile
+++ b/Makefile
@@ -73,7 +73,7 @@ integration-env-up: $(INSTALL_TARGET) $(AUTOGENERATED_BLOCKLIST_CLIENT_CLIENT)
 	docker compose --file docker-compose.test.yml up --detach
 
 integration-test: $(INSTALL_TARGET) $(AUTOGENERATED_BLOCKLIST_CLIENT_CLIENT)
-	cargo test --package signer --test integration --all-features -- --test-threads=1
+	cargo test --package sbtc --package signer --test integration --all-features -- --test-threads=1
 
 integration-env-down: $(INSTALL_TARGET) $(AUTOGENERATED_BLOCKLIST_CLIENT_CLIENT)
 	docker compose --file docker-compose.test.yml down

From 64e796130df96a9b6296682b09ae51b31a2fe0bb Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Tue, 24 Sep 2024 00:21:42 -0400
Subject: [PATCH 11/15] Assess the fees using weight units instead of vbytes.

---
 sbtc/src/rpc.rs | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 23144b6b..91efd587 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -25,7 +25,7 @@ use crate::error::Error;
 /// getrawtransaction RPC.
 ///
 /// The docs for the getrawtransaction RPC call can be found here:
-/// https://bitcoincore.org/en/doc/25.0.0/rpc/rawtransactions/getrawtransaction/.
+/// <https://bitcoincore.org/en/doc/25.0.0/rpc/rawtransactions/getrawtransaction/>.
 #[derive(Debug, Clone, Deserialize)]
 pub struct GetTxResponse {
     /// The raw bitcoin transaction.
@@ -72,18 +72,18 @@ pub struct BitcoinTxInfo {
     /// argument is present in the RPC.
     pub in_active_chain: bool,
     /// The transaction fee paid to the bitcoin miners.
-    /// 
+    ///
     /// This field is returned whenever the "block undo data" is present
     /// for a block. The block undo data is always present for validated
     /// blocks, and block validation is always done for blocks on the
-    /// currently active chain [1]. So if this field is missing then this
+    /// currently active chain [1-4]. So if this field is missing then this
     /// block has not been validated and so is not on the active
     /// blockchain.
-    /// 
-    /// [1]: https://bitcoincore.reviews/23319#l-133,
-    ///     https://bitcoincore.reviews/23319#l-141,
-    ///     https://bitcoincore.reviews/23319#l-147,
-    ///     https://bitcoincore.reviews/23319#l-153
+    ///
+    /// [1]: <https://bitcoincore.reviews/23319#l-133>
+    /// [2]: <https://bitcoincore.reviews/23319#l-141>
+    /// [3]: <https://bitcoincore.reviews/23319#l-147>
+    /// [4]: <https://bitcoincore.reviews/23319#l-153>
     #[serde(default, with = "bitcoin::amount::serde::as_btc")]
     pub fee: Amount,
     /// The raw bitcoin transaction.
@@ -132,18 +132,21 @@ impl BitcoinTxInfo {
     /// always the signers'. So `None` is returned if there is no input,
     /// after the first input, with the given `outpoint`.
     pub fn assess_input_fee(&self, outpoint: OutPoint) -> Option<Amount> {
-        let request_weight_vbytes = self.request_weight().to_vbytes_ceil();
+        // The Weight::to_wu function just returns the inner weight units
+        // as a u64, so this is really just the weight.
+        let request_weight = self.request_weight().to_wu();
         // We skip the first input because that is always the signers'
         // input UTXO.
-        let input_weight_vbytes = self
+        let input_weight = self
             .tx
             .input
             .iter()
+            .skip(1)
             .find(|tx_in| tx_in.previous_output == outpoint)?
             .segwit_weight()
-            .to_vbytes_ceil();
+            .to_wu();
 
-        let fee_sats = (input_weight_vbytes * self.fee.to_sat()).div_ceil(request_weight_vbytes);
+        let fee_sats = (input_weight * self.fee.to_sat()).div_ceil(request_weight);
         Some(Amount::from_sat(fee_sats))
     }
 
@@ -166,8 +169,8 @@ impl BitcoinTxInfo {
         if vout < 2 {
             return None;
         }
-        let request_weight_vbytes = self.request_weight().to_vbytes_ceil();
-        let input_weight_vbytes = self.tx.output.get(vout)?.weight().to_vbytes_ceil();
+        let request_weight_vbytes = self.request_weight().to_wu();
+        let input_weight_vbytes = self.tx.output.get(vout)?.weight().to_wu();
 
         let fee_sats = (input_weight_vbytes * self.fee.to_sat()).div_ceil(request_weight_vbytes);
         Some(Amount::from_sat(fee_sats))

From f39b1da769db39f6d0bdcebe26a9820fef2983f2 Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Tue, 24 Sep 2024 00:21:50 -0400
Subject: [PATCH 12/15] Add tests

---
 sbtc/src/rpc.rs | 204 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 204 insertions(+)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 91efd587..f88b25d3 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -367,3 +367,207 @@ impl BitcoinClient for BitcoinCoreClient {
         self.get_tx(txid)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use bitcoin::hashes::Hash as _;
+    use bitcoin::ScriptBuf;
+
+    use super::*;
+
+    impl BitcoinTxInfo {
+        fn from_tx(tx: Transaction, fee: Amount) -> BitcoinTxInfo {
+            BitcoinTxInfo {
+                in_active_chain: true,
+                fee,
+                txid: tx.compute_txid(),
+                hash: tx.compute_wtxid(),
+                size: tx.base_size() as u64,
+                vsize: tx.vsize() as u64,
+                tx,
+                vin: Vec::new(),
+                vout: Vec::new(),
+                block_hash: BlockHash::from_byte_array([0; 32]),
+                confirmations: 1,
+                block_time: 0,
+            }
+        }
+    }
+
+    /// Return a transaction that is kinda like the signers' transaction
+    /// but it does not service any requests and it does not have any
+    /// signatures.
+    fn base_signer_transaction() -> Transaction {
+        Transaction {
+            version: bitcoin::transaction::Version::TWO,
+            lock_time: bitcoin::locktime::absolute::LockTime::ZERO,
+            input: vec![
+                // This is the signers' previous UTXO
+                bitcoin::TxIn {
+                    previous_output: OutPoint::null(),
+                    script_sig: ScriptBuf::new(),
+                    sequence: bitcoin::Sequence::ZERO,
+                    witness: bitcoin::Witness::new(),
+                },
+            ],
+            output: vec![
+                // This represents the signers' new UTXO.
+                bitcoin::TxOut {
+                    value: Amount::ONE_BTC,
+                    script_pubkey: ScriptBuf::new(),
+                },
+                // This represents the OP_RETURN sBTC UTXO for a
+                // transaction with no withdrawals.
+                bitcoin::TxOut {
+                    value: Amount::ZERO,
+                    script_pubkey: ScriptBuf::new_op_return([0; 21]),
+                },
+            ],
+        }
+    }
+
+    #[test]
+    fn sole_deposit_gets_entire_fee() {
+        let deposit_outpoint = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
+        let mut tx = base_signer_transaction();
+        let deposit = bitcoin::TxIn {
+            previous_output: deposit_outpoint,
+            script_sig: ScriptBuf::new(),
+            sequence: bitcoin::Sequence::ZERO,
+            witness: bitcoin::Witness::new(),
+        };
+        tx.input.push(deposit);
+
+        let fee = Amount::from_sat(500_000);
+
+        let tx_info = BitcoinTxInfo::from_tx(tx, fee);
+        let assessed_fee = tx_info.assess_input_fee(deposit_outpoint).unwrap();
+        assert_eq!(assessed_fee, fee);
+    }
+
+    #[test]
+    fn sole_withdrawal_gets_entire_fee() {
+        let mut tx = base_signer_transaction();
+        let locking_script = ScriptBuf::new_op_return([0; 10]);
+        // This represents the signers' new UTXO.
+        let withdrawal = bitcoin::TxOut {
+            value: Amount::from_sat(250_000),
+            script_pubkey: ScriptBuf::new_p2sh(&locking_script.script_hash()),
+        };
+        tx.output.push(withdrawal);
+        let fee = Amount::from_sat(500_000);
+
+        let tx_info = BitcoinTxInfo::from_tx(tx, fee);
+        let assessed_fee = tx_info.assess_output_fee(2).unwrap();
+        assert_eq!(assessed_fee, fee);
+    }
+
+    #[test]
+    fn first_input_and_first_two_outputs_return_none() {
+        let tx = base_signer_transaction();
+        let fee = Amount::from_sat(500_000);
+
+        let tx_info = BitcoinTxInfo::from_tx(tx, fee);
+        assert!(tx_info.assess_output_fee(0).is_none());
+        assert!(tx_info.assess_output_fee(1).is_none());
+
+        assert!(tx_info.assess_input_fee(OutPoint::null()).is_none());
+    }
+
+    #[test]
+    fn two_deposits_same_weight_split_the_fee() {
+        // These deposit inputs are essentially idential by weight. Since
+        // they are the only requests serviced by this trasnaction, they
+        // will have equal weight.
+        let deposit_outpoint1 = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
+        let deposit_outpoint2 = OutPoint::new(Txid::from_byte_array([2; 32]), 0);
+
+        let mut tx = base_signer_transaction();
+        let deposit1 = bitcoin::TxIn {
+            previous_output: deposit_outpoint1,
+            script_sig: ScriptBuf::new(),
+            sequence: bitcoin::Sequence::ZERO,
+            witness: bitcoin::Witness::new(),
+        };
+        let deposit2 = bitcoin::TxIn {
+            previous_output: deposit_outpoint2,
+            script_sig: ScriptBuf::new(),
+            sequence: bitcoin::Sequence::ZERO,
+            witness: bitcoin::Witness::new(),
+        };
+        tx.input.push(deposit1);
+        tx.input.push(deposit2);
+
+        let fee = Amount::from_sat(500_000);
+
+        let tx_info = BitcoinTxInfo::from_tx(tx, fee);
+        let assessed_fee1 = tx_info.assess_input_fee(deposit_outpoint1).unwrap();
+        assert_eq!(assessed_fee1, fee / 2);
+
+        let assessed_fee2 = tx_info.assess_input_fee(deposit_outpoint2).unwrap();
+        assert_eq!(assessed_fee2, fee / 2);
+    }
+
+    #[test]
+    fn two_withdrawals_same_weight_split_the_fee() {
+        let mut tx = base_signer_transaction();
+        let locking_script = ScriptBuf::new_op_return([0; 10]);
+        let withdrawal = bitcoin::TxOut {
+            value: Amount::from_sat(250_000),
+            script_pubkey: ScriptBuf::new_p2sh(&locking_script.script_hash()),
+        };
+        tx.output.push(withdrawal.clone());
+        tx.output.push(withdrawal);
+        let fee = Amount::from_sat(500_000);
+
+        let tx_info = BitcoinTxInfo::from_tx(tx, fee);
+        let assessed_fee1 = tx_info.assess_output_fee(2).unwrap();
+        assert_eq!(assessed_fee1, fee / 2);
+
+        let assessed_fee2 = tx_info.assess_output_fee(3).unwrap();
+        assert_eq!(assessed_fee2, fee / 2);
+    }
+
+    #[test]
+    fn one_deposit_two_withdrawals_fees_add() {
+        // We're just testing that a "regular" bitcoin transaction,
+        // servicing a deposit and two withdrawals, will assess the fees in
+        // a normal way. Here we test that the fee is
+        let deposit_outpoint = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
+
+        let mut tx = base_signer_transaction();
+        let deposit = bitcoin::TxIn {
+            previous_output: deposit_outpoint,
+            script_sig: ScriptBuf::new(),
+            sequence: bitcoin::Sequence::ZERO,
+            witness: bitcoin::Witness::new(),
+        };
+        tx.input.push(deposit);
+
+        let locking_script = ScriptBuf::new_op_return([0; 10]);
+        let withdrawal = bitcoin::TxOut {
+            value: Amount::from_sat(250_000),
+            script_pubkey: ScriptBuf::new_p2sh(&locking_script.script_hash()),
+        };
+        tx.output.push(withdrawal.clone());
+        tx.output.push(withdrawal);
+
+        let fee = Amount::from_sat(500_000);
+
+        let tx_info = BitcoinTxInfo::from_tx(tx, fee);
+        let input_assessed_fee = tx_info.assess_input_fee(deposit_outpoint).unwrap();
+        let output1_assessed_fee = tx_info.assess_output_fee(2).unwrap();
+        let output2_assessed_fee = tx_info.assess_output_fee(3).unwrap();
+
+        assert!(input_assessed_fee > Amount::ZERO);
+        assert!(output1_assessed_fee > Amount::ZERO);
+        assert!(output2_assessed_fee > Amount::ZERO);
+
+        let combined_fee = input_assessed_fee + output1_assessed_fee + output2_assessed_fee;
+
+        assert!(combined_fee >= fee);
+        // Their fees, in sats, should not add up to more than `fee +
+        // number-of-requests`.
+        assert!(combined_fee <= (fee + Amount::from_sat(3u64)));
+    }
+}

From 80e909ca08446aad06b32d7b42f0ca982e81acfe Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Tue, 24 Sep 2024 00:34:46 -0400
Subject: [PATCH 13/15] Fix typos

---
 sbtc/src/rpc.rs | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index f88b25d3..5a973a97 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -124,7 +124,7 @@ impl BitcoinTxInfo {
     /// # Notes
     ///
     /// Each input and output is assessed a fee that is proportional to
-    /// their weight amount all of the requests serviced by this
+    /// their weight amount all the requests serviced by this
     /// transaction.
     ///
     /// This function assumes that this transaction is an sBTC transaction,
@@ -133,7 +133,7 @@ impl BitcoinTxInfo {
     /// after the first input, with the given `outpoint`.
     pub fn assess_input_fee(&self, outpoint: OutPoint) -> Option<Amount> {
         // The Weight::to_wu function just returns the inner weight units
-        // as a u64, so this is really just the weight.
+        // as an u64, so this is really just the weight.
         let request_weight = self.request_weight().to_wu();
         // We skip the first input because that is always the signers'
         // input UTXO.
@@ -156,7 +156,7 @@ impl BitcoinTxInfo {
     /// # Notes
     ///
     /// Each input and output is assessed a fee that is proportional to
-    /// their weight amount all of the requests serviced by this
+    /// their weight amount all the requests serviced by this
     /// transaction.
     ///
     /// This function assumes that this transaction is an sBTC transaction,
@@ -169,10 +169,10 @@ impl BitcoinTxInfo {
         if vout < 2 {
             return None;
         }
-        let request_weight_vbytes = self.request_weight().to_wu();
-        let input_weight_vbytes = self.tx.output.get(vout)?.weight().to_wu();
+        let request_weight = self.request_weight().to_wu();
+        let input_weight = self.tx.output.get(vout)?.weight().to_wu();
 
-        let fee_sats = (input_weight_vbytes * self.fee.to_sat()).div_ceil(request_weight_vbytes);
+        let fee_sats = (input_weight * self.fee.to_sat()).div_ceil(request_weight);
         Some(Amount::from_sat(fee_sats))
     }
 
@@ -254,7 +254,7 @@ impl TryFrom<Url> for BitcoinCoreClient {
             .ok_or(Error::InvalidUrl(url::ParseError::EmptyHost))?;
         let port = url.port().ok_or(Error::PortRequired)?;
 
-        let endpoint = format!("http://{}:{}", host, port);
+        let endpoint = format!("http://{}:{}",  host, port);
 
         Self::new(&endpoint, username, password)
     }
@@ -314,7 +314,7 @@ impl BitcoinCoreClient {
         let args = [
             serde_json::to_value(txid).map_err(Error::JsonSerialize)?,
             // This is the verbosity level. The acceptable values are 0, 1,
-            // and 2, and we want the 2 because it will include all of the
+            // and 2, and we want the 2 because it will include all the
             // required fields of the type.
             serde_json::Value::Number(serde_json::value::Number::from(2u32)),
             serde_json::to_value(block_hash).map_err(Error::JsonSerialize)?,
@@ -394,8 +394,8 @@ mod tests {
         }
     }
 
-    /// Return a transaction that is kinda like the signers' transaction
-    /// but it does not service any requests and it does not have any
+    /// Return a transaction that is kinda like the signers' transaction,
+    /// but it does not service any requests, and it does not have any
     /// signatures.
     fn base_signer_transaction() -> Transaction {
         Transaction {
@@ -476,8 +476,8 @@ mod tests {
 
     #[test]
     fn two_deposits_same_weight_split_the_fee() {
-        // These deposit inputs are essentially idential by weight. Since
-        // they are the only requests serviced by this trasnaction, they
+        // These deposit inputs are essentially identical by weight. Since
+        // they are the only requests serviced by this transaction, they
         // will have equal weight.
         let deposit_outpoint1 = OutPoint::new(Txid::from_byte_array([1; 32]), 0);
         let deposit_outpoint2 = OutPoint::new(Txid::from_byte_array([2; 32]), 0);

From 20078775a53f5f107bda2eed7d02c1764dbe553e Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Tue, 24 Sep 2024 00:35:59 -0400
Subject: [PATCH 14/15] Use the scheme given in the bitcoin-core client url

---
 sbtc/src/rpc.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index 5a973a97..b6a55b3e 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -254,7 +254,7 @@ impl TryFrom<Url> for BitcoinCoreClient {
             .ok_or(Error::InvalidUrl(url::ParseError::EmptyHost))?;
         let port = url.port().ok_or(Error::PortRequired)?;
 
-        let endpoint = format!("http://{}:{}",  host, port);
+        let endpoint = format!("{}://{host}:{port}", url.scheme());
 
         Self::new(&endpoint, username, password)
     }

From e7d8dd96e2345ae0af01cb96c9c6a25a4d035828 Mon Sep 17 00:00:00 2001
From: djordon <dan.jordon@gmail.com>
Date: Tue, 24 Sep 2024 12:20:59 -0400
Subject: [PATCH 15/15] Update comments and add more test cases.

---
 sbtc/src/rpc.rs | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/sbtc/src/rpc.rs b/sbtc/src/rpc.rs
index b6a55b3e..63dafdb6 100644
--- a/sbtc/src/rpc.rs
+++ b/sbtc/src/rpc.rs
@@ -124,13 +124,15 @@ impl BitcoinTxInfo {
     /// # Notes
     ///
     /// Each input and output is assessed a fee that is proportional to
-    /// their weight amount all the requests serviced by this
-    /// transaction.
+    /// their weight amount all the requests serviced by this transaction.
     ///
     /// This function assumes that this transaction is an sBTC transaction,
     /// which implies that the first input and the first two outputs are
     /// always the signers'. So `None` is returned if there is no input,
     /// after the first input, with the given `outpoint`.
+    ///
+    /// The logic for the fee assessment is from
+    /// <https://github.com/stacks-network/sbtc/issues/182>.
     pub fn assess_input_fee(&self, outpoint: OutPoint) -> Option<Amount> {
         // The Weight::to_wu function just returns the inner weight units
         // as an u64, so this is really just the weight.
@@ -146,6 +148,8 @@ impl BitcoinTxInfo {
             .segwit_weight()
             .to_wu();
 
+        // This computation follows the logic laid out in
+        // <https://github.com/stacks-network/sbtc/issues/182>.
         let fee_sats = (input_weight * self.fee.to_sat()).div_ceil(request_weight);
         Some(Amount::from_sat(fee_sats))
     }
@@ -156,13 +160,15 @@ impl BitcoinTxInfo {
     /// # Notes
     ///
     /// Each input and output is assessed a fee that is proportional to
-    /// their weight amount all the requests serviced by this
-    /// transaction.
+    /// their weight amount all the requests serviced by this transaction.
     ///
     /// This function assumes that this transaction is an sBTC transaction,
     /// which implies that the first input and the first two outputs are
     /// always the signers'. So `None` is returned if the given `vout` is 0
     /// or 1 or if there is no output in the transaction at `vout`.
+    ///
+    /// The logic for the fee assessment is from
+    /// <https://github.com/stacks-network/sbtc/issues/182>.
     pub fn assess_output_fee(&self, vout: usize) -> Option<Amount> {
         // We skip the first input because that is always the signers'
         // input UTXO.
@@ -172,6 +178,8 @@ impl BitcoinTxInfo {
         let request_weight = self.request_weight().to_wu();
         let input_weight = self.tx.output.get(vout)?.weight().to_wu();
 
+        // This computation follows the logic laid out in
+        // <https://github.com/stacks-network/sbtc/issues/182>.
         let fee_sats = (input_weight * self.fee.to_sat()).div_ceil(request_weight);
         Some(Amount::from_sat(fee_sats))
     }
@@ -179,9 +187,8 @@ impl BitcoinTxInfo {
     /// Computes the total weight of the inputs and the outputs, excluding
     /// the ones related to the signers.
     fn request_weight(&self) -> Weight {
-        // We skip the first input and output because those are always the
-        // signers' UTXO input and output. We skip the second output
-        // because that is always the OP_RETURN output for sBTC data.
+        // We skip the first input and first two outputs because those are
+        // always the signers' UTXO input and outputs.
         self.tx
             .input
             .iter()
@@ -373,6 +380,8 @@ mod tests {
     use bitcoin::hashes::Hash as _;
     use bitcoin::ScriptBuf;
 
+    use test_case::test_case;
+
     use super::*;
 
     impl BitcoinTxInfo {
@@ -470,7 +479,10 @@ mod tests {
         let tx_info = BitcoinTxInfo::from_tx(tx, fee);
         assert!(tx_info.assess_output_fee(0).is_none());
         assert!(tx_info.assess_output_fee(1).is_none());
-
+        // Since we always skip the first input, and
+        // `base_signer_transaction()` only adds one input, the search for
+        // the given input when `assess_input_fee` executes will always
+        // fail, simulating that the specified outpoint wasn't found.
         assert!(tx_info.assess_input_fee(OutPoint::null()).is_none());
     }
 
@@ -528,8 +540,11 @@ mod tests {
         assert_eq!(assessed_fee2, fee / 2);
     }
 
-    #[test]
-    fn one_deposit_two_withdrawals_fees_add() {
+    #[test_case(500_000; "fee 500_000")]
+    #[test_case(123_456; "fee 123_456")]
+    #[test_case(1_234_567; "fee 1_234_567")]
+    #[test_case(10_007; "fee 10_007")]
+    fn one_deposit_two_withdrawals_fees_add(fee_sats: u64) {
         // We're just testing that a "regular" bitcoin transaction,
         // servicing a deposit and two withdrawals, will assess the fees in
         // a normal way. Here we test that the fee is
@@ -552,7 +567,7 @@ mod tests {
         tx.output.push(withdrawal.clone());
         tx.output.push(withdrawal);
 
-        let fee = Amount::from_sat(500_000);
+        let fee = Amount::from_sat(fee_sats);
 
         let tx_info = BitcoinTxInfo::from_tx(tx, fee);
         let input_assessed_fee = tx_info.assess_input_fee(deposit_outpoint).unwrap();