feat: add bitcoin transaction fee validation (#582)

* Use from instead of as _ casts for integers more
* Use the crate ScriptPubKey in EncryptedDkgShares script_pubkey field
* Switch the config for bitcoin core to use the regtest variables
* Add a new trait function to DbRead for checking whether a scriptPubKey is related to the signers
* Update the function signature on BitcoinTxInfo. Also update a BitcoinTxInfo's subfield and comment
* Fix a lurking bug in a query that fetches the last years worth of scriptPubKeys. It now gets at least one scriptPubKey
* Update stacks validation functions to use bitcoin core
* Add test helper struct for setting up sweep transactions and making them (hopefully) easier to understand

protobufs/stacks/signer/v1/requests.proto

@@ -74,8 +74,8 @@ message AcceptWithdrawal {
   // The outpoint of the bitcoin UTXO that was spent to fulfill the
   // withdrawal request.
   bitcoin.OutPoint outpoint = 2;
-  // The fee that was spent to the bitcoin miner when fulfilling the
-  // withdrawal request.
+  // This is the assessed transaction fee for fulfilling the withdrawal
+  // request.
   uint64 tx_fee = 3;
   // A bitmap of how the signers voted. The length of the list must be less
   // than or equal to 128. Here, we assume that a true implies that the

sbtc/src/testing/regtest.rs

@@ -48,6 +48,11 @@ pub const BITCOIN_CORE_RPC_PASSWORD: &str = "devnet";
 /// The fallback fee in bitcoin core
 pub const BITCOIN_CORE_FALLBACK_FEE: Amount = Amount::from_sat(1000);
 
+/// The minimum height of the bitcoin blockchains. You need 100 blocks
+/// mined on top of a bitcoin block before you can spend the coinbase
+/// rewards.
+pub const MIN_BLOCKCHAIN_HEIGHT: u64 = 101;
+
 /// The name of our wallet on bitcoin-core
 const BITCOIN_CORE_WALLET_NAME: &str = "integration-tests-wallet";
 
@@ -85,7 +90,7 @@ pub fn initialize_blockchain() -> (&'static Client, &'static Faucet) {
         let amount = rpc.get_received_by_address(&faucet.address, None).unwrap();
 
         if amount < Amount::from_int_btc(1) {
-            faucet.generate_blocks(101);
+            faucet.generate_blocks(MIN_BLOCKCHAIN_HEIGHT);
         }
 
         faucet

signer/src/bitcoin/mod.rs

@@ -32,7 +32,7 @@ pub trait BitcoinInteract: Sync + Send {
         txid: &Txid,
     ) -> impl Future<Output = Result<Option<GetTxResponse>, Error>> + Send;
 
-    /// get tx info
+    /// Get a transaction with additional information about it.
     fn get_tx_info(
         &self,
         txid: &Txid,

signer/src/bitcoin/rpc.rs

@@ -131,8 +131,12 @@ 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.
+    ///
+    /// This field is omitted if block undo data is not available, so it is
+    /// missing whenever the `fee` field is missing in the
+    /// [`BitcoinTxInfo`].
+    pub prevout: BitcoinTxInfoVinPrevout,
 }
 
 /// The previous output, omitted if block undo data is not available.
@@ -240,10 +244,6 @@ impl BitcoinCoreClient {
 
         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)),
         }
@@ -275,7 +275,7 @@ impl BitcoinCoreClient {
             // 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
+            // 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)),

signer/src/bitcoin/utxo.rs

@@ -953,7 +953,7 @@ impl BitcoinTxInfo {
     ///
     /// 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> {
+    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.
         let request_weight = self.request_weight().to_wu();
@@ -964,7 +964,7 @@ impl BitcoinTxInfo {
             .input
             .iter()
             .skip(1)
-            .find(|tx_in| tx_in.previous_output == outpoint)?
+            .find(|tx_in| &tx_in.previous_output == outpoint)?
             .segwit_weight()
             .to_wu();
 
@@ -2267,7 +2267,7 @@ mod tests {
         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();
+        let assessed_fee = tx_info.assess_input_fee(&deposit_outpoint).unwrap();
         assert_eq!(assessed_fee, fee);
     }
 
@@ -2300,7 +2300,7 @@ mod tests {
         // `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());
+        assert!(tx_info.assess_input_fee(&OutPoint::null()).is_none());
     }
 
     #[test]
@@ -2330,10 +2330,10 @@ mod tests {
         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();
+        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();
+        let assessed_fee2 = tx_info.assess_input_fee(&deposit_outpoint2).unwrap();
         assert_eq!(assessed_fee2, fee / 2);
     }
 
@@ -2387,7 +2387,7 @@ mod tests {
         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();
+        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();
 

signer/src/block_observer.rs

@@ -359,7 +359,9 @@ mod tests {
     use blockstack_lib::types::chainstate::StacksAddress;
     use blockstack_lib::types::chainstate::StacksBlockId;
     use fake::Dummy;
+    use fake::Fake;
     use model::BitcoinTxId;
+    use model::ScriptPubKey;
     use rand::seq::IteratorRandom;
     use rand::SeedableRng;
 
@@ -626,7 +628,7 @@ mod tests {
         // 4. We try "extracting" a block with two transactions where one
         //    of them spends to the signers. The one transaction should be
         //    stored in our storage.
-        let signers_script_pubkey = vec![1, 2, 3, 4];
+        let signers_script_pubkey: ScriptPubKey = fake::Faker.fake_with_rng(&mut rng);
 
         // We start by storing our `scriptPubKey`.
         let storage = storage::in_memory::Store::new_shared();
@@ -655,7 +657,7 @@ mod tests {
         let mut tx_setup0 = sbtc::testing::deposits::tx_setup(0, 0, 100);
         tx_setup0.tx.output.push(TxOut {
             value: Amount::ONE_BTC,
-            script_pubkey: ScriptBuf::from_bytes(signers_script_pubkey.clone()),
+            script_pubkey: signers_script_pubkey.into(),
         });
 
         // This one does not spend to the signers :(

signer/src/config/default.toml

@@ -45,7 +45,7 @@ endpoints = [
 # Environment: SIGNER_BITCOIN__RPC_ENDPOINTS
 # Environment Example: http://user:pass@seed-1:4122,http://foo:bar@seed-2:4122
 rpc_endpoints = [
-    "http://user:pass@localhost:18443",
+    "http://devnet:devnet@localhost:18443",
 ]
 
 # The URI(s) of the Bitcoin Core ZMQ block hash stream(s) to connect to.

signer/src/config/mod.rs

@@ -410,10 +410,10 @@ mod tests {
 
         assert_eq!(
             settings.bitcoin.rpc_endpoints,
-            vec![url("http://user:pass@localhost:18443")]
+            vec![url("http://devnet:devnet@localhost:18443")]
         );
-        assert_eq!(settings.bitcoin.rpc_endpoints[0].username(), "user");
-        assert_eq!(settings.bitcoin.rpc_endpoints[0].password(), Some("pass"));
+        assert_eq!(settings.bitcoin.rpc_endpoints[0].username(), "devnet");
+        assert_eq!(settings.bitcoin.rpc_endpoints[0].password(), Some("devnet"));
         assert_eq!(
             settings.signer.event_observer.bind,
             "0.0.0.0:8801".parse::<SocketAddr>().unwrap()

signer/src/context/mod.rs

@@ -69,7 +69,7 @@ pub struct SignerContext<S, BC, ST> {
 impl<S, BC, ST> SignerContext<S, BC, ST>
 where
     S: DbRead + DbWrite + Clone + Sync + Send,
-    BC: for<'a> TryFrom<&'a [Url]> + BitcoinInteract + Clone + Sync + Send + 'static,
+    BC: for<'a> TryFrom<&'a [Url]> + BitcoinInteract + Clone + 'static,
     ST: for<'a> TryFrom<&'a Settings> + StacksInteract + Clone + Sync + Send + 'static,
     Error: for<'a> From<<BC as TryFrom<&'a [Url]>>::Error>,
     Error: for<'a> From<<ST as TryFrom<&'a Settings>>::Error>,
@@ -87,7 +87,7 @@ where
 impl<S, BC, ST> SignerContext<S, BC, ST>
 where
     S: DbRead + DbWrite + Clone + Sync + Send,
-    BC: BitcoinInteract + Clone + Sync + Send,
+    BC: BitcoinInteract + Clone,
     ST: StacksInteract + Clone + Sync + Send,
 {
     /// Create a new signer context.
@@ -112,7 +112,7 @@ where
 impl<S, BC, ST> Context for SignerContext<S, BC, ST>
 where
     S: DbRead + DbWrite + Clone + Sync + Send,
-    BC: BitcoinInteract + Clone + Sync + Send,
+    BC: BitcoinInteract + Clone,
     ST: StacksInteract + Clone + Sync + Send,
 {
     fn config(&self) -> &Settings {

signer/src/proto/generated/stacks.signer.v1.rs

@@ -161,8 +161,8 @@ pub struct AcceptWithdrawal {
     /// withdrawal request.
     #[prost(message, optional, tag = "2")]
     pub outpoint: ::core::option::Option<super::super::super::bitcoin::OutPoint>,
-    /// The fee that was spent to the bitcoin miner when fulfilling the
-    /// withdrawal request.
+    /// This is the assessed transaction fee for fulfilling the withdrawal
+    /// request.
     #[prost(uint64, tag = "3")]
     pub tx_fee: u64,
     /// A bitmap of how the signers voted. The length of the list must be less