From c342adbb1de1adc3dfc52f1b1cf8116f1d37b049 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Tue, 5 Oct 2021 15:07:55 +0200 Subject: [PATCH 1/5] Introduce Witness struct mainly to improve ser/de performance Witness struct is in place of the Vec> we have before this commit. Implementation of Default, Iterator and others allows to have similar behaviour but using a single Vec prevent many allocations during deserialization which in turns results in better performance, even 20% better perfomance on recent block. --- src/blockdata/block.rs | 5 +- src/blockdata/constants.rs | 3 +- src/blockdata/mod.rs | 1 + src/blockdata/transaction.rs | 22 +-- src/blockdata/witness.rs | 328 +++++++++++++++++++++++++++++++++++ src/util/bip143.rs | 7 +- src/util/psbt/mod.rs | 22 +-- src/util/sighash.rs | 7 +- 8 files changed, 363 insertions(+), 32 deletions(-) create mode 100644 src/blockdata/witness.rs diff --git a/src/blockdata/block.rs b/src/blockdata/block.rs index da5b6772..de8c6770 100644 --- a/src/blockdata/block.rs +++ b/src/blockdata/block.rs @@ -199,9 +199,10 @@ impl Block { o.script_pubkey[0..6] == [0x6a, 0x24, 0xaa, 0x21, 0xa9, 0xed] }) { let commitment = WitnessCommitment::from_slice(&coinbase.output[pos].script_pubkey.as_bytes()[6..38]).unwrap(); // witness reserved value is in coinbase input witness - if coinbase.input[0].witness.len() == 1 && coinbase.input[0].witness[0].len() == 32 { + let witness_vec: Vec<_> = coinbase.input[0].witness.iter().collect(); + if witness_vec.len() == 1 && witness_vec[0].len() == 32 { match self.witness_root() { - Some(witness_root) => return commitment == Self::compute_witness_commitment(&witness_root, coinbase.input[0].witness[0].as_slice()), + Some(witness_root) => return commitment == Self::compute_witness_commitment(&witness_root, witness_vec[0]), None => return false, } } diff --git a/src/blockdata/constants.rs b/src/blockdata/constants.rs index bdf8e2ca..72ceb940 100644 --- a/src/blockdata/constants.rs +++ b/src/blockdata/constants.rs @@ -29,6 +29,7 @@ use blockdata::opcodes; use blockdata::script; use blockdata::transaction::{OutPoint, Transaction, TxOut, TxIn}; use blockdata::block::{Block, BlockHeader}; +use blockdata::witness::Witness; use network::constants::Network; use util::uint::Uint256; @@ -93,7 +94,7 @@ fn bitcoin_genesis_tx() -> Transaction { previous_output: OutPoint::null(), script_sig: in_script, sequence: MAX_SEQUENCE, - witness: vec![], + witness: Witness::default(), }); // Outputs diff --git a/src/blockdata/mod.rs b/src/blockdata/mod.rs index 58cec8e7..48f80ef8 100644 --- a/src/blockdata/mod.rs +++ b/src/blockdata/mod.rs @@ -23,4 +23,5 @@ pub mod opcodes; pub mod script; pub mod transaction; pub mod block; +pub mod witness; diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index 045e0aba..c0534754 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -36,6 +36,7 @@ use util::endian; use blockdata::constants::WITNESS_SCALE_FACTOR; #[cfg(feature="bitcoinconsensus")] use blockdata::script; use blockdata::script::Script; +use blockdata::witness::Witness; use consensus::{encode, Decodable, Encodable}; use consensus::encode::MAX_VEC_SIZE; use hash_types::{SigHash, Txid, Wtxid}; @@ -197,7 +198,7 @@ pub struct TxIn { /// Encodable/Decodable, as it is (de)serialized at the end of the full /// Transaction. It *is* (de)serialized with the rest of the TxIn in other /// (de)serialization routines. - pub witness: Vec> + pub witness: Witness } impl Default for TxIn { @@ -206,7 +207,7 @@ impl Default for TxIn { previous_output: OutPoint::default(), script_sig: Script::new(), sequence: u32::max_value(), - witness: Vec::new(), + witness: Witness::default(), } } } @@ -280,7 +281,7 @@ impl Transaction { let cloned_tx = Transaction { version: self.version, lock_time: self.lock_time, - input: self.input.iter().map(|txin| TxIn { script_sig: Script::new(), witness: vec![], .. *txin }).collect(), + input: self.input.iter().map(|txin| TxIn { script_sig: Script::new(), witness: Witness::default(), .. *txin }).collect(), output: self.output.clone(), }; cloned_tx.txid().into() @@ -357,7 +358,7 @@ impl Transaction { previous_output: self.input[input_index].previous_output, script_sig: script_pubkey.clone(), sequence: self.input[input_index].sequence, - witness: vec![], + witness: Witness::default(), }]; } else { tx.input = Vec::with_capacity(self.input.len()); @@ -366,7 +367,7 @@ impl Transaction { previous_output: input.previous_output, script_sig: if n == input_index { script_pubkey.clone() } else { Script::new() }, sequence: if n != input_index && (sighash == EcdsaSigHashType::Single || sighash == EcdsaSigHashType::None) { 0 } else { input.sequence }, - witness: vec![], + witness: Witness::default(), }); } } @@ -473,10 +474,7 @@ impl Transaction { input.script_sig.len()); if !input.witness.is_empty() { inputs_with_witnesses += 1; - input_weight += VarInt(input.witness.len() as u64).len(); - for elem in &input.witness { - input_weight += VarInt(elem.len() as u64).len() + elem.len(); - } + input_weight += input.witness.serialized_len(); } } let mut output_size = 0; @@ -578,7 +576,7 @@ impl Decodable for TxIn { previous_output: Decodable::consensus_decode(&mut d)?, script_sig: Decodable::consensus_decode(&mut d)?, sequence: Decodable::consensus_decode(d)?, - witness: vec![], + witness: Witness::default(), }) } } @@ -1471,7 +1469,9 @@ mod tests { }).is_err()); // test that we get a failure if we corrupt a signature - spending.input[1].witness[0][10] = 42; + let mut witness: Vec<_> = spending.input[1].witness.iter().map(|el| el.to_vec()).collect(); + witness[0][10] = 42; + spending.input[1].witness = witness.into(); match spending.verify(|point: &OutPoint| { if let Some(tx) = spent3.remove(&point.txid) { return tx.output.get(point.vout as usize).cloned(); diff --git a/src/blockdata/witness.rs b/src/blockdata/witness.rs new file mode 100644 index 00000000..3334eefe --- /dev/null +++ b/src/blockdata/witness.rs @@ -0,0 +1,328 @@ +//! Witness +//! +//! This module contains the [`Witness`] struct and relative methods to operate on it +//! + +use consensus::encode::{Error, MAX_VEC_SIZE}; +use consensus::{Decodable, Encodable, WriteExt}; +use io::{self, Read, Write}; +use prelude::*; +use VarInt; + +#[cfg(feature = "serde")] use serde; + +/// The Witness is the data used to unlock bitcoins since the [segwit upgrade](https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki) +/// +/// Can be logically seen as an array of byte-arrays `Vec>` and indeed you can convert from +/// it and collect the iteration to convert into it. +/// For serialization and deserialization performance it is stored internally as a single `Vec`, +/// saving some allocations +/// +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] +pub struct Witness { + /// contains the witness Vec> serialization without the initial varint indicating the + /// number of elements (which is stored in len) + content: Vec, + + /// Number of elements in the witness. + /// It is stored separately (instead of as VarInt in the initial part of content) so that method + /// like [`Witness::push`] doesn't have case requiring to shift the entire array + witness_elements: u64, +} + +/// Support structure to allow efficient and convenient iteration over the Witness elements +pub struct WitnessIterator<'a> { + witness: &'a Witness, + cursor: usize, +} + +impl From>> for Witness { + fn from(vec: Vec>) -> Self { + let witness_elements = vec.len() as u64; + + let content_size: usize = vec + .iter() + .map(|el| el.len() + VarInt(el.len() as u64).len()) + .sum(); + let mut content = vec![0u8; content_size]; + let mut cursor = 0usize; + for el in vec { + let el_len_varint = VarInt(el.len() as u64); + el_len_varint + .consensus_encode(&mut content[cursor..cursor + el_len_varint.len()]) + .expect("writers on vec don't errors, space granted by content_size"); + cursor += el_len_varint.len(); + content[cursor..cursor + el.len()].copy_from_slice(&el); + cursor += el.len(); + } + + Witness { + witness_elements, + content, + } + } +} + +impl Decodable for Witness { + fn consensus_decode(mut d: D) -> Result { + let witness_elements = VarInt::consensus_decode(&mut d)?.0; + if witness_elements == 0 { + Ok(Witness::default()) + } else { + let mut cursor = 0usize; + + // this number should be determined as high enough to cover most witness, and low enough + // to avoid wasting space without reallocating + let mut content = vec![0u8; 128]; + + for _ in 0..witness_elements { + let element_size_varint = VarInt::consensus_decode(&mut d)?; + let element_size_varint_len = element_size_varint.len(); + let element_size = element_size_varint.0 as usize; + let required_len = cursor + .checked_add(element_size) + .ok_or_else(|| self::Error::OversizedVectorAllocation { + requested: usize::max_value(), + max: MAX_VEC_SIZE, + })? + .checked_add(element_size_varint_len) + .ok_or_else(|| self::Error::OversizedVectorAllocation { + requested: usize::max_value(), + max: MAX_VEC_SIZE, + })?; + + if required_len > MAX_VEC_SIZE { + return Err(self::Error::OversizedVectorAllocation { + requested: required_len, + max: MAX_VEC_SIZE, + }); + } + + resize_if_needed(&mut content, required_len); + element_size_varint + .consensus_encode(&mut content[cursor..cursor + element_size_varint_len])?; + cursor += element_size_varint_len; + d.read_exact(&mut content[cursor..cursor + element_size])?; + cursor += element_size; + } + content.truncate(cursor); + Ok(Witness { + content, + witness_elements, + }) + } + } +} + +fn resize_if_needed(vec: &mut Vec, required_len: usize) { + if required_len >= vec.len() { + let mut new_len = vec.len().max(1); + while new_len <= required_len { + new_len *= 2; + } + vec.resize(new_len, 0); + } +} + +impl Encodable for Witness { + fn consensus_encode(&self, mut writer: W) -> Result { + let len = VarInt(self.witness_elements); + len.consensus_encode(&mut writer)?; + writer.emit_slice(&self.content[..])?; + Ok(self.content.len() + len.len()) + } +} + +impl Witness { + /// Returns `true` if the witness contains no element + pub fn is_empty(&self) -> bool { + self.witness_elements == 0 + } + + /// Returns a struct implementing [`Iterator`] + pub fn iter(&self) -> WitnessIterator { + WitnessIterator { + witness: &self, + cursor: 0, + } + } + + /// Returns the number of elements this witness holds + pub fn len(&self) -> usize { + self.witness_elements as usize + } + + /// Returns the bytes required when this Witness is consensus encoded + pub fn serialized_len(&self) -> usize { + self.iter() + .map(|el| VarInt(el.len() as u64).len() + el.len()) + .sum::() + + VarInt(self.witness_elements).len() + } + + /// Clear the witness + pub fn clear(&mut self) { + self.content.clear(); + self.witness_elements = 0; + } + + /// Push a new element on the witness, require an allocation + pub fn push>(&mut self, new_element: T) { + let new_element = new_element.as_ref(); + self.witness_elements += 1; + let element_len_varint = VarInt(new_element.len() as u64); + let current_content_len = self.content.len(); + self.content.resize( + current_content_len + element_len_varint.len() + new_element.len(), + 0, + ); + let end_varint = current_content_len + element_len_varint.len(); + element_len_varint + .consensus_encode(&mut self.content[current_content_len..end_varint]) + .expect("writers on vec don't error, space granted through previous resize"); + self.content[end_varint..].copy_from_slice(new_element); + } +} + +impl Default for Witness { + fn default() -> Self { + // from https://doc.rust-lang.org/std/vec/struct.Vec.html#method.new + // The vector will not allocate until elements are pushed onto it. + Witness { + content: Vec::new(), + witness_elements: 0, + } + } +} + +impl<'a> Iterator for WitnessIterator<'a> { + type Item = &'a [u8]; + + fn next(&mut self) -> Option { + let vec = &self.witness.content; + if self.cursor >= vec.len() { + None + } else { + let var = VarInt::consensus_decode(&vec[self.cursor..]) + .expect("is granted witness.content contains varint because created only from internal methods"); + let start = self.cursor + var.len(); + let end = start + var.0 as usize; + self.cursor = end; + Some(&vec[start..end]) + } + } +} + +// Serde keep backward compatibility with old Vec> format +#[cfg(feature = "serde")] +impl serde::Serialize for Witness { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let vec: Vec<_> = self.iter().map(|e| e.to_vec()).collect(); + serde::Serialize::serialize(&vec, serializer) + } +} +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for Witness { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let vec: Vec> = serde::Deserialize::deserialize(deserializer)?; + Ok(vec.into()) + } +} + +#[cfg(test)] +mod test { + use blockdata::witness::Witness; + use consensus::{deserialize, serialize}; + use hashes::hex::{FromHex, ToHex}; + use Transaction; + + #[test] + fn test_push() { + let mut witness = Witness::default(); + witness.push(&vec![0u8]); + let expected = Witness { + witness_elements: 1, + content: vec![1u8, 0], + }; + assert_eq!(witness, expected); + witness.push(&vec![2u8, 3u8]); + let expected = Witness { + witness_elements: 2, + content: vec![1u8, 0, 2, 2, 3], + }; + assert_eq!(witness, expected); + } + + #[test] + fn test_witness() { + let w0 = + Vec::from_hex("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105") + .unwrap(); + let w1 = Vec::from_hex("000000").unwrap(); + let witness_vec = vec![w0, w1]; + let witness_serialized: Vec = serialize(&witness_vec); + let witness = Witness { + content: witness_serialized[1..].to_vec(), + witness_elements: 2, + }; + for (i, el) in witness.iter().enumerate() { + assert_eq!(witness_vec[i], el); + } + + let w_into: Witness = witness_vec.into(); + assert_eq!(w_into, witness); + + assert_eq!(witness_serialized, serialize(&witness)); + + //assert_eq!(32, std::mem::size_of::()); + //assert_eq!(24, std::mem::size_of::>>()); + } + + #[test] + fn test_tx() { + let s = "02000000000102b44f26b275b8ad7b81146ba3dbecd081f9c1ea0dc05b97516f56045cfcd3df030100000000ffffffff1cb4749ae827c0b75f3d0a31e63efc8c71b47b5e3634a4c698cd53661cab09170100000000ffffffff020b3a0500000000001976a9143ea74de92762212c96f4dd66c4d72a4deb20b75788ac630500000000000016001493a8dfd1f0b6a600ab01df52b138cda0b82bb7080248304502210084622878c94f4c356ce49c8e33a063ec90f6ee9c0208540888cfab056cd1fca9022014e8dbfdfa46d318c6887afd92dcfa54510e057565e091d64d2ee3a66488f82c0121026e181ffb98ebfe5a64c983073398ea4bcd1548e7b971b4c175346a25a1c12e950247304402203ef00489a0d549114977df2820fab02df75bebb374f5eee9e615107121658cfa02204751f2d1784f8e841bff6d3bcf2396af2f1a5537c0e4397224873fbd3bfbe9cf012102ae6aa498ce2dd204e9180e71b4fb1260fe3d1a95c8025b34e56a9adf5f278af200000000"; + let tx_bytes = Vec::from_hex(s).unwrap(); + let tx: Transaction = deserialize(&tx_bytes).unwrap(); + + let expected_wit = ["304502210084622878c94f4c356ce49c8e33a063ec90f6ee9c0208540888cfab056cd1fca9022014e8dbfdfa46d318c6887afd92dcfa54510e057565e091d64d2ee3a66488f82c01", "026e181ffb98ebfe5a64c983073398ea4bcd1548e7b971b4c175346a25a1c12e95"]; + for (i, wit_el) in tx.input[0].witness.iter().enumerate() { + assert_eq!(expected_wit[i], wit_el.to_hex()); + } + let tx_bytes_back = serialize(&tx); + assert_eq!(tx_bytes_back, tx_bytes); + } + + #[test] + fn fuzz_cases() { + let s = "26ff0000000000c94ce592cf7a4cbb68eb00ce374300000057cd0000000000000026"; + let bytes = Vec::from_hex(s).unwrap(); + assert!(deserialize::(&bytes).is_err()); // OversizedVectorAllocation + + let s = "24000000ffffffffffffffffffffffff"; + let bytes = Vec::from_hex(s).unwrap(); + assert!(deserialize::(&bytes).is_err()); // OversizedVectorAllocation + } + + #[cfg(feature = "serde")] + #[test] + fn test_serde() { + use serde_json; + + let old_witness_format = vec![vec![0u8], vec![2]]; + let new_witness_format: Witness = old_witness_format.clone().into(); + + let old = serde_json::to_string(&old_witness_format).unwrap(); + let new = serde_json::to_string(&new_witness_format).unwrap(); + + assert_eq!(old, new); + + let back = serde_json::from_str(&new).unwrap(); + assert_eq!(new_witness_format, back); + } +} diff --git a/src/util/bip143.rs b/src/util/bip143.rs index a00da490..434ae4e7 100644 --- a/src/util/bip143.rs +++ b/src/util/bip143.rs @@ -22,11 +22,10 @@ use hashes::Hash; use hash_types::SigHash; use blockdata::script::Script; +use blockdata::witness::Witness; use blockdata::transaction::{Transaction, TxIn, EcdsaSigHashType}; use consensus::{encode, Encodable}; -use prelude::*; - use io; use core::ops::{Deref, DerefMut}; use util::sighash; @@ -177,10 +176,10 @@ impl> SigHashCache { /// let prevout_script = Script::new(); /// let _sighash = sig_hasher.signature_hash(inp, &prevout_script, 42, EcdsaSigHashType::All); /// // ... sign the sighash - /// sig_hasher.access_witness(inp).push(Vec::new()); + /// sig_hasher.access_witness(inp).push(&[]); /// } /// ``` - pub fn access_witness(&mut self, input_index: usize) -> &mut Vec> { + pub fn access_witness(&mut self, input_index: usize) -> &mut Witness { self.cache.witness_mut(input_index).unwrap() } } diff --git a/src/util/psbt/mod.rs b/src/util/psbt/mod.rs index 85c7b82f..90e8b7ca 100644 --- a/src/util/psbt/mod.rs +++ b/src/util/psbt/mod.rs @@ -111,7 +111,7 @@ impl PartiallySignedTransaction { for (vin, psbtin) in tx.input.iter_mut().zip(self.inputs.into_iter()) { vin.script_sig = psbtin.final_script_sig.unwrap_or_else(Script::new); - vin.witness = psbtin.final_script_witness.unwrap_or_else(Vec::new); + vin.witness = psbtin.final_script_witness.unwrap_or_else(Vec::new).into(); } tx @@ -261,6 +261,7 @@ mod tests { use super::PartiallySignedTransaction; use util::psbt::raw::ProprietaryKey; use std::collections::BTreeMap; + use blockdata::witness::Witness; #[test] fn trivial_psbt() { @@ -344,7 +345,7 @@ mod tests { }, script_sig: Script::new(), sequence: 4294967294, - witness: vec![], + witness: Witness::default(), }], output: vec![ TxOut { @@ -419,7 +420,7 @@ mod tests { }, script_sig: hex_script!("160014be18d152a9b012039daf3da7de4f53349eecb985"), sequence: 4294967295, - witness: vec![Vec::from_hex("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105").unwrap()], + witness: vec![Vec::from_hex("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105").unwrap()].into(), }], output: vec![ TxOut { @@ -458,7 +459,7 @@ mod tests { unsigned_tx: { let mut unsigned = tx.clone(); unsigned.input[0].script_sig = Script::new(); - unsigned.input[0].witness = Vec::new(); + unsigned.input[0].witness = Witness::default(); unsigned }, proprietary: proprietary.clone(), @@ -513,6 +514,7 @@ mod tests { use util::psbt::raw; use util::psbt::{PartiallySignedTransaction, Error}; use std::collections::BTreeMap; + use blockdata::witness::Witness; #[test] #[should_panic(expected = "InvalidMagic")] @@ -605,7 +607,7 @@ mod tests { }, script_sig: Script::new(), sequence: 4294967294, - witness: vec![], + witness: Witness::default(), }], output: vec![ TxOut { @@ -639,7 +641,7 @@ mod tests { witness: vec![ Vec::from_hex("304402202712be22e0270f394f568311dc7ca9a68970b8025fdd3b240229f07f8a5f3a240220018b38d7dcd314e734c9276bd6fb40f673325bc4baa144c800d2f2f02db2765c01").unwrap(), Vec::from_hex("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105").unwrap(), - ], + ].into(), }, TxIn { previous_output: OutPoint { @@ -653,7 +655,7 @@ mod tests { witness: vec![ Vec::from_hex("3045022100d12b852d85dcd961d2f5f4ab660654df6eedcc794c0c33ce5cc309ffb5fce58d022067338a8e0e1725c197fb1a88af59f51e44e4255b20167c8684031c05d1f2592a01").unwrap(), Vec::from_hex("0223b72beef0965d10be0778efecd61fcac6f79a4ea169393380734464f84f2ab3").unwrap(), - ], + ].into(), }], output: vec![ TxOut { @@ -837,7 +839,7 @@ mod tests { }, script_sig: Script::new(), sequence: 4294967294, - witness: vec![], + witness: Witness::default(), }], output: vec![ TxOut { @@ -871,7 +873,7 @@ mod tests { witness: vec![ Vec::from_hex("304402202712be22e0270f394f568311dc7ca9a68970b8025fdd3b240229f07f8a5f3a240220018b38d7dcd314e734c9276bd6fb40f673325bc4baa144c800d2f2f02db2765c01").unwrap(), Vec::from_hex("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105").unwrap(), - ], + ].into(), }, TxIn { previous_output: OutPoint { @@ -885,7 +887,7 @@ mod tests { witness: vec![ Vec::from_hex("3045022100d12b852d85dcd961d2f5f4ab660654df6eedcc794c0c33ce5cc309ffb5fce58d022067338a8e0e1725c197fb1a88af59f51e44e4255b20167c8684031c05d1f2592a01").unwrap(), Vec::from_hex("0223b72beef0965d10be0778efecd61fcac6f79a4ea169393380734464f84f2ab3").unwrap(), - ], + ].into(), }], output: vec![ TxOut { diff --git a/src/util/sighash.rs b/src/util/sighash.rs index 02a3286e..f2f6db6a 100644 --- a/src/util/sighash.rs +++ b/src/util/sighash.rs @@ -21,6 +21,7 @@ //! pub use blockdata::transaction::EcdsaSigHashType; +use blockdata::witness::Witness; use consensus::{encode, Encodable}; use core::fmt; use core::ops::{Deref, DerefMut}; @@ -30,8 +31,6 @@ use util::taproot::{TapLeafHash, TapSighashHash}; use SigHash; use {Script, Transaction, TxOut}; -use prelude::*; - use super::taproot::LeafVersion; /// Efficiently calculates signature hash message for legacy, segwit and taproot inputs. @@ -657,10 +656,10 @@ impl> SigHashCache { /// let prevout_script = Script::new(); /// let _sighash = sig_hasher.segwit_signature_hash(inp, &prevout_script, 42, EcdsaSigHashType::All); /// // ... sign the sighash - /// sig_hasher.witness_mut(inp).unwrap().push(Vec::new()); + /// sig_hasher.witness_mut(inp).unwrap().push(&Vec::new()); /// } /// ``` - pub fn witness_mut(&mut self, input_index: usize) -> Option<&mut Vec>> { + pub fn witness_mut(&mut self, input_index: usize) -> Option<&mut Witness> { self.tx.input.get_mut(input_index).map(|i| &mut i.witness) } } From 5a9a18c1a33230573a4e5e3a055f2597d9cb6f19 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Tue, 5 Oct 2021 15:09:23 +0200 Subject: [PATCH 2/5] Add fuzzing for Witness struct --- .github/workflows/fuzz.yml | 2 +- fuzz/Cargo.toml | 4 ++ fuzz/fuzz_targets/deserialize_transaction.rs | 2 +- fuzz/fuzz_targets/deserialize_witness.rs | 59 ++++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 fuzz/fuzz_targets/deserialize_witness.rs diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index d0b9a86f..43d63419 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - fuzz_target: [deser_net_msg, deserialize_address, deserialize_amount, deserialize_block, deserialize_psbt, deserialize_script, deserialize_transaction, outpoint_string, uint128_fuzz, script_bytes_to_asm_fmt] + fuzz_target: [deser_net_msg, deserialize_address, deserialize_amount, deserialize_block, deserialize_psbt, deserialize_script, deserialize_transaction, deserialize_witness, outpoint_string, uint128_fuzz, script_bytes_to_asm_fmt] steps: - name: Install test dependencies run: sudo apt-get update -y && sudo apt-get install -y binutils-dev libunwind8-dev libcurl4-openssl-dev libelf-dev libdw-dev cmake gcc libiberty-dev diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index e8dff82d..b893483b 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -59,3 +59,7 @@ path = "fuzz_targets/uint128_fuzz.rs" [[bin]] name = "script_bytes_to_asm_fmt" path = "fuzz_targets/script_bytes_to_asm_fmt.rs" + +[[bin]] +name = "deserialize_witness" +path = "fuzz_targets/deserialize_witness.rs" diff --git a/fuzz/fuzz_targets/deserialize_transaction.rs b/fuzz/fuzz_targets/deserialize_transaction.rs index 67ad6ce7..ca7b2de2 100644 --- a/fuzz/fuzz_targets/deserialize_transaction.rs +++ b/fuzz/fuzz_targets/deserialize_transaction.rs @@ -10,7 +10,7 @@ fn do_test(data: &[u8]) { let len = ser.len(); let calculated_weight = tx.get_weight(); for input in &mut tx.input { - input.witness = vec![]; + input.witness = bitcoin::blockdata::witness::Witness::default(); } let no_witness_len = bitcoin::consensus::encode::serialize(&tx).len(); // For 0-input transactions, `no_witness_len` will be incorrect because diff --git a/fuzz/fuzz_targets/deserialize_witness.rs b/fuzz/fuzz_targets/deserialize_witness.rs new file mode 100644 index 00000000..b445e8d5 --- /dev/null +++ b/fuzz/fuzz_targets/deserialize_witness.rs @@ -0,0 +1,59 @@ +extern crate bitcoin; + +use bitcoin::consensus::{serialize, deserialize}; +use bitcoin::blockdata::witness::Witness; + +fn do_test(data: &[u8]) { + let w: Result = deserialize(data); + if let Ok(witness) = w { + let serialized = serialize(&witness); + assert_eq!(data, serialized); + } +} + +#[cfg(feature = "afl")] +#[macro_use] extern crate afl; +#[cfg(feature = "afl")] +fn main() { + fuzz!(|data| { + do_test(&data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +#[cfg(test)] +mod tests { + fn extend_vec_from_hex(hex: &str, out: &mut Vec) { + let mut b = 0; + for (idx, c) in hex.as_bytes().iter().enumerate() { + b <<= 4; + match *c { + b'A'..=b'F' => b |= c - b'A' + 10, + b'a'..=b'f' => b |= c - b'a' + 10, + b'0'..=b'9' => b |= c - b'0', + _ => panic!("Bad hex"), + } + if (idx & 1) == 1 { + out.push(b); + b = 0; + } + } + } + + #[test] + fn duplicate_crash() { + let mut a = Vec::new(); + extend_vec_from_hex("00", &mut a); + super::do_test(&a); + } +} From 6c7fdce9b912701c4900b91018593663fefa8ab1 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 13 Oct 2021 13:46:25 +0200 Subject: [PATCH 3/5] rename WitnessIterator to just Iter as guidelines recommend, fix word --- src/blockdata/witness.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/blockdata/witness.rs b/src/blockdata/witness.rs index 3334eefe..27c4e7d7 100644 --- a/src/blockdata/witness.rs +++ b/src/blockdata/witness.rs @@ -1,6 +1,6 @@ //! Witness //! -//! This module contains the [`Witness`] struct and relative methods to operate on it +//! This module contains the [`Witness`] struct and related methods to operate on it //! use consensus::encode::{Error, MAX_VEC_SIZE}; @@ -31,7 +31,7 @@ pub struct Witness { } /// Support structure to allow efficient and convenient iteration over the Witness elements -pub struct WitnessIterator<'a> { +pub struct Iter<'a> { witness: &'a Witness, cursor: usize, } @@ -140,8 +140,8 @@ impl Witness { } /// Returns a struct implementing [`Iterator`] - pub fn iter(&self) -> WitnessIterator { - WitnessIterator { + pub fn iter(&self) -> Iter { + Iter { witness: &self, cursor: 0, } @@ -195,7 +195,7 @@ impl Default for Witness { } } -impl<'a> Iterator for WitnessIterator<'a> { +impl<'a> Iterator for Iter<'a> { type Item = &'a [u8]; fn next(&mut self) -> Option { From baa6ebe4a285b26d16e2a378eed50ba8fd15f946 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 13 Oct 2021 16:38:01 +0200 Subject: [PATCH 4/5] Use bytes iterator for the witness iterator --- src/blockdata/witness.rs | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/blockdata/witness.rs b/src/blockdata/witness.rs index 27c4e7d7..76c6a048 100644 --- a/src/blockdata/witness.rs +++ b/src/blockdata/witness.rs @@ -31,10 +31,7 @@ pub struct Witness { } /// Support structure to allow efficient and convenient iteration over the Witness elements -pub struct Iter<'a> { - witness: &'a Witness, - cursor: usize, -} +pub struct Iter<'a> (::core::slice::Iter<'a, u8>); impl From>> for Witness { fn from(vec: Vec>) -> Self { @@ -141,10 +138,7 @@ impl Witness { /// Returns a struct implementing [`Iterator`] pub fn iter(&self) -> Iter { - Iter { - witness: &self, - cursor: 0, - } + Iter(self.content.iter()) } /// Returns the number of elements this witness holds @@ -199,17 +193,15 @@ impl<'a> Iterator for Iter<'a> { type Item = &'a [u8]; fn next(&mut self) -> Option { - let vec = &self.witness.content; - if self.cursor >= vec.len() { - None - } else { - let var = VarInt::consensus_decode(&vec[self.cursor..]) - .expect("is granted witness.content contains varint because created only from internal methods"); - let start = self.cursor + var.len(); - let end = start + var.0 as usize; - self.cursor = end; - Some(&vec[start..end]) + let varint = VarInt::consensus_decode(self.0.as_slice()).ok()?; + self.0.nth(varint.len() - 1)?; // VarInt::len returns at least 1 + let len = varint.0 as usize; + let slice = &self.0.as_slice()[..len]; + if len > 0 { + // we don't need to advance if the element is empty + self.0.nth(len - 1)?; } + Some(slice) } } From 00b2dd542637e545ef71e50667276c6e8f3c302b Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Wed, 1 Dec 2021 12:16:05 +0100 Subject: [PATCH 5/5] add last() and second_to_last() to efficiently access those witness elements --- src/blockdata/witness.rs | 90 +++++++++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 11 deletions(-) diff --git a/src/blockdata/witness.rs b/src/blockdata/witness.rs index 76c6a048..4a01ec44 100644 --- a/src/blockdata/witness.rs +++ b/src/blockdata/witness.rs @@ -9,7 +9,8 @@ use io::{self, Read, Write}; use prelude::*; use VarInt; -#[cfg(feature = "serde")] use serde; +#[cfg(feature = "serde")] +use serde; /// The Witness is the data used to unlock bitcoins since the [segwit upgrade](https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki) /// @@ -27,15 +28,23 @@ pub struct Witness { /// Number of elements in the witness. /// It is stored separately (instead of as VarInt in the initial part of content) so that method /// like [`Witness::push`] doesn't have case requiring to shift the entire array - witness_elements: u64, + witness_elements: usize, + + /// If `witness_elements > 0` it's a valid index pointing to the last witness element in `content` + /// (Including the varint specifying the length of the element) + last: usize, + + /// If `witness_elements > 1` it's a valid index pointing to the second-to-last witness element in `content` + /// (Including the varint specifying the length of the element) + second_to_last: usize, } /// Support structure to allow efficient and convenient iteration over the Witness elements -pub struct Iter<'a> (::core::slice::Iter<'a, u8>); +pub struct Iter<'a>(::core::slice::Iter<'a, u8>); impl From>> for Witness { fn from(vec: Vec>) -> Self { - let witness_elements = vec.len() as u64; + let witness_elements = vec.len(); let content_size: usize = vec .iter() @@ -43,7 +52,11 @@ impl From>> for Witness { .sum(); let mut content = vec![0u8; content_size]; let mut cursor = 0usize; + let mut last = 0; + let mut second_to_last = 0; for el in vec { + second_to_last = last; + last = cursor; let el_len_varint = VarInt(el.len() as u64); el_len_varint .consensus_encode(&mut content[cursor..cursor + el_len_varint.len()]) @@ -56,23 +69,29 @@ impl From>> for Witness { Witness { witness_elements, content, + last, + second_to_last, } } } impl Decodable for Witness { fn consensus_decode(mut d: D) -> Result { - let witness_elements = VarInt::consensus_decode(&mut d)?.0; + let witness_elements = VarInt::consensus_decode(&mut d)?.0 as usize; if witness_elements == 0 { Ok(Witness::default()) } else { let mut cursor = 0usize; + let mut last = 0usize; + let mut second_to_last = 0usize; // this number should be determined as high enough to cover most witness, and low enough // to avoid wasting space without reallocating let mut content = vec![0u8; 128]; for _ in 0..witness_elements { + second_to_last = last; + last = cursor; let element_size_varint = VarInt::consensus_decode(&mut d)?; let element_size_varint_len = element_size_varint.len(); let element_size = element_size_varint.0 as usize; @@ -106,6 +125,8 @@ impl Decodable for Witness { Ok(Witness { content, witness_elements, + last, + second_to_last, }) } } @@ -123,7 +144,7 @@ fn resize_if_needed(vec: &mut Vec, required_len: usize) { impl Encodable for Witness { fn consensus_encode(&self, mut writer: W) -> Result { - let len = VarInt(self.witness_elements); + let len = VarInt(self.witness_elements as u64); len.consensus_encode(&mut writer)?; writer.emit_slice(&self.content[..])?; Ok(self.content.len() + len.len()) @@ -151,7 +172,7 @@ impl Witness { self.iter() .map(|el| VarInt(el.len() as u64).len() + el.len()) .sum::() - + VarInt(self.witness_elements).len() + + VarInt(self.witness_elements as u64).len() } /// Clear the witness @@ -164,6 +185,8 @@ impl Witness { pub fn push>(&mut self, new_element: T) { let new_element = new_element.as_ref(); self.witness_elements += 1; + self.second_to_last = self.last; + self.last = self.content.len(); let element_len_varint = VarInt(new_element.len() as u64); let current_content_len = self.content.len(); self.content.resize( @@ -176,6 +199,30 @@ impl Witness { .expect("writers on vec don't error, space granted through previous resize"); self.content[end_varint..].copy_from_slice(new_element); } + + fn element_at(&self, index: usize) -> Option<&[u8]> { + let varint = VarInt::consensus_decode(&self.content[index..]).ok()?; + let start = index + varint.len(); + Some(&self.content[start..start + varint.0 as usize]) + } + + /// Return the last element in the witness, if any + pub fn last(&self) -> Option<&[u8]> { + if self.witness_elements == 0 { + None + } else { + self.element_at(self.last) + } + } + + /// Return the second_to_last element in the witness, if any + pub fn second_to_last(&self) -> Option<&[u8]> { + if self.witness_elements <= 1 { + None + } else { + self.element_at(self.second_to_last) + } + } } impl Default for Witness { @@ -185,6 +232,8 @@ impl Default for Witness { Witness { content: Vec::new(), witness_elements: 0, + last: 0, + second_to_last: 0, } } } @@ -237,18 +286,28 @@ mod test { #[test] fn test_push() { let mut witness = Witness::default(); + assert_eq!(witness.last(), None); + assert_eq!(witness.second_to_last(), None); witness.push(&vec![0u8]); let expected = Witness { witness_elements: 1, content: vec![1u8, 0], + last: 0, + second_to_last: 0, }; assert_eq!(witness, expected); + assert_eq!(witness.last(), Some(&[0u8][..])); + assert_eq!(witness.second_to_last(), None); witness.push(&vec![2u8, 3u8]); let expected = Witness { witness_elements: 2, content: vec![1u8, 0, 2, 2, 3], + last: 2, + second_to_last: 0, }; assert_eq!(witness, expected); + assert_eq!(witness.last(), Some(&[2u8, 3u8][..])); + assert_eq!(witness.second_to_last(), Some(&[0u8][..])); } #[test] @@ -257,23 +316,24 @@ mod test { Vec::from_hex("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105") .unwrap(); let w1 = Vec::from_hex("000000").unwrap(); - let witness_vec = vec![w0, w1]; + let witness_vec = vec![w0.clone(), w1.clone()]; let witness_serialized: Vec = serialize(&witness_vec); let witness = Witness { content: witness_serialized[1..].to_vec(), witness_elements: 2, + last: 34, + second_to_last: 0, }; for (i, el) in witness.iter().enumerate() { assert_eq!(witness_vec[i], el); } + assert_eq!(witness.last(), Some(&w1[..])); + assert_eq!(witness.second_to_last(), Some(&w0[..])); let w_into: Witness = witness_vec.into(); assert_eq!(w_into, witness); assert_eq!(witness_serialized, serialize(&witness)); - - //assert_eq!(32, std::mem::size_of::()); - //assert_eq!(24, std::mem::size_of::>>()); } #[test] @@ -286,6 +346,14 @@ mod test { for (i, wit_el) in tx.input[0].witness.iter().enumerate() { assert_eq!(expected_wit[i], wit_el.to_hex()); } + assert_eq!( + expected_wit[1], + tx.input[0].witness.last().unwrap().to_hex() + ); + assert_eq!( + expected_wit[0], + tx.input[0].witness.second_to_last().unwrap().to_hex() + ); let tx_bytes_back = serialize(&tx); assert_eq!(tx_bytes_back, tx_bytes); }