Handle max_inbound_peers and max_outbound_peers properly (paritytech#27)

* Use API from rust-bitcoin instead of calling bitcoinconsensus directly

* Nits

* Introduce chain_params module

* Check whether utxo is spent in current block

* Introduce const MEDIAN_TIME_SPAN in chain_params

* Use chain_params.csv_height

* Handle max_outbound_peers properly

* Handle max_inbound_peers properly

* Add script_flag_exceptions

* Fix tests

Cargo.toml

@@ -30,7 +30,7 @@ default-members = ["crates/subcoin-node"]
 ansi_term = "0.12.1"
 async-trait = "0.1"
 bitcoin = { git = "https://github.com/liuchengxu/rust-bitcoin", branch = "0.32.x-subcoin", default-features = false }
-bitcoinconsensus = "0.106.0"
+bitcoinconsensus = "0.105.0+25.1"
 bitcoin-explorer = { git = "https://github.com/liuchengxu/Rusty-Bitcoin-Explorer", branch = "rust-bitcoin-upgrade", default-features = false }
 chrono = "0.4.37"
 clap = { version = "4", features = ["derive"] }

crates/sc-consensus-nakamoto/Cargo.toml

@@ -8,7 +8,7 @@ license.workspace = true
 
 [dependencies]
 async-trait = { workspace = true }
-bitcoin = { workspace = true }
+bitcoin = { workspace = true, features = ["bitcoinconsensus"] }
 bitcoinconsensus = { workspace = true }
 clap = { workspace = true, optional = true }
 codec = { workspace = true }

crates/sc-consensus-nakamoto/src/chain_params.rs

@@ -0,0 +1,76 @@
+use bitcoin::consensus::Params;
+use bitcoin::{BlockHash, Network};
+use std::collections::HashMap;
+
+pub const MEDIAN_TIME_SPAN: usize = 11;
+
+/// Extended [`Params`].
+#[derive(Debug, Clone)]
+pub struct ChainParams {
+    /// Chain params defined in rust-bitcoin.
+    pub params: Params,
+    /// Block height at which CSV becomes active.
+    pub csv_height: u32,
+    /// Block height at which Segwit becomes active.
+    pub segwit_height: u32,
+    pub script_flag_exceptions: HashMap<BlockHash, u32>,
+}
+
+impl ChainParams {
+    /// Constructs a new instance of [`ChainParams`].
+    // https://github.com/bitcoin/bitcoin/blob/6f9db1ebcab4064065ccd787161bf2b87e03cc1f/src/kernel/chainparams.cpp#L71
+    pub fn new(network: Network) -> Self {
+        let params = Params::new(network);
+        match network {
+            Network::Bitcoin => Self {
+                params,
+                csv_height: 419328, // 000000000000000004a1b34462cb8aeebd5799177f7a29cf28f2d1961716b5b5
+                segwit_height: 481824, // 0000000000000000001c8018d9cb3b742ef25114f27563e3fc4a1902167f9893
+                script_flag_exceptions: [
+                    // BIP16 exception
+                    (
+                        "00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22",
+                        bitcoinconsensus::VERIFY_NONE,
+                    ),
+                    // Taproot exception
+                    (
+                        "0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad",
+                        bitcoinconsensus::VERIFY_P2SH | bitcoinconsensus::VERIFY_WITNESS,
+                    ),
+                ]
+                .into_iter()
+                .map(|(block_hash, flag)| {
+                    (block_hash.parse().expect("Hash must be valid; qed"), flag)
+                })
+                .collect(),
+            },
+            Network::Testnet => Self {
+                params,
+                csv_height: 770112, // 00000000025e930139bac5c6c31a403776da130831ab85be56578f3fa75369bb
+                segwit_height: 834624, // 00000000002b980fcd729daaa248fd9316a5200e9b367f4ff2c42453e84201ca
+                script_flag_exceptions: HashMap::from_iter([
+                    // BIP16 exception
+                    (
+                        "00000000dd30457c001f4095d208cc1296b0eed002427aa599874af7a432b105"
+                            .parse()
+                            .expect("Hash must be valid; qed"),
+                        bitcoinconsensus::VERIFY_NONE,
+                    ),
+                ]),
+            },
+            Network::Signet => Self {
+                params,
+                csv_height: 1,
+                segwit_height: 1,
+                script_flag_exceptions: Default::default(),
+            },
+            Network::Regtest => Self {
+                params,
+                csv_height: 1,    // Always active unless overridden
+                segwit_height: 0, // Always active unless overridden
+                script_flag_exceptions: Default::default(),
+            },
+            _ => unreachable!("Unknown Bitcoin Network"),
+        }
+    }
+}

crates/sc-consensus-nakamoto/src/lib.rs

@@ -1,5 +1,6 @@
 mod block_executor;
 mod block_import;
+mod chain_params;
 mod import_queue;
 mod verification;
 

crates/sc-consensus-nakamoto/src/verification.rs

@@ -1,15 +1,18 @@
 mod header_verify;
 mod tx_verify;
 
+use crate::chain_params::ChainParams;
 use bitcoin::blockdata::constants::MAX_BLOCK_SIGOPS_COST;
-use bitcoin::consensus::{Encodable, Params};
+use bitcoin::consensus::Encodable;
 use bitcoin::{
-    Amount, Block as BitcoinBlock, OutPoint, ScriptBuf, TxMerkleNode, TxOut, Txid, Weight,
+    Amount, Block as BitcoinBlock, BlockHash, OutPoint, ScriptBuf, TxMerkleNode, TxOut, Txid,
+    Weight,
 };
 use sc_client_api::{AuxStore, Backend, StorageProvider};
 use sp_blockchain::HeaderBackend;
 use sp_runtime::traits::Block as BlockT;
 use std::collections::{HashMap, HashSet};
+use std::ffi::c_uint;
 use std::marker::PhantomData;
 use std::sync::Arc;
 use subcoin_primitives::runtime::{bitcoin_block_subsidy, Coin};
@@ -58,13 +61,20 @@ pub enum Error {
     TransactionNotFinal,
     #[error("Block contains duplicate transaction at index {0}")]
     DuplicateTransaction(usize),
-    /// Referenced output does not exist or has already been spent.
-    #[error("UTXO spent in #{block_number}:{txid} not found: {out_point:?}")]
+    /// Referenced output does not exist or was spent before.
+    #[error("UTXO not found (#{block_number}:{txid}: {out_point:?})")]
     UtxoNotFound {
         block_number: u32,
         txid: Txid,
         out_point: OutPoint,
     },
+    /// Referenced output has already been spent in this block.
+    #[error("UTXO already spent in current block (#{block_number}:{txid}: {out_point:?})")]
+    AlreadySpentInCurrentBlock {
+        block_number: u32,
+        txid: Txid,
+        out_point: OutPoint,
+    },
     #[error("Total output amount exceeds total input amount")]
     InsufficientFunds,
     // Invalid coinbase value.
@@ -75,8 +85,8 @@ pub enum Error {
     /// Block header error.
     #[error(transparent)]
     Header(#[from] HeaderError),
-    #[error("Script verification: {0}")]
-    Script(#[from] bitcoinconsensus::Error),
+    #[error(transparent)]
+    BitcoinConsensus(#[from] bitcoin::consensus::validation::BitcoinconsensusError),
     #[error("Bitcoin codec: {0:?}")]
     BitcoinCodec(bitcoin::io::Error),
     /// An error occurred in the client.
@@ -88,6 +98,7 @@ pub enum Error {
 #[derive(Clone)]
 pub struct BlockVerifier<Block, Client, BE> {
     client: Arc<Client>,
+    chain_params: ChainParams,
     header_verifier: HeaderVerifier<Block, Client>,
     block_verification: BlockVerification,
     coin_storage_key: Arc<dyn CoinStorageKey>,
@@ -102,10 +113,11 @@ impl<Block, Client, BE> BlockVerifier<Block, Client, BE> {
         block_verification: BlockVerification,
         coin_storage_key: Arc<dyn CoinStorageKey>,
     ) -> Self {
-        let consensus_params = Params::new(network);
-        let header_verifier = HeaderVerifier::new(client.clone(), consensus_params);
+        let chain_params = ChainParams::new(network);
+        let header_verifier = HeaderVerifier::new(client.clone(), chain_params.clone());
         Self {
             client,
+            chain_params,
             header_verifier,
             block_verification,
             coin_storage_key,
@@ -246,7 +258,10 @@ where
                 .expect("Txid must exist as initialized in `check_block_sanity()`; qed")
         };
 
+        let flags = get_block_script_flags(block_number, block.block_hash(), &self.chain_params);
+
         let mut block_fee = 0;
+        let mut spent_utxos = HashSet::new();
 
         // Buffer for encoded transaction data.
         let mut tx_data = Vec::<u8>::new();
@@ -272,6 +287,14 @@ where
             for (input_index, input) in tx.input.iter().enumerate() {
                 let out_point = input.previous_output;
 
+                if spent_utxos.contains(&out_point) {
+                    return Err(Error::AlreadySpentInCurrentBlock {
+                        block_number,
+                        txid: get_txid(tx_index),
+                        out_point,
+                    });
+                }
+
                 let spent_output = match self.find_utxo_in_state(parent_hash, out_point) {
                     Some(coin) => TxOut {
                         value: Amount::from_sat(coin.amount),
@@ -285,20 +308,15 @@ where
                         })?,
                 };
 
-                let script_verify_result = bitcoinconsensus::verify(
-                    spent_output.script_pubkey.as_bytes(),
-                    spent_output.value.to_sat(),
-                    spending_transaction,
-                    None,
+                bitcoin::consensus::validation::verify_script_with_flags(
+                    &spent_output.script_pubkey,
                     input_index,
-                );
-
-                if let Err(err) = script_verify_result {
-                    if err != bitcoinconsensus::Error::ERR_SCRIPT {
-                        return Err(err)?;
-                    }
-                }
+                    spent_output.value,
+                    spending_transaction,
+                    flags,
+                )?;
 
+                spent_utxos.insert(out_point);
                 total_input_value += spent_output.value.to_sat();
             }
 
@@ -373,17 +391,45 @@ fn find_utxo_in_current_block(
         .cloned()
 }
 
+/// Returns the script validation flags for the specified block.
+fn get_block_script_flags(
+    height: u32,
+    block_hash: BlockHash,
+    chain_params: &ChainParams,
+) -> c_uint {
+    if let Some(flag) = chain_params
+        .script_flag_exceptions
+        .get(&block_hash)
+        .copied()
+    {
+        return flag;
+    }
+
+    let mut flags = bitcoinconsensus::VERIFY_P2SH | bitcoinconsensus::VERIFY_WITNESS;
+
+    if height >= chain_params.params.bip65_height {
+        flags |= bitcoinconsensus::VERIFY_CHECKLOCKTIMEVERIFY;
+    }
+
+    if height >= chain_params.params.bip66_height {
+        flags |= bitcoinconsensus::VERIFY_DERSIG;
+    }
+
+    if height >= chain_params.csv_height {
+        flags |= bitcoinconsensus::VERIFY_CHECKSEQUENCEVERIFY;
+    }
+
+    if height >= chain_params.segwit_height {
+        flags |= bitcoinconsensus::VERIFY_NULLDUMMY;
+    }
+
+    flags
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
-    use bitcoin::consensus::Decodable;
-    use bitcoin::hex::FromHex;
-
-    fn decode_raw_block(hex_str: &str) -> BitcoinBlock {
-        let data = Vec::<u8>::from_hex(hex_str).expect("Failed to convert hex str");
-        BitcoinBlock::consensus_decode(&mut data.as_slice())
-            .expect("Failed to convert hex data to Block")
-    }
+    use bitcoin::consensus::encode::deserialize_hex;
 
     #[test]
     fn test_find_utxo_in_current_block() {
@@ -396,7 +442,7 @@ mod tests {
             .join("test_data")
             .join("btc_mainnet_385044.data");
         let raw_block = std::fs::read_to_string(test_block).unwrap();
-        let block = decode_raw_block(raw_block.trim());
+        let block = deserialize_hex::<BitcoinBlock>(raw_block.trim()).unwrap();
 
         let txids = block
             .txdata