feat(l1): add interactive REPL #6164
Conversation
Add an interactive REPL (Read-Eval-Print Loop) that supports all ethrex JSON-RPC namespaces (eth, debug, admin, net, web3, txpool) with tab completion, multi-line input, command history, and utility functions (toWei, fromWei, keccak256, etc). Integrated as `ethrex repl` subcommand.
Resolve ENS names (e.g. vitalik.eth) in any command argument typed as Address. Queries the ENS registry for the resolver then the resolver for the address, prints feedback like "Resolved vitalik.eth -> 0xd8dA...", and passes the resolved address to the RPC call. No new dependencies.
ilitteri
changed the title
🤖 Kimi Code ReviewReview SummaryThis PR introduces a new REPL tool for ethrex with JSON-RPC capabilities. Overall, the code is well-structured and follows Rust best practices. Here are my findings: Security Issues
Performance Issues
Code Quality Issues
Suggested Fixes// In tooling/repl/src/ens.rs, add resolver validation:
const ENS_REGISTRY: &str = "0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e";
const EXPECTED_RESOLVER: &str = "0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e"; // Add expected resolver
// In tooling/repl/src/lib.rs, sanitize paths:
fn expand_tilde(path: &str) -> String {
if path.starts_with('~') {
if let Ok(home) = std::env::var("HOME") {
let sanitized = path.replacen('~', &home, 1);
// Prevent path traversal
if sanitized.contains("..") {
return home + "/.ethrex/history"; // fallback
}
return sanitized;
}
}
path.to_string()
}
// In tooling/repl/src/commands/mod.rs, use constants:
const ADDRESS_LENGTH: usize = 42;
const HASH_LENGTH: usize = 66;Positive Aspects
The PR is ready to merge after addressing the security issues mentioned above. Automated review by Kimi (Moonshot AI) · custom prompt |
ilitteri
changed the title
ilitteri
changed the title
Lines of code reportTotal lines added: Detailed view |
🤖 Claude Code ReviewBased on my comprehensive review of this PR, here's my detailed code review: PR Review: feat(repl): geth-like REPL for ethrexOverviewThis PR adds a comprehensive interactive REPL for ethrex with ENS name resolution support. The implementation is well-structured with good separation of concerns across modules (parser, ENS, client, commands, formatter, completer). Security Issues🔴 CRITICAL: Integer Overflow in toWei (repl.rs:334)Location: let wei = (amount * multiplier) as u128;Issue: Converting Example: let amount = 1e40f64; // Much larger than u128::MAX
let wei = (amount * 1e18) as u128; // Silently overflows!Fix: Add overflow checking: let product = amount * multiplier;
if product > u128::MAX as f64 || product < 0.0 {
return formatter::format_error("value too large or negative");
}
let wei = product as u128;🟡 MEDIUM: ENS Resolution Network Dependency (ens.rs:7)Location: const ENS_REGISTRY: &str = "0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e";Issue: Hardcoded ENS registry address only works on mainnet, Goerli, and Sepolia. Will fail silently on other networks (returning "ENS name not found") rather than informing users ENS isn't supported. Recommendation: Either:
🟡 MEDIUM: Untrusted RPC Input to eth_call (ens.rs:41, 50)Location: The ENS resolution calls
Recommendation: Add explicit gas limit to eth_call: let params = vec![
json!({"to": to, "data": data, "gas": "0x100000"}), // Add reasonable gas limit
Value::String("latest".to_string()),
];Code Quality Issues🟡 MEDIUM: Panic in Parser (parser.rs:307)Location: Err(ParseError::UnexpectedChar(trimmed.chars().next().unwrap()))Issue: Will panic if Fix: Err(ParseError::UnexpectedChar(trimmed.chars().next().unwrap_or('\0')))🟢 MINOR: Precision Loss in fromWei (repl.rs:359)Location: let result = wei as f64 / divisor;Issue: Converting Example: Recommendation: For a REPL tool this is acceptable, but consider documenting the precision limitation or using a decimal library for exact arithmetic. 🟢 MINOR: Hash Parsing Bounds Check (ens.rs:80-86)Location: if hex.len() < 40 {
return Err(format!("unexpected response length: 0x{hex}"));
}
let addr = &hex[hex.len() - 40..];Issue: If Suggestion: More explicit length check: if hex.len() < 64 { // Full ABI word should be 64 hex chars
return Err(format!("unexpected response length: 0x{hex}"));
}Best Practices Issues🟢 MINOR: Missing Input Sanitization DocumentationLocation: The address and hash validation functions are well-implemented: fn is_valid_address(s: &str) -> bool {
s.starts_with("0x") && s.len() == 42 && s[2..].chars().all(|c| c.is_ascii_hexdigit())
}However, the ENS pub fn looks_like_ens_name(s: &str) -> bool {
!s.starts_with("0x") && s.contains('.')
}This will treat things like Recommendation: Add basic validation (e.g., alphanumeric + allowed ENS chars). 🟢 MINOR: Error Context in Client (client.rs:61)Location: .map_err(|e| RpcError::Transport(e.to_string()))?;The error messages lose context about which endpoint failed. Consider including the endpoint URL in transport errors for better debugging. 🟢 MINOR: Test Coverage GapThe ENS module has good unit tests for namehash, parsing, and checksumming. However, there are no tests for:
Recommendation: Add tests with mocked RPC client. Rust Idioms & Performance✅ GOOD: Proper Error HandlingThe codebase uses ✅ GOOD: Memory Management
✅ GOOD: Async UsageProper use of 🟢 MINOR: Unnecessary String AllocationsLocation: Various places in Multiple Value::String(s) => format_string_value(s),
// Inside format_string_value:
s.cyan().to_string() // colored::ColoredString already owns the stringThis is minor and doesn't significantly impact a REPL's performance. Ethereum-Specific Correctness✅ GOOD: ENS Namehash ImplementationLocation: The namehash algorithm is correctly implemented per EIP-137:
✅ GOOD: EIP-55 Checksum AddressesLocation: Checksum encoding is correct per EIP-55. Includes proper test coverage. ✅ GOOD: JSON-RPC ComplianceThe RPC client properly formats JSON-RPC 2.0 requests with incrementing IDs. Documentation & Usability✅ GOOD: Help SystemComprehensive help with ✅ GOOD: User FeedbackENS resolution prints 🟢 MINOR: Command History Default LocationLocation: #[arg(long, default_value = "~/.ethrex/history")]
history_file: String,The tilde expansion is manually implemented. Consider documenting that the directory must exist or auto-create it (currently SummaryMust Fix (before merge):
Should Fix (before or shortly after merge): Nice to Have: Positive Highlights:
The code is well-written overall. The critical integer overflow issue must be addressed, but otherwise this is a solid implementation of a useful developer tool. Automated review by Claude (Anthropic) · custom prompt |
🤖 Codex Code ReviewFound a few correctness/robustness issues in the new REPL code. No EVM/consensus/state logic changes in this PR. Findings
If you want, I can suggest concrete patches for the above. Automated review by OpenAI Codex · custom prompt |
ilitteri
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a new ethrex-repl crate and integrates it into the main ethrex CLI as a repl subcommand, with on-chain ENS name resolution for address-typed JSON-RPC arguments.
Changes:
- Introduces an interactive REPL (history, completion, multiline input) for invoking Ethereum JSON-RPC namespaces.
- Adds ENS namehash + on-chain resolution (registry → resolver →
addr(bytes32)), and hooks it into RPC execution forParamType::Addressparameters. - Wires the new crate into the workspace and the
ethrex replsubcommand.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tooling/repl/src/repl.rs | REPL loop, command execution, ENS pre-processing, builtins + utility functions. |
| tooling/repl/src/parser.rs | Tokenizer + parser for RPC calls, builtins, and utility calls (with tests). |
| tooling/repl/src/main.rs | Standalone ethrex-repl binary entrypoint. |
| tooling/repl/src/lib.rs | Public run() API and tilde expansion used by both binaries. |
| tooling/repl/src/formatter.rs | Pretty-printing for JSON-RPC results and errors (color + box formatting). |
| tooling/repl/src/ens.rs | ENS namehash + on-chain resolution helpers (with tests). |
| tooling/repl/src/completer.rs | Rustyline tab-completion + hints from the command registry. |
| tooling/repl/src/commands/mod.rs | Command/param schema, param validation/conversion, and registry wiring. |
| tooling/repl/src/commands/eth.rs | eth_* namespace command definitions and parameter schemas. |
| tooling/repl/src/commands/debug.rs | debug_* namespace command definitions and parameter schemas. |
| tooling/repl/src/commands/admin.rs | admin_* namespace command definitions and parameter schemas. |
| tooling/repl/src/commands/net.rs | net_* namespace command definitions and parameter schemas. |
| tooling/repl/src/commands/web3.rs | web3_* namespace command definitions and parameter schemas. |
| tooling/repl/src/commands/txpool.rs | txpool_* namespace command definitions and parameter schemas. |
| tooling/repl/src/client.rs | Minimal JSON-RPC HTTP client used by the REPL and ENS resolution. |
| tooling/repl/Cargo.toml | Declares the new ethrex-repl crate and its dependencies. |
| tooling/Cargo.toml | Adds sha3 to tooling workspace dependencies. |
| tooling/Cargo.lock | Lockfile updates for the new tooling crate dependency graph. |
| cmd/ethrex/cli.rs | Adds ethrex repl subcommand and dispatches to ethrex_repl::run. |
| cmd/ethrex/Cargo.toml | Adds workspace dependency on ethrex-repl. |
| Cargo.toml | Adds tooling/repl workspace member and ethrex-repl workspace dependency. |
| Cargo.lock | Lockfile updates for the new workspace crate and its dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tooling/repl/src/repl.rs
Outdated
| let addr = args[0] | ||
| .strip_prefix("0x") | ||
| .unwrap_or(&args[0]) | ||
| .to_lowercase(); | ||
| if addr.len() != 40 { | ||
| return formatter::format_error("invalid address length"); | ||
| } | ||
| use sha3::{Digest, Keccak256}; | ||
| let hash = Keccak256::digest(addr.as_bytes()); | ||
| let hash_hex = hex::encode(hash); | ||
| let mut checksummed = String::from("0x"); | ||
| for (i, c) in addr.chars().enumerate() { | ||
| if c.is_ascii_alphabetic() { | ||
| let hash_nibble = | ||
| u8::from_str_radix(&hash_hex[i..i + 1], 16).unwrap_or(0); | ||
| if hash_nibble >= 8 { | ||
| checksummed.push(c.to_ascii_uppercase()); | ||
| } else { | ||
| checksummed.push(c); | ||
| } | ||
| } else { | ||
| checksummed.push(c); | ||
| } | ||
| } | ||
| checksummed |
There was a problem hiding this comment.
There are two separate EIP-55 checksum implementations (toChecksumAddress utility in repl.rs and to_checksum_address in ens.rs). This duplication risks divergence if one is fixed/extended later (e.g., stricter input validation). Consider extracting a single shared helper (e.g., in ens or a small address module) and reusing it in both places.
| let addr = args[0] | |
| .strip_prefix("0x") | |
| .unwrap_or(&args[0]) | |
| .to_lowercase(); | |
| if addr.len() != 40 { | |
| return formatter::format_error("invalid address length"); | |
| } | |
| use sha3::{Digest, Keccak256}; | |
| let hash = Keccak256::digest(addr.as_bytes()); | |
| let hash_hex = hex::encode(hash); | |
| let mut checksummed = String::from("0x"); | |
| for (i, c) in addr.chars().enumerate() { | |
| if c.is_ascii_alphabetic() { | |
| let hash_nibble = | |
| u8::from_str_radix(&hash_hex[i..i + 1], 16).unwrap_or(0); | |
| if hash_nibble >= 8 { | |
| checksummed.push(c.to_ascii_uppercase()); | |
| } else { | |
| checksummed.push(c); | |
| } | |
| } else { | |
| checksummed.push(c); | |
| } | |
| } | |
| checksummed | |
| // Delegate to the shared ENS helper to avoid duplicating EIP-55 logic. | |
| ens::to_checksum_address(&args[0]) |
tooling/repl/Cargo.toml
Outdated
| serde.workspace = true | ||
| serde_json.workspace = true | ||
| tokio = { workspace = true, features = ["full"] } | ||
| colored = "3" |
There was a problem hiding this comment.
This crate pulls colored = "3", while the rest of the repo appears to use colored 2.x (resulting in multiple versions in the lockfile). To avoid duplicate deps and potential behavior differences, consider aligning on the same colored major version (e.g., add colored to [workspace.dependencies] and use colored.workspace = true).
| colored = "3" | |
| colored.workspace = true |
There was a problem hiding this comment.
Fixed in abd9009 — downgraded to colored 2.1.0 to match the rest of the workspace.
| rl.set_helper(Some(helper)); | ||
|
|
||
| if let Err(e) = rl.load_history(&self.history_path) | ||
| && !matches!(e, ReadlineError::Io(_)) |
There was a problem hiding this comment.
load_history error is bound to e, but matches!(e, ReadlineError::Io(_)) pattern-matches by value and will move e, making {e} unusable in the warning message. Use matches!(&e, ...) or bind Err(ref e) (or restructure into a nested if let Err(e) { ... }).
| && !matches!(e, ReadlineError::Io(_)) | |
| && !matches!(&e, ReadlineError::Io(_)) |
| '[' => bracket_depth += 1, | ||
| ']' => bracket_depth -= 1, | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
is_balanced can return true for inputs where the depth goes negative mid-scan (e.g. starts with }), because depths can recover back to 0 by the end. Consider returning false immediately if brace_depth or bracket_depth becomes negative while iterating.
| } | |
| } | |
| if brace_depth < 0 || bracket_depth < 0 { | |
| return false; | |
| } |
There was a problem hiding this comment.
Fixed in ad81935 — is_balanced now returns false immediately when depth goes negative mid-scan.
| // Check for namespace.method pattern (RPC call) | ||
| if tokens.len() >= 3 | ||
| && let (Token::Ident(ns), Token::Dot, Token::Ident(method)) = | ||
| (&tokens[0], &tokens[1], &tokens[2]) | ||
| { | ||
| let args = parse_rpc_args(&tokens[3..])?; | ||
| return Ok(ParsedCommand::RpcCall { | ||
| namespace: ns.clone(), | ||
| method: method.clone(), | ||
| args, | ||
| }); | ||
| } | ||
|
|
||
| // Check for utility call | ||
| if let Token::Ident(name) = &tokens[0] | ||
| && UTILITY_NAMES.contains(&name.as_str()) | ||
| { | ||
| let args = tokens[1..] | ||
| .iter() | ||
| .filter(|t| !matches!(t, Token::LParen | Token::RParen | Token::Comma)) | ||
| .map(token_to_string) | ||
| .collect(); | ||
| return Ok(ParsedCommand::UtilityCall { | ||
| name: name.clone(), | ||
| args, | ||
| }); | ||
| } | ||
|
|
||
| // Fallback: treat as utility call with first ident | ||
| if let Token::Ident(name) = &tokens[0] { | ||
| let args = tokens[1..] | ||
| .iter() | ||
| .filter(|t| !matches!(t, Token::LParen | Token::RParen | Token::Comma)) | ||
| .map(token_to_string) | ||
| .collect(); | ||
| return Ok(ParsedCommand::UtilityCall { | ||
| name: name.clone(), | ||
| args, | ||
| }); | ||
| } |
There was a problem hiding this comment.
ParseError::ExpectedMethod is defined but never produced. Inputs like eth. currently won’t surface a helpful error and will fall through to the “utility call” path instead. Consider detecting Ident + Dot with no following method token and returning ParseError::ExpectedMethod.
There was a problem hiding this comment.
Fixed in 665c65a — removed the unused ParseError::ExpectedMethod variant.
| "exit" | "quit" => { | ||
| println!("Bye!"); | ||
| std::process::exit(0); | ||
| } |
There was a problem hiding this comment.
.exit/.quit uses std::process::exit(0), which bypasses the save_history call after the REPL loop, so command history won’t be persisted when users exit normally. Prefer setting a flag / breaking the loop so the function can save history before returning.
There was a problem hiding this comment.
Fixed in ad81935 — .exit/.quit now breaks the loop instead of calling process::exit(0), so save_history runs.
There was a problem hiding this comment.
Fixed in ad81935 — .exit now breaks the loop so save_history runs.
tooling/repl/src/repl.rs
Outdated
| let amount: f64 = match args[0].parse() { | ||
| Ok(v) => v, | ||
| Err(_) => { | ||
| return formatter::format_error(&format!("invalid number: {}", args[0])) | ||
| } | ||
| }; | ||
| let multiplier: f64 = match args[1].to_lowercase().as_str() { | ||
| "wei" => 1.0, | ||
| "gwei" => 1e9, | ||
| "ether" | "eth" => 1e18, | ||
| other => { | ||
| return formatter::format_error(&format!( | ||
| "unknown unit: {other}. Use: wei, gwei, ether" | ||
| )) | ||
| } | ||
| }; | ||
| let wei = (amount * multiplier) as u128; | ||
| format!("{wei}") | ||
| } |
There was a problem hiding this comment.
toWei uses f64 and then casts to u128, which will introduce rounding/truncation errors (and can silently lose precision for many values). Since this is an Ethereum tool, consider parsing decimal strings precisely (e.g., split on '.', scale to wei using integer math / U256) and error on overflow instead of truncating.
There was a problem hiding this comment.
Fixed in ad81935 — toWei now uses integer math with string-based decimal parsing.
tooling/repl/src/repl.rs
Outdated
| let divisor: f64 = match args[1].to_lowercase().as_str() { | ||
| "wei" => 1.0, | ||
| "gwei" => 1e9, | ||
| "ether" | "eth" => 1e18, | ||
| other => { | ||
| return formatter::format_error(&format!( | ||
| "unknown unit: {other}. Use: wei, gwei, ether" | ||
| )) | ||
| } | ||
| }; | ||
| let result = wei as f64 / divisor; | ||
| let s = format!("{result}"); | ||
| s.trim_end_matches('0').trim_end_matches('.').to_string() |
There was a problem hiding this comment.
fromWei converts u128 to f64 and formats it back, which will lose precision for values above 2^53 and can produce incorrect decimal output. Prefer doing unit conversion with integer math and formatting the decimal string explicitly (or using U256 + fixed-point formatting).
| let divisor: f64 = match args[1].to_lowercase().as_str() { | |
| "wei" => 1.0, | |
| "gwei" => 1e9, | |
| "ether" | "eth" => 1e18, | |
| other => { | |
| return formatter::format_error(&format!( | |
| "unknown unit: {other}. Use: wei, gwei, ether" | |
| )) | |
| } | |
| }; | |
| let result = wei as f64 / divisor; | |
| let s = format!("{result}"); | |
| s.trim_end_matches('0').trim_end_matches('.').to_string() | |
| let unit = args[1].to_lowercase(); | |
| match unit.as_str() { | |
| "wei" => wei.to_string(), | |
| "gwei" => { | |
| let divisor: u128 = 1_000_000_000; | |
| let scale: usize = 9; | |
| let integer = wei / divisor; | |
| let frac = wei % divisor; | |
| if frac == 0 { | |
| integer.to_string() | |
| } else { | |
| let mut frac_str = format!("{:0width$}", frac, width = scale); | |
| while frac_str.ends_with('0') { | |
| frac_str.pop(); | |
| } | |
| format!("{integer}.{frac_str}") | |
| } | |
| } | |
| "ether" | "eth" => { | |
| let divisor: u128 = 1_000_000_000_000_000_000; | |
| let scale: usize = 18; | |
| let integer = wei / divisor; | |
| let frac = wei % divisor; | |
| if frac == 0 { | |
| integer.to_string() | |
| } else { | |
| let mut frac_str = format!("{:0width$}", frac, width = scale); | |
| while frac_str.ends_with('0') { | |
| frac_str.pop(); | |
| } | |
| format!("{integer}.{frac_str}") | |
| } | |
| } | |
| other => { | |
| return formatter::format_error(&format!( | |
| "unknown unit: {other}. Use: wei, gwei, ether" | |
| )) | |
| } | |
| } |
There was a problem hiding this comment.
Fixed in ad81935 — fromWei now uses integer division/modulo instead of f64.
| for token in &tokens[1..] { | ||
| match token { | ||
| Token::RParen => break, | ||
| Token::Comma => continue, | ||
| t => args.push(token_to_value(t)), | ||
| } | ||
| } |
There was a problem hiding this comment.
The parenthesized RPC arg parser doesn’t validate that a closing ) was present; if the user forgets it (especially in -x mode where is_balanced isn’t applied), parse_rpc_args will still return Ok(args) instead of an error. Consider explicitly requiring a Token::RParen and returning ParseError::UnexpectedEof/UnterminatedString when missing.
| for token in &tokens[1..] { | |
| match token { | |
| Token::RParen => break, | |
| Token::Comma => continue, | |
| t => args.push(token_to_value(t)), | |
| } | |
| } | |
| let mut found_rparen = false; | |
| for token in &tokens[1..] { | |
| match token { | |
| Token::RParen => { | |
| found_rparen = true; | |
| break; | |
| } | |
| Token::Comma => continue, | |
| t => args.push(token_to_value(t)), | |
| } | |
| } | |
| if !found_rparen { | |
| return Err(ParseError::UnexpectedEof); | |
| } |
There was a problem hiding this comment.
Fixed in 665c65a — parenthesized args now validate that a closing RParen is present.
tooling/repl/src/formatter.rs
Outdated
| if s.len() <= max_len || max_len < 7 { | ||
| return s.to_string(); | ||
| } | ||
| let keep = (max_len - 3) / 2; | ||
| format!("{}...{}", &s[..keep], &s[s.len() - keep..]) |
There was a problem hiding this comment.
truncate_middle slices strings by byte indices (&s[..keep]), which can panic for non-ASCII / multi-byte UTF-8 strings. Since RPC values and error messages can contain arbitrary Unicode, consider truncating using char_indices (or a safe grapheme-aware approach) to avoid panics.
| if s.len() <= max_len || max_len < 7 { | |
| return s.to_string(); | |
| } | |
| let keep = (max_len - 3) / 2; | |
| format!("{}...{}", &s[..keep], &s[s.len() - keep..]) | |
| // Work in characters, not bytes, to avoid slicing in the middle of a UTF-8 code point. | |
| let char_count = s.chars().count(); | |
| if char_count <= max_len || max_len < 7 { | |
| return s.to_string(); | |
| } | |
| let keep = (max_len - 3) / 2; | |
| let chars: Vec<char> = s.chars().collect(); | |
| let end_start = char_count.saturating_sub(keep); | |
| let mut result = String::with_capacity(max_len); | |
| for c in &chars[..keep] { | |
| result.push(*c); | |
| } | |
| result.push_str("..."); | |
| for c in &chars[end_start..] { | |
| result.push(*c); | |
| } | |
| result |
There was a problem hiding this comment.
Fixed in 665c65a — truncate_middle now uses char-based iteration instead of byte slicing.
Add 211 tests covering all REPL modules: Unit tests (189): - parser: tokenizer edge cases, error paths, all command variants - commands: build_params validation, all ParamType conversions, registry - formatter: hex_to_decimal, truncate, inline_value, flatten_object - ens: namehash vectors, name detection, ABI parsing, checksum encoding - completer: dot commands, namespaces, methods, utilities, hints - repl: all 7 utility functions, is_balanced with string/escape handling E2E tests (22): - Mock JSON-RPC server returning canned responses - Full pipeline: parse -> validate -> RPC call -> format - Error paths: unknown commands, missing args, invalid addresses - Utility functions via execute_command
Greptile OverviewGreptile SummaryThis PR adds a new Key merge blockers are in the ENS module: (1) Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| Cargo.toml | Adds tooling/repl as a workspace member and exposes ethrex-repl as a workspace dependency; no logic changes. |
| cmd/ethrex/cli.rs | Adds ethrex repl subcommand wiring to ethrex_repl::run; REPL errors are not propagated (run returns ()). |
| tooling/repl/src/ens.rs | Implements ENS namehash and on-chain resolution via eth_call; issues: missing ENS normalization (case/IDN) and overly-permissive ABI word parsing. |
| tooling/repl/src/repl.rs | Implements interactive REPL loop, parsing/dispatch, and ENS pre-resolution for address params; ENS resolution depends on ens module correctness. |
| tooling/repl/src/client.rs | Adds a simple JSON-RPC client on top of reqwest; returns JSON-RPC errors via RpcError. |
| tooling/repl/src/parser.rs | Adds tokenizer/parser for RPC calls, builtin dot-commands, and utilities; arguments are primarily represented as strings/JSON values. |
| tooling/repl/Cargo.toml | Introduces new ethrex-repl crate with rustyline/reqwest/colored/sha3 deps; adds standalone bin and library. |
| tooling/repl/src/formatter.rs | Adds colored JSON pretty-printing for REPL output; no functional issues found. |
| tooling/repl/src/commands/mod.rs | Defines command/param schema and validation/conversion for REPL RPC calls; no issues found. |
| Cargo.lock | Lockfile updates to add the new ethrex-repl crate and its transitive deps (rustyline, colored v3, etc.). |
Sequence Diagram
sequenceDiagram
participant User
participant REPL as ethrex repl (Repl)
participant Parser
participant ENS as ens::resolve
participant RPC as RpcClient
participant Node as JSON-RPC Endpoint
User->>REPL: Enter `eth.getBalance vitalik.eth`
REPL->>Parser: parse(input)
Parser-->>REPL: ParsedCommand::RpcCall(namespace, method, args)
REPL->>REPL: lookup CommandDef + ParamDefs
REPL->>ENS: resolve_ens_in_args (for Address params)
ENS->>RPC: send_request("eth_call", [registry.resolver(node), "latest"])
RPC->>Node: HTTP POST JSON-RPC
Node-->>RPC: resolver address (ABI-encoded)
ENS->>RPC: send_request("eth_call", [resolver.addr(node), "latest"])
RPC->>Node: HTTP POST JSON-RPC
Node-->>RPC: resolved address (ABI-encoded)
ENS-->>REPL: checksummed 0x address
REPL->>REPL: build_params(resolved_args)
REPL->>RPC: send_request(cmd.rpc_method, params)
RPC->>Node: HTTP POST JSON-RPC
Node-->>RPC: result
RPC-->>REPL: JSON value
REPL-->>User: print formatted result
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn namehash(name: &str) -> [u8; 32] { | ||
| if name.is_empty() { | ||
| return [0u8; 32]; | ||
| } | ||
|
|
||
| let mut node = [0u8; 32]; | ||
| for label in name.rsplit('.') { | ||
| let label_hash = Keccak256::digest(label.as_bytes()); | ||
| let mut data = [0u8; 64]; | ||
| data[..32].copy_from_slice(&node); | ||
| data[32..].copy_from_slice(&label_hash); | ||
| node = Keccak256::digest(&data).into(); | ||
| } | ||
| node |
There was a problem hiding this comment.
ENS namehash not normalized
namehash hashes raw label bytes (label.as_bytes()), but ENS names are effectively case-insensitive and IDNs require UTS-46 normalization/punycode before hashing. As-is, inputs like Vitalik.eth (or any internationalized ENS name) will compute a different namehash and resolution will reliably fail even if the name exists. Consider normalizing the name (lowercasing + UTS-46) before computing the namehash.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tooling/repl/src/ens.rs
Line: 13:26
Comment:
**ENS namehash not normalized**
`namehash` hashes raw label bytes (`label.as_bytes()`), but ENS names are effectively case-insensitive and IDNs require UTS-46 normalization/punycode before hashing. As-is, inputs like `Vitalik.eth` (or any internationalized ENS name) will compute a different namehash and resolution will reliably fail even if the name exists. Consider normalizing the name (lowercasing + UTS-46) before computing the namehash.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in d253b51 — labels are now lowercased before hashing.
| fn parse_address_from_abi_word(hex_str: &str) -> Result<String, String> { | ||
| let hex = hex_str.strip_prefix("0x").unwrap_or(hex_str); | ||
| if hex.len() < 40 { | ||
| return Err(format!("unexpected response length: 0x{hex}")); | ||
| } | ||
| // Last 40 hex chars = 20 bytes = address | ||
| let addr = &hex[hex.len() - 40..]; | ||
| Ok(format!("0x{addr}")) | ||
| } |
There was a problem hiding this comment.
ABI word parsing too lax
parse_address_from_abi_word only checks len() < 40 and then slices the last 40 hex chars. If eth_call returns empty 0x, revert data, or any non-standard length, this can mis-parse into a bogus address (or produce a confusing error). For ENS resolver resolver(bytes32) / addr(bytes32), the return should be a single 32-byte ABI word (64 hex chars); validating exact length and hex-ness before slicing avoids incorrect resolution results.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tooling/repl/src/ens.rs
Line: 79:87
Comment:
**ABI word parsing too lax**
`parse_address_from_abi_word` only checks `len() < 40` and then slices the last 40 hex chars. If `eth_call` returns empty `0x`, revert data, or any non-standard length, this can mis-parse into a bogus address (or produce a confusing error). For ENS resolver `resolver(bytes32)` / `addr(bytes32)`, the return should be a single 32-byte ABI word (64 hex chars); validating exact length and hex-ness before slicing avoids incorrect resolution results.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in d253b51 — now validates exactly 64 hex chars instead of >= 40.
- Deduplicate EIP-55 checksum: toChecksumAddress utility now delegates to ens::to_checksum_address instead of reimplementing it - Consolidate UTILITY_NAMES: single pub const in parser.rs, imported in completer.rs - Reduce commands test boilerplate with required_param/cmd_with_param helpers - Reduce e2e test boilerplate with MockServer struct and Drop-based cleanup - Remove redundant comments and unused variable
Add a new page under the developers section covering usage, supported namespaces, ENS resolution, utility functions, and dev-mode integration.
ilitteri
changed the title
- Remove needless borrow in ens.rs namehash (clippy::needless_borrows_for_generic_args) - Collapse nested if in formatter.rs inline_value (clippy::collapsible_if) - Regenerate tooling/Cargo.lock - Add repl subcommand to docs/CLI.md
The planner stage runs cargo-chef prepare which calls cargo metadata. Since the root Cargo.toml now depends on ethrex-repl (in tooling/repl/), the tooling directory must be present for metadata resolution to succeed.
iovoid
left a comment
iovoid
left a comment
There was a problem hiding this comment.
The eth-to-wei problem should be fixed, since many use cases need exact answers.
tooling/repl/src/repl.rs
Outdated
| "Usage: toWei <amount> <unit>\nUnits: wei, gwei, ether", | ||
| ); | ||
| } | ||
| let amount: f64 = match args[0].parse() { |
There was a problem hiding this comment.
f64 isn't precise enough, 1.1eth will give the wrong answer
There was a problem hiding this comment.
Fixed in ad81935 — replaced f64 arithmetic with pure integer math.
| "exit" | "quit" => { | ||
| println!("Bye!"); | ||
| std::process::exit(0); | ||
| } |
tooling/repl/src/ens.rs
Outdated
|
|
||
| use crate::client::RpcClient; | ||
|
|
||
| /// ENS registry contract address (same on mainnet, Goerli, Sepolia). |
There was a problem hiding this comment.
Fixed in d253b51 — removed Goerli from the comment.
…depth - Replace f64 arithmetic in toWei/fromWei with pure integer math to avoid precision loss (e.g. toWei 1.1 ether now returns the correct value) - Make .exit/.quit break the REPL loop instead of calling process::exit(0), so that save_history runs before shutdown - Return false early in is_balanced when brace/bracket depth goes negative mid-scan, preventing "}..." from being treated as balanced
…reference - Lowercase labels before hashing in namehash() per ENS name normalization rules (UTS-46), making resolution case-insensitive - Validate ABI-encoded words are exactly 64 hex chars instead of only checking >= 40, preventing bogus addresses from malformed responses - Remove defunct Goerli from ENS registry comment
- Use char-based iteration in truncate_middle instead of byte slicing to prevent panics on multi-byte UTF-8 characters - Widen normalize_uint from u64 to u128 to handle large gas/value fields - Validate closing RParen in parenthesized RPC args, returning an error instead of silently accepting incomplete input - Remove unused ParseError::ExpectedMethod variant
- Set a 30-second timeout on the reqwest client to prevent indefinite hangs on unresponsive endpoints - Add error_for_status() check to surface non-200 HTTP responses before attempting JSON parsing - Downgrade colored from 3.x to 2.1.0 to match the rest of the workspace
| } | ||
| "connect" => { | ||
| if let Some(url) = args.first() { | ||
| println!("Reconnecting to {url}..."); |
There was a problem hiding this comment.
It's not necessarily a reconnection.
| ); | ||
| } else { | ||
| println!("Current endpoint: {}", self.client.endpoint()); | ||
| println!("Usage: .connect <url>"); |
There was a problem hiding this comment.
nit: This prints "Reconnecting to {url}..." but does not actually reconnect — RpcClient::set_endpoint exists but is never called. Since execute_builtin takes &self (not &mut self), it cannot mutate the client without a structural change. Consider changing the message to something like "To change endpoint, restart with --endpoint {url}" to avoid confusion.
avilagaston9
left a comment
avilagaston9
left a comment
There was a problem hiding this comment.
Nice work! One thought: we should at least file an issue to add L2 RPC commands (e.g. the ethrex L2-specific namespace) so this REPL can also be used for debugging the rollup.
4 participants
Motivation
ethrex lacks an interactive way to query and debug JSON-RPC endpoints. Users currently have to use external tools like
curlor Foundry'scast. An integrated REPL provides a much faster feedback loop for development and debugging.Description
Adds the
ethrex-replcrate — an interactive REPL (Read-Eval-Print Loop) for Ethereum JSON-RPC, integrated as theethrex replsubcommand.Features
eth(30),debug(8),admin(4),net(2),web3(1),txpool(2)~/.ethrex/historytoWei,fromWei,toHex,fromHex,keccak256,toChecksumAddress,isAddress.help,.exit,.clear,.connect,.history-xflag (e.g.ethrex repl -x "eth.blockNumber")eth.getBalance vitalik.eth), resolved on-chain via the ENS registryTest suite (211 tests)
Documentation
tooling/repl/README.md— usage guide and architecture overviewdocs/developers/repl.md— official docs page linked from the book summaryCommits
cf442a0— Base REPL: parser, command registry, RPC client, completer, formatter, repl loopb4150f7— ENS name resolution for address arguments6c64db3— Comprehensive unit and e2e test suiteb324df8— Simplify and deduplicate test and utility code20a8cc3— README with usage guide and architecture overview02a8ad1— Official docs page atdocs/developers/repl.mdHow to Test