Implement bob-l2 #37
Implement bob-l2 #37
Conversation
There was a problem hiding this comment.
sry, my editor auto-lints on save, could be reverted
Disabling whitespace diff looks fine 🙂
There was a problem hiding this comment.
Yeah probably better to stick it in a separate formatting PR
There was a problem hiding this comment.
Pull Request Overview
This PR implements the bob-l2 image variant, which replaces lighthouse with op-node for L2 blockchain operations. The implementation includes system configuration, service setup, firewall rules, and build processes for running op-node instead of lighthouse.
- Adds bob-l2 configuration with op-node v1.13.7 integration
- Updates shared components to support both lighthouse and op-node commands
- Includes comprehensive firewall configuration for L2 operations
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| bob-l2/mkosi.postinst | Creates op-node user/group and enables systemd service |
| bob-l2/mkosi.extra/etc/systemd/system/op-node.service | Defines op-node systemd service configuration |
| bob-l2/mkosi.extra/etc/firewall-config | Sets up firewall rules for op-node and op-geth P2P communications |
| bob-l2/mkosi.build | Builds op-node binary from optimism repository |
| bob-l2/bob-l2.conf | Configures build dependencies and scripts for bob-l2 image |
| bob-l2.conf | Top-level configuration including bob-l2 components |
| bob-base/searchersh.c | Adds restart-op-node command and improves code formatting |
| bob-base/mkosi.extra/etc/sudoers.d/99-searcher | Grants sudo permissions for op-node restart command |
| Makefile | Updates mkosi command line arguments for consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| --l1 http://FIXME:8545 \ | ||
| --l1.beacon http://FIXME:3500 \ |
There was a problem hiding this comment.
The L1 endpoint URLs contain placeholder 'FIXME' values that need to be replaced with actual endpoints before deployment. Consider using environment variables or a configuration template mechanism.
Copilot uses AI. Check for mistakes.
| accept_dst_ip_port $CHAIN_ALWAYS_OUT tcp 1.2.3.4 8545 "op-node access to L1 archive EL RPC" | ||
| accept_dst_ip_port $CHAIN_ALWAYS_OUT tcp 1.2.3.4 3500 "op-node access to L1 archive CL RPC" |
There was a problem hiding this comment.
The IP address '1.2.3.4' is a placeholder that needs to be replaced with the actual L1 archive node IP. Consider using a configuration variable or environment substitution.
Copilot uses AI. Check for mistakes.
| StandardOutput=/persistent/cl_logs/op-node.log | ||
| StandardError=/persistent/cl_logs/op-node.err |
There was a problem hiding this comment.
Using file redirection for StandardOutput and StandardError is deprecated in systemd. Consider using 'journal' or configuring a proper logging service instead.
| StandardOutput=/persistent/cl_logs/op-node.log | |
| StandardError=/persistent/cl_logs/op-node.err | |
| StandardOutput=journal | |
| StandardError=journal |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Yeah probably better to stick it in a separate formatting PR
There was a problem hiding this comment.
How much of this is shared with the l1 image? I'm worried about us keeping two separate firewall files, so we should probably try to keep the specific firewall-config files as minimal as possible. We're probably going to rework a lot of this to not be ip-based soon anyway, or at least add in some sort of reverse-proxy outside the container to reduce the attack surface, so we can overfit here without worrying about adding debt
There was a problem hiding this comment.
lmao classic, idk why my vscode does this despite explicitly configuring it not to. we should include this as part of a separate formatting PR and I'll try harder to get my vscode to automatically remove lines with only whitespace and add trailing newlines.
| # to speed up development. | ||
| build-dev: check-perms setup ## Build module with development tools | ||
| @$(WRAPPER) mkosi --force --profile=devtools -I $(IMAGE).conf | ||
| @$(WRAPPER) mkosi --force --incremental=yes --profile=devtools --include=$(IMAGE).conf |
There was a problem hiding this comment.
| @$(WRAPPER) mkosi --force --incremental=yes --profile=devtools --include=$(IMAGE).conf | |
| @$(WRAPPER) mkosi --force --profile=devtools --include=$(IMAGE).conf |
Incremental is broken right now unfortunately. I'll figure out the exact failure case and open an issue on the mkosi repo
This adds a basic implementation of bob image with op-node running instead of lighthouse