Future ChannelKeys API

[TheBlueMatt]

Figured I should document the goal I had in my head for ChannelKeys. I think we should:
a) cache the local keys currently fetched via ChannelKeys::pubkeys() in Channel and ChannelMonitor,
b) only ever have one instance of a ChannelKeys per channel - fetching it during Channel init, shoving it into the created ChannelMonitor (this implies finally removing the channel_monitor from Channel, but we're really close to that), and then moving it into the OnchainTxHandler.
c) Drop the Clone requirement from ChannelKeys (though maybe implementing Clone for ChannelMonitor only if ChannelKeys has it, and documenting this).

[ariard]

Okay I spend a bit of time on this, I finally agree with you we should store LocalOnchainCommitment and RemoteCommitment in ChannelMonitor, and just have OnchainRequest exchanged between ChannelMonitor and OnchainTxHandler

OnchainRequest {
     aggregation: bool,
     bump_strategy: BumpStrategy::{RBF,CPFP},
     timelock: u32,
     height: u32,
     tx_template: TxTemplate
}

TxTemplate {
       LocalCommitmentTransaction
       PreSignedJusticeTx
       MalleableJusticeTx
       RemoteHTLC
}

I think middle-term OnchainTxHandler should be its own trait to serve other component like splicing/dual-funding. And so with watchtower integration I've something like this in mind

               ___________________________
              |                          |
              |  MultiPartyTxComposition | (dual-funding/splicing)
              |__________________________|
                           |
                           | OnchainRequest
                           |                                   __________________
                           |                                  |                  |
                           |                       |----------|  ChannelMonitor  |--- LocalOnchainCommitment+RemoteCommitment
                           |                       |          |__________________|
                           |                       |                    |
                           |                       |                    |
                           |    OnchainRequest     |           _________|___________
                           |_______________________|          |                     |
                           |                                  | ConfidentialWatcher |
                           |                                  |_____________________|
                  _________V_________
                 |                   |
                 |  OnchainTxHandler |
                 |___________________|
                           ^
                           |
                           |
                           |
                           |
                    _______V______
                   |              |
                   |  ChanSigner  |
                   |______________|

And ConfidentialWatcher would be trait to implement a privacy-preserving watchtower

ConfidentialWatcher::is_channel_commitment(commitment_tx) --> bool
ConfidentialWatcher::get_template_output(commitment_tx.output[i]) --> TxTemplate::{PreSignedTx, MalleableTx}
ConfidentialWatcher::provide_revocation_secret(Secret) // actual stuff
ConfidentialWatcher::provide_bolt13_blob(Blob)

[TheBlueMatt]

Wow, yea, I think at a high level that's the right design, thanks for putting it together! Refactoring the OnchainTxHandler/ConfidentialWatchtower design a bit to share common code for the actual transaction generation I think will also need to happen, but most of those can probably be static functions (though maybe long-term functions on the ChanSigner or some other struct - I think moderately long-term we need to also be thinking about how to let users change the scripts that appear in the transactions themselves).

[ariard]

Refactoring the OnchainTxHandler/ConfidentialWatchtower design a bit to share common code

Yes of course getting helpers as we have in chan_utils

though maybe long-term functions on the ChanSigner or some other struct - I think moderately long-term we need to also be thinking about how to let users change the scripts that appear in the transactions themselves

Yes, I exactly made the point for in #605 for PTLC experimentation it should be doable to swap scripts in ChanSigner, that way avoid any state machine disruption. We would have to abstract verification too, but that's few lines of changes