Skip to content

Add bootnode component for EL and CL nodes. #99

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

Closed
wants to merge 6 commits into from

Conversation

simon0x1800
Copy link

I attempted to address #66 with this PR. @ferranbt please lmk if any changes need to be done

@metachris
Copy link
Contributor

Thanks for contributing! Ferran is out of office at the moment and will be back next week.

@metachris metachris requested a review from Copilot April 8, 2025 08:52
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

@metachris metachris requested a review from ferranbt April 8, 2025 08:53
@@ -253,6 +254,7 @@ func (r *RethEL) Run(svc *service, ctx *ExContext) {
"--ipcpath", "{{.Dir}}/reth.ipc",
"--addr", "127.0.0.1",
"--port", `{{Port "rpc" 30303}}`,
"--bootnodes", "enode://"+defaultDiscoveryEnodeID+"@bootnode:30301",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You cannot hardcode any service name (bootnode in this case).

Copy link
Author

@simon0x1800 simon0x1800 May 4, 2025

Choose a reason for hiding this comment

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

hey @ferranbt! do i take it from commandline args? and only the service name? the port and enode ID will be like this right?

@@ -49,18 +53,33 @@ func (l *L1Recipe) Artifacts() *ArtifactsBuilder {
return builder
}

func (m *Manifest) AddServiceWithDeps(name string, service Service, deps ...string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dependencies are mostly defined at the components level, not at the recipe.

Copy link
Collaborator

@ferranbt ferranbt left a comment

Choose a reason for hiding this comment

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

Added some comments

@simon0x1800 simon0x1800 force-pushed the add-bootnode branch 2 times, most recently from 265aeff to c91c6c0 Compare May 9, 2025 08:28
@simon0x1800
Copy link
Author

@ferranbt can you please check now if the changes look good?

@ferranbt
Copy link
Collaborator

Hey, appreciate your work here. I am going to follow up the work myself here #147 because there are too many layers. This was not a correct "Good first issue".

@ferranbt ferranbt closed this May 10, 2025
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.

3 participants