Handle fallible per commitment point in channel establishment

[alecchendev]

Handles fallible get_per_commitment_point signer method (except for channel reestablish).

We make get_per_commitment_point return a result type so that in the Err case, we (LDK) can resume operation while an async signer fetches the commitment point. This will typically block sending out a message, but otherwise most state updates will occur. When the signature arrives, the user is expected to call ChannelManager::signer_unblocked and we will retry get_per_commitment_point however this time the signer will return the commitment point with Ok, and we'll send out our message.

This PR implements this flow for channel establishment, i.e. open_channel, accept_channel, and channel_ready.

Rough outline of how our HolderCommitmentPoint advances and gets new signatures:

• sending open_channel - creates new outbound channel, immediately requests the first commit point, waits to have the first 2 commit points to be unblocked to send the message
• sending accept_channel - same ^
• sending funding_created - no op regarding commit points
• receiving funding_created - uses point to verify first commitment, then advances commitment, requesting next point
• sending funding_signed - no op
• receiving funding_signed - same as above, verifies, advances, requests next point
• sending channel_ready - waits to have the next commit point ready, then once unblocked sends the current point in the channel_ready message

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 83.42857% with 58 lines in your changes missing coverage. Please review.

Project coverage is 89.70%. Comparing base (1a8bf62) to head (d1e94bd). Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	84.33%	39 Missing and 8 partials :warning:
lightning/src/ln/channelmanager.rs	88.09%	1 Missing and 4 partials :warning:
lightning/src/ln/functional_test_utils.rs	0.00%	3 Missing :warning:
lightning/src/util/test_utils.rs	40.00%	0 Missing and 3 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3109      +/-   ##
==========================================
- Coverage   89.74%   89.70%   -0.05%     
==========================================
  Files         130      130              
  Lines      107793   107882      +89     
  Branches   107793   107882      +89     
==========================================
+ Hits        96743    96778      +35     
- Misses       8651     8695      +44     
- Partials     2399     2409      +10     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[alecchendev]

the commits from #3115 are second from the end, will rebase on top later this week, but generally most of the stuff here is in place

[TheBlueMatt]

When we get back to this, please address the comments from https://github.com/lightningdevkit/rust-lightning/pull/3152#pullrequestreview-2183958862

[TheBlueMatt]

This also needs rebase now.

[alecchendev]

Will probably squash some of these commits together at some point but just kept them separate for now

[alecchendev]

rebased and force pushed with the new approach. Previous branch is still here if it's helpful to see

[TheBlueMatt]

Also CI is very sad

[TheBlueMatt]

Finally, if you get a chance, can you flesh out your commit messages more? Describe why we're doing each commit, what considerations went into the approach, and what risks we might have in the design we took, even just a few sentences may be helpful in the future when someone is looking at old code for what we were thinking when we made changes.

[TheBlueMatt]

CI still looks to be failing.

[TheBlueMatt]

Any update here? When you get a chance it looks like we're missing test coverage here - https://github.com/lightningdevkit/rust-lightning/pull/3355#discussion_r1824932135

[alecchendev]

Hello! Sorry for the delay, I should be able to get back to this either this week or next week

[alecchendev]

pushed some, still addressing more comments

[alecchendev]

almost there...

[alecchendev]

Okay, I think I've addressed all the comments up to now. Lmk when/if I'm good to squash!

[alecchendev]

Squashed

[valentinewallace]

Looks like CI is sad :(

[alecchendev]

hmmm, nothing was changed in my force push and the checks it's failing are running fine locally...? The failure on the latest commit:

    Checking lightning v0.0.124 (/home/runner/work/rust-lightning/rust-lightning/lightning)
error[E0609]: no field `holder_commitment_point` on type `ChannelContext<SP>`
    --> lightning/src/ln/channel.rs:7806:16
     |
7806 |         self.context.holder_commitment_point.try_resolve_pending(
     |                      ^^^^^^^^^^^^^^^^^^^^^^^ unknown field
     |
     = note: available fields are: `config`, `prev_config`, `inbound_handshake_limits_override`, `user_id`, `channel_id` ... and 80 others

But I don't even have that line? It's also weird it says 0.0.124...should I maybe rebase or something?

[TheBlueMatt]

CI gets run rebased against upstream so you probably have a silent merge conflict. A rebase should turn it up and then we can land.