Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a context to interactive tx session #2849

Closed
wants to merge 1 commit into from
Closed

Conversation

pm47
Copy link
Member

@pm47 pm47 commented Apr 19, 2024

There are three closely related, but separate parameters:

  • InteractiveTxParams: low-level bitcoin parameters (amounts, outputs, locktime, confirmations)
  • Purpose: whether the interactive-tx is an initial funding, a splice, a rbf
  • SessionContext: higher level meta information about the interactive-tx (why are we opening or splicing this channel?)

This PR adds a SessionContext and carries it all the way to the completion (success or failure) of the interactive-tx process. It is useful for better debugging, diagnostics, metrics.

There are three closely related, but separate parameters:
- `InteractiveTxParams`: low-level bitcoin parameters (amounts, outputs, locktime, confirmations)
- `Purpose`: whether the interactive-tx is an initial funding, a splice, a rbf
- `SessionContext`: higher level meta information about the interactive-tx (why are we opening or splicing this channel?)

This PR adds a `SessionContext` and carries it all the way to the completion (success or failure) of the interactive-tx process.
@pm47 pm47 requested a review from t-bast April 19, 2024 12:56
/** The splice transaction has been negotiated, we're exchanging signatures. */
case class SpliceWaitingForSigs(signingSession: InteractiveTxSigningSession.WaitingForSigs) extends QuiescentSpliceStatus
case class SpliceWaitingForSigs(sessionContext: SessionContext, signingSession: InteractiveTxSigningSession.WaitingForSigs) extends QuiescentSpliceStatus
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether sessionContext should be (a) in [Rbf/Splice]WaitingForSigs, or (b) in InteractiveTxSigningSession.WaitingForSigs.

(a) is somewhat more consistent at the [Rbf/Splice]Status level, because we have to add sessionContext to [Rbf/Splice]InProgress in any case
(b) allows for better factorization between rbf and splice, and simpler codecs.

I went with (a) for this first shot, lmk what you think 🙏

def sessionId: ByteVector32
}
object SessionContext {
case class Unspecified(sessionId: ByteVector32) extends SessionContext
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, we could have a LiquidityRequest context which contains the requested LiquidityAds.Lease. There are various other cases in feature branches.

@t-bast
Copy link
Member

t-bast commented Jul 19, 2024

I don't think this will be necessary after #2848, which adds a liquidityPurchase_opt field to InteractiveTxBuilder. If liquidity ads is used, this field will be filled. Otherwise, the channel funding / splice will have been manually initiated by the node operator.

Every on-the-fly funding made to wallet nodes will simply be a liquidity purchase, and the details of what triggered it will be contained in the paymentDetails field.

@t-bast
Copy link
Member

t-bast commented Sep 18, 2024

Closing this in favor of #2848

@t-bast t-bast closed this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants