From b5dae01c3bbe3e0c049b5e8e0ed0cc52fc561e18 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Mon, 7 Sep 2020 10:02:24 +0100 Subject: [PATCH] cleanup based on PR feedback --- chain/src/chain.rs | 6 +++--- chain/src/pipe.rs | 2 +- chain/src/txhashset/txhashset.rs | 2 +- chain/src/txhashset/utxo_view.rs | 8 ++++---- chain/tests/test_coinbase_maturity.rs | 6 +++--- core/src/core/transaction.rs | 16 ---------------- pool/src/pool.rs | 4 +++- pool/src/transaction_pool.rs | 5 +---- pool/src/types.rs | 4 ++-- pool/tests/common.rs | 4 ++-- servers/src/common/adapters.rs | 4 ++-- 11 files changed, 22 insertions(+), 39 deletions(-) diff --git a/chain/src/chain.rs b/chain/src/chain.rs index 9af4212c50..530224f359 100644 --- a/chain/src/chain.rs +++ b/chain/src/chain.rs @@ -296,7 +296,7 @@ impl Chain { pipe::rewind_and_apply_fork(&previous_header, ext, batch)?; ext.extension .utxo_view(ext.header_extension) - .validate_inputs(block.inputs(), batch) + .validate_inputs(&block.inputs(), batch) .map(|outputs| outputs.into_iter().map(|(out, _)| out).collect()) })?; let inputs = inputs.as_slice().into(); @@ -647,7 +647,7 @@ impl Chain { /// that would be spent by the inputs. pub fn validate_inputs( &self, - inputs: Inputs, + inputs: &Inputs, ) -> Result, Error> { let header_pmmr = self.header_pmmr.read(); let txhashset = self.txhashset.read(); @@ -663,7 +663,7 @@ impl Chain { /// Verify we are not attempting to spend a coinbase output /// that has not yet sufficiently matured. - pub fn verify_coinbase_maturity(&self, inputs: Inputs) -> Result<(), Error> { + pub fn verify_coinbase_maturity(&self, inputs: &Inputs) -> Result<(), Error> { let height = self.next_block_height()?; let header_pmmr = self.header_pmmr.read(); let txhashset = self.txhashset.read(); diff --git a/chain/src/pipe.rs b/chain/src/pipe.rs index c71ccc822a..39f64bd4b9 100644 --- a/chain/src/pipe.rs +++ b/chain/src/pipe.rs @@ -424,7 +424,7 @@ fn verify_coinbase_maturity( let header_extension = &ext.header_extension; extension .utxo_view(header_extension) - .verify_coinbase_maturity(block.inputs(), block.header.height, batch) + .verify_coinbase_maturity(&block.inputs(), block.header.height, batch) } /// Verify kernel sums across the full utxo and kernel sets based on block_sums diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index 7ceffe0c94..2e99ac1135 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -1105,7 +1105,7 @@ impl<'a> Extension<'a> { // Remove the spent outputs from the output_pos index. let spent = self .utxo_view(header_ext) - .validate_inputs(b.inputs(), batch)?; + .validate_inputs(&b.inputs(), batch)?; for (out, pos) in &spent { self.apply_input(out.commitment(), *pos)?; affected_pos.push(pos.pos); diff --git a/chain/src/txhashset/utxo_view.rs b/chain/src/txhashset/utxo_view.rs index 7d61ab1e80..b963f3a4d7 100644 --- a/chain/src/txhashset/utxo_view.rs +++ b/chain/src/txhashset/utxo_view.rs @@ -56,7 +56,7 @@ impl<'a> UTXOView<'a> { for output in block.outputs() { self.validate_output(output, batch)?; } - self.validate_inputs(block.inputs(), batch) + self.validate_inputs(&block.inputs(), batch) } /// Validate a transaction against the current UTXO set. @@ -70,7 +70,7 @@ impl<'a> UTXOView<'a> { for output in tx.outputs() { self.validate_output(output, batch)?; } - self.validate_inputs(tx.inputs(), batch) + self.validate_inputs(&tx.inputs(), batch) } /// Validate the provided inputs. @@ -78,7 +78,7 @@ impl<'a> UTXOView<'a> { /// that would be spent by the provided inputs. pub fn validate_inputs( &self, - inputs: Inputs, + inputs: &Inputs, batch: &Batch<'_>, ) -> Result, Error> { match inputs { @@ -166,7 +166,7 @@ impl<'a> UTXOView<'a> { /// that have not sufficiently matured. pub fn verify_coinbase_maturity( &self, - inputs: Inputs, + inputs: &Inputs, height: u64, batch: &Batch<'_>, ) -> Result<(), Error> { diff --git a/chain/tests/test_coinbase_maturity.rs b/chain/tests/test_coinbase_maturity.rs index b6f071ce34..3f5587c973 100644 --- a/chain/tests/test_coinbase_maturity.rs +++ b/chain/tests/test_coinbase_maturity.rs @@ -122,7 +122,7 @@ fn test_coinbase_maturity() { // Confirm the tx attempting to spend the coinbase output // is not valid at the current block height given the current chain state. - match chain.verify_coinbase_maturity(coinbase_txn.inputs()) { + match chain.verify_coinbase_maturity(&coinbase_txn.inputs()) { Ok(_) => {} Err(e) => match e.kind() { ErrorKind::ImmatureCoinbase => {} @@ -204,7 +204,7 @@ fn test_coinbase_maturity() { // Confirm the tx attempting to spend the coinbase output // is not valid at the current block height given the current chain state. - match chain.verify_coinbase_maturity(coinbase_txn.inputs()) { + match chain.verify_coinbase_maturity(&coinbase_txn.inputs()) { Ok(_) => {} Err(e) => match e.kind() { ErrorKind::ImmatureCoinbase => {} @@ -255,7 +255,7 @@ fn test_coinbase_maturity() { // Confirm the tx spending the coinbase output is now valid. // The coinbase output has matured sufficiently based on current chain state. chain - .verify_coinbase_maturity(coinbase_txn.inputs()) + .verify_coinbase_maturity(&coinbase_txn.inputs()) .unwrap(); let txs = &[coinbase_txn]; diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 90004033bb..04ac4ad17e 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -1557,14 +1557,6 @@ impl From<&OutputIdentifier> for Input { } } -// impl ::std::hash::Hash for Input { -// fn hash(&self, state: &mut H) { -// let mut vec = Vec::new(); -// ser::serialize_default(&mut vec, &self).expect("serialization failed"); -// ::std::hash::Hash::hash(&vec, state); -// } -// } - /// Implementation of Writeable for a transaction Input, defines how to write /// an Input as binary. impl Writeable for Input { @@ -1889,14 +1881,6 @@ impl AsRef for Output { } } -// impl ::std::hash::Hash for Output { -// fn hash(&self, state: &mut H) { -// let mut vec = Vec::new(); -// ser::serialize_default(&mut vec, &self).expect("serialization failed"); -// ::std::hash::Hash::hash(&vec, state); -// } -// } - /// Implementation of Writeable for a transaction Output, defines how to write /// an Output as binary. impl Writeable for Output { diff --git a/pool/src/pool.rs b/pool/src/pool.rs index eea7f67865..c3b446b0a7 100644 --- a/pool/src/pool.rs +++ b/pool/src/pool.rs @@ -269,6 +269,8 @@ where Ok(valid_txs) } + /// Lookup unspent outputs to be spent by the provided transaction. + /// We look for unspent outputs in the current txpool and then in the current utxo. pub fn locate_spends( &self, tx: &Transaction, @@ -292,7 +294,7 @@ where transaction::cut_through(&mut inputs[..], &mut outputs[..])?; // Lookup remaining outputs to be spent from the current utxo. - let spent_utxo = self.blockchain.validate_inputs(spent_utxo.into())?; + let spent_utxo = self.blockchain.validate_inputs(&spent_utxo.into())?; Ok((spent_pool.to_vec(), spent_utxo)) } diff --git a/pool/src/transaction_pool.rs b/pool/src/transaction_pool.rs index d11bcbac0f..29496834e8 100644 --- a/pool/src/transaction_pool.rs +++ b/pool/src/transaction_pool.rs @@ -160,9 +160,6 @@ where stem: bool, header: &BlockHeader, ) -> Result<(), PoolError> { - // - // TODO - hash here is not sufficient as we have v2 vs. v3 txs... - // // Quick check for duplicate txs. // Our stempool is private and we do not want to reveal anything about the txs contained. // If this is a stem tx and is already present in stempool then fluff by adding to txpool. @@ -223,7 +220,7 @@ where .cloned() .collect(); self.blockchain - .verify_coinbase_maturity(coinbase_inputs.as_slice().into())?; + .verify_coinbase_maturity(&coinbase_inputs.as_slice().into())?; // Convert the tx to "v2" compatibility with "features and commit" inputs. let ref entry = self.convert_tx_v2(entry, &spent_pool, &spent_utxo)?; diff --git a/pool/src/types.rs b/pool/src/types.rs index bc34cb8f62..9f6eea428e 100644 --- a/pool/src/types.rs +++ b/pool/src/types.rs @@ -275,7 +275,7 @@ impl From for PoolError { pub trait BlockChain: Sync + Send { /// Verify any coinbase outputs being spent /// have matured sufficiently. - fn verify_coinbase_maturity(&self, inputs: Inputs) -> Result<(), PoolError>; + fn verify_coinbase_maturity(&self, inputs: &Inputs) -> Result<(), PoolError>; /// Verify any coinbase outputs being spent /// have matured sufficiently. @@ -287,7 +287,7 @@ pub trait BlockChain: Sync + Send { /// Validate inputs against the current utxo. /// Returns the vec of output identifiers that would be spent /// by these inputs if they can all be successfully spent. - fn validate_inputs(&self, inputs: Inputs) -> Result, PoolError>; + fn validate_inputs(&self, inputs: &Inputs) -> Result, PoolError>; fn chain_head(&self) -> Result; diff --git a/pool/tests/common.rs b/pool/tests/common.rs index 7e3e1468b3..52ef16a890 100644 --- a/pool/tests/common.rs +++ b/pool/tests/common.rs @@ -136,14 +136,14 @@ impl BlockChain for ChainAdapter { }) } - fn validate_inputs(&self, inputs: Inputs) -> Result, PoolError> { + fn validate_inputs(&self, inputs: &Inputs) -> Result, PoolError> { self.chain .validate_inputs(inputs) .map(|outputs| outputs.into_iter().map(|(out, _)| out).collect::>()) .map_err(|_| PoolError::Other("failed to validate inputs".into())) } - fn verify_coinbase_maturity(&self, inputs: Inputs) -> Result<(), PoolError> { + fn verify_coinbase_maturity(&self, inputs: &Inputs) -> Result<(), PoolError> { self.chain .verify_coinbase_maturity(inputs) .map_err(|_| PoolError::ImmatureCoinbase) diff --git a/servers/src/common/adapters.rs b/servers/src/common/adapters.rs index f1f8f9e2e8..cd17e069b0 100644 --- a/servers/src/common/adapters.rs +++ b/servers/src/common/adapters.rs @@ -953,14 +953,14 @@ impl pool::BlockChain for PoolToChainAdapter { .map_err(|_| pool::PoolError::Other("failed to validate tx".to_string())) } - fn validate_inputs(&self, inputs: Inputs) -> Result, pool::PoolError> { + fn validate_inputs(&self, inputs: &Inputs) -> Result, pool::PoolError> { self.chain() .validate_inputs(inputs) .map(|outputs| outputs.into_iter().map(|(out, _)| out).collect::>()) .map_err(|_| pool::PoolError::Other("failed to validate tx".to_string())) } - fn verify_coinbase_maturity(&self, inputs: Inputs) -> Result<(), pool::PoolError> { + fn verify_coinbase_maturity(&self, inputs: &Inputs) -> Result<(), pool::PoolError> { self.chain() .verify_coinbase_maturity(inputs) .map_err(|_| pool::PoolError::ImmatureCoinbase)