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

Get ready for storing partial commit signatures #2896

Draft
wants to merge 1 commit into
base: input-info-taproot
Choose a base branch
from

Conversation

sstone
Copy link
Member

@sstone sstone commented Aug 7, 2024

This is another preparation PR for simple taproot channels that builds on top of #2895.
We currently store our peer's signature for our remote commit tx, so we can publish it if needed. If we upgrade funding tx to use musig2 instead of multisig 2-of-2 we will need to store a partial signature instead.

@sstone sstone force-pushed the input-info-taproot branch from aa0a099 to cf583ea Compare September 9, 2024 11:32
@sstone sstone force-pushed the store-partial-signatures branch from 46ef2b7 to 155b8dc Compare September 9, 2024 11:32
@sstone sstone force-pushed the store-partial-signatures branch 2 times, most recently from 452c0e0 to 16e75b0 Compare September 10, 2024 17:59
Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

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

Just one nit, but otherwise seems like straight forward changes.

val commitTxAndRemoteSigCodec: Codec[CommitTxAndRemoteSig] = (
private case class CommitTxAndRemoteSigEx(commitTx: CommitTx, remoteSig: ByteVector64, partialSig: Either[ByteVector64, PartialSignatureWithNonce], dummy: Boolean)

// remoteSig is now either a signature or a partial signature with nonce. To retain compatibility with the previous codec, we use remoteSig as a left/write indicator,
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: s/write/right

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 57310de

@sstone sstone force-pushed the store-partial-signatures branch from 57310de to 0ac5a45 Compare September 26, 2024 19:55
@sstone sstone force-pushed the input-info-taproot branch from 4163f0e to e15cb29 Compare October 1, 2024 07:39
@sstone sstone force-pushed the store-partial-signatures branch from 0ac5a45 to af1d5c7 Compare October 1, 2024 07:40
@sstone sstone force-pushed the input-info-taproot branch from e15cb29 to f83f006 Compare October 14, 2024 13:37
@sstone sstone force-pushed the store-partial-signatures branch from af1d5c7 to 5e9706a Compare October 14, 2024 13:38
@sstone sstone force-pushed the input-info-taproot branch from f83f006 to bde8e07 Compare October 15, 2024 13:26
@sstone sstone force-pushed the store-partial-signatures branch from 5e9706a to 1926d8a Compare October 15, 2024 13:26
@sstone sstone force-pushed the input-info-taproot branch from bde8e07 to a35921c Compare October 21, 2024 14:37
@sstone sstone force-pushed the store-partial-signatures branch from 1926d8a to f9fa5ba Compare October 21, 2024 14:38
@sstone sstone force-pushed the input-info-taproot branch from a35921c to d560a8c Compare November 26, 2024 13:38
@sstone sstone force-pushed the store-partial-signatures branch from f9fa5ba to ca706c7 Compare November 26, 2024 13:39
We currently store our peer's signature for our remote commit tx, so we can publish it if needed.
If we upgrade funding tx to use musig2 instead of multisig 2-of-2 we will need to store a partial signature instead.
@sstone sstone force-pushed the input-info-taproot branch from d560a8c to 154235d Compare December 9, 2024 14:23
@sstone sstone force-pushed the store-partial-signatures branch from ca706c7 to 65e74f6 Compare December 9, 2024 14:23
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