From 983a25248b4a03058a4dc4c19bba6850429b17d5 Mon Sep 17 00:00:00 2001
From: Antioch Peverell <30642645+antiochp@users.noreply.github.com>
Date: Fri, 10 Aug 2018 13:57:47 +0100
Subject: [PATCH] call tx.validate() as part of tx.read() (#1334)

and move some validation out of read() and into validate()
---
 core/src/consensus.rs        | 18 ++++++++------
 core/src/core/transaction.rs | 46 +++++++++++++++++++++++++-----------
 p2p/src/msg.rs               |  4 ++--
 3 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/core/src/consensus.rs b/core/src/consensus.rs
index 749c7b88..0c8800f4 100644
--- a/core/src/consensus.rs
+++ b/core/src/consensus.rs
@@ -88,18 +88,22 @@ pub const BLOCK_KERNEL_WEIGHT: usize = 2;
 /// Total maximum block weight
 pub const MAX_BLOCK_WEIGHT: usize = 80_000;
 
+/// Reused consistently for various max lengths below.
+/// Max transaction is effectively a full block of data.
+const MAX_INP_OUT_KERN_LEN: usize = 300_000;
+
 /// Maximum inputs for a block (issue#261)
 /// Hundreds of inputs + 1 output might be slow to validate (issue#258)
-pub const MAX_BLOCK_INPUTS: usize = 300_000; // soft fork down when too_high
+pub const MAX_BLOCK_INPUTS: usize = MAX_INP_OUT_KERN_LEN; // soft fork down when too_high
 
-/// Maximum inputs for a transaction
-pub const MAX_TX_INPUTS: u64 = 2048;
+/// Maximum inputs in a transaction.
+pub const MAX_TX_INPUTS: usize = MAX_INP_OUT_KERN_LEN;
 
-/// Maximum outputs for a transaction
-pub const MAX_TX_OUTPUTS: u64 = 500; // wallet uses 500 as max
+/// Maximum outputs in a transaction.
+pub const MAX_TX_OUTPUTS: usize = MAX_INP_OUT_KERN_LEN;
 
-/// Maximum kernels for a transaction
-pub const MAX_TX_KERNELS: u64 = 2048;
+/// Maximum kernels in a transaction.
+pub const MAX_TX_KERNELS: usize = MAX_INP_OUT_KERN_LEN;
 
 /// Whether a block exceeds the maximum acceptable weight
 pub fn exceeds_weight(input_len: usize, output_len: usize, kernel_len: usize) -> bool {
diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs
index a4cc978c..cc73e47c 100644
--- a/core/src/core/transaction.rs
+++ b/core/src/core/transaction.rs
@@ -54,8 +54,12 @@ pub enum Error {
 	/// The sum of output minus input commitments does not
 	/// match the sum of kernel commitments
 	KernelSumMismatch,
-	/// Restrict number of incoming inputs
+	/// Restrict number of tx inputs.
 	TooManyInputs,
+	/// Restrict number of tx outputs.
+	TooManyOutputs,
+	/// Retrict number of tx kernels.
+	TooManyKernels,
 	/// Underlying consensus error (currently for sort order)
 	ConsensusError(consensus::Error),
 	/// Error originating from an invalid lock-height
@@ -299,23 +303,25 @@ impl Readable for Transaction {
 		let (input_len, output_len, kernel_len) =
 			ser_multiread!(reader, read_u64, read_u64, read_u64);
 
-		if input_len > consensus::MAX_TX_INPUTS
-			|| output_len > consensus::MAX_TX_OUTPUTS
-			|| kernel_len > consensus::MAX_TX_KERNELS
-		{
-			return Err(ser::Error::CorruptedData);
-		}
-
 		let inputs = read_and_verify_sorted(reader, input_len)?;
 		let outputs = read_and_verify_sorted(reader, output_len)?;
 		let kernels = read_and_verify_sorted(reader, kernel_len)?;
 
-		Ok(Transaction {
+		let tx = Transaction {
 			offset,
 			inputs,
 			outputs,
 			kernels,
-		})
+		};
+
+		// Now validate the tx.
+		// Treat any validation issues as data corruption.
+		// An example of this would be reading a tx
+		// that exceeded the allowed number of inputs.
+		tx.validate()
+			.map_err(|_| ser::Error::CorruptedData)?;
+
+		Ok(tx)
 	}
 }
 
@@ -428,20 +434,32 @@ impl Transaction {
 		Ok(())
 	}
 
+	// Verify the tx is not too big in terms of
+	// number of inputs|outputs|kernels.
+	fn verify_size(&self) -> Result<(), Error> {
+		if self.inputs.len() > consensus::MAX_TX_INPUTS {
+			return Err(Error::TooManyInputs);
+		}
+		if self.outputs.len() > consensus::MAX_TX_OUTPUTS {
+			return Err(Error::TooManyOutputs);
+		}
+		if self.kernels.len() > consensus::MAX_TX_KERNELS {
+			return Err(Error::TooManyKernels);
+		}
+		Ok(())
+	}
+
 	/// Validates all relevant parts of a fully built transaction. Checks the
 	/// excess value against the signature as well as range proofs for each
 	/// output.
 	pub fn validate(&self) -> Result<(), Error> {
-		if self.inputs.len() > consensus::MAX_BLOCK_INPUTS {
-			return Err(Error::TooManyInputs);
-		}
 		self.verify_features()?;
+		self.verify_size()?;
 		self.verify_sorted()?;
 		self.verify_cut_through()?;
 		self.verify_kernel_sums(self.overage(), self.offset)?;
 		self.verify_rangeproofs()?;
 		self.verify_kernel_signatures()?;
-
 		Ok(())
 	}
 
diff --git a/p2p/src/msg.rs b/p2p/src/msg.rs
index 6b836da8..4730d8c1 100644
--- a/p2p/src/msg.rs
+++ b/p2p/src/msg.rs
@@ -85,8 +85,8 @@ fn max_msg_size(msg_type: Type) -> u64 {
 		Type::Block => MAX_MSG_LEN,
 		Type::GetCompactBlock => 32,
 		Type::CompactBlock => MAX_MSG_LEN / 10,
-		Type::StemTransaction => 1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS,
-		Type::Transaction => 1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS,
+		Type::StemTransaction => (1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS) as u64,
+		Type::Transaction => (1000 * MAX_TX_INPUTS + 710 * MAX_TX_OUTPUTS + 114 * MAX_TX_KERNELS) as u64,
 		Type::TxHashSetRequest => 40,
 		Type::TxHashSetArchive => 64,
 		Type::BanReason => 64,