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.
This commit is contained in:
Tobin C. Harding 2024-10-18 10:38:50 +11:00
parent ebfef3f114
commit efd7f9f06c
No known key found for this signature in database
GPG Key ID: 40BF9E4C269D6607
9 changed files with 37 additions and 28 deletions

View File

@ -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::internal_macros::{impl_array_newtype_stringify, impl_consensus_encoding};
use crate::prelude::Vec; use crate::prelude::Vec;
use crate::transaction::TxIdentifier; use crate::transaction::TxIdentifier;
use crate::{block, Block, BlockHash, Transaction}; use crate::{block, consensus, Block, BlockHash, Transaction};
/// A BIP-152 error /// A BIP-152 error
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
@ -84,8 +84,9 @@ impl Decodable for PrefilledTransaction {
#[inline] #[inline]
fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> { fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
let idx = r.read_compact_size()?; let idx = r.read_compact_size()?;
let idx = u16::try_from(idx) let idx = u16::try_from(idx).map_err(|_| {
.map_err(|_| encode::Error::ParseFailed("BIP152 prefilled tx index out of bounds"))?; consensus::parse_failed_error("BIP152 prefilled tx index out of bounds")
})?;
let tx = Transaction::consensus_decode(r)?; let tx = Transaction::consensus_decode(r)?;
Ok(PrefilledTransaction { idx, tx }) 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()) { 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), 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. // transactions that would be allowed in a vector.
let byte_size = nb_indexes let byte_size = nb_indexes
.checked_mul(mem::size_of::<Transaction>()) .checked_mul(mem::size_of::<Transaction>())
.ok_or(encode::Error::ParseFailed("invalid length"))?; .ok_or(consensus::parse_failed_error("invalid length"))?;
if byte_size > encode::MAX_VEC_SIZE { if byte_size > encode::MAX_VEC_SIZE {
return Err(encode::Error::OversizedVectorAllocation { return Err(encode::Error::OversizedVectorAllocation {
requested: byte_size, requested: byte_size,
@ -326,12 +327,12 @@ impl Decodable for BlockTransactionsRequest {
let differential = r.read_compact_size()?; let differential = r.read_compact_size()?;
last_index = match last_index.checked_add(differential) { last_index = match last_index.checked_add(differential) {
Some(i) => i, 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); indexes.push(last_index);
last_index = match last_index.checked_add(1) { last_index = match last_index.checked_add(1) {
Some(i) => i, Some(i) => i,
None => return Err(encode::Error::ParseFailed("block index overflow")), None => return Err(consensus::parse_failed_error("block index overflow")),
}; };
} }
indexes indexes

View File

@ -20,7 +20,7 @@ use io::{BufRead, Write};
use primitives::Sequence; use primitives::Sequence;
use super::Weight; 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::internal_macros::{impl_consensus_encoding, impl_hashencode};
use crate::locktime::absolute::{self, Height, Time}; use crate::locktime::absolute::{self, Height, Time};
use crate::prelude::{Borrow, Vec}; use crate::prelude::{Borrow, Vec};
@ -900,7 +900,9 @@ impl Decodable for Transaction {
txin.witness = Decodable::consensus_decode_from_finite_reader(r)?; txin.witness = Decodable::consensus_decode_from_finite_reader(r)?;
} }
if !input.is_empty() && input.iter().all(|input| input.witness.is_empty()) { 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 { } else {
Ok(Transaction { Ok(Transaction {
version, version,

View File

@ -60,7 +60,7 @@ pub fn deserialize<T: Decodable>(data: &[u8]) -> Result<T, Error> {
if consumed == data.len() { if consumed == data.len() {
Ok(rv) Ok(rv)
} else { } 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] #[inline]
fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<String, Error> { fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<String, Error> {
String::from_utf8(Decodable::consensus_decode(r)?) 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] #[inline]
fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Cow<'static, str>, Error> { fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Cow<'static, str>, Error> {
String::from_utf8(Decodable::consensus_decode(r)?) 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) .map(Cow::Owned)
} }
} }

View File

@ -155,3 +155,7 @@ impl From<OddLengthStringError> for FromHexError {
#[inline] #[inline]
fn from(e: OddLengthStringError) -> Self { Self::OddLengthString(e) } 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) }

View File

@ -22,6 +22,7 @@ pub use self::{
encode::{deserialize, deserialize_partial, serialize, Decodable, Encodable, ReadExt, WriteExt}, encode::{deserialize, deserialize_partial, serialize, Decodable, Encodable, ReadExt, WriteExt},
error::{Error, FromHexError, DecodeError}, error::{Error, FromHexError, DecodeError},
}; };
pub(crate) use self::error::parse_failed_error;
struct IterReader<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> { struct IterReader<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> {
iterator: core::iter::Fuse<I>, iterator: core::iter::Fuse<I>,

View File

@ -10,6 +10,7 @@ use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6, ToSoc
use io::{BufRead, Read, Write}; use io::{BufRead, Read, Write};
use crate::consensus;
use crate::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt}; use crate::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt};
use crate::p2p::ServiceFlags; use crate::p2p::ServiceFlags;
@ -165,28 +166,28 @@ impl Decodable for AddrV2 {
let network_id = u8::consensus_decode(r)?; let network_id = u8::consensus_decode(r)?;
let len = r.read_compact_size()?; let len = r.read_compact_size()?;
if len > 512 { 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 { Ok(match network_id {
1 => { 1 => {
if len != 4 { 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)?; let addr: [u8; 4] = Decodable::consensus_decode(r)?;
AddrV2::Ipv4(Ipv4Addr::new(addr[0], addr[1], addr[2], addr[3])) AddrV2::Ipv4(Ipv4Addr::new(addr[0], addr[1], addr[2], addr[3]))
} }
2 => { 2 => {
if len != 16 { 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)?; let addr: [u16; 8] = read_be_address(r)?;
if addr[0..3] == ONION { if addr[0..3] == ONION {
return Err(encode::Error::ParseFailed( return Err(consensus::parse_failed_error(
"OnionCat address sent with IPv6 network id", "OnionCat address sent with IPv6 network id",
)); ));
} }
if addr[0..6] == [0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0xFFFF] { 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", "IPV4 wrapped address sent with IPv6 network id",
)); ));
} }
@ -196,33 +197,33 @@ impl Decodable for AddrV2 {
} }
3 => { 3 => {
if len != 10 { 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)?; let id = Decodable::consensus_decode(r)?;
AddrV2::TorV2(id) AddrV2::TorV2(id)
} }
4 => { 4 => {
if len != 32 { 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)?; let pubkey = Decodable::consensus_decode(r)?;
AddrV2::TorV3(pubkey) AddrV2::TorV3(pubkey)
} }
5 => { 5 => {
if len != 32 { 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)?; let hash = Decodable::consensus_decode(r)?;
AddrV2::I2p(hash) AddrV2::I2p(hash)
} }
6 => { 6 => {
if len != 16 { 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)?; let addr: [u16; 8] = read_be_address(r)?;
// check the first byte for the CJDNS marker // check the first byte for the CJDNS marker
if addr[0] >> 8 != 0xFC { 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( AddrV2::Cjdns(Ipv6Addr::new(
addr[0], addr[1], addr[2], addr[3], addr[4], addr[5], addr[6], addr[7], addr[0], addr[1], addr[2], addr[3], addr[4], addr[5], addr[6], addr[7],

View File

@ -19,7 +19,7 @@ use crate::p2p::{
Magic, Magic,
}; };
use crate::prelude::{Box, Cow, String, ToOwned, Vec}; 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. /// 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 { for _ in 0..len {
ret.push(Decodable::consensus_decode(r)?); ret.push(Decodable::consensus_decode(r)?);
if u8::consensus_decode(r)? != 0u8 { if u8::consensus_decode(r)? != 0u8 {
return Err(encode::Error::ParseFailed( return Err(consensus::parse_failed_error(
"Headers message should not contain transactions", "Headers message should not contain transactions",
)); ));
} }

View File

@ -6,7 +6,7 @@
use io::{BufRead, Write}; 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::internal_macros::impl_consensus_encoding;
/// `filterload` message sets the current bloom filter /// `filterload` message sets the current bloom filter
@ -52,7 +52,7 @@ impl Decodable for BloomFlags {
0 => BloomFlags::None, 0 => BloomFlags::None,
1 => BloomFlags::All, 1 => BloomFlags::All,
2 => BloomFlags::PubkeyOnly, 2 => BloomFlags::PubkeyOnly,
_ => return Err(encode::Error::ParseFailed("unknown bloom flag")), _ => return Err(consensus::parse_failed_error("unknown bloom flag")),
}) })
} }
} }

View File

@ -8,7 +8,7 @@
use hashes::sha256d; use hashes::sha256d;
use io::{BufRead, Write}; 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::internal_macros::impl_consensus_encoding;
use crate::p2p; use crate::p2p;
use crate::p2p::address::Address; use crate::p2p::address::Address;
@ -126,7 +126,7 @@ impl Decodable for RejectReason {
0x41 => RejectReason::Dust, 0x41 => RejectReason::Dust,
0x42 => RejectReason::Fee, 0x42 => RejectReason::Fee,
0x43 => RejectReason::Checkpoint, 0x43 => RejectReason::Checkpoint,
_ => return Err(encode::Error::ParseFailed("unknown reject code")), _ => return Err(consensus::parse_failed_error("unknown reject code")),
}) })
} }
} }