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

Add a Bootstrap File Flag #6316

Closed
nisdas opened this issue Jun 19, 2020 · 6 comments · Fixed by #6351
Closed

Add a Bootstrap File Flag #6316

nisdas opened this issue Jun 19, 2020 · 6 comments · Fixed by #6351
Labels
Enhancement New feature or request Good First Issue Good for newcomers Help Wanted Extra attention is needed

Comments

@nisdas
Copy link
Member

nisdas commented Jun 19, 2020

🚀 Feature Request

Description

Currently the only way to specify multiple bootnodes is using the bootstrap-node flag
multiple times or specifying the values comma-separated in a single instance. Both these methods are prone to be specified incorrectly due to how ENRs are encoded.

Describe the solution you'd like

Instead of manually typing out each bootstrap node we can instead provide the whole list in a file
and simply read the bootstrap nodes from there. That is a much easier and simpler way to manage
multiple bootnodes.

Describe alternatives you've considered

N.A

@nisdas nisdas added Enhancement New feature or request Good First Issue Good for newcomers Help Wanted Extra attention is needed labels Jun 19, 2020
@dv8silencer
Copy link
Contributor

I'd be happy to try to tackle this.

@prestonvanloon
Copy link
Member

Can't you do this already by loading config flags from file (#4878)?

flags.yaml

bootstrap-node: enr:-Ku4QMKVC_MowDsmEa20d5uGjrChI0h8_KsKXDmgVQbIbngZV0idV6_RL7fEtZGo-kTNZ5o7_EJI_vCPJ6scrhwX0Z4Bh2F0dG5ldHOIAAAAAAAAAACEZXRoMpD1pf1CAAAAAP__________gmlkgnY0gmlwhBLf22SJc2VjcDI1NmsxoQJxCnE6v_x2ekgY_uoE1rtwzvGy40mq9eD66XfHPBWgIIN1ZHCCD6A
./prysm.sh beacon-chain --config-file=flags.yaml

@dv8silencer
Copy link
Contributor

I think one reason is to separate configuration files consisting of client-specific flags from a file which would simply be a list of ENR nodes. A list of nodes in YAML would be more portable [between clients] while a config file would be specific to a client.

@nisdas
Copy link
Member Author

nisdas commented Jun 25, 2020

While that is possible, it is kind of messy, if you have 10 different bootnodes you would have to clump them all together. And with how unreadable ENRs are it can be difficult to see if you have made any mistake(ex: missed a comma, etc). Also having a separate file just for bootnodes can make it easy to handle it between clients.

@prestonvanloon
Copy link
Member

prestonvanloon commented Jun 25, 2020

Does it not work if you define the flag multiple times?

bootstrap-node: enr:-Ku4QMKVC_MowDsmEa20d5uGjrChI0h8_KsKXDmgVQbIbngZV0idV6_RL7fEtZGo-kTNZ5o7_EJI_vCPJ6scrhwX0Z4Bh2F0dG5ldHOIAAAAAAAAAACEZXRoMpD1pf1CAAAAAP__________gmlkgnY0gmlwhBLf22SJc2VjcDI1NmsxoQJxCnE6v_x2ekgY_uoE1rtwzvGy40mq9eD66XfHPBWgIIN1ZHCCD6A

bootstrap-node: enr:-Ku4QMKVC_MowDsmEa20d5uGjrChI0h8_KsKXDmgVQbIbngZV0idV6_RL7fEtZGo-kTNZ5o7_EJI_vCPJ6scrhwX0Z4Bh2F0dG5ldHOIAAAAAAAAAACEZXRoMpD1pf1CAAAAAP__________gmlkgnY0gmlwhBLf22SJc2VjcDI1NmsxoQJxCnE6v_x2ekgY_uoE1rtwzvGy40mq9eD66XfHPBWgIIN1ZHCCD6A

bootstrap-node: enr:-Ku4QMKVC_MowDsmEa20d5uGjrChI0h8_KsKXDmgVQbIbngZV0idV6_RL7fEtZGo-kTNZ5o7_EJI_vCPJ6scrhwX0Z4Bh2F0dG5ldHOIAAAAAAAAAACEZXRoMpD1pf1CAAAAAP__________gmlkgnY0gmlwhBLf22SJc2VjcDI1NmsxoQJxCnE6v_x2ekgY_uoE1rtwzvGy40mq9eD66XfHPBWgIIN1ZHCCD6A

Edit: I think it would have to be a StringSliceFlag, but that's a simple change.

@dv8silencer
Copy link
Contributor

I wrote another branch using a StringSliceFlag. It:

  • Does not add another flag
  • Can accept multiple --bootstrap-node flag/value pairs
  • Any one of the values can be a file name ending with .enr -- If it is, then it will YAML parse it and extract out all the nodes
  • Users cannot pass multiple nodes using comma delimiter into one value

Branch compared to prysm/master

Question: Should I create a new PR and remove the older one? Should I create another PR alongside it? Should I try to merge this branch with the one that is already part of a PR? (the commits are adding up on that one)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Good First Issue Good for newcomers Help Wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants