Skip to content

Conversation

@bhandras
Copy link
Member

@bhandras bhandras commented Jan 9, 2023

This pull requests includes a draft version of changes required to make Taproot HTLCs optionally MuSig2 1.0 compatible.

Depends on:
- lightningnetwork/lnd#7171
- lightninglabs/lndclient#141

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

@bhandras bhandras mentioned this pull request Jan 9, 2023
1 task
@bhandras bhandras force-pushed the versioned-musig2-htlc branch 2 times, most recently from 9965619 to 1c42214 Compare January 9, 2023 20:16
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Changes so far look good to me. So there isn't going to be a new swap version that can be used to decide between MuSig2 0.4 and 1.0? Or is this just not implemented yet since it's a draft?

QuoteHtlcP2TR, _ = NewHtlcV3(
^int32(0), dummyPubKey, dummyPubKey, dummyPubKey, dummyPubKey,
quoteHash, &chaincfg.MainNetParams,
input.MuSig2Version100RC2, ^int32(0), dummyPubKey, dummyPubKey,
Copy link
Contributor

Choose a reason for hiding this comment

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

This uses 1.0 while the other changes in the PR use 0.4. Is that on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this PR is just making it possible to pass in the version if needed. We just need this specific dummy htlc for fee estimation so the version can be anything here.

@bhandras
Copy link
Member Author

Changes so far look good to me. So there isn't going to be a new swap version that can be used to decide between MuSig2 0.4 and 1.0? Or is this just not implemented yet since it's a draft?

Protocol version HTLC_V3 did include experimental MuSig2 support for loop out and we'll keep those users on 0.4 (see other PRs). For new clients we'll require everyone to use 1.0.

@bhandras bhandras force-pushed the versioned-musig2-htlc branch 6 times, most recently from 959a170 to 6884bd0 Compare January 16, 2023 13:47
@sputn1ck sputn1ck self-requested a review January 24, 2023 16:36
@bhandras bhandras force-pushed the versioned-musig2-htlc branch from 6884bd0 to 37fa043 Compare February 6, 2023 17:40
@bhandras bhandras changed the title [draft] loop: versioned MuSig2 HTLCs loop: versioned MuSig2 HTLCs Feb 6, 2023
@GeorgeTsagk GeorgeTsagk self-requested a review February 6, 2023 17:49
@bhandras bhandras force-pushed the versioned-musig2-htlc branch from 37fa043 to 7380b31 Compare February 6, 2023 19:42
@bhandras bhandras marked this pull request as ready for review February 6, 2023 19:43
This commit bumps LND and lndclient to make it possible to upgrade
taproot HTLC construction with a MuSig2 version. This is required to
support both old (MuSig2 0.4) and new (MuSig2 1.0) clients.
This commit adds the MuSig2 version as an input parameter when creating
Taproot htlcs. This will allow us to create both old and new MuSig2
Taproot HTLCs correctly.
@bhandras bhandras force-pushed the versioned-musig2-htlc branch from 7380b31 to 01970ad Compare February 6, 2023 19:46
@lightninglabs-deploy
Copy link

@bhandras, remember to re-request review from reviewers when ready

Copy link
Member

@sputn1ck sputn1ck left a comment

Choose a reason for hiding this comment

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

LGTM!

@bhandras bhandras requested a review from guggero February 7, 2023 08:37
AppMajor: 0,
AppMinor: 15,
AppPatch: 4,
AppPatch: 99,
Copy link
Member

Choose a reason for hiding this comment

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

That will be changed to 0.16 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.

Yes, once out but for now we need this to make sure that if someone runs HEAD of loop will also run with HEAD of LND.

Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

LGTM ➿

"github.com/lightninglabs/loop/swap"
"github.com/lightningnetwork/lnd/chainntnfs"
"github.com/lightningnetwork/lnd/channeldb"
invpkg "github.com/lightningnetwork/lnd/invoices"
Copy link
Member

Choose a reason for hiding this comment

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

was there a conflict with invoices?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just a convention that we seemed to have picked up in the LND codebase so I was just meant to conform to that.

@bhandras bhandras merged commit 3bd4624 into lightninglabs:master Feb 7, 2023
@bhandras bhandras deleted the versioned-musig2-htlc branch February 7, 2023 10:18
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.

5 participants