Skip to content

Commit

Permalink
Extend ParseIntError to carry more context
Browse files Browse the repository at this point in the history
When debugging parsing errors it's very useful to know some context:
what the input was and what integer type was parsed. `ParseIntError`
from `core` doesn't contain this information.

In this commit a custom `ParseIntError` type is crated that carries the
one from `core` as well as additional information. Its `Display`
implementation displays this additional information as a well-formed
English sentence to aid users with understanding the problem. A helper
function parses any integer type from common string types returning the
new `ParseIntError` type on error.

To clean up the error code a bit some new macros are added and used.
New modules are added to organize the types, functions and macros.

Closes #1113
  • Loading branch information
Kixunil committed Jul 26, 2022
1 parent 58c1afe commit 3ba3e60
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 53 deletions.
90 changes: 43 additions & 47 deletions src/blockdata/locktime.rs
Expand Up @@ -21,7 +21,8 @@ use core::{mem, fmt};
use core::cmp::{PartialOrd, Ordering};
use core::convert::TryFrom;
use core::str::FromStr;
use core::num::ParseIntError;
use crate::error::ParseIntError;
use crate::parse::int as parse_int;

use crate::consensus::encode::{self, Decodable, Encodable};
use crate::io::{self, Read, Write};
Expand Down Expand Up @@ -134,30 +135,39 @@ impl From<PackedLockTime> for u32 {
}
}

impl FromStr for PackedLockTime {
type Err = ParseIntError;
/// Implements `TryFrom<$from> for $to` using `parse_int`, mapping the output using `fn`
macro_rules! impl_tryfrom_str_single {
($($from:ty, $to:ident $(, $fn:ident)?);*) => {
$(
impl TryFrom<$from> for $to {
type Error = ParseIntError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
s.parse().map(PackedLockTime)
fn try_from(s: $from) -> Result<Self, Self::Error> {
parse_int(s).map($to $(:: $fn)?)
}
}
)*
}
}

impl TryFrom<&str> for PackedLockTime {
type Error = ParseIntError;
/// Implements `TryFrom<{&str, String, Box<str>}> for $to` using `parse_int`, mapping the output using `fn`
macro_rules! impl_tryfrom_str {
($to:ident $(, $fn:ident)?) => {
impl_tryfrom_str_single!(&str, $to $(, $fn)?; String, $to $(, $fn)?; Box<str>, $to $(, $fn)?);

fn try_from(s: &str) -> Result<Self, Self::Error> {
PackedLockTime::from_str(s)
}
}
impl FromStr for $to {
type Err = ParseIntError;

impl TryFrom<String> for PackedLockTime {
type Error = ParseIntError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
parse_int(s).map($to $(:: $fn)?)
}
}

fn try_from(s: String) -> Result<Self, Self::Error> {
PackedLockTime::from_str(&s)
}
}

impl_tryfrom_str!(PackedLockTime);

impl fmt::LowerHex for PackedLockTime {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{:x}", self.0)
Expand Down Expand Up @@ -366,29 +376,7 @@ impl LockTime {
}
}

impl FromStr for LockTime {
type Err = ParseIntError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
s.parse().map(LockTime::from_consensus)
}
}

impl TryFrom<&str> for LockTime {
type Error = ParseIntError;

fn try_from(s: &str) -> Result<Self, Self::Error> {
LockTime::from_str(s)
}
}

impl TryFrom<String> for LockTime {
type Error = ParseIntError;

fn try_from(s: String) -> Result<Self, Self::Error> {
LockTime::from_str(&s)
}
}
impl_tryfrom_str!(LockTime, from_consensus);

impl From<Height> for LockTime {
fn from(h: Height) -> Self {
Expand Down Expand Up @@ -503,7 +491,7 @@ impl FromStr for Height {
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let n = s.parse::<u32>().map_err(|e| Error::Parse(e, s.to_owned()))?;
let n = parse_int(s)?;
Height::from_consensus(n)
}
}
Expand All @@ -512,15 +500,16 @@ impl TryFrom<&str> for Height {
type Error = Error;

fn try_from(s: &str) -> Result<Self, Self::Error> {
Height::from_str(s)
let n = parse_int(s)?;
Height::from_consensus(n)
}
}

impl TryFrom<String> for Height {
type Error = Error;

fn try_from(s: String) -> Result<Self, Self::Error> {
let n = s.parse::<u32>().map_err(|e| Error::Parse(e, s))?;
let n = parse_int(s)?;
Height::from_consensus(n)
}
}
Expand Down Expand Up @@ -585,7 +574,7 @@ impl FromStr for Time {
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let n = s.parse::<u32>().map_err(|e| Error::Parse(e, s.to_owned()))?;
let n = parse_int(s)?;
Time::from_consensus(n)
}
}
Expand All @@ -594,15 +583,16 @@ impl TryFrom<&str> for Time {
type Error = Error;

fn try_from(s: &str) -> Result<Self, Self::Error> {
Time::from_str(s)
let n = parse_int(s)?;
Time::from_consensus(n)
}
}

impl TryFrom<String> for Time {
type Error = Error;

fn try_from(s: String) -> Result<Self, Self::Error> {
let n = s.parse::<u32>().map_err(|e| Error::Parse(e, s))?;
let n = parse_int(s)?;
Time::from_consensus(n)
}
}
Expand All @@ -626,7 +616,7 @@ pub enum Error {
/// An error occurred while operating on lock times.
Operation(OperationError),
/// An error occurred while parsing a string into an `u32`.
Parse(ParseIntError, String),
Parse(ParseIntError),
}

impl fmt::Display for Error {
Expand All @@ -636,7 +626,7 @@ impl fmt::Display for Error {
match *self {
Conversion(ref e) => write_err!(f, "error converting lock time value"; e),
Operation(ref e) => write_err!(f, "error during lock time operation"; e),
Parse(ref e, ref s) => write_err!(f, "error parsing string: {}", s; e),
Parse(ref e) => write_err!(f, "failed to parse lock time from string"; e),
}
}
}
Expand All @@ -650,7 +640,7 @@ impl std::error::Error for Error {
match *self {
Conversion(ref e) => Some(e),
Operation(ref e) => Some(e),
Parse(ref e, _) => Some(e),
Parse(ref e) => Some(e),
}
}
}
Expand All @@ -667,6 +657,12 @@ impl From<OperationError> for Error {
}
}

impl From<ParseIntError> for Error {
fn from(e: ParseIntError) -> Self {
Error::Parse(e)
}
}

/// An error that occurs when converting a `u32` to a lock time variant.
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct ConversionError {
Expand Down
6 changes: 3 additions & 3 deletions src/blockdata/transaction.rs
Expand Up @@ -107,7 +107,7 @@ pub enum ParseOutPointError {
/// Error in TXID part.
Txid(hashes::hex::Error),
/// Error in vout part.
Vout(::core::num::ParseIntError),
Vout(crate::error::ParseIntError),
/// Error in general format.
Format,
/// Size exceeds max.
Expand Down Expand Up @@ -151,7 +151,7 @@ fn parse_vout(s: &str) -> Result<u32, ParseOutPointError> {
return Err(ParseOutPointError::VoutNotCanonical);
}
}
s.parse().map_err(ParseOutPointError::Vout)
crate::parse::int(s).map_err(ParseOutPointError::Vout)
}

impl core::str::FromStr for OutPoint {
Expand Down Expand Up @@ -1308,7 +1308,7 @@ mod tests {
assert_eq!(OutPoint::from_str("5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c945X:1"),
Err(ParseOutPointError::Txid(Txid::from_hex("5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c945X").unwrap_err())));
assert_eq!(OutPoint::from_str("5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456:lol"),
Err(ParseOutPointError::Vout(u32::from_str("lol").unwrap_err())));
Err(ParseOutPointError::Vout(crate::parse::int::<u32, _>("lol").unwrap_err())));

assert_eq!(OutPoint::from_str("5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456:42"),
Ok(OutPoint{
Expand Down
2 changes: 1 addition & 1 deletion src/internal_macros.rs
Expand Up @@ -581,7 +581,7 @@ pub(crate) use user_enum;
/// because `e.source()` is only available in std builds, without this macro the error source is
/// lost for no-std builds.
macro_rules! write_err {
($writer:expr, $string:literal $(, $args:expr),*; $source:expr) => {
($writer:expr, $string:literal $(, $args:expr)*; $source:expr) => {
{
#[cfg(feature = "std")]
{
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Expand Up @@ -78,11 +78,13 @@ mod test_macros;
mod internal_macros;
#[cfg(feature = "serde")]
mod serde_utils;
mod parse;

#[macro_use]
pub mod network;
pub mod blockdata;
pub mod consensus;
pub mod error;
pub mod hash_types;
pub mod policy;
pub mod util;
Expand Down
4 changes: 2 additions & 2 deletions src/util/address.rs
Expand Up @@ -26,7 +26,7 @@ use crate::prelude::*;

use core::convert::TryFrom;
use core::fmt;
use core::num::ParseIntError;
use crate::error::ParseIntError;
use core::str::FromStr;

use secp256k1::{Secp256k1, Verification, XOnlyPublicKey};
Expand Down Expand Up @@ -239,7 +239,7 @@ impl FromStr for WitnessVersion {
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let version: u8 = s.parse().map_err(Error::UnparsableWitnessVersion)?;
let version: u8 = crate::parse::int(s).map_err(Error::UnparsableWitnessVersion)?;
WitnessVersion::try_from(version)
}
}
Expand Down

0 comments on commit 3ba3e60

Please sign in to comment.