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

Unofficial gossip tlv records using reserved ids #1037

Open
joostjager opened this issue Nov 1, 2022 · 6 comments
Open

Unofficial gossip tlv records using reserved ids #1037

joostjager opened this issue Nov 1, 2022 · 6 comments

Comments

@joostjager
Copy link
Collaborator

joostjager commented Nov 1, 2022

About tlv identifiers, the spec says:

type identifiers below 2^16 are reserved for use in this specification. type identifiers greater than or equal to 2^16 are available for custom records. Any record not defined in this specification is considered a custom record. This includes experimental and application-specific messages.

Reserved ids however are used today for unofficial extensions. For example by liquidity ads (id 1) and for random experiments (also id 1).

Does it make sense to even make an attempt to reserve a range of identifiers in the spec if there is no enforcement?

Or should nodes only relay gossip message that have custom records in the upper range? This may make it harder to deploy official extensions.

@t-bast
Copy link
Collaborator

t-bast commented Nov 2, 2022

For example by liquidity ads (id 1)

Since this is a proposal that intends to be added to the specification, it makes perfect sense that it uses a tlv in the spec range (and 1 is the right choice here IMO). I think you're confusing the spec and the implementation: CLN should not have used tlv 1 in their experimental version of liquidity ads deployed on mainnet (since it will likely create compatibility issues with the final version of liquidity ads), but the spec PR is completely right to use it.

@joostjager
Copy link
Collaborator Author

Okay yes, agreed. Perhaps picking reserved ids in a pr based on the pr number is a useful guideline too, like we do for blips.

Question remains as to whether experimental gossip in the reserved range should be relayed?

@t-bast
Copy link
Collaborator

t-bast commented Nov 2, 2022

Question remains as to whether experimental gossip in the reserved range should be relayed?

I think we have to relay them, otherwise non-upgraded nodes wouldn't relay gossip that contains tlvs that have been recently added to the spec, which would split the network every time a new feature adds optional tlvs to a gossip message.

@joostjager
Copy link
Collaborator Author

That then gets back to:

Does it make sense to even make an attempt to reserve a range of identifiers in the spec if there is no enforcement?

@t-bast
Copy link
Collaborator

t-bast commented Nov 2, 2022

It's a gentlemen's agreement to minimize conflict between experimentation and spec features, I think it's still valuable even if we cannot actually enforce it (since non-upgraded nodes can't tell the difference between a future spec feature and an experimentation).

This will be somewhat enforced socially though, if your app used tlv 1 for a feature that doesn't make sense to ever be integrated in the spec, I will publicly call you out with the following meme and my 7 followers may boycott your app:

shame

@joostjager
Copy link
Collaborator Author

Node images are definitely intended to be integrated in the spec 🤣

I suppose that if you choose to deploy with a reserved id, you shouldn't complain if a pr that is merged sooner takes it. The race is on for tlv id 1 😄

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

No branches or pull requests

2 participants