diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index cd9c484da2a..e923ef882f2 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -89,7 +89,7 @@ struct FuzzRouter {} impl Router for FuzzRouter { fn find_route( &self, _payer: &PublicKey, _params: &RouteParameters, _first_hops: Option<&[&ChannelDetails]>, - _inflight_htlcs: &InFlightHtlcs + _inflight_htlcs: InFlightHtlcs ) -> Result { Err(msgs::LightningError { err: String::from("Not implemented"), diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 9caf9104034..1fbd7dbec88 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -131,7 +131,7 @@ struct FuzzRouter {} impl Router for FuzzRouter { fn find_route( &self, _payer: &PublicKey, _params: &RouteParameters, _first_hops: Option<&[&ChannelDetails]>, - _inflight_htlcs: &InFlightHtlcs + _inflight_htlcs: InFlightHtlcs ) -> Result { Err(msgs::LightningError { err: String::from("Not implemented"), diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index bf56a84ca46..751a43e0233 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -26,7 +26,7 @@ use crate::ln::chan_utils::{ use crate::ln::features::ChannelTypeFeatures; use crate::ln::PaymentPreimage; use crate::prelude::*; -use crate::sign::{ChannelSigner, EcdsaChannelSigner, SignerProvider}; +use crate::sign::{ChannelSigner, EcdsaChannelSigner, SignerProvider, WriteableEcdsaChannelSigner}; use crate::sync::Mutex; use crate::util::logger::Logger; @@ -102,9 +102,9 @@ impl AnchorDescriptor { } /// Derives the channel signer required to sign the anchor input. - pub fn derive_channel_signer(&self, signer_provider: &SP) -> ::Signer + pub fn derive_channel_signer(&self, signer_provider: &SP) -> S where - SP::Target: SignerProvider + SP::Target: SignerProvider { let mut signer = signer_provider.derive_channel_signer( self.channel_derivation_parameters.value_satoshis, @@ -211,9 +211,9 @@ impl HTLCDescriptor { } /// Derives the channel signer required to sign the HTLC input. - pub fn derive_channel_signer(&self, signer_provider: &SP) -> ::Signer + pub fn derive_channel_signer(&self, signer_provider: &SP) -> S where - SP::Target: SignerProvider + SP::Target: SignerProvider { let mut signer = signer_provider.derive_channel_signer( self.channel_derivation_parameters.value_satoshis, @@ -464,12 +464,12 @@ pub trait CoinSelectionSource { /// which UTXOs to double spend is left to the implementation, but it must strive to keep the /// set of other claims being double spent to a minimum. fn select_confirmed_utxos( - &self, claim_id: ClaimId, must_spend: &[Input], must_pay_to: &[TxOut], + &self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &[TxOut], target_feerate_sat_per_1000_weight: u32, ) -> Result; /// Signs and provides the full witness for all inputs within the transaction known to the /// trait (i.e., any provided via [`CoinSelectionSource::select_confirmed_utxos`]). - fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()>; + fn sign_tx(&self, tx: Transaction) -> Result; } /// An alternative to [`CoinSelectionSource`] that can be implemented and used along [`Wallet`] to @@ -483,7 +483,7 @@ pub trait WalletSource { /// Signs and provides the full [`TxIn::script_sig`] and [`TxIn::witness`] for all inputs within /// the transaction known to the wallet (i.e., any provided via /// [`WalletSource::list_confirmed_utxos`]). - fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()>; + fn sign_tx(&self, tx: Transaction) -> Result; } /// A wrapper over [`WalletSource`] that implements [`CoinSelection`] by preferring UTXOs that would @@ -600,7 +600,7 @@ where L::Target: Logger { fn select_confirmed_utxos( - &self, claim_id: ClaimId, must_spend: &[Input], must_pay_to: &[TxOut], + &self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &[TxOut], target_feerate_sat_per_1000_weight: u32, ) -> Result { let utxos = self.source.list_confirmed_utxos()?; @@ -629,7 +629,7 @@ where .or_else(|_| do_coin_selection(true, true)) } - fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()> { + fn sign_tx(&self, tx: Transaction) -> Result { self.source.sign_tx(tx) } } @@ -726,7 +726,7 @@ where satisfaction_weight: commitment_tx.weight() as u64 + ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT, }]; let coin_selection = self.utxo_source.select_confirmed_utxos( - claim_id, &must_spend, &[], anchor_target_feerate_sat_per_1000_weight, + claim_id, must_spend, &[], anchor_target_feerate_sat_per_1000_weight, )?; let mut anchor_tx = Transaction { @@ -748,7 +748,8 @@ where let unsigned_tx_weight = anchor_tx.weight() as u64 - (anchor_tx.input.len() as u64 * EMPTY_SCRIPT_SIG_WEIGHT); log_debug!(self.logger, "Signing anchor transaction {}", anchor_txid); - self.utxo_source.sign_tx(&mut anchor_tx)?; + anchor_tx = self.utxo_source.sign_tx(anchor_tx)?; + let signer = anchor_descriptor.derive_channel_signer(&self.signer_provider); let anchor_sig = signer.sign_holder_anchor_input(&anchor_tx, 0, &self.secp)?; anchor_tx.input[0].witness = anchor_descriptor.tx_input_witness(&anchor_sig); @@ -799,20 +800,24 @@ where log_debug!(self.logger, "Peforming coin selection for HTLC transaction targeting {} sat/kW", target_feerate_sat_per_1000_weight); + #[cfg(debug_assertions)] + let must_spend_satisfaction_weight = + must_spend.iter().map(|input| input.satisfaction_weight).sum::(); let coin_selection = self.utxo_source.select_confirmed_utxos( - claim_id, &must_spend, &htlc_tx.output, target_feerate_sat_per_1000_weight, + claim_id, must_spend, &htlc_tx.output, target_feerate_sat_per_1000_weight, )?; #[cfg(debug_assertions)] let total_satisfaction_weight = coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::() + - must_spend.iter().map(|input| input.satisfaction_weight).sum::(); + must_spend_satisfaction_weight; self.process_coin_selection(&mut htlc_tx, coin_selection); #[cfg(debug_assertions)] let unsigned_tx_weight = htlc_tx.weight() as u64 - (htlc_tx.input.len() as u64 * EMPTY_SCRIPT_SIG_WEIGHT); log_debug!(self.logger, "Signing HTLC transaction {}", htlc_tx.txid()); - self.utxo_source.sign_tx(&mut htlc_tx)?; + htlc_tx = self.utxo_source.sign_tx(htlc_tx)?; + let mut signers = BTreeMap::new(); for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() { let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 546dc6c5bcd..30e718dccd6 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -669,7 +669,7 @@ impl OutboundPayments { let route = router.find_route_with_id( &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params, - Some(&first_hops.iter().collect::>()), &inflight_htlcs(), + Some(&first_hops.iter().collect::>()), inflight_htlcs(), payment_hash, payment_id, ).map_err(|_| RetryableSendFailure::RouteNotFound)?; @@ -712,7 +712,7 @@ impl OutboundPayments { let route = match router.find_route_with_id( &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params, - Some(&first_hops.iter().collect::>()), &inflight_htlcs(), + Some(&first_hops.iter().collect::>()), inflight_htlcs(), payment_hash, payment_id, ) { Ok(route) => route, diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index b23b6364182..2ae606106c9 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -3218,7 +3218,7 @@ fn do_claim_from_closed_chan(fail_payment: bool) { final_value_msat: 10_000_000, }; let mut route = nodes[0].router.find_route(&nodes[0].node.get_our_node_id(), &route_params, - None, &nodes[0].node.compute_inflight_htlcs()).unwrap(); + None, nodes[0].node.compute_inflight_htlcs()).unwrap(); // Make sure the route is ordered as the B->D path before C->D route.paths.sort_by(|a, _| if a.hops[0].pubkey == nodes[1].node.get_our_node_id() { std::cmp::Ordering::Less } else { std::cmp::Ordering::Greater }); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 9242e98b1ab..f0822ed5b38 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -64,7 +64,7 @@ impl< G: Deref>, L: Deref, S: Deref, SP: Sized, Sc: Sco payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&ChannelDetails]>, - inflight_htlcs: &InFlightHtlcs + inflight_htlcs: InFlightHtlcs ) -> Result { let random_seed_bytes = { let mut locked_random_seed_bytes = self.random_seed_bytes.lock().unwrap(); @@ -73,7 +73,7 @@ impl< G: Deref>, L: Deref, S: Deref, SP: Sized, Sc: Sco }; find_route( payer, params, &self.network_graph, first_hops, &*self.logger, - &ScorerAccountingForInFlightHtlcs::new(self.scorer.lock().deref_mut(), inflight_htlcs), + &ScorerAccountingForInFlightHtlcs::new(self.scorer.lock().deref_mut(), &inflight_htlcs), &self.score_params, &random_seed_bytes ) @@ -85,13 +85,13 @@ pub trait Router { /// Finds a [`Route`] between `payer` and `payee` for a payment with the given values. fn find_route( &self, payer: &PublicKey, route_params: &RouteParameters, - first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: &InFlightHtlcs + first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: InFlightHtlcs ) -> Result; /// Finds a [`Route`] between `payer` and `payee` for a payment with the given values. Includes /// `PaymentHash` and `PaymentId` to be able to correlate the request with a specific payment. fn find_route_with_id( &self, payer: &PublicKey, route_params: &RouteParameters, - first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: &InFlightHtlcs, + first_hops: Option<&[&ChannelDetails]>, inflight_htlcs: InFlightHtlcs, _payment_hash: PaymentHash, _payment_id: PaymentId ) -> Result { self.find_route(payer, route_params, first_hops, inflight_htlcs) @@ -112,7 +112,7 @@ pub struct ScorerAccountingForInFlightHtlcs<'a, S: Score, SP: impl<'a, S: Score, SP: Sized> ScorerAccountingForInFlightHtlcs<'a, S, SP> { /// Initialize a new `ScorerAccountingForInFlightHtlcs`. - pub fn new(scorer: &'a mut S, inflight_htlcs: &'a InFlightHtlcs) -> Self { + pub fn new(scorer: &'a mut S, inflight_htlcs: &'a InFlightHtlcs) -> Self { ScorerAccountingForInFlightHtlcs { scorer, inflight_htlcs diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index d83adaf97f6..615cc1a19eb 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -175,7 +175,7 @@ pub trait WriteableScore<'a>: LockableScore<'a> + Writeable {} #[cfg(not(c_bindings))] impl<'a, T> WriteableScore<'a> for T where T: LockableScore<'a> + Writeable {} -/// This is not exported to bindings users +#[cfg(not(c_bindings))] impl<'a, T: 'a + Score> LockableScore<'a> for Mutex { type Score = T; type Locked = MutexGuard<'a, T>; @@ -185,6 +185,7 @@ impl<'a, T: 'a + Score> LockableScore<'a> for Mutex { } } +#[cfg(not(c_bindings))] impl<'a, T: 'a + Score> LockableScore<'a> for RefCell { type Score = T; type Locked = RefMut<'a, T>; @@ -255,21 +256,7 @@ impl<'a, T: 'a + Score> Deref for MultiThreadedScoreLock<'a, T> { } } -#[cfg(c_bindings)] -/// This is not exported to bindings users -impl<'a, T: Writeable> Writeable for RefMut<'a, T> { - fn write(&self, writer: &mut W) -> Result<(), io::Error> { - T::write(&**self, writer) - } -} -#[cfg(c_bindings)] -/// This is not exported to bindings users -impl<'a, S: Writeable> Writeable for MutexGuard<'a, S> { - fn write(&self, writer: &mut W) -> Result<(), io::Error> { - S::write(&**self, writer) - } -} /// Proposed use of a channel passed as a parameter to [`Score::channel_penalty_msat`]. #[derive(Clone, Copy, Debug, PartialEq)] diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index bf5aa02bb41..65c0483a59c 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -112,13 +112,13 @@ impl<'a> TestRouter<'a> { impl<'a> Router for TestRouter<'a> { fn find_route( &self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&channelmanager::ChannelDetails]>, - inflight_htlcs: &InFlightHtlcs + inflight_htlcs: InFlightHtlcs ) -> Result { if let Some((find_route_query, find_route_res)) = self.next_routes.lock().unwrap().pop_front() { assert_eq!(find_route_query, *params); if let Ok(ref route) = find_route_res { let mut binding = self.scorer.lock().unwrap(); - let scorer = ScorerAccountingForInFlightHtlcs::new(binding.deref_mut(), inflight_htlcs); + let scorer = ScorerAccountingForInFlightHtlcs::new(binding.deref_mut(), &inflight_htlcs); for path in &route.paths { let mut aggregate_msat = 0u64; for (idx, hop) in path.hops.iter().rev().enumerate() { @@ -1105,20 +1105,20 @@ impl TestWalletSource { } impl WalletSource for TestWalletSource { - fn list_confirmed_utxos(&self) -> Result, ()> { + fn list_confirmed_utxos(&self) -> Result, ()> { Ok(self.utxos.borrow().clone()) - } + } - fn get_change_script(&self) -> Result { + fn get_change_script(&self) -> Result { let public_key = bitcoin::PublicKey::new(self.secret_key.public_key(&self.secp)); Ok(Script::new_p2pkh(&public_key.pubkey_hash())) - } + } - fn sign_tx(&self, tx: &mut Transaction) -> Result<(), ()> { + fn sign_tx(&self, mut tx: Transaction) -> Result { let utxos = self.utxos.borrow(); for i in 0..tx.input.len() { if let Some(utxo) = utxos.iter().find(|utxo| utxo.outpoint == tx.input[i].previous_output) { - let sighash = SighashCache::new(&*tx) + let sighash = SighashCache::new(&tx) .legacy_signature_hash(i, &utxo.output.script_pubkey, EcdsaSighashType::All as u32) .map_err(|_| ())?; let sig = self.secp.sign_ecdsa(&sighash.as_hash().into(), &self.secret_key); @@ -1129,6 +1129,6 @@ impl WalletSource for TestWalletSource { .into_script(); } } - Ok(()) - } + Ok(tx) + } }