feat: Add support for Midnight network #257
Conversation
* feat: Add network and address models * docs: Add reference in docstring for midnight address * feat: Add relayer, transaction and request draft models * fix: Tests * chore: Add Midnight network config * chore: Add Midnight transaction request and data field * test: Add address tests * chore: Create Midnight Transaction Builder (#267) * chore: Add types and scaffolding for transaction builder * chore: Use generic types for midnight_transaction * chore: Update builder and prover logic * chore: Update builder and prover logic * chore: Add typo * feat: Implement relayer model for Midnight (#321) * feat: Implement relayer model for Midnight * feat: Add wallet sync service (#335) * chore: Add indexer url to network config * feat: Add service sync manager * chore: Implement tx preparation scaffolding * chore: Implement get_balance from relayer service * chore: Replace repo w/ signertrait and clarify sync * feat: Implement relayer submission functionality (#337) * feat: Working implementation for transaction submission * feat: Use sync state repo to allow incremental sync * fix: Comments for token_type * feat: Implement status checker * chore: Upgrade package * feat: Implement signing for midnight * refactor: Replace dyn with EventHandlerType enum
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /// Helper function, hence private. | ||
| fn validate_url_scheme(url: &str) -> Result<()> { | ||
| if !url.starts_with("http://") && !url.starts_with("https://") { | ||
| let supported_protocols = ["http://", "https://", "wss://", "ws://"]; |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: javascript.lang.security.detect-insecure-websocket.detect-insecure-websocket
* test: Add unit tests * docs: Update script fixture readme * test: Add tests for config file
WalkthroughAdds first-class Midnight network support across config, models, providers, signer, relayer, transaction domain, a SyncManager with persistent sync-state (in-memory/Redis), fixtures/tools, docs/examples, many API/type signature changes to thread a SyncStateTrait (RSR) through app state and services. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as Relayer API
participant Factory as RelayerFactory
participant Repo as Repositories
participant Signer as MidnightSigner
participant Provider as MidnightProvider
participant Sync as SyncManager
Client->>API: Create relayer (network=midnight)
API->>Factory: create_relayer(state, model)
Factory->>Repo: load network config & signer model
Factory->>Provider: new(rpc_cfgs, indexer_urls, network_id, timeout)
Factory->>Signer: create_midnight_signer(model, network_id)
Factory->>Sync: new(indexer_client, wallet_seed, network_id, sync_state_store, relayer_id)
Factory-->>API: return NetworkRelayer::Midnight(...)
Note over Sync: SyncManager may restore context from sync_state_store
sequenceDiagram
autonumber
participant Sync as SyncManager
participant Indexer as MidnightIndexer (WS)
participant Strat as QuickSyncStrategy
participant Disp as EventDispatcher
participant Store as SyncStateStore
participant Ledger as LedgerContext
Sync->>Strat: sync(start_index?)
Strat->>Indexer: connect_wallet(viewing_key) / subscribe_wallet(start_index)
loop incoming events
Indexer-->>Strat: WalletSyncEvent
Strat->>Disp: dispatch(converted SyncEvent)
end
Disp-->>Sync: buffered chronological updates
Sync->>Ledger: apply merkle updates then transactions (ordered)
Sync->>Store: set_sync_state(last_index, serialized_context)
Sync-->>Caller: sync completed
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 46
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/models/relayer/rpc_config.rs (1)
74-85: Update docs and tests to reflect ws/wss acceptanceThe code now allows ws:// and wss://, but:
- The doc comment still says “HTTP or HTTPS” (Line 74).
- Tests still expect ws:// to be invalid (Lines 237-245).
Please align both with the new behavior.
Suggested test fix (rename + clarify failure cause is FTP, not WS):
- #[test] - fn test_validate_list_with_all_invalid_urls() { + #[test] + fn test_validate_list_with_mixed_urls_ws_is_valid() { let configs = vec![ - RpcConfig::new("ws://websocket.example.com".to_string()), - RpcConfig::new("ftp://invalid-scheme.com".to_string()), + RpcConfig::new("ws://websocket.example.com".to_string()), // valid now + RpcConfig::new("ftp://invalid-scheme.com".to_string()), // still invalid ]; let result = RpcConfig::validate_list(&configs); - assert!(result.is_err(), "Should fail with all invalid URLs"); + assert!(result.is_err(), "Should fail due to FTP URL; WS is valid now"); }Also update the doc comment to mention “HTTP, HTTPS, WS, WSS” to match the error message.
src/services/provider/retry.rs (2)
209-213: Guard against zero providers to avoid underflow and misleading logsIf provider_count() returns 0,
total_providers - 1underflows and panics. Fail fast with a clear error.Apply:
- let total_providers = selector.provider_count(); - let max_failovers = std::cmp::min(config.max_failovers as usize, total_providers - 1); + let total_providers = selector.provider_count(); + if total_providers == 0 { + let error_message = format!( + "RPC call '{}' failed: no providers configured or all providers unavailable", + operation_name + ); + log::error!("{}", error_message); + return Err(E::from(error_message)); + } + let max_failovers = + std::cmp::min(config.max_failovers as usize, total_providers.saturating_sub(1));
224-241: Don’t mark the last provider as failed on initialization errorsIn the provider init error path, the code marks the “current” provider as failed unconditionally (except when available count is 0). This violates the “last provider is protected” semantics you enforce elsewhere and can drop the last usable provider.
Gate marking on there being more than one available provider:
- Err(e) => { + Err(e) => { last_error = Some(e); failover_count += 1; - // If we've exhausted all providers or reached max failovers, stop - if failover_count > max_failovers || selector.available_provider_count() == 0 { + // If we've exhausted failovers or there is no alternative provider, stop + if failover_count > max_failovers || selector.available_provider_count() <= 1 { break; } // Mark current as failed to get a different one next time selector.mark_current_as_failed(); continue; }Cargo.toml (1)
8-11: Release builds are compiled without optimizations (opt-level=0). This will severely degrade performance.Release profile should use at least opt-level=3. Unless you have a specific reason to ship unoptimized binaries, switch to the standard optimized settings.
Apply this diff:
[profile.release] -opt-level = 0 +opt-level = 3 overflow-checks = false panic = "unwind"Optionally, for smaller/faster binaries you can also enable LTO and reduce codegen units:
[profile.release] opt-level = 3 +lto = "fat" +codegen-units = 1 overflow-checks = false panic = "unwind"src/config/config_file/mod.rs (1)
49-54: Add missing tests and documentation for the new Midnight variantAll existing
matcharms onConfigFileNetworkTypealready includeMidnight, but we still need to:
src/config/config_file/mod.rs
- (Lines 6–8) Update the module-level docs to mention
Midnightalongside EVM, Solana, and Stellar.- In
test_config_file_network_type_display(around line 1423), add an assertion forConfigFileNetworkType::Midnight(e.g.assert_eq!(format!("{:?}", ConfigFileNetworkType::Midnight), "Midnight");).src/models/relayer/config.rs
- Add a test that deserializes JSON with
"network_type": "midnight"into aRelayerFileConfigand assertsconfig.network_type == ConfigFileNetworkType::Midnight.These additions will ensure the new variant is fully documented and covered by tests.
src/services/plugins/socket.rs (2)
211-212: Ignore-on-write errors can hide client disconnects; break the loop on write failureSwallowing write errors can loop indefinitely while the client is gone. Propagate or break to close the connection cleanly.
Apply this diff:
- let _ = w.write_all(response_str.as_bytes()).await; + if let Err(e) = w.write_all(response_str.as_bytes()).await { + return Err(PluginError::SocketError(format!("Failed to write response: {}", e))); + }
229-234: Tests: missing AsyncWriteExt import causes method resolution failures for write_all/shutdownInside the test module, AsyncWriteExt isn’t imported, yet methods write_all and shutdown are used. Bring the trait into scope to avoid compilation errors.
Apply this diff:
- use tokio::{ - io::{AsyncBufReadExt, BufReader}, - time::timeout, - }; + use tokio::{ + io::{AsyncBufReadExt, AsyncWriteExt, BufReader}, + time::timeout, + };Also applies to: 311-313, 339-341
♻️ Duplicate comments (1)
src/models/relayer/rpc_config.rs (1)
77-81: Allowing ws:// increases risk; prefer wss:// in productionPermitting ws:// is reasonable for local/dev, but it’s insecure over public networks. Ensure downstream validation or environment policy enforces wss:// in production or add a config flag to disallow ws:// outside dev.
This echoes a prior security scan warning about insecure websockets.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Well this all looks like easy implementation ;).
Great job! Handling all complexities with new network and shaping it to match relayer format is just incredible.
I left few comments, nothing major, mostly nits.
I think we should improve documentation (full docker example + specific docs page). We can do it additional PR due to size of this one if easier.
Also, many CI checks are failing so I guess this is not yet ready for merging due to issues with packages.
Thank you for amazing work!
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (4)
src/services/signer/midnight/mod.rs (2)
20-24: Security: Avoid exposing wallet_seed through public APIReturning a reference to the raw wallet seed enables accidental logging/exfiltration. This is a critical security concern for key material.
26-29: Remove deprecated VaultCloud variantPer the previous review comment, VaultCloud support should be removed as the service is deprecated.
Apply this change:
pub enum MidnightSigner { Local(LocalSigner), Vault(LocalSigner), - VaultCloud(LocalSigner), }Also update the pattern matching in the trait implementations:
impl MidnightSignerTrait for MidnightSigner { fn wallet_seed(&self) -> &midnight_node_ledger_helpers::WalletSeed { match self { - Self::Local(s) | Self::Vault(s) | Self::VaultCloud(s) => s.wallet_seed(), + Self::Local(s) | Self::Vault(s) => s.wallet_seed(), } } }src/config/config_file/network/midnight.rs (1)
37-50: Strengthen URL validation to enforce expected schemesCurrently, any valid URL scheme passes. Enforce http/https for indexer.http and ws/wss for indexer.ws to fail early on misconfiguration.
src/domain/relayer/midnight/midnight_relayer.rs (1)
189-198: Don't swallow repository errors when resolving the Midnight network
.ok().flatten()converts any repository error into "Network not found", masking real failures (I/O, deserialization, etc.).
🧹 Nitpick comments (31)
examples/midnight-basic-example/.gitignore (1)
1-5: Good baseline for keeping secrets out of VCS; consider keeping the keys directory tracked and ignoring typical local-only env variants.Current rules will ignore the entire keys dir, which also prevents committing a placeholder to keep the directory structure. If the example expects the folder to exist, prefer ignoring contents but allowing a
.gitkeep(or README) to be tracked. Also, ignoring common local-only env files helps avoid accidental commits while keeping.env.exampletracked.Apply this diff:
# Environment file with secrets .env +.env.local +.env.*.local # Keys directory -config/keys/ +config/keys/* +!config/keys/.gitkeepFollow-up:
- Add an empty file at
examples/midnight-basic-example/config/keys/.gitkeepso the directory structure is preserved without committing real keys.- If the directory is not needed at runtime until generated, feel free to ignore this suggestion.
examples/midnight-basic-example/.env.example (2)
1-5: Tighten formatting for dotenv, add quoting, and satisfy dotenv-linter ordering.
- Quote values to safely accommodate spaces/special characters.
- Reorder
KEYSTORE_PASSPHRASEbeforeWEBHOOK_SIGNING_KEYto appease dotenv-linter.- Add brief hints to reduce setup friction.
Apply this diff:
-# Required configuration -API_KEY= -WEBHOOK_SIGNING_KEY= -KEYSTORE_PASSPHRASE= +# Required configuration +# Tip: keep quotes if your values contain spaces or special characters. +API_KEY="" +KEYSTORE_PASSPHRASE="" +# If a specific encoding is required for the signing key (e.g., hex/base64), +# document it here to avoid integration issues. +WEBHOOK_SIGNING_KEY=""Follow-up:
- Please confirm the expected encoding/format for
WEBHOOK_SIGNING_KEYand any constraints forKEYSTORE_PASSPHRASE(allowed charset, min length). I can update the template comments accordingly to prevent misconfiguration.
6-12: Scope-and-cleanup guidance for temporary GitHub credentials; also fix dotenv-linter ordering.
- Move
GITHUB_PATbeforeGITHUB_USERto satisfy the linter.- Add scope guidance and an explicit reminder not to commit real secrets.
- Cross-reference PLAT-6814 for removal once repos are public.
Apply this diff:
# ============================================================================ # TEMPORARY: GitHub credentials for accessing private Midnight repositories # These are only required until Midnight repositories are open-sourced # Once the repos become public, these credentials can be removed entirely # ============================================================================ -GITHUB_USER= -GITHUB_PAT= +# Do NOT commit real secrets. Use least-privilege, read-only access. +# Suggested scopes: "repo" (read-only) or a fine-grained token with read +# access limited to the specific Midnight repositories. +GITHUB_PAT="" +GITHUB_USER=""Optional:
- If your tooling allows, consider supporting both
GITHUB_PATandGITHUB_TOKENto align with common conventions, while keepingGITHUB_PATin this example for backward compatibility.- Add a TODO here referencing PLAT-6814 to ensure removal when the blocking item is resolved.
Dockerfile.midnight.development (1)
69-69: Container hardening: Consider keeping apk for security updatesRemoving
apk-toolsprevents future security patches. For production images, consider keeping minimal package management capability.Consider keeping apk-tools or document the security update strategy for this container.
src/domain/relayer/midnight/midnight_relayer.rs (2)
376-382: Document unsupported operation clearlyPer previous review comment, deleting transactions is not supported on Midnight. The error message should be more informative.
Err(RelayerError::NotSupported( - "Deleting transactions is not supported for Midnight".to_string(), + "Deleting pending transactions is not supported for Midnight network".to_string(), ))
399-406: Document RPC limitationPer previous review comment, RPC might not be supported. Add clarification.
Err(RelayerError::NotSupported( - "RPC is not supported for Midnight".to_string(), + "Direct RPC calls are not supported for Midnight network".to_string(), ))examples/midnight-basic-example/README.md (14)
70-81: Correct step heading and anchor (Step 2).Heading “Step 2: Create a Signer” is fine, but it must align with the corrected TOC anchors.
No diff needed here once the TOC is fixed.
108-135: Rename “Step 5: Configure Notifications” to Step 4.To keep numbering consistent after the previous fix.
-### Step 5: Configure Notifications +### Step 4: Configure Notifications
135-153: Rename “Step 6: Configure API Key” to Step 5 and use placeholders instead of realistic UUIDs to avoid secret scanners.The realistic-looking UUIDs are triggering gitleaks (even though they’re examples). Use angle-bracket placeholders.
-### Step 6: Configure API Key +### Step 5: Configure API Key-API_KEY=a1b2c3d4-e5f6-7890-abcd-ef1234567890 -WEBHOOK_SIGNING_KEY=f9e8d7c6-b5a4-3210-fedc-ba0987654321 -KEYSTORE_PASSPHRASE=YourSecurePassword123! +API_KEY=<YOUR_API_KEY_UUID> +WEBHOOK_SIGNING_KEY=<YOUR_WEBHOOK_SIGNING_KEY_UUID> +KEYSTORE_PASSPHRASE=<YOUR_SECURE_PASSWORD>
153-160: Rename “Step 7: Run the Service” to Step 6.Follow-on from step renumbering.
-### Step 7: Run the Service +### Step 6: Run the Service
167-168: Call out temporary dependency on MIDNIGHT_LEDGER_TEST_STATIC_DIR (merge blocker PLAT-6814).Docs should match the blocking item that requires removing this env var dependency before merge.
-**Note**: The `MIDNIGHT_LEDGER_TEST_STATIC_DIR` environment variable is automatically set to `/tmp/midnight-test-static` as required by the Midnight test environment. +**Note (Temporary)**: The `MIDNIGHT_LEDGER_TEST_STATIC_DIR` environment variable is set to `/tmp/midnight-test-static` for the current Midnight test environment. Per PLAT-6814, this dependency will be removed before merging; this example will be updated accordingly.Would you like me to open a follow-up issue to track the doc change tied to PLAT-6814?
175-187: Rename “Step 8: Verify the Setup” to Step 7 and fix Authorization header casing.HTTP headers are case-insensitive, but examples should use conventional “Authorization”.
-### Step 8: Verify the Setup +### Step 7: Verify the Setup- -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq + -H "Authorization: Bearer $API_KEY" | jq
181-187: Avoid inlining secrets and prefer environment substitution in curl commands.This reduces accidental leaks and satisfies gitleaks.
-curl -X GET http://localhost:8080/api/v1/relayers \ - -H "Content-Type: application/json" \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq +curl -X GET http://localhost:8080/api/v1/relayers \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer $API_KEY" | jq
213-215: Same header-casing fix for balance endpoint.- -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq + -H "Authorization: Bearer $API_KEY" | jq
222-246: Security note: seeds in request body are sensitive; add a warning and prepare for API change.Examples include origin/destination wallet seeds. That’s acceptable for testnet, but we should add a prominent warning and note that destination seed usage is slated for removal per PLAT-6814.
-H "AUTHORIZATION: Bearer YOUR_API_KEY" \ -d '{ @@ }' | jq + +> Warning: Never use real/production wallet seeds in examples or logs. The origin and destination seeds here are for testnet-only workflows. The API is expected to evolve to remove destination wallet seed usage (see PLAT-6814); this example will be updated accordingly.Would you like me to prepare an updated example that uses non-seed identifiers as soon as the LedgerContext change lands?
253-255: Fix header casing for transaction status.- -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq + -H "Authorization: Bearer $API_KEY" | jq
260-262: Fix header casing for list transactions.- -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq + -H "Authorization: Bearer $API_KEY" | jq
279-279: Add a language hint to the fenced code block (MD040).-``` +```text
369-401: JSON comments are invalid; switch to AsciiDoc or quote values.The “// Minimum balance in speck” comment will break JSON parsing if copy-pasted.
- "min_balance": 1000000000 // Minimum balance in speck + "min_balance": 1000000000Add the explanatory text outside the code block or as an AsciiDoc callout below the snippet.
433-437: Ensure stable, canonical external URLsPlease update the external links in examples/midnight-basic-example/README.md (lines 433–437) as follows:
Replace the release-preview API docs URL
- Review the [API documentation](https://release-v1-0-0--openzeppelin-relayer.netlify.app/api_docs.html) + Review the [API documentation](https://docs.openzeppelin.com/relayer/1.1.x/api_reference)The canonical reference lives on the OpenZeppelin docs site at docs.openzeppelin.com/relayer//api_reference (current version 1.1.x) (docs.openzeppelin.com)
Verify that the Midnight links still resolve correctly:
• https://midnight.network/docs
• https://docs.midnight.network/develop/tutorial/introduction/Telegram link nit (optional): consider linking to the base channel (
https://t.me/openzeppelin_tg) unless the/2deep‐link to a specific message is intentional.examples/midnight-basic-example/docker-compose.yaml (8)
16-25: Healthcheck relies on curl; verify it exists in the prover image or use CMD-SHELL.If the image lacks curl, the healthcheck will always fail.
- healthcheck: - test: - - CMD - - curl - - -f - - http://localhost:6300/health + healthcheck: + test: ["CMD-SHELL", "curl -fsS http://localhost:6300/health >/dev/null 2>&1 || exit 1"] interval: 30s timeout: 10s retries: 3 start_period: 40sIf curl is not available, switch to wget similarly or add a tiny sidecar for health probing.
32-37: Build secrets: make BuildKit usage explicit and avoid leaking GH creds.Compose build secrets work best when declared with ids; ensure BuildKit is enabled (DOCKER_BUILDKIT=1).
build: context: ../../ dockerfile: Dockerfile.midnight.development - # TEMPORARY: GitHub secrets are only required until Midnight repositories are open-sourced - # Once the repos are public, these secrets can be removed - secrets: - - github_user - - github_pat + # TEMPORARY: GitHub secrets required until Midnight repos are open-sourced + secrets: + - id: github_user + env: GITHUB_USER + - id: github_pat + env: GITHUB_PATConfirm the Dockerfile uses
RUN --mount=type=secret,id=github_userandid=github_pat. If not, I can provide a patch.
43-51: Avoid duplicating secrets as environment variables; prefer runtime secrets.You’re passing WEBHOOK_SIGNING_KEY, API_KEY, and KEYSTORE_PASSPHRASE via envs (and also declaring them as secrets). Env vars are easier to leak via logs. Prefer reading from
/run/secrets/*.- WEBHOOK_SIGNING_KEY: ${WEBHOOK_SIGNING_KEY} - API_KEY: ${API_KEY} - KEYSTORE_PASSPHRASE: ${KEYSTORE_PASSPHRASE} + # Prefer reading from /run/secrets/{webhook_signing_key,api_key,keystore_passphrase} in the appIf the app requires envs today, keep as-is for the example, but consider a follow-up to load from files.
57-58: Mount config volumes read-only to reduce risk.- - ./config:/app/config/ - - ../../config/networks:/app/config/networks + - ./config:/app/config/:ro + - ../../config/networks:/app/config/networks:ro
62-63: Add a relayer healthcheck for better readiness detection.This improves local DX and avoids “connection refused” in first requests.
restart: on-failure:5 + healthcheck: + test: ["CMD-SHELL", "curl -fsS http://localhost:8080/health >/dev/null 2>&1 || exit 1"] + interval: 15s + timeout: 5s + retries: 5 + start_period: 30s
65-83: Redis: good defaults; consider adding an explicit healthcheck.It’s optional, but helps compose orchestrations.
restart: on-failure:5 + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 10s
90-104: Compose “secrets:” with environment sources may not be portable; prefer file-based secrets.Not all Compose implementations support
secrets: { environment: VAR }. File-backed secrets are the most portable.-secrets: - api_key: - environment: API_KEY - webhook_signing_key: - environment: WEBHOOK_SIGNING_KEY - keystore_passphrase: - environment: KEYSTORE_PASSPHRASE - # TEMPORARY: GitHub credentials are only required until Midnight repositories are open-sourced - # Once the repos are public, github_user and github_pat can be removed from secrets - github_user: - environment: GITHUB_USER - github_pat: - environment: GITHUB_PAT +secrets: + api_key: + file: ./secrets/api_key + webhook_signing_key: + file: ./secrets/webhook_signing_key + keystore_passphrase: + file: ./secrets/keystore_passphrase + # TEMPORARY: remove once the repos are public + github_user: + file: ./secrets/github_user + github_pat: + file: ./secrets/github_patOptionally add a helper script to materialize these files from your
.envduring local dev.
10-11: Temporary env var: MIDNIGHT_LEDGER_TEST_STATIC_DIR (merge blocker).This example should label this dependency as temporary to mirror PLAT-6814.
Add a comment above this line:
# TEMPORARY: Will be removed per PLAT-6814 before mergedocs/modules/ROOT/pages/midnight.adoc (3)
106-111: Fix docker run syntax for prover server.Using
--before the command is unusual unless the image has a custom entrypoint expecting args. Prefer the simpler, portable form.-docker run -p 6300:6300 midnightnetwork/proof-server -- 'midnight-proof-server --network testnet' +docker run --rm -p 6300:6300 midnightnetwork/proof-server midnight-proof-server --network testnetIf the image requires a different ENTRYPOINT/CMD, adjust accordingly.
199-225: Good example; keep secrets placeholder style.Using
<api_key>and seed placeholders is correct. Consider adding a short warning about test-only seeds (as in README).You can add:
[WARNING] Never use real wallet seeds outside test environments.
306-306: docs/modules/ROOT/pages/midnight.adoc: update API Reference link to canonical docsLines: 306
See link:https://release-v1-0-0%2D%2Dopenzeppelin-relayer.netlify.app/api_docs.html[API Reference^] for full details and examples.The current URL points at the v1.0.0 Netlify preview build. To ensure the link remains stable and always reflects the published API docs, please update this to either:
Link to the canonical site:
link:https://openzeppelin-relayer.netlify.app/api_docs.html[API Reference^]Or use an Antora cross-reference to the local API docs page:
xref:api_docs.adoc[API Reference^]This change will keep the link accurate across versions and environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
Dockerfile.midnight.development(1 hunks)docs/modules/ROOT/nav.adoc(1 hunks)docs/modules/ROOT/pages/midnight.adoc(1 hunks)examples/midnight-basic-example/.env.example(1 hunks)examples/midnight-basic-example/.gitignore(1 hunks)examples/midnight-basic-example/README.md(1 hunks)examples/midnight-basic-example/config/config.json(1 hunks)examples/midnight-basic-example/docker-compose.yaml(1 hunks)src/config/config_file/network/midnight.rs(1 hunks)src/domain/relayer/midnight/midnight_relayer.rs(1 hunks)src/services/signer/midnight/mod.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/modules/ROOT/nav.adoc
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T14:31:09.686Z
Learnt from: LuisUrrutia
PR: OpenZeppelin/openzeppelin-relayer#427
File: src/models/network/evm/network.rs:1-1
Timestamp: 2025-08-19T14:31:09.686Z
Learning: In the OpenZeppelin relayer codebase, test helper functions may use network names (like "optimism") as string literals for creating mock networks, which are different from network tag constants (like OPTIMISM_BASED_TAG). These should not be flagged as inconsistencies when reviewing tag-related changes.
Applied to files:
src/config/config_file/network/midnight.rs
🧬 Code graph analysis (3)
src/services/signer/midnight/mod.rs (2)
src/services/signer/midnight/local_signer.rs (3)
wallet_seed(79-81)address(86-94)sign_transaction(96-128)src/services/signer/mod.rs (4)
address(72-72)address(92-99)sign_transaction(75-78)sign_transaction(101-111)
src/config/config_file/network/midnight.rs (4)
src/models/network/repository.rs (3)
common(26-33)common(153-155)config(158-160)src/config/config_file/network/collection.rs (2)
flatten(140-149)validate(206-211)src/config/config_file/network/common.rs (2)
validate(41-73)merge_with_parent(82-95)src/config/config_file/network/mod.rs (2)
validate(68-75)is_testnet(108-115)
src/domain/relayer/midnight/midnight_relayer.rs (7)
src/domain/transaction/stellar/utils.rs (1)
next_sequence_u64(19-25)src/models/notification/webhook_notification.rs (1)
produce_relayer_disabled_payload(83-100)src/services/sync/midnight/handler/manager.rs (7)
sync(203-326)sync(342-344)sync(392-412)new(141-196)new(384-390)get_context(328-330)get_context(346-348)src/services/sync/midnight/handler/mod.rs (2)
sync(21-21)get_context(24-24)src/models/transaction/repository.rs (8)
try_from(825-956)try_from(989-1011)try_from(1017-1041)try_from(1047-1062)try_from(1068-1070)try_from(1076-1093)try_from(1099-1101)from(1105-1112)src/services/signer/midnight/mod.rs (2)
wallet_seed(23-23)wallet_seed(50-54)src/services/provider/midnight/mod.rs (2)
get_balance(67-71)get_balance(254-271)
🪛 dotenv-linter (3.3.0)
examples/midnight-basic-example/.env.example
[warning] 4-4: [UnorderedKey] The KEYSTORE_PASSPHRASE key should go before the WEBHOOK_SIGNING_KEY key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The GITHUB_PAT key should go before the GITHUB_USER key
(UnorderedKey)
🪛 LanguageTool
examples/midnight-basic-example/README.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...data. ## Table of Contents - Overview - Prerequisites - [Gettin...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...- Overview - Prerequisites - Getting Started - [...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ites](#prerequisites) - Getting Started - [Step 1: Clone the Repository](#step-1-cl...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...arted) - Step 1: Clone the Repository - [Step 2: Fetch Midnight Dependencies](#st...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... - Step 2: Fetch Midnight Dependencies - [Step 3: Create a Signer](#step-3-create-...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...-temporary) - Step 3: Create a Signer - [Step 4: Configure Environment](#step-4-c...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...gner) - Step 4: Configure Environment - [Step 5: Configure Notifications](#step-5...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...nt) - Step 5: Configure Notifications - [Step 6: Configure API Key](#step-6-confi...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...ications) - Step 6: Configure API Key - [Step 7: Run the Service](#step-7-run-the...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...re-api-key) - Step 7: Run the Service - [Step 8: Verify the Setup](#step-8-verify...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...e-service) - Step 8: Verify the Setup - [Testing the Relayer](#testing-the-relaye...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...verify-the-setup) - Testing the Relayer - API Examples - [Architec...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...r](#testing-the-relayer) - API Examples - Architecture - [Troubles...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...Examples](#api-examples) - Architecture - Troubleshooting - [Ad...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...cture](#architecture) - Troubleshooting - [Advanced Configuration](#advanced-config...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... - Setting up a Midnight testnet relayer - Configuring the prover service for zero-...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...vice for zero-knowledge proof generation - Submitting privacy-preserving transactio...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...bmitting privacy-preserving transactions - Monitoring transaction status ### Key F...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...om/get-docker/) (version 20.10 or later) - [Docker Compose](https://docs.docker.com/...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...compose/install/) (version 2.0 or later) - [Rust](https://www.rust-lang.org/tools/in...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...1.86 or later, for key generation tools) - jq (opt...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ....io/jq/) (optional, for JSON formatting) - Access to Midnight testnet (wallet seeds...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...night testnet (wallet seeds for testing) - [TEMPORARY] GitHub credentials with ac...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...private. To build the relayer, you need: > - A GitHub username with access to the Mid...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...with access to the Midnight repositories > - A GitHub Personal Access Token (PAT) or ...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...erates zero-knowledge proofs (port 6300) - OpenZeppelin Relayer: Manages transact...
(QB_NEW_EN)
[grammar] ~164-~164: There might be a mistake here.
Context: ...ayer**: Manages transactions (port 8080) - Redis: Stores relayer state (port 6379...
(QB_NEW_EN)
[grammar] ~268-~268: There might be a mistake here.
Context: ...ng: - Complex transactions with intents - Fallible offers - Transaction filtering ...
(QB_NEW_EN)
[grammar] ~269-~269: There might be a mistake here.
Context: ...nsactions with intents - Fallible offers - Transaction filtering - Pagination See ...
(QB_NEW_EN)
[grammar] ~270-~270: There might be a mistake here.
Context: ... Fallible offers - Transaction filtering - Pagination See [midnight-api-examples.m...
(QB_NEW_EN)
[grammar] ~407-~407: There might be a mistake here.
Context: ...e TTL**: Cache duration for Merkle roots 2. Rate Limiting: Adjust `RATE_LIMIT_REQU...
(QB_NEW_EN)
[grammar] ~408-~408: There might be a mistake here.
Context: ...UESTS_PER_SECOND` in docker-compose.yaml 3. Redis Persistence: Configure save inte...
(QB_NEW_EN)
[grammar] ~426-~426: There might be a mistake here.
Context: ... for keystores 3. Network Security: - Restrict network access to necessary por...
(QB_NEW_EN)
🪛 Gitleaks (8.27.2)
examples/midnight-basic-example/README.md
181-186: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
213-214: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
222-224: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
253-254: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
260-261: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
149-149: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
examples/midnight-basic-example/README.md
11-11: Link fragments should be valid
(MD051, link-fragments)
12-12: Link fragments should be valid
(MD051, link-fragments)
279-279: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (5)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: semgrep/ci
- GitHub Check: Pages changed - openzeppelin-relayer
🔇 Additional comments (9)
src/services/signer/midnight/mod.rs (1)
69-71: Inconsistent signer implementation for Vault variantThe Vault variant uses
LocalSignerinternally, which seems incorrect. If Vault support isn't implemented yet, return an error instead of silently using local signing.SignerConfig::Vault(_) => { - return Err(SignerFactoryError::UnsupportedType("Vault".into())) + return Err(SignerFactoryError::UnsupportedType("Vault (not yet implemented for Midnight)".into())) }Likely an incorrect or invalid review comment.
src/domain/relayer/midnight/midnight_relayer.rs (2)
429-429: Ignore theSome(0)suggestion –.sync(0)is correctThe
syncmethod’s signature is:async fn sync(&mut self, start_index: u64) -> Result<(), SyncError>;It takes a plain
u64, not anOption<u64>. Passing0properly indicates “from genesis,” so no change is needed here.Likely an incorrect or invalid review comment.
413-419: Replace println with proper loggingUsing
println!bypasses logging configuration. Use the logging macros instead.The code already uses
info!on lines 413 and 217-218, but the comment on lines 415-419 should use proper logging:- // First sync the wallet from genesis on initial setup info!( - "Performing initial wallet sync from genesis for relayer: {}", + "First sync the wallet from genesis on initial setup. Performing initial wallet sync from genesis for relayer: {}", self.relayer.id );Likely an incorrect or invalid review comment.
examples/midnight-basic-example/README.md (1)
314-322: Troubleshooting commands: consider adding service names consistently.All three commands include the compose file; the restart also should include the service name “prover” (it does). LGTM.
examples/midnight-basic-example/docker-compose.yaml (1)
84-89: Networks: good separation with internal metrics network.LGTM.
docs/modules/ROOT/pages/midnight.adoc (4)
9-9: Clearly mark API as “subject to change”: already present.Good proactive note given ongoing development.
389-404: Troubleshooting: solid actionable checks.LGTM.
1-4: Page title and description: clear and concise.LGTM.
139-144: min_balance default and unit confirmedThe documentation’s default of
0with unit “speck” accurately reflects the code:
- The smallest‐unit name is defined as
"speck"insrc/constants/relayer.rs(line 32) and used throughout the relayer implementation.- The decimal precision is set to
6(i.e. 1 base unit = 10⁶ specks) insrc/constants/token.rs(line 6).- There is no
DEFAULT_MIDNIGHT_MIN_BALANCEconstant in the codebase, so an implicit default of0specks is correct.No changes are needed in the docs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/models/transaction/response.rs (1)
103-119: MidnightTransactionResponse parity achieved (status_reason included) — thanksThis restores API symmetry with EVM/Solana/Stellar by carrying status_reason for Midnight responses. Looks correct and consistent with surrounding types and OpenAPI annotations.
🧹 Nitpick comments (2)
src/models/transaction/response.rs (2)
173-185: Field mapping for Midnight is correct; add targeted tests for coverageThe mapping faithfully forwards id, hash, pallet_hash, block_hash, status, status_reason, and timestamps. Good.
To prevent regressions, add unit tests mirroring the EVM/Solana/Stellar cases:
- From → TransactionResponse::Midnight mapping (assert each field, especially status_reason propagation).
- Optional: serde round-trip of TransactionResponse::Midnight to ensure the JSON shape is stable.
Example tests you can adapt (place inside this file’s tests module). Adjust MidnightTransactionData fields to match its definition:
#[test] fn test_from_transaction_repo_model_midnight() { use crate::models::{NetworkType, TransactionRepoModel, MidnightTransactionData}; let now = chrono::Utc::now().to_rfc3339(); let model = TransactionRepoModel { id: "tx_mid_1".to_string(), status: TransactionStatus::Pending, status_reason: Some("queued".to_string()), created_at: now.clone(), sent_at: None, confirmed_at: None, relayer_id: "relayer-mid-1".to_string(), priced_at: None, hashes: vec![], network_data: NetworkTransactionData::Midnight(MidnightTransactionData { hash: Some("0xmid_tx".to_string()), pallet_hash: None, block_hash: None, // fill any additional required fields here }), valid_until: None, network_type: NetworkType::Midnight, noop_count: None, is_canceled: Some(false), delete_at: None, }; let response = TransactionResponse::from(model.clone()); match response { TransactionResponse::Midnight(mid) => { assert_eq!(mid.id, model.id); assert_eq!(mid.hash, Some("0xmid_tx".to_string())); assert_eq!(mid.pallet_hash, None); assert_eq!(mid.block_hash, None); assert_eq!(mid.status, TransactionStatus::Pending); assert_eq!(mid.status_reason, Some("queued".to_string())); assert_eq!(mid.created_at, now); assert_eq!(mid.sent_at, None); assert_eq!(mid.confirmed_at, None); } _ => panic!("Expected MidnightTransactionResponse"), } } #[test] fn test_serde_roundtrip_midnight_response() { let now = chrono::Utc::now().to_rfc3339(); let resp = TransactionResponse::Midnight(Box::new(MidnightTransactionResponse { id: "tx_mid_2".into(), hash: Some("0xabc".into()), pallet_hash: Some("0xpal".into()), block_hash: None, status: TransactionStatus::Confirmed, status_reason: None, created_at: now.clone(), sent_at: Some(now.clone()), confirmed_at: Some(now.clone()), })); let json = serde_json::to_string(&resp).unwrap(); let de: TransactionResponse = serde_json::from_str(&json).unwrap(); match de { TransactionResponse::Midnight(m) => { assert_eq!(m.id, "tx_mid_2"); assert_eq!(m.hash, Some("0xabc".into())); assert_eq!(m.pallet_hash, Some("0xpal".into())); assert_eq!(m.block_hash, None); assert_eq!(m.status, TransactionStatus::Confirmed); } _ => panic!("Expected Midnight variant after round-trip"), } }If you’d like, I can open a follow-up PR to add these tests after we confirm MidnightTransactionData’s exact shape.
17-18: Document untagged enum deserialization behaviorSince the codebase only ever serializes
TransactionResponseto JSON (and does not callserde_json::from_*<TransactionResponse>anywhere), the shape-based matching risk of an untagged enum applies only when clients deserialize our API payloads, not on the server.Optional recommendations:
- In src/models/transaction/response.rs, add a doc-comment above
#[serde(untagged)] pub enum TransactionResponseexplaining that variants are matched by field shape and that theMidnightvariant’s mostly-optional fields could overlap with other shapes.- Consider annotating
MidnightTransactionResponsewith#[serde(deny_unknown_fields)]to prevent stray fields from slipping through (trade-off: stricter forward compatibility).- For a future major version, switch to a tagged enum (e.g.
#[serde(tag = "type")]) so consumers can unambiguously pick the correct variant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/models/transaction/response.rs(3 hunks)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
Dockerfile.midnight.development (4)
44-45: Cache target path corrected—nice catchThe target cache now matches WORKDIR (/usr/app/target), so Cargo build caching will be effective.
20-38: Avoid persisting GitHub credentials; fail-fast and don’t log identifiersGood move to use BuildKit secrets. However, this step still:
- Writes a global git config with embedded token in one layer, then clears it in a later layer (the earlier layer still contains the secreted config in its history).
- Echoes the GitHub username to logs.
- Doesn’t fail the build when access to the private repo fails.
- Sends error messages to stdout instead of stderr.
Use an ephemeral config (no writes to disk) and make the access check fail-fast. Example patch within this RUN block:
-RUN --mount=type=secret,id=github_user,mode=0400 \ - --mount=type=secret,id=github_pat,mode=0400 \ - if [ ! -f /run/secrets/github_user ] || [ ! -f /run/secrets/github_pat ]; then \ - echo "ERROR: GitHub credentials must be provided as Docker secrets" && \ - echo "Please ensure GITHUB_USER and GITHUB_PAT are set in your .env file" && \ - echo "Note: This is temporary until Midnight repositories are open-sourced" && \ - exit 1; \ - fi && \ - GITHUB_USER=$(cat /run/secrets/github_user) && \ - GITHUB_PAT=$(cat /run/secrets/github_pat) && \ - echo "Configuring git for user: ${GITHUB_USER}" && \ - echo "Note: GitHub authentication is temporary until Midnight repos are public" && \ - mkdir -p /root/.cargo && \ - echo '[net]' >> /root/.cargo/config.toml && \ - echo 'git-fetch-with-cli = true' >> /root/.cargo/config.toml && \ - git config --global url."https://${GITHUB_USER}:${GITHUB_PAT}@github.com/".insteadOf "https://github.com/" && \ - echo "Git config set. Testing access..." && \ - git ls-remote https://github.com/midnightntwrk/midnight-ledger-prototype > /dev/null 2>&1 && echo "✓ Git access successful" || echo "✗ Git access failed" +RUN --mount=type=secret,id=github_user,mode=0400 \ + --mount=type=secret,id=github_pat,mode=0400 \ + set -euo pipefail; \ + if [ ! -f /run/secrets/github_user ] || [ ! -f /run/secrets/github_pat ]; then \ + >&2 echo "ERROR: GitHub credentials must be provided as Docker secrets."; \ + exit 1; \ + fi; \ + GITHUB_USER="$(cat /run/secrets/github_user)"; \ + GITHUB_PAT="$(cat /run/secrets/github_pat)"; \ + mkdir -p /root/.cargo; \ + printf '[net]\ngit-fetch-with-cli = true\n' >> /root/.cargo/config.toml; \ + echo "Testing GitHub access to private Midnight repositories..."; \ + git -c url."https://${GITHUB_USER}:${GITHUB_PAT}@github.com/".insteadOf="https://github.com/" \ + ls-remote https://github.com/midnightntwrk/midnight-ledger-prototype >/dev/null 2>&1 \ + || { >&2 echo "✗ Git access failed"; exit 1; }; \ + echo "✓ Git access successful"This avoids persistent global config, doesn’t log the username, and hard-fails on auth errors. When the repos are public, you can delete this whole block.
47-53: Unify build profile, avoid double compile, and keep credentials ephemeral during cargo fetchYou build with --release, but install with --debug, and you compile twice (cargo build + cargo install). Also, credentials are still written to global git config and cleaned later. Do both the credential wiring and install in a single layer with an ephemeral config and avoid the redundant build:
RUN --mount=type=cache,target=/usr/local/cargo/registry \ --mount=type=cache,target=/usr/app/target \ --mount=type=secret,id=github_user,mode=0400 \ --mount=type=secret,id=github_pat,mode=0400 \ - GITHUB_USER=$(cat /run/secrets/github_user) && \ - GITHUB_PAT=$(cat /run/secrets/github_pat) && \ - cargo build --release && \ - cargo install --root /usr/app --path . --debug && \ - git config --global --unset-all url."https://${GITHUB_USER}:${GITHUB_PAT}@github.com/".insteadOf && \ - echo "Git credentials cleared" + set -euo pipefail; \ + GITHUB_USER="$(cat /run/secrets/github_user)"; \ + GITHUB_PAT="$(cat /run/secrets/github_pat)"; \ + printf '[url "https://%s:%[email protected]/"]\n\tinsteadOf = https://github.com/\n' "$GITHUB_USER" "$GITHUB_PAT" > /tmp/gitconfig; \ + export GIT_CONFIG_GLOBAL=/tmp/gitconfig; \ + # cargo install defaults to a release build; no need for a separate cargo build + cargo install --locked --root /usr/app --path .; \ + rm -f /tmp/gitconfigBenefits: consistent release build, smaller/faster image, and no credential residue in any layer.
50-50: Inconsistent flags: building release, installing debugMixing profiles bloats the image and surprises perf. Use a single profile (release) end-to-end. If you keep the current structure, replace --debug with the default (release).
🧹 Nitpick comments (12)
Dockerfile.midnight.development (3)
62-64: Remove unused ARG or actually use itTARGETARCH is declared but unused. Either remove it or use it (e.g., to select the Node package if arch-specific).
-ARG TARGETARCH ARG NODE_VERSION=20.19
67-73: Harden Node toolchain installs (pin versions; reduce drift)Global npm installs without versions reduce reproducibility. Pin versions to known working releases (adjust as needed):
-RUN npm install -g pnpm ts-node typescript +RUN npm install -g [email protected] [email protected] [email protected]Also consider setting:
- npm_config_fund=false and npm_config_update_notifier=false to quiet noisy logs
- Using corepack for pnpm if you prefer vendored version management
75-75: Don’t remove base meta package; only drop apk-tools if you must
apk del wolfi-base apk-toolsis risky: wolfi-base may not be an installed package, and removing apk-tools is optional. If your goal is to trim, just remove apk-tools; otherwise omit this step.- RUN apk del wolfi-base apk-tools + RUN apk del apk-tools || truePlease confirm this line currently succeeds in CI for all arches.
examples/midnight-basic-example/README.md (9)
183-186: Use standard header casing and shell variable for API key.HTTP header names are case-insensitive, but “Authorization” is the conventional form. Also prefer
$API_KEYto reduce copy/paste mistakes.Apply:
- -H "Content-Type: application/json" \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq + -H "Authorization: Bearer $API_KEY" | jq
211-214: Consistent Authorization header usage.Align with the convention and reuse
$API_KEY.-curl -X GET http://localhost:8080/api/v1/relayers/midnight-testnet-example/balance \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq +curl -X GET http://localhost:8080/api/v1/relayers/midnight-testnet-example/balance \ + -H "Authorization: Bearer $API_KEY" | jq
222-224: Consistent Authorization header and shell variable for POST.-curl -X POST http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions \ - -H "Content-Type: application/json" \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" \ +curl -X POST http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions \ + -H "Content-Type: application/json" \ + -H "Authorization: Bearer $API_KEY" \
247-254: Consistent Authorization header for status lookup.-curl -X GET http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions/{transaction_id} \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq +curl -X GET http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions/{transaction_id} \ + -H "Authorization: Bearer $API_KEY" | jq
259-261: Consistent Authorization header for list endpoint.-curl -X GET "http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions?per_page=5" \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq +curl -X GET "http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions?per_page=5" \ + -H "Authorization: Bearer $API_KEY" | jq
267-283: Add a language to fenced block to satisfy MD040 and improve rendering.The architecture ASCII block lacks a language identifier.
-``` +```text ...--- `384-389`: **Invalid JSON due to comment; switch to jsonc or move comment out.** The snippet uses “// Minimum balance in speck,” which is not valid JSON. Either change the fence to jsonc: ```diff -```json +```jsonc { "policies": { "min_balance": 1000000000 // Minimum balance in speck } }Or remove the inline comment and keep json.
49-57: Re-validate the temporary private-repos requirement and scope it tightly.This section says GitHub access to private Midnight repos is required and will be removed later. Given the PR’s blockers include “open-sourcing midnight-node and dependencies,” this may already be obsolete by the time users read it.
Proposed tweak:
-> **⚠️ TEMPORARY REQUIREMENT - WILL BE REMOVED** +> **⚠️ Temporary requirement (pending open-sourcing)** @@ -> **This requirement is temporary.** Once the Midnight repositories are open-sourced, all GitHub authentication code will be removed from this example. +> This requirement will be removed once the Midnight repositories are open-sourced (see PLAT-6814). If they are already public in your clone, you can skip these credentials.
182-185: Remove unnecessary Content-Type header on GET.GET requests don’t require Content-Type and including it can confuse users.
-curl -X GET http://localhost:8080/api/v1/relayers \ - -H "Content-Type: application/json" \ - -H "AUTHORIZATION: Bearer YOUR_API_KEY" | jq +curl -X GET http://localhost:8080/api/v1/relayers \ + -H "Authorization: Bearer $API_KEY" | jq
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Dockerfile.midnight.development(1 hunks)docs/modules/ROOT/pages/midnight.adoc(1 hunks)examples/midnight-basic-example/README.md(1 hunks)examples/midnight-basic-example/config/config.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/modules/ROOT/pages/midnight.adoc
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/midnight-basic-example/config/config.json
🧰 Additional context used
🪛 LanguageTool
examples/midnight-basic-example/README.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...data. ## Table of Contents - Overview - Prerequisites - [Gettin...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...- Overview - Prerequisites - Getting Started - [...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ites](#prerequisites) - Getting Started - [Step 1: Clone the Repository](#step-1-cl...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...arted) - Step 1: Clone the Repository - [Step 2: Create a Signer](#step-2-create-...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...repository) - Step 2: Create a Signer - [Step 3: Configure Environment](#step-3-c...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...gner) - Step 3: Configure Environment - [Step 4: Configure Notifications](#step-4...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...nt) - Step 4: Configure Notifications - [Step 5: Configure API Key](#step-5-confi...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...ications) - Step 5: Configure API Key - [Step 6: Run the Service](#step-6-run-the...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...re-api-key) - Step 6: Run the Service - [Step 7: Verify the Setup](#step-7-verify...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...e-service) - Step 7: Verify the Setup - [Testing the Relayer](#testing-the-relaye...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...verify-the-setup) - Testing the Relayer - Architecture - [Troubles...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...r](#testing-the-relayer) - Architecture - Troubleshooting - [Ad...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...cture](#architecture) - Troubleshooting - [Advanced Configuration](#advanced-config...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ... - Setting up a Midnight testnet relayer - Configuring the prover service for zero-...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...vice for zero-knowledge proof generation - Submitting privacy-preserving transactio...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...bmitting privacy-preserving transactions - Monitoring transaction status ### Key F...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...om/get-docker/) (version 20.10 or later) - [Docker Compose](https://docs.docker.com/...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...compose/install/) (version 2.0 or later) - [Rust](https://www.rust-lang.org/tools/in...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...1.86 or later, for key generation tools) - jq (opt...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ....io/jq/) (optional, for JSON formatting) - Access to Midnight testnet (wallet seeds...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...night testnet (wallet seeds for testing) - [TEMPORARY] GitHub credentials with ac...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...with access to the Midnight repositories > - A GitHub Personal Access Token (PAT) or ...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ...erates zero-knowledge proofs (port 6300) - OpenZeppelin Relayer: Manages transact...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...ayer**: Manages transactions (port 8080) - Redis: Stores relayer state (port 6379...
(QB_NEW_EN)
[grammar] ~395-~395: There might be a mistake here.
Context: ...e TTL**: Cache duration for Merkle roots 2. Rate Limiting: Adjust `RATE_LIMIT_REQU...
(QB_NEW_EN)
[grammar] ~396-~396: There might be a mistake here.
Context: ...UESTS_PER_SECOND` in docker-compose.yaml 3. Redis Persistence: Configure save inte...
(QB_NEW_EN)
[grammar] ~414-~414: There might be a mistake here.
Context: ... for keystores 3. Network Security: - Restrict network access to necessary por...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
examples/midnight-basic-example/README.md
267-267: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (8)
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: test
- GitHub Check: clippy
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (rust)
🔇 Additional comments (6)
Dockerfile.midnight.development (2)
80-83: Good: non-root install for plugins with frozen lockfileNice to see non-root and a reproducible install. Consider adding a .npmrc/.pnpmfile to further lock auth/scope if private registries are used.
87-93: Runtime wiring looks solidNon-root execution, explicit ports, and a single binary entrypoint are good defaults for prod images.
examples/midnight-basic-example/README.md (4)
420-425: Double-check external links (docs and API docs) before merge.External URLs tend to drift. Please verify these remain valid at merge time, or adjust to canonical/permalinked locations.
Would you like me to prepare a link-check script (excluding localhost) to run in CI to prevent regressions?
1-37: Overall: strong structure and actionable flow.TOC matches headings, step order is consistent, and copy/paste commands are clear. Nice job providing health checks and end-to-end verification.
216-224: Update README to Alert Users of PLAT-6814 Schema ChangeBefore the “Submit a simple value transfer transaction” curl example in
examples/midnight-basic-example/README.md(around lines 216–224), add a prominent warning that destination wallet seeds are deprecated and will be removed by PLAT-6814. This prevents users from copying insecure patterns and reminds them to switch to addresses/handles once the SDK and LedgerContext update lands.• File to update:
– examples/midnight-basic-example/README.md (lines 216–224)• Diff to apply:
Submit a simple value transfer transaction: +> **Important:** The request schema is being updated by PLAT-6814 to remove destination wallet seeds. +> Until the SDK and LedgerContext changes land, treat the fields below as placeholders and never paste real seeds. +> After the update, replace these with destination addresses or handles. ```bash curl -X POST http://localhost:8080/api/v1/relayers/midnight-testnet-example/transactions \
42-48: Rust version requirement is accurateThe repository’s
rust-toolchain.tomlpins the toolchain to 1.86.0, and yourCargo.tomldeclaresrust-version = "1.86"as the MSRV. The README’s “Rust (version 1.86 or later)” exactly matches these settings, so no update is needed here.
2 participants
Summary
https://linear.app/openzeppelin-development/issue/PLAT-6527/midnight-support
Parent PR that consists of the following children:
❌ Unable to merge until this is resolved:
MIDNIGHT_LEDGER_TEST_STATIC_DIRLedgerContext::new_from_wallet_seeds[…]midnight-nodeand dependency reposTODO
LedgerContextis updatedTesting Process
Checklist
Summary by CodeRabbit
New Features
Documentation
Examples
Chores