Skip to content

Conversation

@bhandras
Copy link
Member

@bhandras bhandras commented Jan 9, 2023

This PR defaults new swaps to MuSig 1.0rc and adds support for MuSig loopin swaps for the success case.

Depends on:
- #546

Pull Request Checklist

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

@bhandras bhandras force-pushed the musig-keyreveal branch 8 times, most recently from 6edbef8 to 086f21a Compare January 16, 2023 19:39
@sputn1ck sputn1ck self-requested a review January 24, 2023 16:36
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.

Very nice PR, easy to grok changes 💯

@GeorgeTsagk GeorgeTsagk self-requested a review February 7, 2023 22:06
@sputn1ck sputn1ck self-requested a review February 9, 2023 13:07
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.

This is not a draft anymore right? LGTM 🐐

loopdb/store.go Outdated
// path: loopInBucket/loopOutBucket -> swapBucket[hash]
// -> senderInternalKeyKey
// value: serialized private key.
senderInternalKeyKey = []byte("sender-internal-key")
Copy link
Member

Choose a reason for hiding this comment

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

nit: rename to senderInternalPrivKeyKey

Copy link
Member Author

Choose a reason for hiding this comment

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

done

}

var receiverKeyArray [33]byte
var receiverKeyArray, receriverInternalKeyArray [33]byte
Copy link
Member

Choose a reason for hiding this comment

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

nit: typo receriverInternalKeyArray

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@sputn1ck sputn1ck self-requested a review February 16, 2023 15:10
@bhandras bhandras marked this pull request as ready for review March 7, 2023 14:19
@bhandras
Copy link
Member Author

bhandras commented Mar 7, 2023

Added shared key encryption to the stored priv keys.

@bhandras bhandras changed the title [draft] loop: Loop In MuSig2 support loop: Loop In MuSig2 support Mar 7, 2023
@lightninglabs-deploy
Copy link

@sputn1ck: review reminder

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➰

minor comments

// HtlcKeys is a holder of all keys used when constructing the swap HTLC. Since
// it's used for both loop in and loop out swaps it may hold partial information
// about the sender or receiver depending on the swap type.
type HtlcKeys struct {
Copy link
Member

Choose a reason for hiding this comment

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

👍 really like this HtlcKeys struct holding all keys

loopdb/store.go Outdated
if err != nil {
return err
}
// Store the htlc keys if the contract is recent.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is not super clear to me

Copy link
Member Author

Choose a reason for hiding this comment

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

ptal

client.go Outdated

import (
"context"
"encoding/hex"
Copy link
Member

Choose a reason for hiding this comment

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

this commit message has a typo:

  • loopdb: encrpyt stored.. -> loopdb: encrypt stored..
  • This technique allows us to encrpyt.. -> This technique allows us to encrypt..

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, fixed!


// If this is a MuSig2 swap then we'll generate a brand new key pair
// and will use that as the internal key for the HTLC.
if loopdb.CurrentProtocolVersion() >= loopdb.ProtocolVersionMuSig2 {
Copy link
Member

Choose a reason for hiding this comment

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

What does the >= imply for the order of protocol versions? Anything that comes after ProtocolVersionMuSig2 is gonna share the same code in the block below? If yes, why?

Copy link
Member Author

Choose a reason for hiding this comment

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

This just means that currently we make the assumption that all further protocol upgrades will include musig2 loopin. If this is not the case we'll simply change it.

@bhandras
Copy link
Member Author

ping @sputn1ck

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!

This commit adds a new protocol version which will add MuSig2 loop in
and loop out using key reveal and extends the existing protocol with new
message members to be able to pass around htlc internal public keys.

The commit also fixes some minor formatting issues in the server proto.
This commit adds a new struct to hold all HTLC keys and refactors the
SwapContract which is used by both loopin and loopout swaps to use this
new struct. The newly added internal keys will for now hold the script
keys to keep everything equivalent but are already stored and read back
if the protocol version is set to MuSig2.
This commit changes how we create loopin swaps if the client activates
the experimental MuSig2 features. When creating a new loopin swap the
client will create (and store) a new key that will be used as the
sender's internal key when constructing the HTLC. The client will send
the public part to the server and will also receive (and store) the
server's (receiver) internal public key.
This commit adds key reveal to MuSig2 loopin swaps' success path. In
this case the client reveals their internal HTLC key to the server when
the swap invoice is settled. With this key the server can sweep the swap
HTLC without any more interaction from the client. We'll do this every
block (after the invoice has been settled).
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.

I like the change to deriving the shared secret

@bhandras bhandras merged commit 7f19c43 into lightninglabs:master Mar 23, 2023
@bhandras bhandras deleted the musig-keyreveal branch March 23, 2023 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.

4 participants