Skip to content

Commit 66c4f45

Browse files
committed
Ensure a 1:1 mapping of value sendable to send success in fuzzing
Now that the value available to send is expected to match the success or failure of sending exactly, we should assert this in the `chanmon_consistency` fuzzer. In the next commit we'll actually rip the checks out of `send_htlc` which will make this a somewhat less useful test, however fuzzing on this specific commit can help to reveal bugs.
1 parent 7edbf54 commit 66c4f45

File tree

2 files changed

+38
-11
lines changed

2 files changed

+38
-11
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ impl KeyProvider {
285285
}
286286

287287
#[inline]
288-
fn check_api_err(api_err: APIError) {
288+
fn check_api_err(api_err: APIError, sendable_bounds_violated: bool) {
289289
match api_err {
290290
APIError::APIMisuseError { .. } => panic!("We can't misuse the API"),
291291
APIError::FeeRateTooHigh { .. } => panic!("We can't send too much fee?"),
@@ -305,6 +305,7 @@ fn check_api_err(api_err: APIError) {
305305
_ if err.starts_with("Cannot send value that would put our exposure to dust HTLCs at") => {},
306306
_ => panic!("{}", err),
307307
}
308+
assert!(sendable_bounds_violated);
308309
},
309310
APIError::MonitorUpdateInProgress => {
310311
// We can (obviously) temp-fail a monitor update
@@ -313,17 +314,17 @@ fn check_api_err(api_err: APIError) {
313314
}
314315
}
315316
#[inline]
316-
fn check_payment_err(send_err: PaymentSendFailure) {
317+
fn check_payment_err(send_err: PaymentSendFailure, sendable_bounds_violated: bool) {
317318
match send_err {
318-
PaymentSendFailure::ParameterError(api_err) => check_api_err(api_err),
319+
PaymentSendFailure::ParameterError(api_err) => check_api_err(api_err, sendable_bounds_violated),
319320
PaymentSendFailure::PathParameterError(per_path_results) => {
320-
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err); } }
321+
for res in per_path_results { if let Err(api_err) = res { check_api_err(api_err, sendable_bounds_violated); } }
321322
},
322323
PaymentSendFailure::AllFailedResendSafe(per_path_results) => {
323-
for api_err in per_path_results { check_api_err(api_err); }
324+
for api_err in per_path_results { check_api_err(api_err, sendable_bounds_violated); }
324325
},
325326
PaymentSendFailure::PartialFailure { results, .. } => {
326-
for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
327+
for res in results { if let Err(api_err) = res { check_api_err(api_err, sendable_bounds_violated); } }
327328
},
328329
PaymentSendFailure::DuplicatePayment => panic!(),
329330
}
@@ -351,6 +352,11 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
351352
let mut payment_id = [0; 32];
352353
payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes());
353354
*payment_idx += 1;
355+
let (min_value_sendable, max_value_sendable) = source.list_usable_channels()
356+
.iter().find(|chan| chan.short_channel_id == Some(dest_chan_id))
357+
.map(|chan|
358+
(chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat))
359+
.unwrap_or((0, 0));
354360
if let Err(err) = source.send_payment_with_route(&Route {
355361
paths: vec![Path { hops: vec![RouteHop {
356362
pubkey: dest.get_our_node_id(),
@@ -362,9 +368,15 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
362368
}], blinded_tail: None }],
363369
payment_params: None,
364370
}, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) {
365-
check_payment_err(err);
371+
check_payment_err(err, amt > max_value_sendable || amt < min_value_sendable);
366372
false
367-
} else { true }
373+
} else {
374+
// Note that while the max is a strict upper-bound, we can occasionally send substantially
375+
// below the minimum, with some gap which is unusable immediately below the minimum. Thus,
376+
// we don't check against min_value_sendable here.
377+
assert!(amt <= max_value_sendable);
378+
true
379+
}
368380
}
369381
#[inline]
370382
fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, dest: &ChanMan, dest_chan_id: u64, amt: u64, payment_id: &mut u8, payment_idx: &mut u64) -> bool {
@@ -373,13 +385,19 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
373385
let mut payment_id = [0; 32];
374386
payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes());
375387
*payment_idx += 1;
388+
let (min_value_sendable, max_value_sendable) = source.list_usable_channels()
389+
.iter().find(|chan| chan.short_channel_id == Some(middle_chan_id))
390+
.map(|chan|
391+
(chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat))
392+
.unwrap_or((0, 0));
393+
let first_hop_fee = 50_000;
376394
if let Err(err) = source.send_payment_with_route(&Route {
377395
paths: vec![Path { hops: vec![RouteHop {
378396
pubkey: middle.get_our_node_id(),
379397
node_features: middle.node_features(),
380398
short_channel_id: middle_chan_id,
381399
channel_features: middle.channel_features(),
382-
fee_msat: 50000,
400+
fee_msat: first_hop_fee,
383401
cltv_expiry_delta: 100,
384402
},RouteHop {
385403
pubkey: dest.get_our_node_id(),
@@ -391,9 +409,16 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
391409
}], blinded_tail: None }],
392410
payment_params: None,
393411
}, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) {
394-
check_payment_err(err);
412+
let sent_amt = amt + first_hop_fee;
413+
check_payment_err(err, sent_amt < min_value_sendable || sent_amt > max_value_sendable);
395414
false
396-
} else { true }
415+
} else {
416+
// Note that while the max is a strict upper-bound, we can occasionally send substantially
417+
// below the minimum, with some gap which is unusable immediately below the minimum. Thus,
418+
// we don't check against min_value_sendable here.
419+
assert!(amt + first_hop_fee <= max_value_sendable);
420+
true
421+
}
397422
}
398423

399424
#[inline]

lightning/src/ln/channel.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6029,6 +6029,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
60296029
return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat)));
60306030
}
60316031

6032+
debug_assert!(amount_msat <= self.get_available_balances().next_outbound_htlc_limit_msat);
6033+
60326034
let need_holding_cell = (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateInProgress as u32)) != 0;
60336035
log_debug!(logger, "Pushing new outbound HTLC for {} msat {}", amount_msat,
60346036
if force_holding_cell { "into holding cell" }

0 commit comments

Comments
 (0)