From efd7f9f06cf9156517bef775545da713d982945a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 18 Oct 2024 10:38:50 +1100 Subject: [PATCH] Add error constructor parse_failed_error The `encode::Error::ParseFailed` variant holds an inner string, this is suboptimal. In an effort to patch the `encode::Error` while mimizing the diffs required add a helper function that creates the variant. The benefit is that later patches that effect this variant will only need to update the constructor function and not every call site. Internal change only. --- bitcoin/src/bip152.rs | 15 ++++++++------- bitcoin/src/blockdata/transaction.rs | 6 ++++-- bitcoin/src/consensus/encode.rs | 6 +++--- bitcoin/src/consensus/error.rs | 4 ++++ bitcoin/src/consensus/mod.rs | 1 + bitcoin/src/p2p/address.rs | 21 +++++++++++---------- bitcoin/src/p2p/message.rs | 4 ++-- bitcoin/src/p2p/message_bloom.rs | 4 ++-- bitcoin/src/p2p/message_network.rs | 4 ++-- 9 files changed, 37 insertions(+), 28 deletions(-) diff --git a/bitcoin/src/bip152.rs b/bitcoin/src/bip152.rs index 96ffcdc79..c3ccee11c 100644 --- a/bitcoin/src/bip152.rs +++ b/bitcoin/src/bip152.rs @@ -16,7 +16,7 @@ use crate::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt}; use crate::internal_macros::{impl_array_newtype_stringify, impl_consensus_encoding}; use crate::prelude::Vec; use crate::transaction::TxIdentifier; -use crate::{block, Block, BlockHash, Transaction}; +use crate::{block, consensus, Block, BlockHash, Transaction}; /// A BIP-152 error #[derive(Debug, Clone, PartialEq, Eq)] @@ -84,8 +84,9 @@ impl Decodable for PrefilledTransaction { #[inline] fn consensus_decode(r: &mut R) -> Result { let idx = r.read_compact_size()?; - let idx = u16::try_from(idx) - .map_err(|_| encode::Error::ParseFailed("BIP152 prefilled tx index out of bounds"))?; + let idx = u16::try_from(idx).map_err(|_| { + consensus::parse_failed_error("BIP152 prefilled tx index out of bounds") + })?; let tx = Transaction::consensus_decode(r)?; Ok(PrefilledTransaction { idx, tx }) } @@ -172,7 +173,7 @@ impl Decodable for HeaderAndShortIds { }; match header_short_ids.short_ids.len().checked_add(header_short_ids.prefilled_txs.len()) { Some(x) if x <= u16::MAX.into() => Ok(header_short_ids), - _ => Err(encode::Error::ParseFailed("indexes overflowed 16 bits")), + _ => Err(consensus::parse_failed_error("indexes overflowed 16 bits")), } } } @@ -312,7 +313,7 @@ impl Decodable for BlockTransactionsRequest { // transactions that would be allowed in a vector. let byte_size = nb_indexes .checked_mul(mem::size_of::()) - .ok_or(encode::Error::ParseFailed("invalid length"))?; + .ok_or(consensus::parse_failed_error("invalid length"))?; if byte_size > encode::MAX_VEC_SIZE { return Err(encode::Error::OversizedVectorAllocation { requested: byte_size, @@ -326,12 +327,12 @@ impl Decodable for BlockTransactionsRequest { let differential = r.read_compact_size()?; last_index = match last_index.checked_add(differential) { Some(i) => i, - None => return Err(encode::Error::ParseFailed("block index overflow")), + None => return Err(consensus::parse_failed_error("block index overflow")), }; indexes.push(last_index); last_index = match last_index.checked_add(1) { Some(i) => i, - None => return Err(encode::Error::ParseFailed("block index overflow")), + None => return Err(consensus::parse_failed_error("block index overflow")), }; } indexes diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 469a9a70c..009605baf 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -20,7 +20,7 @@ use io::{BufRead, Write}; use primitives::Sequence; use super::Weight; -use crate::consensus::{encode, Decodable, Encodable}; +use crate::consensus::{self, encode, Decodable, Encodable}; use crate::internal_macros::{impl_consensus_encoding, impl_hashencode}; use crate::locktime::absolute::{self, Height, Time}; use crate::prelude::{Borrow, Vec}; @@ -900,7 +900,9 @@ impl Decodable for Transaction { txin.witness = Decodable::consensus_decode_from_finite_reader(r)?; } if !input.is_empty() && input.iter().all(|input| input.witness.is_empty()) { - Err(encode::Error::ParseFailed("witness flag set but no witnesses present")) + Err(consensus::parse_failed_error( + "witness flag set but no witnesses present", + )) } else { Ok(Transaction { version, diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs index cf427199f..eb89dfe11 100644 --- a/bitcoin/src/consensus/encode.rs +++ b/bitcoin/src/consensus/encode.rs @@ -60,7 +60,7 @@ pub fn deserialize(data: &[u8]) -> Result { if consumed == data.len() { Ok(rv) } else { - Err(Error::ParseFailed("data not consumed entirely when explicitly deserializing")) + Err(super::parse_failed_error("data not consumed entirely when explicitly deserializing")) } } @@ -418,7 +418,7 @@ impl Decodable for String { #[inline] fn consensus_decode(r: &mut R) -> Result { String::from_utf8(Decodable::consensus_decode(r)?) - .map_err(|_| self::Error::ParseFailed("String was not valid UTF8")) + .map_err(|_| super::parse_failed_error("String was not valid UTF8")) } } @@ -433,7 +433,7 @@ impl Decodable for Cow<'static, str> { #[inline] fn consensus_decode(r: &mut R) -> Result, Error> { String::from_utf8(Decodable::consensus_decode(r)?) - .map_err(|_| self::Error::ParseFailed("String was not valid UTF8")) + .map_err(|_| super::parse_failed_error("String was not valid UTF8")) .map(Cow::Owned) } } diff --git a/bitcoin/src/consensus/error.rs b/bitcoin/src/consensus/error.rs index e1e0e2d1a..e74c54ec8 100644 --- a/bitcoin/src/consensus/error.rs +++ b/bitcoin/src/consensus/error.rs @@ -155,3 +155,7 @@ impl From for FromHexError { #[inline] fn from(e: OddLengthStringError) -> Self { Self::OddLengthString(e) } } + +/// Constructs a `Error::ParseFailed` error. +// This whole variant should go away because of the inner string. +pub(crate) fn parse_failed_error(msg: &'static str) -> Error { Error::ParseFailed(msg) } diff --git a/bitcoin/src/consensus/mod.rs b/bitcoin/src/consensus/mod.rs index a12b22e13..8a43ae329 100644 --- a/bitcoin/src/consensus/mod.rs +++ b/bitcoin/src/consensus/mod.rs @@ -22,6 +22,7 @@ pub use self::{ encode::{deserialize, deserialize_partial, serialize, Decodable, Encodable, ReadExt, WriteExt}, error::{Error, FromHexError, DecodeError}, }; +pub(crate) use self::error::parse_failed_error; struct IterReader>> { iterator: core::iter::Fuse, diff --git a/bitcoin/src/p2p/address.rs b/bitcoin/src/p2p/address.rs index a531618e0..d90992001 100644 --- a/bitcoin/src/p2p/address.rs +++ b/bitcoin/src/p2p/address.rs @@ -10,6 +10,7 @@ use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6, ToSoc use io::{BufRead, Read, Write}; +use crate::consensus; use crate::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt}; use crate::p2p::ServiceFlags; @@ -165,28 +166,28 @@ impl Decodable for AddrV2 { let network_id = u8::consensus_decode(r)?; let len = r.read_compact_size()?; if len > 512 { - return Err(encode::Error::ParseFailed("IP must be <= 512 bytes")); + return Err(consensus::parse_failed_error("IP must be <= 512 bytes")); } Ok(match network_id { 1 => { if len != 4 { - return Err(encode::Error::ParseFailed("invalid IPv4 address")); + return Err(consensus::parse_failed_error("invalid IPv4 address")); } let addr: [u8; 4] = Decodable::consensus_decode(r)?; AddrV2::Ipv4(Ipv4Addr::new(addr[0], addr[1], addr[2], addr[3])) } 2 => { if len != 16 { - return Err(encode::Error::ParseFailed("invalid IPv6 address")); + return Err(consensus::parse_failed_error("invalid IPv6 address")); } let addr: [u16; 8] = read_be_address(r)?; if addr[0..3] == ONION { - return Err(encode::Error::ParseFailed( + return Err(consensus::parse_failed_error( "OnionCat address sent with IPv6 network id", )); } if addr[0..6] == [0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0xFFFF] { - return Err(encode::Error::ParseFailed( + return Err(consensus::parse_failed_error( "IPV4 wrapped address sent with IPv6 network id", )); } @@ -196,33 +197,33 @@ impl Decodable for AddrV2 { } 3 => { if len != 10 { - return Err(encode::Error::ParseFailed("invalid TorV2 address")); + return Err(consensus::parse_failed_error("invalid TorV2 address")); } let id = Decodable::consensus_decode(r)?; AddrV2::TorV2(id) } 4 => { if len != 32 { - return Err(encode::Error::ParseFailed("invalid TorV3 address")); + return Err(consensus::parse_failed_error("invalid TorV3 address")); } let pubkey = Decodable::consensus_decode(r)?; AddrV2::TorV3(pubkey) } 5 => { if len != 32 { - return Err(encode::Error::ParseFailed("invalid I2P address")); + return Err(consensus::parse_failed_error("invalid I2P address")); } let hash = Decodable::consensus_decode(r)?; AddrV2::I2p(hash) } 6 => { if len != 16 { - return Err(encode::Error::ParseFailed("invalid CJDNS address")); + return Err(consensus::parse_failed_error("invalid CJDNS address")); } let addr: [u16; 8] = read_be_address(r)?; // check the first byte for the CJDNS marker if addr[0] >> 8 != 0xFC { - return Err(encode::Error::ParseFailed("invalid CJDNS address")); + return Err(consensus::parse_failed_error("invalid CJDNS address")); } AddrV2::Cjdns(Ipv6Addr::new( addr[0], addr[1], addr[2], addr[3], addr[4], addr[5], addr[6], addr[7], diff --git a/bitcoin/src/p2p/message.rs b/bitcoin/src/p2p/message.rs index 8776aa96a..9ae2aa0a2 100644 --- a/bitcoin/src/p2p/message.rs +++ b/bitcoin/src/p2p/message.rs @@ -19,7 +19,7 @@ use crate::p2p::{ Magic, }; use crate::prelude::{Box, Cow, String, ToOwned, Vec}; -use crate::{block, transaction}; +use crate::{block, consensus, transaction}; /// The maximum number of [super::message_blockdata::Inventory] items in an `inv` message. /// @@ -418,7 +418,7 @@ impl Decodable for HeaderDeserializationWrapper { for _ in 0..len { ret.push(Decodable::consensus_decode(r)?); if u8::consensus_decode(r)? != 0u8 { - return Err(encode::Error::ParseFailed( + return Err(consensus::parse_failed_error( "Headers message should not contain transactions", )); } diff --git a/bitcoin/src/p2p/message_bloom.rs b/bitcoin/src/p2p/message_bloom.rs index 1e6b5e235..2a75f9a64 100644 --- a/bitcoin/src/p2p/message_bloom.rs +++ b/bitcoin/src/p2p/message_bloom.rs @@ -6,7 +6,7 @@ use io::{BufRead, Write}; -use crate::consensus::{encode, Decodable, Encodable, ReadExt}; +use crate::consensus::{self, encode, Decodable, Encodable, ReadExt}; use crate::internal_macros::impl_consensus_encoding; /// `filterload` message sets the current bloom filter @@ -52,7 +52,7 @@ impl Decodable for BloomFlags { 0 => BloomFlags::None, 1 => BloomFlags::All, 2 => BloomFlags::PubkeyOnly, - _ => return Err(encode::Error::ParseFailed("unknown bloom flag")), + _ => return Err(consensus::parse_failed_error("unknown bloom flag")), }) } } diff --git a/bitcoin/src/p2p/message_network.rs b/bitcoin/src/p2p/message_network.rs index 470a07dbb..942578148 100644 --- a/bitcoin/src/p2p/message_network.rs +++ b/bitcoin/src/p2p/message_network.rs @@ -8,7 +8,7 @@ use hashes::sha256d; use io::{BufRead, Write}; -use crate::consensus::{encode, Decodable, Encodable, ReadExt}; +use crate::consensus::{self, encode, Decodable, Encodable, ReadExt}; use crate::internal_macros::impl_consensus_encoding; use crate::p2p; use crate::p2p::address::Address; @@ -126,7 +126,7 @@ impl Decodable for RejectReason { 0x41 => RejectReason::Dust, 0x42 => RejectReason::Fee, 0x43 => RejectReason::Checkpoint, - _ => return Err(encode::Error::ParseFailed("unknown reject code")), + _ => return Err(consensus::parse_failed_error("unknown reject code")), }) } }