connectd: synchronize replacement subdaemon routing - #9460
Draft
niftynei wants to merge 10 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.
…race Flake in test_rbf_reconnect_tx_construct indicates that there's a race in the teardown of the old dualopend/HSM code, which causes mismatches in the inflights and BROKEN HSM messages. **BROKEN** dualopend-chan#1: STATUS_FAIL_HSM_IO: Bad get_per_commitment_point_reply Changelog-Fixed: dualopend: restarting `dualopend` after an RBF abort now waits for the previous HSM client to exit, avoiding commitment-point reply mismatches.
After a dualopend route receives tx_abort, connectd hides it from normal dispatch and schedules it to close. If lightningd starts a replacement owner during that interval, it previously had no way to know when connectd had installed the replacement route. The new owner could begin its peer protocol while the route was absent or still belonged to the retiring owner. Add a tracked attachment request and defer its reply until io_new_conn has installed the replacement duplex plan. Validate the peer generation and connection state before accepting it, and report rejection to lightningd so it does not start an owner against a half-live route. A replacement may instead inherit the existing endpoint that was quarantined after tx_abort. Add a resume request which finds that exact route, cancels its close timer, and makes it eligible for dispatch again. Keep ordinary subdaemon attachment on the existing unacknowledged path, so the synchronization barrier applies only to replacement dualopend routes. Changelog-Fixed: connectd: replacement `dualopend` routes are now activated only after `connectd` confirms installation, preventing reconnects from using missing or retiring routes.
niftynei
force-pushed
the
review/dualopend-owner-routing
branch
from
August 31, 2026 17:12
ebce88a to
9670b19
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 #9459
Prevents restarting an owner before the old route is released.