Repository navigation
Conversation
|
👋 I see @tnull was un-assigned. |
|
Nice find! Perhaps we could also add the same setters for the UPD: it's already there. LGTM |
| /// | ||
| /// Defaults to 1 000 000 - 10 000 000 msat (1k - 10k sats). | ||
| pub fn amount_range_msat(&mut self, min_msat: u64, max_msat: u64) -> &mut Self { | ||
| debug_assert!(min_msat <= max_msat, "min_msat must not exceed max_msat"); |
There was a problem hiding this comment.
would be better to return a Result/Error here imo
There was a problem hiding this comment.
@joschisan Would be good to address this before we can merge.
There was a problem hiding this comment.
Done: amount_range_msat now returns Result<&mut Self, BuildError> and rejects min_msat > max_msat with a new BuildError::InvalidProbeAmountRange, and the uniffi set_amount_range_msat throws it ([Throws=BuildError]), like the other fallible builder setters.
The bounds each probe's amount is drawn from were baked-in crate defaults (1k - 10k sats). Nodes that mostly send larger payments want larger probes, since knowing a route passes 10k sats says little about whether it passes 500k - add amount_range_msat to the builder (plus the uniffi Arced variant and UDL) and thread it into both built-in strategies. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013fi458uGXp51nh5wMNhwWY
7fda0ca to
b805f7d
Compare
| use lightning_invoice::DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA; | ||
| use lightning_types::features::{ChannelFeatures, NodeFeatures}; | ||
|
|
||
| use crate::builder::BuildError; |
There was a problem hiding this comment.
No, I don't think this should import and use BuildError here. Let's introduce a dedicated error type.
The amount each probe is drawn from was fixed to the crate defaults of 1k - 10k sats. Nodes that mostly send larger payments want larger probes, since knowing a route passes 10k sats says little about whether it passes 500k. This adds
amount_range_msattoProbingConfigBuilder(andset_amount_range_msaton the uniffi variant) and threads it into both built-in strategies; the defaults are unchanged.I am using this in prod already for a custom fedimint gateway.