Skip to content

Commit 2b3df6f

Browse files
committed
- Changed test file name descriptor_macro.rs
- Changed style for macro - Remove println from tests - Updated the valiid values and invalid values in test file `descriptor_macro.rs` - Removed impl with Error::Relocktime and made use of existing Error::Miniscript - Removed the RelLockTime enum Error in error.rs
1 parent 442c242 commit 2b3df6f

File tree

3 files changed

+48
-38
lines changed

3 files changed

+48
-38
lines changed

src/descriptor/dsl.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -724,10 +724,10 @@ macro_rules! fragment {
724724
$crate::impl_leaf_opcode_value!(After, $crate::miniscript::AbsLockTime::from_consensus($value).expect("valid `AbsLockTime`"))
725725
});
726726
( older ( $value:expr ) ) => ({
727-
match $crate::miniscript::RelLockTime::from_consensus($value) {
728-
Ok(lock_time) => $crate::impl_leaf_opcode_value!(Older, lock_time),
729-
Err(e) => Err($crate::descriptor::DescriptorError::RelLockTime(e)),
730-
}
727+
$crate::miniscript::RelLockTime::from_consensus($value)
728+
// Corrected variant name: RelativeLockTime (capital T)
729+
.map_err(|e| $crate::descriptor::DescriptorError::Miniscript($crate::miniscript::Error::RelativeLockTime(e)))
730+
.and_then(|rel_lock_time| $crate::impl_leaf_opcode_value!(Older, rel_lock_time))
731731
});
732732
( sha256 ( $hash:expr ) ) => ({
733733
$crate::impl_leaf_opcode_value!(Sha256, $hash)

src/descriptor/error.rs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ pub enum Error {
4444
Hex(bitcoin::hex::HexToBytesError),
4545
/// The provided wallet descriptors are identical
4646
ExternalAndInternalAreTheSame,
47-
/// RelLockTime validation error
48-
RelLockTime(miniscript::RelLockTimeError),
4947
}
5048

5149
impl From<crate::keys::KeyError> for Error {
@@ -86,13 +84,11 @@ impl fmt::Display for Error {
8684
Self::ExternalAndInternalAreTheSame => {
8785
write!(f, "External and internal descriptors are the same")
8886
}
89-
Self::RelLockTime(err) => write!(f, "RelLockTime error: {err}"),
9087
}
9188
}
9289
}
9390

94-
#[cfg(feature = "std")]
95-
impl std::error::Error for Error {}
91+
impl core::error::Error for Error {}
9692

9793
impl From<bitcoin::bip32::Error> for Error {
9894
fn from(err: bitcoin::bip32::Error) -> Self {
@@ -129,9 +125,3 @@ impl From<crate::descriptor::policy::PolicyError> for Error {
129125
Error::Policy(err)
130126
}
131127
}
132-
133-
impl From<miniscript::RelLockTimeError> for Error {
134-
fn from(err: miniscript::RelLockTimeError) -> Self {
135-
Error::RelLockTime(err)
136-
}
137-
}
Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/// Test to demonstrate the RelLockTime issue in dsl.rs
22
///
33
/// The issue: older() macro uses .expect() on RelLockTime::from_consensus(),
4-
/// which can panic if given an invalid value (> 16,777,215).
4+
/// which can panic if given an invalid value (> 65,535).
55
///
66
/// This file shows the problem and how to catch it.
77
use bdk_wallet::descriptor;
@@ -20,17 +20,18 @@ fn test_older_with_invalid_rellocktime_too_large() {
2020
);
2121

2222
// Check that it's the right kind of error
23-
if let Err(descriptor::DescriptorError::RelLockTime(_)) = result {
24-
// Good, it's the expected error type
23+
if let Err(descriptor::DescriptorError::Miniscript(miniscript::Error::RelativeLockTime(_))) =
24+
result
25+
{
2526
} else {
2627
panic!("Expected RelLockTime error, got {:?}", result);
2728
}
2829
}
2930

3031
#[test]
3132
fn test_older_with_valid_rellocktime_max() {
32-
// Max valid value is 16,777,215 (0xFFFFFF)
33-
let max_valid_value = 16_777_215;
33+
// Max valid value is 65,535 (0xFFFF)
34+
let max_valid_value = 65_535;
3435
let result = descriptor!(wsh(older(max_valid_value)));
3536
assert!(result.is_ok(), "Max valid RelLockTime should work");
3637
}
@@ -70,33 +71,52 @@ fn test_demonstrate_proper_error_handling() {
7071
use miniscript::RelLockTime;
7172

7273
// Valid case
73-
match RelLockTime::from_consensus(144) {
74-
Ok(lock_time) => println!("Valid: {:?}", lock_time),
75-
Err(e) => println!("Invalid: {}", e),
76-
}
74+
let valid_lock = RelLockTime::from_consensus(144);
75+
assert!(
76+
valid_lock.is_ok(),
77+
"144 blocks should be a valid RelLockTime"
78+
);
7779

78-
// Invalid case - properly handled
79-
match RelLockTime::from_consensus(16_777_216) {
80-
Ok(lock_time) => println!("Valid: {:?}", lock_time),
81-
Err(e) => {
82-
println!("Invalid RelLockTime value: {}", e);
83-
}
84-
}
80+
// Invalid case
81+
let invalid_value = 0x8000_0000;
82+
let invalid_result = RelLockTime::from_consensus(invalid_value);
83+
assert!(
84+
invalid_result.is_err(),
85+
"Value 0x{:x} should return a RelLockTime error",
86+
invalid_value
87+
);
8588
}
8689

8790
#[test]
8891
fn test_rel_lock_time_error() {
89-
// Create a RelLockTimeError by trying to create an invalid RelLockTime
90-
let invalid_value = 0x80000000u32; // Value that causes RelLockTime::from_consensus to fail
92+
// 1. Create the underlying RelLockTime error (high bit set)
93+
let invalid_value = 0x80000000u32;
9194
let rel_lock_time_result = miniscript::RelLockTime::from_consensus(invalid_value);
9295
assert!(rel_lock_time_result.is_err());
96+
9397
let rel_lock_time_error = rel_lock_time_result.unwrap_err();
9498

95-
// Test the From impl
96-
let error: descriptor::DescriptorError = rel_lock_time_error.into();
97-
assert!(matches!(error, descriptor::DescriptorError::RelLockTime(_)));
99+
// 2. Wrap it in the Miniscript error variant (matching your new macro logic)
100+
let minisc_err = miniscript::Error::RelativeLockTime(rel_lock_time_error);
101+
102+
// 3. Test the From impl (Miniscript -> DescriptorError)
103+
let error: descriptor::DescriptorError = minisc_err.into();
104+
105+
// Check that it matches the nested structure
106+
assert!(matches!(
107+
error,
108+
descriptor::DescriptorError::Miniscript(miniscript::Error::RelativeLockTime(_))
109+
));
98110

99-
// Test the Display impl
111+
// 4. Test the Display impl
100112
let display_string = format!("{}", error);
101-
assert!(display_string.starts_with("RelLockTime error:"));
113+
114+
// FIX: Since the error is now wrapped in DescriptorError::Miniscript,
115+
// the string likely starts with "Miniscript error:" or similar.
116+
// Using .contains() is more robust for nested errors.
117+
assert!(
118+
display_string.contains("relative locktime"),
119+
"Display string was: '{}'",
120+
display_string
121+
);
102122
}

0 commit comments

Comments
 (0)