dualopend: make opening teardown failures explicit - #9459
Draft
niftynei wants to merge 8 commits into
Draft
Conversation
A callback can close a connection while readiness from the current poll call is still being dispatched. Deleting that connection compacts the fd table, and registering a replacement can restore its previous length with different occupants. A snapshot tied to mutable table positions can consequently skip an original event or deliver stale readiness to the replacement. Give each fd a stable registration object and retain that registration while a poll result refers to it. Deletion clears the registration's fd pointer, so pending readiness for a retired connection is ignored, while a replacement receives a distinct registration and cannot inherit the old event. Snapshot only entries with nonzero revents and preserve the IO_ALWAYS marker in the existing fairness order. Reuse a growable snapshot buffer between loop iterations and dispatch directly through the retained registration, avoiding both a fresh allocation on every poll and the repeated linear lookup required by the earlier generation-based approach. Register one process-exit cleanup for that reusable buffer. Retaining it for the daemon lifetime preserves allocation reuse, while releasing it after event dispatch can no longer be active avoids a reachable allocation in Valgrind and LeakSanitizer runs. Changelog-Fixed: io: replacing a file descriptor during poll dispatch no longer delivers stale readiness events to the replacement connection.
Closing a connection while dispatching poll results compacts the fd table. If the callback immediately registers a replacement, the table can return to its original length with a different connection occupying a slot represented in the current poll result. Dispatching readiness by mutable table position can then deliver an old event to the replacement or skip another ready connection. Use a fake poll implementation to make three original connections ready. The connection selected first closes itself and installs a replacement while the other readiness events remain pending. Verify that every original event is delivered exactly once, that the replacement does not inherit stale readiness, and that it is handled normally by the following poll call. Register an exit assertion before enabling protection so atexit LIFO ordering checks that the backend releases its reusable snapshot buffer at process exit. This keeps leak-sensitive unit runs from accepting a permanently reachable allocation. Add the regression to check-units so this CCAN behavior is covered by the project's normal unit-test suite. Changelog-None
Add test_inflight_disconnect_commitment_v2 which triggers a disconnect at +WIRE_COMMITMENT_SIGNED during a dual-funded open. Changelog-None
The error message in ElementsProject#8902 indicates that we're failing to correctly parse an error message from lightningd lightningd-2 2026-02-16T00:50:21.721Z **BROKEN** 038194b5f32bdf0aa59812c86c4ef7ad2f294104fa027d1ace9b469bb6f88cf37b-dualopend-chan#2: STATUS_FAIL_MASTER_IO: Error parsing 7011: 1b5b50656572206572726f7220776974682050534254207369676e6174757265732e00 The openchannel2_sign_hook_cb in lightningd can return error messages, not just the DUALOPEND_SEND_TX_SIGS message at this point. We handle this here. Changelog-Fixed: dualopend: dual-funding signing-hook errors are now reported correctly instead of causing a `dualopend` master-reply parse failure.
Issue ElementsProject#8902 demonstrates that there are races conditions ocurring when we use the peer disconnection notifications. In theory, we don't actually need to listen for peer disconnects, as we're already listening for open attempt failures with both the state_change and the channel_open_failed notifications. Changelog-Fixed: funder: peer disconnects no longer race channel-open failure handling when cleaning up pending dual-funded opens.
opener_commits allocates a temporary penalty base before negotiating the commitment and validating the remote signature. Both failure paths revert the channel state and return without transferring ownership, so they must release that allocation explicitly. Changelog-Fixed: dualopend: failed commitment negotiations no longer leak their temporary penalty-base allocation.
fetch_per_commitment_point previously ignored a failed write and passed a NULL read result to the wire decoder. An HSM disconnect could therefore appear as a malformed reply or crash instead of exposing the underlying transport failure. Check both sides of the synchronous HSM exchange and fail with STATUS_FAIL_HSM_IO at the point of failure. Clear errno before reading so a NULL result reports EOF rather than an unrelated stale error. Changelog-Fixed: dualopend: HSM transport failures are now reported directly instead of appearing as malformed replies or crashes.
hsmd permits only one client for a channel database ID. Starting channeld before the completed openingd owner has exited can therefore race creation of the replacement HSM client, particularly when several v1 opens finish at the same time. Release openingd after preserving its peer endpoint and before asking channeld to acquire the channel's HSM client. This makes owner exit an explicit barrier without blocking the handling of other completed opens. Changelog-Fixed: openingd: completed v1 channel opens no longer race `openingd` teardown against `channeld` HSM client creation.
niftynei
force-pushed
the
review/dualopend-prereqs
branch
from
August 31, 2026 17:12
547d071 to
eb39e18
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.
Builds on #9458
Prerequisites for safely replacing opening subdaemons.