From eb55b2ae84874af43b4fd8007cf8263624e3d006 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 16 Oct 2018 11:28:32 +0300 Subject: [PATCH] BCH Nov2018 HF: canonical transaction ordering --- miner/src/block_assembler.rs | 71 +++++++++++++++++-- network/src/consensus.rs | 25 +++++++ network/src/lib.rs | 2 +- verification/src/accept_block.rs | 95 ++++++++++++++++++++++++-- verification/src/accept_transaction.rs | 6 +- verification/src/chain_verifier.rs | 68 ++++++++++++++++-- verification/src/duplex_store.rs | 17 +++++ verification/src/error.rs | 2 + verification/src/work_bch.rs | 4 +- 9 files changed, 271 insertions(+), 19 deletions(-) diff --git a/miner/src/block_assembler.rs b/miner/src/block_assembler.rs index 0ee418dc..75c48208 100644 --- a/miner/src/block_assembler.rs +++ b/miner/src/block_assembler.rs @@ -3,9 +3,9 @@ use primitives::hash::H256; use primitives::compact::Compact; use chain::{OutPoint, TransactionOutput, IndexedTransaction}; use storage::{SharedStore, TransactionOutputProvider}; -use network::ConsensusParams; +use network::{ConsensusParams, TransactionOrdering}; use memory_pool::{MemoryPool, OrderingStrategy, Entry}; -use verification::{work_required, block_reward_satoshi, transaction_sigops}; +use verification::{work_required, block_reward_satoshi, transaction_sigops, median_timestamp_inclusive}; const BLOCK_VERSION: u32 = 0x20000000; const BLOCK_HEADER_SIZE: u32 = 4 + 32 + 32 + 4 + 4 + 4; @@ -116,7 +116,9 @@ impl SizePolicy { /// Block assembler pub struct BlockAssembler { + /// Maximal block size. pub max_block_size: u32, + /// Maximal # of sigops in the block. pub max_block_sigops: u32, } @@ -246,7 +248,13 @@ impl BlockAssembler { let mut transactions = Vec::new(); let mempool_iter = mempool.iter(OrderingStrategy::ByTransactionScore); - let tx_iter = FittingTransactionsIterator::new(store.as_transaction_output_provider(), mempool_iter, self.max_block_size, self.max_block_sigops, height, time); + let tx_iter = FittingTransactionsIterator::new( + store.as_transaction_output_provider(), + mempool_iter, + self.max_block_size, + self.max_block_sigops, + height, + time); for entry in tx_iter { // miner_fee is i64, but we can safely cast it to u64 // memory pool should restrict miner fee to be positive @@ -255,6 +263,15 @@ impl BlockAssembler { transactions.push(tx); } + // sort block transactions + let median_time_past = median_timestamp_inclusive(previous_header_hash.clone(), store.as_block_header_provider()); + match consensus.fork.transaction_ordering(median_time_past) { + TransactionOrdering::Canonical => transactions.sort_unstable_by(|tx1, tx2| + tx1.hash.cmp(&tx2.hash)), + // memory pool iter returns transactions in topological order + TransactionOrdering::Topological => (), + } + BlockTemplate { version: version, previous_header_hash: previous_header_hash, @@ -271,7 +288,16 @@ impl BlockAssembler { #[cfg(test)] mod tests { - use super::{SizePolicy, NextStep}; + extern crate test_data; + + use std::sync::Arc; + use db::BlockChainDatabase; + use primitives::hash::H256; + use storage::SharedStore; + use network::{ConsensusParams, ConsensusFork, Network, BitcoinCashConsensusParams}; + use memory_pool::MemoryPool; + use self::test_data::{ChainBuilder, TransactionBuilder}; + use super::{BlockAssembler, SizePolicy, NextStep, BlockTemplate}; #[test] fn test_size_policy() { @@ -317,4 +343,41 @@ mod tests { fn test_fitting_transactions_iterator_locked_transaction() { // TODO } + + #[test] + fn block_assembler_transaction_order() { + fn construct_block(consensus: ConsensusParams) -> (BlockTemplate, H256, H256) { + let chain = &mut ChainBuilder::new(); + TransactionBuilder::with_default_input(0).set_output(30).store(chain) // transaction0 + .into_input(0).set_output(50).store(chain); // transaction0 -> transaction1 + let hash0 = chain.at(0).hash(); + let hash1 = chain.at(1).hash(); + + let mut pool = MemoryPool::new(); + let storage: SharedStore = Arc::new(BlockChainDatabase::init_test_chain(vec![test_data::genesis().into()])); + pool.insert_verified(chain.at(0).into()); + pool.insert_verified(chain.at(1).into()); + + (BlockAssembler { + max_block_size: 0xffffffff, + max_block_sigops: 0xffffffff, + }.create_new_block(&storage, &pool, 0, &consensus), hash0, hash1) + } + + // when topological consensus is used + let topological_consensus = ConsensusParams::new(Network::Mainnet, ConsensusFork::BitcoinCore); + let (block, hash0, hash1) = construct_block(topological_consensus); + assert!(hash1 < hash0); + assert_eq!(block.transactions[0].hash, hash0); + assert_eq!(block.transactions[1].hash, hash1); + + // when canonocal consensus is used + let mut canonical_fork = BitcoinCashConsensusParams::new(Network::Mainnet); + canonical_fork.magnetic_anomaly_time = 0; + let canonical_consensus = ConsensusParams::new(Network::Mainnet, ConsensusFork::BitcoinCash(canonical_fork)); + let (block, hash0, hash1) = construct_block(canonical_consensus); + assert!(hash1 < hash0); + assert_eq!(block.transactions[0].hash, hash1); + assert_eq!(block.transactions[1].hash, hash0); + } } diff --git a/network/src/consensus.rs b/network/src/consensus.rs index bfc460ec..fc545ca0 100644 --- a/network/src/consensus.rs +++ b/network/src/consensus.rs @@ -41,6 +41,9 @@ pub struct BitcoinCashConsensusParams { /// Time of monolith (aka May 2018) hardfork. /// https://github.com/bitcoincashorg/spec/blob/4fbb0face661e293bcfafe1a2a4744dcca62e50d/may-2018-hardfork.md pub monolith_time: u32, + /// Time of magnetic anomaly (aka Nov 2018) hardfork. + /// https://github.com/bitcoincashorg/bitcoincash.org/blob/f92f5412f2ed60273c229f68dd8703b6d5d09617/spec/2018-nov-upgrade.md + pub magnetic_anomaly_time: u32, } #[derive(Debug, Clone)] @@ -57,6 +60,17 @@ pub enum ConsensusFork { BitcoinCash(BitcoinCashConsensusParams), } +#[derive(Debug, Clone, Copy)] +/// Describes the ordering of transactions within single block. +pub enum TransactionOrdering { + /// Topological tranasaction ordering: if tx TX2 depends on tx TX1, + /// it should come AFTER TX1 (not necessary **right** after it). + Topological, + /// Canonical transaction ordering: transactions are ordered by their + /// hash (in ascending order). + Canonical, +} + impl ConsensusParams { pub fn new(network: Network, fork: ConsensusFork) -> Self { match network { @@ -225,6 +239,14 @@ impl ConsensusFork { unreachable!("BitcoinCash has no SegWit; weight is only checked with SegWit activated; qed"), } } + + pub fn transaction_ordering(&self, median_time_past: u32) -> TransactionOrdering { + match *self { + ConsensusFork::BitcoinCash(ref fork) if median_time_past >= fork.magnetic_anomaly_time + => TransactionOrdering::Canonical, + _ => TransactionOrdering::Topological, + } + } } impl BitcoinCashConsensusParams { @@ -234,16 +256,19 @@ impl BitcoinCashConsensusParams { height: 478559, difficulty_adjustion_height: 504031, monolith_time: 1526400000, + magnetic_anomaly_time: 1542300000, }, Network::Testnet => BitcoinCashConsensusParams { height: 1155876, difficulty_adjustion_height: 1188697, monolith_time: 1526400000, + magnetic_anomaly_time: 1542300000, }, Network::Regtest | Network::Unitest => BitcoinCashConsensusParams { height: 0, difficulty_adjustion_height: 0, monolith_time: 1526400000, + magnetic_anomaly_time: 1542300000, }, } } diff --git a/network/src/lib.rs b/network/src/lib.rs index 40c47655..20f2ebbe 100644 --- a/network/src/lib.rs +++ b/network/src/lib.rs @@ -10,6 +10,6 @@ mod network; pub use primitives::{hash, compact}; -pub use consensus::{ConsensusParams, ConsensusFork, BitcoinCashConsensusParams}; +pub use consensus::{ConsensusParams, ConsensusFork, BitcoinCashConsensusParams, TransactionOrdering}; pub use deployments::Deployment; pub use network::{Magic, Network}; diff --git a/verification/src/accept_block.rs b/verification/src/accept_block.rs index d656b523..39e1085c 100644 --- a/verification/src/accept_block.rs +++ b/verification/src/accept_block.rs @@ -1,11 +1,11 @@ -use network::{ConsensusParams, ConsensusFork}; +use network::{ConsensusParams, ConsensusFork, TransactionOrdering}; use crypto::dhash256; use storage::{TransactionOutputProvider, BlockHeaderProvider}; use script; use ser::Stream; use sigops::{transaction_sigops, transaction_sigops_cost} ; use work::block_reward_satoshi; -use duplex_store::DuplexTransactionOutputProvider; +use duplex_store::{transaction_index_for_output_check, DuplexTransactionOutputProvider}; use deployments::BlockDeployments; use canon::CanonBlock; use error::{Error, TransactionError}; @@ -19,6 +19,7 @@ pub struct BlockAcceptor<'a> { pub coinbase_claim: BlockCoinbaseClaim<'a>, pub coinbase_script: BlockCoinbaseScript<'a>, pub witness: BlockWitness<'a>, + pub ordering: BlockTransactionOrdering<'a>, } impl<'a> BlockAcceptor<'a> { @@ -35,9 +36,10 @@ impl<'a> BlockAcceptor<'a> { finality: BlockFinality::new(block, height, deployments, headers), serialized_size: BlockSerializedSize::new(block, consensus, deployments, height, median_time_past), coinbase_script: BlockCoinbaseScript::new(block, consensus, height), - coinbase_claim: BlockCoinbaseClaim::new(block, store, height), + coinbase_claim: BlockCoinbaseClaim::new(block, consensus, store, height, median_time_past), sigops: BlockSigops::new(block, store, consensus, height), witness: BlockWitness::new(block, deployments), + ordering: BlockTransactionOrdering::new(block, consensus, median_time_past), } } @@ -48,6 +50,7 @@ impl<'a> BlockAcceptor<'a> { self.coinbase_claim.check()?; self.coinbase_script.check()?; self.witness.check()?; + self.ordering.check()?; Ok(()) } } @@ -187,14 +190,22 @@ pub struct BlockCoinbaseClaim<'a> { block: CanonBlock<'a>, store: &'a TransactionOutputProvider, height: u32, + transaction_ordering: TransactionOrdering, } impl<'a> BlockCoinbaseClaim<'a> { - fn new(block: CanonBlock<'a>, store: &'a TransactionOutputProvider, height: u32) -> Self { + fn new( + block: CanonBlock<'a>, + consensus_params: &ConsensusParams, + store: &'a TransactionOutputProvider, + height: u32, + median_time_past: u32 + ) -> Self { BlockCoinbaseClaim { block: block, store: store, height: height, + transaction_ordering: consensus_params.fork.transaction_ordering(median_time_past), } } @@ -207,8 +218,9 @@ impl<'a> BlockCoinbaseClaim<'a> { // (1) Total sum of all referenced outputs let mut incoming: u64 = 0; for input in tx.raw.inputs.iter() { - let (sum, overflow) = incoming.overflowing_add( - store.transaction_output(&input.previous_output, tx_idx).map(|o| o.value).unwrap_or(0)); + let prevout_tx_idx = transaction_index_for_output_check(self.transaction_ordering, tx_idx); + let prevout = store.transaction_output(&input.previous_output, prevout_tx_idx); + let (sum, overflow) = incoming.overflowing_add(prevout.map(|o| o.value).unwrap_or(0)); if overflow { return Err(Error::ReferencedInputsSumOverflow); } @@ -339,12 +351,43 @@ impl<'a> BlockWitness<'a> { } } +pub struct BlockTransactionOrdering<'a> { + block: CanonBlock<'a>, + transaction_ordering: TransactionOrdering, +} + +impl<'a> BlockTransactionOrdering<'a> { + fn new(block: CanonBlock<'a>, consensus: &'a ConsensusParams, median_time_past: u32) -> Self { + BlockTransactionOrdering { + block, + transaction_ordering: consensus.fork.transaction_ordering(median_time_past), + } + } + + fn check(&self) -> Result<(), Error> { + match self.transaction_ordering { + // topological transaction ordering is checked in TransactionMissingInputs + TransactionOrdering::Topological => Ok(()), + // canonical transaction ordering means that transactions are ordered by + // their id (i.e. hash) in ascending order + TransactionOrdering::Canonical => + if self.block.transactions.windows(2).skip(1).all(|w| w[0].hash < w[1].hash) { + Ok(()) + } else { + Err(Error::NonCanonicalTransactionOrdering) + }, + } + } +} + #[cfg(test)] mod tests { extern crate test_data; + use chain::{IndexedBlock, Transaction}; + use network::{Network, ConsensusFork, ConsensusParams, BitcoinCashConsensusParams}; use {Error, CanonBlock}; - use super::BlockCoinbaseScript; + use super::{BlockCoinbaseScript, BlockTransactionOrdering}; #[test] fn test_block_coinbase_script() { @@ -374,4 +417,42 @@ mod tests { assert_eq!(coinbase_script_validator2.check(), Err(Error::CoinbaseScript)); } + + #[test] + fn block_transaction_ordering_works() { + let tx1: Transaction = test_data::TransactionBuilder::with_output(1).into(); + let tx2: Transaction = test_data::TransactionBuilder::with_output(2).into(); + let tx3: Transaction = test_data::TransactionBuilder::with_output(3).into(); + let bad_block: IndexedBlock = test_data::block_builder() + .with_transaction(tx1.clone()) + .with_transaction(tx2.clone()) + .with_transaction(tx3.clone()) + .header().build() + .build() + .into(); + let good_block: IndexedBlock = test_data::block_builder() + .with_transaction(tx1) + .with_transaction(tx3) + .with_transaction(tx2) + .header().build() + .build() + .into(); + + let bad_block = CanonBlock::new(&bad_block); + let good_block = CanonBlock::new(&good_block); + + // when topological ordering is used => we don't care about tx ordering + let consensus = ConsensusParams::new(Network::Unitest, ConsensusFork::BitcoinCore); + let checker = BlockTransactionOrdering::new(bad_block, &consensus, 0); + assert_eq!(checker.check(), Ok(())); + + // when topological ordering is used => we care about tx ordering + let mut bch = BitcoinCashConsensusParams::new(Network::Unitest); + bch.magnetic_anomaly_time = 0; + let consensus = ConsensusParams::new(Network::Unitest, ConsensusFork::BitcoinCash(bch)); + let checker = BlockTransactionOrdering::new(bad_block, &consensus, 0); + assert_eq!(checker.check(), Err(Error::NonCanonicalTransactionOrdering)); + let checker = BlockTransactionOrdering::new(good_block, &consensus, 0); + assert_eq!(checker.check(), Ok(())); + } } diff --git a/verification/src/accept_transaction.rs b/verification/src/accept_transaction.rs index fb219514..063ead20 100644 --- a/verification/src/accept_transaction.rs +++ b/verification/src/accept_transaction.rs @@ -3,7 +3,7 @@ use primitives::bytes::Bytes; use storage::{TransactionMetaProvider, TransactionOutputProvider}; use network::{ConsensusParams, ConsensusFork}; use script::{Script, verify_script, VerificationFlags, TransactionSignatureChecker, TransactionInputSigner, SignatureVersion}; -use duplex_store::DuplexTransactionOutputProvider; +use duplex_store::{DuplexTransactionOutputProvider, transaction_index_for_output_check}; use deployments::BlockDeployments; use script::Builder; use sigops::transaction_sigops; @@ -41,10 +41,12 @@ impl<'a> TransactionAcceptor<'a> { deployments: &'a BlockDeployments<'a>, ) -> Self { trace!(target: "verification", "Tx verification {}", transaction.hash.to_reversed_str()); + let tx_ordering = consensus.fork.transaction_ordering(median_time_past); + let missing_input_tx_index = transaction_index_for_output_check(tx_ordering,transaction_index); TransactionAcceptor { premature_witness: TransactionPrematureWitness::new(transaction, deployments), bip30: TransactionBip30::new_for_sync(transaction, meta_store, consensus, block_hash, height), - missing_inputs: TransactionMissingInputs::new(transaction, output_store, transaction_index), + missing_inputs: TransactionMissingInputs::new(transaction, output_store, missing_input_tx_index), maturity: TransactionMaturity::new(transaction, meta_store, height), overspent: TransactionOverspent::new(transaction, output_store), double_spent: TransactionDoubleSpend::new(transaction, output_store), diff --git a/verification/src/chain_verifier.rs b/verification/src/chain_verifier.rs index 769d96f7..246dce83 100644 --- a/verification/src/chain_verifier.rs +++ b/verification/src/chain_verifier.rs @@ -154,11 +154,12 @@ mod tests { extern crate test_data; use std::sync::Arc; - use chain::IndexedBlock; - use storage::{Error as DBError}; + use chain::{IndexedBlock, Transaction, Block}; + use storage::Error as DBError; use db::BlockChainDatabase; - use network::{Network, ConsensusParams, ConsensusFork}; + use network::{Network, ConsensusParams, ConsensusFork, BitcoinCashConsensusParams}; use script; + use constants::DOUBLE_SPACING_SECONDS; use super::BackwardsCompatibleChainVerifier as ChainVerifier; use {Verify, Error, TransactionError, VerificationLevel}; @@ -178,7 +179,6 @@ mod tests { assert!(verifier.verify(VerificationLevel::Full, &b1.into()).is_ok()); } - #[test] fn first_tx() { let storage = BlockChainDatabase::init_test_chain( @@ -335,6 +335,66 @@ mod tests { assert_eq!(expected, verifier.verify(VerificationLevel::Full, &block.into())); } + #[test] + fn transaction_references_same_block_and_goes_before_previous() { + let mut blocks = vec![test_data::block_builder() + .transaction() + .coinbase() + .output().value(50).build() + .build() + .merkled_header().build() + .build()]; + let input_tx = blocks[0].transactions()[0].clone(); + let mut parent_hash = blocks[0].hash(); + // waiting 100 blocks for genesis coinbase to become valid + for _ in 0..100 { + let block: Block = test_data::block_builder() + .transaction().coinbase().build() + .merkled_header().parent(parent_hash).build() + .build() + .into(); + parent_hash = block.hash(); + blocks.push(block); + } + + let storage = Arc::new(BlockChainDatabase::init_test_chain(blocks.into_iter().map(Into::into).collect())); + + let tx1: Transaction = test_data::TransactionBuilder::with_version(4) + .add_input(&input_tx, 0) + .add_output(40) + .into(); + let tx2: Transaction = test_data::TransactionBuilder::with_version(1) + .add_input(&tx1, 0) + .add_output(30) + .into(); + let block = test_data::block_builder() + .transaction() + .coinbase() + .output().value(2).build() + .build() + .with_transaction(tx2) + .with_transaction(tx1) + .merkled_header() + .time(DOUBLE_SPACING_SECONDS + 101) // to pass BCH work check + .parent(parent_hash) + .build() + .build(); + + // when topological order is required + let topological_consensus = ConsensusParams::new(Network::Unitest, ConsensusFork::BitcoinCore); + let verifier = ChainVerifier::new(storage.clone(), topological_consensus); + let expected = Err(Error::Transaction(1, TransactionError::Overspend)); + assert_eq!(expected, verifier.verify(VerificationLevel::Header, &block.clone().into())); + + // when canonical order is required + let mut canonical_params = BitcoinCashConsensusParams::new(Network::Unitest); + canonical_params.magnetic_anomaly_time = 0; + let canonical_consensus = ConsensusParams::new(Network::Unitest, ConsensusFork::BitcoinCash(canonical_params)); + let verifier = ChainVerifier::new(storage, canonical_consensus); + let expected = Ok(()); + assert_eq!(expected, verifier.verify(VerificationLevel::Header, &block.into())); + } + #[test] #[ignore] fn coinbase_happy() { diff --git a/verification/src/duplex_store.rs b/verification/src/duplex_store.rs index 72f3dd00..a9ed797b 100644 --- a/verification/src/duplex_store.rs +++ b/verification/src/duplex_store.rs @@ -2,6 +2,7 @@ //! require sophisticated (in more than one source) previous transaction lookups use chain::{OutPoint, TransactionOutput}; +use network::TransactionOrdering; use storage::TransactionOutputProvider; #[derive(Clone, Copy)] @@ -41,3 +42,19 @@ impl TransactionOutputProvider for NoopStore { false } } + +/// Converts actual transaction index into transaction index to use in +/// TransactionOutputProvider::transaction_output call. +/// When topological ordering is used, we expect ascendant transaction (TX1) +/// to come BEFORE descendant transaction (TX2) in the block, like this: +/// [ ... TX1 ... TX2 ... ] +/// When canonical ordering is used, transactions order within block is not +/// relevant for this check and ascendant transaction (TX1) can come AFTER +/// descendant, like this: +/// [ ... TX2 ... TX1 ... ] +pub fn transaction_index_for_output_check(ordering: TransactionOrdering, tx_idx: usize) -> usize { + match ordering { + TransactionOrdering::Topological => tx_idx, + TransactionOrdering::Canonical => ::std::usize::MAX, + } +} \ No newline at end of file diff --git a/verification/src/error.rs b/verification/src/error.rs index af0d4e70..849ce404 100644 --- a/verification/src/error.rs +++ b/verification/src/error.rs @@ -57,6 +57,8 @@ pub enum Error { WitnessMerkleCommitmentMismatch, /// SegWit: unexpected witness UnexpectedWitness, + /// Non-canonical tranasctions ordering within block + NonCanonicalTransactionOrdering, /// Database error Database(DBError), } diff --git a/verification/src/work_bch.rs b/verification/src/work_bch.rs index 3e1b1d14..79157a9d 100644 --- a/verification/src/work_bch.rs +++ b/verification/src/work_bch.rs @@ -141,7 +141,7 @@ fn work_required_bitcoin_cash_adjusted(parent_header: IndexedBlockHeader, time: // If the new block's timestamp is more than 2 * 10 minutes then allow // mining of a min-difficulty block. let max_bits = consensus.network.max_bits(); - if consensus.network == Network::Testnet { + if consensus.network == Network::Testnet || consensus.network == Network::Unitest { let max_time_gap = parent_header.raw.time + DOUBLE_SPACING_SECONDS; if time > max_time_gap { return max_bits.into(); @@ -218,6 +218,7 @@ mod tests { height: 1000, difficulty_adjustion_height: 0xffffffff, monolith_time: 0xffffffff, + magnetic_anomaly_time: 0xffffffff, })); let mut header_provider = MemoryBlockHeaderProvider::default(); header_provider.insert(BlockHeader { @@ -271,6 +272,7 @@ mod tests { height: 1000, difficulty_adjustion_height: 0xffffffff, monolith_time: 0xffffffff, + magnetic_anomaly_time: 0xffffffff, }));