Take the full funding transaction from the user on generation 2021-03-check-tx
authorMatt Corallo <git@bluematt.me>
Fri, 26 Mar 2021 22:07:24 +0000 (18:07 -0400)
committerMatt Corallo <git@bluematt.me>
Fri, 9 Apr 2021 23:59:06 +0000 (19:59 -0400)
Instead of relying on the user to ensure the funding transaction is
correct (and panicing when it is confirmed), we should check it is
correct when it is generated. By taking the full funding transaciton
from the user on generation, we can also handle broadcasting for
them instead of doing so via an event.

background-processor/src/lib.rs
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/events.rs

index c3db4d55ade989e13fb8ac228ac77d6460d0502f..49be55cf9b60c59e3f646501c4a0741ccb1009df 100644 (file)
@@ -189,7 +189,7 @@ mod tests {
                        $node_a.node.handle_accept_channel(&$node_b.node.get_our_node_id(), InitFeatures::known(), &get_event_msg!($node_b, MessageSendEvent::SendAcceptChannel, $node_a.node.get_our_node_id()));
                        let events = $node_a.node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 1);
-                       let (temporary_channel_id, tx, funding_output) = match events[0] {
+                       let (temporary_channel_id, tx) = match events[0] {
                                Event::FundingGenerationReady { ref temporary_channel_id, ref channel_value_satoshis, ref output_script, user_channel_id } => {
                                        assert_eq!(*channel_value_satoshis, $channel_value);
                                        assert_eq!(user_channel_id, 42);
@@ -197,13 +197,12 @@ mod tests {
                                        let tx = Transaction { version: 1 as i32, lock_time: 0, input: Vec::new(), output: vec![TxOut {
                                                value: *channel_value_satoshis, script_pubkey: output_script.clone(),
                                        }]};
-                                       let funding_outpoint = OutPoint { txid: tx.txid(), index: 0 };
-                                       (*temporary_channel_id, tx, funding_outpoint)
+                                       (*temporary_channel_id, tx)
                                },
                                _ => panic!("Unexpected event"),
                        };
 
-                       $node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
+                       $node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
                        $node_b.node.handle_funding_created(&$node_a.node.get_our_node_id(), &get_event_msg!($node_a, MessageSendEvent::SendFundingCreated, $node_b.node.get_our_node_id()));
                        $node_a.node.handle_funding_signed(&$node_b.node.get_our_node_id(), &get_event_msg!($node_b, MessageSendEvent::SendFundingSigned, $node_a.node.get_our_node_id()));
                        tx
index 7c5875ac1ef62d31590241228027fd75b954aba2..3feeaf46d5e1636d01736a904c9a52aeb8f2b5fa 100644 (file)
@@ -397,7 +397,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                                value: *channel_value_satoshis, script_pubkey: output_script.clone(),
                                        }]};
                                        funding_output = OutPoint { txid: tx.txid(), index: 0 };
-                                       $source.funding_transaction_generated(&temporary_channel_id, funding_output);
+                                       $source.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
                                        channel_txn.push(tx);
                                } else { panic!("Wrong event type"); }
                        }
@@ -420,12 +420,6 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                        };
                        $source.handle_funding_signed(&$dest.get_our_node_id(), &funding_signed);
 
-                       {
-                               let events = $source.get_and_clear_pending_events();
-                               assert_eq!(events.len(), 1);
-                               if let events::Event::FundingBroadcastSafe { .. } = events[0] {
-                               } else { panic!("Wrong event type"); }
-                       }
                        funding_output
                } }
        }
index bffb3e8e214b3a1d6d4d4d75c3e46e4dc6dabfa5..3acf7ba53ca7c642c6d3bcf4685480eab1131741 100644 (file)
@@ -51,7 +51,7 @@ use bitcoin::secp256k1::Secp256k1;
 use std::cell::RefCell;
 use std::collections::{HashMap, hash_map};
 use std::cmp;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 use std::sync::atomic::{AtomicU64,AtomicUsize,Ordering};
 
 #[inline]
@@ -116,9 +116,13 @@ impl FeeEstimator for FuzzEstimator {
        }
 }
 
-struct TestBroadcaster {}
+struct TestBroadcaster {
+       txn_broadcasted: Mutex<Vec<Transaction>>,
+}
 impl BroadcasterInterface for TestBroadcaster {
-       fn broadcast_transaction(&self, _tx: &Transaction) {}
+       fn broadcast_transaction(&self, tx: &Transaction) {
+               self.txn_broadcasted.lock().unwrap().push(tx.clone());
+       }
 }
 
 #[derive(Clone)]
@@ -340,7 +344,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                Err(_) => return,
        };
 
-       let broadcast = Arc::new(TestBroadcaster{});
+       let broadcast = Arc::new(TestBroadcaster{ txn_broadcasted: Mutex::new(Vec::new()) });
        let monitor = Arc::new(chainmonitor::ChainMonitor::new(None, broadcast.clone(), Arc::clone(&logger), fee_est.clone(), Arc::new(TestPersister{})));
 
        let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), counter: AtomicU64::new(0) });
@@ -370,7 +374,6 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
        let mut payments_sent = 0;
        let mut pending_funding_generation: Vec<([u8; 32], u64, Script)> = Vec::new();
        let mut pending_funding_signatures = HashMap::new();
-       let mut pending_funding_relay = Vec::new();
 
        loop {
                match get_slice!(1)[0] {
@@ -513,18 +516,19 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                                                        continue 'outer_loop;
                                                }
                                        };
-                                       channelmanager.funding_transaction_generated(&funding_generation.0, funding_output.clone());
+                                       channelmanager.funding_transaction_generated(&funding_generation.0, tx.clone()).unwrap();
                                        pending_funding_signatures.insert(funding_output, tx);
                                }
                        },
                        11 => {
-                               if !pending_funding_relay.is_empty() {
-                                       loss_detector.connect_block(&pending_funding_relay[..]);
+                               let mut txn = broadcast.txn_broadcasted.lock().unwrap();
+                               if !txn.is_empty() {
+                                       loss_detector.connect_block(&txn[..]);
                                        for _ in 2..100 {
                                                loss_detector.connect_block(&[]);
                                        }
                                }
-                               for tx in pending_funding_relay.drain(..) {
+                               for tx in txn.drain(..) {
                                        loss_detector.funding_txn.push(tx);
                                }
                        },
@@ -566,9 +570,6 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                                Event::FundingGenerationReady { temporary_channel_id, channel_value_satoshis, output_script, .. } => {
                                        pending_funding_generation.push((temporary_channel_id, channel_value_satoshis, output_script));
                                },
-                               Event::FundingBroadcastSafe { funding_txo, .. } => {
-                                       pending_funding_relay.push(pending_funding_signatures.remove(&funding_txo).unwrap());
-                               },
                                Event::PaymentReceived { payment_hash, payment_secret, amt } => {
                                        //TODO: enhance by fetching random amounts from fuzz input?
                                        payments_received.push((payment_hash, payment_secret, amt));
index a4cc5a02ae2e9da9406ba3bd59ea4c2d667e3dec..dc37e3f6c7421295691f34f1b821d1fe9cb2f1ea 100644 (file)
@@ -1825,7 +1825,7 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
 
        let (temporary_channel_id, funding_tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 43);
 
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
 
        *nodes[1].chain_monitor.update_ret.lock().unwrap() = Some(Err(ChannelMonitorUpdateErr::TemporaryFailure));
@@ -1846,14 +1846,9 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
        check_added_monitors!(nodes[0], 0);
 
        let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 43);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events.len(), 0);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)[0].txid(), funding_output.txid);
 
        if confirm_a_first {
                confirm_transaction(&nodes[0], &funding_tx);
index ca1887d8b2595c1485f7e120841b9368019237c5..174aecd3ba036f46dc9acdc95d2b287ef536d812 100644 (file)
@@ -409,9 +409,9 @@ pub(super) struct Channel<Signer: Sign> {
        counterparty_forwarding_info: Option<CounterpartyForwardingInfo>,
 
        pub(crate) channel_transaction_parameters: ChannelTransactionParameters,
+       funding_transaction: Option<Transaction>,
 
        counterparty_cur_commitment_point: Option<PublicKey>,
-
        counterparty_prev_commitment_point: Option<PublicKey>,
        counterparty_node_id: PublicKey,
 
@@ -603,8 +603,9 @@ impl<Signer: Sign> Channel<Signer> {
                                counterparty_parameters: None,
                                funding_outpoint: None
                        },
-                       counterparty_cur_commitment_point: None,
+                       funding_transaction: None,
 
+                       counterparty_cur_commitment_point: None,
                        counterparty_prev_commitment_point: None,
                        counterparty_node_id,
 
@@ -844,8 +845,9 @@ impl<Signer: Sign> Channel<Signer> {
                                }),
                                funding_outpoint: None
                        },
-                       counterparty_cur_commitment_point: Some(msg.first_per_commitment_point),
+                       funding_transaction: None,
 
+                       counterparty_cur_commitment_point: Some(msg.first_per_commitment_point),
                        counterparty_prev_commitment_point: None,
                        counterparty_node_id,
 
@@ -1608,7 +1610,7 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Handles a funding_signed message from the remote end.
        /// If this call is successful, broadcast the funding transaction (and not before!)
-       pub fn funding_signed<L: Deref>(&mut self, msg: &msgs::FundingSigned, last_block_hash: BlockHash, logger: &L) -> Result<ChannelMonitor<Signer>, ChannelError> where L::Target: Logger {
+       pub fn funding_signed<L: Deref>(&mut self, msg: &msgs::FundingSigned, last_block_hash: BlockHash, logger: &L) -> Result<(ChannelMonitor<Signer>, Transaction), ChannelError> where L::Target: Logger {
                if !self.is_outbound() {
                        return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
                }
@@ -1670,7 +1672,7 @@ impl<Signer: Sign> Channel<Signer> {
                self.cur_holder_commitment_transaction_number -= 1;
                self.cur_counterparty_commitment_transaction_number -= 1;
 
-               Ok(channel_monitor)
+               Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap()))
        }
 
        pub fn funding_locked(&mut self, msg: &msgs::FundingLocked) -> Result<(), ChannelError> {
@@ -2771,20 +2773,21 @@ impl<Signer: Sign> Channel<Signer> {
        /// Indicates that the latest ChannelMonitor update has been committed by the client
        /// successfully and we should restore normal operation. Returns messages which should be sent
        /// to the remote side.
-       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, bool, Option<msgs::FundingLocked>) where L::Target: Logger {
+       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L) -> (Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option<Transaction>, Option<msgs::FundingLocked>) where L::Target: Logger {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
 
-               let needs_broadcast_safe = self.channel_state & (ChannelState::FundingSent as u32) != 0 && self.is_outbound();
+               let funding_broadcastable = if self.channel_state & (ChannelState::FundingSent as u32) != 0 && self.is_outbound() {
+                       self.funding_transaction.take()
+               } else { None };
 
-               // Because we will never generate a FundingBroadcastSafe event when we're in
-               // MonitorUpdateFailed, if we assume the user only broadcast the funding transaction when
-               // they received the FundingBroadcastSafe event, we can only ever hit
-               // monitor_pending_funding_locked when we're an inbound channel which failed to persist the
-               // monitor on funding_created, and we even got the funding transaction confirmed before the
-               // monitor was persisted.
+               // We will never broadcast the funding transaction when we're in MonitorUpdateFailed (and
+               // we assume the user never directly broadcasts the funding transaction and waits for us to
+               // do it). Thus, we can only ever hit monitor_pending_funding_locked when we're an inbound
+               // channel which failed to persist the monitor on funding_created, and we got the funding
+               // transaction confirmed before the monitor was persisted.
                let funding_locked = if self.monitor_pending_funding_locked {
-                       assert!(!self.is_outbound(), "Funding transaction broadcast without FundingBroadcastSafe!");
+                       assert!(!self.is_outbound(), "Funding transaction broadcast by the local client before it should have - LDK didn't do it!");
                        self.monitor_pending_funding_locked = false;
                        let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
@@ -2801,7 +2804,7 @@ impl<Signer: Sign> Channel<Signer> {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) != 0 {
                        self.monitor_pending_revoke_and_ack = false;
                        self.monitor_pending_commitment_signed = false;
-                       return (None, None, RAACommitmentOrder::RevokeAndACKFirst, forwards, failures, needs_broadcast_safe, funding_locked);
+                       return (None, None, RAACommitmentOrder::RevokeAndACKFirst, forwards, failures, funding_broadcastable, funding_locked);
                }
 
                let raa = if self.monitor_pending_revoke_and_ack {
@@ -2815,11 +2818,11 @@ impl<Signer: Sign> Channel<Signer> {
                self.monitor_pending_commitment_signed = false;
                let order = self.resend_order.clone();
                log_trace!(logger, "Restored monitor updating resulting in {}{} commitment update and {} RAA, with {} first",
-                       if needs_broadcast_safe { "a funding broadcast safe, " } else { "" },
+                       if funding_broadcastable.is_some() { "a funding broadcastable, " } else { "" },
                        if commitment_update.is_some() { "a" } else { "no" },
                        if raa.is_some() { "an" } else { "no" },
                        match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
-               (raa, commitment_update, order, forwards, failures, needs_broadcast_safe, funding_locked)
+               (raa, commitment_update, order, forwards, failures, funding_broadcastable, funding_locked)
        }
 
        pub fn update_fee<F: Deref>(&mut self, fee_estimator: &F, msg: &msgs::UpdateFee) -> Result<(), ChannelError>
@@ -3734,7 +3737,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// Note that channel_id changes during this call!
        /// Do NOT broadcast the funding transaction until after a successful funding_signed call!
        /// If an Err is returned, it is a ChannelError::Close.
-       pub fn get_outbound_funding_created<L: Deref>(&mut self, funding_txo: OutPoint, logger: &L) -> Result<msgs::FundingCreated, ChannelError> where L::Target: Logger {
+       pub fn get_outbound_funding_created<L: Deref>(&mut self, funding_transaction: Transaction, funding_txo: OutPoint, logger: &L) -> Result<msgs::FundingCreated, ChannelError> where L::Target: Logger {
                if !self.is_outbound() {
                        panic!("Tried to create outbound funding_created message on an inbound channel!");
                }
@@ -3765,6 +3768,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                self.channel_state = ChannelState::FundingCreated as u32;
                self.channel_id = funding_txo.to_channel_id();
+               self.funding_transaction = Some(funding_transaction);
 
                Ok(msgs::FundingCreated {
                        temporary_channel_id,
@@ -4489,8 +4493,9 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                }
 
                self.channel_transaction_parameters.write(writer)?;
-               self.counterparty_cur_commitment_point.write(writer)?;
+               self.funding_transaction.write(writer)?;
 
+               self.counterparty_cur_commitment_point.write(writer)?;
                self.counterparty_prev_commitment_point.write(writer)?;
                self.counterparty_node_id.write(writer)?;
 
@@ -4659,6 +4664,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                };
 
                let channel_parameters = Readable::read(reader)?;
+               let funding_transaction = Readable::read(reader)?;
+
                let counterparty_cur_commitment_point = Readable::read(reader)?;
 
                let counterparty_prev_commitment_point = Readable::read(reader)?;
@@ -4731,8 +4738,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        counterparty_forwarding_info,
 
                        channel_transaction_parameters: channel_parameters,
-                       counterparty_cur_commitment_point,
+                       funding_transaction,
 
+                       counterparty_cur_commitment_point,
                        counterparty_prev_commitment_point,
                        counterparty_node_id,
 
@@ -5000,7 +5008,7 @@ mod tests {
                        value: 10000000, script_pubkey: output_script.clone(),
                }]};
                let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
-               let funding_created_msg = node_a_chan.get_outbound_funding_created(funding_outpoint, &&logger).unwrap();
+               let funding_created_msg = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap();
                let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, last_block_hash, &&logger).unwrap();
 
                // Node B --> Node A: funding signed
index 50f8ccbb74f4833175b00060fcd7ee882873c71b..2f7c17b58d75736dc688dbf8b586a72ea8634623 100644 (file)
@@ -19,6 +19,7 @@
 //!
 
 use bitcoin::blockdata::block::{Block, BlockHeader};
+use bitcoin::blockdata::transaction::Transaction;
 use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::network::constants::Network;
 
@@ -850,10 +851,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        /// Creates a new outbound channel to the given remote node and with the given value.
        ///
-       /// user_id will be provided back as user_channel_id in FundingGenerationReady and
-       /// FundingBroadcastSafe events to allow tracking of which events correspond with which
-       /// create_channel call. Note that user_channel_id defaults to 0 for inbound channels, so you
-       /// may wish to avoid using 0 for user_id here.
+       /// user_id will be provided back as user_channel_id in FundingGenerationReady events to allow
+       /// tracking of which events correspond with which create_channel call. Note that the
+       /// user_channel_id defaults to 0 for inbound channels, so you may wish to avoid using 0 for
+       /// user_id here. user_id has no meaning inside of LDK, it is simply copied to events and
+       /// otherwise ignored.
        ///
        /// If successful, will generate a SendOpenChannel message event, so you should probably poll
        /// PeerManager::process_events afterwards.
@@ -1525,32 +1527,75 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        /// Call this upon creation of a funding transaction for the given channel.
        ///
-       /// Note that ALL inputs in the transaction pointed to by funding_txo MUST spend SegWit outputs
-       /// or your counterparty can steal your funds!
+       /// Returns an [`APIError::APIMisuseError`] if the funding_transaction spent non-SegWit outputs
+       /// or if no output was found which matches the parameters in [`Event::FundingGenerationReady`].
        ///
        /// Panics if a funding transaction has already been provided for this channel.
        ///
-       /// May panic if the funding_txo is duplicative with some other channel (note that this should
-       /// be trivially prevented by using unique funding transaction keys per-channel).
-       pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_txo: OutPoint) {
+       /// May panic if the output found in the funding transaction is duplicative with some other
+       /// channel (note that this should be trivially prevented by using unique funding transaction
+       /// keys per-channel).
+       ///
+       /// Do NOT broadcast the funding transaction yourself. When we have safely received our
+       /// counterparty's signature the funding transaction will automatically be broadcast via the
+       /// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed.
+       ///
+       /// Note that this includes RBF or similar transaction replacement strategies - lightning does
+       /// not currently support replacing a funding transaction on an existing channel. Instead,
+       /// create a new channel with a conflicting funding transaction.
+       pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
 
+               for inp in funding_transaction.input.iter() {
+                       if inp.witness.is_empty() {
+                               return Err(APIError::APIMisuseError {
+                                       err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
+                               });
+                       }
+               }
+
                let (chan, msg) = {
                        let (res, chan) = match self.channel_state.lock().unwrap().by_id.remove(temporary_channel_id) {
                                Some(mut chan) => {
-                                       (chan.get_outbound_funding_created(funding_txo, &self.logger)
+                                       let mut output_index = None;
+                                       let expected_spk = chan.get_funding_redeemscript().to_v0_p2wsh();
+                                       for (idx, outp) in funding_transaction.output.iter().enumerate() {
+                                               if outp.script_pubkey == expected_spk && outp.value == chan.get_value_satoshis() {
+                                                       if output_index.is_some() {
+                                                               return Err(APIError::APIMisuseError {
+                                                                       err: "Multiple outputs matched the expected script and value".to_owned()
+                                                               });
+                                                       }
+                                                       if idx > u16::max_value() as usize {
+                                                               return Err(APIError::APIMisuseError {
+                                                                       err: "Transaction had more than 2^16 outputs, which is not supported".to_owned()
+                                                               });
+                                                       }
+                                                       output_index = Some(idx as u16);
+                                               }
+                                       }
+                                       if output_index.is_none() {
+                                               return Err(APIError::APIMisuseError {
+                                                       err: "No output matched the script_pubkey and value in the FundingGenerationReady event".to_owned()
+                                               });
+                                       }
+                                       let funding_txo = OutPoint { txid: funding_transaction.txid(), index: output_index.unwrap() };
+
+                                       (chan.get_outbound_funding_created(funding_transaction, funding_txo, &self.logger)
                                                .map_err(|e| if let ChannelError::Close(msg) = e {
                                                        MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.force_shutdown(true), None)
                                                } else { unreachable!(); })
                                        , chan)
                                },
-                               None => return
+                               None => { return Err(APIError::ChannelUnavailable { err: "No such channel".to_owned() }) },
                        };
                        match handle_error!(self, res, chan.get_counterparty_node_id()) {
                                Ok(funding_msg) => {
                                        (chan, funding_msg)
                                },
-                               Err(_) => { return; }
+                               Err(_) => { return Err(APIError::ChannelUnavailable {
+                                       err: "Error deriving keys or signing initial commitment transactions - either our RNG or our counterparty's RNG is broken or the Signer refused to sign".to_owned()
+                               }) },
                        }
                };
 
@@ -1567,6 +1612,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                e.insert(chan);
                        }
                }
+               Ok(())
        }
 
        fn get_announcement_sigs(&self, chan: &Channel<Signer>) -> Option<msgs::AnnouncementSignatures> {
@@ -2359,7 +2405,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                return;
                        }
 
-                       let (raa, commitment_update, order, pending_forwards, mut pending_failures, needs_broadcast_safe, funding_locked) = channel.monitor_updating_restored(&self.logger);
+                       let (raa, commitment_update, order, pending_forwards, mut pending_failures, funding_broadcastable, funding_locked) = channel.monitor_updating_restored(&self.logger);
                        if !pending_forwards.is_empty() {
                                htlc_forwards.push((channel.get_short_channel_id().expect("We can't have pending forwards before funding confirmation"), funding_txo.clone(), pending_forwards));
                        }
@@ -2391,11 +2437,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        handle_cs!();
                                },
                        }
-                       if needs_broadcast_safe {
-                               pending_events.push(events::Event::FundingBroadcastSafe {
-                                       funding_txo: channel.get_funding_txo().unwrap(),
-                                       user_channel_id: channel.get_user_id(),
-                               });
+                       if let Some(tx) = funding_broadcastable {
+                               self.tx_broadcaster.broadcast_transaction(&tx);
                        }
                        if let Some(msg) = funding_locked {
                                pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
@@ -2529,7 +2572,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        }
 
        fn internal_funding_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> {
-               let (funding_txo, user_id) = {
+               let funding_tx = {
                        let last_block_hash = *self.last_block_hash.read().unwrap();
                        let mut channel_lock = self.channel_state.lock().unwrap();
                        let channel_state = &mut *channel_lock;
@@ -2538,23 +2581,19 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if chan.get().get_counterparty_node_id() != *counterparty_node_id {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.channel_id));
                                        }
-                                       let monitor = match chan.get_mut().funding_signed(&msg, last_block_hash, &self.logger) {
+                                       let (monitor, funding_tx) = match chan.get_mut().funding_signed(&msg, last_block_hash, &self.logger) {
                                                Ok(update) => update,
                                                Err(e) => try_chan_entry!(self, Err(e), channel_state, chan),
                                        };
                                        if let Err(e) = self.chain_monitor.watch_channel(chan.get().get_funding_txo().unwrap(), monitor) {
                                                return_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::RevokeAndACKFirst, false, false);
                                        }
-                                       (chan.get().get_funding_txo().unwrap(), chan.get().get_user_id())
+                                       funding_tx
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
                        }
                };
-               let mut pending_events = self.pending_events.lock().unwrap();
-               pending_events.push(events::Event::FundingBroadcastSafe {
-                       funding_txo,
-                       user_channel_id: user_id,
-               });
+               self.tx_broadcaster.broadcast_transaction(&funding_tx);
                Ok(())
        }
 
index e7f61c6a3dfeefb23f845b7a47d6ab0efa5abf60..2ebdac5d031f4a00b0efe2df418744a3b54199b9 100644 (file)
@@ -421,7 +421,7 @@ pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, '
 
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, channel_value, 42);
 
-       node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(node_a, 0);
 
        node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id()));
@@ -441,14 +441,11 @@ pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, '
        }
 
        let events_4 = node_a.node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
+
+       assert_eq!(node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(node_a.tx_broadcaster.txn_broadcasted.lock().unwrap()[0], tx);
+       node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
 
        tx
 }
index c7a79e32210639fddcd4c3d01ab16e6f311248a3..7c198e4803b97c7a5f1b3dbc5dad3705cbeadd97 100644 (file)
@@ -460,7 +460,7 @@ fn do_test_sanity_on_in_flight_opens(steps: u8) {
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
 
        if steps & 0x0f == 3 { return; }
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
        let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
 
@@ -484,14 +484,7 @@ fn do_test_sanity_on_in_flight_opens(steps: u8) {
        }
 
        let events_4 = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
 
        if steps & 0x0f == 6 { return; }
        create_chan_between_nodes_with_value_confirm_first(&nodes[0], &nodes[1], &tx, 2);
@@ -4320,7 +4313,7 @@ fn test_manager_serialize_deserialize_events() {
        let nodes_0_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
-       // Start creating a channel, but stop right before broadcasting the event message FundingBroadcastSafe
+       // Start creating a channel, but stop right before broadcasting the funding transaction
        let channel_value = 100000;
        let push_msat = 10001;
        let a_flags = InitFeatures::known();
@@ -4333,7 +4326,7 @@ fn test_manager_serialize_deserialize_events() {
 
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&node_a, channel_value, 42);
 
-       node_a.node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(node_a, 0);
 
        node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id()));
@@ -4351,7 +4344,7 @@ fn test_manager_serialize_deserialize_events() {
                assert_eq!(added_monitors[0].0, funding_output);
                added_monitors.clear();
        }
-       // Normally, this is where node_a would check for a FundingBroadcastSafe event, but the test de/serializes first instead
+       // Normally, this is where node_a would broadcast the funding transaction, but the test de/serializes first instead
 
        nodes.push(node_a);
        nodes.push(node_b);
@@ -4395,16 +4388,11 @@ fn test_manager_serialize_deserialize_events() {
        assert!(nodes[0].chain_monitor.watch_channel(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok());
        nodes[0].node = &nodes_0_deserialized;
 
-       // After deserializing, make sure the FundingBroadcastSafe event is still held by the channel manager
+       // After deserializing, make sure the funding_transaction is still held by the channel manager
        let events_4 = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].txid(), funding_output.txid);
 
        // Make sure the channel is functioning as though the de/serialization never happened
        assert_eq!(nodes[0].node.list_channels().len(), 1);
@@ -8347,9 +8335,9 @@ fn test_pre_lockin_no_chan_closed_update() {
        nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_chan_msg);
 
        // Move the first channel through the funding flow...
-       let (temporary_channel_id, _tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
+       let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], 100000, 42);
 
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
 
        let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -8631,7 +8619,7 @@ fn test_duplicate_chan_id() {
        // Move the first channel through the funding flow...
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(&nodes[0], 100000, 42);
 
-       nodes[0].node.funding_transaction_generated(&temporary_channel_id, funding_output);
+       nodes[0].node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(nodes[0], 0);
 
        let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
@@ -8680,7 +8668,7 @@ fn test_duplicate_chan_id() {
                let mut a_channel_lock = nodes[0].node.channel_state.lock().unwrap();
                let mut as_chan = a_channel_lock.by_id.get_mut(&open_chan_2_msg.temporary_channel_id).unwrap();
                let logger = test_utils::TestLogger::new();
-               as_chan.get_outbound_funding_created(funding_outpoint, &&logger).unwrap()
+               as_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap()
        };
        check_added_monitors!(nodes[0], 0);
        nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created);
@@ -8714,14 +8702,9 @@ fn test_duplicate_chan_id() {
        }
 
        let events_4 = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events_4.len(), 1);
-       match events_4[0] {
-               Event::FundingBroadcastSafe { ref funding_txo, user_channel_id } => {
-                       assert_eq!(user_channel_id, 42);
-                       assert_eq!(*funding_txo, funding_output);
-               },
-               _ => panic!("Unexpected event"),
-       };
+       assert_eq!(events_4.len(), 0);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].txid(), funding_output.txid);
 
        let (funding_locked, _) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx);
        let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked);
@@ -8758,6 +8741,7 @@ fn test_error_chans_closed() {
        nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &msgs::ErrorMessage { channel_id: chan_2.2, data: "ERR".to_owned() });
        check_added_monitors!(nodes[0], 1);
        check_closed_broadcast!(nodes[0], false);
+       assert_eq!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0).len(), 1);
        assert_eq!(nodes[0].node.list_usable_channels().len(), 2);
        assert!(nodes[0].node.list_usable_channels()[0].channel_id == chan_1.2 || nodes[0].node.list_usable_channels()[1].channel_id == chan_1.2);
        assert!(nodes[0].node.list_usable_channels()[0].channel_id == chan_3.2 || nodes[0].node.list_usable_channels()[1].channel_id == chan_3.2);
index 118503c040d09fafc6d6783a7f5b1edfe839430b..8b8cc1230372821aa52c0cd4e6edbdc01cb5f1bd 100644 (file)
@@ -16,7 +16,6 @@
 
 use ln::msgs;
 use ln::channelmanager::{PaymentPreimage, PaymentHash, PaymentSecret};
-use chain::transaction::OutPoint;
 use chain::keysinterface::SpendableOutputDescriptor;
 use util::ser::{Writeable, Writer, MaybeReadable, Readable};
 
@@ -49,16 +48,6 @@ pub enum Event {
                /// The value passed in to ChannelManager::create_channel
                user_channel_id: u64,
        },
-       /// Used to indicate that the client may now broadcast the funding transaction it created for a
-       /// channel. Broadcasting such a transaction prior to this event may lead to our counterparty
-       /// trivially stealing all funds in the funding transaction!
-       FundingBroadcastSafe {
-               /// The output, which was passed to ChannelManager::funding_transaction_generated, which is
-               /// now safe to broadcast.
-               funding_txo: OutPoint,
-               /// The value passed in to ChannelManager::create_channel
-               user_channel_id: u64,
-       },
        /// Indicates we've received money! Just gotta dig out that payment preimage and feed it to
        /// ChannelManager::claim_funds to get it....
        /// Note that if the preimage is not known or the amount paid is incorrect, you should call
@@ -140,19 +129,14 @@ impl Writeable for Event {
                                // We never write out FundingGenerationReady events as, upon disconnection, peers
                                // drop any channels which have not yet exchanged funding_signed.
                        },
-                       &Event::FundingBroadcastSafe { ref funding_txo, ref user_channel_id } => {
-                               1u8.write(writer)?;
-                               funding_txo.write(writer)?;
-                               user_channel_id.write(writer)?;
-                       },
                        &Event::PaymentReceived { ref payment_hash, ref payment_secret, ref amt } => {
-                               2u8.write(writer)?;
+                               1u8.write(writer)?;
                                payment_hash.write(writer)?;
                                payment_secret.write(writer)?;
                                amt.write(writer)?;
                        },
                        &Event::PaymentSent { ref payment_preimage } => {
-                               3u8.write(writer)?;
+                               2u8.write(writer)?;
                                payment_preimage.write(writer)?;
                        },
                        &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest,
@@ -161,7 +145,7 @@ impl Writeable for Event {
                                #[cfg(test)]
                                ref error_data,
                        } => {
-                               4u8.write(writer)?;
+                               3u8.write(writer)?;
                                payment_hash.write(writer)?;
                                rejected_by_dest.write(writer)?;
                                #[cfg(test)]
@@ -170,12 +154,12 @@ impl Writeable for Event {
                                error_data.write(writer)?;
                        },
                        &Event::PendingHTLCsForwardable { time_forwardable: _ } => {
-                               5u8.write(writer)?;
+                               4u8.write(writer)?;
                                // We don't write the time_fordwardable out at all, as we presume when the user
                                // deserializes us at least that much time has elapsed.
                        },
                        &Event::SpendableOutputs { ref outputs } => {
-                               6u8.write(writer)?;
+                               5u8.write(writer)?;
                                (outputs.len() as u64).write(writer)?;
                                for output in outputs.iter() {
                                        output.write(writer)?;
@@ -189,19 +173,15 @@ impl MaybeReadable for Event {
        fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Option<Self>, msgs::DecodeError> {
                match Readable::read(reader)? {
                        0u8 => Ok(None),
-                       1u8 => Ok(Some(Event::FundingBroadcastSafe {
-                                       funding_txo: Readable::read(reader)?,
-                                       user_channel_id: Readable::read(reader)?,
-                               })),
-                       2u8 => Ok(Some(Event::PaymentReceived {
+                       1u8 => Ok(Some(Event::PaymentReceived {
                                        payment_hash: Readable::read(reader)?,
                                        payment_secret: Readable::read(reader)?,
                                        amt: Readable::read(reader)?,
                                })),
-                       3u8 => Ok(Some(Event::PaymentSent {
+                       2u8 => Ok(Some(Event::PaymentSent {
                                        payment_preimage: Readable::read(reader)?,
                                })),
-                       4u8 => Ok(Some(Event::PaymentFailed {
+                       3u8 => Ok(Some(Event::PaymentFailed {
                                        payment_hash: Readable::read(reader)?,
                                        rejected_by_dest: Readable::read(reader)?,
                                        #[cfg(test)]
@@ -209,10 +189,10 @@ impl MaybeReadable for Event {
                                        #[cfg(test)]
                                        error_data: Readable::read(reader)?,
                                })),
-                       5u8 => Ok(Some(Event::PendingHTLCsForwardable {
+                       4u8 => Ok(Some(Event::PendingHTLCsForwardable {
                                        time_forwardable: Duration::from_secs(0)
                                })),
-                       6u8 => {
+                       5u8 => {
                                let outputs_len: u64 = Readable::read(reader)?;
                                let mut outputs = Vec::new();
                                for _ in 0..outputs_len {