feat: add dappnode pkg & update ci docker images tags [skip-line-limit]#1061
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR refactors GitHub Actions workflows to support dual-image CI/CD (ciphernode and support images), removes the EC2 deployment automation, and introduces a complete DAppNode package configuration including Docker setup, entrypoint script, and deployment metadata. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![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)
templates/default/client/index.html (1)
14-17: Consider extracting repeated description text to reduce maintenance burden.The same description text is duplicated across three meta tags (standard
description,og:description, andtwitter:description). While this is intentional for SEO/social purposes, the verbatim repetition creates a maintenance burden if the description needs updating in the future.If this template is processed at build time or runtime (e.g., via a template engine or framework), extracting this to a constant or variable would improve DRY adherence. Otherwise, this is an acceptable trade-off for static HTML.
Also applies to: 24-27, 34-37
.github/workflows/releases.yml (1)
79-79: Minor: Redundant local environment variable overrides.Lines 79 and 112 redefine
CIPHERNODE_IMAGE_NAMEandSUPPORT_IMAGE_NAMElocally, but these are already defined globally (lines 16–17). While harmless, the local definitions can be removed to keep the workflow DRY.Consider removing the local
env:blocks from both jobs and relying on the global definitions, unless there's a need to override them per job.Also applies to: 112-112
dappnode/entrypoint.sh (1)
78-79: Quote the variable expansion on line 79 for robustness and shell best practices.The
exec enclave start $CLI_ARGSline should quote the variable:exec enclave start "$CLI_ARGS"to prevent unintended word splitting if any arguments contain spaces or special characters. While the current controlled values likely don't contain spaces, quoting is more robust and idiomatic.Apply this diff:
# Start log "Starting: enclave start $CLI_ARGS" -exec enclave start $CLI_ARGS +exec enclave start "$CLI_ARGS"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dappnode/avatar-default.pngis excluded by!**/*.png
📒 Files selected for processing (14)
.github/workflows/ci.yml(2 hunks).github/workflows/ec2-deployment.yml(0 hunks).github/workflows/releases.yml(3 hunks)dappnode/.gitignore(1 hunks)dappnode/Dockerfile(1 hunks)dappnode/README.md(1 hunks)dappnode/config.template.yaml(1 hunks)dappnode/dappnode_package.json(1 hunks)dappnode/docker-compose.yml(1 hunks)dappnode/entrypoint.sh(1 hunks)dappnode/releases.json(1 hunks)dappnode/setup-wizard.yml(1 hunks)docs/public/site.webmanifest(1 hunks)templates/default/client/index.html(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/ec2-deployment.yml
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-29T01:03:50.414Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/config/src/app_config.rs:21-26
Timestamp: 2024-10-29T01:03:50.414Z
Learning: In `packages/ciphernode/config/src/app_config.rs`, the `rpc_url` field in the `ChainConfig` struct is not considered sensitive and does not need to be encrypted.
Applied to files:
dappnode/config.template.yaml
📚 Learning: 2024-10-23T01:59:27.215Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: tests/basic_integration/test.sh:21-21
Timestamp: 2024-10-23T01:59:27.215Z
Learning: In `tests/basic_integration/test.sh`, the hardcoded `CIPHERNODE_SECRET` is acceptable for testing purposes and does not need to be changed.
Applied to files:
dappnode/config.template.yaml.github/workflows/releases.yml
📚 Learning: 2024-10-23T02:03:02.008Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/encryption.rs:45-45
Timestamp: 2024-10-23T02:03:02.008Z
Learning: In the `packages/ciphernode/keyshare/src/encryption.rs` file, the environment variable `CIPHERNODE_SECRET` is used for the encryption password. A secure secret management solution is not currently available, but may be considered in future iterations.
Applied to files:
dappnode/config.template.yaml.github/workflows/releases.yml
📚 Learning: 2024-10-23T01:59:42.967Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:274-274
Timestamp: 2024-10-23T01:59:42.967Z
Learning: In the `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs` file and other test files within this project, hardcoding `CIPHERNODE_SECRET` is acceptable for testing purposes.
Applied to files:
.github/workflows/releases.yml
📚 Learning: 2025-10-10T12:56:40.538Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 830
File: templates/default/README.md:123-128
Timestamp: 2025-10-10T12:56:40.538Z
Learning: In the Enclave repository, the hard-coded Hardhat development private key `0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` is acceptable in template README files and documentation for local testing/interaction purposes.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-08-27T13:49:03.811Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: .github/workflows/ci.yml:202-206
Timestamp: 2025-08-27T13:49:03.811Z
Learning: In GitHub Actions workflows for Noir circuits, the user prefers to use "stable" toolchain with noir-lang/noirup action rather than pinning to specific versions like 1.0.0-beta.11, and prefers to update the noirup action version (e.g., to 0.1.4) to get tooling improvements.
Applied to files:
.github/workflows/ci.yml
🪛 actionlint (1.7.9)
.github/workflows/ci.yml
188-188: shellcheck reported issue in this script: SC2086:info:8:22: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: template_integration
- GitHub Check: test_enclave_init
- GitHub Check: crisp_e2e
- GitHub Check: test_net
- GitHub Check: build_ciphernode_image
- GitHub Check: integration_prebuild
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: crisp_rust_unit
🔇 Additional comments (23)
docs/public/site.webmanifest (1)
1-19: No issues identified — formatting changes only.All modifications are JSON indentation and whitespace reformatting with no semantic changes to the manifest structure or content.
dappnode/releases.json (1)
1-8: LGTM!Straightforward release manifest with correct IPFS hash format and RFC 2822 timestamp. Structure aligns with DAppNode SDK expectations.
dappnode/.gitignore (1)
1-33: LGTM!Comprehensive and well-organized gitignore covering DAppNode-specific patterns: secrets, build artifacts, SDK output, and OS/IDE files. No security or functional concerns.
.github/workflows/releases.yml (4)
14-20: Environment variables properly centralized.Global environment variables for Dockerfile paths and image names (lines 14–20) are appropriately defined here and can be referenced by all jobs.
74-105: Build-and-push job for ciphernode image is well-structured.The new
build-ciphernode-image-releasejob correctly:
- Builds from
crates/Dockerfile- Tags with both version and
latesttags- Uses BuildKit caching scoped to
ciphernode- Depends on the validation step to ensure version consistency
107-138: Build-and-push job for e3-support image follows same solid pattern.The
build-e3-support-releasejob mirrors the ciphernode job with correct adjustments:
- Context set to
./crates/supportfor the support image- Caching scoped appropriately to
e3-support- Both images now publish together on release tags (aligning with PR objectives)
290-298: Release job dependency ordering updated correctly.The
create-github-releasejob now properly depends on both image build jobs (build-ciphernode-image-releaseandbuild-e3-support-release), ensuring images are built and published before the release is created..github/workflows/ci.yml (4)
18-21: Environment variables properly structure dual-image CI/CD.New global environment variables cleanly separate Dockerfile paths and image names for ciphernode and e3-support, enabling both images to be built and tagged consistently in CI.
151-179: Support image build correctly implements conditional tagging and push.The
build_e3_support_risc0job now:
- Generates SHORT_SHA for all builds
- Tags with both SHORT_SHA and main (when on main branch)
- Pushes only when on main branch (not on PRs) ✓
- Uses BuildKit caching scoped to e3-support
This correctly implements the PR objective: build on all branches, push only on main.
181-219: New ciphernode image job adds complete dual-image support.The
build_ciphernode_imagejob follows the same pattern as the support image:
- Conditional login to GHCR (main branch only)
- Conditional push (main branch only)
- Proper SHORT_SHA tagging and main branch tag
- BuildKit caching scoped to ciphernode
This ensures PR builds compile and verify without pushing images, while main and release branches publish as intended.
188-188: Fix unquoted shell variables for safer word splitting handling.ActionLint reported SC2086 on line 188. While the current variables (CIPHERNODE_IMAGE_NAME and SHORT_SHA) are controlled values unlikely to contain spaces, shell best practice recommends quoting for robustness:
run: | SHORT_SHA=$(git rev-parse --short=9 HEAD) - TAGS="${CIPHERNODE_IMAGE_NAME}:${SHORT_SHA}" + TAGS="${CIPHERNODE_IMAGE_NAME}:${SHORT_SHA}" if [ "${GITHUB_REF}" = "refs/heads/main" ]; then - TAGS="$TAGS,${CIPHERNODE_IMAGE_NAME}:main" + TAGS="$TAGS,${CIPHERNODE_IMAGE_NAME}:main" fi echo "tags=$TAGS" >> $GITHUB_OUTPUT(Note: The quotes are already present in your code, so this may be a false positive from actionlint or a formatting artifact. If unquoted, add quotes as shown above.)
Also applies to: 152-152
dappnode/README.md (1)
1-240: Excellent comprehensive documentation for DAppNode package.The README is well-structured, clearly written, and covers all necessary aspects:
- Clear purpose and single-configuration design explanation
- Good file layout with ASCII tree
- Separate quick-start sections for users and developers
- Comprehensive configuration reference with examples
- Clear explanation of internal operation and data persistence
- Appropriate links to Enclave and DAppNode documentation
The documentation aligns well with the accompanying code files and provides users/developers with sufficient guidance to deploy and configure the package.
dappnode/docker-compose.yml (1)
1-46: Docker Compose configuration is well-structured and properly configured.The configuration correctly:
- Uses UPSTREAM_VERSION arg for flexibility in updating the ciphernode image
- Maps the QUIC P2P port (UDP 37173) as documented
- Mounts persistent storage at
/datafor database/state- Sets sensible defaults (sepolia network, ciphernode role, info log level)
- Leaves critical config empty (RPC_URL, contract addresses) for user to fill via setup wizard
- Implements security best practice with
no-new-privileges:true- Includes a healthcheck that verifies the enclave process is running
Environment variables align with README documentation.
dappnode/dappnode_package.json (1)
1-38: Package manifest is complete and properly structured.The DAppNode package manifest correctly defines:
- Package identity with semantic versioning (0.1.0) separate from upstream version (0.1.5)
- Upstream tracking via
upstreamVersionandupstreamArgfor future updates- Clear descriptions and appropriate categorization/keywords for discoverability
- License alignment with upstream (LGPL-3.0-only)
- Backup configuration for persistent data volume (matches docker-compose.yml)
- All links and repository information correct
- Minimum DAppNode version requirement (0.2.50) appropriately set
dappnode/config.template.yaml (1)
1-25: Configuration template properly structured for environment substitution.The YAML template:
- Uses correct ${VAR} syntax for environment variable substitution
- Properly formats numeric values (quic_port, deploy_block) without quotes
- Includes all required sections (node, chains, contracts) matching the README documentation
- Auto-generation flags (autonetkey, autopassword, autowallet) set appropriately
Environment variables align with those defined in docker-compose.yml and documented in the README.
Verify that
dappnode/entrypoint.shvalidates required environment variables (RPC_URL, contract addresses, etc.) before rendering this template withenvsubst, and that optional secrets/keys are handled correctly when empty.dappnode/Dockerfile (3)
8-26: LGTM!The runtime image setup follows security best practices: minimal Debian base, non-root user, justified dependencies, and explicit binary permissions. No concerns here.
28-30: LGTM!Permissions and paths are correct; template path aligns with entrypoint usage at
/opt/config.template.yaml.
32-43: LGTM!OCI labels are standard, user/workdir setup is correct, and entrypoint path is properly configured. The healthcheck is simplistic but acceptable; it verifies the enclave process is running via
pgrep. More sophisticated health probes (HTTP, custom checks) can be added if needed in future iterations.dappnode/entrypoint.sh (2)
1-25: LGTM!Error handling with
set -eis appropriate, RPC_URL validation is thorough (checks both presence and WebSocket scheme), and error messages are clear. The log function and banner provide good observability.
27-39: Verify that all variables referenced in config.template.yaml are exported before envsubst runs.Line 39 uses
envsubstto render the template, which will fail if any variable inconfig.template.yamlis not exported. The script exports defaults for NETWORK, QUIC_PORT, NODE_ROLE, NODE_ADDRESS, and LOG_LEVEL, but is missing exports for critical contract address variables: ENCLAVE_CONTRACT, ENCLAVE_DEPLOY_BLOCK, CIPHERNODE_REGISTRY_CONTRACT, CIPHERNODE_REGISTRY_DEPLOY_BLOCK, BONDING_REGISTRY_CONTRACT, BONDING_REGISTRY_DEPLOY_BLOCK, and RPC_URL. These must be provided via environment variables or the envsubst call will fail with undefined variable errors.⛔ Skipped due to learnings
Learnt from: ryardley Repo: gnosisguild/enclave PR: 184 File: packages/ciphernode/net/tests/entrypoint.sh:4-8 Timestamp: 2024-11-25T09:47:48.863Z Learning: When reviewing test scripts like `packages/ciphernode/net/tests/entrypoint.sh`, avoid suggesting additional error handling and cleanup for `iptables` commands, as it may not be necessary.Learnt from: ryardley Repo: gnosisguild/enclave PR: 156 File: tests/basic_integration/test.sh:21-21 Timestamp: 2024-10-23T01:59:27.215Z Learning: In `tests/basic_integration/test.sh`, the hardcoded `CIPHERNODE_SECRET` is acceptable for testing purposes and does not need to be changed.dappnode/setup-wizard.yml (3)
1-44: Approved with optional note on RPC_URL validation.The schema correctly defines NETWORK as an enum, NODE_ADDRESS with strict Ethereum address validation, and PRIVATE_KEY as optional. The RPC_URL pattern (
^wss?://.*) is loose and accepts any WebSocket URL, but this is acceptable since the entrypoint will validate connectivity at runtime. Consider adding tighter URL structure validation (e.g., domain/port checks) in a future iteration if desired.
72-137: Approved.The contract address and deploy block fields follow consistent validation patterns. Block number patterns accept any positive integer; stricter range validation (if needed) can be deferred to runtime configuration validation. All fields marked required seems appropriate if these contracts are essential for ciphernode operation on any network.
139-194: Approved.Optional fields are well-structured: PEERS are documented with examples (validation deferred to runtime), secrets are properly marked, LOG_LEVEL aligns with entrypoint mappings, and EXTRA_OPTS provides flexibility for advanced users. The lack of strict peer address validation is acceptable for now; tighter validation can be added if needed.
hmzakhalid
changed the title
ryardley
left a comment
ryardley
left a comment
There was a problem hiding this comment.
Don't see any glaring issues.
utACK 🚀
2 participants
Adds a DappNode package that pulls the Ciphernode image from upstream and uploads it to IPFS
The currently published DappNode package(
/ipfs/QmeX7jxDFcwbW7kAbs8Tgn5T4vonYxe4WmemUQsaca8xDQ) is based onmainand uploaded tohmzakhalid/enclave:latestbecause there was no upstream release of Ciphernode tagged0.1.5available at the time. So, this CI change was added to allow us to bump the version and publish the DappNode package. Since it is not yet published and signed, which is acceptable for testing purposes because the IPFS hash is available.Updated Tagging Policy
Currently, a Docker image is pushed to GHCR on every commit made to a PR, which is incorrect. The original intent was to build images only to verify they compile during PRs and push images only after merging to
main. The release CI now handles official tagging of Docker images.On PRs / development branches
On
mainBuild and push:
image:<short_sha>image:main-> latestmainbuildOn release tags
v0.x.yBuild and push:
image:0.x.y-> stable release versionimage:latest-> most recent stable releaseSummary by CodeRabbit
Release Notes
New Features
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.