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

blip-0038: block header gossip #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

t-bast
Copy link
Contributor

@t-bast t-bast commented Jul 3, 2024

Lightning nodes must stay in sync with the blockchain to protect themselves against malicious peers that could publish revoked commitment transactions. Honest nodes should help their peers by letting them know about the latest block header they have received. This allows detecting when a node is behind and may need to fix their bitcoin node.

We extend lightning messages to include information about the last header we've received and the corresponding block height.

@Roasbeef can you detail what lnd is doing regarding this feature? You mentioned encoding it in ping and pong, but what exactly do you include?

Lightning nodes must stay in sync with the blockchain to protect
themselves against malicious peers that could publish revoked
commitment transactions. Honest nodes should help their peers by
letting them know about the latest block header they have received.
This allows detecting when a node is behind and may need to fix
their bitcoin node.

We extend lightning messages to include information about the last
header we've received and the corresponding block height.
@TheBlueMatt
Copy link
Contributor

If we're just gonna include this once, should we include a proof that the given header is at the given height with a coinbase merkle path? I dunno maybe that means some nodes can't provide the proof, but giving a height value that can't be verified at all isn't that useful, in many cases :/

@t-bast
Copy link
Contributor Author

t-bast commented Jul 5, 2024

If we're just gonna include this once

I think we'll want to include it in ping and pong as well, but I didn't do it yet, I'm waiting for @Roasbeef to let me know how they're currently encoding it in lnd first.

But it's probably a good idea to include whatever proof would help to validate the height, I'll need to look into this!

@guggero
Copy link

guggero commented Jul 8, 2024

The current ping payload in lnd is the best block header serialized in the default wire format, so 80 bytes: https://github.com/lightningnetwork/lnd/blob/e61cba8d227faa959e1c591cafccf74a8ba98e30/peer/brontide.go#L582

@t-bast
Copy link
Contributor Author

t-bast commented Jul 8, 2024

The current ping payload in lnd is the best block header serialized in the default wire format, so 80 bytes: https://github.com/lightningnetwork/lnd/blob/e61cba8d227faa959e1c591cafccf74a8ba98e30/peer/brontide.go#L582

This is directly encoded inside the ping message instead of using a TLV? That doesn't seem very future-proof, it doesn't support padding with random bytes to create ping messages of any chosen length? And it doesn't contain any magic byte to indicate that the ping contents contain a block header, so the reader just tries interpreting it as such and ignores it if that doesn't work?

Can you detail why that encoding was chosen? It feels like using an empty ping / pong content and using the TLV stream is more flexible, as it allows padding and combining with potentially other data we'd want to exchange in those messages.

@guggero
Copy link

guggero commented Jul 8, 2024

I would guess that this is a case of "let's just prepare the code to be able to send a custom ping payload but then discuss the actual format at a later point", since we don't currently even look at the payload on the other side, it's just being logged (on trace level) and exposed on the RPC: lightningnetwork/lnd#5621

I do agree that using TLV here definitely makes sense.

@Roasbeef
Copy link
Contributor

This is directly encoded inside the ping message instead of using a TLV? That doesn't seem very future-proof, it doesn't support padding with random bytes to create ping messages of any chosen length

Yeah it's just directly encoded, as there's no existing structure defined in the ping messages. Just as the requirement of pong is just to send the N random bytes.

And it doesn't contain any magic byte to indicate that the ping contents contain a block header, so the reader just tries interpreting it as such and ignores it if that doesn't work?

Up until now, there've been no readers 🤷 , they'd identify the payload based on the 80 byte length, and potentially knowledge of the prev block header referenced.


## Specification

We add the following TLV field to the `init` message:
Copy link
Contributor

@Roasbeef Roasbeef Jul 13, 2024

Choose a reason for hiding this comment

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

An init message is only sent once when the connection is established. Putting the payload in ping would mean that the new headers can continually be sent and monitored.

To allow implementations to more easily distinguish the header message amidst a string of other random bytes in the payload, perhaps we can add special marker/magic bytes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see two possible designs for this:

  • keep the body of the ping messages random and include the block header in the message's tlv stream
  • make the body of the ping message a TLV stream, that contains the block header (+ padding)

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