lnwallet+lnwire: handle boundary cases

[yyforyongyu]

Summary

• Complete missing wallet reservation responses.
• Defensively handle zero-block channel ranges and avoid zero-block reply
  prefixes.
• Add focused regressions and a v0.21.3 release note.

Change Description

The wallet funding handler now returns both completion results when a pending
reservation is absent. Channel-range helpers retain their first height for
zero-block input, and the graph syncer skips a nonexistent prefix when the
first queried block exceeds one reply chunk.

Verified with:

• go test ./lnwallet ./lnwire ./discovery -count=1
• go test -race ./lnwallet -run '^TestHandleFundingCounterPartySigsMissingReservation$' -count=1
• go test -race ./discovery -run '^(TestGossipSyncerReplyChanRangeQueryBlockRange|TestGossipSyncerReplyChanRangeQueryDenseFirstBlock)$' -count=1

[github-actions]

🔴 PR Severity: CRITICAL

gh pr view | 6 files | 169 lines changed

🔴 Critical (3 files)

• lnwallet/wallet.go - wallet operations / channel funding logic
• lnwire/query_channel_range.go - Lightning wire protocol message
• lnwire/reply_channel_range.go - Lightning wire protocol message

🟢 Low (3 files)

• docs/release-notes/release-notes-0.21.3.md - release notes only
• lnwallet/wallet_test.go - test-only change
• lnwire/reply_channel_range_test.go - test-only change

Analysis

This PR touches both lnwallet/* (wallet operations) and lnwire/* (wire protocol message encoding/decoding for channel range queries), both of which are CRITICAL-tier packages. The core change is small (1 line in wallet.go, ~8 lines across the two lnwire files) and is accompanied by good test coverage, but changes to wire message encoding and wallet logic warrant expert review given their correctness sensitivity and potential for peer-interop or fund-safety impact.

---

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

[MPins]

Fix looks right, and I confirmed the behavior locally.

One thought on coverage: TestChannelRangeLastBlockHeight pins the arithmetic, but the bug it's fixing only exists where callers act on the result, and all of those are in discovery/syncer.go.

TestGossipSyncerReplyChanRangeQueryBlockRange is already the boundary table for this (full range, small query, overflow) — it's just missing the empty one. Two entries complete it:

 		// overflow example
 		{
 			FirstBlockHeight: uint32(1000),
 			NumBlocks:        uint32(math.MaxUint32),
 		},
+
+		// empty range at the genesis block
+		{
+			FirstBlockHeight: uint32(0),
+			NumBlocks:        uint32(0),
+		},
+
+		// empty range after the genesis block
+		{
+			FirstBlockHeight: uint32(1000),
+			NumBlocks:        uint32(0),
+		},
 	}

 		{
 			startHeight: uint32(1000),
 			endHeight:   uint32(math.MaxUint32),
 		},
+		{
+			startHeight: uint32(0),
+			endHeight:   uint32(0),
+		},
+		{
+			startHeight: uint32(1000),
+			endHeight:   uint32(1000),
+		},
 	}

[ziggie1984]

Non-blocking proposal: as a follow-up, we could replace the separate completion and error channels with a single buffered result channel.

CompleteReservation currently reconstructs one logical result from two independent receives:

return <-completeChan, <-errChan

That makes every terminal handler path responsible for sending both halves, which is how this missing-reservation branch was able to leave the caller blocked. A small internal result type could make the response atomic:

type fundingCompletionResult struct {
    channel *chanstate.OpenChannel
    err     error
}

Both addCounterPartySigsMsg and addSingleFunderSigsMsg could then carry one chan fundingCompletionResult, with each success or error path performing exactly one send.

I don't think this should block or expand this PR: the current one-line fix is correct, focused, and easier to audit/backport. The single-result-channel change would just remove this class of half-response bug in a separate cleanup.

[ziggie1984]

LGTM

[ziggie1984]

One remaining sender-side zero path is the wrong-chain early return in replyChanRangeQuery. It runs before LastBlockHeight is used and copies query.NumBlocks verbatim. A peer can send a QueryChannelRange with a mismatched ChainHash and NumBlocks=0, and we reply with Complete=0 and NumBlocks=0.

Since this PR also prevents lnd from constructing the dense-first-block zero prefix, should we ensure this branch cannot emit a zero-block reply either? For the current defensive-clamping approach, the narrow fix would be to use at least one block in the wrong-chain response. It would also be good to add a zero-block case to TestGossipSyncerQueryChannelRangeWrongChainHash so the outbound response is covered.

[ziggie1984]

LGTM, had a small thing regarding a wrong chain-hash were we would set the numBlos=0, other than that this is gtg

[starius]

LGTM!

[github-actions]

Successfully created backport PR for v0.21.x-branch:

• [v0.21.x-branch] Backport #11035: lnwallet+lnwire: handle boundary cases #11077

[github-actions]

Created backport PR for v0.20.x-branch:

• [v0.20.x-branch] Backport #11035: lnwallet+lnwire: handle boundary cases #11079 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-11035-to-v0.20.x-branch
git worktree add --checkout .worktree/backport-11035-to-v0.20.x-branch backport-11035-to-v0.20.x-branch
cd .worktree/backport-11035-to-v0.20.x-branch
git reset --hard HEAD^
git cherry-pick -x 081adebca6d2726f232211f437e8e1f8806aeb18
git push --force-with-lease

[github-actions]

Backport failed for v0.21.x-branch, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin v0.21.x-branch
git worktree add -d .worktree/backport-11035-to-v0.21.x-branch origin/v0.21.x-branch
cd .worktree/backport-11035-to-v0.21.x-branch
git switch --create backport-11035-to-v0.21.x-branch
git cherry-pick -x 8bf173e5638db989741a935067f4a66985614692 b423c8bf07af197f6a56c9c3456b06fec320ba40 081adebca6d2726f232211f437e8e1f8806aeb18