feat: v0.17.0 — Security & Efficiency Hardening by LucienSong · Pull Request #22 · ShellDAO/shell-chain

LucienSong · 2026-04-21T08:12:03Z

Summary

This PR brings v0.17.0 security and efficiency hardening across the full shell-chain codebase — result of a 16-phase dual-axis (security × efficiency) code review.

Security Fixes

Area	Fix
RPC CORS	Default changed from `*` → `None` (same-origin only)
RPC DoS	`eth_call` gas capped at 50 M (was unbounded)
RPC leakage	`internal_err()` logs server-side, returns generic message to caller
CLI keystore	Rejects world/group-readable keystore files on Unix (`chmod 600` enforced)
Consensus	Slashing wired: `slash_authority()` now mutates `PoaConfig.slashed`, `is_authority()` excludes slashed
Network	`BodyResponse` unicast to requesting peer (was broadcast → O(n) amplification)
Memory	`tx_broadcast` channel bounded to 4096 + `try_send` backpressure

Reliability

--storage-profile archive + --pruning N now fails early with a clear error (was silently ignored)
RegistryError and WindowError now implement std::error::Error

Code Quality

crates/node/src/node.rs (4 575 lines) split into 6 modules
crates/rpc/src/handler.rs (4 762 lines) split into 7 modules
All 2 remaining production unwrap() calls eliminated
Fixes Libp2pNetwork::broadcast() missing match arms for BodyRequest/BodyResponse (compile error with libp2p feature)

CI / Supply Chain

New supply-chain CI job: cargo deny check + cargo audit on every push and PR.

Test Results

208 passed, 0 failed

(3 pre-existing failures in RPC handler tests were also fixed as part of this PR)

P1 - Architecture (background agent): - Split node/node.rs (4575 lines) into node/ module: mod.rs, event_loop.rs, block_producer.rs, block_importer.rs, p2p_handlers.rs, dev_rpc.rs - Split rpc/handler.rs (4762 lines) into handler/ module: mod.rs, eth.rs, evm.rs, shell_api.rs, net.rs, debug.rs, admin.rs - Fix production unwrap() in cli/commands/run.rs and historical_sync.rs P2 - Core types: with direct &buf[1..] slice (buf.is_empty() already checked above) P4 - Consensus safety: - Add HashSet<Address> slashed field to PoaConfig - Modify is_authority() to exclude slashed addresses - Add slash_authority() to PoaConfig and PoaEngine - Wire equivocation slashing in event_loop.rs: replace TODO log-only with consensus.write().slash_authority(&equivocation.offender) P7 - Network efficiency: - Add send_to_peer() to NetworkService trait with default broadcast fallback - Fix BodyResponse amplification: change broadcast() to send_to_peer(&peer) preventing O(n) data amplification to all connected peers - Fix Libp2pNetwork::broadcast() missing BodyRequest/BodyResponse match arms (compile error when libp2p feature enabled) P9 - Memory safety: - Change tx_broadcast from unbounded_channel to channel(4096) - Use try_send (non-blocking) in RPC handler to avoid blocking on full channel - Update RPC server and handler types: UnboundedSender -> Sender Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- P8 RPC: default CORS from '*' → None (same-origin only) - P8 RPC: cap eth_call/eth_estimateGas gas at 50M (DoS prevention) - P8 RPC: internal_err() logs details server-side, returns generic message to caller (prevent implementation leakage) - P10 CLI: reject world-readable keystore on Unix (mode & 0o077 != 0) - P10 CLI: reject --storage-profile archive + --pruning >0 conflict - P11: RegistryError + WindowError now implement std::error::Error - P15 CI: add supply-chain job with cargo-deny + cargo-audit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add not_found_err() helper (code -32001) for resource-not-found responses — these are valid user-facing messages, not internal errors - lookup_tx_with_block: use not_found_err() for tx/block/receipt misses - resolve_block: use not_found_err() for missing blocks - send_raw_transaction: use invalid_params_err() for bad RLP/JSON format (user input error, not internal server error) - Fixes: debug_trace_transaction_not_found, trace_oe_transaction_not_found, m6b1_send_raw_transaction_invalid_format_rejected Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR targets the v0.17.0 release by hardening RPC/network/node behavior for security and operational efficiency, while also splitting large modules into smaller ones to improve maintainability.

Changes:

Tighten RPC defaults and inputs (CORS default off, eth_call gas cap, reduce error leakage, bound tx broadcast channel).
Reduce network amplification by introducing a unicast API surface for BodyResponse and wiring the node to use it.
Refactor large RPC handler and node modules into focused submodules; add CI supply-chain checks.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
crates/rpc/src/server.rs	RPC config hardening (CORS default) and bounded tx broadcast channel type.
crates/rpc/src/handler/mod.rs	RPC handler refactor + helper/error semantics changes + tx broadcast backpressure + `eth_call` gas cap.
crates/rpc/src/handler/shell_api.rs	New shell RPC namespace module (governance/node stats/witness APIs).
crates/rpc/src/handler/net.rs	New web3/net/debug RPC implementations (sha3 limits, peer count).
crates/rpc/src/handler/evm.rs	Extracted dev EVM RPC endpoints (mine/snapshot/etc).
crates/rpc/src/handler/eth.rs	Extracted ETH RPC endpoints; adjusts error classification in some paths.
crates/rpc/src/handler/debug.rs	Extracted trace RPC endpoints (OE-compatible traces).
crates/rpc/src/handler/admin.rs	Extracted admin RPC endpoints (node_info/peers/add_peer stub).
crates/node/src/node/mod.rs	Split monolithic node implementation into submodules and re-exports.
crates/node/src/node/event_loop.rs	Event loop extracted; bounded tx broadcast channel; attempts unicast `BodyResponse`; slashing application.
crates/node/src/node/block_producer.rs	Block production logic extracted.
crates/node/src/node/block_importer.rs	Block import/validation logic extracted.
crates/node/src/node/dev_rpc.rs	Dev RPC control logic extracted.
crates/node/src/node/p2p_handlers.rs	P2P transaction/attestation handlers extracted.
crates/node/src/historical_sync.rs	Minor control-flow simplification in historical body sync.
crates/network/src/service.rs	Adds `send_to_peer` API to network trait (intended unicast).
crates/network/src/libp2p_service.rs	Adds missing topic match arms for body messages under libp2p.
crates/core/src/account.rs	Removes production `unwrap()` from RLP decoding logic.
crates/consensus/src/window.rs	Implements `std::error::Error` for `WindowError`.
crates/consensus/src/prover_registry.rs	Implements `std::error::Error` for `RegistryError`.
crates/consensus/src/poa.rs	Adds slashing tracking and enforcement (`slashed` set + `slash_authority`).
crates/cli/src/commands/run.rs	Enforces keystore permissions on Unix; rejects archive+pruning conflict; improves retention banner output.
Cargo.toml	Bumps workspace version to 0.17.0.
CHANGELOG.md	Adds v0.17.0 release notes.
.github/workflows/ci.yml	Adds supply-chain CI job (`cargo deny` + `cargo audit`).

Comments suppressed due to low confidence (6)

crates/rpc/src/handler/mod.rs:672

validate_block_is_latest reports malformed block tags via internal_err (-32603). Since this is pure user input validation, prefer invalid_params_err so clients get -32602 and internal error logs stay reserved for actual server issues.
crates/rpc/src/handler/mod.rs:595
parse_address maps user input errors to internal_err, which now logs server-side and returns a generic -32603. Invalid addresses are caller mistakes and should use invalid_params_err so clients get -32602 without polluting internal error logs.
crates/rpc/src/handler/mod.rs:608
parse_hex_u64 converts parse failures into internal_err (-32603). Since this is user-controlled input, it should return invalid_params_err (-32602) instead.
crates/rpc/src/handler/mod.rs:617
parse_hex_u256 rejects oversized inputs via internal_err (-32603). Oversized/invalid hex is an invalid-params condition; use invalid_params_err so clients can correct the request and the server doesn’t log it as an internal error.
crates/rpc/src/handler/mod.rs:646
parse_block_tag is used for user-supplied block identifiers, but its error paths return internal_err (-32603). These should be invalid_params_err (-32602) so callers can distinguish invalid input from server faults and so the server doesn’t log bad input as an internal error.
crates/rpc/src/handler/mod.rs:602
parse_hex_hash treats malformed/incorrect-length hashes as internal_err (-32603). These are invalid params; returning invalid_params_err will preserve the caller-visible error class and avoid logging as an internal server error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+
+        Ok(serde_json::json!({
+            "version": "ShellChain/v0.6.0/rust",


+    async fn send_raw_transaction(&self, data: String) -> Result<ShellHash, ErrorObjectOwned> {
+        // Decode hex payload: "0x" + hex-encoded transaction bytes.
+        let raw = data.strip_prefix("0x").unwrap_or(&data);
+        let bytes = hex::decode(raw).map_err(|e| internal_err(format!("invalid hex: {e}")))?;


+                                            "L4: serving BodyResponse via unicast to requesting peer"
+                                        );
+                                        let _ = network.send_to_peer(&peer, NetworkMessage::BodyResponse { blocks }).await;
+                                    }


+            .map(|r| format!("0x{}", hex::encode(r.as_bytes())))
+            .unwrap_or_else(|| "null".into());


+        const MAX_HEX_LEN: usize = 32 * 1024 * 2; // 32 KB decoded = 64 KB hex
+        if raw.len() > MAX_HEX_LEN {
+            return Err(internal_err("input too large (max 32 KB)"));
+        }
+        let bytes = hex::decode(raw).map_err(|e| internal_err(format!("invalid hex: {e}")))?;


+    /// Send a message to a specific peer only (unicast).
+    ///
+    /// Default implementation falls back to broadcast. Implementations that
+    /// support direct peer messaging should override this to avoid amplification.


+                "blockHash": block_hash,
+                "witnessRoot": witness_root,
+                "witnessCount": null,
+                "witnesses": null,
+                "error": "witness store not available on this node",


+#[jsonrpsee::core::async_trait]
+impl<S: KvStore + 'static> Web3ApiServer for RpcHandler<S> {
+    async fn client_version(&self) -> Result<String, ErrorObjectOwned> {
+        Ok("shell-chain/0.6.0".to_string())


- net.rs: web3_sha3 returns invalid_params_err for oversized/invalid hex input - shell_api.rs: witness_root uses serde_json::Value::Null instead of string 'null' - eth.rs: send_raw_transaction hex decode error uses invalid_params_err - service.rs: send_to_peer default impl has explicit broadcast-fallback comment - event_loop.rs: BodyResponse unicast call site documents fallback behavior - deny.toml: unmaintained = 'none' (valid cargo-deny >=0.16 enum value) - cargo fmt: remove trailing blank lines across handler files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Update web3_client_version and get_node_info tests to use - Remove [licenses] deny array from deny.toml (removed in cargo-deny per PR #611; the allow list implicitly denies everything else) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- cargo update: rand 0.8.5->0.8.6 (fixes RUSTSEC-2026-0097 unsound) - cargo update: rustls-webpki 0.103.10->0.103.13 (fixes RUSTSEC-2026-0098/0099) - deny.toml: remove exceptions=[] (invalid in cargo-deny >=0.16) - deny.toml: remove syn skip entries (unmatched in tree; cargo-deny 0.16 promotes unmatched-skip to error level) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add license.workspace = true to shell-e2e-tests, shell-load-test, shell-multi-prover, shell-stark-bench (were unlicensed) - Add Unicode-3.0 and CDLA-Permissive-2.0 to [licenses] allow list (required by icu_collections and related transitive deps) - Change wildcards = 'deny' -> 'warn' (workspace = true deps in member Cargo.toml files are flagged as wildcards by cargo-deny 0.19) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

LucienSong and others added 4 commits April 21, 2026 04:44

chore: bump version to 0.17.0

38aa9bd

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 21, 2026 08:12

Copilot started reviewing on behalf of LucienSong April 21, 2026 08:12 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

LucienSong and others added 4 commits April 21, 2026 17:00

LucienSong merged commit b6e1e03 into ShellDAO:main Apr 21, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: v0.17.0 — Security & Efficiency Hardening#22

feat: v0.17.0 — Security & Efficiency Hardening#22
LucienSong merged 8 commits into
ShellDAO:mainfrom
LucienSong:main

LucienSong commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		.map(\|r\| format!("0x{}", hex::encode(r.as_bytes())))
		.unwrap_or_else(\|\| "null".into());

Conversation

LucienSong commented Apr 21, 2026

Summary

Security Fixes

Reliability

Code Quality

CI / Supply Chain

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants