From e96be5ee6e141b5d8a9163dc4066301d81957fa9 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 17 Aug 2023 09:34:46 +1000 Subject: [PATCH 1/4] Fix Witness debug display bug Currently if the witness has zero elements or any of the individual witnesses is empty we panic. Panic is caused by subtracting 1 from a zero length. Check the length is non-zero before subtracting 1, print `[]` if empty. --- bitcoin/src/blockdata/witness.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs index 390ff0b0b0..3d10a0a0f1 100644 --- a/bitcoin/src/blockdata/witness.rs +++ b/bitcoin/src/blockdata/witness.rs @@ -68,20 +68,26 @@ fn fmt_debug(w: &Witness, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> f.write_str("witnesses: [")?; let instructions = w.iter(); - let last_instruction = instructions.len() - 1; - - for (i, instruction) in instructions.enumerate() { - let bytes = instruction.iter(); - let last_byte = bytes.len() - 1; - - f.write_str("[")?; - - for (j, byte) in bytes.enumerate() { - write!(f, "{:#04x}", byte)?; - f.write_str(comma_or_close(j, last_byte))?; + if instructions.len() > 0 { + let last_instruction = instructions.len() - 1; + for (i, instruction) in instructions.enumerate() { + let bytes = instruction.iter(); + if bytes.len() > 0 { + let last_byte = bytes.len() - 1; + f.write_str("[")?; + for (j, byte) in bytes.enumerate() { + write!(f, "{:#04x}", byte)?; + f.write_str(comma_or_close(j, last_byte))?; + } + } else { + // This is possible because the varint is not part of the instruction (see Iter). + write!(f, "[]")?; + } + f.write_str(comma_or_close(i, last_instruction))?; } - - f.write_str(comma_or_close(i, last_instruction))?; + } else { + // Witnesses can be empty because the 0x00 var int is not stored in content. + write!(f, "]")?; } f.write_str(" }") From 84614d9997a69bac249f3e8ac3b90240b38ca747 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Aug 2023 16:03:33 +1000 Subject: [PATCH 2/4] Unit test debug print of witness with empty instruction We recently fixed a bug that causes a panic if a `Witness` contains an empty instruction. Add a unit test to verify it. --- bitcoin/src/blockdata/witness.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs index 3d10a0a0f1..43e69d6957 100644 --- a/bitcoin/src/blockdata/witness.rs +++ b/bitcoin/src/blockdata/witness.rs @@ -535,6 +535,16 @@ mod test { v } + #[test] + fn witness_debug_can_display_empty_instruction() { + let witness = Witness { + witness_elements: 1, + content: append_u32_vec(vec![], &[0]), + indices_start: 2, + }; + println!("{:?}", witness); + } + #[test] fn test_push() { let mut witness = Witness::default(); From adcc01c0bdbf781cfc6c210aa71b9a647d2ab262 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 29 Aug 2023 12:33:19 +1000 Subject: [PATCH 3/4] CI: Fix pinning More crates broke our MSRV by using edition 2021 without doing a major release, pin them in the CI script. diff --git a/contrib/test.sh b/contrib/test.sh index 74f8ffb3..79932ad5 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -13,6 +13,12 @@ if cargo --version | grep ${MSRV}; then cargo update -p quote --precise 1.0.30 cargo update -p proc-macro2 --precise 1.0.63 cargo update -p serde_test --precise 1.0.175 + # Have to pin this so we can pin `schemars_derive` + cargo update -p schemars --precise 0.8.12 + # schemars_derive 0.8.13 uses edition 2021 + cargo update -p schemars_derive --precise 0.8.12 + # memcrh 2.6.0 uses edition 2021 + cargo update -p memchr --precise 2.5.0 cargo update -p bitcoin:0.30.1 --precise 0.30.0 --- contrib/test.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/test.sh b/contrib/test.sh index 74f8ffb38e..79932ad512 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -13,6 +13,12 @@ if cargo --version | grep ${MSRV}; then cargo update -p quote --precise 1.0.30 cargo update -p proc-macro2 --precise 1.0.63 cargo update -p serde_test --precise 1.0.175 + # Have to pin this so we can pin `schemars_derive` + cargo update -p schemars --precise 0.8.12 + # schemars_derive 0.8.13 uses edition 2021 + cargo update -p schemars_derive --precise 0.8.12 + # memcrh 2.6.0 uses edition 2021 + cargo update -p memchr --precise 2.5.0 cargo update -p bitcoin:0.30.1 --precise 0.30.0 From 18e2854a42e829319490d929f95349a0922492cf Mon Sep 17 00:00:00 2001 From: junderw Date: Sun, 27 Aug 2023 17:02:58 -0700 Subject: [PATCH 4/4] Update base64 usage to 0.21.3 --- Cargo-minimal.lock | 4 +-- Cargo-recent.lock | 4 +-- bitcoin/Cargo.toml | 2 +- bitcoin/src/psbt/mod.rs | 5 +-- bitcoin/src/sign_message.rs | 63 +++++++++++++++++++------------------ 5 files changed, 41 insertions(+), 37 deletions(-) diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index 2994ad2f08..cbf6123846 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -10,9 +10,9 @@ checksum = "08f9b8508dccb7687a1d6c4ce66b2b0ecef467c94667de27d8d7fe1f8d2a9cdc" [[package]] name = "base64" -version = "0.13.0" +version = "0.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "904dfeac50f3cdaba28fc6f57fdcddb75f49ed61346676a78c4ffe55877802fd" +checksum = "414dcefbc63d77c526a76b3afcf6fbb9b5e2791c19c3aa2297733208750c6e53" [[package]] name = "bech32" diff --git a/Cargo-recent.lock b/Cargo-recent.lock index 8cced22117..35a7086a06 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -10,9 +10,9 @@ checksum = "9c7d0618f0e0b7e8ff11427422b64564d5fb0be1940354bfe2e0529b18a9d9b8" [[package]] name = "base64" -version = "0.13.1" +version = "0.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" +checksum = "414dcefbc63d77c526a76b3afcf6fbb9b5e2791c19c3aa2297733208750c6e53" [[package]] name = "bech32" diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index c928b6b01c..b54524e836 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -41,7 +41,7 @@ hashes = { package = "bitcoin_hashes", version = "0.13.0", default-features = fa secp256k1 = { version = "0.27.0", default-features = false, features = ["bitcoin_hashes"] } hex_lit = "0.1.1" -base64 = { version = "0.13.0", optional = true } +base64 = { version = "0.21.3", optional = true } # Only use this feature for no-std builds, otherwise use bitcoinconsensus-std. bitcoinconsensus = { version = "0.20.2-0.5.0", default-features = false, optional = true } diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 4683dc5a34..0dcd4b5117 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -818,6 +818,7 @@ mod display_from_str { use core::str::FromStr; use base64::display::Base64Display; + use base64::prelude::{Engine as _, BASE64_STANDARD}; use internals::write_err; use super::{Error, Psbt}; @@ -857,7 +858,7 @@ mod display_from_str { impl Display for Psbt { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", Base64Display::with_config(&self.serialize(), base64::STANDARD)) + write!(f, "{}", Base64Display::new(&self.serialize(), &BASE64_STANDARD)) } } @@ -865,7 +866,7 @@ mod display_from_str { type Err = PsbtParseError; fn from_str(s: &str) -> Result { - let data = base64::decode(s).map_err(PsbtParseError::Base64Encoding)?; + let data = BASE64_STANDARD.decode(s).map_err(PsbtParseError::Base64Encoding)?; Psbt::deserialize(&data).map_err(PsbtParseError::PsbtEncoding) } } diff --git a/bitcoin/src/sign_message.rs b/bitcoin/src/sign_message.rs index bb9d47b769..80cdb7a66b 100644 --- a/bitcoin/src/sign_message.rs +++ b/bitcoin/src/sign_message.rs @@ -26,8 +26,6 @@ mod message_signing { use crate::address::{Address, AddressType}; use crate::crypto::key::PublicKey; - #[cfg(feature = "base64")] - use crate::prelude::*; /// An error used for dealing with Bitcoin Signed Messages. #[derive(Debug, Clone, PartialEq, Eq)] @@ -159,37 +157,40 @@ mod message_signing { None => Ok(false), } } - - /// Convert a signature from base64 encoding. - #[cfg(feature = "base64")] - pub fn from_base64(s: &str) -> Result { - let bytes = base64::decode(s).map_err(|_| MessageSignatureError::InvalidBase64)?; - MessageSignature::from_slice(&bytes) - } - - /// Convert to base64 encoding. - #[cfg(feature = "base64")] - pub fn to_base64(self) -> String { base64::encode(&self.serialize()[..]) } } #[cfg(feature = "base64")] - impl fmt::Display for MessageSignature { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let bytes = self.serialize(); - // This avoids the allocation of a String. - write!( - f, - "{}", - base64::display::Base64Display::with_config(&bytes[..], base64::STANDARD) - ) + mod base64_impls { + use base64::prelude::{Engine as _, BASE64_STANDARD}; + + use super::*; + use crate::prelude::String; + + impl MessageSignature { + /// Convert a signature from base64 encoding. + pub fn from_base64(s: &str) -> Result { + let bytes = + BASE64_STANDARD.decode(s).map_err(|_| MessageSignatureError::InvalidBase64)?; + MessageSignature::from_slice(&bytes) + } + + /// Convert to base64 encoding. + pub fn to_base64(self) -> String { BASE64_STANDARD.encode(self.serialize()) } } - } - #[cfg(feature = "base64")] - impl core::str::FromStr for MessageSignature { - type Err = MessageSignatureError; - fn from_str(s: &str) -> Result { - MessageSignature::from_base64(s) + impl fmt::Display for MessageSignature { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let bytes = self.serialize(); + // This avoids the allocation of a String. + write!(f, "{}", base64::display::Base64Display::new(&bytes, &BASE64_STANDARD)) + } + } + + impl core::str::FromStr for MessageSignature { + type Err = MessageSignatureError; + fn from_str(s: &str) -> Result { + MessageSignature::from_base64(s) + } } } } @@ -260,6 +261,7 @@ mod tests { #[test] #[cfg(all(feature = "secp-recovery", feature = "base64"))] fn test_incorrect_message_signature() { + use base64::prelude::{Engine as _, BASE64_STANDARD}; use secp256k1; use crate::crypto::key::PublicKey; @@ -276,8 +278,9 @@ mod tests { let signature = super::MessageSignature::from_base64(signature_base64).expect("message signature"); - let pubkey = PublicKey::from_slice(&base64::decode(pubkey_base64).expect("base64 string")) - .expect("pubkey slice"); + let pubkey = + PublicKey::from_slice(&BASE64_STANDARD.decode(pubkey_base64).expect("base64 string")) + .expect("pubkey slice"); let p2pkh = Address::p2pkh(&pubkey, Network::Bitcoin); assert_eq!(signature.is_signed_by_address(&secp, &p2pkh, msg_hash), Ok(false));