From be163eec99ac42bdaefa2f3c65f0c3aa0bb0bf9f Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 24 Sep 2024 10:20:04 +1000 Subject: [PATCH 1/3] Use Witness::len instead of accessing field In preparation for moving the `Witness` oven to `primitives` use the `len` function instead of accessing the `witness_elements` field. No logic change, `Witness::len()` returns `witness_elements`. --- bitcoin/src/blockdata/witness.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs index d60ce398b..12cd648d9 100644 --- a/bitcoin/src/blockdata/witness.rs +++ b/bitcoin/src/blockdata/witness.rs @@ -231,7 +231,7 @@ impl Encodable for Witness { // `self.content` includes the varints so encoding here includes them, as expected. fn consensus_encode(&self, w: &mut W) -> Result { let content_with_indices_len = self.content.len(); - let indices_size = self.witness_elements * 4; + let indices_size = self.len() * 4; let content_len = content_with_indices_len - indices_size; Ok(w.emit_compact_size(self.witness_elements)? + w.emit_slice(&self.content[..content_len])?) } From 6389d1cbb30441cd54f675df1399bcb90e093906 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 24 Sep 2024 10:25:03 +1000 Subject: [PATCH 2/3] Stop using push_slice The `Witness::push_slice` function is called by `Witness::push` after calling `as_ref`, hence is equivalent for all types that implement `AsRef<[u8]>`. Also, `push_slice` is a private method on `Witness`. In preparation for moving `Witness` over to `primitives` stop using `push_slice` in favour of `push`. Internal change only. --- bitcoin/src/blockdata/witness.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs index 12cd648d9..3ece6fcc8 100644 --- a/bitcoin/src/blockdata/witness.rs +++ b/bitcoin/src/blockdata/witness.rs @@ -252,15 +252,15 @@ impl Witness { /// It is expected that `pubkey` is related to the secret key used to create `signature`. pub fn p2wpkh(signature: ecdsa::Signature, pubkey: secp256k1::PublicKey) -> Witness { let mut witness = Witness::new(); - witness.push_slice(&signature.serialize()); - witness.push_slice(&pubkey.serialize()); + witness.push(signature.serialize()); + witness.push(pubkey.serialize()); witness } /// Creates a witness required to do a key path spend of a P2TR output. pub fn p2tr_key_spend(signature: &taproot::Signature) -> Witness { let mut witness = Witness::new(); - witness.push_slice(&signature.serialize()); + witness.push(signature.serialize()); witness } @@ -363,7 +363,7 @@ impl Witness { /// /// Pushes the DER encoded signature + sighash_type, requires an allocation. pub fn push_ecdsa_signature(&mut self, signature: ecdsa::Signature) { - self.push_slice(&signature.serialize()) + self.push(signature.serialize()) } /// Note `index` is the index into the `content` vector and should be the result of calling From 3f3f30d6c7ab8351b7f7f8a0b81b76a6e1096be0 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 24 Sep 2024 10:50:43 +1000 Subject: [PATCH 3/3] Use iter instead of accessing content field We would like to move the `Witness` to `primitives` however in the `Encodable` implementation we are currently accessing the private `content` field. Instead of accessing `content` we can iterate over the witness elements and write each individually, this has the same result but does a bunch of additional calls to `Write::write_all` (via `emit_slice`). This patch effects performance negatively but makes no changes to the encoding. --- bitcoin/src/blockdata/witness.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs index 3ece6fcc8..d51fd14c4 100644 --- a/bitcoin/src/blockdata/witness.rs +++ b/bitcoin/src/blockdata/witness.rs @@ -12,8 +12,8 @@ use arbitrary::{Arbitrary, Unstructured}; use internals::compact_size; use io::{BufRead, Write}; -use crate::consensus::encode::{Error, ReadExt, MAX_VEC_SIZE}; -use crate::consensus::{Decodable, Encodable, WriteExt}; +use crate::consensus::encode::{self, Error, MAX_VEC_SIZE, ReadExt, WriteExt}; +use crate::consensus::{Decodable, Encodable}; use crate::crypto::ecdsa; use crate::prelude::Vec; #[cfg(doc)] @@ -230,10 +230,13 @@ fn resize_if_needed(vec: &mut Vec, required_len: usize) { impl Encodable for Witness { // `self.content` includes the varints so encoding here includes them, as expected. fn consensus_encode(&self, w: &mut W) -> Result { - let content_with_indices_len = self.content.len(); - let indices_size = self.len() * 4; - let content_len = content_with_indices_len - indices_size; - Ok(w.emit_compact_size(self.witness_elements)? + w.emit_slice(&self.content[..content_len])?) + let mut written = w.emit_compact_size(self.len())?; + + for element in self.iter() { + written += encode::consensus_encode_with_size(element, w)? + } + + Ok(written) } }