From a3713c9652d8db6f5f1269836ce127abdd361c03 Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Thu, 15 Oct 2026 22:51:25 -0500 Subject: [PATCH] smite-scenarios: panic on unreachable initial commitment errors Now that AcceptChannelOracle validates the channel parameters prior to saving the negotiation entry, it is impossible for new_initial_commitments() to fail. push_msat is always less than the funding amount, which is always less than MAX_MONEY. This also enables us to remove the final two tests that still use negotiation injection. --- smite-scenarios/src/executor.rs | 41 +++++++++++++++------------ smite-scenarios/src/executor/tests.rs | 30 -------------------- smite-scenarios/src/scenarios/ir.rs | 6 ---- 3 files changed, 23 insertions(+), 54 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index fe58d6ae..e244b7c0 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -226,10 +226,6 @@ pub enum ExecuteError { #[error("funding: {0}")] InsufficientFunds(#[from] smite::channel_tx::InsufficientFunds), - /// Failed to construct the initial commitment state. - #[error("commitment: {0}")] - Commitment(#[from] smite::channel_tx::CommitmentError), - /// The target broke a protocol invariant. Surfaced to the scenario as a /// failure; see [`Violation`] for the full catalog of target-bug findings. #[error(transparent)] @@ -479,7 +475,7 @@ impl Executor { &mut self.channel_states, &mut self.negotiations, &self.mined_txids, - )?; + ); let encoded = Message::FundingCreated(fc).encode(); log::debug!( "[{:?}] SendFundingCreated: {} bytes", @@ -828,6 +824,13 @@ fn build_open_channel(variables: &[Option], inputs: &[usize]) -> OpenC /// /// If the negotiation for `temporary_channel_id` is incomplete, emits a /// `funding_created` with the derived outpoint and an all-zero signature. +/// +/// # Panics +/// +/// Panics if the negotiated parameters cannot form the initial commitments. +/// This should be unreachable: the commitments are only built once the +/// negotiation has an `accept_channel`, which is only recorded after +/// `AcceptChannelOracle` has validated the `open_channel` parameters. #[allow(clippy::too_many_lines)] fn build_funding_created( variables: &[Option], @@ -835,7 +838,7 @@ fn build_funding_created( channel_states: &mut HashMap, negotiations: &mut HashMap, mined_txids: &HashSet, -) -> Result { +) -> FundingCreated { let funding_tx = resolve_funding_transaction(variables, inputs[0]); let opener_funding_privkey_bytes = resolve_private_key(variables, inputs[1]); let opener_htlc_basepoint_privkey_bytes = resolve_private_key(variables, inputs[2]); @@ -852,23 +855,23 @@ fn build_funding_created( // we cannot build the commitment to sign, so fall back to an unsigned // `funding_created` and leave `channel_states` untouched. let Some(pending) = negotiations.get(&temporary_channel_id) else { - return Ok(FundingCreated { + return FundingCreated { temporary_channel_id, funding_txid: funding_outpoint.txid, funding_output_index, signature: Signature::from_compact(&[0u8; 64]) .expect("zero bytes parse as a signature"), - }); + }; }; let open_channel = &pending.open_channel; let Some(accept_channel) = pending.accept_channel.as_ref() else { - return Ok(FundingCreated { + return FundingCreated { temporary_channel_id, funding_txid: funding_outpoint.txid, funding_output_index, signature: Signature::from_compact(&[0u8; 64]) .expect("zero bytes parse as a signature"), - }); + }; }; let opener_funding_privkey = @@ -903,12 +906,14 @@ fn build_funding_created( minimum_depth: accept_channel.minimum_depth, }; - let commitments = config.new_initial_commitments( - open_channel.push_msat, - open_channel.feerate_per_kw, - open_channel.first_per_commitment_point, - accept_channel.first_per_commitment_point, - )?; + let commitments = config + .new_initial_commitments( + open_channel.push_msat, + open_channel.feerate_per_kw, + open_channel.first_per_commitment_point, + accept_channel.first_per_commitment_point, + ) + .expect("valid initial commitment amounts"); let holder = HolderIdentity { side: Side::Opener, funding_privkey: opener_funding_privkey, @@ -963,12 +968,12 @@ fn build_funding_created( pending.funding_built = true; } - Ok(FundingCreated { + FundingCreated { temporary_channel_id, funding_txid: funding_outpoint.txid, funding_output_index, signature, - }) + } } /// Builds a `ChannelReady` from 3 input variables (wire order). diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index b682200e..49ab83b4 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1061,36 +1061,6 @@ fn execute_send_funding_created_after_funding_built_does_not_track_channel() { assert!(fx.channel_states().contains_key(&funding_channel_id())); } -#[test] -fn execute_send_funding_created_push_exceeds_funding() { - // A negotiated push_msat larger than the funding amount surfaces the - // commitment construction error. - let mut negotiation = sample_funding_negotiation(); - negotiation.open_channel.push_msat = 20_000_000_000; - let err = Fixture::new() - .with_negotiation(negotiation) - .run_err(&send_funding_created_program()); - assert!(matches!( - err, - ExecuteError::Commitment(smite::channel_tx::CommitmentError::PushExceedsFunding) - )); -} - -#[test] -fn execute_send_funding_created_funding_msat_overflow() { - // A negotiated funding_satoshis of u64::MAX overflows when converted to - // millisatoshis. - let mut negotiation = sample_funding_negotiation(); - negotiation.open_channel.funding_satoshis = u64::MAX; - let err = Fixture::new() - .with_negotiation(negotiation) - .run_err(&send_funding_created_program()); - assert!(matches!( - err, - ExecuteError::Commitment(smite::channel_tx::CommitmentError::FundingMsatOverflow) - )); -} - #[test] fn execute_send_funding_created_no_open_channel() { // No negotiation exists for this temporary_channel_id, so we get a diff --git a/smite-scenarios/src/scenarios/ir.rs b/smite-scenarios/src/scenarios/ir.rs index 827bfff0..1bf51193 100644 --- a/smite-scenarios/src/scenarios/ir.rs +++ b/smite-scenarios/src/scenarios/ir.rs @@ -99,12 +99,6 @@ impl> Scenario for IrScenario { // the available UTXOs can't cover. Not a bug in the target. log::debug!("[{:?}] insufficient funds: {e}", start.elapsed()); } - Err(ExecuteError::Commitment(e)) => { - // The mutator generated a funding amount/push_msat combination - // that can't form a valid initial commitment transaction. Not a - // bug in the target. - log::debug!("[{:?}] invalid commitment: {e}", start.elapsed()); - } Err(ExecuteError::Violation(v)) => { let violation = v.to_string(); if T::is_violation_known(&violation) {