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

EIP-778: Ethereum Node Records #778

Merged
merged 8 commits into from Mar 23, 2018

Conversation

Projects
None yet
7 participants
@fjl
Contributor

fjl commented Nov 23, 2017

This pull request proposes Ethereum Node Records, a new format for p2p connectivity information.

encoded as the concatenation of `r` and `s`.
- To verify a record, check that the signature was made by the public key in the
"secp256k1" key/value pair.
- To derive a node address, take the keccak256 hash of the public key.

This comment has been minimized.

@karalabe

karalabe Nov 23, 2017

Member

What's the definition of a node address?

This comment has been minimized.

@fjl

fjl Nov 23, 2017

Contributor

It's the byte sequence used for Kademlia distance calculations.

This comment has been minimized.

@FrankSzendzielarz

FrankSzendzielarz Dec 4, 2017

An interesting consequence of using hash(node id) for XOR distance calculations is that it becomes very difficult to implement the Kademlia bucket refresh part of the Kademlia system. The Maymounkov and Mazieres paper explains that after the idle k-bucket timeout (1 hour in the v4 rlpx.md), the k-bucket should refresh itself by calling FindNeighbours on a random node id within the k-bucket range. I don't see how that can be implemented if the k-bucket range is hash(node id) and FindNeighbours accepts (node id). I see the go implementation avoids the issue by just picking a random id irrespective of the bucket. For 'distant' buckets, the contents could get stale quickly (1 hour).

This comment has been minimized.

@fjl

fjl Dec 4, 2017

Contributor

In v5, FindNode will contain hash instead of the public key. See ethereum/devp2p#25

@fjl fjl force-pushed the fjl:enr branch from e6dd298 to 095488a Nov 23, 2017

| Key | Value |
|:-------------|:-------------------------------------------------|
| `id` | name of identity scheme, e.g. "secp256k1-keccak" |

This comment has been minimized.

@Arachnid

Arachnid Nov 23, 2017

Collaborator

Perhaps use IDs here specified in a table? secp256k1-keccak is about 5% of the maximum allowed record size on its own.

This comment has been minimized.

@fjl

fjl Nov 23, 2017

Contributor

I agree the name is a bit long.

@nonsense nonsense referenced this pull request Nov 30, 2017

Merged

p2p/enr: initial implementation #15585

9 of 9 tasks complete

@pirapira pirapira added the EIP label Dec 1, 2017

- The remainder of the record consists of arbitrary key/value pairs, which must be sorted
by key.
A record's signature is made and validated according to an *identy scheme*. The identity

This comment has been minimized.

@ligi

ligi Dec 9, 2017

Member

small typo - should be "identity scheme"

This comment has been minimized.

@fjl

fjl Dec 9, 2017

Contributor

fixed

fjl added a commit to ethereum/go-ethereum that referenced this pull request Dec 29, 2017

p2p/enr: initial implementation (#15585)
Initial implementation of ENR according to ethereum/EIPs#778
@fulldecent

This comment has been minimized.

Contributor

fulldecent commented Mar 22, 2018

@Arachnid You nominated this as an "EIPs that should be merged". Can you please share your notes on that here?

@Arachnid

This comment has been minimized.

Collaborator

Arachnid commented Mar 22, 2018

This EIP is referenced by #868, which has already been merged. As such, it needs to either be finalised and merged, or the dependency in the existing EIP needs to be removed.

@Arachnid Arachnid merged commit 872cec4 into ethereum:master Mar 23, 2018

@Arachnid Arachnid removed the needs-merge label Mar 23, 2018

detroitpro added a commit to akroma-project/akroma that referenced this pull request Jun 7, 2018

upstream/uint64 gas limit (#24)
* p2p/enr: initial implementation (#15585)

Initial implementation of ENR according to ethereum/EIPs#778

* accounts/abi: handle named ouputs prefixed with underscores (#15766)

* accounts/abi: handle named ouputs prefixed with underscores

* accounts/abi: handle collinding outputs for struct unpacks

* accounts: handle purely underscore output names

* consensus/ethash: fix overdue link (#15786)

* crypto: ensure private keys are < N (#15745)

Fixes #15744

* whisper/whisper2: fix Go 1.10 vet issues on type mismatches (#15783)

* containers/docker: change docker images to go1.9 (#15789)

* build: fix version comparison for go1.10 and beyond (#15781)

* eth/downloader: avoid hidden reference to finished statesync request (#15545)

* build: set CC through a command-line flag (#15784)

This avoids setting CC for the go run invocation, which fails on go1.10.

* vendor: update github.com/rjeczalik/notify for go1.10 (#15785)

* cmd/geth: remove trailing newline in license command (#15782)

* eth: enable preimage recording when tracing (#15787)

* core, p2p/discv5: use time.NewTicker instead of time.Tick (#15747)

* console: remove comment about 'invalid' input (#15735)

All inputs are saved into history, including 'invalid' inputs.

* eth: uncaptialize tracer preimage error message (#15792)

* eth: uncaptialize tracer preimage error message

* eth: improve very important error message

* eth: revert tracer preimage recording (#15800)

This reverts commits 85a1eda (#15792) and c495bca (#15787)
because they introduce database writes during tracing.

* various: remove redundant parentheses (#15793)

* all: switch gas limits from big.Int to uint64

* vendor: update github.com/rjeczalik/notify (#15801)

* lint: go fmt

* lint

* Merge pull request #16115 from nonsense/update_rjeczalik_notify

vendor: update rjeczalik/notify so that it compiles on go1.10

mariameda added a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018

hatricker pushed a commit to thundercore/thundercore-localchain that referenced this pull request Oct 8, 2018

p2p/enr: initial implementation (#15585)
Initial implementation of ENR according to ethereum/EIPs#778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment