Add flag to allow reading bootstrap nodes from a file #6351

dv8silencer · 2020-06-22T20:59:37Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?
It adds a new flag:

--bootstrap-node-file value Path to a YAML file that lists bootstrap nodes that the beacon node will connect for peer discovery via DHT. Setting this flag will cause the bootstrap-node flag to be ignored. One line per node in YAML list format.

Which issues(s) does this PR fix?

Fixes #6316

Other notes for review

author dv8silencer <dv8silencer+github@gmail.com> 1592805180 -0500 committer dv8silencer <dv8silencer+github@gmail.com> 1592855989 -0500 Create a bootstrap-node-file flag which reads nodes from a YAML file and make the flag mutually exclusive to the bootstrap-node flag

CLAassistant · 2020-06-22T20:59:42Z

All committers have signed the CLA.

shared/cmd/flags.go

beacon-chain/node/node.go

Co-authored-by: terence tsao <terence@prysmaticlabs.com>

beacon-chain/node/node.go

beacon-chain/node/node_test.go

shared/cmd/flags.go

beacon-chain/node/node_test.go

Co-authored-by: Nishant Das <nish1993@hotmail.com>

… be passed. Each value can be .enr file which will be YAML parsed to extract nodes

… for it.

…check for the string prefix of a enr record or not to determine if file.

codecov · 2020-06-26T03:18:09Z

Codecov Report

Merging #6351 into master will decrease coverage by 0.06%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master    #6351      +/-   ##
==========================================
- Coverage   61.35%   61.28%   -0.07%     
==========================================
  Files         349      348       -1     
  Lines       28791    28920     +129     
==========================================
+ Hits        17664    17724      +60     
- Misses       8764     8818      +54     
- Partials     2363     2378      +15

prestonvanloon

One minor comment, otherwise LGTM

shared/cmd/flags.go

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

beacon-chain/node/node.go

shared/cmd/flags.go

dv8silencer added 2 commits June 22, 2020 15:04

parent c837dfb

b1d11d0

author dv8silencer <dv8silencer+github@gmail.com> 1592805180 -0500 committer dv8silencer <dv8silencer+github@gmail.com> 1592855989 -0500 Create a bootstrap-node-file flag which reads nodes from a YAML file and make the flag mutually exclusive to the bootstrap-node flag

Merge branch 'master' into dv8s-iss6316

977dcbe

dv8silencer requested a review from a team as a code owner June 22, 2020 20:59

dv8silencer requested review from 0xKiwi, prestonvanloon and terencechain June 22, 2020 20:59

terencechain reviewed Jun 22, 2020

View reviewed changes

shared/cmd/flags.go Outdated Show resolved Hide resolved

beacon-chain/node/node.go Outdated Show resolved Hide resolved

dv8silencer changed the title ~~PR for issue #6316~~ Add a flag that allows bootstrap nodes to be read from a file (issue#6316) Jun 22, 2020

0xKiwi changed the title ~~Add a flag that allows bootstrap nodes to be read from a file (issue#6316)~~ Add a flag that allows bootstrap nodes to be read from a file Jun 22, 2020

0xKiwi changed the title ~~Add a flag that allows bootstrap nodes to be read from a file~~ Add flag to allow reading bootstrap nodes from a file Jun 22, 2020

Refactor so that boot node reading is in its own function. Added test

77e9233

dv8silencer requested a review from terencechain June 23, 2020 00:14

Added period

0fc7f9f

Co-authored-by: terence tsao <terence@prysmaticlabs.com>

dv8silencer commented Jun 23, 2020

View reviewed changes

beacon-chain/node/node.go Outdated Show resolved Hide resolved

Merge branch 'master' into dv8s-iss6316

84a7aaf

dv8silencer commented Jun 23, 2020

View reviewed changes

beacon-chain/node/node_test.go Show resolved Hide resolved

dv8silencer and others added 7 commits June 22, 2020 20:49

Edit Bazel to include go-yaml dep

01abde2

Help Bazel build successfully

33a959b

Merge branch 'master' into dv8s-iss6316

542fb6b

bazel run //:gazella -- fix

eb0a658

Handle error from YAML parser to address build checks

c29bad4

Merge branch 'master' into dv8s-iss6316

01b14e9

Merge branch 'master' into dv8s-iss6316

557762f

nisdas requested changes Jun 23, 2020

View reviewed changes

shared/cmd/flags.go Outdated Show resolved Hide resolved

beacon-chain/node/node_test.go Outdated Show resolved Hide resolved

Updated flags.go to refine the Usage for the BootStrapNodeFile flag

a374c0b

Co-authored-by: Nishant Das <nish1993@hotmail.com>

dv8silencer marked this pull request as draft June 23, 2020 05:59

dv8silencer and others added 3 commits June 23, 2020 09:13

Merge branch 'master' into dv8s-iss6316

d55e880

Utilize stdlib TempFile to avoid polluting

0492cad

Merge branch 'master' into dv8s-iss6316

1fa4617

dv8silencer and others added 18 commits June 24, 2020 07:38

Merge branch 'master' into dv8s-iss6316

8d51d93

merge master into this branch

6a4a7cc

Merge branch 'master' into dv8s-iss6316

3b1a7c7

Merge branch 'master' into dv8s-iss6316

60585fb

Merge branch 'master' into dv8s-iss6316

7fa3d31

Merge branch 'master' into dv8s-iss6316

5ec6e9d

Merge branch 'master' into dv8s-iss6316

346e359

Merge branch 'master' into dv8s-iss6316

cc43387

Merge branch 'master' into dv8s-iss6316

c979c9e

Merge branch 'master' into dv8s-iss6316

e50b258

Merge branch 'master' into dv8s-iss6316

c782e13

Changed bootstrap-node to StringSlice flag to allow multiple nodes to…

a7923fa

… be passed. Each value can be .enr file which will be YAML parsed to extract nodes

Refactored to create separate readbootNodes function and added a test…

97597f6

… for it.

More cleaning up

6f28169

Changed wording in the cli help for --bootstrap-node

2b41366

Merge branch 'master' into dv8s-iss6316

b5d8dee

Since we are taking YAML files, got rid of .enr check and instead we …

6bd15cb

…check for the string prefix of a enr record or not to determine if file.

Merge with dv8silencer/tempissue6316

94a8fb2

prestonvanloon reviewed Jun 26, 2020

View reviewed changes

shared/cmd/flags.go Outdated Show resolved Hide resolved

Correct spacing

a2d48c7

Co-authored-by: Preston Van Loon <preston@prysmaticlabs.com>

nisdas reviewed Jun 26, 2020

View reviewed changes

beacon-chain/node/node.go Outdated Show resolved Hide resolved

Changed how we check for YAML file vs CLI-direct ENR

4ce0fff

nisdas reviewed Jun 26, 2020

View reviewed changes

shared/cmd/flags.go Outdated Show resolved Hide resolved

Minor: Changed to 1 string from concatenating 2

4c0714d

nisdas approved these changes Jun 26, 2020

View reviewed changes

nisdas added the OK to merge label Jun 26, 2020

Merge branch 'master' into dv8s-iss6316

d492828

prylabs-bulldozer bot merged commit 812a663 into prysmaticlabs:master Jun 26, 2020

dv8silencer deleted the dv8s-iss6316 branch July 3, 2020 21:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flag to allow reading bootstrap nodes from a file #6351

Add flag to allow reading bootstrap nodes from a file #6351

dv8silencer commented Jun 22, 2020

CLAassistant commented Jun 22, 2020 •

edited

Loading

codecov bot commented Jun 26, 2020 •

edited

Loading

prestonvanloon left a comment

Add flag to allow reading bootstrap nodes from a file #6351

Add flag to allow reading bootstrap nodes from a file #6351

Conversation

dv8silencer commented Jun 22, 2020

CLAassistant commented Jun 22, 2020 • edited Loading

codecov bot commented Jun 26, 2020 • edited Loading

Codecov Report

prestonvanloon left a comment

Choose a reason for hiding this comment

CLAassistant commented Jun 22, 2020 •

edited

Loading

codecov bot commented Jun 26, 2020 •

edited

Loading