refactor: implement vulnerability detection in Rust crate#16
refactor: implement vulnerability detection in Rust crate#16LORDBABUINO merged 11 commits intostealth-bitcoin:mainfrom
Conversation
9409a2b to
b2da99c
Compare
|
Previous force pushes were adapting the PR to reviewer requests. There will not be further changed unless a reviewer asks. @LORDBABUINO. |
e8c1e2a to
3311ec5
Compare
Removed from crates/stealth-bitcoincore and moved to bitcoincore as a standalone package. This change is part of the refactor to create separate packages for each component of the stealth project, allowing for better modularity and separation of concerns.
3311ec5 to
9eb6b8b
Compare
|
@LORDBABUINO Thanks for the great review! All but one requests implemented. |
|
Thanks for addressing all the review comments — the code is looking much better. The adoption of bitcoin::Txid, bitcoin::Address, and bitcoin::Amount goes beyond what I asked and is a welcome improvement. However, I need to flag again: please don't force push during review. We've talked about this before. I need to see what changed per comment without re-reviewing the entire PR. Use fixup commits during review. Now, I apologize for not raising this sooner — I should have run this comparison before the first round of review, which would have saved you from fixing code that may need further changes. After testing both branches against the same regtest environment, your code detects 7 fewer findings than main:
The biggest gap is CHANGE_DETECTION. Please investigate and fix these regressions. How to reproduce
cd backend/script
./setup.sh --fresh
python3 reproduce.py
git checkout main
mkdir -p crates/stealth-app/examplesCreate use std::env;
use stealth_bitcoincore::{BitcoinCoreConfig, BitcoinCoreRpc};
use stealth_core::engine::{AnalysisEngine, EngineSettings, ScanTarget};
fn main() {
let wallet = env::args().nth(1).unwrap_or_else(|| "alice".into());
let cookie = std::fs::read_to_string("backend/script/bitcoin-data/regtest/.cookie")
.expect("cannot read cookie file — is regtest running?");
let mut parts = cookie.trim().splitn(2, ':');
let user = parts.next().unwrap().to_string();
let pass = parts.next().unwrap().to_string();
let config = BitcoinCoreConfig {
network: "regtest".into(),
datadir: None,
rpchost: "127.0.0.1".into(),
rpcport: 18443,
rpcuser: Some(user),
rpcpassword: Some(pass),
};
let gateway = BitcoinCoreRpc::new(config).expect("failed to connect");
let engine = AnalysisEngine::new(&gateway, EngineSettings::default());
let report = engine.analyze(ScanTarget::WalletName(wallet)).expect("analysis failed");
println!("{}", serde_json::to_string_pretty(&report).unwrap());
}cargo run -p stealth-app --example scan_wallet 2>/dev/null | python3 -c "
import json, sys; r = json.load(sys.stdin)
from collections import Counter
types = Counter(f['type'] for f in r['findings'])
for t, c in sorted(types.items()): print(f' {t}: {c}')
print(f' TOTAL: {sum(types.values())}')
"
git checkout add-rust-detector
mkdir -p engine/examplesCreate use std::env;
use stealth_bitcoincore::BitcoinCoreRpc;
use stealth_engine::gateway::BlockchainGateway;
use stealth_engine::{EngineSettings, TxGraph};
fn main() {
let wallet = env::args().nth(1).unwrap_or_else(|| "alice".into());
let cookie = std::fs::read_to_string("backend/script/bitcoin-data/regtest/.cookie")
.expect("cannot read cookie file — is regtest running?");
let mut parts = cookie.trim().splitn(2, ':');
let user = parts.next().unwrap().to_string();
let pass = parts.next().unwrap().to_string();
let gateway = BitcoinCoreRpc::from_url("http://127.0.0.1:18443", Some(user), Some(pass))
.expect("failed to connect");
let history = gateway.scan_wallet(&wallet).expect("scan_wallet failed");
let graph = TxGraph::from_wallet_history(history);
let settings = EngineSettings::default();
let report = graph.detect_all(
&settings.config.thresholds,
settings.known_risky_txids.as_ref(),
settings.known_exchange_txids.as_ref(),
);
println!("{}", serde_json::to_string_pretty(&report).unwrap());
}cargo run --example scan_wallet 2>/dev/null | python3 -c "
import json, sys; r = json.load(sys.stdin)
from collections import Counter
types = Counter(f['type'] for f in r['findings'])
for t, c in sorted(types.items()): print(f' {t}: {c}')
print(f' TOTAL: {sum(types.values())}')
"Important: both runs must use the same regtest chain — don't run |
LORDBABUINO
left a comment
There was a problem hiding this comment.
Requesting changes — see my comment above for regression details and reproduction steps.
…nterfaces Track all addresses derived from wallet descriptors in WalletHistory, not only internal/change addresses. This gives TxGraph a complete address set to compare ownership against later.
Derive addresses from all wallet descriptors, external and internal, and persist both internal_addresses and derived_addresses on WalletHistory. This aligns the Rust path with the Python reference behavior.
…hestration Seed TxGraph ownership state from all derived addresses, not just addresses discovered from transaction outputs. This makes is_ours() recognize every descriptor-derived address and preserves internal/change tagging separately.
|
Nice catch @LORDBABUINO! My bad for not validating this properly The Rust However, reproducing in my machine I was able to get only 38 detections from reproduction in main branch versus your mentioned 39, suggesting we may be dealing with a different regtest state. I generated mine directly from given command. Command used in main and in my branch (using the exact same regtest): |
…nterfaces Tighten the derived_addresses doc comment to describe the field's purpose without referencing the Python implementation details.
Trim the scan_wallet comment so it only states the Rust-side behavior: derive every descriptor address, without the extra cross-reference to the Python path.
…hestration Shorten the TxGraph comment to describe seeding ownership from derived addresses without the removed Python-reference explanation.
|
These last 3 commits just remove some irrelevant comments accidentally left. |
Partially solves #10
This PR is a combination of rust refactors from @brenorb in #15 and me.
New crates:
This PR:
AnalysisEngine+BlockchainGatewayarchitecture so API, CLI, and library usage all go through one main scan path.TxGraphinto a prebuilt graph fromWalletHistory, with precomputed caches and a spending index for deterministic analysis.Reviewer Notes
In future PRs, a bitcoin core adapter, cli and api will be added. The only review possible at this point is running tests with
cargo test.