lnrpc+itest: return channel Memo for Pending channels

[shaurya947]

Change Description

In a previous PR #7668 we added a Memo field for channels that could be specified when opening a channel. This was a reference note-to-self with no bearing on the functioning of the channel. In that PR, the memo value was returned only through ListChannels. This PR builds upon that PR by also returning the Memo field for pending open and waiting close channels.

Steps to Test

An itest has been updated to verify that the Memo is returned but to manually test, one can verify the Memo field on a pending-open or waiting-close channel

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 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.
• There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

[shaurya947]

Hey @guggero and @positiveblue I created this follow-up PR to add the Memo field to the PendingChannels response. To avoid bloating the itest suite (as was recommended in the last PR), I've added small bit of extra logic to an existing itest. I also haven't edited release notes—I read over the note from the last PR and I think it sufficiently covers this change as well. Let me know if all that is ok or if I should do something differently.

One thing to note is that PendingChannels returns three types of channels:

1. pending open
2. pending force close
3. waiting close

(1) and (3) fetch from the openChannelBucket and populate using channeldb.OpenChannel instance so the Memo field is present and all we do is pass it along in the protobuf message. That is this PR currently.

(2) on the other hand fetches from the closedChannelBucket and populates using channeldb.ChannelCloseSummary so the Memo field is not available (neither in the bucket nor in the deserialized struct).

Should we update the code that marks a channel as closed (thereby moving it from the open -> closed bucket) to also serialize the Memo in closedChannelBucket? And should we update the ChannelCloseSummary when deserializing for (2)? I guess the underlying question is at what point does the Memo stop being useful? If the answer is "never" then we should probably also return it in ClosedChannels?

[guggero]

Let me know if all that is ok or if I should do something differently.

No entry in the release notes is fine, I added the no-changelog label that makes the CI ignore that step (will work for the next push).

If the answer is "never" then we should probably also return it in ClosedChannels?

I think the memo is definitely helpful for pending force closed or fully closed channels. So adding it to the ChannelCloseSummary is a good idea. I don't know if we already have TLV values there, so it might be a bit more involved.

[yyforyongyu]

I think the skip ci in the commit message needs to be removed to let the build run.

[shaurya947]

I think the skip ci in the commit message needs to be removed to let the build run.

Aah my bad I thought the flag was necessary if I didn't update release notes. Will remove it.

@guggero I realized that the full state of the channel does get written to the historicalChannelBucket which is actually available when querying the pending force closing channel.

lnd/rpcserver.go

Line 3499 in bbbf7d3

historical, err := r.server.chanStateDB.FetchHistoricalChannel(

This allows me to populate the Memo in all kinds of channels returned by PendingChannels. I also verified in the same itest that the Memo is indeed returned even when the channel transitions to the pending force closing state 🙂

[lightninglabs-deploy]

@guggero: review reminder
@yyforyongyu: review reminder

[yyforyongyu]

Thanks for the PR!

[guggero]

LGTM, thanks a lot!

cc @saubyk

[saubyk]

LGTM, thanks a lot!

cc @saubyk

Concept Ack

[shaurya947]

Apologies for the late turnaround here, I didn't realize a merge conflict had appeared. Just fixed, should be all good now 🤞

[positiveblue]

Thank you for this @shaurya947 I think we are good to go 🚀