Merge pull request #594 from TheBlueMatt/2020-04-cleanups
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 20 Apr 2020 21:54:35 +0000 (21:54 +0000)
committerGitHub <noreply@github.com>
Mon, 20 Apr 2020 21:54:35 +0000 (21:54 +0000)
Trivial Cleanups

1  2 
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs

index 600702dfcb6413e9ef29383fff1515dd7836bca8,62ac986639620a1be3f751fa0bdc6e3576fbdd9d..5d52e4a8d16da16740e2f5604509cf23d39a22dd
@@@ -1432,7 -1432,7 +1432,7 @@@ impl<ChanSigner: ChannelKeys, M: Deref
                };
                // Because we have exclusive ownership of the channel here we can release the channel_state
                // lock before add_monitor
-               if let Err(e) = self.monitor.add_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
+               if let Err(e) = self.monitor.add_monitor(chan_monitor.get_funding_txo(), chan_monitor) {
                        match e {
                                ChannelMonitorUpdateErr::PermanentFailure => {
                                        match handle_error!(self, Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", *temporary_channel_id, chan.force_shutdown(true), None)), chan.get_their_node_id()) {
                                                                        }
                                                                        if total_value >= msgs::MAX_VALUE_MSAT || total_value > data.total_msat  {
                                                                                for htlc in htlcs.iter() {
 +                                                                                      let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
 +                                                                                      htlc_msat_height_data.extend_from_slice(
 +                                                                                              &byte_utils::be32_to_array(
 +                                                                                                      self.latest_block_height.load(Ordering::Acquire)
 +                                                                                                              as u32,
 +                                                                                              ),
 +                                                                                      );
                                                                                        failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
                                                                                                        short_channel_id: htlc.prev_hop.short_channel_id,
                                                                                                        htlc_id: htlc.prev_hop.htlc_id,
                                                                                                        incoming_packet_shared_secret: htlc.prev_hop.incoming_packet_shared_secret,
                                                                                                }), payment_hash,
 -                                                                                              HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(htlc.value).to_vec() }
 +                                                                                              HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data }
                                                                                        ));
                                                                                }
                                                                        } else if total_value == data.total_msat {
                if let Some(mut sources) = removed_source {
                        for htlc in sources.drain(..) {
                                if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
 +                              let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
 +                              htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
 +                                      self.latest_block_height.load(Ordering::Acquire) as u32,
 +                              ));
                                self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
                                                HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
 -                                              HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(htlc.value).to_vec() });
 +                                              HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data });
                        }
                        true
                } else { false }
                                match &onion_error {
                                        &HTLCFailReason::LightningError { ref err } => {
  #[cfg(test)]
 -                                              let (channel_update, payment_retryable, onion_error_code) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
 +                                              let (channel_update, payment_retryable, onion_error_code, onion_error_data) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
  #[cfg(not(test))]
 -                                              let (channel_update, payment_retryable, _) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
 +                                              let (channel_update, payment_retryable, _, _) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
                                                // TODO: If we decided to blame ourselves (or one of our channels) in
                                                // process_onion_failure we should close that channel as it implies our
                                                // next-hop is needlessly blaming us!
                                                                payment_hash: payment_hash.clone(),
                                                                rejected_by_dest: !payment_retryable,
  #[cfg(test)]
 -                                                              error_code: onion_error_code
 +                                                              error_code: onion_error_code,
 +#[cfg(test)]
 +                                                              error_data: onion_error_data
                                                        }
                                                );
                                        },
                                        &HTLCFailReason::Reason {
  #[cfg(test)]
                                                        ref failure_code,
 +#[cfg(test)]
 +                                                      ref data,
                                                        .. } => {
                                                // we get a fail_malformed_htlc from the first hop
                                                // TODO: We'd like to generate a PaymentFailureNetworkUpdate for temporary
                                                                rejected_by_dest: path.len() == 1,
  #[cfg(test)]
                                                                error_code: Some(*failure_code),
 +#[cfg(test)]
 +                                                              error_data: Some(data.clone()),
                                                        }
                                                );
                                        }
                        for htlc in sources.drain(..) {
                                if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
                                if (is_mpp && !valid_mpp) || (!is_mpp && (htlc.value < expected_amount || htlc.value > expected_amount * 2)) {
 -                                      let mut htlc_msat_data = byte_utils::be64_to_array(htlc.value).to_vec();
 -                                      let mut height_data = byte_utils::be32_to_array(self.latest_block_height.load(Ordering::Acquire) as u32).to_vec();
 -                                      htlc_msat_data.append(&mut height_data);
 +                                      let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
 +                                      htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
 +                                              self.latest_block_height.load(Ordering::Acquire) as u32,
 +                                      ));
                                        self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
                                                                         HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash,
 -                                                                       HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_data });
 +                                                                       HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_height_data });
                                } else {
                                        match self.claim_funds_from_hop(channel_state.as_mut().unwrap(), htlc.prev_hop, payment_preimage) {
                                                Err(Some(e)) => {
                };
                // Because we have exclusive ownership of the channel here we can release the channel_state
                // lock before add_monitor
-               if let Err(e) = self.monitor.add_monitor(monitor_update.get_funding_txo().unwrap(), monitor_update) {
+               if let Err(e) = self.monitor.add_monitor(monitor_update.get_funding_txo(), monitor_update) {
                        match e {
                                ChannelMonitorUpdateErr::PermanentFailure => {
                                        // Note that we reply with the new channel_id in error messages if we gave up on the
index 21d8686033059386d40fcc3777b1988563dbaa1e,add73e4b336a6b4501826aff433340e241f3fc81..11cf69b40c4715eade4da12c6b24c568f7012d30
@@@ -3766,7 -3766,7 +3766,7 @@@ fn test_no_txn_manager_serialize_deseri
        keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new()));
        let (_, nodes_0_deserialized_tmp) = {
                let mut channel_monitors = HashMap::new();
-               channel_monitors.insert(chan_0_monitor.get_funding_txo().unwrap(), &mut chan_0_monitor);
+               channel_monitors.insert(chan_0_monitor.get_funding_txo(), &mut chan_0_monitor);
                <(Sha256dHash, ChannelManager<EnforcingChannelKeys, &test_utils::TestChannelMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
                        default_config: config,
                        keys_manager: &keys_manager,
        nodes_0_deserialized = nodes_0_deserialized_tmp;
        assert!(nodes_0_read.is_empty());
  
-       assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo().unwrap(), chan_0_monitor).is_ok());
+       assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo(), chan_0_monitor).is_ok());
        nodes[0].node = &nodes_0_deserialized;
        nodes[0].block_notifier.register_listener(nodes[0].node);
        assert_eq!(nodes[0].node.list_channels().len(), 1);
@@@ -3839,7 -3839,7 +3839,7 @@@ fn test_simple_manager_serialize_deseri
        keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new()));
        let (_, nodes_0_deserialized_tmp) = {
                let mut channel_monitors = HashMap::new();
-               channel_monitors.insert(chan_0_monitor.get_funding_txo().unwrap(), &mut chan_0_monitor);
+               channel_monitors.insert(chan_0_monitor.get_funding_txo(), &mut chan_0_monitor);
                <(Sha256dHash, ChannelManager<EnforcingChannelKeys, &test_utils::TestChannelMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
                        default_config: UserConfig::default(),
                        keys_manager: &keys_manager,
        nodes_0_deserialized = nodes_0_deserialized_tmp;
        assert!(nodes_0_read.is_empty());
  
-       assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo().unwrap(), chan_0_monitor).is_ok());
+       assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo(), chan_0_monitor).is_ok());
        nodes[0].node = &nodes_0_deserialized;
        check_added_monitors!(nodes[0], 1);
  
@@@ -3935,7 -3935,7 +3935,7 @@@ fn test_manager_serialize_deserialize_i
                monitor: nodes[0].chan_monitor,
                tx_broadcaster: nodes[0].tx_broadcaster.clone(),
                logger: Arc::new(test_utils::TestLogger::new()),
-               channel_monitors: &mut node_0_stale_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().unwrap(), monitor) }).collect(),
+               channel_monitors: &mut node_0_stale_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo(), monitor) }).collect(),
        }) { } else {
                panic!("If the monitor(s) are stale, this indicates a bug and we should get an Err return");
        };
                monitor: nodes[0].chan_monitor,
                tx_broadcaster: nodes[0].tx_broadcaster.clone(),
                logger: Arc::new(test_utils::TestLogger::new()),
-               channel_monitors: &mut node_0_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().unwrap(), monitor) }).collect(),
+               channel_monitors: &mut node_0_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo(), monitor) }).collect(),
        }).unwrap();
        nodes_0_deserialized = nodes_0_deserialized_tmp;
        assert!(nodes_0_read.is_empty());
        }
  
        for monitor in node_0_monitors.drain(..) {
-               assert!(nodes[0].chan_monitor.add_monitor(monitor.get_funding_txo().unwrap(), monitor).is_ok());
+               assert!(nodes[0].chan_monitor.add_monitor(monitor.get_funding_txo(), monitor).is_ok());
                check_added_monitors!(nodes[0], 1);
        }
        nodes[0].node = &nodes_0_deserialized;
@@@ -5326,7 -5326,7 +5326,7 @@@ fn run_onion_failure_test_with_fail_int
  
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
 -      if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code } = &events[0] {
 +      if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code, error_data: _ } = &events[0] {
                assert_eq!(*rejected_by_dest, !expected_retryable);
                assert_eq!(*error_code, expected_error_code);
        } else {
@@@ -6914,20 -6914,9 +6914,20 @@@ fn test_check_htlc_underpaying() 
  
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
 -      if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code } = &events[0] {
 +      if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code, ref error_data } = &events[0] {
                assert_eq!(*rejected_by_dest, true);
                assert_eq!(error_code.unwrap(), 0x4000|15);
 +              // 10_000 msat as u64, followed by a height of 99 as u32
 +              assert_eq!(&error_data.as_ref().unwrap()[..], &[
 +                      ((10_000u64 >> 7*8) & 0xff) as u8,
 +                      ((10_000u64 >> 6*8) & 0xff) as u8,
 +                      ((10_000u64 >> 5*8) & 0xff) as u8,
 +                      ((10_000u64 >> 4*8) & 0xff) as u8,
 +                      ((10_000u64 >> 3*8) & 0xff) as u8,
 +                      ((10_000u64 >> 2*8) & 0xff) as u8,
 +                      ((10_000u64 >> 1*8) & 0xff) as u8,
 +                      ((10_000u64 >> 0*8) & 0xff) as u8,
 +                      0, 0, 0, 99]);
        } else {
                panic!("Unexpected event");
        }