-
Notifications
You must be signed in to change notification settings - Fork 1
add second listener that forwards txs through TOR #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
Add Dual-Listener Architecture for Tor Transaction BroadcastingThis PR implements a bidirectional proxy system that enables secure transaction broadcasting through the Tor network while maintaining backward compatibility with existing RPC functionality. Key Changes1. Dual Server Architecture
Both servers run concurrently using 2. Tor Network Integration (
|
MOO Comment Analysis & SolutionsI've analyzed all the TODO (MOO) comments in the PR and proposed solutions with tradeoffs for each. 1. Variable Naming:
|
| Solution | Pros | Cons |
|---|---|---|
A. Rename to EL_CLIENT_URL |
✅ More accurate ✅ Future-proof |
❌ Breaking change for existing configs ❌ Less familiar to users |
B. Keep GETH_URL with alias |
✅ Backward compatible ✅ Clear migration path |
❌ Technical debt ❌ Maintains confusion |
| C. Support both with deprecation | ✅ Smooth migration ✅ User-friendly |
❌ More code to maintain ❌ Longer deprecation period |
Recommendation: Solution C - Support both with deprecation warning
let el_client_url = std::env::var("EL_CLIENT_URL")
.or_else(|_| {
if let Ok(url) = std::env::var("GETH_URL") {
warn\!("GETH_URL is deprecated, use EL_CLIENT_URL instead");
Ok(url)
} else {
Ok("http://127.0.0.1:8545".to_string())
}
})
.unwrap();2. Default Port: 8545 vs 8656
Location: src/main.rs:49
.unwrap_or_else(|_| "http://127.0.0.1:8656".to_string()); // MOO: figure out default addr\!Problem: Changed from standard 8545 to 8656 - unclear why
Port Analysis:
- 8545: Standard Ethereum HTTP-RPC port (widely expected)
- 8656: Non-standard (possibly custom setup?)
- 8551: Engine API port (post-merge, authenticated)
Solutions:
| Port | Use Case | Pros | Cons |
|---|---|---|---|
| 8545 | Standard HTTP RPC | ✅ Industry standard ✅ User expectation |
❌ May conflict with local node |
| 8656 | Custom setup | ✅ Avoids conflicts | ❌ Non-standard ❌ Confusing |
| Make it required | No default | ✅ Explicit config ✅ No wrong assumptions |
❌ Less convenient |
Recommendation: Use 8545 (standard) with clear documentation
.unwrap_or_else(|_| {
warn\!("EL_CLIENT_URL not set, defaulting to http://127.0.0.1:8545");
"http://127.0.0.1:8545".to_string()
})3. Empty Onion Peers Handling ⚠️ CRITICAL
Location: src/main.rs:56-62
// MOO: how we want to handle case with no onion peers?
let onion_peers: Vec<_> = std::env::var("ONION_PEERS")
.unwrap_or_else(|_| {
"ethereumbbdyhyy33d4f3frmsxm6anm6bdrffvzft4kdk5p43odcvaid.onion:8545".to_string()
})
.split(',')
.map(|s| s.to_string())
.collect();Problem: What if no peers configured or all fail to connect?
Solutions:
| Solution | Behavior | Pros | Cons |
|---|---|---|---|
| A. Fail Fast (current in tor.rs) | Return error if no peers | ✅ Explicit failure ✅ No silent degradation |
❌ Service won't start ❌ Less resilient |
| B. Fallback to Direct | Route through local node | ✅ Service stays up ✅ Degraded mode |
❌ Privacy leak ❌ Silent failure |
| C. Make Optional | Allow empty, reject sends | ✅ Flexible deployment ✅ Clear error messages |
❌ Runtime failures ❌ More error handling |
| D. Dynamic Peer Discovery | Bootstrap from known peers | ✅ Self-healing ✅ Resilient |
❌ Complex implementation ❌ Trust assumptions |
Recommendation: Solution C - Make optional with clear error messages
// In InboundState
pub struct InboundState {
pub proxy_state: ProxyState,
pub onion_peers: Option<Onion>, // Make optional
}
// In handle_inbound
if is_send_method(&request.method) {
match &state.onion_peers {
Some(onion) => Ok(Json(onion.send_request(&request, 3).await?)),
None => Err(ProxyError::ServiceUnavailable(
"Tor network unavailable - transaction broadcasting disabled".to_string()
))
}
}Alternative: Environment-driven mode
enum TorMode {
Required, // Fail if no peers
Optional, // Warn and continue without Tor
Fallback, // Use direct connection if Tor fails
}
let tor_mode = std::env::var("TOR_MODE")
.unwrap_or_else(|_| "required".to_string());4. Additional Send Methods for Tor Routing
Location: src/whitelist.rs:58
methods.insert("eth_sendRawTransaction");
// MOO: do we want to handle other send methods ?Problem: Only eth_sendRawTransaction routes through Tor. What about other submission methods?
Candidate Methods:
| Method | Should Route Through Tor? | Privacy Risk |
|---|---|---|
eth_sendRawTransaction |
✅ Yes (current) | High - reveals IP |
eth_sendTransaction |
High - but requires unlocked account | |
eth_sendBundle |
✅ Yes | High - MEV bundle submission |
eth_sendPrivateTransaction |
✅ Yes | Very High - explicitly private |
eth_submitWork |
✅ Yes (mining) | Medium - reveals miner IP |
debug_* / admin_* |
❌ No | N/A - should be blocked anyway |
Solutions:
| Approach | Pros | Cons |
|---|---|---|
| A. Conservative (current) | ✅ Simple ✅ Predictable |
❌ Incomplete privacy ❌ Manual updates needed |
| B. Comprehensive List | ✅ Better privacy coverage ✅ Future-proof |
❌ May route unnecessary traffic ❌ Performance overhead |
| C. Configurable | ✅ Flexible ✅ User choice |
❌ Complex config ❌ Easy to misconfigure |
| D. Pattern-based | ✅ Catch all send variants ✅ Automatic |
❌ May over-match ❌ Less explicit |
Recommendation: Solution B - Comprehensive list
static SEND_METHODS: Lazy<HashSet<&'static str>> = Lazy::new(|| {
let mut methods = HashSet::new();
// Transaction submission (primary use case)
methods.insert("eth_sendRawTransaction");
methods.insert("eth_sendTransaction");
// MEV-related submissions
methods.insert("eth_sendBundle");
methods.insert("eth_sendPrivateTransaction");
methods.insert("flashbots_sendBundle");
methods.insert("flashbots_sendPrivateTransaction");
// Mining submissions (if supporting PoW)
methods.insert("eth_submitWork");
methods.insert("eth_submitHashrate");
methods
});5. Module Organization for is_send_method()
Location: src/whitelist.rs:72
/// Check if a method is send tx method
/// MOO: move this to other module?
pub fn is_send_method(method: &str) -> bool {
SEND_METHODS.contains(method)
}Problem: whitelist.rs is for allowed methods, but is_send_method() is for routing logic
Solutions:
| Location | Justification | Pros | Cons |
|---|---|---|---|
A. Keep in whitelist.rs |
It's method classification | ✅ No refactor needed ✅ Related to method filtering |
❌ Mixed concerns ❌ Module confusion |
B. Move to proxy.rs |
Used only in proxy | ✅ Collocated with usage ✅ Clear ownership |
❌ Not reusable ❌ Business logic in handler |
C. New routing.rs module |
Dedicated routing logic | ✅ Clean separation ✅ Extensible |
❌ Overhead for small function ❌ More files |
D. New method_types.rs |
Method categorization | ✅ Semantic clarity ✅ Reusable taxonomy |
❌ Generic naming |
Recommendation: Solution C - Create routing.rs for future extensibility
// src/routing.rs
use once_cell::sync::Lazy;
use std::collections::HashSet;
/// Routing decision for RPC methods
pub enum MethodRoute {
TorNetwork, // Must go through Tor
Direct, // Can go direct to EL client
Blocked, // Not allowed
}
static TOR_METHODS: Lazy<HashSet<&'static str>> = Lazy::new(|| {
// ... send methods
});
pub fn get_route_for_method(method: &str) -> MethodRoute {
if \!is_method_allowed(method) {
return MethodRoute::Blocked;
}
if TOR_METHODS.contains(method) {
return MethodRoute::TorNetwork;
}
MethodRoute::Direct
}This enables future enhancements like:
- Method-specific Tor circuits
- Per-method retry policies
- Dynamic routing based on network conditions
- A/B testing routing strategies
Summary Recommendations
| Issue | Recommended Solution | Priority |
|---|---|---|
| Variable naming | Support both GETH_URL + EL_CLIENT_URL with deprecation |
Medium |
| Default port | Use 8545 (standard) | High |
| Empty peers | Optional Tor with clear error messages | Critical |
| Send methods | Expand to comprehensive list | High |
| Module structure | Create routing.rs module |
Low |
Happy to implement any of these if helpful!
|
^^^ nice summary Regarding The trick is to listen default JSON-RPC port |
|
regarding tests: while existing functionality is not broken, my code lacks test coverage ('cause I'm not quire sure how to test it properly); I hand-tested it for basic scenario and it works, but it's obviously not enough I will be glad to receive recommendations on how to test this new feature |
|
What is this PR exactly trying to do? COuld you explain a bit? |
one part is kinda similar to https://github.com/CPerezz/torpc?tab=readme-ov-file#flashbots-integration and a second listener is a .onion hidden address listener? |
|
Yep, I try to cover two scenarios using torpc:
|
No description provided.