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

p2p/enode: use unix timestamp as base ENR sequence number #19903

Merged
merged 3 commits into from Sep 7, 2021

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Jul 31, 2019

This PR ensures that wiping all data associated with a node (apart from its key) will not generate already used sequence number for the ENRs, since all remote nodes would reject them until they out-number the previously published largest one.

@karalabe karalabe added this to the 1.9.2 milestone Jul 31, 2019
@karalabe karalabe modified the milestones: 1.9.2, 1.9.x Jul 31, 2019
@fjl fjl self-assigned this Aug 15, 2019
@karalabe karalabe modified the milestones: 1.9.x, 1.9.6 Oct 2, 2019
@fjl fjl modified the milestones: 1.9.6, 1.9.7 Oct 3, 2019
@karalabe karalabe modified the milestones: 1.9.7, 1.9.8 Nov 8, 2019
@karalabe karalabe modified the milestones: 1.9.8, 1.9.9 Nov 27, 2019
@karalabe karalabe modified the milestones: 1.9.9, 1.9.10 Dec 6, 2019
@karalabe karalabe modified the milestones: 1.9.10, 1.9.11 Jan 21, 2020
@karalabe karalabe modified the milestones: 1.9.11, 1.9.12 Feb 18, 2020
@karalabe karalabe modified the milestones: 1.9.12, 1.9.13 Mar 16, 2020
@karalabe karalabe modified the milestones: 1.9.13, 1.9.14 Apr 20, 2020
@karalabe karalabe modified the milestones: 1.9.14, 1.9.15 May 13, 2020
@karalabe karalabe modified the milestones: 1.9.20, 1.9.21 Aug 26, 2020
@karalabe karalabe modified the milestones: 1.9.21, 1.9.22 Sep 9, 2020
@karalabe karalabe modified the milestones: 1.9.22, 1.9.23 Oct 2, 2020
@karalabe karalabe modified the milestones: 1.9.23, 1.9.24 Oct 19, 2020
@karalabe karalabe removed this from the 1.9.24 milestone May 4, 2021
@fjl
Copy link
Contributor

fjl commented May 4, 2021

The general issue with timestamp as sequence number is that we cannot sign new records faster than once per second. Otherwise the sequence number may advance too fast and the original problem is not solved at all.

One idea we had in the triage today was to increase the granularity to millisecond. Another approach would be keeping the sequence number in two parts, with a timestamp part + update counter.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

karalabe and others added 3 commits September 7, 2021 10:59
This changes the local ENR update mechanism to block for up to 1ms in
LocalNode.Node when the previous update was less than 1ms ago.

To make this work, the granularity of sequence numbers is increased to
millisecond.
@fjl fjl merged commit 6ef3a16 into ethereum:master Sep 7, 2021
@fjl
Copy link
Contributor

fjl commented Sep 7, 2021

We did it! Merged after > 2 years. 😁

@fjl fjl removed their assignment Sep 7, 2021
yongjun925 pushed a commit to DODOEX/go-ethereum that referenced this pull request Dec 3, 2022
…9903)

This PR ensures that wiping all data associated with a node (apart from its nodekey)
will not generate already used sequence number for the ENRs, since all remote nodes
would reject them until they out-number the previously published largest one.

The big complication with this scheme is that every local update to the ENR can
potentially bump the sequence number by one. In order to ensure that local updates
do not outrun the clock, the sequence number is a millisecond-precision timestamp,
and updates are throttled to occur at most once per millisecond.

Co-authored-by: Felix Lange <fjl@twurst.com>
nibty pushed a commit to FairCrypto/go-ethereum that referenced this pull request Apr 10, 2024
…9903)

This PR ensures that wiping all data associated with a node (apart from its nodekey)
will not generate already used sequence number for the ENRs, since all remote nodes
would reject them until they out-number the previously published largest one.

The big complication with this scheme is that every local update to the ENR can
potentially bump the sequence number by one. In order to ensure that local updates
do not outrun the clock, the sequence number is a millisecond-precision timestamp,
and updates are throttled to occur at most once per millisecond.

Co-authored-by: Felix Lange <fjl@twurst.com>
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.

None yet

5 participants