Discovery only bootnode docker - #194
Merged
Merged
Conversation
- Introduced a new Docker Compose file for bootnode service. - Updated `.dockerignore` to exclude the new `build/bin` directory. - Enhanced `Makefile` to include bootnode as a target. - Added `bootnodes.list` file for bootstrap node configurations. - Modified `main.go` to support bootnode configuration via command-line flags. - Updated Dockerfile to ensure bootnode is built and available. - Refined `start-bootnode.sh` to handle bootnode parameters and logging more effectively.
- Updated Dockerfile to include curl installation and clean up APT lists. - Modified start-bootnode.sh to auto-detect public IP if EXTIP is not set, with added logging for detection failures.
- Added DISABLE_EXTIP environment variable to docker-compose.bootnode.yml for controlling external IP detection. - Updated start-bootnode.sh to handle DISABLE_EXTIP, allowing for bootnode startup without NAT external IP if set.
- Added multiple new enode entries to the bootnodes.list for mainnet - Removed the k8s-bootnode.yaml file
📝 WalkthroughWalkthroughThis PR adds bootnode discovery input support, introduces a bootnode container/service setup, and updates the bootnode launch script to derive runtime parameters, keys, logging, and NAT/address settings from environment variables. ChangesBootnode packaging and startup
Sequence Diagram(s)sequenceDiagram
participant BootnodeService as bootnode service
participant StartScript as docker/start-bootnode.sh
participant Curl as curl
participant Bootnode as bootnode
participant Discover as discover.ListenUDP
BootnodeService->>StartScript: start container with bootnode env vars
StartScript->>Curl: resolve EXTIP when unset and extip is enabled
StartScript->>Bootnode: exec bootnode -nodekey "$NODEKEY_FILE" $params "$@"
Bootnode->>Discover: pass Bootnodes in discover.Config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
anunay-xin
marked this pull request as ready for review
June 26, 2026 13:24
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
cmd/bootnode/main.go (1)
103-127: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
utils.Fatalfon any single bad entry aborts the whole bootnode.With a large, externally-maintained
bootnodes.list, one malformed enode (or a stray non-enode line) kills startup entirely. Consider logging and skipping invalid entries instead of fatally exiting, so the bootnode still comes up with the valid peers.♻️ Skip-and-warn instead of fatal
for _, url := range urls { node, err := discover.ParseNode(url) if err != nil { - utils.Fatalf("invalid bootnode %q: %v", url, err) + log.Warn("skipping invalid bootnode", "url", url, "err", err) + continue } bootnodeList = append(bootnodeList, node) }discover.ParseNode behavior (rejecting non-
enode://lines, comments, blanks) per go-ethereum 1.9.11.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/bootnode/main.go` around lines 103 - 127, The bootnode parsing in main should not abort startup when a single entry fails to parse. Update the bootnodes processing in main and the discover.ParseNode loop to warn and skip invalid URLs or non-enode/comment/blank lines instead of calling utils.Fatalf for each bad entry, while still collecting all valid peers into bootnodeList.docker/Dockerfile (1)
3-4: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
--no-install-recommendstoapt-get install.Reduces image size and attack surface by avoiding recommended-but-unneeded packages. Applies to both the builder and runtime stages.
As per static analysis hint Trivy DS-0029 ('apt-get' missing '--no-install-recommends').🔧 Proposed change
-RUN apt-get update && apt-get install -y git build-essential curl \ - && rm -rf /var/lib/apt/lists/* +RUN apt-get update && apt-get install -y --no-install-recommends git build-essential curl \ + && rm -rf /var/lib/apt/lists/*Also applies to: 14-15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile` around lines 3 - 4, The apt-get install step in the Dockerfile is missing --no-install-recommends, which should be added to reduce the image size and avoid pulling unnecessary packages. Update the install command used in the builder/runtime stages so the apt-get install invocations include --no-install-recommends alongside the existing packages, keeping the same cleanup step afterward.Source: Linters/SAST tools
docker/start-bootnode.sh (1)
5-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the duplicated
DISABLE_EXTIPblocks.The
DISABLE_EXTIPguard is evaluated twice (lines 5 and 17) with the first block only printing a message. The two branches can be merged into a single conditional, simplifying the extip flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/start-bootnode.sh` around lines 5 - 24, The EXTIP flow in the bootnode startup script duplicates the DISABLE_EXTIP check in two separate conditionals, with the first branch only emitting a message and the second handling NAT setup. Merge the logic into a single conditional around the existing EXTIP detection and parameter assignment so DISABLE_EXTIP is handled once, and keep the behavior for auto-detection, explicit EXTIP, and the fallback warning intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bootnode/bootnodes.list`:
- Line 9: The bootnode entry uses an inconsistent discovery port compared with
the other entries, so verify the enode address in bootnodes.list and change the
port to match the expected discovery port if this was accidental. Update the
specific bootnode line with the correct port value so it is consistent with the
rest of the list and remains discoverable.
In `@docker/start-bootnode.sh`:
- Around line 40-45: The PRIVATE_KEY path in start-bootnode.sh writes directly
to NODEKEY_FILE without ensuring the parent directory exists, so the echo
redirection can fail when the bootnode directory is missing. Update the
PRIVATE_KEY branch to create the parent directory before writing, matching the
mkdir -p behavior used in the bootnode -genkey branch. Keep the fix localized to
the NODEKEY_FILE handling logic so both branches consistently prepare the
destination path.
- Around line 67-70: Quote the shell variable expansions in the bootnode startup
script to satisfy SC2086: update the `address=` assignment in
`start-bootnode.sh` so `NODEKEY_FILE` is passed as a quoted argument to
`bootnode`, and change the `echo` write that uses `address` so the variable is
quoted when redirected to `BOOTNODE_ENODE_OUT`. Leave the later `$params` usage
unchanged, since it is intentionally unquoted for splitting.
---
Nitpick comments:
In `@cmd/bootnode/main.go`:
- Around line 103-127: The bootnode parsing in main should not abort startup
when a single entry fails to parse. Update the bootnodes processing in main and
the discover.ParseNode loop to warn and skip invalid URLs or
non-enode/comment/blank lines instead of calling utils.Fatalf for each bad
entry, while still collecting all valid peers into bootnodeList.
In `@docker/Dockerfile`:
- Around line 3-4: The apt-get install step in the Dockerfile is missing
--no-install-recommends, which should be added to reduce the image size and
avoid pulling unnecessary packages. Update the install command used in the
builder/runtime stages so the apt-get install invocations include
--no-install-recommends alongside the existing packages, keeping the same
cleanup step afterward.
In `@docker/start-bootnode.sh`:
- Around line 5-24: The EXTIP flow in the bootnode startup script duplicates the
DISABLE_EXTIP check in two separate conditionals, with the first branch only
emitting a message and the second handling NAT setup. Merge the logic into a
single conditional around the existing EXTIP detection and parameter assignment
so DISABLE_EXTIP is handled once, and keep the behavior for auto-detection,
explicit EXTIP, and the fallback warning intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3225e8e-564c-4dc2-bee0-29c9ad0cfc08
📒 Files selected for processing (7)
.dockerignoreMakefilebootnode/bootnodes.listcmd/bootnode/main.godocker-compose.bootnode.ymldocker/Dockerfiledocker/start-bootnode.sh
| enode://938f2e3f409a12573e6da6460b6497c45e2bec393756b989b8874f647911cca39d0ffef8554a45698a8f21a7e870288beb638b3770537a12118e30bd6f9ae806@109.199.104.176:30303 | ||
| enode://f8848e405142b8e88f054fe85ac5e4a75cfd7e353aee7e66797719828d3d5aa2cd62f1355140c0852d3dcb2439a076234c77415ca701318ea1f69a496a0b4b32@109.123.232.199:30303 | ||
| enode://0857894c01314e75520fbdb7e37869666f230c8ab96c0e3067561077209e8f48a9cefb3a71c3c8094448629c152f22c2e5e66bb7ed2c38bfbd9f24941f571beb@103.7.54.103:30303 | ||
| enode://91e59fa1b034ae35e9f4e8a99cc6621f09d74e76a6220abb6c93b29ed41a9e1fc4e5b70e2c5fc43f883cffbdcd6f4f6cbc1d23af077f28c2aecc22403355d4b1@144.126.142.140:30304 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Verify the discovery port 30304 on line 9.
Every other entry uses 30303; line 9 alone uses 30304. If unintentional, peers will fail to discover this node on the expected port.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bootnode/bootnodes.list` at line 9, The bootnode entry uses an inconsistent
discovery port compared with the other entries, so verify the enode address in
bootnodes.list and change the port to match the expected discovery port if this
was accidental. Update the specific bootnode line with the correct port value so
it is consistent with the rest of the list and remains discoverable.
Comment on lines
40
to
45
| if [[ ! -z "$PRIVATE_KEY" ]]; then | ||
| echo "$PRIVATE_KEY" > bootnode.key | ||
| elif [[ ! -f ./bootnode.key ]]; then | ||
| bootnode -genkey bootnode.key | ||
| echo "$PRIVATE_KEY" > "$NODEKEY_FILE" | ||
| elif [[ ! -f "${NODEKEY_FILE}" ]]; then | ||
| mkdir -p "$(dirname "${NODEKEY_FILE}")" | ||
| bootnode -genkey "$NODEKEY_FILE" | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
PRIVATE_KEY branch can fail when the key directory doesn't exist.
The genkey branch creates the parent dir via mkdir -p, but the PRIVATE_KEY branch writes directly to $NODEKEY_FILE. If the directory (e.g. bootnode/) is missing, the redirection on line 41 fails and the script aborts. Create the directory before writing.
🐛 Proposed fix
if [[ ! -z "$PRIVATE_KEY" ]]; then
+ mkdir -p "$(dirname "${NODEKEY_FILE}")"
echo "$PRIVATE_KEY" > "$NODEKEY_FILE"
elif [[ ! -f "${NODEKEY_FILE}" ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [[ ! -z "$PRIVATE_KEY" ]]; then | |
| echo "$PRIVATE_KEY" > bootnode.key | |
| elif [[ ! -f ./bootnode.key ]]; then | |
| bootnode -genkey bootnode.key | |
| echo "$PRIVATE_KEY" > "$NODEKEY_FILE" | |
| elif [[ ! -f "${NODEKEY_FILE}" ]]; then | |
| mkdir -p "$(dirname "${NODEKEY_FILE}")" | |
| bootnode -genkey "$NODEKEY_FILE" | |
| fi | |
| if [[ ! -z "$PRIVATE_KEY" ]]; then | |
| mkdir -p "$(dirname "${NODEKEY_FILE}")" | |
| echo "$PRIVATE_KEY" > "$NODEKEY_FILE" | |
| elif [[ ! -f "${NODEKEY_FILE}" ]]; then | |
| mkdir -p "$(dirname "${NODEKEY_FILE}")" | |
| bootnode -genkey "$NODEKEY_FILE" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/start-bootnode.sh` around lines 40 - 45, The PRIVATE_KEY path in
start-bootnode.sh writes directly to NODEKEY_FILE without ensuring the parent
directory exists, so the echo redirection can fail when the bootnode directory
is missing. Update the PRIVATE_KEY branch to create the parent directory before
writing, matching the mkdir -p behavior used in the bootnode -genkey branch.
Keep the fix localized to the NODEKEY_FILE handling logic so both branches
consistently prepare the destination path.
Comment on lines
+67
to
+70
| address="enode://$(bootnode -nodekey ${NODEKEY_FILE} -writeaddress)@${host}:${BOOTNODE_PORT}" | ||
| echo "Starting the bootnode with address at $address" | ||
| BOOTNODE_ENODE_OUT="${BOOTNODE_ENODE_OUT:-bootnode/bootnode.enode}" | ||
| echo $address > "$BOOTNODE_ENODE_OUT" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Quote variable expansions (SC2086) on lines 67 and 70.
${NODEKEY_FILE} (line 67) and $address (line 70) should be quoted to prevent word splitting/globbing. Note line 78's $params is intentionally left unquoted for argument splitting and can be ignored.
🔧 Proposed change
-address="enode://$(bootnode -nodekey ${NODEKEY_FILE} -writeaddress)@${host}:${BOOTNODE_PORT}"
+address="enode://$(bootnode -nodekey "${NODEKEY_FILE}" -writeaddress)@${host}:${BOOTNODE_PORT}"
@@
-echo $address > "$BOOTNODE_ENODE_OUT"
+echo "$address" > "$BOOTNODE_ENODE_OUT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| address="enode://$(bootnode -nodekey ${NODEKEY_FILE} -writeaddress)@${host}:${BOOTNODE_PORT}" | |
| echo "Starting the bootnode with address at $address" | |
| BOOTNODE_ENODE_OUT="${BOOTNODE_ENODE_OUT:-bootnode/bootnode.enode}" | |
| echo $address > "$BOOTNODE_ENODE_OUT" | |
| address="enode://$(bootnode -nodekey "${NODEKEY_FILE}" -writeaddress)@${host}:${BOOTNODE_PORT}" | |
| echo "Starting the bootnode with address at $address" | |
| BOOTNODE_ENODE_OUT="${BOOTNODE_ENODE_OUT:-bootnode/bootnode.enode}" | |
| echo "$address" > "$BOOTNODE_ENODE_OUT" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 67-67: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 70-70: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/start-bootnode.sh` around lines 67 - 70, Quote the shell variable
expansions in the bootnode startup script to satisfy SC2086: update the
`address=` assignment in `start-bootnode.sh` so `NODEKEY_FILE` is passed as a
quoted argument to `bootnode`, and change the `echo` write that uses `address`
so the variable is quoted when redirected to `BOOTNODE_ENODE_OUT`. Leave the
later `$params` usage unchanged, since it is intentionally unquoted for
splitting.
Source: Linters/SAST tools
gzliudan
approved these changes
Jun 28, 2026
benjamin202410
approved these changes
Jun 28, 2026
wanwiset25
approved these changes
Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) thatSummary by CodeRabbit
New Features
Bug Fixes
Chores