Implement updated retransmission logic for splice_locked

[t-bast]

While working on taproot tests for splicing, @sstone discovered that what was previously a harmless race condition actually becomes an issue when using taproot channels. This is detailed in lightning/bolts#1223 and lead to changes in the reestablish logic to fix that race condition, which was added to the splicing spec in lightning/bolts@2c1b500

I think we should implement those changes immediately in eclair and lightning-kmp, with the following tweaks for backwards-compatibility:

• we should handle reading the my_current_funding_locked
• if it is included, we should apply the requirements laid out in lightning/bolts@2c1b500
• if it isn't included, we should keep retransmitting our previous splice_locked like we currently do
• but we shouldn't set my_current_funding_locked yet, because current nodes don't know how to read it and it has an even type
  • we will start setting it in Use final spec values for splicing #2887 when the remote node supports the official splicing feature bit

[remyers]

I recreated a variant of the test outlined in the proposed updated Disconnection with concurrent splice_locked from splicing-test.md that ends with Alice sending commit_sig and receiving revoke_and_ack from Bob before Alice has received splice_locked from Bob. The result in both the master and splicing_official branches before implementing the my_current_funding_locked change is that Alice does a force close after receiving Bob's commit_sig that has too few signatures. The modified end of the test looks like this:

     |                              |
     |      channel_reestablish     | next_funding_txid = null
     |----------------------------->|
     |      channel_reestablish     | next_funding_txid = null
     |<-----------------------------|
     |                              | Alice doesn't need to retransmit splice_locked, since Bob's my_current_funding_locked_txid indicates that he received it.
     |                              | Bob's my_current_funding_locked_txid lets Alice know that Bob has locked funding_tx2 while they were disconnected and will send his splice_locked.
     |                              | She can thus immediately lock it as well even though she hasn't received yet Bob's splice_locked.
     |                              |
     |                              | Active commitments:
     |                              |
     |                              |    +------------+
     |                              |    | FundingTx2 |
     |                              |    +------------+
     |                              |
     |       update_add_htlc        |
     |----------------------------->|
     |         commit_sig           | Alice doesn't need to sent commit_sig for funding_tx1 since funding_tx2 was locked. 
     |                              | But Bob would have ignored it anyway.
     |----------------------------->|
     |       revoke_and_ack         |
     |<-----------------------------|
     |         commit_sig           | Bob sends only one signature, for funding_tx2 - Alice will force close here without the 
     |                              | `my_current_funding_locked_txid` change because Alice expects two signatures.
     |<-----------------------------|

[t-bast]

I think that's because you incorrectly modified the end of the test, isn't it? When does Bob retransmit splice_locked in your updated test? Alice should receive Bob's splice_locked before receiving Bob's revoke_and_ack, since Bob retransmits splice_locked on reconnection.

So this shouldn't trigger a force-close on splicing_official: if it does even with a correct test, it is worth investigating further!

[remyers]

If Alice receives splice_locked from Bob before Bob replies to Alice's commit_sig with their own commit_sig, then there is no problem when using splicing_official.

In my modified test the splice_locked from Bob comes after Bob replies to Alice's commit_sig. If the spec says that Bob MUST send splice_locked after channel_reestablish (before any other messages) then my modified test message sequence would be impossible. I didn't see that requirement in the spec, but after adding my_current_funding_locked to the channel_reestablish messages Alice will send and expect only one signature, so it won't be necessary to enforce any particular priority for sending splice_locked.

I'll keep my test the way it is unless there's some reason to expect Bob must send splice_locked before any other messages.

[t-bast]

The sequence you're describing is impossible: I suspect that in your test you're intercepting splice_locked and re-ordering it with revoke_and_ack and commit_sig, right? This cannot happen because the Bolt 8 connection guarantees message ordering.

The sequence you're artificially using in your test is impossible regardless of reestablish logic. Remember that we're in a case where Alice has sent splice_locked but not Bob. So when Bob is ready to send splice_locked, he will prune the previous commitments: but before sending splice_locked, he still considers the previous commitments active!

That means that if Bob hasn't sent splice_locked yet, he will respond to Alice with two commit_sig messages, not one (which Alice will gladly accept). If he has sent splice_locked, then he responds with a single commit_sig, but Alice will receive his splice_locked first and will thus be ok with receiving a single commit_sig: they will both have deactivated the previous commitments.