Feat/mixed probing strategy by dzdidi · Pull Request #10 · synonymdev/scorer

dzdidi · 2026-04-22T10:17:45Z

Based on last bitkit call:

mixed probing strategy
significnat refactoring

Copilot

Pull request overview

Adds a mixed probing strategy (peer-list + random-graph) and refactors probing/event handling into dedicated modules to support the new behavior.

Changes:

Introduces src/probing.rs with a unified probing loop supporting both peer-list probing and random-graph probing modes.
Extracts LDK event handling into src/events.rs, wiring probe success/failure events into a shared ProbeTracker.
Extends probing configuration (random_min_amount_msat, random_nodes_per_interval) and updates examples/docs accordingly.

Reviewed changes

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

Show a summary per file

File	Description
src/probing.rs	New probing module implementing mixed probing strategy and probe tracking.
src/events.rs	New event handler module; forwards probe events to `ProbeTracker` and contains prior event logic.
src/main.rs	Wires in `events` + `probing` modules; updates event handler and probing loop startup.
src/config.rs	Adds new probing config fields + defaults, and maps them into CLI config.
src/cli.rs	Extends `ProbingConfig` to carry new random-probing settings.
README.md	Documents random-graph probing settings and updates example config snippet.
config.example.toml	Adds random probing fields to the example probing config.
prober_config.json.example	Adds random probing fields to the JSON example config.

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

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

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

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

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

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

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

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

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

Copilot · 2026-04-23T13:35:26Z

+pub(crate) struct EventContext {
+	pub(crate) channel_manager: Arc<ChannelManager>,
+	pub(crate) bitcoind_client: Arc<BitcoindClient>,
+	pub(crate) network_graph: Arc<NetworkGraph>,
+	pub(crate) keys_manager: Arc<crate::KeysManager>,
+	pub(crate) bump_tx_event_handler: Arc<BumpTxEventHandler>,
+	pub(crate) peer_manager: Arc<PeerManager>,


EventContext.keys_manager is typed as Arc<crate::KeysManager>, but the crate does not appear to define/export a KeysManager type alias (it’s only imported in main.rs). This will fail to compile. Use Arc<lightning::sign::KeysManager> (and import KeysManager here), or re-export KeysManager from the crate root (e.g., pub(crate) use lightning::sign::KeysManager;) and keep the current path consistent across modules.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

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

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

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

Copilot

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

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

Copilot · 2026-04-24T12:03:02Z

+				for (id, payment) in outbound.payments.iter_mut() {
+					if *id == payment_id {
+						payment.preimage = Some(payment_preimage);
+						payment.status = HTLCStatus::Succeeded;
+						println!(
+							"\nEVENT: successfully sent payment of {} millisatoshis{} from \
+									 payment hash {} with preimage {}",
+							payment.amt_msat,
+							if let Some(fee) = fee_paid_msat {
+								format!(" (fee {} msat)", fee)
+							} else {
+								"".to_string()
+							},
+							payment_hash,
+							payment_preimage
+						);
+						print!("> ");
+						let _ = std::io::stdout().flush();
+					}


In Event::PaymentSent, iterating over all outbound payments to find a single payment_id is unnecessary and continues scanning even after a match is found. Using outbound.payments.get_mut(&payment_id) (and returning early/breaking after updating) would avoid O(n) work and makes it clearer that only one entry should be updated/logged.

Suggested change

for (id, payment) in outbound.payments.iter_mut() {

if *id == payment_id {

payment.preimage = Some(payment_preimage);

payment.status = HTLCStatus::Succeeded;

println!(

"\nEVENT: successfully sent payment of {} millisatoshis{} from \

payment hash {} with preimage {}",

payment.amt_msat,

if let Some(fee) = fee_paid_msat {

format!(" (fee {} msat)", fee)

} else {

"".to_string()

},

payment_hash,

payment_preimage

);

print!("> ");

let _ = std::io::stdout().flush();

}

if let Some(payment) = outbound.payments.get_mut(&payment_id) {

payment.preimage = Some(payment_preimage);

payment.status = HTLCStatus::Succeeded;

println!(

"\nEVENT: successfully sent payment of {} millisatoshis{} from \

payment hash {} with preimage {}",

payment.amt_msat,

if let Some(fee) = fee_paid_msat {

format!(" (fee {} msat)", fee)

} else {

"".to_string()

},

payment_hash,

payment_preimage

);

print!("> ");

let _ = std::io::stdout().flush();

Copilot · 2026-04-24T12:03:02Z

+				let requested_count =
+					probe_config.random_nodes_per_interval.try_into().unwrap_or(usize::MAX);
+				let random_recipients = {
+					let graph_read_only = network_graph.read_only();
+					let mut rng = thread_rng();
+					reservoir_sample(
+						graph_read_only.nodes().unordered_iter().filter_map(|(node_id, _)| {
+							if *node_id == our_node_id {
+								return None;
+							}
+							node_id.as_pubkey().ok()
+						}),
+						requested_count,
+						&mut rng,


requested_count falls back to usize::MAX on conversion failure, which can lead to attempting extremely large allocations/iterations when sampling graph nodes (especially on 32-bit targets). Consider clamping random_nodes_per_interval to a sane upper bound (and/or the current graph node count) and returning a config validation error when it exceeds that bound instead of using usize::MAX.

Copilot · 2026-04-24T12:03:03Z

+	let mut reservoir = Vec::with_capacity(sample_size);
+	let mut seen = 0usize;
+
+	for candidate in candidates {
+		seen += 1;
+		if reservoir.len() < sample_size {
+			reservoir.push(candidate);
+			continue;


reservoir_sample pre-allocates with Vec::with_capacity(sample_size). If sample_size is user-controlled (via config) and large, this can cause very large allocations or OOM. Consider bounding the capacity (e.g., min(sample_size, max_cap)), and/or building the reservoir without pre-allocating to the full requested size when it exceeds the candidate population.

Copilot

Pull request overview

Copilot reviewed 36 out of 37 changed files in this pull request and generated 2 comments.

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

Copilot · 2026-04-24T12:48:44Z

+	pub(crate) channel_manager: Arc<ChannelManager>,
+	pub(crate) bitcoind_client: Arc<BitcoindClient>,
+	pub(crate) network_graph: Arc<NetworkGraph>,
+	pub(crate) keys_manager: Arc<crate::KeysManager>,
+	pub(crate) bump_tx_event_handler: Arc<BumpTxEventHandler>,
+	pub(crate) peer_manager: Arc<PeerManager>,


EventContext stores keys_manager as Arc<crate::KeysManager>, but the crate doesn't define/re-export a KeysManager type (it's lightning::sign::KeysManager in main.rs). This looks like a compile error; change the field type/import to use lightning::sign::KeysManager (or re-export it from the crate if that’s the intent).

Copilot · 2026-04-24T12:48:45Z

+				let requested_count =
+					probe_config.random_nodes_per_interval.try_into().unwrap_or(usize::MAX);
+				let random_recipients = {
+					let graph_read_only = network_graph.read_only();


requested_count falls back to usize::MAX when random_nodes_per_interval doesn’t fit in usize. That value is then used as Vec::with_capacity(sample_size) inside reservoir_sample, which can attempt an enormous allocation and crash/abort. Clamp requested_count to a sane upper bound (e.g. graph node count) and avoid using usize::MAX as a sentinel capacity.

Suggested change

let requested_count =

probe_config.random_nodes_per_interval.try_into().unwrap_or(usize::MAX);

let random_recipients = {

let graph_read_only = network_graph.read_only();

let random_recipients = {

let graph_read_only = network_graph.read_only();

let available_node_count = graph_read_only

.nodes()

.unordered_iter()

.filter(|(node_id, _)| **node_id != our_node_id)

.count();

let requested_count = probe_config

.random_nodes_per_interval

.try_into()

.unwrap_or(available_node_count)

.min(available_node_count);

ovitrif

lgtm

…st probes Run graph-wide random probing in parallel with whitelisted peer probing to improve coverage while keeping targeted checks.

Drop deprecated JSON config path and keep TOML as the single config source for simpler startup behavior and maintenance.

Restructure CLI/probing/events modules and related plumbing to improve separation of concerns and make future changes safer.

dzdidi requested a review from Copilot April 22, 2026 10:17

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread README.md

Comment thread config.example.toml

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

Comment thread src/main.rs Outdated

dzdidi requested a review from Copilot April 22, 2026 11:14

Copilot started reviewing on behalf of dzdidi April 22, 2026 11:15 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

Comment thread README.md

Comment thread src/probing.rs Outdated

dzdidi force-pushed the feat/mixed-probing-strategy branch from 4d36f8f to e3f3f6b Compare April 23, 2026 12:43

dzdidi changed the base branch from main to tmp/file-rename April 23, 2026 12:43

dzdidi requested a review from Copilot April 23, 2026 12:49

Copilot started reviewing on behalf of dzdidi April 23, 2026 12:49 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

dzdidi requested a review from Copilot April 23, 2026 13:12

Copilot started reviewing on behalf of dzdidi April 23, 2026 13:13 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread src/probing.rs Outdated

Comment thread src/probing.rs Outdated

dzdidi requested a review from Copilot April 23, 2026 13:30

Copilot started reviewing on behalf of dzdidi April 23, 2026 13:30 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

dzdidi requested a review from Copilot April 23, 2026 13:55

Copilot started reviewing on behalf of dzdidi April 23, 2026 13:56 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread src/probing.rs Outdated

dzdidi force-pushed the feat/mixed-probing-strategy branch from a3eb9d1 to 54948c8 Compare April 24, 2026 10:27

dzdidi changed the base branch from tmp/file-rename to fix/no-route-error April 24, 2026 10:28

dzdidi requested a review from Copilot April 24, 2026 11:22

Copilot started reviewing on behalf of dzdidi April 24, 2026 11:22 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread src/probing/runner.rs

Comment thread src/probing/runner.rs

Comment thread src/main.rs

Comment thread config.example.toml

dzdidi requested a review from Copilot April 24, 2026 11:58

Copilot started reviewing on behalf of dzdidi April 24, 2026 11:59 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

dzdidi requested a review from Copilot April 24, 2026 12:44

Copilot started reviewing on behalf of dzdidi April 24, 2026 12:45 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

ovitrif approved these changes Apr 28, 2026

View reviewed changes

Base automatically changed from fix/no-route-error to main May 5, 2026 09:29

dzdidi force-pushed the feat/mixed-probing-strategy branch from 889548f to 764d876 Compare May 5, 2026 09:37

dzdidi added 3 commits May 5, 2026 12:41

feat(probing): mixed probing strategy with parallel graph and whiteli…

a2624eb

…st probes Run graph-wide random probing in parallel with whitelisted peer probing to improve coverage while keeping targeted checks.

chore(config): remove legacy JSON config support

14bb3dc

Drop deprecated JSON config path and keep TOML as the single config source for simpler startup behavior and maintenance.

refactor: modularize cli probing and event flow

eb62d81

Restructure CLI/probing/events modules and related plumbing to improve separation of concerns and make future changes safer.

dzdidi force-pushed the feat/mixed-probing-strategy branch from 764d876 to eb62d81 Compare May 5, 2026 09:52

dzdidi merged commit b6dcc17 into main May 5, 2026

dzdidi deleted the feat/mixed-probing-strategy branch May 5, 2026 09:54

Conversation

dzdidi commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

dzdidi commented Apr 22, 2026 •

edited

Loading