Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check MAX_BLOCK_SIGOPS in the block verifier #3049

Merged
merged 12 commits into from
Nov 15, 2021
Merged
40 changes: 1 addition & 39 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion zebra-chain/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub mod merkle;

#[cfg(any(test, feature = "proptest-impl"))]
pub mod arbitrary;
#[cfg(any(test, feature = "bench"))]
#[cfg(any(test, feature = "bench", feature = "proptest-impl"))]
pub mod tests;

use std::{collections::HashMap, convert::TryInto, fmt, ops::Neg};
Expand Down
2 changes: 1 addition & 1 deletion zebra-chain/src/block/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Tests for Zebra blocks

// XXX generate should be rewritten as strategies
#[cfg(any(test, feature = "bench"))]
#[cfg(any(test, feature = "bench", feature = "proptest-impl"))]
pub mod generate;
#[cfg(test)]
mod preallocate;
Expand Down
28 changes: 26 additions & 2 deletions zebra-consensus/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ pub enum VerifyBlockError {
Transaction(#[from] TransactionError),
}

/// The maximum allowed number of legacy signature check operations in a block.
///
/// This consensus rule is not documented, so Zebra follows the `zcashd` implementation.
/// We re-use some `zcashd` C++ script code via `zebra-script` and `zcash_script`.
///
/// See:
/// https://github.com/zcash/zcash/blob/bad7f7eadbbb3466bebe3354266c7f69f607fcfd/src/consensus/consensus.h#L30
pub const MAX_BLOCK_SIGOPS: u64 = 20_000;

impl<S, V> BlockVerifier<S, V>
where
S: Service<zs::Request, Response = zs::Response, Error = BoxError> + Send + Clone + 'static,
Expand Down Expand Up @@ -119,7 +128,6 @@ where
// We don't include the block hash, because it's likely already in a parent span
let span = tracing::debug_span!("block", height = ?block.coinbase_height());

// TODO(jlusby): Error = Report, handle errors from state_service.
async move {
let hash = block.hash();
// Check that this block is actually a new block.
Expand All @@ -143,6 +151,9 @@ where
let height = block
.coinbase_height()
.ok_or(BlockError::MissingHeight(hash))?;

// TODO: support block heights up to u32::MAX (#1113)
// In practice, these blocks are invalid anyway, because their parent block doesn't exist.
if height > block::Height::MAX {
Err(BlockError::MaxHeight(height, hash, block::Height::MAX))?;
}
Expand Down Expand Up @@ -197,12 +208,25 @@ where
}
tracing::trace!(len = async_checks.len(), "built async tx checks");

let mut legacy_sigop_count = 0;

use futures::StreamExt;
while let Some(result) = async_checks.next().await {
tracing::trace!(?result, remaining = async_checks.len());
result
let response = result
.map_err(Into::into)
.map_err(VerifyBlockError::Transaction)?;
legacy_sigop_count += response
.legacy_sigop_count()
.expect("block transaction responses must have a legacy sigop count");
}

if legacy_sigop_count > MAX_BLOCK_SIGOPS {
Err(BlockError::TooManyTransparentSignatureOperations {
height,
hash,
legacy_sigop_count,
})?;
}

let new_outputs = Arc::try_unwrap(known_utxos)
Expand Down
73 changes: 64 additions & 9 deletions zebra-consensus/src/block/tests.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
//! Tests for block verification

use crate::{
parameters::{SLOW_START_INTERVAL, SLOW_START_SHIFT},
script,
};

use super::*;

use std::sync::Arc;

use chrono::Utc;
Expand All @@ -15,15 +8,25 @@ use once_cell::sync::Lazy;
use tower::{buffer::Buffer, util::BoxService};

use zebra_chain::{
block::{self, Block, Height},
block::{
self,
tests::generate::{large_multi_transaction_block, large_single_transaction_block},
Block, Height,
},
parameters::{Network, NetworkUpgrade},
serialization::{ZcashDeserialize, ZcashDeserializeInto},
transaction::{arbitrary::transaction_to_fake_v5, Transaction},
work::difficulty::{ExpandedDifficulty, INVALID_COMPACT_DIFFICULTY},
};
use zebra_script::CachedFfiTransaction;
use zebra_test::transcript::{ExpectedTranscriptError, Transcript};

use crate::transaction;
use crate::{
parameters::{SLOW_START_INTERVAL, SLOW_START_SHIFT},
script, transaction,
};

use super::*;

static VALID_BLOCK_TRANSCRIPT: Lazy<
Vec<(Arc<Block>, Result<block::Hash, ExpectedTranscriptError>)>,
Expand Down Expand Up @@ -606,3 +609,55 @@ fn merkle_root_fake_v5_for_network(network: Network) -> Result<(), Report> {

Ok(())
}

#[test]
fn legacy_sigops_count_for_large_generated_blocks() {
zebra_test::init();

// We can't test sigops using the transaction verifier, because it looks up UTXOs.

let block = large_single_transaction_block();
let mut legacy_sigop_count = 0;
for transaction in block.transactions {
let cached_ffi_transaction = Arc::new(CachedFfiTransaction::new(transaction.clone()));
let tx_sigop_count = cached_ffi_transaction.legacy_sigop_count();
assert_eq!(tx_sigop_count, Ok(0));
legacy_sigop_count += tx_sigop_count.expect("unexpected invalid sigop count");
}
// We know this block has no sigops.
assert_eq!(legacy_sigop_count, 0);

let block = large_multi_transaction_block();
let mut legacy_sigop_count = 0;
for transaction in block.transactions {
let cached_ffi_transaction = Arc::new(CachedFfiTransaction::new(transaction.clone()));
let tx_sigop_count = cached_ffi_transaction.legacy_sigop_count();
assert_eq!(tx_sigop_count, Ok(1));
legacy_sigop_count += tx_sigop_count.expect("unexpected invalid sigop count");
}
// Test that large blocks can actually fail the sigops check.
assert!(legacy_sigop_count > MAX_BLOCK_SIGOPS);
}

#[test]
fn legacy_sigops_count_for_historic_blocks() {
zebra_test::init();

// We can't test sigops using the transaction verifier, because it looks up UTXOs.

for block in zebra_test::vectors::BLOCKS.iter() {
let mut legacy_sigop_count = 0;

let block: Block = block
.zcash_deserialize_into()
.expect("block test vector is valid");
for transaction in block.transactions {
let cached_ffi_transaction = Arc::new(CachedFfiTransaction::new(transaction.clone()));
legacy_sigop_count += cached_ffi_transaction
.legacy_sigop_count()
.expect("unexpected invalid sigop count");
}
// Test that historic blocks pass the sigops check.
assert!(legacy_sigop_count <= MAX_BLOCK_SIGOPS);
}
}
12 changes: 11 additions & 1 deletion zebra-consensus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use thiserror::Error;

use zebra_chain::{orchard, sapling, sprout, transparent};

use crate::BoxError;
use crate::{block::MAX_BLOCK_SIGOPS, BoxError};

#[cfg(any(test, feature = "proptest-impl"))]
use proptest_derive::Arbitrary;
Expand Down Expand Up @@ -208,4 +208,14 @@ pub enum BlockError {

#[error("transaction has wrong consensus branch id for block network upgrade")]
WrongTransactionConsensusBranchId,

#[error(
"block {height:?} {hash:?} has {legacy_sigop_count} legacy transparent signature operations, \
but the limit is {MAX_BLOCK_SIGOPS}"
)]
TooManyTransparentSignatureOperations {
height: zebra_chain::block::Height,
hash: zebra_chain::block::Hash,
legacy_sigop_count: u64,
},
}
Loading