Skip to content

Commit

Permalink
cleanup based on PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
antiochp committed Sep 7, 2020
1 parent a5be223 commit 7c32e4e
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 38 deletions.
6 changes: 3 additions & 3 deletions chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -647,7 +647,7 @@ impl Chain {
/// that would be spent by the inputs.
pub fn validate_inputs(
&self,
inputs: Inputs,
inputs: &Inputs,
) -> Result<Vec<(OutputIdentifier, CommitPos)>, Error> {
let header_pmmr = self.header_pmmr.read();
let txhashset = self.txhashset.read();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion chain/src/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion chain/src/txhashset/txhashset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions chain/src/txhashset/utxo_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -70,15 +70,15 @@ 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.
/// Returns a vec of output identifiers corresponding to outputs
/// that would be spent by the provided inputs.
pub fn validate_inputs(
&self,
inputs: Inputs,
inputs: &Inputs,
batch: &Batch<'_>,
) -> Result<Vec<(OutputIdentifier, CommitPos)>, Error> {
match inputs {
Expand Down Expand Up @@ -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> {
Expand Down
6 changes: 3 additions & 3 deletions chain/tests/test_coinbase_maturity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {}
Expand Down Expand Up @@ -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 => {}
Expand Down Expand Up @@ -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];
Expand Down
16 changes: 0 additions & 16 deletions core/src/core/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1557,14 +1557,6 @@ impl From<&OutputIdentifier> for Input {
}
}

// impl ::std::hash::Hash for Input {
// fn hash<H: ::std::hash::Hasher>(&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 {
Expand Down Expand Up @@ -1889,14 +1881,6 @@ impl AsRef<Commitment> for Output {
}
}

// impl ::std::hash::Hash for Output {
// fn hash<H: ::std::hash::Hasher>(&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 {
Expand Down
4 changes: 3 additions & 1 deletion pool/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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))
}
Expand Down
5 changes: 1 addition & 4 deletions pool/src/transaction_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 2 additions & 2 deletions pool/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ impl From<committed::Error> 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.
Expand All @@ -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<Vec<OutputIdentifier>, PoolError>;
fn validate_inputs(&self, inputs: &Inputs) -> Result<Vec<OutputIdentifier>, PoolError>;

fn chain_head(&self) -> Result<BlockHeader, PoolError>;

Expand Down
4 changes: 2 additions & 2 deletions pool/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ impl BlockChain for ChainAdapter {
})
}

fn validate_inputs(&self, inputs: Inputs) -> Result<Vec<OutputIdentifier>, PoolError> {
fn validate_inputs(&self, inputs: &Inputs) -> Result<Vec<OutputIdentifier>, PoolError> {
self.chain
.validate_inputs(inputs)
.map(|outputs| outputs.into_iter().map(|(out, _)| out).collect::<Vec<_>>())
.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)
Expand Down
2 changes: 1 addition & 1 deletion servers/src/common/adapters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ impl pool::BlockChain for PoolToChainAdapter {
.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)
Expand Down

0 comments on commit 7c32e4e

Please sign in to comment.