BCH Nov2018 HF: canonical transaction ordering

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);
+	}
 }

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,
 			},
 		}
 	}

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};

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(()));
+	}
 }

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),

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() {

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,
+	}
+}

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),
 }

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,
 		}));