Add support for node discovery v5 #42

timxor · 2018-06-27T01:36:31Z

WIP: Please do not merge yet

Fixes #19.

Issue update for node discovery v5

Completion status: ~46%
Today's date: Saturday August 25th 2018
Target finish date: Monday September 3rd 2018
Task break down: ~46% = [6.5 / 14] = [ ((4 ✅ + (3🚧/2)) ) / (14✅🚧✖️)]
Issue: #19
Pull request: #42

Structural tasks

Task name	Implemented	Task Description
node records	✖️ No	add node records support for node discovery v5
packet types	🚧 Wip	add packet types support for node discovery v5
topics	🚧 Wip	add topics support for node discovery v5
cli parameter	✅ Yes	add v5 cli parameter support for node discovery v5
documentation	✅ Yes	add v5 cli parameter support for node discovery v5

Version 5 demo example tasks

Demo name	Implemented
peer-communication-v5.js	🚧 Wip
peer-communication-les-v5.js	✖️ No

New test coverage tasks

Test name	Implemented	Test Description
dpt-message-v5.js	✖️ No	static tests for v5 message types, node records, topics
dpt-simulator-v5.js	✖️ No	dpt integration communication tests for v5
eth-simulator-v5.js	✖️ No	eth integration communication tests for v5
les-simulator-v5.js	✖️ No	les integration communication tests for v5
util.js	✅ Yes

Pass existing tests, style, coverage

Name	Passing
coveralls	✖️ No
travis	✅ Yes

almost there 😀

holgerd77 · 2018-06-27T09:20:52Z

Hi Tim, cool, that already looks like a substantial amount of work and additions, happy to see the progress! Will think about the testing questions, can't promise though that I will make it today.

Let's not over-rush now in the other direction 😄 , you can really take your time, I would actually prefer a bit if this can ripen a bit during 2-3 weeks of ongoing work. Of course this also depends on the time-slots you can set free.

Anyway, cool to see progress here!

holgerd77 · 2018-06-28T10:01:22Z

Hi Tim, can you make version an option in dpt/server.js - probably defaulting to 0x04 if not provided - remove the VERSION constant and then do appropriate switches depending on what version someone chooses to initialize the server with?

For testing you should probably be able to build upon the integration tests and extend the DPT setup in integration/util.js with possibilities to either instantiate a v4 or v5 setup (or a mixed one? 😄 ) and then add some tests to the integration/dpt-simulator.js file who should behave like they should behave 😸.

Not sure if this is enough for tests but that would be a start, does this makes sense or do I miss something? Maybe also start early with this since according to my experience especially on the devp2p library having some tests makes further development a lot easier.

timxor · 2018-06-28T23:19:23Z

Awesome sounds good. Yeah, baby steps! Sure thing, will have a bunch of free time this weekend. Good call on the cli args for versioning, that ran through my mind, will add!

holgerd77 · 2018-06-29T11:16:17Z

@tcsiwula Just to make sure that there is no misunderstanding: version should be an option for the constructor in server.js, so options.version. This can of course be used from CLI but also in other programmatic contexts.

timxor · 2018-07-02T19:26:42Z

@holgerd77 option for the constructor, got it, thanks for clarifying -- pr update coming today

timxor · 2018-07-02T21:44:07Z

Hey @holgerd77 how is your linting working for the CI or whatever? I saw there was a "standard": { "parser": "babel-eslint" } though not to sure how to get this working or auto detecting with my ide.

holgerd77 · 2018-07-02T23:12:25Z

@tcsiwula Just run npm run lint for checking.

timxor · 2018-07-03T00:51:51Z

What about for fixing my linting errors? Running:

eslint --fix src

Oops! Something went wrong! :(

ESLint: 4.19.1.
ESLint couldn't find a configuration file. To set up a configuration file for this project, please run:

.
Should I make a .eslint config or how should I handle this and what settings?

holgerd77 · 2018-07-03T13:09:02Z

Just temporarily add "--fix" to the standard lint command in package.json and fix the remaining ones manually.

coveralls · 2018-07-03T18:39:19Z

Pull Request Test Coverage Report for Build 134

10 of 11 (90.91%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.08%) to 88.487%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/dpt/server.js	4	5	80.0%

Totals
Change from base Build 124:	-0.08%
Covered Lines:	908
Relevant Lines:	963

💛 - Coveralls

timxor · 2018-08-26T05:03:47Z

Issue update for node discovery v5

Completion status: ~46%
Today's date: Saturday August 25th 2018
Target finish date: Monday September 3rd 2018
Task break down: ~46% = [6.5 / 14] = [ ((4 ✅ + (3🚧/2)) ) / (14✅🚧✖️)]
Issue: #19
Pull request: #42

Structural tasks

Task name	Implemented	Task Description
node records	✖️ No	add node records support for node discovery v5
packet types	🚧 Wip	add packet types support for node discovery v5
topics	🚧 Wip	add topics support for node discovery v5
cli parameter	✅ Yes	add v5 cli parameter support for node discovery v5
documentation	✅ Yes	add v5 cli parameter support for node discovery v5

Version 5 demo example tasks

Demo name	Implemented
peer-communication-v5.js	🚧 Wip
peer-communication-les-v5.js	✖️ No

New test coverage tasks

Test name	Implemented	Test Description
dpt-message-v5.js	✖️ No	static tests for v5 message types, node records, topics
dpt-simulator-v5.js	✖️ No	dpt integration communication tests for v5
eth-simulator-v5.js	✖️ No	eth integration communication tests for v5
les-simulator-v5.js	✖️ No	les integration communication tests for v5
util.js	✅ Yes

Pass existing tests, style, coverage

Name	Passing
coveralls	✖️ No
travis	✅ Yes

update next on monday 😀

fjl · 2018-09-05T23:07:30Z

It's best if you start with node records. The actual v5 protocol in geth is a temporary placeholder.

timxor · 2018-09-10T18:41:08Z

@fjl thanks for the heads up, sounds good ^_^

fjl · 2018-09-21T23:02:51Z

@tcsiwula There is now a tracking issue for the actual protocol spec. Let's discuss there :).

holgerd77 · 2018-10-11T08:20:19Z

Hi @tcsiwula, thanks for all the active engagement in the direct protocol discussion with @fjl, I am also realizing that this is much more a work-in-progress spec than I thought. I would nevertheless try to see that we find some clean finish for this bounty PR, so that we don't have to carry this into 2019. 😛

Do you think it's realistic to get to a point where we have a somewhat complete version of a currently consistent draft state of the protocol implemented, or - if this is still too much work left - can't judge at the moment: do you see a consistent set of subfeatures/-parts that can be finished implementing, so that

a) tests are running and
b) this doesn't prevent the old protocol v4 version from running

(that would be the base conditions I could think of now, feel free to add).

Best
Holger

timxor · 2018-12-31T18:40:20Z

Succeeded by new PR #48 allowing administrative access & direct collaborator branch discovery-v5

timxor added 4 commits March 1, 2018 15:50

update bootnodes and debugging for parity detection

7186e72

Merge branch 'master' of https://github.com/ethereumjs/ethereumjs-devp2p

879c185

update VERSION to 0x05, add WIP types: 0x05 topicquery & 0x06 topicnodes

e4e5ffc

update contributors (me)

8751af5

timxor mentioned this pull request Jun 27, 2018

Node discovery v5 #19

Closed

add programitic support and default for version

0cc97bc

timxor added 2 commits July 3, 2018 10:51

resolve lint errors

0998404

remove debugging and revert timeout error for tests

dca1e98

timxor added 5 commits July 3, 2018 13:51

update bootstrap nodes with latest

01d71d7

added support cli support for v5

2c466a2

added documentation and package script for discv4 and discv5

9ad7d99

turn off verbose debugging for body payload and tx received

e17ef22

clean up for coveralls

b44a133

timxor changed the title ~~WIP: Add support for node discovery v5 with backwards compatibility for v4~~ Add support for node discovery v5 Aug 26, 2018

created discv5 folder to seperate logic better

2147753

adding support for ethereum node records

ea26454

fjl mentioned this pull request Sep 21, 2018

Specify Discovery v5 ethereum/devp2p#48

Closed

5 tasks

holgerd77 added the PR state: WIP label Sep 24, 2018

create new PR from existing PR ethereumjs#42

55bca8b

timxor mentioned this pull request Dec 31, 2018

discovery v5 support #48

Open

timxor closed this Dec 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for node discovery v5 #42

Add support for node discovery v5 #42

timxor commented Jun 27, 2018 •

edited

Loading

holgerd77 commented Jun 27, 2018

holgerd77 commented Jun 28, 2018 •

edited

Loading

timxor commented Jun 28, 2018

holgerd77 commented Jun 29, 2018

timxor commented Jul 2, 2018 •

edited

Loading

timxor commented Jul 2, 2018

holgerd77 commented Jul 2, 2018

timxor commented Jul 3, 2018

holgerd77 commented Jul 3, 2018

coveralls commented Jul 3, 2018 •

edited

Loading

timxor commented Aug 26, 2018

fjl commented Sep 5, 2018

timxor commented Sep 10, 2018

fjl commented Sep 21, 2018

holgerd77 commented Oct 11, 2018

timxor commented Dec 31, 2018

Add support for node discovery v5 #42

Add support for node discovery v5 #42

Conversation

timxor commented Jun 27, 2018 • edited Loading

WIP: Please do not merge yet

Issue update for node discovery v5

Structural tasks

Version 5 demo example tasks

New test coverage tasks

Pass existing tests, style, coverage

almost there 😀

holgerd77 commented Jun 27, 2018

holgerd77 commented Jun 28, 2018 • edited Loading

timxor commented Jun 28, 2018

holgerd77 commented Jun 29, 2018

timxor commented Jul 2, 2018 • edited Loading

timxor commented Jul 2, 2018

holgerd77 commented Jul 2, 2018

timxor commented Jul 3, 2018

holgerd77 commented Jul 3, 2018

coveralls commented Jul 3, 2018 • edited Loading

Pull Request Test Coverage Report for Build 134

💛 - Coveralls

timxor commented Aug 26, 2018

Issue update for node discovery v5

Structural tasks

Version 5 demo example tasks

New test coverage tasks

Pass existing tests, style, coverage

update next on monday 😀

fjl commented Sep 5, 2018

timxor commented Sep 10, 2018

fjl commented Sep 21, 2018

holgerd77 commented Oct 11, 2018

timxor commented Dec 31, 2018

timxor commented Jun 27, 2018 •

edited

Loading

holgerd77 commented Jun 28, 2018 •

edited

Loading

timxor commented Jul 2, 2018 •

edited

Loading

coveralls commented Jul 3, 2018 •

edited

Loading