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

BOLT 1: add networks to init message. #682

Open
wants to merge 1 commit into
base: master
from

Conversation

@rustyrussell
Copy link
Collaborator

commented Oct 3, 2019

This has been discussed for forever, but now we have TLVs
the correct encoding seems obvious.

Signed-off-by: Rusty Russell rusty@rustcorp.com.au

This has been discussed for forever, but now we have TLVs
the correct encoding seems obvious.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Copy link
Collaborator

left a comment

ACK, does this supercede #678 or does it only complement it?

@rustyrussell

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 4, 2019

I think it replaces it, TBH. I'm not sure that query_channels means you should hang up.

@t-bast
t-bast approved these changes Oct 4, 2019
Copy link
Collaborator

left a comment

Sounds good then, I do think it supercedes it but wanted confirmation.

@Roasbeef

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

We have no real support for cross-chain routing in the protocol as is today, as a result, what use is this?

@rustyrussell

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 8, 2019

We have no real support for cross-chain routing in the protocol as is today, as a result, what use is this?

It means you can hang up if you accidentally connect to a testnet node?

@@ -225,8 +225,16 @@ Both fields `globalfeatures` and `localfeatures` MUST be padded to bytes with 0s
* [`gflen*byte`:`globalfeatures`]
* [`u16`:`lflen`]
* [`lflen*byte`:`localfeatures`]
* [`init_tlvs`:`tlvs`]

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Oct 8, 2019

Collaborator

qq on how we define this (i'm not super familiar with the parsing code): intuitively i would think that this would be

  * [`tlvs`: `init_tlvs`]

where tlvs is the type, and init_tlvs is the name. the current way is flipped from what I would expect, maybe i'm just not thinking about it correctly?

This comment has been minimized.

Copy link
@rustyrussell

rustyrussell Oct 9, 2019

Author Collaborator

The type is actually defined below... Confusing I know, but it's a consequence of incrementally implementing parsing.

We could just imply TLV from now on, with msg foo having a foo_tlv implicitly?

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Oct 9, 2019

Collaborator

either works for me, was just curious what the guidelines are :)

@@ -243,6 +252,8 @@ The receiving node:
- MUST ignore the bit.
- upon receiving unknown _even_ feature bits that are non-zero:
- MUST fail the connection.
- upon receiving `networks` containing no common chains

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Oct 8, 2019

Collaborator

what is the prescribed action if some chains intersect but not all? would think receivers should restrict all messages they send to this intersection?

This comment has been minimized.

Copy link
@rustyrussell

rustyrussell Oct 9, 2019

Author Collaborator

Hmm, like @Roasbeef I haven't thought too hard about multi-chain implementation.

Hmm, I think that addition (SHOULD) would be positive, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.