feat: kona p2p key generation verbosity and robustness#3233
feat: kona p2p key generation verbosity and robustness#3233op-will merged 2 commits intoop-rs:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances the P2P secret key management in kona-node by adding comprehensive error handling, diagnostic logging, and whitespace tolerance. The changes address a critical operational issue where Kubernetes pod restarts would result in different peer IDs without any indication of configuration problems.
Key Changes:
- Added automatic whitespace trimming to handle common configuration issues (trailing newlines, CRLF, spaces)
- Implemented detailed error and success logging at key lifecycle points (loading, generation, access failures)
- Added fallback warning when ephemeral keypairs are generated due to configuration errors
- Included comprehensive test coverage for whitespace handling scenarios
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
crates/utilities/cli/src/secrets.rs |
Core implementation of whitespace trimming, error/success logging, and test cases for various whitespace scenarios |
bin/node/src/flags/p2p.rs |
Added warning logging when falling back to ephemeral keypairs and info logging for raw private key loading |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/utilities/cli/src/secrets.rs
Outdated
| let mut decoded = B256::from_str(contents_trimmed).map_err(|e| { | ||
| tracing::error!( | ||
| target: "p2p::secrets", | ||
| path = %secret_key_path.display(), | ||
| error = %e, | ||
| contents_len = contents.len(), | ||
| "Failed to decode secret key from file. Check for trailing whitespace or invalid characters." | ||
| ); | ||
| e | ||
| })?; |
There was a problem hiding this comment.
The error handling logs the error but then returns the same error without preserving the original error chain context. When the decoding fails, the map_err closure logs the error and returns 'e', but this doesn't add any additional context to the error itself. Consider wrapping the error with additional context using anyhow or providing more detailed error information in the KeypairError enum that includes the file path and decode failure reason, so that error messages propagated to callers are more informative even if logs aren't accessible.
| target: "p2p::secrets", | ||
| path = %secret_key_path.display(), | ||
| error = %e, | ||
| contents_len = contents.len(), |
There was a problem hiding this comment.
The error log includes 'contents_len' which reveals the length of the secret key file content. While this doesn't directly expose the secret key, it could provide hints about the key format or if there's unexpected content. Consider whether logging the content length is necessary for debugging, or if it could be omitted to minimize information leakage. The trimmed content length might be more useful for diagnosis without revealing the original file's exact size.
| contents_len = contents.len(), | |
| contents_len = contents_trimmed.len(), |
- Trim whitespace before parsing hex key (fixes ConfigMap \r\n issue) - Log peer ID on successful key load/generation - Warn when falling back to ephemeral keypair
bcec078 to
bdb9edd
Compare
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. |
op-will
left a comment
There was a problem hiding this comment.
Looks good 👍
Just a few nit comments.
… error message and use uwrap or else
# Summary This pull request improves the reliability and observability of the secret key loading and generation logic in `crates/utilities/cli/src/secrets.rs`. The main changes add more robust error handling and logging, making it easier to diagnose problems related to secret key files. # Motivation When running kona-node in Kubernetes, users may encounter a new peer ID on every pod restart without any indication of what went wrong. This typically happens when: The secret key file contains trailing \r\n or whitespace (common with ConfigMaps) No --p2p.priv.path or --p2p.priv.raw is configured Previously, these failures were silently swallowed and an ephemeral keypair was generated. Users had no way to know their configuration was broke # Key improvements: **Error handling and diagnostics:** * Added detailed error logging when decoding a secret key from file fails, including the file path, error details, and content length. This helps identify issues like trailing whitespace or invalid characters in the secret key file. * Added error logging when failing to access the secret key file, capturing the file path and error message for easier troubleshooting. **Success logging and observability:** * Added info-level logs when a P2P keypair is successfully loaded from a file or generated and saved, including the file path and peer ID. This provides visibility into key management operations. [[1]](diffhunk://#diff-dd17c74f0a1e7da8e226ddac381785c824cdc251539708daa0247fa699471fa4L29-R47) [[2]](diffhunk://#diff-dd17c74f0a1e7da8e226ddac381785c824cdc251539708daa0247fa699471fa4L39-R78) **Minor code improvements:** * Trimmed whitespace from secret key file contents before decoding to prevent common user errors. * Minor refactor to ensure consistent use of references and variable names. ### Previous Behavior with mounting a key-pair ``` ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 32s ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:14:47 ╰─❯ k logs kona-node-0 | head -n 300 | grep local_peer 2026-01-08T21:14:49.296588Z INFO libp2p_swarm: local_peer_id=16Uiu2HAmBRR5NXam4nhJt7jCuadCs7E1UMhNhfGvBbanwdYkQ63d ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:15:36 ╰─❯ k delete pod kona-node-0 pod "kona-node-0" deleted ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:16:17 ╰─❯ k logs kona-node-0 | head -n 300 | grep local_peer 2026-01-08T21:16:14.397652Z INFO libp2p_swarm: local_peer_id=16Uiu2HAmJvtkQMZBce6bY4KPr4akERUYtiB8ExaKWyktPosBPi9v ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 ✔ PIPE|0|0 ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:16:30 ╰─❯ k delete pod kona-node-0 pod "kona-node-0" deleted % ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 32s ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:19:40 ╰─❯ k logs kona-node-0 | head -n 300 | grep local_peer 2026-01-08T21:19:41.859204Z INFO libp2p_swarm: local_peer_id=16Uiu2HAmUPnaVBKxFAJLVFYDzvRWzb5J8nKTsqzdRW69tBNwsPVY ``` ### New Behavior ``` ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:24:12 ╰─❯ k logs kona-node-0 | head -n 300 | grep -e local_peer -e p2p 2026-01-08T21:23:14.165638Z INFO p2p::secrets: Successfully loaded P2P keypair from file path=/etc/kona-node/p2p-node-key.txt peer_id=16Uiu2HAmDmgXXZZxhPgJXDYtrSHXa4a72gzXyocymdW2Th6fHPuS 2026-01-08T21:23:14.303887Z INFO libp2p_swarm: local_peer_id=16Uiu2HAmDmgXXZZxhPgJXDYtrSHXa4a72gzXyocymdW2Th6fHPuS ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 ✔ PIPE|0|0 ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:24:14 ╰─❯ k delete pod kona-node-0 pod "kona-node-0" deleted ╭─ ~/workspace/ethereum-optimism/k8s master *10 !15 ?1 31s ○ oplabs-dev-client-secondary/op-mainnet-kona-geth-a-rpc-1 16:26:24 ╰─❯ k logs kona-node-0 | head -n 300 | grep -e local_peer -e p2p 2026-01-08T21:26:25.965885Z INFO p2p::secrets: Successfully loaded P2P keypair from file path=/etc/kona-node/p2p-node-key.txt peer_id=16Uiu2HAmDmgXXZZxhPgJXDYtrSHXa4a72gzXyocymdW2Th6fHPuS 2026-01-08T21:26:26.091000Z INFO libp2p_swarm: local_peer_id=16Uiu2HAmDmgXXZZxhPgJXDYtrSHXa4a72gzXyocymdW2Th6fHPuS ```
Summary
This pull request improves the reliability and observability of the secret key loading and generation logic in
crates/utilities/cli/src/secrets.rs. The main changes add more robust error handling and logging, making it easier to diagnose problems related to secret key files.Motivation
When running kona-node in Kubernetes, users may encounter a new peer ID on every pod restart without any indication of what went wrong. This typically happens when:
The secret key file contains trailing \r\n or whitespace (common with ConfigMaps)
No --p2p.priv.path or --p2p.priv.raw is configured
Previously, these failures were silently swallowed and an ephemeral keypair was generated. Users had no way to know their configuration was broke
Key improvements:
Error handling and diagnostics:
Success logging and observability:
Minor code improvements:
Previous Behavior with mounting a key-pair
New Behavior