Refactor funding-tx-related fields into FundingScope
#3604
Conversation
When establishing a channel, the funding transaction may be replaced either: - after the funding transaction has confirmed using splicing, - before the funding transaction has confirmed for v2 channel establishment using tx_init_rbf, or - before the splice's funding transaction has confirmed using tx_init_rbf. In each of these cases, fields in ChannelContext will need to be updated once the funding transaction confirms. Additionally, the same fields for a pending attempt may need to be considered instead of a previously confirmed funding. This commit introduces a FundingScope to hold the aforementioned fields. It lives next to ChannelContext and will be needed whenever these fields are accessed. The next few commits will move the relevant fields to FundingScope and provide access to them whenever needed, allowing to swap in another FundingScope when necessary.
The funding pubkeys will be moved to FundingScope in the next commits. And since there isn't a similar method for holder's funding_pubkey, it makes sense to remove this method.
|
@wpaulino This primarily moves fields from The drawback is we'd be duplicating fields that wouldn't change. But this seems better than duplicating fields that would change and risk having inconsistent data. Also, it could prevent us from needing to worry about changing other uses of What are your thoughts? |
|
I was thinking we could deprecate the funding scope related fields from |
Yeah, this PR takes that approach. But when removing the fields we'd need to update all uses to take both For instance, writing an There's also I haven't looked exhaustively to see all usages, but it seems simpler to keep |
|
Hm, I'm not opposed to always writing As for |
|
Here's the alternative approach: https://github.com/jkczyz/rust-lightning/tree/2025-02-channel-funding-pubkeys-alt |
Yeah this looks pretty simple, and would also allow us to handle transitioning to new channel types in a splice since it already tracks the channel type features. I don't think the duplicate data is all that bad here once we have multiple scopes since it will eventually go away when the splice confirms. We should at least be able to avoid persisting the duplicate data, if we choose to, by cloning it from the "main" (currently confirmed) scope when reading. Thoughts on either of these approaches @TheBlueMatt? |
|
@TheBlueMatt Added some more commits to the alternative branch after pairing with @wpaulino yesterday. They move |
Continues the work of #3592 by moving fields related to the funding transaction into
FundingScope. This includes fields fromChannelContextfor the funding transaction andChannelTransactionParametersfor the funding outpoint and pubkeys.Based on #3592