From 19e6a7f0ae17cbf07751418d49944eeba4205f69 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sat, 20 Jul 2024 00:30:54 +0800 Subject: [PATCH] Verify script via bitcoinconsensus (#24) * Minor pure refactor * Replace tx_serialize_size_no_witness() with tx.base_size() This function is already equivalently implemented in rust-bitcoin. * Replace block_serialize_size_no_witness() with block.base_size() * Use block.weight() and tx.weight() * Separate out tx_verify module * Rename header_verifier.rs to header_verify.rs * Inline verify_transaction clsoure * Verify script using bitcoinconsensus * Fix difficulty bits check * Double the stall timeout when the chain is higher than 300000 --- Cargo.lock | 10 + Cargo.toml | 1 + crates/sc-consensus-nakamoto/Cargo.toml | 1 + .../sc-consensus-nakamoto/src/verification.rs | 290 +++++------------- .../{header_verifier.rs => header_verify.rs} | 94 ++++-- .../src/verification/tx_verify.rs | 106 +++++++ .../subcoin-network/src/block_downloader.rs | 12 +- .../src/block_downloader/headers_first.rs | 36 +-- 8 files changed, 295 insertions(+), 255 deletions(-) rename crates/sc-consensus-nakamoto/src/verification/{header_verifier.rs => header_verify.rs} (67%) create mode 100644 crates/sc-consensus-nakamoto/src/verification/tx_verify.rs diff --git a/Cargo.lock b/Cargo.lock index 0c7b88ea1626..215812a044f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -873,6 +873,15 @@ dependencies = [ "serde", ] +[[package]] +name = "bitcoinconsensus" +version = "0.106.0+26.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e12cba9cce5043cdda968e07b9df6d05ec6b0b38aa27a9a40bb575cf3e521ae9" +dependencies = [ + "cc", +] + [[package]] name = "bitflags" version = "1.3.2" @@ -6802,6 +6811,7 @@ version = "0.1.0" dependencies = [ "async-trait", "bitcoin 0.32.2", + "bitcoinconsensus", "clap", "futures", "hex-literal", diff --git a/Cargo.toml b/Cargo.toml index 90efb3f9b608..9f495e37bab4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +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" 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"] } diff --git a/crates/sc-consensus-nakamoto/Cargo.toml b/crates/sc-consensus-nakamoto/Cargo.toml index f5cab5a927ee..d8f05b093f7b 100644 --- a/crates/sc-consensus-nakamoto/Cargo.toml +++ b/crates/sc-consensus-nakamoto/Cargo.toml @@ -9,6 +9,7 @@ license.workspace = true [dependencies] async-trait = { workspace = true } bitcoin = { workspace = true } +bitcoinconsensus = { workspace = true } clap = { workspace = true, optional = true } codec = { workspace = true } futures = { workspace = true } diff --git a/crates/sc-consensus-nakamoto/src/verification.rs b/crates/sc-consensus-nakamoto/src/verification.rs index cd9f3fa48a99..bac7a070fa45 100644 --- a/crates/sc-consensus-nakamoto/src/verification.rs +++ b/crates/sc-consensus-nakamoto/src/verification.rs @@ -1,11 +1,10 @@ -mod header_verifier; +mod header_verify; +mod tx_verify; -use bitcoin::absolute::{LockTime, LOCK_TIME_THRESHOLD}; use bitcoin::blockdata::constants::MAX_BLOCK_SIGOPS_COST; -use bitcoin::blockdata::weight::WITNESS_SCALE_FACTOR; -use bitcoin::consensus::Params; +use bitcoin::consensus::{Encodable, Params}; use bitcoin::{ - Amount, Block as BitcoinBlock, OutPoint, Transaction, TxMerkleNode, Txid, VarInt, Weight, + Amount, Block as BitcoinBlock, OutPoint, ScriptBuf, TxMerkleNode, TxOut, Txid, Weight, }; use sc_client_api::{AuxStore, Backend, StorageProvider}; use sp_blockchain::HeaderBackend; @@ -15,16 +14,12 @@ use std::marker::PhantomData; use std::sync::Arc; use subcoin_primitives::runtime::{bitcoin_block_subsidy, Coin}; use subcoin_primitives::CoinStorageKey; +use tx_verify::{check_transaction_sanity, is_final}; -pub use header_verifier::{Error as HeaderError, HeaderVerifier}; +pub use header_verify::{Error as HeaderError, HeaderVerifier}; +pub use tx_verify::Error as TxError; -// MinCoinbaseScriptLen is the minimum length a coinbase script can be. -const MIN_COINBASE_SCRIPT_LEN: usize = 2; - -// MaxCoinbaseScriptLen is the maximum length a coinbase script can be. -const MAX_COINBASE_SCRIPT_LEN: usize = 100; - -const MAX_BLOCK_WEIGHT: usize = Weight::MAX_BLOCK.to_wu() as usize; +const MAX_BLOCK_WEIGHT: Weight = Weight::MAX_BLOCK; /// Represents the level of block verification. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -49,12 +44,6 @@ pub enum Error { EmptyTransactionList, #[error("Block is too large")] BadBlockLength, - #[error("Transaction has no inputs")] - EmptyInput, - #[error("Transaction has no outputs")] - EmptyOutput, - #[error("Transaction is too large")] - BadTransactionLength, #[error("First transaction is not coinbase")] FirstTransactionIsNotCoinbase, #[error("Block contains multiple coinbase transactions")] @@ -69,19 +58,6 @@ pub enum Error { TransactionNotFinal, #[error("Block contains duplicate transaction at index {0}")] DuplicateTransaction(usize), - #[error("Transaction contains duplicate inputs at index {0}")] - DuplicateTxInput(usize), - #[error("Output value (0) is too large")] - OutputValueTooLarge(Amount), - #[error("Total output value (0) is too large")] - TotalOutputValueTooLarge(Amount), - #[error( - "Coinbase transaction script length of {0} is out of range \ - (min: {MIN_COINBASE_SCRIPT_LEN}, max: {MAX_COINBASE_SCRIPT_LEN})" - )] - BadCoinbaseScriptSigLength(usize), - #[error("Transaction input refers to previous output that is null")] - BadTxInput, /// Referenced output does not exist or has already been spent. #[error("UTXO spent in #{block_number}:{txid} not found: {out_point:?}")] UtxoNotFound { @@ -94,9 +70,15 @@ pub enum Error { // Invalid coinbase value. #[error("Block reward is larger than the sum of block fee and subsidy")] InvalidBlockReward, + #[error(transparent)] + Transaction(#[from] TxError), /// Block header error. #[error(transparent)] Header(#[from] HeaderError), + #[error("Script verification: {0}")] + Script(#[from] bitcoinconsensus::Error), + #[error("Bitcoin codec: {0:?}")] + BitcoinCodec(bitcoin::io::Error), /// An error occurred in the client. #[error(transparent)] Client(#[from] sp_blockchain::Error), @@ -159,7 +141,7 @@ where Ok(()) } - /// Performs context-free preliminary checks. + /// Performs preliminary checks. /// /// - Transaction list must be non-empty. /// - Block size must not exceed `MAX_BLOCK_WEIGHT`. @@ -183,15 +165,12 @@ where } // Size limits. - let tx_count = block.txdata.len(); - - if tx_count * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT - || block_serialize_size_no_witness(block) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT - { + if block.weight() > MAX_BLOCK_WEIGHT { return Err(Error::BadBlockLength); } // Check duplicate transactions + let tx_count = block.txdata.len(); let mut seen_transactions = HashSet::with_capacity(tx_count); let mut txids = HashMap::with_capacity(tx_count); @@ -240,8 +219,6 @@ where return Err(Error::BadMerkleRoot); } - // Once segwit is active, we will still need to check for block mutability. - Ok(txids) } @@ -262,67 +239,82 @@ where "Parent block #{parent_number} not found" )))?; - // Verifies non-coinbase transaction. - let verify_transaction = |tx_index: usize, tx: &Transaction| -> Result { - let total_output_value = tx - .output - .iter() - .map(|output| output.value.to_sat()) - .sum::(); + let get_txid = |tx_index: usize| { + txids + .get(&tx_index) + .copied() + .expect("Txid must exist as initialized in `check_block_sanity()`; qed") + }; + + let mut block_fee = 0; + + // Buffer for encoded transaction data. + let mut tx_data = Vec::::new(); + + // TODO: verify transactions in parallel. + for (tx_index, tx) in transactions.iter().enumerate() { + if tx_index == 0 { + // Coinbase script was already checked in check_block_sanity(). + continue; + } + + if !is_final(tx, block_number, time) { + return Err(Error::TransactionNotFinal); + } + + tx_data.clear(); + tx.consensus_encode(&mut tx_data) + .map_err(Error::BitcoinCodec)?; + let spending_transaction = tx_data.as_slice(); let mut total_input_value = 0; - for input in &tx.input { + for (input_index, input) in tx.input.iter().enumerate() { let out_point = input.previous_output; - let amount = match self.find_utxo_in_state(parent_hash, out_point) { - Some(coin) => coin.amount, - None => { - let get_txid = |tx_index: usize| { - txids.get(&tx_index).copied().expect( - "Txid must exist as initialized in `check_block_sanity()`; qed", - ) - }; - - find_utxo_in_current_block(block, out_point, tx_index, get_txid) - .ok_or_else(|| Error::UtxoNotFound { - 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), + script_pubkey: ScriptBuf::from_bytes(coin.script_pubkey), + }, + None => find_utxo_in_current_block(block, out_point, tx_index, get_txid) + .ok_or_else(|| Error::UtxoNotFound { + block_number, + txid: get_txid(tx_index), + out_point, + })?, }; - total_input_value += amount; + let script_verify_result = bitcoinconsensus::verify( + spent_output.script_pubkey.as_bytes(), + spent_output.value.to_sat(), + spending_transaction, + None, + input_index, + ); + + if let Err(err) = script_verify_result { + if err != bitcoinconsensus::Error::ERR_SCRIPT { + return Err(err)?; + } + } + + total_input_value += spent_output.value.to_sat(); } + let total_output_value = tx + .output + .iter() + .map(|output| output.value.to_sat()) + .sum::(); + // Total input value must be no less than total output value. // Tx fee is the difference between inputs and outputs. let tx_fee = total_input_value .checked_sub(total_output_value) .ok_or(Error::InsufficientFunds)?; - Ok(tx_fee) - }; - - let mut block_fee = 0; - - // TODO: verify transactions in parallel. - for (index, tx) in transactions.iter().enumerate() { - if index == 0 { - // Coinbase script was already checked in check_block_sanity(). - continue; - } - - if !is_final(tx, block_number, time) { - return Err(Error::TransactionNotFinal); - } - - let tx_fee = verify_transaction(index, tx)?; - block_fee += tx_fee; - - // TODO: Verify the script. } let coinbase_value = transactions[0] @@ -363,75 +355,13 @@ where } } -fn block_serialize_size_no_witness(block: &BitcoinBlock) -> usize { - let base_size = 80 + VarInt(block.txdata.len() as u64).size(); - let tx_size: usize = block.txdata.iter().map(tx_serialize_size_no_witness).sum(); - base_size + tx_size -} - -/// Returns the serialized transaction size without witness. -/// -/// Credit: https://github.com/jrawsthorne/rust-bitcoin-node/blob/d84e4a63c4ae4d6818ab22bd0c25531d367961be/src/primitives/tx.rs#L287 -fn tx_serialize_size_no_witness(tx: &Transaction) -> usize { - // OutPoint (32+4) - const OUTPOINT_SIZE: usize = 32 + 4; - // Sequence (4) - const SEQUENCE_SIZE: usize = 4; - - let input_size: usize = tx - .input - .iter() - .map(|txin| { - let script_sig_len = txin.script_sig.len(); - OUTPOINT_SIZE + SEQUENCE_SIZE + VarInt(script_sig_len as u64).size() + script_sig_len - }) - .sum(); - - // Amount (8) - const VALUE_SIZE: usize = 8; - let output_size: usize = tx - .output - .iter() - .map(|txout| { - let script_pubkey_len = txout.script_pubkey.len(); - VALUE_SIZE + VarInt(script_pubkey_len as u64).size() + script_pubkey_len - }) - .sum(); - - const VERSION_SIZE: usize = 4; - const LOCK_TIME_SIZE: usize = 4; - - VERSION_SIZE - + LOCK_TIME_SIZE - + VarInt(tx.input.len() as u64).size() + input_size // Vec - + VarInt(tx.output.len() as u64).size() + output_size // Vec -} - -fn is_final(tx: &Transaction, height: u32, block_time: u32) -> bool { - if tx.lock_time == LockTime::ZERO { - return true; - } - - let lock_time = if tx.lock_time.to_consensus_u32() < LOCK_TIME_THRESHOLD { - height - } else { - block_time - }; - - if tx.lock_time.to_consensus_u32() < lock_time { - return true; - } - - tx.input.iter().all(|txin| txin.sequence.is_final()) -} - // Find a UTXO from the previous transactions in current block. fn find_utxo_in_current_block( block: &BitcoinBlock, out_point: OutPoint, tx_index: usize, get_txid: impl Fn(usize) -> Txid, -) -> Option { +) -> Option { let OutPoint { txid, vout } = out_point; block .txdata @@ -439,65 +369,8 @@ fn find_utxo_in_current_block( .take(tx_index) .enumerate() .find_map(|(index, tx)| (get_txid(index) == txid).then_some(tx)) - .and_then(|tx| { - tx.output - .get(vout as usize) - .map(|txout| txout.value.to_sat()) - }) -} - -fn check_transaction_sanity(tx: &Transaction) -> Result<(), Error> { - if tx.input.is_empty() { - return Err(Error::EmptyInput); - } - - if tx.output.is_empty() { - return Err(Error::EmptyOutput); - } - - if tx_serialize_size_no_witness(tx) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT { - return Err(Error::BadTransactionLength); - } - - // Check for duplicate inputs. - let mut seen_inputs = HashSet::new(); - for (index, txin) in tx.input.iter().enumerate() { - if !seen_inputs.insert(txin.previous_output) { - return Err(Error::DuplicateTxInput(index)); - } - } - - let mut total_output_value = Amount::ZERO; - tx.output.iter().try_for_each(|txout| { - if txout.value > Amount::MAX_MONEY { - return Err(Error::OutputValueTooLarge(txout.value)); - } - - total_output_value += txout.value; - - if total_output_value > Amount::MAX_MONEY { - return Err(Error::TotalOutputValueTooLarge(total_output_value)); - } - - Ok(()) - })?; - - // Coinbase script length must be between min and max length. - if tx.is_coinbase() { - let script_sig_len = tx.input[0].script_sig.len(); - - if !(MIN_COINBASE_SCRIPT_LEN..=MAX_COINBASE_SCRIPT_LEN).contains(&script_sig_len) { - return Err(Error::BadCoinbaseScriptSigLength(script_sig_len)); - } - } else { - // Previous transaction outputs referenced by the inputs to this - // transaction must not be null. - if tx.input.iter().any(|txin| txin.previous_output.is_null()) { - return Err(Error::BadTxInput); - } - } - - Ok(()) + .and_then(|tx| tx.output.get(vout as usize)) + .cloned() } #[cfg(test)] @@ -546,7 +419,8 @@ mod tests { find_utxo_in_current_block(&block, out_point, 36, |index| txids .get(&index) .copied() - .unwrap()), + .unwrap()) + .map(|txout| txout.value.to_sat()), Some(295600000) ); } diff --git a/crates/sc-consensus-nakamoto/src/verification/header_verifier.rs b/crates/sc-consensus-nakamoto/src/verification/header_verify.rs similarity index 67% rename from crates/sc-consensus-nakamoto/src/verification/header_verifier.rs rename to crates/sc-consensus-nakamoto/src/verification/header_verify.rs index 165c79eb0bdd..02b6e11ccf5e 100644 --- a/crates/sc-consensus-nakamoto/src/verification/header_verifier.rs +++ b/crates/sc-consensus-nakamoto/src/verification/header_verify.rs @@ -14,20 +14,21 @@ use subcoin_primitives::BackendExt; // 2 hours const MAX_FUTURE_BLOCK_TIME: u32 = 2 * 60 * 60; -/// Block header verification error. +/// Block header error. #[derive(Debug, thiserror::Error)] pub enum Error { - /// The block's timestamp is too far in the future. + /// Block's difficulty is invalid. + #[error("Incorrect proof-of-work: {{ got: {got:?}, expected: {expected:?} }}")] + BadDifficultyBits { got: Target, expected: Target }, + /// Block's proof-of-work is invalid. + #[error("proof-of-work validation failed: {0:?}")] + InvalidProofOfWork(ValidationError), + /// Block's timestamp is too far in the future. #[error("Block time is too far in the future")] TooFarInFuture, - #[error("Time is the median time of last 11 blocks or before ")] + /// Block's timestamp is too old. + #[error("Time is the median time of last 11 blocks or before")] TimeTooOld, - /// The block header is invalid. - #[error("proof-of-work validation failed: {0:?}")] - InvalidProofOfWork(ValidationError), - /// The block does not have enough proof-of-work. - #[error("Insufficient proof-of-work")] - NotEnoughPow, /// An error occurred in the client. #[error(transparent)] Client(#[from] sp_blockchain::Error), @@ -61,30 +62,38 @@ where /// transactions. /// /// - Check proof of work. - /// - Check the timestamp of the block is in the range: + /// - Check the timestamp of block. /// - Time is not greater than 2 hours from now. /// - Time is not the median time of last 11 blocks or before. + /// + /// https://github.com/bitcoin/bitcoin/blob/6f9db1ebcab4064065ccd787161bf2b87e03cc1f/src/validation.cpp#L4146 pub fn verify_header(&self, header: &BitcoinHeader) -> Result { - let last_block_header = self.client.block_header(header.prev_blockhash).ok_or( - sp_blockchain::Error::MissingHeader(header.prev_blockhash.to_string()), + let prev_block_hash = header.prev_blockhash; + + let prev_block_header = self.client.block_header(prev_block_hash).ok_or( + sp_blockchain::Error::MissingHeader(prev_block_hash.to_string()), )?; - let last_block_height = self + let prev_block_height = self .client - .block_number(last_block_header.block_hash()) - .expect("Parent block must exist as we checked before; qed"); + .block_number(prev_block_hash) + .expect("Prev block must exist as we checked before; qed"); let expected_target = get_next_work_required( - last_block_height, - last_block_header, + prev_block_height, + prev_block_header, &self.consensus_params, &self.client, ); + let expected_bits = expected_target.to_compact_lossy().to_consensus(); let actual_target = header.target(); - if actual_target > expected_target { - return Err(Error::NotEnoughPow); + if actual_target.to_compact_lossy().to_consensus() != expected_bits { + return Err(Error::BadDifficultyBits { + got: actual_target, + expected: expected_target, + }); } header @@ -101,7 +110,7 @@ where return Err(Error::TooFarInFuture); } - let block_number = last_block_height + 1; + let block_number = prev_block_height + 1; // TODO: check deployment state properly. const MAINNET_CSV_HEIGHT: u32 = 419328; @@ -159,12 +168,16 @@ where /// it will be calculated from the last 2016 blocks (about two weeks for Bitcoin mainnet). /// /// https://github.com/bitcoin/bitcoin/blob/89b910711c004c21b7d67baa888073742f7f94f0/src/pow.cpp#L13 -fn get_next_work_required + AuxStore>( +fn get_next_work_required( last_block_height: u32, last_block: BitcoinHeader, consensus_params: &Params, client: &Arc, -) -> Target { +) -> Target +where + Block: BlockT, + Client: HeaderBackend + AuxStore, +{ if consensus_params.no_pow_retargeting { return last_block.target(); } @@ -173,6 +186,7 @@ fn get_next_work_required + AuxStore let difficulty_adjustment_interval = consensus_params.difficulty_adjustment_interval() as u32; + // Only change once per difficulty adjustment interval. if height >= difficulty_adjustment_interval && height % difficulty_adjustment_interval == 0 { let last_retarget_height = height - difficulty_adjustment_interval; @@ -190,7 +204,7 @@ fn get_next_work_required + AuxStore let last_block_time = last_block.time; calculate_next_work_required( - retarget_header.target().0, + last_block.target().0, first_block_time.into(), last_block_time.into(), consensus_params, @@ -232,3 +246,37 @@ fn calculate_next_work_required( target } } + +#[cfg(test)] +mod tests { + use super::*; + use bitcoin::consensus::encode::deserialize_hex; + + #[test] + fn test_calculate_next_work_required() { + // block_354816 + let block_354816: BitcoinHeader = deserialize_hex + ("020000003f99814a36d2a2043b1d4bf61a410f71828eca1decbf56000000000000000000b3762ed278ac44bb953e24262cfeb952d0abe6d3b7f8b74fd24e009b96b6cb965d674655dd1317186436e79d").unwrap(); + + let expected_target = block_354816.target(); + + // block_352800, first block in this period. + let first_block: BitcoinHeader = deserialize_hex("0200000074c51c1cc53aaf478c643bb612da6bd17b268cd9bdccc4000000000000000000ccc0a2618a1f973dfac37827435b463abd18cbfd0f280a90432d3d78497a36cc02f33355f0171718b72a1dc7").unwrap(); + + // block_354815, last block in this period. + let last_block: BitcoinHeader = deserialize_hex("030000004c9c1b59250f30b8d360886a5433501120b056a000bdc0160000000000000000caca1bf0c55a5ba2299f9e60d10c01c679bb266c7df815ff776a1b97fd3a199ac1644655f01717182707bd59").unwrap(); + + let new_target = calculate_next_work_required( + last_block.target().0, + first_block.time as u64, + last_block.time as u64, + &Params::new(bitcoin::Network::Bitcoin), + ); + + assert_eq!( + new_target.to_compact_lossy().to_consensus(), + expected_target.to_compact_lossy().to_consensus(), + "Difficulty bits must match" + ); + } +} diff --git a/crates/sc-consensus-nakamoto/src/verification/tx_verify.rs b/crates/sc-consensus-nakamoto/src/verification/tx_verify.rs new file mode 100644 index 000000000000..bc7391ad1931 --- /dev/null +++ b/crates/sc-consensus-nakamoto/src/verification/tx_verify.rs @@ -0,0 +1,106 @@ +use super::MAX_BLOCK_WEIGHT; +use bitcoin::absolute::{LockTime, LOCK_TIME_THRESHOLD}; +use bitcoin::{Amount, Transaction}; +use std::collections::HashSet; + +// MinCoinbaseScriptLen is the minimum length a coinbase script can be. +const MIN_COINBASE_SCRIPT_LEN: usize = 2; + +// MaxCoinbaseScriptLen is the maximum length a coinbase script can be. +const MAX_COINBASE_SCRIPT_LEN: usize = 100; + +/// Transaction verification error. +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Transaction has no inputs")] + EmptyInput, + #[error("Transaction has no outputs")] + EmptyOutput, + #[error("Transaction is too large")] + BadTransactionLength, + #[error("Transaction contains duplicate inputs at index {0}")] + DuplicateTxInput(usize), + #[error("Output value (0) is too large")] + OutputValueTooLarge(Amount), + #[error("Total output value (0) is too large")] + TotalOutputValueTooLarge(Amount), + #[error( + "Coinbase transaction script length of {0} is out of range \ + (min: {MIN_COINBASE_SCRIPT_LEN}, max: {MAX_COINBASE_SCRIPT_LEN})" + )] + BadCoinbaseScriptSigLength(usize), + #[error("Transaction input refers to previous output that is null")] + BadTxInput, +} + +pub fn is_final(tx: &Transaction, height: u32, block_time: u32) -> bool { + if tx.lock_time == LockTime::ZERO { + return true; + } + + let lock_time = if tx.lock_time.to_consensus_u32() < LOCK_TIME_THRESHOLD { + height + } else { + block_time + }; + + if tx.lock_time.to_consensus_u32() < lock_time { + return true; + } + + tx.input.iter().all(|txin| txin.sequence.is_final()) +} + +pub fn check_transaction_sanity(tx: &Transaction) -> Result<(), Error> { + if tx.input.is_empty() { + return Err(Error::EmptyInput); + } + + if tx.output.is_empty() { + return Err(Error::EmptyOutput); + } + + if tx.weight() > MAX_BLOCK_WEIGHT { + return Err(Error::BadTransactionLength); + } + + // Check for duplicate inputs. + let mut seen_inputs = HashSet::new(); + for (index, txin) in tx.input.iter().enumerate() { + if !seen_inputs.insert(txin.previous_output) { + return Err(Error::DuplicateTxInput(index)); + } + } + + let mut total_output_value = Amount::ZERO; + tx.output.iter().try_for_each(|txout| { + if txout.value > Amount::MAX_MONEY { + return Err(Error::OutputValueTooLarge(txout.value)); + } + + total_output_value += txout.value; + + if total_output_value > Amount::MAX_MONEY { + return Err(Error::TotalOutputValueTooLarge(total_output_value)); + } + + Ok(()) + })?; + + // Coinbase script length must be between min and max length. + if tx.is_coinbase() { + let script_sig_len = tx.input[0].script_sig.len(); + + if !(MIN_COINBASE_SCRIPT_LEN..=MAX_COINBASE_SCRIPT_LEN).contains(&script_sig_len) { + return Err(Error::BadCoinbaseScriptSigLength(script_sig_len)); + } + } else { + // Previous transaction outputs referenced by the inputs to this + // transaction must not be null. + if tx.input.iter().any(|txin| txin.previous_output.is_null()) { + return Err(Error::BadTxInput); + } + } + + Ok(()) +} diff --git a/crates/subcoin-network/src/block_downloader.rs b/crates/subcoin-network/src/block_downloader.rs index 1db5e5d8bc79..3ede91f32f1c 100644 --- a/crates/subcoin-network/src/block_downloader.rs +++ b/crates/subcoin-network/src/block_downloader.rs @@ -83,9 +83,6 @@ pub(crate) struct BlockDownloadManager { } impl BlockDownloadManager { - /// The downloader is considered as stalled if no progress in 60 seconds. - const STALL_TIMEOUT: u64 = 60; - fn new() -> Self { Self { requested_blocks: HashSet::new(), @@ -101,7 +98,14 @@ impl BlockDownloadManager { } fn is_stalled(&self) -> bool { - self.last_progress_time.elapsed().as_secs() > Self::STALL_TIMEOUT + // The downloader is considered as stalled if no progress for some time. + let stall_timeout = if self.best_queued_number > 300_000 { + 120 + } else { + 60 + }; + + self.last_progress_time.elapsed().as_secs() > stall_timeout } fn block_exists(&self, block_hash: BlockHash) -> bool { diff --git a/crates/subcoin-network/src/block_downloader/headers_first.rs b/crates/subcoin-network/src/block_downloader/headers_first.rs index 5af72f846c46..524aff16af77 100644 --- a/crates/subcoin-network/src/block_downloader/headers_first.rs +++ b/crates/subcoin-network/src/block_downloader/headers_first.rs @@ -75,11 +75,9 @@ impl Display for DownloadState { Self::Restarting => write!(f, "Restarting"), Self::Disconnecting => write!(f, "Disconnecting"), Self::DownloadingHeaders { start, end } => { - write!(f, "DownloadingHeaders {{ start: {}, end: {} }}", start, end) - } - Self::DownloadingBlocks(_range) => { - write!(f, "DownloadingBlocks") + write!(f, "DownloadingHeaders {{ start: {start}, end: {end} }}") } + Self::DownloadingBlocks(_range) => write!(f, "DownloadingBlocks"), Self::Completed => write!(f, "Completed"), } } @@ -440,32 +438,32 @@ where } pub(crate) fn on_block(&mut self, block: BitcoinBlock, from: PeerId) -> SyncAction { + let block_hash = block.block_hash(); + let block_download_range = match &mut self.download_state { DownloadState::DownloadingBlocks(download_range) => download_range, state => { + // TODO: we may receive the blocks from a peer that has been considered as stalled, + // should we try to cache and use such blocks since the bandwidth has been consumed + // already? tracing::warn!( ?state, ?from, current_sync_peer = ?self.peer_id, - "Not in the block download mode, dropping block {}", - block.block_hash() + "Not in the block download mode, dropping block {block_hash}", ); return SyncAction::None; } }; - let block_hash = block.block_hash(); - let receive_requested_block = self.download_manager.on_block_response(block_hash); let parent_block_hash = block.header.prev_blockhash; - let maybe_parent = - if let Some(number) = self.download_manager.block_number(parent_block_hash) { - Some(number) - } else { - self.client.block_number(parent_block_hash) - }; + let maybe_parent = self + .download_manager + .block_number(parent_block_hash) + .or_else(|| self.client.block_number(parent_block_hash)); if let Some(parent_block_number) = maybe_parent { let block_number = parent_block_number + 1; @@ -601,12 +599,10 @@ fn prepare_ordered_block_data_request( let mut blocks = blocks .into_iter() .map(|block_hash| { - ( - downloaded_headers.get(&block_hash).expect( - "Header of the block to download must exist in headers-first sync; qed", - ), - block_hash, - ) + let block_number = downloaded_headers + .get(&block_hash) + .expect("Header must exist before downloading blocks in headers-first sync; qed"); + (block_number, block_hash) }) .collect::>();