diff --git a/core/src/consensus.rs b/core/src/consensus.rs index f5e907994..bbb118f2f 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -19,7 +19,6 @@ //! here. use std::cmp::{max, min}; -use std::fmt; use global; use pow::Difficulty; @@ -216,19 +215,6 @@ pub const UNIT_DIFFICULTY: u64 = /// ethereum GPUs (assuming 1GPU can solve a block at diff 1 in one block interval) pub const INITIAL_DIFFICULTY: u64 = 1_000_000 * UNIT_DIFFICULTY; -/// Consensus errors -#[derive(Clone, Debug, Eq, PartialEq, Fail)] -pub enum Error { - /// Inputs/outputs/kernels must be sorted lexicographically. - SortError, -} - -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Sort Error") - } -} - /// Minimal header information required for the Difficulty calculation to /// take place #[derive(Clone, Debug, Eq, PartialEq)] @@ -386,12 +372,6 @@ pub fn secondary_pow_scaling(height: u64, diff_data: &[HeaderInfo]) -> u32 { max(MIN_DIFFICULTY, min(scale, MAX_SECONDARY_SCALING)) as u32 } -/// Consensus rule that collections of items are sorted lexicographically. -pub trait VerifySortOrder { - /// Verify a collection of items is sorted as required. - fn verify_sort_order(&self) -> Result<(), Error>; -} - #[cfg(test)] mod test { use super::*; diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 4f2bfc0a5..287b33c05 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -22,7 +22,7 @@ use std::iter::FromIterator; use std::sync::Arc; use util::RwLock; -use consensus::{self, reward, REWARD}; +use consensus::{reward, REWARD}; use core::committed::{self, Committed}; use core::compact_block::{CompactBlock, CompactBlockBody}; use core::hash::{Hash, Hashed, ZERO_HASH}; @@ -60,8 +60,6 @@ pub enum Error { Secp(secp::Error), /// Underlying keychain related error Keychain(keychain::Error), - /// Underlying consensus error (sort order currently) - Consensus(consensus::Error), /// Underlying Merkle proof error MerkleProof, /// Error when verifying kernel sums via committed trait. @@ -69,6 +67,8 @@ pub enum Error { /// Validation error relating to cut-through. /// Specifically the tx is spending its own output, which is not valid. CutThrough, + /// Underlying serialization error. + Serialization(ser::Error), /// Other unspecified error condition Other(String), } @@ -85,6 +85,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: ser::Error) -> Error { + Error::Serialization(e) + } +} + impl From for Error { fn from(e: secp::Error) -> Error { Error::Secp(e) @@ -97,12 +103,6 @@ impl From for Error { } } -impl From for Error { - fn from(e: consensus::Error) -> Error { - Error::Consensus(e) - } -} - impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "Block Error (display needs implementation") diff --git a/core/src/core/compact_block.rs b/core/src/core/compact_block.rs index e5f7bdc34..fc695f706 100644 --- a/core/src/core/compact_block.rs +++ b/core/src/core/compact_block.rs @@ -16,12 +16,11 @@ use rand::{thread_rng, Rng}; -use consensus::VerifySortOrder; use core::block::{Block, BlockHeader, Error}; use core::hash::Hashed; use core::id::ShortIdentifiable; use core::{KernelFeatures, Output, OutputFeatures, ShortId, TxKernel}; -use ser::{self, read_multi, Readable, Reader, Writeable, Writer}; +use ser::{self, read_multi, Readable, Reader, VerifySortOrder, Writeable, Writer}; /// Container for full (full) outputs and kernels and kern_ids for a compact block. #[derive(Debug, Clone)] @@ -74,11 +73,11 @@ impl CompactBlockBody { Ok(()) } - // Verify everything is sorted in lexicographical order. + // Verify everything is sorted in lexicographical order and no duplicates present. fn verify_sorted(&self) -> Result<(), Error> { - self.out_full.verify_sort_order()?; - self.kern_full.verify_sort_order()?; - self.kern_ids.verify_sort_order()?; + self.out_full.verify_sorted_and_unique()?; + self.kern_full.verify_sorted_and_unique()?; + self.kern_ids.verify_sorted_and_unique()?; Ok(()) } } diff --git a/core/src/core/hash.rs b/core/src/core/hash.rs index c52e34208..40f6cc93e 100644 --- a/core/src/core/hash.rs +++ b/core/src/core/hash.rs @@ -25,7 +25,6 @@ use std::{fmt, ops}; use blake2::blake2b::Blake2b; -use consensus; use ser::{self, AsFixedBytes, Error, FixedLength, Readable, Reader, Writeable, Writer}; use util; @@ -230,19 +229,3 @@ impl Hashed for W { Hash(ret) } } - -impl consensus::VerifySortOrder for Vec { - fn verify_sort_order(&self) -> Result<(), consensus::Error> { - if self - .iter() - .map(|item| item.hash()) - .collect::>() - .windows(2) - .any(|pair| pair[0] > pair[1]) - { - Err(consensus::Error::SortError) - } else { - Ok(()) - } - } -} diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index 4b1e61efc..a55364009 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -23,12 +23,12 @@ use util::RwLock; use byteorder::{BigEndian, ByteOrder}; -use consensus::{self, VerifySortOrder}; +use consensus; use core::hash::Hashed; use core::verifier_cache::VerifierCache; use core::{committed, Committed}; use keychain::{self, BlindingFactor}; -use ser::{self, read_multi, FixedLength, PMMRable, Readable, Reader, Writeable, Writer}; +use ser::{self, read_multi, FixedLength, PMMRable, Readable, Reader, VerifySortOrder, Writeable, Writer}; use util; use util::secp; use util::secp::pedersen::{Commitment, RangeProof}; @@ -58,8 +58,6 @@ pub enum Error { KernelSumMismatch, /// Restrict tx total weight. TooHeavy, - /// Underlying consensus error (currently for sort order) - ConsensusError(consensus::Error), /// Error originating from an invalid lock-height LockHeight(u64), /// Range proof validation error @@ -85,6 +83,8 @@ pub enum Error { InvalidKernelFeatures, /// Signature verification error. IncorrectSignature, + /// Underlying serialization error. + Serialization(ser::Error), } impl error::Error for Error { @@ -103,15 +103,15 @@ impl fmt::Display for Error { } } -impl From for Error { - fn from(e: secp::Error) -> Error { - Error::Secp(e) +impl From for Error { + fn from(e: ser::Error) -> Error { + Error::Serialization(e) } } -impl From for Error { - fn from(e: consensus::Error) -> Error { - Error::ConsensusError(e) +impl From for Error { + fn from(e: secp::Error) -> Error { + Error::Secp(e) } } @@ -561,11 +561,12 @@ impl TransactionBody { Ok(()) } - // Verify that inputs|outputs|kernels are all sorted in lexicographical order. + // Verify that inputs|outputs|kernels are sorted in lexicographical order + // and that there are no duplicates (they are all unique within this transaction). fn verify_sorted(&self) -> Result<(), Error> { - self.inputs.verify_sort_order()?; - self.outputs.verify_sort_order()?; - self.kernels.verify_sort_order()?; + self.inputs.verify_sorted_and_unique()?; + self.outputs.verify_sorted_and_unique()?; + self.kernels.verify_sorted_and_unique()?; Ok(()) } diff --git a/core/src/ser.rs b/core/src/ser.rs index 72a4fb62f..9f87e6d8e 100644 --- a/core/src/ser.rs +++ b/core/src/ser.rs @@ -22,7 +22,6 @@ use std::time::Duration; use byteorder::{BigEndian, ByteOrder, ReadBytesExt}; -use consensus; use core::hash::{Hash, Hashed}; use keychain::{BlindingFactor, Identifier, IDENTIFIER_SIZE}; use std::fmt::Debug; @@ -54,10 +53,12 @@ pub enum Error { CountError, /// When asked to read too much data TooLargeReadErr, - /// Consensus rule failure (currently sort order) - ConsensusError(consensus::Error), /// Error from from_hex deserialization HexError(String), + /// Inputs/outputs/kernels must be sorted lexicographically. + SortError, + /// Inputs/outputs/kernels must be unique. + DuplicateError, } impl From for Error { @@ -66,12 +67,6 @@ impl From for Error { } } -impl From for Error { - fn from(e: consensus::Error) -> Error { - Error::ConsensusError(e) - } -} - impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { @@ -82,8 +77,9 @@ impl fmt::Display for Error { } => write!(f, "expected {:?}, got {:?}", e, r), Error::CorruptedData => f.write_str("corrupted data"), Error::CountError => f.write_str("count error"), + Error::SortError => f.write_str("sort order"), + Error::DuplicateError => f.write_str("duplicate"), Error::TooLargeReadErr => f.write_str("too large read"), - Error::ConsensusError(ref e) => write!(f, "consensus error {:?}", e), Error::HexError(ref e) => write!(f, "hex error {:?}", e), } } @@ -103,8 +99,9 @@ impl error::Error for Error { Error::UnexpectedData { .. } => "unexpected data", Error::CorruptedData => "corrupted data", Error::CountError => "count error", + Error::SortError => "sort order", + Error::DuplicateError => "duplicate error", Error::TooLargeReadErr => "too large read", - Error::ConsensusError(_) => "consensus error (sort order)", Error::HexError(_) => "hex error", } } @@ -539,6 +536,30 @@ impl FixedLength for Signature { const LEN: usize = AGG_SIGNATURE_SIZE; } +/// Collections of items must be sorted lexicographically and all unique. +pub trait VerifySortedAndUnique { + /// Verify a collection of items is sorted and all unique. + fn verify_sorted_and_unique(&self) -> Result<(), Error>; +} + +impl VerifySortedAndUnique for Vec { + fn verify_sorted_and_unique(&self) -> Result<(), Error> { + let hashes = self + .iter() + .map(|item| item.hash()) + .collect::>(); + let pairs = hashes.windows(2); + for pair in pairs { + if pair[0] > pair[1] { + return Err(Error::SortError); + } else if pair[0] == pair[1] { + return Err(Error::DuplicateError); + } + } + Ok(()) + } +} + /// Utility wrapper for an underlying byte Writer. Defines higher level methods /// to write numbers, byte vectors, hashes, etc. pub struct BinWriter<'a> { diff --git a/pool/tests/transaction_pool.rs b/pool/tests/transaction_pool.rs index b9526328b..94211e872 100644 --- a/pool/tests/transaction_pool.rs +++ b/pool/tests/transaction_pool.rs @@ -80,6 +80,18 @@ fn test_the_transaction_pool() { assert_eq!(write_pool.total_size(), 1); } + // Test adding a tx that "double spends" an output currently spent by a tx + // already in the txpool. In this case we attempt to spend the original coinbase twice. + { + let tx = test_transaction_spending_coinbase(&keychain, &header, vec![501]); + let mut write_pool = pool.write(); + assert!( + write_pool + .add_to_pool(test_source(), tx, true, &header) + .is_err() + ); + } + // tx1 spends some outputs from the initial test tx. let tx1 = test_transaction(&keychain, vec![500, 600], vec![499, 599]); // tx2 spends some outputs from both tx1 and the initial test tx. @@ -119,8 +131,8 @@ fn test_the_transaction_pool() { ); } - // Test adding a duplicate tx with the same input and outputs (not the *same* - // tx). + // Test adding a duplicate tx with the same input and outputs. + // Note: not the *same* tx, just same underlying inputs/outputs. { let tx1a = test_transaction(&keychain, vec![500, 600], vec![499, 599]); let mut write_pool = pool.write();