Skip to content

fix(dsl): handle invalid AbsLockTime without panicking#409

Merged
ValuedMammal merged 1 commit intobitcoindevkit:masterfrom
AmosOO7:fix/abslocktime-error
Mar 20, 2026
Merged

fix(dsl): handle invalid AbsLockTime without panicking#409
ValuedMammal merged 1 commit intobitcoindevkit:masterfrom
AmosOO7:fix/abslocktime-error

Conversation

@AmosOO7
Copy link
Contributor

@AmosOO7 AmosOO7 commented Mar 16, 2026

Summary

This PR fixes a potential panic in the descriptor! DSL macro when provided with an invalid absolute locktime. Previously, the after(<value>) rule used .expect("valid AbsLockTime") on AbsLockTime::from_consensus(). This caused the macro to panic if the value was 0 or exceeded the MAX_ABSOLUTE_LOCKTIME. The macro now correctly propagates a Result, ensuring the library remains crash-safe for all inputs.

Changelog notice

Fixed

  • dsl: handle invalid AbsLockTime without panicking

Testing

  • Ran cargo test to verify all existing descriptor tests pass.
  • Added new unit tests specifically targeting the after() macro's boundary conditions and error mapping.
  • Verified that providing after(0) now returns a Error::AbsLockTime instead of terminating the process.

Checklist

Bugfixes:

  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Related issue
Closes #408

Thanks for the review! This ensures the descriptor! macro is robust against malformed or out-of-range absolute locktime values.

@Serious-Sam-Dev
Copy link

ACK, test and clippy pass, maybe change title of PR using conventional commits is good, but all is ok with code

@AmosOO7 AmosOO7 force-pushed the fix/abslocktime-error branch from e9101bc to 937c9e4 Compare March 18, 2026 17:11
@AmosOO7 AmosOO7 requested a review from ValuedMammal March 18, 2026 21:21
@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.93%. Comparing base (a910444) to head (6980403).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
+ Coverage   79.91%   79.93%   +0.01%     
==========================================
  Files          24       24              
  Lines        5283     5287       +4     
  Branches      241      241              
==========================================
+ Hits         4222     4226       +4     
  Misses        984      984              
  Partials       77       77              
Flag Coverage Δ
rust 79.93% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AmosOO7 AmosOO7 force-pushed the fix/abslocktime-error branch from bd3beb0 to 53d928e Compare March 19, 2026 16:57
@ValuedMammal ValuedMammal added this to the Wallet 3.0.0 milestone Mar 20, 2026
@ValuedMammal ValuedMammal moved this to Needs Review in BDK Wallet Mar 20, 2026
@ValuedMammal ValuedMammal changed the title Fix/abslocktime error fix(dsl): handle invalid AbsLockTime without panicking Mar 20, 2026
@ValuedMammal ValuedMammal changed the title fix(dsl): handle invalid AbsLockTime without panicking fix(dsl): handle invalid AbsLockTime without panicking Mar 20, 2026
@ValuedMammal ValuedMammal force-pushed the fix/abslocktime-error branch from 53d928e to 9a0bed7 Compare March 20, 2026 06:00
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.
@ValuedMammal ValuedMammal force-pushed the fix/abslocktime-error branch from 9a0bed7 to 6980403 Compare March 20, 2026 06:03
Copy link
Collaborator

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 6980403

@ValuedMammal ValuedMammal merged commit 4d4adae into bitcoindevkit:master Mar 20, 2026
17 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Bug: panic in DSL macro when using invalid AbsLockTime value

3 participants