From 8552534b612dc7d059368e6c630c16640379b949 Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 24 Jun 2025 11:20:01 -0500 Subject: [PATCH 1/4] Use u32 for struct and member variables in IWP, saturating to u32::MAX To prevent panics during addition if `usize` is `u64`, use `u32` member variables internally. TK use `u32::saturating_add` instead of basic addition. However, to use `u32::saturating_add()`, the variables need to be of type `u32`. Therefore, this commit transforms the internal types to `u32`. In so doing, add a `const` function `saturate_to_u32()` which saturates a usize to `u32`. Also, replace `compact_size::encoded_size()` with a new function `encoded_size()` which returns a `u32`, avoiding needless casts. --- bitcoin/src/blockdata/transaction.rs | 37 ++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 0e64bdd3c..f9a31b30b 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -932,8 +932,8 @@ pub const fn predict_weight_from_slices( /// associated constants/methods. #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub struct InputWeightPrediction { - script_size: usize, - witness_size: usize, + script_size: u32, + witness_size: u32, } impl InputWeightPrediction { @@ -996,6 +996,23 @@ impl InputWeightPrediction { /// [`InputWeightPrediction::new`]. pub const P2TR_KEY_NON_DEFAULT_SIGHASH: Self = InputWeightPrediction::from_slice(0, &[65]); + const fn saturate_to_u32(x: usize) -> u32 { + if x > u32::MAX as usize { + u32::MAX + } else { + x as u32 //cast ok, condition prevents larger than u32::MAX. + } + } + + const fn encoded_size(value: usize) -> u32 { + match value { + 0..=0xFC => 1, + 0xFD..=0xFFFF => 3, + 0x10000..=0xFFFFFFFF => 5, + _ => 9, + } + } + /// Input weight prediction corresponding to spending of P2WPKH output using [signature /// grinding]. /// @@ -1065,16 +1082,16 @@ impl InputWeightPrediction { T::Item: Borrow, { let (count, total_size) = witness_element_lengths.into_iter().fold( - (0usize, 0), + (0usize, 0u32), |(count, total_size), elem_len| { let elem_len = *elem_len.borrow(); - let elem_size = elem_len + compact_size::encoded_size(elem_len); + let elem_size = Self::saturate_to_u32(elem_len) + Self::encoded_size(elem_len); (count + 1, total_size + elem_size) }, ); - let witness_size = - if count > 0 { total_size + compact_size::encoded_size(count) } else { 0 }; - let script_size = input_script_len + compact_size::encoded_size(input_script_len); + let witness_size = if count > 0 { total_size + Self::encoded_size(count) } else { 0 }; + let script_size = + Self::saturate_to_u32(input_script_len) + Self::encoded_size(input_script_len); InputWeightPrediction { script_size, witness_size } } @@ -1090,17 +1107,17 @@ impl InputWeightPrediction { // for loops not supported in const fn while i < witness_element_lengths.len() { let elem_len = witness_element_lengths[i]; - let elem_size = elem_len + compact_size::encoded_size_const(elem_len as u64); + let elem_size = Self::saturate_to_u32(elem_len) + Self::encoded_size(elem_len); total_size += elem_size; i += 1; } let witness_size = if !witness_element_lengths.is_empty() { - total_size + compact_size::encoded_size_const(witness_element_lengths.len() as u64) + total_size + Self::encoded_size(witness_element_lengths.len()) } else { 0 }; let script_size = - input_script_len + compact_size::encoded_size_const(input_script_len as u64); + Self::saturate_to_u32(input_script_len) + Self::encoded_size(input_script_len); InputWeightPrediction { script_size, witness_size } } From e4c3d1e7a62a446d930e2a73dc98415999aefdcb Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 17 Jun 2025 13:18:16 -0500 Subject: [PATCH 2/4] Use saturating add in IWP constructors In order to avoid panics during weight prediction replace addition with `u32::saturating_add()`. --- bitcoin/src/blockdata/transaction.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index f9a31b30b..690996ece 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -1085,8 +1085,9 @@ impl InputWeightPrediction { (0usize, 0u32), |(count, total_size), elem_len| { let elem_len = *elem_len.borrow(); - let elem_size = Self::saturate_to_u32(elem_len) + Self::encoded_size(elem_len); - (count + 1, total_size + elem_size) + let elem_size = + Self::saturate_to_u32(elem_len).saturating_add(Self::encoded_size(elem_len)); + (count + 1, total_size.saturating_add(elem_size)) }, ); let witness_size = if count > 0 { total_size + Self::encoded_size(count) } else { 0 }; @@ -1103,21 +1104,22 @@ impl InputWeightPrediction { /// `new` and thus is intended to be only used in `const` context. pub const fn from_slice(input_script_len: usize, witness_element_lengths: &[usize]) -> Self { let mut i = 0; - let mut total_size = 0; + let mut total_size: u32 = 0; // for loops not supported in const fn while i < witness_element_lengths.len() { let elem_len = witness_element_lengths[i]; - let elem_size = Self::saturate_to_u32(elem_len) + Self::encoded_size(elem_len); - total_size += elem_size; + let elem_size = + Self::saturate_to_u32(elem_len).saturating_add(Self::encoded_size(elem_len)); + total_size = total_size.saturating_add(elem_size); i += 1; } let witness_size = if !witness_element_lengths.is_empty() { - total_size + Self::encoded_size(witness_element_lengths.len()) + total_size.saturating_add(Self::encoded_size(witness_element_lengths.len())) } else { 0 }; - let script_size = - Self::saturate_to_u32(input_script_len) + Self::encoded_size(input_script_len); + let script_size = Self::saturate_to_u32(input_script_len) + .saturating_add(Self::encoded_size(input_script_len)); InputWeightPrediction { script_size, witness_size } } From 8559a49e03be2b8b3ea4215227ae003e822a663c Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 27 May 2025 18:58:02 -0500 Subject: [PATCH 3/4] Do not bound Arbitrary parameters passed to InputWeightPrediction Now that InputWeightPrediction can no longer overflow due to extreme values, there is no longer need to bound the Arbitrary parameters passed. --- bitcoin/src/blockdata/transaction.rs | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 690996ece..2eecfed62 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -1165,22 +1165,6 @@ mod sealed { #[cfg(feature = "arbitrary")] impl<'a> Arbitrary<'a> for InputWeightPrediction { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { - // limit script size to 4Mwu block size. - let max_block = Weight::MAX_BLOCK.to_wu() as usize; - let input_script_len = u.int_in_range(0..=max_block)?; - let remaining = max_block - input_script_len; - - // create witness data if there is remaining space. - let mut witness_length = u.int_in_range(0..=remaining)?; - let mut witness_element_lengths = Vec::new(); - - // build vec of random witness element lengths. - while witness_length > 0 { - let elem = u.int_in_range(1..=witness_length)?; - witness_element_lengths.push(elem); - witness_length -= elem; - } - match u.int_in_range(0..=7)? { 0 => Ok(InputWeightPrediction::P2WPKH_MAX), 1 => Ok(InputWeightPrediction::NESTED_P2WPKH_MAX), @@ -1188,8 +1172,16 @@ impl<'a> Arbitrary<'a> for InputWeightPrediction { 3 => Ok(InputWeightPrediction::P2PKH_UNCOMPRESSED_MAX), 4 => Ok(InputWeightPrediction::P2TR_KEY_DEFAULT_SIGHASH), 5 => Ok(InputWeightPrediction::P2TR_KEY_NON_DEFAULT_SIGHASH), - 6 => Ok(InputWeightPrediction::new(input_script_len, witness_element_lengths)), - _ => Ok(InputWeightPrediction::from_slice(input_script_len, &witness_element_lengths)), + 6 => { + let input_script_len = usize::arbitrary(u)?; + let witness_element_lengths: Vec = Vec::arbitrary(u)?; + Ok(InputWeightPrediction::new(input_script_len, witness_element_lengths)) + } + _ => { + let input_script_len = usize::arbitrary(u)?; + let witness_element_lengths: Vec = Vec::arbitrary(u)?; + Ok(InputWeightPrediction::from_slice(input_script_len, &witness_element_lengths)) + } } } } From 3355400d6706ce8fee3daa258e9dbbd648a87dca Mon Sep 17 00:00:00 2001 From: yancy Date: Thu, 19 Jun 2025 13:27:40 -0500 Subject: [PATCH 4/4] docs: document IWP function return limit and panic case Document the newly added saturation point for internal arithmetic. --- bitcoin/src/blockdata/transaction.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 2eecfed62..b5e81bae2 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -1126,12 +1126,18 @@ impl InputWeightPrediction { /// Computes the **signature weight** added to a transaction by an input with this weight prediction, /// not counting the prevout (txid, index), sequence, potential witness flag bytes or the witness count varint. + /// + /// This function's internal arithmetic saturates at u32::MAX, so the return value of this + /// function may be inaccurate for extremely large witness predictions. #[deprecated(since = "TBD", note = "use `InputWeightPrediction::witness_weight()` instead")] pub const fn weight(&self) -> Weight { Self::witness_weight(self) } /// Computes the signature, prevout (txid, index), and sequence weights of this weight /// prediction. /// + /// This function's internal arithmetic saturates at u32::MAX, so the return value of this + /// function may be inaccurate for extremely large witness predictions. + /// /// See also [`InputWeightPrediction::witness_weight`] pub const fn total_weight(&self) -> Weight { // `impl const Trait` is currently unavailable: rust/issues/67792 @@ -1142,6 +1148,9 @@ impl InputWeightPrediction { /// Computes the **signature weight** added to a transaction by an input with this weight prediction, /// not counting the prevout (txid, index), sequence, potential witness flag bytes or the witness count varint. + /// + /// This function's internal arithmetic saturates at u32::MAX, so the return value of this + /// function may be inaccurate for extremely large witness predictions. /// /// See also [`InputWeightPrediction::total_weight`] pub const fn witness_weight(&self) -> Weight {