funding: wait for commitment restoration before channel_ready

[Amirhosein]

Change Description

I would like to propose this fix for #11259, reordering the release of discoverySignal relative to commitment restoration during channel opening.

Background & Root Cause

During channel opening, if an incoming channel_ready is processed before the funding goroutine finishes calling lnwallet.NewLightningChannel(...), a link can be instantiated and process commitment updates (e.g. update_add_htlc + commitment_signed). This can write an unacknowledged remote commitment diff to disk.

When the concurrent funding goroutine runs restoreCommitState, it fetches the unacked remote commitment diff from the database via RemoteCommitChainTip(). However, the in-memory channel copy passed to NewLightningChannel still has RemoteNextRevocation == nil (since processChannelReady has not populated it yet). Dereferencing this nil revocation point during DeriveCommitmentKeys causes a latent nil-pointer panic.

Changes

1. Deferred Discovery Signaling: Removed the premature signal closes in handleFundingConfirmation and advancePendingChannelState. I moved the release of discoverySignal to occur immediately after lnwallet.NewLightningChannel returns in advanceFundingState. This covers both regular confirmed channels and zero-confirmation channels in a unified location.
2. Restart Barrier Restoration: In Manager.start(), I restored the barrier for channels where channel.IsPending || channel.RemoteNextRevocation == nil. If a node restarted after MarkAsOpen was persisted to the database but before commitment restoration or channel_ready processing completed, this ensures an early channel_ready from a reconnected peer cannot bypass the restoration barrier.
3. Restoration Failure Handling: If lnwallet.NewLightningChannel returns an error, I ensure discoverySignal is closed (with an appropriate warning log) before returning, preventing queued peer messages from hanging indefinitely until daemon shutdown.
4. Regression Tests: Added funding/manager_discovery_test.go with 8 test cases covering both confirmed and zero-conf channels, with and without restart, and across successful and failed restoration paths. The tests use a custom aux leaf-store hook to reliably pause restoration and verify that peer channel_ready messages remain queued and no link is started until restoration completes.
5. Release Notes: Added a release note entry under # Bug Fixes in docs/release-notes/release-notes-0.22.0.md.

Fixes #11259.

Steps to Test

Reviewers can run the unit tests and targeted integration tests:

# Run the funding unit tests
make unit pkg=funding timeout=5m

# Run the new regression test under the race detector
go test -race -tags='dev nolog lowscrypt' ./funding -run '^TestFundingManagerDiscoverySignal$' -count=10 -timeout=2m

# Targeted integration tests for channel opening and immediate payments
make itest icase='(immediate_payment_after_channel_opened|zero_conf_channel_open)' timeout=10m

I have verified all 8 test cases locally with -race and 10 repetitions, and verified that without the fix the assertions fail as expected.

Pull Request Checklist

Testing

• Your PR passes all CI checks.
• Tests covering the positive and negative (error paths) are included.
• Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

• The change is not insubstantial. Typo fixes are not accepted to fight bot spam.
• The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• Commits follow the Ideal Git Commit Structure.
• Any new logging statements use an appropriate subsystem and logging level.
• Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
• There is a change description in the release notes, or [skip ci] in the commit message for small changes.

Thank you very much for taking the time to review this PR. I would welcome any feedback or suggestions on the approach!

[github-actions]

🔴 PR Severity: CRITICAL

gh pr view | 3 files | 353 lines changed

🔴 Critical (1 file)

• funding/manager.go - channel funding workflow coordination; changes the ordering of discovery signaling and commitment restoration relative to channel_ready

🟢 Low (2 files)

• funding/manager_discovery_test.go - new regression test file
• docs/release-notes/release-notes-0.22.0.md - release notes entry

Analysis

The only non-test, non-doc file touched is funding/manager.go, which sits in the funding/* package (channel funding workflow coordination) and is classified CRITICAL per the severity rules. The change itself is subtle: it reorders when discoverySignal is released relative to commitment-state restoration from a channel backup, to avoid a race where an early channel_ready lets a link process a commitment update before RemoteNextRevocation is restored. This directly affects channel-open correctness and crash-safety on restart, so it warrants expert review of the funding manager's state machine and barrier/signal handling, despite the overall diff being modest in size (no severity bump triggered — single critical package, well under the file/line thresholds).

---

_{To override, add a severity-override-{critical,high,medium,low} label.}