rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Make `tx_abort` a variant of `InteractiveTxMessageSend` and refactor

Open dunxen opened this issue 1 year ago • 3 comments

Shouldn't hold up this PR, but is there any reason why we don't simply add a TxAbort variant to InteractiveTxMessageSend? Then we wouldn't need either InteractiveTxMessageSendResult or HandleTxCompleteResult, IIUC, which would remove two layers of structs.

Otherwise, we go through the following conversions just to end up with a MessageSendEvent:

  • Result<InteractiveTxMessageSend, AbortReason>
  • InteractiveTxMessageSendResult(Result<InteractiveTxMessageSend, msgs::TxAbort>)
  • MessageSendEvent

The Results don't seem to buy us anything more than the extra variant would as we aren't doing anything special for the Err case. Then it would simply be:

  • InteractiveTxMessageSend
  • MessageSendEvent

And AFAICT, InteractiveTxConstructor has the channel_id, so there's no need to grab it from the ChannelContext. Or will it change in some case?

Originally posted by @jkczyz in https://github.com/lightningdevkit/rust-lightning/pull/3137#discussion_r1838980862

dunxen avatar Nov 13 '24 11:11 dunxen

We're gonna walk back our v2 enablement for 0.1, so this needs to happen for 0.2.

TheBlueMatt avatar Dec 12 '24 02:12 TheBlueMatt

FYI, dual funding will slip to 0.3 as development has been focused on splicing.

jkczyz avatar Sep 15 '25 19:09 jkczyz

This may not be relevant anymore. After #4060, InteractiveTxMessageSendResult was removed and Result<InteractiveTxMessageSend, AbortReason> is now used to fail the ongoing negotiation internally.

wpaulino avatar Sep 15 '25 22:09 wpaulino