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

Reply channel range simplification #737

Open

Conversation

@rustyrussell
Copy link
Collaborator

rustyrussell commented Feb 3, 2020

First a trivial renaming of the confusing complete field.

Then, we insist that query_channel_range replies be in order (everyone now does this anyway) and non-overlapping (ditto: c-lightning had strict enforcement on this, which was overzealous but means everyone now does it that way). Currently c-lightning maintains a bitmap for every block it requested, but it's much simpler to assume ordered replies, and simply check if we received the final one.

rustyrussell added 2 commits Feb 3, 2020
…onale

This was confusing: the flag name made implementers *think* they
knew what it was for.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current loose constraints causes confusion (and now all major
implementations meet the stricter requirements anyway).

You are allowed to provide more blocks than requested, but you have
to be complete and in order, and each reply has to have some overlap
with the requested range.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the rustyrussell:reply_channel_range-simplification branch from a769e08 to c950d4e Feb 3, 2020
- MUST set `first_blocknum` less than or equal to the `first_blocknum` in `query_channel_range`
- MUST set `first_blocknum` plus `number_of_blocks` greater than `first_blocknum` in `query_channel_range`.
- successive `reply_channel_range` message:
- MUST set `first_blocknum` to the previous `first_blocknum` plus `number_of_blocks` minus one.

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 4, 2020

Collaborator

I think that's the subtle part where everyone differed.
I like that proposal, but I'm not sure all implementations can read reply_channel_ranges that do this?
If all implementations do we're good, but if they don't we need to first roll out support for reading such format before we can roll out support for writing it (with a few months in-between).

require that each reply be relevant (overlapping the requested range).

By insisting that replies be in increasing order, the receiver can easily
determine if replies are done: simply check if `first_blocknum` plus

This comment has been minimized.

Copy link
@t-bast

t-bast Feb 4, 2020

Collaborator

I think this doesn't work in all cases...
For example when we split the last requested block between multiple responses:

* query: first_blocknum = 10, number_of_blocks = 2
* response 1: first_blocknum = 10, number_of_blocks = 2, short_channel_ids = {10x1x1, 11x0x2}
* response 2: first_blocknum = 11, number_of_blocks = 1, short_channel_ids = {11x1x1}

You will consider it done after response 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.