From 563997e935f02abde4ba0f969ae50bbe4c4be5cd Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 2 Dec 2016 14:45:14 +0300 Subject: [PATCH 1/3] no double-spend transactions in mempool --- chain/src/transaction.rs | 2 +- miner/src/memory_pool.rs | 165 +++++++++++++++++++++++++++--- sync/src/synchronization_chain.rs | 38 +++++-- test-data/src/chain_builder.rs | 4 + 4 files changed, 190 insertions(+), 19 deletions(-) diff --git a/chain/src/transaction.rs b/chain/src/transaction.rs index 5ab7a1bc..327fa76f 100644 --- a/chain/src/transaction.rs +++ b/chain/src/transaction.rs @@ -34,7 +34,7 @@ pub const SEQUENCE_LOCKTIME_MASK: u32 = 0x0000ffff; /// otherwise as UNIX timestamp. pub const LOCKTIME_THRESHOLD: u32 = 500000000; // Tue Nov 5 00:53:20 1985 UTC -#[derive(Debug, PartialEq, Clone, Default)] +#[derive(Debug, PartialEq, Eq, Clone, Default)] pub struct OutPoint { pub hash: H256, pub index: u32, diff --git a/miner/src/memory_pool.rs b/miner/src/memory_pool.rs index f8799946..5c3d1f4a 100644 --- a/miner/src/memory_pool.rs +++ b/miner/src/memory_pool.rs @@ -13,6 +13,8 @@ use std::cmp::Ordering; use std::collections::HashMap; use std::collections::HashSet; use std::collections::BTreeSet; +use std::collections::VecDeque; +use std::hash::{Hash, Hasher}; use ser::{Serializable, serialize}; use heapsize::HeapSizeOf; @@ -78,6 +80,8 @@ struct Storage { transactions_size_in_bytes: usize, /// By-hash storage by_hash: HashMap, + /// Transactions by previous output + by_previous_output: HashMap, /// References storage references: ReferenceStorage, } @@ -136,6 +140,27 @@ struct ByPackageScoreOrderedEntry { package_miner_virtual_fee: i64, } +#[derive(Debug, PartialEq, Eq, Clone)] +struct HashedOutPoint { + /// Transasction output point + out_point: OutPoint, +} + +impl From for HashedOutPoint { + fn from(out_point: OutPoint) -> Self { + HashedOutPoint { + out_point: out_point, + } + } +} + +impl Hash for HashedOutPoint { + fn hash(&self, state: &mut H) where H: Hasher { + state.write(&serialize(&self.out_point)); + state.finish(); + } +} + impl<'a> From<&'a Entry> for ByTimestampOrderedEntry { fn from(entry: &'a Entry) -> Self { ByTimestampOrderedEntry { @@ -236,6 +261,7 @@ impl Storage { counter: 0, transactions_size_in_bytes: 0, by_hash: HashMap::new(), + by_previous_output: HashMap::new(), references: ReferenceStorage { by_input: HashMap::new(), pending: HashSet::new(), @@ -280,6 +306,12 @@ impl Storage { self.references.ordered.insert_to_orderings(&entry); } + // remember that all inputs of this transaction are spent + for input in &entry.transaction.inputs { + let previous_tx = self.by_previous_output.insert(input.previous_output.clone().into(), entry.hash.clone()); + assert_eq!(previous_tx, None); // transaction must be verified before => no double spend + } + // add to by_hash storage self.by_hash.insert(entry.hash.clone(), entry); } @@ -292,6 +324,10 @@ impl Storage { self.by_hash.contains_key(hash) } + pub fn is_output_spent(&self, prevout: &OutPoint) -> bool { + self.by_previous_output.contains_key(&prevout.clone().into()) + } + pub fn set_virtual_fee(&mut self, h: &H256, virtual_fee: i64) { // for updating ancestors let mut miner_virtual_fee_change = 0i64; @@ -392,6 +428,12 @@ impl Storage { // update pool information self.transactions_size_in_bytes -= entry.size; + // forget that all inputs of this transaction are spent + for input in &entry.transaction.inputs { + let spent_in_tx = self.by_previous_output.remove(&input.previous_output.clone().into()).expect("every spent output must be indexed"); + assert_eq!(&spent_in_tx, h); + } + // remove from storage self.references.remove(None, &self.by_hash, &entry); @@ -399,6 +441,25 @@ impl Storage { }) } + pub fn remove_by_prevout(&mut self, prevout: &OutPoint) -> Option> { + let mut queue: VecDeque = VecDeque::new(); + let mut removed: Vec = Vec::new(); + queue.push_back(prevout.clone()); + + while let Some(prevout) = queue.pop_front() { + if let Some(entry_hash) = self.by_previous_output.get(&prevout.clone().into()).cloned() { + let entry = self.remove_by_hash(&entry_hash).expect("checket that it exists line above; qed"); + queue.extend(entry.transaction.outputs.iter().enumerate().map(|(idx, _)| OutPoint { + hash: entry_hash.clone(), + index: idx as u32, + })); + removed.push(entry.transaction); + } + } + + Some(removed) + } + pub fn remove_by_parent_hash(&mut self, h: &H256) -> Option> { // this code will run only when ancestor transaction is inserted // in memory pool after its descendants @@ -579,6 +640,11 @@ impl MemoryPool { self.storage.remove_by_hash(h).map(|entry| entry.transaction) } + /// Removes transaction (and all its descendants) which has spent given output + pub fn remove_by_prevout(&mut self, prevout: &OutPoint) -> Option> { + self.storage.remove_by_prevout(prevout) + } + /// Reads single transaction by its hash. pub fn read_by_hash(&self, h: &H256) -> Option<&Transaction> { self.storage.read_by_hash(h) @@ -712,8 +778,8 @@ impl PreviousTransactionOutputProvider for MemoryPool { .cloned() } - fn is_spent(&self, _prevout: &OutPoint) -> bool { - false + fn is_spent(&self, prevout: &OutPoint) -> bool { + self.storage.is_output_spent(prevout) } } @@ -725,7 +791,8 @@ impl HeapSizeOf for MemoryPool { #[cfg(test)] mod tests { - use chain::Transaction; + use db::PreviousTransactionOutputProvider; + use chain::{Transaction, OutPoint}; use heapsize::HeapSizeOf; use super::{MemoryPool, OrderingStrategy}; use test_data::{ChainBuilder, TransactionBuilder}; @@ -965,7 +1032,7 @@ mod tests { // all transactions of same size TransactionBuilder::with_default_input(0).set_output(30).store(chain) // transaction0 .into_input(0).set_output(50).store(chain) // transaction0 -> transaction1 - .set_default_input(0).set_output(35).store(chain) // transaction2 + .set_default_input(1).set_output(35).store(chain) // transaction2 .into_input(0).set_output(10).store(chain) // transaction2 -> transaction3 .into_input(0).set_output(100).store(chain); // transaction2 -> transaction3 -> transaction4 @@ -1020,8 +1087,8 @@ mod tests { // all transactions of same size TransactionBuilder::with_default_input(0).set_output(17).store(chain) // transaction0 .into_input(0).set_output(50).store(chain) // transaction0 -> transaction1 - .into_input(0).set_output(7).store(chain) // transaction0 -> transaction1 -> transaction2 - .set_default_input(0).set_output(20).store(chain); // transaction3 + .into_input(0).set_output(7).store(chain) // transaction0 -> transaction1 -> transaction2 + .set_default_input(1).set_output(20).store(chain); // transaction3 let mut pool = MemoryPool::new(); @@ -1046,14 +1113,14 @@ mod tests { let chain = &mut ChainBuilder::new(); // all transactions of same size (=> 3 inputs) // construct level0 - TransactionBuilder::with_default_input(0).add_default_input(0).add_default_input(0).set_output(10).store(chain) // transaction0 - .set_default_input(0).add_default_input(0).add_default_input(0).set_output(20).store(chain) // transaction1 - .set_default_input(0).add_default_input(0).add_default_input(0).set_output(30).store(chain) // transaction2 + TransactionBuilder::with_default_input(0).add_default_input(1).add_default_input(2).set_output(10).add_output(10).store(chain) // transaction0 + .set_default_input(3).add_default_input(4).add_default_input(5).set_output(20).add_output(20).store(chain) // transaction1 + .set_default_input(6).add_default_input(7).add_default_input(8).set_output(30).add_output(30).store(chain) // transaction2 // construct level1 - .set_default_input(0).add_default_input(0).add_input(&chain.at(0), 0).set_output(40).store(chain) // transaction0 -> transaction3 - .set_default_input(0).add_input(&chain.at(0), 0).add_input(&chain.at(1), 0).set_output(50).store(chain) // transaction0 + transaction1 -> transaction4 + .set_default_input(9).add_default_input(10).add_input(&chain.at(0), 0).set_output(40).add_output(40).store(chain) // transaction0 -> transaction3 + .set_default_input(11).add_input(&chain.at(0), 1).add_input(&chain.at(1), 0).set_output(50).add_output(50).store(chain) // transaction0 + transaction1 -> transaction4 // construct level3 - .set_input(&chain.at(2), 0).add_input(&chain.at(3), 0).add_input(&chain.at(4), 0).set_output(60).store(chain); // transaction2 + transaction3 + transaction4 -> transaction5 + .set_input(&chain.at(2), 0).add_input(&chain.at(3), 0).add_input(&chain.at(4), 0).set_output(60).add_output(60).store(chain); // transaction2 + transaction3 + transaction4 -> transaction5 let mut pool = MemoryPool::new(); @@ -1096,4 +1163,78 @@ mod tests { assert_eq!(pool.read_n_with_strategy(6, OrderingStrategy::ByTransactionScore), expected); assert_eq!(pool.read_n_with_strategy(6, OrderingStrategy::ByPackageScore), expected); } + + #[test] + fn test_memory_pool_spent_transaction_output() { + let chain = &mut ChainBuilder::new(); + + // all transactions of same size (=> 3 inputs) + // construct level0 + TransactionBuilder::with_output(10).store(chain) // transaction0 + .set_output(20).store(chain) // transaction1 + .set_input(&chain.at(0), 0).add_output(30).store(chain); // transaction0 -> transaction2 + + let mut pool = MemoryPool::new(); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(0), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(1), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(2), index: 0, })); + + pool.insert_verified(chain.at(0)); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(0), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(1), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(2), index: 0, })); + + pool.insert_verified(chain.at(1)); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(0), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(1), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(2), index: 0, })); + + pool.insert_verified(chain.at(2)); + assert!(pool.is_spent(&OutPoint { hash: chain.hash(0), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(1), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(2), index: 0, })); + + pool.remove_by_hash(&chain.at(2).hash()); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(0), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(1), index: 0, })); + assert!(!pool.is_spent(&OutPoint { hash: chain.hash(2), index: 0, })); + } + + #[test] + fn test_memory_pool_remove_by_prevout() { + let chain = &mut ChainBuilder::new(); + + // all transactions of same size (=> 3 inputs) + // construct level0 + TransactionBuilder::with_output(10).store(chain) // transaction0 + .into_input(0).add_output(20).store(chain) // transaction0 -> transaction1 + .into_input(0).add_output(30).store(chain) // transaction0 -> transaction1 -> transaction2 + .reset().add_output(40).store(chain); // transaction3 + let mut pool = MemoryPool::new(); +println!("{:?}", chain.hash(0)); +println!("{:?}", chain.hash(1)); +println!("{:?}", chain.hash(2)); +println!("{:?}", chain); + pool.insert_verified(chain.at(0)); + pool.insert_verified(chain.at(1)); + pool.insert_verified(chain.at(2)); + pool.insert_verified(chain.at(3)); + assert_eq!(pool.information().transactions_count, 4); + + assert_eq!(pool.remove_by_prevout(&OutPoint { hash: chain.hash(0), index: 0 }), Some(vec![chain.at(1), chain.at(2)])); + assert_eq!(pool.information().transactions_count, 2); + } } +/* +5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a +1ea5f32ad0cdc457aa051a7ce974e168ac378317631ce732be97ad0329900532 +58d4d3f3f5091e03c875d56458c0dbab6362e88eeb8acce237d76952658bb8bb +[ + Transaction { version: 0, inputs: [], outputs: [TransactionOutput { value: 10, script_pubkey: }], lock_time: 0 }, + Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 20, script_pubkey: }], lock_time: 0 }, + Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 1ea5f32ad0cdc457aa051a7ce974e168ac378317631ce732be97ad0329900532, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 30, script_pubkey: }], lock_time: 0 }, + Transaction { version: 0, inputs: [], outputs: [TransactionOutput { value: 40, script_pubkey: }], lock_time: 0 } +] +thread 'memory_pool::tests::test_memory_pool_remove_by_prevout' panicked at 'assertion failed: `(left == right)` (left: `Some([Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 20, script_pubkey: }], lock_time: 0 }])`, right: `Some([Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 20, script_pubkey: }], lock_time: 0 }, Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 1ea5f32ad0cdc457aa051a7ce974e168ac378317631ce732be97ad0329900532, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 30, script_pubkey: }], lock_time: 0 }])`)', miner/src/memory_pool.rs:1219 +stack backtrace: +*/ diff --git a/sync/src/synchronization_chain.rs b/sync/src/synchronization_chain.rs index c58284b6..171362fa 100644 --- a/sync/src/synchronization_chain.rs +++ b/sync/src/synchronization_chain.rs @@ -354,10 +354,13 @@ impl Chain { // all transactions from this block were accepted // => delete accepted transactions from verification queue and from the memory pool - let this_block_transactions_hashes: Vec = block.transaction_hashes().iter().cloned().collect(); - for transaction_accepted in this_block_transactions_hashes { - self.memory_pool.remove_by_hash(&transaction_accepted); - self.verifying_transactions.remove(&transaction_accepted); + // + also remove transactions which spent outputs which have been spent by transactions from the block + for (tx_hash, tx) in block.transactions() { + self.memory_pool.remove_by_hash(tx_hash); + self.verifying_transactions.remove(tx_hash); + for tx_input in &tx.inputs { + self.memory_pool.remove_by_prevout(&tx_input.previous_output); + } } // no transactions to reverify, because we have just appended new transactions to the blockchain @@ -403,8 +406,7 @@ impl Chain { .map(|h| (h.clone(), self.storage.transaction(&h).expect("block in storage => block transaction in storage"))) .collect(); - // reverify memory pool transactions - // TODO: maybe reverify only transactions, which depends on other reverifying transactions + transactions from new main branch? + // reverify memory pool transactions, sorted by timestamp let memory_pool_transactions_count = self.memory_pool.information().transactions_count; let memory_pool_transactions: Vec<_> = self.memory_pool .remove_n_with_strategy(memory_pool_transactions_count, MemoryPoolOrderingStrategy::ByTimestamp) @@ -1168,4 +1170,28 @@ mod tests { assert_eq!(insert_result.canonized_blocks_hashes, vec![b3.hash(), b4.hash(), b5.hash()]); assert_eq!(chain.information().transactions.transactions_count, 0); // tx3, tx4, tx5 are added to the database } + + #[test] + fn double_spend_transaction_is_removed_from_memory_pool_when_output_is_spent_in_block_transaction() { + let genesis = test_data::genesis(); + let tx0 = genesis.transactions[0].clone(); + let b0 = test_data::block_builder().header().nonce(1).parent(genesis.hash()).build() + .transaction() + .lock_time(1) + .input().hash(tx0.hash()).index(0).build() + .build() + .build(); // genesis -> b0[tx1] + // tx1 && tx2 are spending same output + let tx2: Transaction = test_data::TransactionBuilder::with_output(20).add_input(&tx0, 0).into(); + let tx3: Transaction = test_data::TransactionBuilder::with_output(20).add_input(&tx0, 1).into(); + + // insert tx2 to memory pool + let mut chain = Chain::new(Arc::new(db::TestStorage::with_genesis_block())); + chain.insert_verified_transaction(tx2.clone()); + chain.insert_verified_transaction(tx3.clone()); + // insert verified block with tx1 + chain.insert_best_block(b0.hash(), &b0.into()).expect("no error"); + // => tx2 is removed from memory pool, but tx3 remains + assert_eq!(chain.information().transactions.transactions_count, 1); + } } diff --git a/test-data/src/chain_builder.rs b/test-data/src/chain_builder.rs index c239742b..5a37f9b1 100644 --- a/test-data/src/chain_builder.rs +++ b/test-data/src/chain_builder.rs @@ -55,6 +55,10 @@ impl TransactionBuilder { builder.add_input(&Transaction::default(), output_index) } + pub fn reset(self) -> TransactionBuilder { + TransactionBuilder::default() + } + pub fn into_input(self, output_index: u32) -> TransactionBuilder { let builder = TransactionBuilder::default(); builder.add_input(&self.transaction, output_index) From 83939e1380ca21590237dcc1a25705dd10ec91c1 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 2 Dec 2016 14:49:50 +0300 Subject: [PATCH 2/3] removed debugging comment --- miner/src/memory_pool.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/miner/src/memory_pool.rs b/miner/src/memory_pool.rs index 5c3d1f4a..cdd08677 100644 --- a/miner/src/memory_pool.rs +++ b/miner/src/memory_pool.rs @@ -1225,16 +1225,3 @@ println!("{:?}", chain); assert_eq!(pool.information().transactions_count, 2); } } -/* -5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a -1ea5f32ad0cdc457aa051a7ce974e168ac378317631ce732be97ad0329900532 -58d4d3f3f5091e03c875d56458c0dbab6362e88eeb8acce237d76952658bb8bb -[ - Transaction { version: 0, inputs: [], outputs: [TransactionOutput { value: 10, script_pubkey: }], lock_time: 0 }, - Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 20, script_pubkey: }], lock_time: 0 }, - Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 1ea5f32ad0cdc457aa051a7ce974e168ac378317631ce732be97ad0329900532, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 30, script_pubkey: }], lock_time: 0 }, - Transaction { version: 0, inputs: [], outputs: [TransactionOutput { value: 40, script_pubkey: }], lock_time: 0 } -] -thread 'memory_pool::tests::test_memory_pool_remove_by_prevout' panicked at 'assertion failed: `(left == right)` (left: `Some([Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 20, script_pubkey: }], lock_time: 0 }])`, right: `Some([Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 5ea04b4fa956b5a00393ab57979396b8fdab84f29883fda5403878406b48272a, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 20, script_pubkey: }], lock_time: 0 }, Transaction { version: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: 1ea5f32ad0cdc457aa051a7ce974e168ac378317631ce732be97ad0329900532, index: 0 }, script_sig: , sequence: 0 }], outputs: [TransactionOutput { value: 30, script_pubkey: }], lock_time: 0 }])`)', miner/src/memory_pool.rs:1219 -stack backtrace: -*/ From 14cde7adf998c5593cb0628f79cdb13d7ed6a90d Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 2 Dec 2016 14:51:32 +0300 Subject: [PATCH 3/3] removed diagnostic println --- miner/src/memory_pool.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/miner/src/memory_pool.rs b/miner/src/memory_pool.rs index cdd08677..59c7a5a6 100644 --- a/miner/src/memory_pool.rs +++ b/miner/src/memory_pool.rs @@ -1211,10 +1211,7 @@ mod tests { .into_input(0).add_output(30).store(chain) // transaction0 -> transaction1 -> transaction2 .reset().add_output(40).store(chain); // transaction3 let mut pool = MemoryPool::new(); -println!("{:?}", chain.hash(0)); -println!("{:?}", chain.hash(1)); -println!("{:?}", chain.hash(2)); -println!("{:?}", chain); + pool.insert_verified(chain.at(0)); pool.insert_verified(chain.at(1)); pool.insert_verified(chain.at(2));