Linting: Disallow unwraps, replace with expect or error propagation#868
Open
tnull wants to merge 12 commits intolightningdevkit:mainfrom
Open
Linting: Disallow unwraps, replace with expect or error propagation#868tnull wants to merge 12 commits intolightningdevkit:mainfrom
unwraps, replace with expect or error propagation#868tnull wants to merge 12 commits intolightningdevkit:mainfrom
Conversation
Introduce a small lock helper module so mutex and rwlock access can use a single naming convention instead of repeating direct locking at every call site. Co-Authored-By: HAL 9000
Switch the codebase over to the new lock helper names so lock poisoning behavior is centralized and the call sites stay shorter and more consistent. Co-Authored-By: HAL 9000
Document the internal invariants behind the remaining non-lock unwrap sites so panic paths explain why they should be unreachable, while keeping the known reachable cases explicit for later handling. Co-Authored-By: HAL 9000
Log and skip runtime listener failures instead of panicking when accepting inbound connections or converting accepted sockets. These errors can happen in normal operation, so keeping the node running is safer than treating them as unreachable. Co-Authored-By: HAL 9000
Replace the success-path unwrap on payment amounts with an expect that explains why outbound payments must already have a recorded amount by the time LDK reports them as sent. Co-Authored-By: HAL 9000
Replace the pending-channel unwrap with an expect that records why supported LDK Node state should always include the former temporary channel id. Older rust-lightning state could omit it, but LDK Node never shipped before that field existed. Co-Authored-By: HAL 9000
Replace the remaining channel-details unwraps with expects that point to the supported LDK Node upgrade boundary. The missing fields only occur in rust-lightning versions older than the ldk-node v0.1.0 baseline. Co-Authored-By: HAL 9000
Replace the user_version query unwrap with normal io::Error propagation so database initialization failures are reported cleanly instead of panicking. Co-Authored-By: HAL 9000
Replace the Tokio runtime builder unwrap with io::Error propagation so VSS startup failures surface through the constructor instead of panicking. Co-Authored-By: HAL 9000
Use a zero-millisecond fallback for elapsed-time logging so clock adjustments do not panic the chain polling loop. Co-Authored-By: HAL 9000
Return Esplora client construction failures through build-time error handling instead of panicking so invalid headers or reqwest setup errors fail node construction cleanly. Co-Authored-By: HAL 9000
|
👋 Thanks for assigning @joostjager as a reviewer! |
Fail library clippy runs when new unwrap calls are introduced so the unwrap policy stays enforced without pulling tests, benches, or docs into the restriction. Co-Authored-By: HAL 9000
tnull
force-pushed
the
2026-04-disallow-unwrap
branch
from
a05b2b6 to
2eb9f09
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For the longest time we wanted to enforce clear rules around
unwrapuse. Here we finally do that:Mutex/RwLocklocking toExthelpers that avoid theunwrapand are shorter, reducing clutter in codeunwraps to useexpects, providing clear rationale why they are deemed unreachableunwraps throughclippyin CI (in non-test/documentation code, that is)