Skip to content

Commit b868835

Browse files
committed
Merge rust-bitcoin#4915: units: Deprecate from_witness_data_size and from_non_witness_data_size
0cd384d Update `segwit_weight` and `legacy_weight` to use non-deprecated functions (Shing Him Ng) a11c037 Remove deprecated constructors from fuzz target (Shing Him Ng) fcc3473 Update `Weight` and `TxInExt` functions with panic details (Shing Him Ng) Pull request description: Weight::from_non_witness_data_size has an overflow bug. These methods duplicate the functionality of `Weight::from_vb` and `Weight::from_wu`, respectively. Furthermore, they are less discoverable than the methods they duplicate, so they will be removed in this PR. Related discussion in rust-bitcoin#4847 ACKs for top commit: tcharding: ACK 0cd384d apoelstra: ACK 0cd384d; successfully ran local tests Tree-SHA512: 56a370f02edde7108fa90120e359063788f2d2ce4b5b665216dfe67fe76485f79ba9a5d0c2fd454a8fe04795315bd8e81777c48ae39e663d4559bbf57d0e74cd
2 parents d26ef5d + 0cd384d commit b868835

File tree

4 files changed

+31
-5
lines changed

4 files changed

+31
-5
lines changed

bitcoin/src/blockdata/block.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ mod tests {
659659
assert_eq!(block_base_size(real_decode.transactions()), some_block.len());
660660
assert_eq!(
661661
real_decode.weight(),
662-
Weight::from_non_witness_data_size(some_block.len().to_u64())
662+
Weight::from_vb_unchecked(some_block.len().to_u64())
663663
);
664664

665665
assert_eq!(serialize(&real_decode), some_block);

bitcoin/src/blockdata/transaction.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,12 @@ internal_macros::define_extension_trait! {
134134
/// Keep in mind that when adding a TxIn to a transaction, the total weight of the transaction
135135
/// might increase more than `TxIn::legacy_weight`. This happens when the new input added causes
136136
/// the input length `CompactSize` to increase its encoding length.
137+
///
138+
/// # Panics
139+
///
140+
/// If the coversion overflows.
137141
fn legacy_weight(&self) -> Weight {
138-
Weight::from_non_witness_data_size(self.base_size().to_u64())
142+
Weight::from_vb(self.base_size().to_u64()).unwrap()
139143
}
140144

141145
/// The weight of the TxIn when it's included in a SegWit transaction (i.e., a transaction
@@ -149,14 +153,22 @@ internal_macros::define_extension_trait! {
149153
/// - the new input added causes the input length `CompactSize` to increase its encoding length
150154
/// - the new input is the first segwit input added - this will add an additional 2WU to the
151155
/// transaction weight to take into account the SegWit marker
156+
///
157+
/// # Panics
158+
///
159+
/// If the coversion overflows.
152160
fn segwit_weight(&self) -> Weight {
153-
Weight::from_non_witness_data_size(self.base_size().to_u64())
154-
+ Weight::from_witness_data_size(self.witness.size().to_u64())
161+
Weight::from_vb(self.base_size().to_u64())
162+
.and_then(|w| w.checked_add(Weight::from_wu(self.witness.size().to_u64()))).unwrap()
155163
}
156164

157165
/// Returns the base size of this input.
158166
///
159167
/// Base size excludes the witness data (see [`Self::total_size`]).
168+
///
169+
/// # Panics
170+
///
171+
/// If the size calculation overflows.
160172
fn base_size(&self) -> usize {
161173
let mut size = OutPoint::SIZE;
162174

@@ -169,6 +181,10 @@ internal_macros::define_extension_trait! {
169181
/// Returns the total number of bytes that this input contributes to a transaction.
170182
///
171183
/// Total size includes the witness data (for base size see [`Self::base_size`]).
184+
///
185+
/// # Panics
186+
///
187+
/// If the size calculation overflows.
172188
fn total_size(&self) -> usize { self.base_size() + self.witness.size() }
173189
}
174190
}

fuzz/fuzz_targets/units/arbitrary_weight.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn do_test(data: &[u8]) {
3434

3535
// Constructors that return a Weight
3636
for constructor in
37-
[Weight::from_wu, Weight::from_witness_data_size, Weight::from_non_witness_data_size]
37+
[Weight::from_wu]
3838
{
3939
if let Ok(val) = u.arbitrary() {
4040
constructor(val);

units/src/weight.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,15 @@ impl Weight {
9696
}
9797

9898
/// Constructs a new [`Weight`] from witness size.
99+
#[deprecated(since = "TBD", note = "use `from_wu` instead")]
99100
pub const fn from_witness_data_size(witness_size: u64) -> Self { Weight::from_wu(witness_size) }
100101

101102
/// Constructs a new [`Weight`] from non-witness size.
103+
///
104+
/// # Panics
105+
///
106+
/// If the conversion from virtual bytes overflows.
107+
#[deprecated(since = "TBD", note = "use `from_vb` or `from_vb_unchecked` instead")]
102108
pub const fn from_non_witness_data_size(non_witness_size: u64) -> Self {
103109
Weight::from_wu(non_witness_size * Self::WITNESS_SCALE_FACTOR)
104110
}
@@ -362,6 +368,8 @@ mod tests {
362368
fn from_vb_unchecked_panic() { Weight::from_vb_unchecked(u64::MAX); }
363369

364370
#[test]
371+
#[allow(deprecated)] // tests the deprecated function
372+
#[allow(deprecated_in_future)]
365373
fn from_witness_data_size() {
366374
let witness_data_size = 1;
367375
let got = Weight::from_witness_data_size(witness_data_size);
@@ -370,6 +378,8 @@ mod tests {
370378
}
371379

372380
#[test]
381+
#[allow(deprecated)] // tests the deprecated function
382+
#[allow(deprecated_in_future)]
373383
fn from_non_witness_data_size() {
374384
let non_witness_data_size = 1;
375385
let got = Weight::from_non_witness_data_size(non_witness_data_size);

0 commit comments

Comments
 (0)