Skip to content

BOLT7: Signing channel updates with the same timestamp may get you bl…#614

Closed
jtimon wants to merge 1 commit into
lightning:masterfrom
jtimon:b7-timestamp-blacklist
Closed

BOLT7: Signing channel updates with the same timestamp may get you bl…#614
jtimon wants to merge 1 commit into
lightning:masterfrom
jtimon:b7-timestamp-blacklist

Conversation

@jtimon
Copy link
Copy Markdown
Contributor

@jtimon jtimon commented May 25, 2019

…acklisted

The current text seems to contradict itself.
If equal timestamps should be ignored, there's no chance to blacklist the node because of it as it said right afterwards.
Perhaps this is not the best fix, but it seems like a fix is needed.

@cdecker
Copy link
Copy Markdown
Collaborator

cdecker commented May 27, 2019

We don't have a concept of blacklisting in the spec. So far we will drop the channel_update if the timestamp is not strictly greater than any prior channel_update timestamp. Notice also that the timestamp is defined as a logical timestamp and not as a unix-timestamp, hence you could for example just have a monotonically increasing version counter instead of using the unix timestamp. c-lightning for example will take max(unix_timestamp, last_timestamp + 1) as the new value to avoid multiple channel_updates being unordered.

@jtimon
Copy link
Copy Markdown
Contributor Author

jtimon commented May 27, 2019

There's the following right below:

- MAY blacklist this `node_id`.

https://github.com/lightningnetwork/lightning-rfc/pull/614/files#diff-210f53c64b4a7ac8da48642029437668R503

Perhaps that part should be removed then?
Both things seem in contradiction.

@cfromknecht
Copy link
Copy Markdown
Collaborator

hence you could for example just have a monotonically increasing version counter instead of using the unix timestamp

won't this get your channel instapruned as a zombie since it will be much older than two weeks? lnd does the same in taking max(unix, last+1) which works well

@t-bast
Copy link
Copy Markdown
Collaborator

t-bast commented Jul 8, 2019

I think #621 replaces this PR and adds more details.

@jtimon
Copy link
Copy Markdown
Contributor Author

jtimon commented Jul 8, 2019

I think #621 replaces this PR and adds more details.

Perhaps. As said, this just seems contradictory, but I'm not sure what the best solution to remove the contradiction is. Perhaps it's #621 , If that one is merged and I haven't closed this one yet, please, remind me.

@t-bast t-bast added the spelling These changes may be merged without additional sign off from the weekly meeting label Jul 9, 2019
@jtimon
Copy link
Copy Markdown
Contributor Author

jtimon commented Jul 17, 2019

Closing since #621 got merged

@jtimon jtimon closed this Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spelling These changes may be merged without additional sign off from the weekly meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants