Merge pull request #1271 from tnull/rename_payee_struct
authorvalentinewallace <valentinewallace@users.noreply.github.com>
Mon, 24 Jan 2022 16:34:48 +0000 (11:34 -0500)
committerGitHub <noreply@github.com>
Mon, 24 Jan 2022 16:34:48 +0000 (11:34 -0500)
Rename `Payee` to `PaymentParameters`

lightning-background-processor/src/lib.rs
lightning/src/chain/keysinterface.rs
lightning/src/ln/channelmanager.rs

index 2dbc8053b4eb1247e5d64dc482e1817ad5db08ec..b60dc6c830432c47777faa4a7b84db9e666ba954 100644 (file)
@@ -61,7 +61,7 @@ const FRESHNESS_TIMER: u64 = 60;
 const FRESHNESS_TIMER: u64 = 1;
 
 #[cfg(all(not(test), not(debug_assertions)))]
-const PING_TIMER: u64 = 5;
+const PING_TIMER: u64 = 10;
 /// Signature operations take a lot longer without compiler optimisations.
 /// Increasing the ping timer allows for this but slower devices will be disconnected if the
 /// timeout is reached.
@@ -219,11 +219,17 @@ impl BackgroundProcessor {
                        let mut have_pruned = false;
 
                        loop {
-                               peer_manager.process_events();
+                               peer_manager.process_events(); // Note that this may block on ChannelManager's locking
                                channel_manager.process_pending_events(&event_handler);
                                chain_monitor.process_pending_events(&event_handler);
+
+                               // We wait up to 100ms, but track how long it takes to detect being put to sleep,
+                               // see `await_start`'s use below.
+                               let await_start = Instant::now();
                                let updates_available =
                                        channel_manager.await_persistable_update_timeout(Duration::from_millis(100));
+                               let await_time = await_start.elapsed();
+
                                if updates_available {
                                        log_trace!(logger, "Persisting ChannelManager...");
                                        persister.persist_manager(&*channel_manager)?;
@@ -232,22 +238,27 @@ impl BackgroundProcessor {
                                // Exit the loop if the background processor was requested to stop.
                                if stop_thread.load(Ordering::Acquire) == true {
                                        log_trace!(logger, "Terminating background processor.");
-                                       return Ok(());
+                                       break;
                                }
                                if last_freshness_call.elapsed().as_secs() > FRESHNESS_TIMER {
                                        log_trace!(logger, "Calling ChannelManager's timer_tick_occurred");
                                        channel_manager.timer_tick_occurred();
                                        last_freshness_call = Instant::now();
                                }
-                               if last_ping_call.elapsed().as_secs() > PING_TIMER * 2 {
+                               if await_time > Duration::from_secs(1) {
                                        // On various platforms, we may be starved of CPU cycles for several reasons.
                                        // E.g. on iOS, if we've been in the background, we will be entirely paused.
                                        // Similarly, if we're on a desktop platform and the device has been asleep, we
                                        // may not get any cycles.
-                                       // In any case, if we've been entirely paused for more than double our ping
-                                       // timer, we should have disconnected all sockets by now (and they're probably
-                                       // dead anyway), so disconnect them by calling `timer_tick_occurred()` twice.
-                                       log_trace!(logger, "Awoke after more than double our ping timer, disconnecting peers.");
+                                       // We detect this by checking if our max-100ms-sleep, above, ran longer than a
+                                       // full second, at which point we assume sockets may have been killed (they
+                                       // appear to be at least on some platforms, even if it has only been a second).
+                                       // Note that we have to take care to not get here just because user event
+                                       // processing was slow at the top of the loop. For example, the sample client
+                                       // may call Bitcoin Core RPCs during event handling, which very often takes
+                                       // more than a handful of seconds to complete, and shouldn't disconnect all our
+                                       // peers.
+                                       log_trace!(logger, "100ms sleep took more than a second, disconnecting peers.");
                                        peer_manager.disconnect_all_peers();
                                        last_ping_call = Instant::now();
                                } else if last_ping_call.elapsed().as_secs() > PING_TIMER {
@@ -269,6 +280,10 @@ impl BackgroundProcessor {
                                        }
                                }
                        }
+                       // After we exit, ensure we persist the ChannelManager one final time - this avoids
+                       // some races where users quit while channel updates were in-flight, with
+                       // ChannelMonitor update(s) persisted without a corresponding ChannelManager update.
+                       persister.persist_manager(&*channel_manager)
                });
                Self { stop_thread: stop_thread_clone, thread_handle: Some(handle) }
        }
index a7965d4f3419a15c4c97460fca7cc77951521800..9a46351c7b4174c7b279684b99b2837400e7b30a 100644 (file)
@@ -526,7 +526,8 @@ impl InMemorySigner {
        /// described by descriptor, returning the witness stack for the input.
        ///
        /// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
-       /// or is not spending the outpoint described by `descriptor.outpoint`.
+       /// is not spending the outpoint described by `descriptor.outpoint`,
+       /// or if an output descriptor script_pubkey does not match the one we can spend.
        pub fn sign_counterparty_payment_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &StaticPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
                // TODO: We really should be taking the SigHashCache as a parameter here instead of
                // spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
@@ -540,6 +541,9 @@ impl InMemorySigner {
                let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
                let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
                let remotesig = secp_ctx.sign(&sighash, &self.payment_key);
+               let payment_script = bitcoin::Address::p2wpkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Bitcoin).unwrap().script_pubkey();
+
+               if payment_script != descriptor.output.script_pubkey  { return Err(()); }
 
                let mut witness = Vec::with_capacity(2);
                witness.push(remotesig.serialize_der().to_vec());
@@ -552,8 +556,9 @@ impl InMemorySigner {
        /// described by descriptor, returning the witness stack for the input.
        ///
        /// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
-       /// is not spending the outpoint described by `descriptor.outpoint`, or does not have a
-       /// sequence set to `descriptor.to_self_delay`.
+       /// is not spending the outpoint described by `descriptor.outpoint`, does not have a
+       /// sequence set to `descriptor.to_self_delay`, or if an output descriptor
+       /// script_pubkey does not match the one we can spend.
        pub fn sign_dynamic_p2wsh_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &DelayedPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
                // TODO: We really should be taking the SigHashCache as a parameter here instead of
                // spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
@@ -570,6 +575,9 @@ impl InMemorySigner {
                let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
                let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
                let local_delayedsig = secp_ctx.sign(&sighash, &delayed_payment_key);
+               let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Bitcoin).script_pubkey();
+
+               if descriptor.output.script_pubkey != payment_script { return Err(()); }
 
                let mut witness = Vec::with_capacity(3);
                witness.push(local_delayedsig.serialize_der().to_vec());
@@ -927,8 +935,9 @@ impl KeysManager {
        /// output to the given change destination (if sufficient change value remains). The
        /// transaction will have a feerate, at least, of the given value.
        ///
-       /// Returns `Err(())` if the output value is greater than the input value minus required fee or
-       /// if a descriptor was duplicated.
+       /// Returns `Err(())` if the output value is greater than the input value minus required fee,
+       /// if a descriptor was duplicated, or if an output descriptor script_pubkey 
+       /// does not match the one we can spend.
        ///
        /// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
        ///
@@ -996,7 +1005,7 @@ impl KeysManager {
                                                        self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
                                                        descriptor.channel_keys_id));
                                        }
-                                       spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
+                                       spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?;
                                },
                                SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
                                        if keys_cache.is_none() || keys_cache.as_ref().unwrap().1 != descriptor.channel_keys_id {
@@ -1004,7 +1013,7 @@ impl KeysManager {
                                                        self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
                                                        descriptor.channel_keys_id));
                                        }
-                                       spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
+                                       spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?;
                                },
                                SpendableOutputDescriptor::StaticOutput { ref output, .. } => {
                                        let derivation_idx = if output.script_pubkey == self.destination_script {
@@ -1029,6 +1038,10 @@ impl KeysManager {
                                                assert_eq!(pubkey.key, self.shutdown_pubkey);
                                        }
                                        let witness_script = bitcoin::Address::p2pkh(&pubkey, Network::Testnet).script_pubkey();
+                                       let payment_script = bitcoin::Address::p2wpkh(&pubkey, Network::Testnet).expect("uncompressed key found").script_pubkey();
+
+                                       if payment_script != output.script_pubkey { return Err(()); };
+
                                        let sighash = hash_to_message!(&bip143::SigHashCache::new(&spend_tx).signature_hash(input_idx, &witness_script, output.value, SigHashType::All)[..]);
                                        let sig = secp_ctx.sign(&sighash, &secret.private_key.key);
                                        spend_tx.input[input_idx].witness.push(sig.serialize_der().to_vec());
index e234b1ed92d2b0723a0aa23ea4cb07937e374555..0ca9dbd465be9985cb6363780c1168be63aff362 100644 (file)
@@ -6215,6 +6215,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                write_tlv_fields!(writer, {
                        (1, pending_outbound_payments_no_retry, required),
                        (3, pending_outbound_payments, required),
+                       (5, self.our_network_pubkey, required)
                });
 
                Ok(())
@@ -6509,10 +6510,13 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                // pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients.
                let mut pending_outbound_payments_no_retry: Option<HashMap<PaymentId, HashSet<[u8; 32]>>> = None;
                let mut pending_outbound_payments = None;
+               let mut received_network_pubkey: Option<PublicKey> = None;
                read_tlv_fields!(reader, {
                        (1, pending_outbound_payments_no_retry, option),
                        (3, pending_outbound_payments, option),
+                       (5, received_network_pubkey, option)
                });
+
                if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() {
                        pending_outbound_payments = Some(pending_outbound_payments_compat);
                } else if pending_outbound_payments.is_none() {
@@ -6575,6 +6579,14 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                        pending_events_read.append(&mut channel_closures);
                }
 
+               let our_network_pubkey = PublicKey::from_secret_key(&secp_ctx, &args.keys_manager.get_node_secret());
+               if let Some(network_pubkey) = received_network_pubkey {
+                       if network_pubkey != our_network_pubkey {
+                               log_error!(args.logger, "Key that was generated does not match the existing key.");
+                               return Err(DecodeError::InvalidValue);
+                       }
+               }
+
                let inbound_pmt_key_material = args.keys_manager.get_inbound_payment_key_material();
                let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material);
                let channel_manager = ChannelManager {
@@ -6597,7 +6609,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                        pending_outbound_payments: Mutex::new(pending_outbound_payments.unwrap()),
 
                        our_network_key: args.keys_manager.get_node_secret(),
-                       our_network_pubkey: PublicKey::from_secret_key(&secp_ctx, &args.keys_manager.get_node_secret()),
+                       our_network_pubkey,
                        secp_ctx,
 
                        last_node_announcement_serial: AtomicUsize::new(last_node_announcement_serial as usize),