Skip to content

Replace the generic parse_int_be with a macro called twice #2934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Replace the generic parse_int_be with a macro called twice #2934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c89b96a
Use std's `from_be_bytes` rather than our `to_int_be` for int conv
TheBlueMatt Mar 13, 2024
39c1d6b
Replace the generic `parse_int_be` with a macro called twice
TheBlueMatt Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lightning-invoice/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@ rustdoc-args = ["--cfg", "docsrs"]
[features]
default = ["std"]
no-std = ["lightning/no-std"]
std = ["bitcoin/std", "num-traits/std", "lightning/std", "bech32/std"]
std = ["bitcoin/std", "lightning/std", "bech32/std"]

[dependencies]
bech32 = { version = "0.9.0", default-features = false }
lightning = { version = "0.0.121", path = "../lightning", default-features = false }
secp256k1 = { version = "0.27.0", default-features = false, features = ["recovery", "alloc"] }
num-traits = { version = "0.2.8", default-features = false }
serde = { version = "1.0.118", optional = true }
bitcoin = { version = "0.30.2", default-features = false }

Expand Down
60 changes: 31 additions & 29 deletions lightning-invoice/src/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ use lightning::ln::PaymentSecret;
use lightning::routing::gossip::RoutingFees;
use lightning::routing::router::{RouteHint, RouteHintHop};

use num_traits::{CheckedAdd, CheckedMul};

use secp256k1::ecdsa::{RecoveryId, RecoverableSignature};
use secp256k1::PublicKey;

Expand Down Expand Up @@ -356,7 +354,7 @@ impl FromBase32 for PositiveTimestamp {
if b32.len() != 7 {
return Err(Bolt11ParseError::InvalidSliceLength("PositiveTimestamp::from_base32()".into()));
}
let timestamp: u64 = parse_int_be(b32, 32)
let timestamp: u64 = parse_u64_be(b32)
.expect("7*5bit < 64bit, no overflow possible");
match PositiveTimestamp::from_unix_timestamp(timestamp) {
Ok(t) => Ok(t),
Expand All @@ -382,16 +380,17 @@ impl FromBase32 for Bolt11InvoiceSignature {
}
}

pub(crate) fn parse_int_be<T, U>(digits: &[U], base: T) -> Option<T>
where T: CheckedAdd + CheckedMul + From<u8> + Default,
U: Into<u8> + Copy
{
digits.iter().fold(Some(Default::default()), |acc, b|
acc
.and_then(|x| x.checked_mul(&base))
.and_then(|x| x.checked_add(&(Into::<u8>::into(*b)).into()))
)
}
macro_rules! define_parse_int_be { ($name: ident, $ty: ty) => {
fn $name(digits: &[u5]) -> Option<$ty> {
digits.iter().fold(Some(Default::default()), |acc, b|
acc
.and_then(|x| x.checked_mul(32))
.and_then(|x| x.checked_add((Into::<u8>::into(*b)).into()))
)
}
} }
define_parse_int_be!(parse_u16_be, u16);
define_parse_int_be!(parse_u64_be, u64);

fn parse_tagged_parts(data: &[u5]) -> Result<Vec<RawTaggedField>, Bolt11ParseError> {
let mut parts = Vec::<RawTaggedField>::new();
Expand All @@ -404,7 +403,7 @@ fn parse_tagged_parts(data: &[u5]) -> Result<Vec<RawTaggedField>, Bolt11ParseErr

// Ignore tag at data[0], it will be handled in the TaggedField parsers and
// parse the length to find the end of the tagged field's data
let len = parse_int_be(&data[1..3], 32).expect("can't overflow");
let len = parse_u16_be(&data[1..3]).expect("can't overflow") as usize;
let last_element = 3 + len;

if data.len() < last_element {
Expand Down Expand Up @@ -517,7 +516,7 @@ impl FromBase32 for ExpiryTime {
type Err = Bolt11ParseError;

fn from_base32(field_data: &[u5]) -> Result<ExpiryTime, Bolt11ParseError> {
match parse_int_be::<u64, u5>(field_data, 32)
match parse_u64_be(field_data)
.map(ExpiryTime::from_seconds)
{
Some(t) => Ok(t),
Expand All @@ -530,7 +529,7 @@ impl FromBase32 for MinFinalCltvExpiryDelta {
type Err = Bolt11ParseError;

fn from_base32(field_data: &[u5]) -> Result<MinFinalCltvExpiryDelta, Bolt11ParseError> {
let expiry = parse_int_be::<u64, u5>(field_data, 32);
let expiry = parse_u64_be(field_data);
if let Some(expiry) = expiry {
Ok(MinFinalCltvExpiryDelta(expiry))
} else {
Expand Down Expand Up @@ -602,12 +601,12 @@ impl FromBase32 for PrivateRoute {

let hop = RouteHintHop {
src_node_id: PublicKey::from_slice(&hop_bytes[0..33])?,
short_channel_id: parse_int_be(&channel_id, 256).expect("short chan ID slice too big?"),
short_channel_id: u64::from_be_bytes(channel_id),
fees: RoutingFees {
base_msat: parse_int_be(&hop_bytes[41..45], 256).expect("slice too big?"),
proportional_millionths: parse_int_be(&hop_bytes[45..49], 256).expect("slice too big?"),
base_msat: u32::from_be_bytes(hop_bytes[41..45].try_into().expect("slice too big?")),
proportional_millionths: u32::from_be_bytes(hop_bytes[45..49].try_into().expect("slice too big?")),
},
cltv_expiry_delta: parse_int_be(&hop_bytes[49..51], 256).expect("slice too big?"),
cltv_expiry_delta: u16::from_be_bytes(hop_bytes[49..51].try_into().expect("slice too big?")),
htlc_minimum_msat: None,
htlc_maximum_msat: None,
};
Expand Down Expand Up @@ -761,12 +760,16 @@ mod test {

#[test]
fn test_parse_int_from_bytes_be() {
use crate::de::parse_int_be;

assert_eq!(parse_int_be::<u32, u8>(&[1, 2, 3, 4], 256), Some(16909060));
assert_eq!(parse_int_be::<u32, u8>(&[1, 3], 32), Some(35));
assert_eq!(parse_int_be::<u32, u8>(&[255, 255, 255, 255], 256), Some(4294967295));
assert_eq!(parse_int_be::<u32, u8>(&[1, 0, 0, 0, 0], 256), None);
use crate::de::parse_u16_be;

assert_eq!(parse_u16_be(&[
u5::try_from_u8(1).unwrap(), u5::try_from_u8(2).unwrap(),
u5::try_from_u8(3).unwrap(), u5::try_from_u8(4).unwrap()]
), Some(34916));
assert_eq!(parse_u16_be(&[
u5::try_from_u8(2).unwrap(), u5::try_from_u8(0).unwrap(),
u5::try_from_u8(0).unwrap(), u5::try_from_u8(0).unwrap()]
), None);
}

#[test]
Expand Down Expand Up @@ -916,7 +919,6 @@ mod test {
use lightning::routing::router::{RouteHint, RouteHintHop};
use crate::PrivateRoute;
use bech32::FromBase32;
use crate::de::parse_int_be;

let input = from_bech32(
"q20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqa\
Expand All @@ -932,7 +934,7 @@ mod test {
0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
][..]
).unwrap(),
short_channel_id: parse_int_be(&[0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08], 256).expect("short chan ID slice too big?"),
short_channel_id: 0x0102030405060708,
fees: RoutingFees {
base_msat: 1,
proportional_millionths: 20,
Expand All @@ -949,7 +951,7 @@ mod test {
0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
][..]
).unwrap(),
short_channel_id: parse_int_be(&[0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a], 256).expect("short chan ID slice too big?"),
short_channel_id: 0x030405060708090a,
fees: RoutingFees {
base_msat: 2,
proportional_millionths: 30,
Expand Down
7 changes: 3 additions & 4 deletions lightning-invoice/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ pub mod utils;

extern crate bech32;
#[macro_use] extern crate lightning;
extern crate num_traits;
extern crate secp256k1;
extern crate alloc;
#[cfg(any(test, feature = "std"))]
Expand Down Expand Up @@ -2064,7 +2063,7 @@ mod test {
let route_1 = RouteHint(vec![
RouteHintHop {
src_node_id: public_key,
short_channel_id: de::parse_int_be(&[123; 8], 256).expect("short chan ID slice too big?"),
short_channel_id: u64::from_be_bytes([123; 8]),
fees: RoutingFees {
base_msat: 2,
proportional_millionths: 1,
Expand All @@ -2075,7 +2074,7 @@ mod test {
},
RouteHintHop {
src_node_id: public_key,
short_channel_id: de::parse_int_be(&[42; 8], 256).expect("short chan ID slice too big?"),
short_channel_id: u64::from_be_bytes([42; 8]),
fees: RoutingFees {
base_msat: 3,
proportional_millionths: 2,
Expand All @@ -2100,7 +2099,7 @@ mod test {
},
RouteHintHop {
src_node_id: public_key,
short_channel_id: de::parse_int_be(&[1; 8], 256).expect("short chan ID slice too big?"),
short_channel_id: u64::from_be_bytes([1; 8]),
fees: RoutingFees {
base_msat: 5,
proportional_millionths: 4,
Expand Down