Fix: Progress channel announcement state when remote_sigs already in DB

[Andezion]

(Fixes #8552)

Depends on #8493 (this PR is stacked on top of the P2TR dust limit fix)

Problem

When a node restarts after exchanging announcement_signatures with a peer - the remote_sigs are loaded from the database during channel_gossip_init(). But when the peer reconnects, it may not resend announcement_signatures if it already sent them before the disconnect!

This causes the channel to get stuck in CGOSSIP_WAITING_FOR_MATCHING_PEER_SIGS state forever, preventing the channel from being announced to the network.

Solution

Modified channel_gossip_channel_reestablished() in channel_gossip.c to check if we already have remote_sigs loaded from the database. If we do - we call update_gossip_state() to attempt to progress the channel gossip state machine instead of waiting forever for the peer to resend signatures
Added comprehensive tests in test_gossip_announcement.py:

• test_channel_announcement_after_restart_with_saved_sigs: Reproduces the exact scenario from issue A specific channel is not announcing my side #8552 (channel announcement after node restart)

• test_channel_announcement_reconnect_without_restart: Verifies channel announcement works after simple reconnect without restart

Important

26.04 FREEZE March 11th: Non-bugfix PRs not ready by this date will wait for 26.06.

RC1 is scheduled on March 23rd

The final release is scheduled for April 15th.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

• The changelog has been updated in the relevant commit(s) according to the guidelines.
• Tests have been added or modified to reflect the changes.
• Documentation has been reviewed and updated as needed.
• Related issues have been listed and linked, including any that this PR closes.
• Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade

[cdecker]

A couple of minor comments, specifically the elements check for 300-546sat outputs is likely important.

[nGoline]

I reviewed the current branch against the commit @cdecker approved (26028f3) to separate genuine rebase updates from new changes.

The core fix is good. lightningd/channel_gossip.c is byte-identical to the approved version (only whitespace cleanup plus adapting to the new function signature from master), and tests/test_gossip_announcement.py is unchanged.

That part is ready, but two things came in after approval that should come back out.

• AssertionError in test_peer_anchor_push: actual_feerate < expected_feerate + 2 #8493 was fixed at the root cause in 8a60a27 (in master since February): the HSM wasn't logging overgrind: short signature length for anchor spends, so did_short_sig() never returned true. With that log in place, the existing +/-2 tolerance is correct. This PR instead loosens check_feerate to +/-10 and rewrites did_short_sig() with a wait_for loop, which is now redundant and weakens the assertion.
• The gRPC regen commit (8abace6) is unrelated to the announcement fix, and the diff removes the grpc version guards and _registered_method=True rather than adding 1.78.0 markers, so it downgrades the generated files relative to master. It's also the source of the current merge conflict, so removing it should resolve that. If this is really needed, regenerating the proto belongs in its own PR.

[nGoline]

ack

[nGoline]

Thanks for stripping this back. Dropping the gRPC regen and the tests/utils.py changes was the right call, and the PR is now clean against master and conflict-free.

The remaining problem is that the tests don't exercise the fix. I checked this by:

1. Built master at 608f6952 with no patch applied, added tests/test_gossip_announcement.py on its own: 2 passed.
2. Applied the channel_gossip.c hunk, rebuilt, ran again: 2 passed. No difference.
3. Forced both tests to fail so the logs would be retained, then grepped for the fix's own log_debug. The string already have remote sigs, checking if we can progress does not appear in either node's log in either test. (channel_gossip: reestablished matches twice in the same logs, so the grep is good.)

The state trace explains it. In test_channel_announcement_after_restart_with_saved_sigs, l1 finishes announcing before the restart:

gossip state: CGOSSIP_WAITING_FOR_MATCHING_PEER_SIGS ->CGOSSIP_WAITING_FOR_ANNOUNCE_DEPTH
gossip state: CGOSSIP_WAITING_FOR_ANNOUNCE_DEPTH->CGOSSIP_ANNOUNCED
Initial channel state CGOSSIP_ANNOUNCED        //<- after l1.restart()

wait_for_announce=False doesn't prevent this, the generate_block(6) and the CHANNELD_NORMAL waits give it time to complete. So the channel is never in CGOSSIP_WAITING_FOR_MATCHING_PEER_SIGS at reestablish, which is the precondition the fix keys on. test_channel_announcement_reconnect_without_restart starts from an already-announced channel and only asserts it stays announced, so it doesn't touch this path either.

The project convention would have caught this: add the test in one commit with @pytest.mark.xfail(strict=True), then remove that line in the commit that fixes it. Here the test would have XPASSed and failed CI.

Stepping back, I'm not sure the new branch is reachable. Reading channel_gossip.c:

• channel_gossip_init sets cg->remote_sigs->scid = *channel->scid when loading from the DB, so loaded sigs match by construction, and the following derive_channel_state gives WAITING_FOR_ANNOUNCE_DEPTH or ANNOUNCED. A restart with sigs in the DB can't land in WAITING_FOR_MATCHING_PEER_SIGS.
• If remote_sigs gets set after init, it came through channel_gossip_got_announcement_sigs, which already calls update_gossip_state() for this state.
• If remote_sigs is set but the scid doesn't match (splice or reorg), derive_channel_state still returns WAITING_FOR_MATCHING_PEER_SIGS, so the call is a no-op.

Related to that: the log in #8552 shows Initial channel state CGOSSIP_WAITING_FOR_MATCHING_PEER_SIGS, which per channel_gossip_init means wallet_remote_ann_sigs_load returned false, so the sigs weren't in the DB. That's the opposite of what the PR title describes, and it makes me think the reporter's stall has a different cause.

Could you produce a test that fails on master? Something that leaves the channel in WAITING_FOR_MATCHING_PEER_SIGS across a restart and then reconnects, asserting the announcement completes. If that turns out not to be constructible, it's worth revisiting whether this is the right fix for #8552.

Minor: commits 2 and 3 are fixups, please squash them into the first so each commit builds and is reviewable on its own.