Merge pull request #2436 from tnull/2023-07-improve-router-logging
authorJeffrey Czyz <jkczyz@gmail.com>
Fri, 21 Jul 2023 20:25:57 +0000 (15:25 -0500)
committerGitHub <noreply@github.com>
Fri, 21 Jul 2023 20:25:57 +0000 (15:25 -0500)
Improve router logging and update documentation

12 files changed:
ci/check-compiles.sh
lightning/src/chain/mod.rs
lightning/src/events/bump_transaction.rs
lightning/src/ln/channel.rs
lightning/src/ln/features.rs
lightning/src/ln/peer_handler.rs
lightning/src/ln/priv_short_conf_tests.rs
lightning/src/offers/invoice.rs
lightning/src/offers/parse.rs
lightning/src/onion_message/offers.rs
lightning/src/sign/mod.rs
lightning/src/util/ser.rs

index 193c2b4ef1fee74d6dbbdfdaee2c09bb86cc9197..af88bceee01127667377c294c69429ae0c2e4630 100755 (executable)
@@ -7,3 +7,4 @@ cargo doc
 cargo doc --document-private-items
 cd fuzz && RUSTFLAGS="--cfg=fuzzing" cargo check --features=stdin_fuzz
 cd ../lightning && cargo check --no-default-features --features=no-std
+cd .. && RUSTC_BOOTSTRAP=1 RUSTFLAGS="--cfg=c_bindings" cargo check -Z avoid-dev-deps
index a32bcb29901fc1182c21b8e3e02ce9c6f92b3bb7..236b10a7b19d1288b5d74f784d066c2a8bacc169 100644 (file)
@@ -391,5 +391,7 @@ where
 }
 
 /// A unique identifier to track each pending output claim within a [`ChannelMonitor`].
+///
+/// This is not exported to bindings users as we just use [u8; 32] directly.
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
 pub struct ClaimId(pub [u8; 32]);
index 751a43e0233a09ed815cc324be4446392fa97859..5963da8e9f213efd023a21f6570369b8775ee2a5 100644 (file)
@@ -341,6 +341,7 @@ pub enum BumpTransactionEvent {
 /// An input that must be included in a transaction when performing coin selection through
 /// [`CoinSelectionSource::select_confirmed_utxos`]. It is guaranteed to be a SegWit input, so it
 /// must have an empty [`TxIn::script_sig`] when spent.
+#[derive(Clone, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)]
 pub struct Input {
        /// The unique identifier of the input.
        pub outpoint: OutPoint,
@@ -354,7 +355,7 @@ pub struct Input {
 
 /// An unspent transaction output that is available to spend resulting from a successful
 /// [`CoinSelection`] attempt.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)]
 pub struct Utxo {
        /// The unique identifier of the output.
        pub outpoint: OutPoint,
@@ -421,6 +422,7 @@ impl Utxo {
 
 /// The result of a successful coin selection attempt for a transaction requiring additional UTXOs
 /// to cover its fees.
+#[derive(Clone, Debug)]
 pub struct CoinSelection {
        /// The set of UTXOs (with at least 1 confirmation) to spend and use within a transaction
        /// requiring additional fees.
index e74f659fbd09ba04a6fc537fd9525b3c8f56a7c6..e559ac3355007808a18abe58acb562cf8e2c112f 100644 (file)
@@ -997,9 +997,9 @@ impl<Signer: ChannelSigner> ChannelContext<Signer> {
                &self.channel_type
        }
 
-       /// Guaranteed to be Some after both ChannelReady messages have been exchanged (and, thus,
-       /// is_usable() returns true).
-       /// Allowed in any state (including after shutdown)
+       /// Gets the channel's `short_channel_id`.
+       ///
+       /// Will return `None` if the channel hasn't been confirmed yet.
        pub fn get_short_channel_id(&self) -> Option<u64> {
                self.short_channel_id
        }
@@ -4887,6 +4887,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
                }
 
+               let short_channel_id = self.context.get_short_channel_id()
+                       .ok_or(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel has not been confirmed yet".to_owned()))?;
                let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
                        .map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?);
                let counterparty_node_id = NodeId::from_pubkey(&self.context.get_counterparty_node_id());
@@ -4895,7 +4897,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                let msg = msgs::UnsignedChannelAnnouncement {
                        features: channelmanager::provided_channel_features(&user_config),
                        chain_hash,
-                       short_channel_id: self.context.get_short_channel_id().unwrap(),
+                       short_channel_id,
                        node_id_1: if were_node_one { node_id } else { counterparty_node_id },
                        node_id_2: if were_node_one { counterparty_node_id } else { node_id },
                        bitcoin_key_1: NodeId::from_pubkey(if were_node_one { &self.context.get_holder_pubkeys().funding_pubkey } else { self.context.counterparty_funding_pubkey() }),
@@ -4953,11 +4955,16 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        },
                        Ok(v) => v
                };
+               let short_channel_id = match self.context.get_short_channel_id() {
+                       Some(scid) => scid,
+                       None => return None,
+               };
+
                self.context.announcement_sigs_state = AnnouncementSigsState::MessageSent;
 
                Some(msgs::AnnouncementSignatures {
                        channel_id: self.context.channel_id(),
-                       short_channel_id: self.context.get_short_channel_id().unwrap(),
+                       short_channel_id,
                        node_signature: our_node_sig,
                        bitcoin_signature: our_bitcoin_sig,
                })
index 6309eea413adb4d913c45379baad6eca16bdeb83..5de383b1f4469273207441892a90d1d2c063494d 100644 (file)
@@ -728,7 +728,7 @@ impl<T: sealed::Context> Features<T> {
        }
 
        /// Returns true if this `Features` object contains required features unknown by `other`.
-       pub fn requires_unknown_bits_from(&self, other: &Features<T>) -> bool {
+       pub fn requires_unknown_bits_from(&self, other: &Self) -> bool {
                // Bitwise AND-ing with all even bits set except for known features will select required
                // unknown features.
                self.flags.iter().enumerate().any(|(i, &byte)| {
index 60230af78eff6bf0ccf062a37a68926cdb96e926..1a39bbb3ae408e7047159e6d9f3f63267188b61f 100644 (file)
@@ -641,6 +641,9 @@ pub type SimpleRefPeerManager<
 /// A generic trait which is implemented for all [`PeerManager`]s. This makes bounding functions or
 /// structs on any [`PeerManager`] much simpler as only this trait is needed as a bound, rather
 /// than the full set of bounds on [`PeerManager`] itself.
+///
+/// This is not exported to bindings users as general cover traits aren't useful in other
+/// languages.
 #[allow(missing_docs)]
 pub trait APeerManager {
        type Descriptor: SocketDescriptor;
index 2c8f824eab61512651e2a59e0f40355c8549f522..40ab57c3e4ccac0590623fc4c34f1cb4f76b1eeb 100644 (file)
@@ -1009,3 +1009,38 @@ fn test_connect_before_funding() {
        connect_blocks(&nodes[0], 1);
        connect_blocks(&nodes[1], 1);
 }
+
+#[test]
+fn test_0conf_ann_sigs_racing_conf() {
+       // Previously we had a bug where we'd panic when receiving a counterparty's
+       // announcement_signatures message for a 0conf channel pending confirmation on-chain. Here we
+       // check that we just error out, ignore the announcement_signatures message, and proceed
+       // instead.
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let mut chan_config = test_default_channel_config();
+       chan_config.manually_accept_inbound_channels = true;
+
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]);
+       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       // This is the default but we force it on anyway
+       chan_config.channel_handshake_config.announced_channel = true;
+       let (tx, ..) = open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config));
+
+       // We can use the channel immediately, but we can't announce it until we get 6+ confirmations
+       send_payment(&nodes[0], &[&nodes[1]], 100_000);
+
+       let scid = confirm_transaction(&nodes[0], &tx);
+       let as_announcement_sigs = get_event_msg!(nodes[0], MessageSendEvent::SendAnnouncementSignatures, nodes[1].node.get_our_node_id());
+
+       // Handling the announcement_signatures prior to the first confirmation would panic before.
+       nodes[1].node.handle_announcement_signatures(&nodes[0].node.get_our_node_id(), &as_announcement_sigs);
+
+       assert_eq!(confirm_transaction(&nodes[1], &tx), scid);
+       let bs_announcement_sigs = get_event_msg!(nodes[1], MessageSendEvent::SendAnnouncementSignatures, nodes[0].node.get_our_node_id());
+
+       nodes[0].node.handle_announcement_signatures(&nodes[1].node.get_our_node_id(), &bs_announcement_sigs);
+       let as_announcement = nodes[0].node.get_and_clear_pending_msg_events();
+       assert_eq!(as_announcement.len(), 1);
+}
index 279e31dd66a78cf86691ca9c9e2d7f83653a189b..c3d4500aaebbb0b8f0c2d6abdb4f4179bfad3cf0 100644 (file)
@@ -475,6 +475,9 @@ impl Bolt12Invoice {
        ///
        /// Blinded paths provide recipient privacy by obfuscating its node id. Note, however, that this
        /// privacy is lost if a public node id is used for [`Bolt12Invoice::signing_pubkey`].
+       ///
+       /// This is not exported to bindings users as slices with non-reference types cannot be ABI
+       /// matched in another language.
        pub fn payment_paths(&self) -> &[(BlindedPayInfo, BlindedPath)] {
                &self.contents.fields().payment_paths[..]
        }
index b722499f251ad5213e1d27bbdb613da2c6aba2f7..e9477086ee981358aea419416e676e4b682c1fc1 100644 (file)
@@ -116,7 +116,7 @@ impl<T: SeekReadable> TryFrom<Vec<u8>> for ParsedMessage<T> {
 }
 
 /// Error when parsing a bech32 encoded message using [`str::parse`].
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub enum Bolt12ParseError {
        /// The bech32 encoding does not conform to the BOLT 12 requirements for continuing messages
        /// across multiple parts (i.e., '+' followed by whitespace).
@@ -135,7 +135,7 @@ pub enum Bolt12ParseError {
 }
 
 /// Error when interpreting a TLV stream as a specific type.
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub enum Bolt12SemanticError {
        /// The current [`std::time::SystemTime`] is past the offer or invoice's expiration.
        AlreadyExpired,
index 9a1f59b84592eac465b718e8ee89c6037c24423e..de373bda1bce81b104f1cd616be1ea4fb3e0b756 100644 (file)
@@ -38,7 +38,7 @@ pub trait OffersMessageHandler {
 /// Possible BOLT 12 Offers messages sent and received via an [`OnionMessage`].
 ///
 /// [`OnionMessage`]: crate::ln::msgs::OnionMessage
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub enum OffersMessage {
        /// A request for a [`Bolt12Invoice`] for a particular [`Offer`].
        ///
index c11d47e9ff9c32e119c34961017b79be1665573f..a71bdae88768c1b7f97da626af9291589205e736 100644 (file)
@@ -1312,7 +1312,7 @@ impl KeysManager {
        ///
        /// May panic if the [`SpendableOutputDescriptor`]s were not generated by channels which used
        /// this [`KeysManager`] or one of the [`InMemorySigner`] created by this [`KeysManager`].
-       pub fn sign_spendable_outputs_psbt<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], psbt: &mut PartiallySignedTransaction, secp_ctx: &Secp256k1<C>) -> Result<(), ()> {
+       pub fn sign_spendable_outputs_psbt<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], mut psbt: PartiallySignedTransaction, secp_ctx: &Secp256k1<C>) -> Result<PartiallySignedTransaction, ()> {
                let mut keys_cache: Option<(InMemorySigner, [u8; 32])> = None;
                for outp in descriptors {
                        match outp {
@@ -1374,7 +1374,7 @@ impl KeysManager {
                        }
                }
 
-               Ok(())
+               Ok(psbt)
        }
 
        /// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
@@ -1396,7 +1396,7 @@ impl KeysManager {
        /// this [`KeysManager`] or one of the [`InMemorySigner`] created by this [`KeysManager`].
        pub fn spend_spendable_outputs<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>, change_destination_script: Script, feerate_sat_per_1000_weight: u32, locktime: Option<PackedLockTime>, secp_ctx: &Secp256k1<C>) -> Result<Transaction, ()> {
                let (mut psbt, expected_max_weight) = SpendableOutputDescriptor::create_spendable_outputs_psbt(descriptors, outputs, change_destination_script, feerate_sat_per_1000_weight, locktime)?;
-               self.sign_spendable_outputs_psbt(descriptors, &mut psbt, secp_ctx)?;
+               psbt = self.sign_spendable_outputs_psbt(descriptors, psbt, secp_ctx)?;
 
                let spend_tx = psbt.extract_tx();
 
index 0e510760e62a3725055cecf3394a17b5b879af9a..1eb5e7424c82343d0bb88aef7df632dc408563f0 100644 (file)
@@ -358,6 +358,7 @@ impl Readable for U48 {
 /// encoded in several different ways, which we must check for at deserialization-time. Thus, if
 /// you're looking for an example of a variable-length integer to use for your own project, move
 /// along, this is a rather poor design.
+#[derive(Clone, Copy, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)]
 pub struct BigSize(pub u64);
 impl Writeable for BigSize {
        #[inline]
@@ -1351,6 +1352,7 @@ impl Readable for Hostname {
        }
 }
 
+/// This is not exported to bindings users as `Duration`s are simply mapped as ints.
 impl Writeable for Duration {
        #[inline]
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
@@ -1358,6 +1360,7 @@ impl Writeable for Duration {
                self.subsec_nanos().write(w)
        }
 }
+/// This is not exported to bindings users as `Duration`s are simply mapped as ints.
 impl Readable for Duration {
        #[inline]
        fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {