Fix coinbase height deserialization (#3129)

* fix parse_coinbase_height()

* move tests and create test for parse_coinbase_height()

* add a coinbase height round trip prop test

* fix range

Co-authored-by: teor <teor@riseup.net>

* extend examples in test

* add more round trip testing

* extend the range of test

Co-authored-by: teor <teor@riseup.net>

* add test for single byte

Co-authored-by: teor <teor@riseup.net>
This commit is contained in:
Alfredo Garcia 2021-12-05 19:38:02 -03:00 committed by GitHub
parent 4d608d3224
commit 9416729db8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 170 additions and 63 deletions

View File

@ -26,7 +26,7 @@ use proptest_derive::Arbitrary;
#[cfg(any(test, feature = "proptest-impl"))]
mod arbitrary;
#[cfg(test)]
mod prop;
mod tests;
use crate::{
amount::{Amount, NonNegative},

View File

@ -1,47 +0,0 @@
//! Property tests for transparent inputs and outputs.
//!
//! TODO: Move this module into a `tests` submodule.
use zebra_test::prelude::*;
use crate::{block, fmt::SummaryDebug, transaction::arbitrary::MAX_ARBITRARY_ITEMS, LedgerState};
use super::Input;
#[test]
fn coinbase_has_height() -> Result<()> {
zebra_test::init();
let strategy =
any::<block::Height>().prop_flat_map(|height| Input::arbitrary_with(Some(height)));
proptest!(|(input in strategy)| {
let is_coinbase = matches!(input, Input::Coinbase { .. });
prop_assert!(is_coinbase);
});
Ok(())
}
#[test]
fn input_coinbase_vecs_only_have_coinbase_input() -> Result<()> {
zebra_test::init();
let strategy = LedgerState::coinbase_strategy(None, None, false)
.prop_flat_map(|ledger_state| Input::vec_strategy(ledger_state, MAX_ARBITRARY_ITEMS));
proptest!(|(inputs in strategy.prop_map(SummaryDebug))| {
let len = inputs.len();
for (ind, input) in inputs.into_iter().enumerate() {
let is_coinbase = matches!(input, Input::Coinbase { .. });
if ind == 0 {
prop_assert!(is_coinbase);
prop_assert_eq!(1, len);
} else {
prop_assert!(!is_coinbase);
}
}
});
Ok(())
}

View File

@ -65,7 +65,7 @@ impl ZcashDeserialize for OutPoint {
///
/// The height may consume `0..=5` bytes at the stat of the coinbase data.
/// The genesis block does not include an encoded coinbase height.
fn parse_coinbase_height(
pub(crate) fn parse_coinbase_height(
mut data: Vec<u8>,
) -> Result<(block::Height, CoinbaseData), SerializationError> {
use block::Height;
@ -78,20 +78,33 @@ fn parse_coinbase_height(
// Blocks 17 through 128 exclusive encode block height with the `0x01` opcode.
// The Bitcoin encoding requires that the most significant byte is below 0x80.
(Some(0x01), len) if len >= 2 && data[1] < 0x80 => {
Ok((Height(data[1] as u32), CoinbaseData(data.split_off(2))))
let h = data[1] as u32;
if (17..128).contains(&h) {
Ok((Height(h), CoinbaseData(data.split_off(2))))
} else {
Err(SerializationError::Parse("Invalid block height"))
}
}
// Blocks 128 through 32768 exclusive encode block height with the `0x02` opcode.
// The Bitcoin encoding requires that the most significant byte is below 0x80.
(Some(0x02), len) if len >= 3 && data[2] < 0x80 => Ok((
Height(data[1] as u32 + ((data[2] as u32) << 8)),
CoinbaseData(data.split_off(3)),
)),
// Blocks 65536 through 2**23 exclusive encode block height with the `0x03` opcode.
(Some(0x02), len) if len >= 3 && data[2] < 0x80 => {
let h = data[1] as u32 + ((data[2] as u32) << 8);
if (128..32_768).contains(&h) {
Ok((Height(h), CoinbaseData(data.split_off(3))))
} else {
Err(SerializationError::Parse("Invalid block height"))
}
}
// Blocks 32768 through 2**23 exclusive encode block height with the `0x03` opcode.
// The Bitcoin encoding requires that the most significant byte is below 0x80.
(Some(0x03), len) if len >= 4 && data[3] < 0x80 => Ok((
Height(data[1] as u32 + ((data[2] as u32) << 8) + ((data[3] as u32) << 16)),
CoinbaseData(data.split_off(4)),
)),
(Some(0x03), len) if len >= 4 && data[3] < 0x80 => {
let h = data[1] as u32 + ((data[2] as u32) << 8) + ((data[3] as u32) << 16);
if (32_768..8_388_608).contains(&h) {
Ok((Height(h), CoinbaseData(data.split_off(4))))
} else {
Err(SerializationError::Parse("Invalid block height"))
}
}
// The genesis block does not encode the block height by mistake; special case it.
// The first five bytes are [4, 255, 255, 7, 31], the little-endian encoding of
// 520_617_983.
@ -112,7 +125,7 @@ fn parse_coinbase_height(
+ ((data[2] as u32) << 8)
+ ((data[3] as u32) << 16)
+ ((data[4] as u32) << 24);
if h <= Height::MAX.0 {
if (8_388_608..=Height::MAX.0).contains(&h) {
Ok((Height(h), CoinbaseData(data.split_off(5))))
} else {
Err(SerializationError::Parse("Invalid block height"))
@ -137,7 +150,7 @@ fn parse_coinbase_height(
///
/// This check is required, because the genesis block does not include an encoded
/// coinbase height,
fn write_coinbase_height<W: io::Write>(
pub(crate) fn write_coinbase_height<W: io::Write>(
height: block::Height,
coinbase_data: &CoinbaseData,
mut w: W,
@ -164,10 +177,10 @@ fn write_coinbase_height<W: io::Write>(
} else if let h @ 17..=127 = height.0 {
w.write_u8(0x01)?;
w.write_u8(h as u8)?;
} else if let h @ 128..=32767 = height.0 {
} else if let h @ 128..=32_767 = height.0 {
w.write_u8(0x02)?;
w.write_u16::<LittleEndian>(h as u16)?;
} else if let h @ 32768..=8_388_607 = height.0 {
} else if let h @ 32_768..=8_388_607 = height.0 {
w.write_u8(0x03)?;
w.write_u8(h as u8)?;
w.write_u8((h >> 8) as u8)?;

View File

@ -0,0 +1,2 @@
mod prop;
mod vectors;

View File

@ -0,0 +1,100 @@
//! Property tests for transparent inputs and outputs.
use zebra_test::prelude::*;
use crate::{block, fmt::SummaryDebug, transaction::arbitrary::MAX_ARBITRARY_ITEMS, LedgerState};
use super::super::{
serialize::{parse_coinbase_height, write_coinbase_height},
Input,
};
use proptest::collection::vec;
#[test]
fn coinbase_has_height() -> Result<()> {
zebra_test::init();
let strategy =
any::<block::Height>().prop_flat_map(|height| Input::arbitrary_with(Some(height)));
proptest!(|(input in strategy)| {
let is_coinbase = matches!(input, Input::Coinbase { .. });
prop_assert!(is_coinbase);
});
Ok(())
}
#[test]
fn input_coinbase_vecs_only_have_coinbase_input() -> Result<()> {
zebra_test::init();
let strategy = LedgerState::coinbase_strategy(None, None, false)
.prop_flat_map(|ledger_state| Input::vec_strategy(ledger_state, MAX_ARBITRARY_ITEMS));
proptest!(|(inputs in strategy.prop_map(SummaryDebug))| {
let len = inputs.len();
for (ind, input) in inputs.into_iter().enumerate() {
let is_coinbase = matches!(input, Input::Coinbase { .. });
if ind == 0 {
prop_assert!(is_coinbase);
prop_assert_eq!(1, len);
} else {
prop_assert!(!is_coinbase);
}
}
});
Ok(())
}
#[test]
fn coinbase_height_round_trip_from_random_input() -> Result<()> {
zebra_test::init();
let strategy =
any::<block::Height>().prop_flat_map(|height| Input::arbitrary_with(Some(height)));
proptest!(|(input in strategy)| {
let (height, data) = match input {
Input::Coinbase { height, data, .. } => (height, data),
_ => unreachable!("all inputs will have coinbase height and data"),
};
let mut encoded = Vec::new();
write_coinbase_height(height, &data, &mut encoded)?;
let decoded = parse_coinbase_height(encoded)?;
prop_assert_eq!(height, decoded.0);
});
Ok(())
}
proptest! {
#[test]
fn coinbase_height_round_trip_from_random_bytes(mut height_bytes in vec(any::<u8>(), 1..5)) {
let mut encoded1 = vec![height_bytes.len() as u8];
encoded1.append(&mut height_bytes);
let decoded = parse_coinbase_height(encoded1.clone()).ok();
if decoded.is_some() {
let mut encoded2 = Vec::new();
write_coinbase_height(decoded.as_ref().unwrap().0, &decoded.unwrap().1, &mut encoded2)?;
prop_assert_eq!(encoded2, encoded1);
}
}
#[test]
fn coinbase_height_round_trip_from_random_byte(height_byte in vec(any::<u8>(), 1..2)) {
let encoded1 = height_byte;
let decoded = parse_coinbase_height(encoded1.clone()).ok();
if decoded.is_some() {
let mut encoded2 = Vec::new();
write_coinbase_height(decoded.as_ref().unwrap().0, &decoded.unwrap().1, &mut encoded2)?;
prop_assert_eq!(encoded2, encoded1);
}
}
}

View File

@ -0,0 +1,39 @@
use super::super::serialize::parse_coinbase_height;
#[test]
fn parse_coinbase_height_mins() {
zebra_test::init();
// examples with height 1:
let case1 = vec![0x51];
assert!(!parse_coinbase_height(case1.clone()).is_err());
assert_eq!(parse_coinbase_height(case1).unwrap().0 .0, 1);
let case2 = vec![0x01, 0x01];
assert!(parse_coinbase_height(case2).is_err());
let case3 = vec![0x02, 0x01, 0x00];
assert!(parse_coinbase_height(case3).is_err());
let case4 = vec![0x03, 0x01, 0x00, 0x00];
assert!(parse_coinbase_height(case4).is_err());
let case5 = vec![0x04, 0x01, 0x00, 0x00, 0x00];
assert!(parse_coinbase_height(case5).is_err());
// examples with height 17:
let case1 = vec![0x01, 0x11];
assert!(!parse_coinbase_height(case1.clone()).is_err());
assert_eq!(parse_coinbase_height(case1).unwrap().0 .0, 17);
let case2 = vec![0x02, 0x11, 0x00];
assert!(parse_coinbase_height(case2).is_err());
let case3 = vec![0x03, 0x11, 0x00, 0x00];
assert!(parse_coinbase_height(case3).is_err());
let case4 = vec![0x04, 0x11, 0x00, 0x00, 0x00];
assert!(parse_coinbase_height(case4).is_err());
}