-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[watchtower/wtwire]: Watchtower Wire Messages #1512
Conversation
a9ab0b4
to
2c8ceab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super excited to have this ultimately land in lnd
! I've completed by initial pass through and didn't really find anything glaring w.r.t the proposed protocol based on the message definitions. I have a few questions w.r.t expected behavior and some edge cases, but I don't see any of them being major blockers.
watchtower/wtwire/session_init.go
Outdated
|
||
// MaxUpdates is the maximum number of updates the watchtower will honor | ||
// for this session. | ||
MaxUpdates uint16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason why this is constraint to a uint16
? 65k-ish updates is quite a bit, but one could imagine the private watch towers would want to simplify their client code a bit, allowing one session to be re-used for the entire duration. Or perhaps there's some underlying incentive to promote sessions with a finite lifetime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no absolute reason to cap at 65k, tho 256 is definitely too small. Ultimately the real limit can always be clamped based on the policy of the tower (public or private). Some thoughts on why we should prefer smaller sessions:
A session constitutes a promise on the watchtower's behalf to reserve future space for the client. The primary concern here is key loss, as the client won't be able to access this state, and the tower can't provably verify such an event. Allowing large sessions means that towers will be more likely to end up with space for which they've committed, but which will go unused. Smaller, more granular sessions with semi-frequent renegotiation would prevent this for the most part.
There is also an economic benefit to clients favoring small sessions in the key loss scenario, as it limits the number of slots a client can lose access to. Since (in the future) those updates will be paid for upfront, this also limits the total amount of funds a client might effectively lose in the process.
Lastly, I think there are decently strong reasons for preferring more ephemeral sessions when it comes to privacy. If one session can cover all (or at least a large number) of updates, there will be little incentive to actually implement session renegotiation and/or rotation, leading to fractured anonymity/behavioral sets.
// Code will be non-zero if the watchtower rejected the session init. | ||
Code uint16 | ||
|
||
// RewardPkScript is the reward address to be included in any sweep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the PR description for this PR doesn't match this particular field. Is the idea that that code returned will determine if this is an opaque payload, and how to interpret it? So for example, if a private tower, then it's blank, while if it's a public tower it may also have a reward script and also payment details.
// | ||
// This is part of the wtwire.Message interface. | ||
func (m *SessionInitReply) MaxPayloadLength(uint32) uint32 { | ||
return 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're also packaging a large-ish payment request (or other additional details depending on the final format of the e-cash tokens), then we may want to bump this up a bit.
watchtower/wtwire/message.go
Outdated
// The currently defined message types within this current version of the | ||
// Watchtower protocol. | ||
const ( | ||
MsgSessionInit MessageType = 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing godoc
comments.
// This number should always be one equal to the total number of | ||
// successful updates sent to the watchtower. This value must always be | ||
// less or equal than the negotiated MaxUpdates for the session. | ||
SeqNum uint16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 1 indexed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smallest LastApplied
we can send is 0, and signals that we've already received an ACK from the tower for that sequence number. Thus, we cannot send sequence number 0 for the first time and already have an ACK for 0. Another interpretation would be to think of sequence number 0 as the SessionInit
message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
watchtower/wtwire/state_update.go
Outdated
// Hint. The ciphertext is to be encrypted using the full transaction ID | ||
// of the revoked commitment transaction. | ||
// | ||
// The plaintext MUST be encoded using the negotiate Version for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
negotiate -> negotiated
watchtower/wtwire/wtwire_test.go
Outdated
// are too complex for the testing/quick package to automatically | ||
// generate. | ||
customTypeGen := map[wtwire.MessageType]typeGenFunc{ | ||
wtwire.MsgSessionInit: func(v []reflect.Value, r *rand.Rand) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the simplicity of many of these messages, I think we can actually leave most of these off all together. These are only required if the struct has complex fields like a *btcec.PublicKey
. I'd try to remove them all, the only add them back due to panics in the internal reflect code for testing/quick
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL, thanks!
@@ -2,14 +2,40 @@ package wtwire | |||
|
|||
import "io" | |||
|
|||
// TODO(conner): enumerate fail codes | |||
type SessionInitCode uint16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete godoc
comments.
@@ -2,14 +2,51 @@ package wtwire | |||
|
|||
import "io" | |||
|
|||
// TODO(conner): enumerate state update fail codes | |||
type StateUpdateCode uint16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing godoc
comment.
@@ -84,5 +95,5 @@ func (m *SessionInitReply) MsgType() MessageType { | |||
// | |||
// This is part of the wtwire.Message interface. | |||
func (m *SessionInitReply) MaxPayloadLength(uint32) uint32 { | |||
return 100 | |||
return 2 + MaxSessionInitReplyDataLength |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly GTM! Will take another pass when fixups are squashed :)
watchtower/wtwire/state_update.go
Outdated
// the client's brontine key used to make the request. | ||
type StateUpdate struct { | ||
// SeqNum is a 1-indexed, monotonically incrementing sequence number. | ||
// This number should always be one equal to the total number of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one?
933b795
to
30a3d28
Compare
thanks @halseth @Roasbeef, fixes are squashed down Any thoughts on limiting Other questions:
|
I think we can just go with 16 bits for now, and based on our experiments in the wild, possibly tweak it to use a larger number. The upside of having a lower-ish value like this is that then we effectively ensure that all clients can properly handle session rotation and management.
Yeah I think this would be a nice addition. So we'd have a version for the protocol itself, and then one for the blob independently. Or we can just add the version as the first field in the init message itself?
We can pad out the reply, but I don't think it's a blocker w.r.t getting an MVP up and running. |
00719a3
to
038ec16
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One repeated error code, otherwise LGTM!
// StateUpdateCodeSeqNumOutOfOrder signals the client sent an update | ||
// that does not follow the required incremental monotonicity required | ||
// by the tower. | ||
StateUpdateCodeSeqNumOutOfOrder StateUpdateCode = 21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code repeated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! was meant to be 11, fixed
Awesome, SGTM
I think this would lead to circular dependency with the parsing the message, as
Sweet, i'll make a reminder to circle back on this |
OK, so only thing we need on this PR is the addition of the new version/innit message. |
7a78dd8
to
dabf9f9
Compare
39c71f0
to
019d036
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
This PR adds the preliminary set of wire messages that will be used to negotiate and backup encrypted sweep info, that can be used by a watchtower in the event that it must sweep a revoked commitment transaction. The design borrows heavily from the existing
lnwire
package, which implements the Lightning wire protocol.NOTE: Keep in mind that nothing in this PR has been formalized under a BOLT specification. We are moving ahead with the implementation in LND in order to facilitate experimentation on testnet. As such, these features will not be available for mainnet until more solidification has occurred. Furthermore, they are likely to change after thorough review.
Looking forward to hearing feedback and criticisms :)
Transport and Session IDs
Though not a part of this PR, communication with watchtowers will use the same
brontide
transport used in the Lightning Protocol. When a connection to a watchtower is established, both the client and tower will authenticate each other's public keys used for the secure transport layer. The watchtower's is expected to be an LN address (pubkey + long-lived IP/Tor addr), potentially being the same as its node pubkey. At some point, these could be advertised along side node announcements or via another gossip/distribution mechanism.The client is not restricted in which pubkeys it uses to establish communication channels. We will use the term session id to refer to the public key of the client.
Messages
Unlike the Lightning protocol, where communication structure is peer-to-peer (peers have equal status), the Watchtower protocol falls more in line with the structure of traditional client-server communication. Thus,
wtwire
defines six message types, which are used for (1) session negotation (see below), (2) uploading encrypted blobs to watchtowers, and (3) exchanging supported features. TheInit
message is identical to the LNInit
message, though it bares a different message type on the wire so watchtower peers don't accidentally communicate with LN peers, and vice versa. Creating and updating sessions are implemented using a request/reply pattern. For consistency, it utilizes the same maximum message size of 65KB as inlnwire
(though this is up for debate really).MsgCreateSession
The
CreateSession
message is sent by a client to the watchtower, in order to request a number of "slots" that it can later fill with encrypted blobs (see StateUpdate). A session specifies parameters used in encoding and signing the sweep transaction, as well as negotiating fee rates and rewards.If a session is successfully initiated, it is identified by both client and watchtower via the connection's session id that made the request. A client wishing to consume updates for a particular session can do so later by making a
StateUpdate
request using the same session id, to the tower that accepted.Initially, the plan is for watchtowers to accept
RewardRate
of 0 sats/Msats, which means that they take no cut of the swept outputs. The same fee rate could be used when communicating with private/enterprise watchtowers. In the long run, private towers may require an out-of-band negotiation to whitelist the public keys of their intended clients, similar to./ssh/authorized_keys
. However, the intent is that they use the same wire protocol and exhibit behavior as close as possible to public towers, to hinder traffic analysis as much as possible.NOTE: A session is not tied to any particular channel. This offers the client freedom in selecting and rotating watchtowers between subsequent updates. Doing so offers better privacy if the appropriate actions are taken by the client, and also means that sessions are transferable if the whole session is not used before a channel is closed.
MsgCreateSessionReply
The
CreateSessionReply
message forms the response to the client to a proposedCreateSession
message. An errorCode
is returned to signal the watchtower's response. The tower may reject for reasons such as: not enough disk space, session id is already initialized, unacceptable fee rates, etc.If the error
Code
is zero, this implies the watchtowers acceptance of the session parameters. In this event, the watchtower provides aRewardPkScript
, which the client will include in an output of any presigned sweep transactions. In the event that a tower accepts with a fee rate of 0, this entry can be omitted as the output will always be dust.In the future, the plan is that this message will also contain an invoice for the client to fulfill, in order to pay for the session (and long-term resources used by the tower).
MsgStateUpdate
Once a session has been accepted, a client can begin making state updates for any revoked commitment transactions that happen in the normal operation of channels. In doing so, the client presigns a sweep transaction using the parameters negotiated in the session, e.g.
SweepFeeRate
, an output honoring theRewardRate
to the [possibly] optionalRewardPkScript
, etc.Once signed, the client serializes the information required to reconstruct sweep transaction and encrypts it using the TXID of the revoked commitment transaction. The details and implementation of this format will be proposed in a later PR.
The client allocates sequence numbers monotonically from 1 to the session's
MaxUpdates
, and includes this sequence number in theStateUpdate
. The sequence numbers are used by the client and tower to provide assurance as to which updates the tower has acknowledged, and are echoed by the client to inform the tower of which responses have been received.MsgStateUpdateReply
In response to
StateUpdate
, the watchtower sendsStateUpdateReply
. This provides an errorCode
for the client to determine if the state update was accepted, denoted by a zero-valuedCode
. In the event of an accepted state update, or some possible failures, theLastApplied
field is populated and interpretted as the last successfully committed sequence number.The client records the
LastApplied
sequence number with each state update. In the event that a response is lost, the watchtower can use this to determine if it needs to resend a failed response. It can also be helpful in detecting faulty clients that are not making progress, or some forms of malicious behavior that attempt to DOS the tower w/o advancing their state.