Skip to content

Commit 9a0bed7

Browse files
AmosOO7ValuedMammal
authored andcommitted
fix(dsl): handle invalid AbsLockTime without panicking
Previously, the `after()` macro in the descriptor DSL used `.expect()` when calling `AbsLockTime::from_consensus()`. This caused the macro to panic and crash the program if an invalid value was provided. This commit: - Updates the `after()` macro rule to handle `Result` from `from_consensus`. - Maps `miniscript::AbsLockTimeError` to `DescriptorError::Miniscript`. - Ensures consistency with the `older()` (RelLockTime) error handling. - Adds comprehensive unit tests for valid heights, timestamps, and invalid boundary values. Fixes a potential panic when using the `descriptor!` macro with untrusted or out-of-range absolute locktimes.
1 parent a910444 commit 9a0bed7

3 files changed

Lines changed: 115 additions & 1 deletion

File tree

src/descriptor/dsl.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,8 +721,11 @@ macro_rules! fragment {
721721
$crate::keys::make_pkh($key, &secp)
722722
});
723723
( after ( $value:expr ) ) => ({
724-
$crate::impl_leaf_opcode_value!(After, $crate::miniscript::AbsLockTime::from_consensus($value).expect("valid `AbsLockTime`"))
724+
$crate::miniscript::AbsLockTime::from_consensus($value)
725+
.map_err($crate::descriptor::error::Error::from)
726+
.and_then(|abs_lock_time| $crate::impl_leaf_opcode_value!(After, abs_lock_time))
725727
});
728+
726729
( older ( $value:expr ) ) => ({
727730
$crate::miniscript::RelLockTime::from_consensus($value)
728731
.map_err($crate::descriptor::error::Error::from)

src/descriptor/error.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,3 +131,9 @@ impl From<miniscript::RelLockTimeError> for Error {
131131
Error::Miniscript(miniscript::Error::RelativeLockTime(err))
132132
}
133133
}
134+
135+
impl From<miniscript::AbsLockTimeError> for Error {
136+
fn from(err: miniscript::AbsLockTimeError) -> Self {
137+
Error::Miniscript(miniscript::Error::AbsoluteLockTime(err))
138+
}
139+
}

tests/descriptor_macro.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
///
66
/// This file shows the problem and how to catch it.
77
use bdk_wallet::descriptor;
8+
use miniscript::AbsLockTime;
89

910
#[test]
1011
fn test_older_with_invalid_rellocktime_too_large() {
@@ -117,3 +118,107 @@ fn test_rel_lock_time_error() {
117118
display_string
118119
);
119120
}
121+
122+
#[test]
123+
fn test_after_with_invalid_abslocktime_zero() {
124+
let invalid_value = 0;
125+
126+
let result = descriptor!(wsh(after(invalid_value)));
127+
128+
assert!(
129+
result.is_err(),
130+
"AbsLockTime of 0 should return an error in this context"
131+
);
132+
133+
// Check for the correct error variant
134+
if let Err(descriptor::DescriptorError::Miniscript(miniscript::Error::AbsoluteLockTime(_))) =
135+
result
136+
{
137+
// Success: Error was caught and mapped correctly
138+
} else {
139+
panic!("Expected AbsLockTime error, got {:?}", result);
140+
}
141+
}
142+
143+
#[test]
144+
fn test_after_with_valid_height() {
145+
// Values < 500,000,000 are interpreted as block heights
146+
let block_height = 840_000; // Example: Halving block height
147+
let result = descriptor!(wsh(after(block_height)));
148+
assert!(result.is_ok(), "Valid block height should work");
149+
}
150+
151+
#[test]
152+
fn test_after_with_valid_timestamp() {
153+
// Values >= 500,000,000 are interpreted as Unix timestamps
154+
let timestamp = 1715817600; // Example: A date in 2024
155+
let result = descriptor!(wsh(after(timestamp)));
156+
assert!(result.is_ok(), "Valid Unix timestamp should work");
157+
}
158+
159+
#[test]
160+
fn test_after_with_exact_max_valid_timestamp() {
161+
// 2,147,483,648 is the absolute maximum value supported
162+
// by this implementation of AbsLockTime.
163+
let max_valid = 2_147_483_648;
164+
let result = descriptor!(wsh(after(max_valid)));
165+
assert!(
166+
result.is_ok(),
167+
"Value 2,147,483,648 should be the valid upper bound"
168+
);
169+
}
170+
171+
#[test]
172+
fn test_after_with_invalid_just_above_max() {
173+
// 2,147,483,649 is too large for the internal representation
174+
let too_large = 2_147_483_649;
175+
let result = descriptor!(wsh(after(too_large)));
176+
177+
assert!(
178+
matches!(
179+
result,
180+
Err(descriptor::DescriptorError::Miniscript(
181+
miniscript::Error::AbsoluteLockTime(_)
182+
))
183+
),
184+
"Should fail specifically with a Miniscript AbsoluteLockTime error"
185+
);
186+
}
187+
188+
#[test]
189+
fn test_after_with_u32_max_is_invalid() {
190+
// Verify that u32::MAX actually triggers the error you're looking for.
191+
let max_value = u32::MAX;
192+
let result = descriptor!(wsh(after(max_value)));
193+
194+
assert!(
195+
result.is_err(),
196+
"u32::MAX is often rejected as an invalid consensus locktime"
197+
);
198+
}
199+
200+
#[test]
201+
fn test_abs_lock_time_error_mapping() {
202+
let invalid_value = 0;
203+
let abs_lock_result = AbsLockTime::from_consensus(invalid_value);
204+
205+
assert!(abs_lock_result.is_err());
206+
let abs_err = abs_lock_result.unwrap_err();
207+
208+
// Wrap in the general Miniscript Error enum
209+
let minisc_err = miniscript::Error::AbsoluteLockTime(abs_err);
210+
211+
// Convert to your local Error type using .into()
212+
let error: descriptor::DescriptorError = minisc_err.into();
213+
214+
// Assert it landed in the Miniscript variant
215+
assert!(matches!(error, descriptor::DescriptorError::Miniscript(_)));
216+
217+
// Verify the error message contains the expected text
218+
let display_string = format!("{}", error);
219+
assert!(
220+
display_string.contains("absolute locktime"),
221+
"Error message '{}' should mention locktime",
222+
display_string
223+
);
224+
}

0 commit comments

Comments
 (0)