From 5ee11046a5eb15919f38f701e398eea315099934 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Fri, 10 Aug 2018 11:45:14 +0100 Subject: [PATCH 1/2] rework block validation to check input|output|kernel sizes consistently with tx validation --- core/src/consensus.rs | 23 +++++++++++++++++------ core/src/core/block.rs | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 0c8800f4d..118649506 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -90,11 +90,20 @@ pub const MAX_BLOCK_WEIGHT: usize = 80_000; /// Reused consistently for various max lengths below. /// Max transaction is effectively a full block of data. +/// Soft fork down when too high. +/// Likely we will need to tweak these all individually, but using a single constant for now. 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 = MAX_INP_OUT_KERN_LEN; // soft fork down when too_high +/// Maximum inputs for a block. +pub const MAX_BLOCK_INPUTS: usize = MAX_INP_OUT_KERN_LEN; + +/// Maximum outputs for a block (max tx + single output for coinbase). +/// This is just a starting point - need to discuss this further. +pub const MAX_BLOCK_OUTPUTS: usize = MAX_INP_OUT_KERN_LEN + 1; + +/// Maximum kernels for a block (max tx + single output for coinbase). +/// This is just a starting point - need to discuss this further. +pub const MAX_BLOCK_KERNELS: usize = MAX_INP_OUT_KERN_LEN + 1; /// Maximum inputs in a transaction. pub const MAX_TX_INPUTS: usize = MAX_INP_OUT_KERN_LEN; @@ -107,9 +116,11 @@ 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 { - input_len * BLOCK_INPUT_WEIGHT - + output_len * BLOCK_OUTPUT_WEIGHT - + kernel_len * BLOCK_KERNEL_WEIGHT > MAX_BLOCK_WEIGHT || input_len > MAX_BLOCK_INPUTS + let weight = + input_len * BLOCK_INPUT_WEIGHT + + output_len * BLOCK_OUTPUT_WEIGHT + + kernel_len * BLOCK_KERNEL_WEIGHT; + weight > MAX_BLOCK_WEIGHT } /// Fork every 250,000 blocks for first 2 years, simple number and just a diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 72acfcec1..8413f045c 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -45,7 +45,13 @@ pub enum Error { InvalidTotalKernelSum, /// Same as above but for the coinbase part of a block, including reward CoinbaseSumMismatch, - /// Too many inputs, outputs or kernels in the block + /// Restrict number of block inputs. + TooManyInputs, + /// Restrict number of block outputs. + TooManyOutputs, + /// Retrict number of block kernels. + TooManyKernels, + /// Block weight (based on inputs|outputs|kernels) exceeded. WeightExceeded, /// Kernel not valid due to lock_height exceeding block header height KernelLockHeight(u64), @@ -399,6 +405,13 @@ impl Readable for Block { let outputs = read_and_verify_sorted(reader, output_len)?; let kernels = read_and_verify_sorted(reader, kernel_len)?; + // TODO - we do not verify the input|output|kernel counts here. + // I think should call block.validate() as part of a call to read() + // but block.validate() as it stands currently requires the previous sums etc. + // So there is no easy way to do this in isolation. + // Maybe we need two variations of validate() where one handles the validation + // rules that *can* be done in isolation. + Ok(Block { header: header, inputs: inputs, @@ -694,7 +707,11 @@ impl Block { prev_kernel_offset: &BlindingFactor, prev_kernel_sum: &Commitment, ) -> Result<(Commitment), Error> { + // Verify we do not exceed the max number of inputs|outputs|kernels + // and that the "weight" based on these does not exceed the max permitted weight. + self.verify_size()?; self.verify_weight()?; + self.verify_sorted()?; self.verify_cut_through()?; self.verify_coinbase()?; @@ -727,6 +744,21 @@ impl Block { Ok(kernel_sum) } + // 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_BLOCK_INPUTS { + return Err(Error::TooManyInputs); + } + if self.outputs.len() > consensus::MAX_BLOCK_OUTPUTS { + return Err(Error::TooManyOutputs); + } + if self.kernels.len() > consensus::MAX_BLOCK_KERNELS { + return Err(Error::TooManyKernels); + } + Ok(()) + } + fn verify_weight(&self) -> Result<(), Error> { if exceeds_weight(self.inputs.len(), self.outputs.len(), self.kernels.len()) { return Err(Error::WeightExceeded); From fb30b02f997144532686af746f824f7b39085344 Mon Sep 17 00:00:00 2001 From: antiochp <30642645+antiochp@users.noreply.github.com> Date: Fri, 10 Aug 2018 14:15:24 +0100 Subject: [PATCH 2/2] cleanup verify_weight code --- core/src/consensus.rs | 9 --------- core/src/core/block.rs | 9 +++++++-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 118649506..21ca470f9 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -114,15 +114,6 @@ pub const MAX_TX_OUTPUTS: usize = MAX_INP_OUT_KERN_LEN; /// 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 { - let weight = - input_len * BLOCK_INPUT_WEIGHT + - output_len * BLOCK_OUTPUT_WEIGHT + - kernel_len * BLOCK_KERNEL_WEIGHT; - weight > MAX_BLOCK_WEIGHT -} - /// Fork every 250,000 blocks for first 2 years, simple number and just a /// little less than 6 months. pub const HARD_FORK_INTERVAL: u64 = 250_000; diff --git a/core/src/core/block.rs b/core/src/core/block.rs index 8413f045c..a1dbf240f 100644 --- a/core/src/core/block.rs +++ b/core/src/core/block.rs @@ -21,7 +21,7 @@ use std::collections::HashSet; use std::fmt; use std::iter::FromIterator; -use consensus::{self, exceeds_weight, reward, VerifySortOrder, REWARD}; +use consensus::{self, reward, VerifySortOrder, REWARD}; use core::committed::{self, Committed}; use core::hash::{Hash, HashWriter, Hashed, ZERO_HASH}; use core::id::ShortIdentifiable; @@ -760,7 +760,12 @@ impl Block { } fn verify_weight(&self) -> Result<(), Error> { - if exceeds_weight(self.inputs.len(), self.outputs.len(), self.kernels.len()) { + let weight = + self.inputs.len() * consensus::BLOCK_INPUT_WEIGHT + + self.outputs.len() * consensus::BLOCK_OUTPUT_WEIGHT + + self.kernels.len() * consensus::BLOCK_KERNEL_WEIGHT; + + if weight > consensus::MAX_BLOCK_WEIGHT { return Err(Error::WeightExceeded); } Ok(())