Merge pull request #1203 from lightning-signer/2021-12-value-to-self
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 8 Dec 2021 02:24:32 +0000 (02:24 +0000)
committerGitHub <noreply@github.com>
Wed, 8 Dec 2021 02:24:32 +0000 (02:24 +0000)
Getter for the total channel balance

21 files changed:
.github/workflows/build.yml
lightning-persister/src/lib.rs
lightning/src/chain/chainmonitor.rs
lightning/src/chain/channelmonitor.rs
lightning/src/lib.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/features.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/monitor_tests.rs
lightning/src/ln/msgs.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/reorg_tests.rs
lightning/src/ln/shutdown_tests.rs
lightning/src/util/errors.rs
lightning/src/util/events.rs
lightning/src/util/ser.rs
lightning/src/util/test_utils.rs

index 260767333f3c599d878eec70683e8875f5195e7d..4b9f23ff091b718d1d918f9e6b43922cf4070685 100644 (file)
@@ -172,7 +172,7 @@ jobs:
           done
       - name: Upload coverage
         if: matrix.coverage
-        uses: codecov/codecov-action@v1
+        uses: codecov/codecov-action@v2
         with:
           # Could you use this to fake the coverage report for your PR? Sure.
           # Will anyone be impressed by your amazing coverage? No
index eba0692485a551403b0ae40dc4e74909a2c1bb71..b853b5796b6e7e226fb2efae1d00495ef5de389d 100644 (file)
@@ -191,8 +191,7 @@ mod tests {
        use lightning::{check_closed_broadcast, check_closed_event, check_added_monitors};
        use lightning::ln::features::InitFeatures;
        use lightning::ln::functional_test_utils::*;
-       use lightning::ln::msgs::ErrorAction;
-       use lightning::util::events::{ClosureReason, Event, MessageSendEventsProvider, MessageSendEvent};
+       use lightning::util::events::{ClosureReason, MessageSendEventsProvider};
        use lightning::util::test_utils;
        use std::fs;
        #[cfg(target_os = "windows")]
index 3c8f9bb561b2456c7c3822f123efdd0f6539b67f..9db666bb03d0ff4e706069d3eba5576258eabb1d 100644 (file)
@@ -639,8 +639,8 @@ where C::Target: chain::Filter,
                                let monitor = &monitor_state.monitor;
                                log_trace!(self.logger, "Updating ChannelMonitor for channel {}", log_funding_info!(monitor));
                                let update_res = monitor.update_monitor(&update, &self.broadcaster, &self.fee_estimator, &self.logger);
-                               if let Err(e) = &update_res {
-                                       log_error!(self.logger, "Failed to update ChannelMonitor for channel {}: {:?}", log_funding_info!(monitor), e);
+                               if update_res.is_err() {
+                                       log_error!(self.logger, "Failed to update ChannelMonitor for channel {}.", log_funding_info!(monitor));
                                }
                                // Even if updating the monitor returns an error, the monitor's state will
                                // still be changed. So, persist the updated monitor despite the error.
@@ -727,10 +727,18 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
 
 #[cfg(test)]
 mod tests {
-       use ::{check_added_monitors, get_local_commitment_txn};
+       use bitcoin::BlockHeader;
+       use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
+       use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
+       use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
+       use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
+       use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
+       use ln::channelmanager::PaymentSendFailure;
        use ln::features::InitFeatures;
        use ln::functional_test_utils::*;
-       use util::events::MessageSendEventsProvider;
+       use ln::msgs::ChannelMessageHandler;
+       use util::errors::APIError;
+       use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
        use util::test_utils::{OnRegisterOutput, TxOutReference};
 
        /// Tests that in-block dependent transactions are processed by `block_connected` when not
@@ -775,4 +783,179 @@ mod tests {
                nodes[1].node.get_and_clear_pending_msg_events();
                nodes[1].node.get_and_clear_pending_events();
        }
+
+       #[test]
+       fn test_async_ooo_offchain_updates() {
+               // Test that if we have multiple offchain updates being persisted and they complete
+               // out-of-order, the ChainMonitor waits until all have completed before informing the
+               // ChannelManager.
+               let chanmon_cfgs = create_chanmon_cfgs(2);
+               let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+               let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+               create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+
+               // Route two payments to be claimed at the same time.
+               let payment_preimage_1 = route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
+               let payment_preimage_2 = route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
+
+               chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
+               chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
+
+               nodes[1].node.claim_funds(payment_preimage_1);
+               check_added_monitors!(nodes[1], 1);
+               nodes[1].node.claim_funds(payment_preimage_2);
+               check_added_monitors!(nodes[1], 1);
+
+               chanmon_cfgs[1].persister.set_update_ret(Ok(()));
+
+               let persistences = chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clone();
+               assert_eq!(persistences.len(), 1);
+               let (funding_txo, updates) = persistences.iter().next().unwrap();
+               assert_eq!(updates.len(), 2);
+
+               // Note that updates is a HashMap so the ordering here is actually random. This shouldn't
+               // fail either way but if it fails intermittently it's depending on the ordering of updates.
+               let mut update_iter = updates.iter();
+               nodes[1].chain_monitor.chain_monitor.channel_monitor_updated(*funding_txo, update_iter.next().unwrap().clone()).unwrap();
+               assert!(nodes[1].chain_monitor.release_pending_monitor_events().is_empty());
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+               nodes[1].chain_monitor.chain_monitor.channel_monitor_updated(*funding_txo, update_iter.next().unwrap().clone()).unwrap();
+
+               // Now manually walk the commitment signed dance - because we claimed two payments
+               // back-to-back it doesn't fit into the neat walk commitment_signed_dance does.
+
+               let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
+               expect_payment_sent_without_paths!(nodes[0], payment_preimage_1);
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &updates.commitment_signed);
+               check_added_monitors!(nodes[0], 1);
+               let (as_first_raa, as_first_update) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa);
+               check_added_monitors!(nodes[1], 1);
+               let bs_second_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_first_update);
+               check_added_monitors!(nodes[1], 1);
+               let bs_first_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+
+               nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_second_updates.update_fulfill_htlcs[0]);
+               expect_payment_sent_without_paths!(nodes[0], payment_preimage_2);
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_updates.commitment_signed);
+               check_added_monitors!(nodes[0], 1);
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa);
+               expect_payment_path_successful!(nodes[0]);
+               check_added_monitors!(nodes[0], 1);
+               let (as_second_raa, as_second_update) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa);
+               check_added_monitors!(nodes[1], 1);
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_update);
+               check_added_monitors!(nodes[1], 1);
+               let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa);
+               expect_payment_path_successful!(nodes[0]);
+               check_added_monitors!(nodes[0], 1);
+       }
+
+       fn do_chainsync_pauses_events(block_timeout: bool) {
+               // When a chainsync monitor update occurs, any MonitorUpdates should be held before being
+               // passed upstream to a `ChannelManager` via `Watch::release_pending_monitor_events`. This
+               // tests that behavior, as well as some ways it might go wrong.
+               let chanmon_cfgs = create_chanmon_cfgs(2);
+               let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+               let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+               let channel = create_announced_chan_between_nodes(
+                       &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+
+               // Get a route for later and rebalance the channel somewhat
+               send_payment(&nodes[0], &[&nodes[1]], 10_000_000);
+               let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
+
+               // First route a payment that we will claim on chain and give the recipient the preimage.
+               let payment_preimage = route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
+               nodes[1].node.claim_funds(payment_preimage);
+               nodes[1].node.get_and_clear_pending_msg_events();
+               check_added_monitors!(nodes[1], 1);
+               let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
+               assert_eq!(remote_txn.len(), 2);
+
+               // Temp-fail the block connection which will hold the channel-closed event
+               chanmon_cfgs[0].persister.chain_sync_monitor_persistences.lock().unwrap().clear();
+               chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
+
+               // Connect B's commitment transaction, but only to the ChainMonitor/ChannelMonitor. The
+               // channel is now closed, but the ChannelManager doesn't know that yet.
+               let new_header = BlockHeader {
+                       version: 2, time: 0, bits: 0, nonce: 0,
+                       prev_blockhash: nodes[0].best_block_info().0,
+                       merkle_root: Default::default() };
+               nodes[0].chain_monitor.chain_monitor.transactions_confirmed(&new_header,
+                       &[(0, &remote_txn[0]), (1, &remote_txn[1])], nodes[0].best_block_info().1 + 1);
+               assert!(nodes[0].chain_monitor.release_pending_monitor_events().is_empty());
+               nodes[0].chain_monitor.chain_monitor.best_block_updated(&new_header, nodes[0].best_block_info().1 + 1);
+               assert!(nodes[0].chain_monitor.release_pending_monitor_events().is_empty());
+
+               // If the ChannelManager tries to update the channel, however, the ChainMonitor will pass
+               // the update through to the ChannelMonitor which will refuse it (as the channel is closed).
+               chanmon_cfgs[0].persister.set_update_ret(Ok(()));
+               unwrap_send_err!(nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret)),
+                       true, APIError::ChannelUnavailable { ref err },
+                       assert!(err.contains("ChannelMonitor storage failure")));
+               check_added_monitors!(nodes[0], 2); // After the failure we generate a close-channel monitor update
+               check_closed_broadcast!(nodes[0], true);
+               check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
+
+               // However, as the ChainMonitor is still waiting for the original persistence to complete,
+               // it won't yet release the MonitorEvents.
+               assert!(nodes[0].chain_monitor.release_pending_monitor_events().is_empty());
+
+               if block_timeout {
+                       // After three blocks, pending MontiorEvents should be released either way.
+                       let latest_header = BlockHeader {
+                               version: 2, time: 0, bits: 0, nonce: 0,
+                               prev_blockhash: nodes[0].best_block_info().0,
+                               merkle_root: Default::default() };
+                       nodes[0].chain_monitor.chain_monitor.best_block_updated(&latest_header, nodes[0].best_block_info().1 + LATENCY_GRACE_PERIOD_BLOCKS);
+               } else {
+                       for (funding_outpoint, update_ids) in chanmon_cfgs[0].persister.chain_sync_monitor_persistences.lock().unwrap().iter() {
+                               for update_id in update_ids {
+                                       nodes[0].chain_monitor.chain_monitor.channel_monitor_updated(*funding_outpoint, *update_id).unwrap();
+                               }
+                       }
+               }
+
+               expect_payment_sent!(nodes[0], payment_preimage);
+       }
+
+       #[test]
+       fn chainsync_pauses_events() {
+               do_chainsync_pauses_events(false);
+               do_chainsync_pauses_events(true);
+       }
+
+       #[test]
+       fn update_during_chainsync_fails_channel() {
+               let chanmon_cfgs = create_chanmon_cfgs(2);
+               let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+               let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+               create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+
+               chanmon_cfgs[0].persister.chain_sync_monitor_persistences.lock().unwrap().clear();
+               chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::PermanentFailure));
+
+               connect_blocks(&nodes[0], 1);
+               // Before processing events, the ChannelManager will still think the Channel is open and
+               // there won't be any ChannelMonitorUpdates
+               assert_eq!(nodes[0].node.list_channels().len(), 1);
+               check_added_monitors!(nodes[0], 0);
+               // ... however once we get events once, the channel will close, creating a channel-closed
+               // ChannelMonitorUpdate.
+               check_closed_broadcast!(nodes[0], true);
+               check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "Failed to persist ChannelMonitor update during chain sync".to_string() });
+               check_added_monitors!(nodes[0], 1);
+       }
 }
index 1699e4827236c89ba6e1dc6fef6b7ff726db6366..5f456c7bfe86ba91ca3a73a19a93e491126adde7 100644 (file)
@@ -115,14 +115,6 @@ impl Readable for ChannelMonitorUpdate {
        }
 }
 
-/// General Err type for ChannelMonitor actions. Generally, this implies that the data provided is
-/// inconsistent with the ChannelMonitor being called. eg for ChannelMonitor::update_monitor this
-/// means you tried to update a monitor for a different channel or the ChannelMonitorUpdate was
-/// corrupted.
-/// Contains a developer-readable error message.
-#[derive(Clone, Debug)]
-pub struct MonitorUpdateError(pub &'static str);
-
 /// An event to be processed by the ChannelManager.
 #[derive(Clone, PartialEq)]
 pub enum MonitorEvent {
@@ -695,6 +687,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        // remote monitor out-of-order with regards to the block view.
        holder_tx_signed: bool,
 
+       // If a spend of the funding output is seen, we set this to true and reject any further
+       // updates. This prevents any further changes in the offchain state no matter the order
+       // of block connection between ChannelMonitors and the ChannelManager.
+       funding_spend_seen: bool,
+
        funding_spend_confirmed: Option<Txid>,
        /// The set of HTLCs which have been either claimed or failed on chain and have reached
        /// the requisite confirmations on the claim/fail transaction (either ANTI_REORG_DELAY or the
@@ -760,6 +757,7 @@ impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
                        self.outputs_to_watch != other.outputs_to_watch ||
                        self.lockdown_from_offchain != other.lockdown_from_offchain ||
                        self.holder_tx_signed != other.holder_tx_signed ||
+                       self.funding_spend_seen != other.funding_spend_seen ||
                        self.funding_spend_confirmed != other.funding_spend_confirmed ||
                        self.htlcs_resolved_on_chain != other.htlcs_resolved_on_chain
                {
@@ -935,6 +933,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                        (1, self.funding_spend_confirmed, option),
                        (3, self.htlcs_resolved_on_chain, vec_type),
                        (5, self.pending_monitor_events, vec_type),
+                       (7, self.funding_spend_seen, required),
                });
 
                Ok(())
@@ -1033,6 +1032,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
                                lockdown_from_offchain: false,
                                holder_tx_signed: false,
+                               funding_spend_seen: false,
                                funding_spend_confirmed: None,
                                htlcs_resolved_on_chain: Vec::new(),
 
@@ -1044,7 +1044,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        }
 
        #[cfg(test)]
-       fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> {
+       fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> {
                self.inner.lock().unwrap().provide_secret(idx, secret)
        }
 
@@ -1066,12 +1066,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
        #[cfg(test)]
        fn provide_latest_holder_commitment_tx(
-               &self,
-               holder_commitment_tx: HolderCommitmentTransaction,
+               &self, holder_commitment_tx: HolderCommitmentTransaction,
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-       ) -> Result<(), MonitorUpdateError> {
-               self.inner.lock().unwrap().provide_latest_holder_commitment_tx(
-                       holder_commitment_tx, htlc_outputs)
+       ) -> Result<(), ()> {
+               self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
        }
 
        #[cfg(test)]
@@ -1112,7 +1110,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                broadcaster: &B,
                fee_estimator: &F,
                logger: &L,
-       ) -> Result<(), MonitorUpdateError>
+       ) -> Result<(), ()>
        where
                B::Target: BroadcasterInterface,
                F::Target: FeeEstimator,
@@ -1687,9 +1685,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
        /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
        /// counterparty commitment transaction's secret, they are de facto pruned (we can use revocation key).
-       fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> {
+       fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> {
                if let Err(()) = self.commitment_secrets.provide_secret(idx, secret) {
-                       return Err(MonitorUpdateError("Previous secret did not match new one"));
+                       return Err("Previous secret did not match new one");
                }
 
                // Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill
@@ -1781,7 +1779,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// is important that any clones of this channel monitor (including remote clones) by kept
        /// up-to-date as our holder commitment transaction is updated.
        /// Panics if set_on_holder_tx_csv has never been called.
-       fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+       fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), &'static str> {
                // block for Rust 1.34 compat
                let mut new_holder_commitment_tx = {
                        let trusted_tx = holder_commitment_tx.trust();
@@ -1804,7 +1802,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                mem::swap(&mut new_holder_commitment_tx, &mut self.current_holder_commitment_tx);
                self.prev_holder_signed_commitment_tx = Some(new_holder_commitment_tx);
                if self.holder_tx_signed {
-                       return Err(MonitorUpdateError("Latest holder commitment signed has already been signed, update is rejected"));
+                       return Err("Latest holder commitment signed has already been signed, update is rejected");
                }
                Ok(())
        }
@@ -1868,7 +1866,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
        }
 
-       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError>
+       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()>
        where B::Target: BroadcasterInterface,
                    F::Target: FeeEstimator,
                    L::Target: Logger,
@@ -1886,12 +1884,17 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                } else if self.latest_update_id + 1 != updates.update_id {
                        panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!");
                }
+               let mut ret = Ok(());
                for update in updates.updates.iter() {
                        match update {
                                ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
                                        if self.lockdown_from_offchain { panic!(); }
-                                       self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone())?
+                                       if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone()) {
+                                               log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
+                                               log_error!(logger, "    {}", e);
+                                               ret = Err(());
+                                       }
                                }
                                ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -1903,7 +1906,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                },
                                ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => {
                                        log_trace!(logger, "Updating ChannelMonitor with commitment secret");
-                                       self.provide_secret(*idx, *secret)?
+                                       if let Err(e) = self.provide_secret(*idx, *secret) {
+                                               log_error!(logger, "Providing latest counterparty commitment secret failed/was refused:");
+                                               log_error!(logger, "    {}", e);
+                                               ret = Err(());
+                                       }
                                },
                                ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => {
                                        log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast);
@@ -1928,7 +1935,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        }
                }
                self.latest_update_id = updates.update_id;
-               Ok(())
+
+               if ret.is_ok() && self.funding_spend_seen {
+                       log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
+                       Err(())
+               } else { ret }
        }
 
        pub fn get_latest_update_id(&self) -> u64 {
@@ -2357,6 +2368,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        let mut balance_spendable_csv = None;
                                        log_info!(logger, "Channel {} closed by funding output spend in txid {}.",
                                                log_bytes!(self.funding_info.0.to_channel_id()), tx.txid());
+                                       self.funding_spend_seen = true;
                                        if (tx.input[0].sequence >> 8*3) as u8 == 0x80 && (tx.lock_time >> 8*3) as u8 == 0x20 {
                                                let (mut new_outpoints, new_outputs) = self.check_spend_counterparty_transaction(&tx, height, &logger);
                                                if !new_outputs.1.is_empty() {
@@ -3206,10 +3218,12 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 
                let mut funding_spend_confirmed = None;
                let mut htlcs_resolved_on_chain = Some(Vec::new());
+               let mut funding_spend_seen = Some(false);
                read_tlv_fields!(reader, {
                        (1, funding_spend_confirmed, option),
                        (3, htlcs_resolved_on_chain, vec_type),
                        (5, pending_monitor_events, vec_type),
+                       (7, funding_spend_seen, option),
                });
 
                let mut secp_ctx = Secp256k1::new();
@@ -3259,6 +3273,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 
                                lockdown_from_offchain,
                                holder_tx_signed,
+                               funding_spend_seen: funding_spend_seen.unwrap(),
                                funding_spend_confirmed,
                                htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
 
@@ -3272,6 +3287,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 
 #[cfg(test)]
 mod tests {
+       use bitcoin::blockdata::block::BlockHeader;
        use bitcoin::blockdata::script::{Script, Builder};
        use bitcoin::blockdata::opcodes;
        use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType};
@@ -3280,24 +3296,128 @@ mod tests {
        use bitcoin::hashes::Hash;
        use bitcoin::hashes::sha256::Hash as Sha256;
        use bitcoin::hashes::hex::FromHex;
-       use bitcoin::hash_types::Txid;
+       use bitcoin::hash_types::{BlockHash, Txid};
        use bitcoin::network::constants::Network;
+       use bitcoin::secp256k1::key::{SecretKey,PublicKey};
+       use bitcoin::secp256k1::Secp256k1;
+
        use hex;
-       use chain::BestBlock;
+
+       use super::ChannelMonitorUpdateStep;
+       use ::{check_added_monitors, check_closed_broadcast, check_closed_event, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err};
+       use chain::{BestBlock, Confirm};
        use chain::channelmonitor::ChannelMonitor;
        use chain::package::{WEIGHT_OFFERED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_RECEIVED_HTLC, WEIGHT_REVOKED_OUTPUT};
        use chain::transaction::OutPoint;
+       use chain::keysinterface::InMemorySigner;
        use ln::{PaymentPreimage, PaymentHash};
        use ln::chan_utils;
        use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
+       use ln::channelmanager::PaymentSendFailure;
+       use ln::features::InitFeatures;
+       use ln::functional_test_utils::*;
        use ln::script::ShutdownScript;
+       use util::errors::APIError;
+       use util::events::{ClosureReason, MessageSendEventsProvider};
        use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
-       use bitcoin::secp256k1::key::{SecretKey,PublicKey};
-       use bitcoin::secp256k1::Secp256k1;
+       use util::ser::{ReadableArgs, Writeable};
        use sync::{Arc, Mutex};
-       use chain::keysinterface::InMemorySigner;
+       use io;
        use prelude::*;
 
+       fn do_test_funding_spend_refuses_updates(use_local_txn: bool) {
+               // Previously, monitor updates were allowed freely even after a funding-spend transaction
+               // confirmed. This would allow a race condition where we could receive a payment (including
+               // the counterparty revoking their broadcasted state!) and accept it without recourse as
+               // long as the ChannelMonitor receives the block first, the full commitment update dance
+               // occurs after the block is connected, and before the ChannelManager receives the block.
+               // Obviously this is an incredibly contrived race given the counterparty would be risking
+               // their full channel balance for it, but its worth fixing nonetheless as it makes the
+               // potential ChannelMonitor states simpler to reason about.
+               //
+               // This test checks said behavior, as well as ensuring a ChannelMonitorUpdate with multiple
+               // updates is handled correctly in such conditions.
+               let chanmon_cfgs = create_chanmon_cfgs(3);
+               let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+               let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+               let channel = create_announced_chan_between_nodes(
+                       &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+               create_announced_chan_between_nodes(
+                       &nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
+
+               // Rebalance somewhat
+               send_payment(&nodes[0], &[&nodes[1]], 10_000_000);
+
+               // First route two payments for testing at the end
+               let payment_preimage_1 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000).0;
+               let payment_preimage_2 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000).0;
+
+               let local_txn = get_local_commitment_txn!(nodes[1], channel.2);
+               assert_eq!(local_txn.len(), 1);
+               let remote_txn = get_local_commitment_txn!(nodes[0], channel.2);
+               assert_eq!(remote_txn.len(), 3); // Commitment and two HTLC-Timeouts
+               check_spends!(remote_txn[1], remote_txn[0]);
+               check_spends!(remote_txn[2], remote_txn[0]);
+               let broadcast_tx = if use_local_txn { &local_txn[0] } else { &remote_txn[0] };
+
+               // Connect a commitment transaction, but only to the ChainMonitor/ChannelMonitor. The
+               // channel is now closed, but the ChannelManager doesn't know that yet.
+               let new_header = BlockHeader {
+                       version: 2, time: 0, bits: 0, nonce: 0,
+                       prev_blockhash: nodes[0].best_block_info().0,
+                       merkle_root: Default::default() };
+               let conf_height = nodes[0].best_block_info().1 + 1;
+               nodes[1].chain_monitor.chain_monitor.transactions_confirmed(&new_header,
+                       &[(0, broadcast_tx)], conf_height);
+
+               let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor<InMemorySigner>)>::read(
+                                               &mut io::Cursor::new(&get_monitor!(nodes[1], channel.2).encode()),
+                                               &nodes[1].keys_manager.backing).unwrap();
+
+               // If the ChannelManager tries to update the channel, however, the ChainMonitor will pass
+               // the update through to the ChannelMonitor which will refuse it (as the channel is closed).
+               let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000);
+               unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)),
+                       true, APIError::ChannelUnavailable { ref err },
+                       assert!(err.contains("ChannelMonitor storage failure")));
+               check_added_monitors!(nodes[1], 2); // After the failure we generate a close-channel monitor update
+               check_closed_broadcast!(nodes[1], true);
+               check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
+
+               // Build a new ChannelMonitorUpdate which contains both the failing commitment tx update
+               // and provides the claim preimages for the two pending HTLCs. The first update generates
+               // an error, but the point of this test is to ensure the later updates are still applied.
+               let monitor_updates = nodes[1].chain_monitor.monitor_updates.lock().unwrap();
+               let mut replay_update = monitor_updates.get(&channel.2).unwrap().iter().rev().skip(1).next().unwrap().clone();
+               assert_eq!(replay_update.updates.len(), 1);
+               if let ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } = replay_update.updates[0] {
+               } else { panic!(); }
+               replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_1 });
+               replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_2 });
+
+               let broadcaster = TestBroadcaster::new(Arc::clone(&nodes[1].blocks));
+               assert!(
+                       pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &&chanmon_cfgs[1].fee_estimator, &nodes[1].logger)
+                       .is_err());
+               // Even though we error'd on the first update, we should still have generated an HTLC claim
+               // transaction
+               let txn_broadcasted = broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+               assert!(txn_broadcasted.len() >= 2);
+               let htlc_txn = txn_broadcasted.iter().filter(|tx| {
+                       assert_eq!(tx.input.len(), 1);
+                       tx.input[0].previous_output.txid == broadcast_tx.txid()
+               }).collect::<Vec<_>>();
+               assert_eq!(htlc_txn.len(), 2);
+               check_spends!(htlc_txn[0], broadcast_tx);
+               check_spends!(htlc_txn[1], broadcast_tx);
+       }
+       #[test]
+       fn test_funding_spend_refuses_updates() {
+               do_test_funding_spend_refuses_updates(true);
+               do_test_funding_spend_refuses_updates(false);
+       }
+
        #[test]
        fn test_prune_preimages() {
                let secp_ctx = Secp256k1::new();
index e6ecd1f3563c6a312a5e22ffe017a7ea011d7186..3338803f9a0c771a7ef976a0afba9bd28d6ded7f 100644 (file)
@@ -18,7 +18,7 @@
 //! generated/etc. This makes it a good candidate for tight integration into an existing wallet
 //! instead of having a rather-separate lightning appendage to a wallet.
 
-#![cfg_attr(not(any(feature = "fuzztarget", feature = "_test_utils")), deny(missing_docs))]
+#![cfg_attr(not(any(test, feature = "fuzztarget", feature = "_test_utils")), deny(missing_docs))]
 #![cfg_attr(not(any(test, feature = "fuzztarget", feature = "_test_utils")), forbid(unsafe_code))]
 #![deny(broken_intra_doc_links)]
 
index cb39b64a44834b83087bd415ee885aea4cf2f633..236b1e498f45ef26b86b1b5b5d46d870028e70ea 100644 (file)
@@ -19,11 +19,10 @@ use bitcoin::network::constants::Network;
 use chain::channelmonitor::ChannelMonitor;
 use chain::transaction::OutPoint;
 use chain::{ChannelMonitorUpdateErr, Listen, Watch};
-use ln::{PaymentPreimage, PaymentHash};
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
 use ln::features::InitFeatures;
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, ErrorAction, RoutingMessageHandler};
+use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
 use util::config::UserConfig;
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason};
@@ -31,9 +30,6 @@ use util::errors::APIError;
 use util::ser::{ReadableArgs, Writeable};
 use util::test_utils::TestBroadcaster;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
-
 use ln::functional_test_utils::*;
 
 use util::test_utils;
index 3b10cba549823566bd658e143bf1093fba5c62c0..a027e6c364734ded0b61ad1c24e1b72e26eaede0 100644 (file)
@@ -862,8 +862,12 @@ impl<Signer: Sign> Channel<Signer> {
                where F::Target: FeeEstimator
        {
                let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               if feerate_per_kw < lower_limit {
-                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
+               // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
+               // occasional issues with feerate disagreements between an initiator that wants a feerate
+               // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
+               // sat/kw before the comparison here.
+               if feerate_per_kw + 250 < lower_limit {
+                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
                }
                // We only bound the fee updates on the upper side to prevent completely absurd feerates,
                // always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
index 342bd059e61f9387412232aaf6be2c00df52e623..9c344fe7621d0c7188493f5870aa0a96fa73ba8f 100644 (file)
@@ -2458,7 +2458,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// 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.
+       /// Returns [`APIError::ChannelUnavailable`] if a funding transaction has already been provided
+       /// for the channel or if the channel has been closed as indicated by [`Event::ChannelClosed`].
        ///
        /// 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
@@ -2473,6 +2474,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
        /// create a new channel with a conflicting funding transaction.
        ///
        /// [`Event::FundingGenerationReady`]: crate::util::events::Event::FundingGenerationReady
+       /// [`Event::ChannelClosed`]: crate::util::events::Event::ChannelClosed
        pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_transaction: Transaction) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
@@ -3368,19 +3370,21 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       /// Provides a payment preimage in response to a PaymentReceived event, returning true and
-       /// generating message events for the net layer to claim the payment, if possible. Thus, you
-       /// should probably kick the net layer to go send messages if this returns true!
+       /// Provides a payment preimage in response to [`Event::PaymentReceived`], generating any
+       /// [`MessageSendEvent`]s needed to claim the payment.
        ///
        /// Note that if you did not set an `amount_msat` when calling [`create_inbound_payment`] or
        /// [`create_inbound_payment_for_hash`] you must check that the amount in the `PaymentReceived`
        /// event matches your expectation. If you fail to do so and call this method, you may provide
        /// the sender "proof-of-payment" when they did not fulfill the full expected payment.
        ///
-       /// May panic if called except in response to a PaymentReceived event.
+       /// Returns whether any HTLCs were claimed, and thus if any new [`MessageSendEvent`]s are now
+       /// pending for processing via [`get_and_clear_pending_msg_events`].
        ///
+       /// [`Event::PaymentReceived`]: crate::util::events::Event::PaymentReceived
        /// [`create_inbound_payment`]: Self::create_inbound_payment
        /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
+       /// [`get_and_clear_pending_msg_events`]: MessageSendEventsProvider::get_and_clear_pending_msg_events
        pub fn claim_funds(&self, payment_preimage: PaymentPreimage) -> bool {
                let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
index 32ba9de758632036942318c756f64708793e1b1a..dab841327802fc8ae3ee0c869395a7ce378c4148 100644 (file)
@@ -126,6 +126,10 @@ mod sealed {
                        ,
                        // Byte 3
                        ,
+                       // Byte 4
+                       ,
+                       // Byte 5
+                       ,
                ],
                optional_features: [
                        // Byte 0
@@ -136,6 +140,10 @@ mod sealed {
                        BasicMPP,
                        // Byte 3
                        ShutdownAnySegwit,
+                       // Byte 4
+                       ,
+                       // Byte 5
+                       ChannelType,
                ],
        });
        define_context!(NodeContext {
@@ -167,7 +175,7 @@ mod sealed {
                        // Byte 4
                        ,
                        // Byte 5
-                       ,
+                       ChannelType,
                        // Byte 6
                        Keysend,
                ],
@@ -223,7 +231,7 @@ mod sealed {
        /// useful for manipulating feature flags.
        macro_rules! define_feature {
                ($odd_bit: expr, $feature: ident, [$($context: ty),+], $doc: expr, $optional_setter: ident,
-                $required_setter: ident) => {
+                $required_setter: ident, $supported_getter: ident) => {
                        #[doc = $doc]
                        ///
                        /// See [BOLT #9] for details.
@@ -320,6 +328,11 @@ mod sealed {
                                        <T as $feature>::set_required_bit(&mut self.flags);
                                        self
                                }
+
+                               /// Checks if this feature is supported.
+                               pub fn $supported_getter(&self) -> bool {
+                                       <T as $feature>::supports_feature(&self.flags)
+                               }
                        }
 
                        $(
@@ -331,41 +344,60 @@ mod sealed {
                                        const ASSERT_ODD_BIT_PARITY: usize = (<Self as $feature>::ODD_BIT % 2) - 1;
                                }
                        )*
-
+               };
+               ($odd_bit: expr, $feature: ident, [$($context: ty),+], $doc: expr, $optional_setter: ident,
+                $required_setter: ident, $supported_getter: ident, $required_getter: ident) => {
+                       define_feature!($odd_bit, $feature, [$($context),+], $doc, $optional_setter, $required_setter, $supported_getter);
+                       impl <T: $feature> Features<T> {
+                               /// Checks if this feature is required.
+                               pub fn $required_getter(&self) -> bool {
+                                       <T as $feature>::requires_feature(&self.flags)
+                               }
+                       }
                }
        }
 
        define_feature!(1, DataLossProtect, [InitContext, NodeContext],
                "Feature flags for `option_data_loss_protect`.", set_data_loss_protect_optional,
-               set_data_loss_protect_required);
+               set_data_loss_protect_required, supports_data_loss_protect, requires_data_loss_protect);
        // NOTE: Per Bolt #9, initial_routing_sync has no even bit.
        define_feature!(3, InitialRoutingSync, [InitContext], "Feature flags for `initial_routing_sync`.",
-               set_initial_routing_sync_optional, set_initial_routing_sync_required);
+               set_initial_routing_sync_optional, set_initial_routing_sync_required,
+               initial_routing_sync);
        define_feature!(5, UpfrontShutdownScript, [InitContext, NodeContext],
                "Feature flags for `option_upfront_shutdown_script`.", set_upfront_shutdown_script_optional,
-               set_upfront_shutdown_script_required);
+               set_upfront_shutdown_script_required, supports_upfront_shutdown_script,
+               requires_upfront_shutdown_script);
        define_feature!(7, GossipQueries, [InitContext, NodeContext],
-               "Feature flags for `gossip_queries`.", set_gossip_queries_optional, set_gossip_queries_required);
+               "Feature flags for `gossip_queries`.", set_gossip_queries_optional, set_gossip_queries_required,
+               supports_gossip_queries, requires_gossip_queries);
        define_feature!(9, VariableLengthOnion, [InitContext, NodeContext, InvoiceContext],
                "Feature flags for `var_onion_optin`.", set_variable_length_onion_optional,
-               set_variable_length_onion_required);
+               set_variable_length_onion_required, supports_variable_length_onion,
+               requires_variable_length_onion);
        define_feature!(13, StaticRemoteKey, [InitContext, NodeContext, ChannelTypeContext],
                "Feature flags for `option_static_remotekey`.", set_static_remote_key_optional,
-               set_static_remote_key_required);
+               set_static_remote_key_required, supports_static_remote_key, requires_static_remote_key);
        define_feature!(15, PaymentSecret, [InitContext, NodeContext, InvoiceContext],
-               "Feature flags for `payment_secret`.", set_payment_secret_optional, set_payment_secret_required);
+               "Feature flags for `payment_secret`.", set_payment_secret_optional, set_payment_secret_required,
+               supports_payment_secret, requires_payment_secret);
        define_feature!(17, BasicMPP, [InitContext, NodeContext, InvoiceContext],
-               "Feature flags for `basic_mpp`.", set_basic_mpp_optional, set_basic_mpp_required);
+               "Feature flags for `basic_mpp`.", set_basic_mpp_optional, set_basic_mpp_required,
+               supports_basic_mpp, requires_basic_mpp);
        define_feature!(27, ShutdownAnySegwit, [InitContext, NodeContext],
                "Feature flags for `opt_shutdown_anysegwit`.", set_shutdown_any_segwit_optional,
-               set_shutdown_any_segwit_required);
+               set_shutdown_any_segwit_required, supports_shutdown_anysegwit, requires_shutdown_anysegwit);
+       define_feature!(45, ChannelType, [InitContext, NodeContext],
+               "Feature flags for `option_channel_type`.", set_channel_type_optional,
+               set_channel_type_required, supports_channel_type, requires_channel_type);
        define_feature!(55, Keysend, [NodeContext],
-               "Feature flags for keysend payments.", set_keysend_optional, set_keysend_required);
+               "Feature flags for keysend payments.", set_keysend_optional, set_keysend_required,
+               supports_keysend, requires_keysend);
 
        #[cfg(test)]
        define_feature!(123456789, UnknownFeature, [NodeContext, ChannelContext, InvoiceContext],
                "Feature flags for an unknown feature used in testing.", set_unknown_feature_optional,
-               set_unknown_feature_required);
+               set_unknown_feature_required, supports_unknown_test_feature, requires_unknown_test_feature);
 }
 
 /// Tracks the set of features which a node implements, templated by the context in which it
@@ -662,25 +694,7 @@ impl<T: sealed::Context> Features<T> {
        }
 }
 
-impl<T: sealed::DataLossProtect> Features<T> {
-       #[cfg(test)]
-       pub(crate) fn requires_data_loss_protect(&self) -> bool {
-               <T as sealed::DataLossProtect>::requires_feature(&self.flags)
-       }
-       #[cfg(test)]
-       pub(crate) fn supports_data_loss_protect(&self) -> bool {
-               <T as sealed::DataLossProtect>::supports_feature(&self.flags)
-       }
-}
-
 impl<T: sealed::UpfrontShutdownScript> Features<T> {
-       #[cfg(test)]
-       pub(crate) fn requires_upfront_shutdown_script(&self) -> bool {
-               <T as sealed::UpfrontShutdownScript>::requires_feature(&self.flags)
-       }
-       pub(crate) fn supports_upfront_shutdown_script(&self) -> bool {
-               <T as sealed::UpfrontShutdownScript>::supports_feature(&self.flags)
-       }
        #[cfg(test)]
        pub(crate) fn clear_upfront_shutdown_script(mut self) -> Self {
                <T as sealed::UpfrontShutdownScript>::clear_bits(&mut self.flags);
@@ -690,13 +704,6 @@ impl<T: sealed::UpfrontShutdownScript> Features<T> {
 
 
 impl<T: sealed::GossipQueries> Features<T> {
-       #[cfg(test)]
-       pub(crate) fn requires_gossip_queries(&self) -> bool {
-               <T as sealed::GossipQueries>::requires_feature(&self.flags)
-       }
-       pub(crate) fn supports_gossip_queries(&self) -> bool {
-               <T as sealed::GossipQueries>::supports_feature(&self.flags)
-       }
        #[cfg(test)]
        pub(crate) fn clear_gossip_queries(mut self) -> Self {
                <T as sealed::GossipQueries>::clear_bits(&mut self.flags);
@@ -704,30 +711,7 @@ impl<T: sealed::GossipQueries> Features<T> {
        }
 }
 
-impl<T: sealed::VariableLengthOnion> Features<T> {
-       #[cfg(test)]
-       pub(crate) fn requires_variable_length_onion(&self) -> bool {
-               <T as sealed::VariableLengthOnion>::requires_feature(&self.flags)
-       }
-       pub(crate) fn supports_variable_length_onion(&self) -> bool {
-               <T as sealed::VariableLengthOnion>::supports_feature(&self.flags)
-       }
-}
-
-impl<T: sealed::StaticRemoteKey> Features<T> {
-       pub(crate) fn supports_static_remote_key(&self) -> bool {
-               <T as sealed::StaticRemoteKey>::supports_feature(&self.flags)
-       }
-       #[cfg(test)]
-       pub(crate) fn requires_static_remote_key(&self) -> bool {
-               <T as sealed::StaticRemoteKey>::requires_feature(&self.flags)
-       }
-}
-
 impl<T: sealed::InitialRoutingSync> Features<T> {
-       pub(crate) fn initial_routing_sync(&self) -> bool {
-               <T as sealed::InitialRoutingSync>::supports_feature(&self.flags)
-       }
        // We are no longer setting initial_routing_sync now that gossip_queries
        // is enabled. This feature is ignored by a peer when gossip_queries has 
        // been negotiated.
@@ -737,32 +721,7 @@ impl<T: sealed::InitialRoutingSync> Features<T> {
        }
 }
 
-impl<T: sealed::PaymentSecret> Features<T> {
-       #[cfg(test)]
-       pub(crate) fn requires_payment_secret(&self) -> bool {
-               <T as sealed::PaymentSecret>::requires_feature(&self.flags)
-       }
-       /// Returns whether the `payment_secret` feature is supported.
-       pub fn supports_payment_secret(&self) -> bool {
-               <T as sealed::PaymentSecret>::supports_feature(&self.flags)
-       }
-}
-
-impl<T: sealed::BasicMPP> Features<T> {
-       #[cfg(test)]
-       pub(crate) fn requires_basic_mpp(&self) -> bool {
-               <T as sealed::BasicMPP>::requires_feature(&self.flags)
-       }
-       // We currently never test for this since we don't actually *generate* multipath routes.
-       pub(crate) fn supports_basic_mpp(&self) -> bool {
-               <T as sealed::BasicMPP>::supports_feature(&self.flags)
-       }
-}
-
 impl<T: sealed::ShutdownAnySegwit> Features<T> {
-       pub(crate) fn supports_shutdown_anysegwit(&self) -> bool {
-               <T as sealed::ShutdownAnySegwit>::supports_feature(&self.flags)
-       }
        #[cfg(test)]
        pub(crate) fn clear_shutdown_anysegwit(mut self) -> Self {
                <T as sealed::ShutdownAnySegwit>::clear_bits(&mut self.flags);
@@ -859,6 +818,11 @@ mod tests {
                assert!(!NodeFeatures::known().requires_basic_mpp());
                assert!(!InvoiceFeatures::known().requires_basic_mpp());
 
+               assert!(InitFeatures::known().supports_channel_type());
+               assert!(NodeFeatures::known().supports_channel_type());
+               assert!(!InitFeatures::known().requires_channel_type());
+               assert!(!NodeFeatures::known().requires_channel_type());
+
                assert!(InitFeatures::known().supports_shutdown_anysegwit());
                assert!(NodeFeatures::known().supports_shutdown_anysegwit());
 
@@ -897,11 +861,15 @@ mod tests {
                        // - var_onion_optin (req) | static_remote_key (req) | payment_secret(req)
                        // - basic_mpp
                        // - opt_shutdown_anysegwit
-                       assert_eq!(node_features.flags.len(), 4);
+                       // -
+                       // - option_channel_type
+                       assert_eq!(node_features.flags.len(), 6);
                        assert_eq!(node_features.flags[0], 0b00000010);
                        assert_eq!(node_features.flags[1], 0b01010001);
                        assert_eq!(node_features.flags[2], 0b00000010);
                        assert_eq!(node_features.flags[3], 0b00001000);
+                       assert_eq!(node_features.flags[4], 0b00000000);
+                       assert_eq!(node_features.flags[5], 0b00100000);
                }
 
                // Check that cleared flags are kept blank when converting back:
index 01f9db53439ca5558cd6f25c2e7a4077e167f7a2..78bf834a6280c33489753992cd5ec28887976607 100644 (file)
@@ -33,8 +33,6 @@ use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::blockdata::transaction::{Transaction, TxOut};
 use bitcoin::network::constants::Network;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
 use bitcoin::hash_types::BlockHash;
 
 use bitcoin::secp256k1::key::PublicKey;
@@ -337,9 +335,11 @@ pub fn create_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(node_a: &'a Node<'b,
        (announcement, as_update, bs_update, channel_id, tx)
 }
 
+#[macro_export]
 macro_rules! get_revoke_commit_msgs {
        ($node: expr, $node_id: expr) => {
                {
+                       use util::events::MessageSendEvent;
                        let events = $node.node.get_and_clear_pending_msg_events();
                        assert_eq!(events.len(), 2);
                        (match events[0] {
@@ -401,13 +401,14 @@ macro_rules! get_event {
 }
 
 #[cfg(test)]
+#[macro_export]
 macro_rules! get_htlc_update_msgs {
        ($node: expr, $node_id: expr) => {
                {
                        let events = $node.node.get_and_clear_pending_msg_events();
                        assert_eq!(events.len(), 1);
                        match events[0] {
-                               MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
+                               $crate::util::events::MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
                                        assert_eq!(*node_id, $node_id);
                                        (*updates).clone()
                                },
@@ -530,7 +531,7 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, channel_value, 42);
        assert_eq!(temporary_channel_id, expected_temporary_channel_id);
 
-       node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
+       assert!(node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).is_ok());
        check_added_monitors!(node_a, 0);
 
        let funding_created_msg = get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id());
@@ -558,6 +559,11 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
        assert_eq!(node_a.tx_broadcaster.txn_broadcasted.lock().unwrap()[0], tx);
        node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
 
+       // Ensure that funding_transaction_generated is idempotent.
+       assert!(node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).is_err());
+       assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
+       check_added_monitors!(node_a, 0);
+
        tx
 }
 
@@ -713,6 +719,7 @@ pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, '
        }
 }
 
+#[macro_export]
 macro_rules! check_spends {
        ($tx: expr, $($spends_txn: expr),*) => {
                {
@@ -777,6 +784,9 @@ macro_rules! get_closing_signed_broadcast {
 #[macro_export]
 macro_rules! check_closed_broadcast {
        ($node: expr, $with_error_msg: expr) => {{
+               use $crate::util::events::MessageSendEvent;
+               use $crate::ln::msgs::ErrorAction;
+
                let msg_events = $node.node.get_and_clear_pending_msg_events();
                assert_eq!(msg_events.len(), if $with_error_msg { 2 } else { 1 });
                match msg_events[0] {
@@ -804,6 +814,8 @@ macro_rules! check_closed_event {
                check_closed_event!($node, $events, $reason, false);
        };
        ($node: expr, $events: expr, $reason: expr, $is_check_discard_funding: expr) => {{
+               use $crate::util::events::Event;
+
                let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), $events);
                let expected_reason = $reason;
@@ -1012,10 +1024,12 @@ macro_rules! get_payment_preimage_hash {
        };
        ($dest_node: expr, $min_value_msat: expr) => {
                {
+                       use bitcoin::hashes::Hash as _;
                        let mut payment_count = $dest_node.network_payment_count.borrow_mut();
-                       let payment_preimage = PaymentPreimage([*payment_count; 32]);
+                       let payment_preimage = $crate::ln::PaymentPreimage([*payment_count; 32]);
                        *payment_count += 1;
-                       let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
+                       let payment_hash = $crate::ln::PaymentHash(
+                               bitcoin::hashes::sha256::Hash::hash(&payment_preimage.0[..]).into_inner());
                        let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200).unwrap();
                        (payment_preimage, payment_hash, payment_secret)
                }
@@ -1023,17 +1037,18 @@ macro_rules! get_payment_preimage_hash {
 }
 
 #[cfg(test)]
+#[macro_export]
 macro_rules! get_route_and_payment_hash {
        ($send_node: expr, $recv_node: expr, $recv_value: expr) => {{
-               get_route_and_payment_hash!($send_node, $recv_node, vec![], $recv_value, TEST_FINAL_CLTV)
+               $crate::get_route_and_payment_hash!($send_node, $recv_node, vec![], $recv_value, TEST_FINAL_CLTV)
        }};
        ($send_node: expr, $recv_node: expr, $last_hops: expr, $recv_value: expr, $cltv: expr) => {{
-               let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node, Some($recv_value));
+               let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value));
                let payee = $crate::routing::router::Payee::from_node_id($recv_node.node.get_our_node_id())
                        .with_features($crate::ln::features::InvoiceFeatures::known())
                        .with_route_hints($last_hops);
-               let scorer = ::util::test_utils::TestScorer::with_fixed_penalty(0);
-               let route = ::routing::router::get_route(
+               let scorer = $crate::util::test_utils::TestScorer::with_fixed_penalty(0);
+               let route = $crate::routing::router::get_route(
                        &$send_node.node.get_our_node_id(), &payee, $send_node.network_graph,
                        Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
                        $recv_value, $cltv, $send_node.logger, &scorer
@@ -1057,6 +1072,9 @@ macro_rules! expect_pending_htlcs_forwardable {
        ($node: expr) => {{
                expect_pending_htlcs_forwardable_ignore!($node);
                $node.node.process_pending_htlc_forwards();
+
+               // Ensure process_pending_htlc_forwards is idempotent.
+               $node.node.process_pending_htlc_forwards();
        }}
 }
 
@@ -1070,6 +1088,9 @@ macro_rules! expect_pending_htlcs_forwardable_from_events {
                };
                if $ignore {
                        $node.node.process_pending_htlc_forwards();
+
+                       // Ensure process_pending_htlc_forwards is idempotent.
+                       $node.node.process_pending_htlc_forwards();
                }
        }}
 }
@@ -1097,6 +1118,7 @@ macro_rules! expect_payment_received {
 }
 
 #[cfg(test)]
+#[macro_export]
 macro_rules! expect_payment_sent_without_paths {
        ($node: expr, $expected_payment_preimage: expr) => {
                expect_payment_sent!($node, $expected_payment_preimage, None::<u64>, false);
@@ -1106,23 +1128,26 @@ macro_rules! expect_payment_sent_without_paths {
        }
 }
 
+#[macro_export]
 macro_rules! expect_payment_sent {
        ($node: expr, $expected_payment_preimage: expr) => {
-               expect_payment_sent!($node, $expected_payment_preimage, None::<u64>, true);
+               $crate::expect_payment_sent!($node, $expected_payment_preimage, None::<u64>, true);
        };
        ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr) => {
-               expect_payment_sent!($node, $expected_payment_preimage, $expected_fee_msat_opt, true);
+               $crate::expect_payment_sent!($node, $expected_payment_preimage, $expected_fee_msat_opt, true);
        };
-       ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr, $expect_paths: expr) => {
+       ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr, $expect_paths: expr) => { {
+               use bitcoin::hashes::Hash as _;
                let events = $node.node.get_and_clear_pending_events();
-               let expected_payment_hash = PaymentHash(Sha256::hash(&$expected_payment_preimage.0).into_inner());
+               let expected_payment_hash = $crate::ln::PaymentHash(
+                       bitcoin::hashes::sha256::Hash::hash(&$expected_payment_preimage.0).into_inner());
                if $expect_paths {
                        assert!(events.len() > 1);
                } else {
                        assert_eq!(events.len(), 1);
                }
                let expected_payment_id = match events[0] {
-                       Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
+                       $crate::util::events::Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
                                assert_eq!($expected_payment_preimage, *payment_preimage);
                                assert_eq!(expected_payment_hash, *payment_hash);
                                assert!(fee_paid_msat.is_some());
@@ -1136,7 +1161,7 @@ macro_rules! expect_payment_sent {
                if $expect_paths {
                        for i in 1..events.len() {
                                match events[i] {
-                                       Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => {
+                                       $crate::util::events::Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => {
                                                assert_eq!(payment_id, expected_payment_id);
                                                assert_eq!(payment_hash, Some(expected_payment_hash));
                                        },
@@ -1144,16 +1169,17 @@ macro_rules! expect_payment_sent {
                                }
                        }
                }
-       }
+       } }
 }
 
 #[cfg(test)]
+#[macro_export]
 macro_rules! expect_payment_path_successful {
        ($node: expr) => {
                let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
                match events[0] {
-                       Event::PaymentPathSuccessful { .. } => {},
+                       $crate::util::events::Event::PaymentPathSuccessful { .. } => {},
                        _ => panic!("Unexpected event"),
                }
        }
@@ -1392,6 +1418,12 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
                        last_update_fulfill_dance!(origin_node, expected_route.first().unwrap());
                }
        }
+
+       // Ensure that claim_funds is idempotent.
+       assert!(!expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage));
+       assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
+       check_added_monitors!(expected_paths[0].last().unwrap(), 0);
+
        expected_total_fee_msat
 }
 pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
@@ -1536,6 +1568,12 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
                        }
                }
        }
+
+       // Ensure that fail_htlc_backwards is idempotent.
+       assert!(!expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
+       assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_events().is_empty());
+       assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
+       check_added_monitors!(expected_paths[0].last().unwrap(), 0);
 }
 
 pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], our_payment_hash: PaymentHash)  {
index 48d56b43b3868cb9ddf9a37f5df24e34a0e9c41a..3a30cb7f1d05bd4c47aca455052e252f14ac0d10 100644 (file)
@@ -42,9 +42,6 @@ use bitcoin::blockdata::opcodes;
 use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::network::constants::Network;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
-
 use bitcoin::secp256k1::Secp256k1;
 use bitcoin::secp256k1::key::{PublicKey,SecretKey};
 
index cfde643bcb93c3ed80b49d102f082fed2a4d15d5..d836b736a98fe4985485f06d2d03d5ae4fd4d681 100644 (file)
 
 use chain::channelmonitor::{ANTI_REORG_DELAY, Balance};
 use chain::transaction::OutPoint;
-use ln::{channel, PaymentPreimage, PaymentHash};
+use ln::channel;
 use ln::channelmanager::BREAKDOWN_TIMEOUT;
 use ln::features::InitFeatures;
-use ln::msgs::{ChannelMessageHandler, ErrorAction};
+use ln::msgs::ChannelMessageHandler;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
 use routing::network_graph::NetworkUpdate;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
-
 use bitcoin::blockdata::script::Builder;
 use bitcoin::blockdata::opcodes;
 use bitcoin::secp256k1::Secp256k1;
index 87944ccae639027da172e120ea83ea8b4d06411a..3c6300ef8eadc5e517c3fb398c021bba8c53dd31 100644 (file)
@@ -394,12 +394,10 @@ pub enum NetAddress {
                port: u16,
        },
        /// An old-style Tor onion address/port on which the peer is listening.
-       OnionV2 {
-               /// The bytes (usually encoded in base32 with ".onion" appended)
-               addr: [u8; 10],
-               /// The port on which the node is listening
-               port: u16,
-       },
+       ///
+       /// This field is deprecated and the Tor network generally no longer supports V2 Onion
+       /// addresses. Thus, the details are not parsed here.
+       OnionV2([u8; 12]),
        /// A new-style Tor onion address/port on which the peer is listening.
        /// To create the human-readable "hostname", concatenate ed25519_pubkey, checksum, and version,
        /// wrap as base32 and append ".onion".
@@ -421,7 +419,7 @@ impl NetAddress {
                match self {
                        &NetAddress::IPv4 {..} => { 1 },
                        &NetAddress::IPv6 {..} => { 2 },
-                       &NetAddress::OnionV2 {..} => { 3 },
+                       &NetAddress::OnionV2(_) => { 3 },
                        &NetAddress::OnionV3 {..} => { 4 },
                }
        }
@@ -431,7 +429,7 @@ impl NetAddress {
                match self {
                        &NetAddress::IPv4 { .. } => { 6 },
                        &NetAddress::IPv6 { .. } => { 18 },
-                       &NetAddress::OnionV2 { .. } => { 12 },
+                       &NetAddress::OnionV2(_) => { 12 },
                        &NetAddress::OnionV3 { .. } => { 37 },
                }
        }
@@ -453,10 +451,9 @@ impl Writeable for NetAddress {
                                addr.write(writer)?;
                                port.write(writer)?;
                        },
-                       &NetAddress::OnionV2 { ref addr, ref port } => {
+                       &NetAddress::OnionV2(bytes) => {
                                3u8.write(writer)?;
-                               addr.write(writer)?;
-                               port.write(writer)?;
+                               bytes.write(writer)?;
                        },
                        &NetAddress::OnionV3 { ref ed25519_pubkey, ref checksum, ref version, ref port } => {
                                4u8.write(writer)?;
@@ -486,12 +483,7 @@ impl Readable for Result<NetAddress, u8> {
                                        port: Readable::read(reader)?,
                                }))
                        },
-                       3 => {
-                               Ok(Ok(NetAddress::OnionV2 {
-                                       addr: Readable::read(reader)?,
-                                       port: Readable::read(reader)?,
-                               }))
-                       },
+                       3 => Ok(Ok(NetAddress::OnionV2(Readable::read(reader)?))),
                        4 => {
                                Ok(Ok(NetAddress::OnionV3 {
                                        ed25519_pubkey: Readable::read(reader)?,
@@ -1922,10 +1914,9 @@ mod tests {
                        });
                }
                if onionv2 {
-                       addresses.push(msgs::NetAddress::OnionV2 {
-                               addr: [255, 254, 253, 252, 251, 250, 249, 248, 247, 246],
-                               port: 9735
-                       });
+                       addresses.push(msgs::NetAddress::OnionV2(
+                               [255, 254, 253, 252, 251, 250, 249, 248, 247, 246, 38, 7]
+                       ));
                }
                if onionv3 {
                        addresses.push(msgs::NetAddress::OnionV3 {
index e80cf98d7b9b358e7713f21447a9f7516d5ed3f0..05b108fbf25e678e18f16cc36fb7c7a87793d44f 100644 (file)
@@ -12,7 +12,7 @@
 //! returned errors decode to the correct thing.
 
 use chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS};
-use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
+use ln::{PaymentHash, PaymentSecret};
 use ln::channelmanager::{HTLCForwardInfo, CLTV_FAR_FAR_AWAY};
 use ln::onion_utils;
 use routing::network_graph::NetworkUpdate;
@@ -26,7 +26,6 @@ use util::config::UserConfig;
 
 use bitcoin::hash_types::BlockHash;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::Hash;
 
 use bitcoin::secp256k1;
index 7c9ff3199d15bfe53e2e72967866c85b7b1702ae..518ccd3b0efe5fd6fc75e4ba72eaadbec690205c 100644 (file)
 use chain::{ChannelMonitorUpdateErr, Confirm, Listen, Watch};
 use chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor, LATENCY_GRACE_PERIOD_BLOCKS};
 use chain::transaction::OutPoint;
-use ln::{PaymentPreimage, PaymentHash};
 use ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, ChannelManagerReadArgs, PaymentId, PaymentSendFailure};
 use ln::features::InitFeatures;
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, ErrorAction};
+use ln::msgs::ChannelMessageHandler;
 use util::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider};
 use util::test_utils;
 use util::errors::APIError;
@@ -26,8 +25,6 @@ use util::enforcing_trait_impls::EnforcingSigner;
 use util::ser::{ReadableArgs, Writeable};
 use io;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
 use bitcoin::{Block, BlockHeader, BlockHash};
 
 use prelude::*;
index 5cd1a890c49d5d12c6438b7369121d579045609c..570dc3d0eeeece8d7a4f50d006e807d589b66d4e 100644 (file)
 use chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor};
 use chain::transaction::OutPoint;
 use chain::{Confirm, Watch};
-use ln::PaymentHash;
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
 use ln::features::InitFeatures;
-use ln::msgs::{ChannelMessageHandler, ErrorAction};
+use ln::msgs::ChannelMessageHandler;
 use routing::network_graph::NetworkUpdate;
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
@@ -25,8 +24,6 @@ use util::ser::{ReadableArgs, Writeable};
 use bitcoin::blockdata::block::{Block, BlockHeader};
 use bitcoin::blockdata::script::Builder;
 use bitcoin::blockdata::opcodes;
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
 use bitcoin::hash_types::BlockHash;
 use bitcoin::secp256k1::Secp256k1;
 
index db76a2c760ccb917d6f98cbbd57735c0114156a8..ffd809bc70d148b3fc37d2301922f914a46d26df 100644 (file)
@@ -11,7 +11,6 @@
 
 use chain::keysinterface::KeysInterface;
 use chain::transaction::OutPoint;
-use ln::{PaymentPreimage, PaymentHash};
 use ln::channelmanager::PaymentSendFailure;
 use routing::router::{Payee, get_route};
 use routing::network_graph::NetworkUpdate;
@@ -28,9 +27,6 @@ use util::config::UserConfig;
 use bitcoin::blockdata::script::Builder;
 use bitcoin::blockdata::opcodes;
 
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
-
 use regex;
 
 use core::default::Default;
index 6495d9de4ed5363fb4a2b70c958366653797121c..0c6099f7d5a5e2e8de07bafa1906da8f1edc6538 100644 (file)
@@ -66,10 +66,10 @@ pub enum APIError {
 impl fmt::Debug for APIError {
        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                match *self {
-                       APIError::APIMisuseError {ref err} => f.write_str(err),
+                       APIError::APIMisuseError {ref err} => write!(f, "Misuse error: {}", err),
                        APIError::FeeRateTooHigh {ref err, ref feerate} => write!(f, "{} feerate: {}", err, feerate),
-                       APIError::RouteError {ref err} => f.write_str(err),
-                       APIError::ChannelUnavailable {ref err} => f.write_str(err),
+                       APIError::RouteError {ref err} => write!(f, "Route error: {}", err),
+                       APIError::ChannelUnavailable {ref err} => write!(f, "Channel unavailable: {}", err),
                        APIError::MonitorUpdateFailed => f.write_str("Client indicated a channel monitor update failed"),
                        APIError::IncompatibleShutdownScript { ref script } => {
                                write!(f, "Provided a scriptpubkey format not accepted by peer: {}", script)
index 46ab9f74924e453b5446c27a783616cab0a0ff5d..959b180d5101330f18edfe631d29315cf1660f24 100644 (file)
@@ -153,10 +153,13 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
 #[derive(Clone, Debug)]
 pub enum Event {
        /// Used to indicate that the client should generate a funding transaction with the given
-       /// parameters and then call ChannelManager::funding_transaction_generated.
-       /// Generated in ChannelManager message handling.
+       /// parameters and then call [`ChannelManager::funding_transaction_generated`].
+       /// Generated in [`ChannelManager`] message handling.
        /// Note that *all inputs* in the funding transaction must spend SegWit outputs or your
        /// counterparty can steal your funds!
+       ///
+       /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
+       /// [`ChannelManager::funding_transaction_generated`]: crate::ln::channelmanager::ChannelManager::funding_transaction_generated
        FundingGenerationReady {
                /// The random channel_id we picked which you'll need to pass into
                /// ChannelManager::funding_transaction_generated.
@@ -271,8 +274,10 @@ pub enum Event {
 #[cfg(test)]
                error_data: Option<Vec<u8>>,
        },
-       /// Used to indicate that ChannelManager::process_pending_htlc_forwards should be called at a
-       /// time in the future.
+       /// Used to indicate that [`ChannelManager::process_pending_htlc_forwards`] should be called at
+       /// a time in the future.
+       ///
+       /// [`ChannelManager::process_pending_htlc_forwards`]: crate::ln::channelmanager::ChannelManager::process_pending_htlc_forwards
        PendingHTLCsForwardable {
                /// The minimum amount of time that should be waited prior to calling
                /// process_pending_htlc_forwards. To increase the effort required to correlate payments,
index 8fb72a8f9fe643f56cfc259e5b7214b56a4fb761..6a52e5e1c898230c0d31a4cb9ce20c6acc236076 100644 (file)
@@ -475,10 +475,9 @@ macro_rules! impl_array {
        );
 }
 
-//TODO: performance issue with [u8; size] with impl_array!()
 impl_array!(3); // for rgb
 impl_array!(4); // for IPv4
-impl_array!(10); // for OnionV2
+impl_array!(12); // for OnionV2
 impl_array!(16); // for IPv6
 impl_array!(32); // for channel id & hmac
 impl_array!(PUBLIC_KEY_SIZE); // for PublicKey
index e2e44a2c1a631e5adc8138b6f3ae15965753a83c..6442c9cfa27bd6b7b8a28efaf45008ab0d8cdd52 100644 (file)
@@ -91,6 +91,7 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface {
 
 pub struct TestChainMonitor<'a> {
        pub added_monitors: Mutex<Vec<(OutPoint, channelmonitor::ChannelMonitor<EnforcingSigner>)>>,
+       pub monitor_updates: Mutex<HashMap<[u8; 32], Vec<channelmonitor::ChannelMonitorUpdate>>>,
        pub latest_monitor_update_id: Mutex<HashMap<[u8; 32], (OutPoint, u64, MonitorUpdateId)>>,
        pub chain_monitor: chainmonitor::ChainMonitor<EnforcingSigner, &'a TestChainSource, &'a chaininterface::BroadcasterInterface, &'a TestFeeEstimator, &'a TestLogger, &'a chainmonitor::Persist<EnforcingSigner>>,
        pub keys_manager: &'a TestKeysInterface,
@@ -103,6 +104,7 @@ impl<'a> TestChainMonitor<'a> {
        pub fn new(chain_source: Option<&'a TestChainSource>, broadcaster: &'a chaininterface::BroadcasterInterface, logger: &'a TestLogger, fee_estimator: &'a TestFeeEstimator, persister: &'a chainmonitor::Persist<EnforcingSigner>, keys_manager: &'a TestKeysInterface) -> Self {
                Self {
                        added_monitors: Mutex::new(Vec::new()),
+                       monitor_updates: Mutex::new(HashMap::new()),
                        latest_monitor_update_id: Mutex::new(HashMap::new()),
                        chain_monitor: chainmonitor::ChainMonitor::new(chain_source, broadcaster, logger, fee_estimator, persister),
                        keys_manager,
@@ -132,6 +134,8 @@ impl<'a> chain::Watch<EnforcingSigner> for TestChainMonitor<'a> {
                assert!(channelmonitor::ChannelMonitorUpdate::read(
                                &mut io::Cursor::new(&w.0)).unwrap() == update);
 
+               self.monitor_updates.lock().unwrap().entry(funding_txo.to_channel_id()).or_insert(Vec::new()).push(update.clone());
+
                if let Some(exp) = self.expect_channel_force_closed.lock().unwrap().take() {
                        assert_eq!(funding_txo.to_channel_id(), exp.0);
                        assert_eq!(update.updates.len(), 1);
@@ -168,6 +172,9 @@ pub struct TestPersister {
        /// When we get an update_persisted_channel call with no ChannelMonitorUpdate, we insert the
        /// MonitorUpdateId here.
        pub chain_sync_monitor_persistences: Mutex<HashMap<OutPoint, HashSet<MonitorUpdateId>>>,
+       /// When we get an update_persisted_channel call *with* a ChannelMonitorUpdate, we insert the
+       /// MonitorUpdateId here.
+       pub offchain_monitor_updates: Mutex<HashMap<OutPoint, HashSet<MonitorUpdateId>>>,
 }
 impl TestPersister {
        pub fn new() -> Self {
@@ -175,6 +182,7 @@ impl TestPersister {
                        update_ret: Mutex::new(Ok(())),
                        next_update_ret: Mutex::new(None),
                        chain_sync_monitor_persistences: Mutex::new(HashMap::new()),
+                       offchain_monitor_updates: Mutex::new(HashMap::new()),
                }
        }
 
@@ -202,6 +210,8 @@ impl<Signer: keysinterface::Sign> chainmonitor::Persist<Signer> for TestPersiste
                }
                if update.is_none() {
                        self.chain_sync_monitor_persistences.lock().unwrap().entry(funding_txo).or_insert(HashSet::new()).insert(update_id);
+               } else {
+                       self.offchain_monitor_updates.lock().unwrap().entry(funding_txo).or_insert(HashSet::new()).insert(update_id);
                }
                ret
        }
@@ -211,6 +221,13 @@ pub struct TestBroadcaster {
        pub txn_broadcasted: Mutex<Vec<Transaction>>,
        pub blocks: Arc<Mutex<Vec<(BlockHeader, u32)>>>,
 }
+
+impl TestBroadcaster {
+       pub fn new(blocks: Arc<Mutex<Vec<(BlockHeader, u32)>>>) -> TestBroadcaster {
+               TestBroadcaster { txn_broadcasted: Mutex::new(Vec::new()), blocks }
+       }
+}
+
 impl chaininterface::BroadcasterInterface for TestBroadcaster {
        fn broadcast_transaction(&self, tx: &Transaction) {
                assert!(tx.lock_time < 1_500_000_000);