Merge pull request #920 from TheBlueMatt/2021-05-tlv-ser

Rebroadcast channel_announcements when we broadcast a node_announce

Author: TheBlueMatt

.github/workflows/build.yml

@@ -144,12 +144,12 @@ jobs:
         uses: actions/cache@v2
         with:
           path: lightning/net_graph-2021-02-12.bin
-          key: net_graph-2021-02-12
+          key: ldk-net_graph-05f0c5a0d772-2020-02-12.bin
       - name: Fetch routing graph snapshot
         if: steps.cache-graph.outputs.cache-hit != 'true'
         run: |
-          wget -O lightning/net_graph-2021-02-12.bin https://bitcoin.ninja/ldk-net_graph-879e309c128-2020-02-12.bin
-          if [ "$(sha256sum lightning/net_graph-2021-02-12.bin | awk '{ print $1 }')" != "890a1f80dfb6ef674a1e4ff0f23cd73d740731c395f99d85abbede0cfbb701ab" ]; then
+          wget -O lightning/net_graph-2021-02-12.bin https://bitcoin.ninja/ldk-net_graph-05f0c5a0d772-2020-02-12.bin
+          if [ "$(sha256sum lightning/net_graph-2021-02-12.bin | awk '{ print $1 }')" != "7116fca78551fedc714a604cec0ad1ca66caa77bb4d0051290258e7a10e0c6e7" ]; then
             echo "Bad hash"
             exit 1
           fi

CONTRIBUTING.md

@@ -113,6 +113,14 @@ rustup component add clippy
 cargo clippy
 ```
 
+Significant structures that users persist should always have their serialization methods (usually
+`Writeable::write` and `ReadableArgs::read`) begin with
+`write_ver_prefix!()`/`read_ver_prefix!()` calls, and end with calls to
+`write_tlv_fields!()`/`read_tlv_fields!()`.
+
+Updates to the serialized format which has implications for backwards or forwards compatibility
+must be included in release notes.
+
 Security
 --------
 

lightning/src/chain/channelmonitor.rs

@@ -502,9 +502,6 @@ enum OnchainEvent {
 	},
 }
 
-const SERIALIZATION_VERSION: u8 = 1;
-const MIN_SERIALIZATION_VERSION: u8 = 1;
-
 #[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 pub(crate) enum ChannelMonitorUpdateStep {
@@ -805,17 +802,17 @@ impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
 
 impl<Signer: Sign> Writeable for ChannelMonitor<Signer> {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
-		//TODO: We still write out all the serialization here manually instead of using the fancy
-		//serialization framework we have, we should migrate things over to it.
-		writer.write_all(&[SERIALIZATION_VERSION; 1])?;
-		writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
-
 		self.inner.lock().unwrap().write(writer)
 	}
 }
 
+const SERIALIZATION_VERSION: u8 = 1;
+const MIN_SERIALIZATION_VERSION: u8 = 1;
+
 impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
+		write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
+
 		self.latest_update_id.write(writer)?;
 
 		// Set in initial Channel-object creation, so should always be set by now:
@@ -991,6 +988,8 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 		self.lockdown_from_offchain.write(writer)?;
 		self.holder_tx_signed.write(writer)?;
 
+		write_tlv_fields!(writer, {}, {});
+
 		Ok(())
 	}
 }
@@ -2754,11 +2753,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 			}
 		}
 
-		let _ver: u8 = Readable::read(reader)?;
-		let min_ver: u8 = Readable::read(reader)?;
-		if min_ver > SERIALIZATION_VERSION {
-			return Err(DecodeError::UnknownVersion);
-		}
+		let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
 		let latest_update_id: u64 = Readable::read(reader)?;
 		let commitment_transaction_number_obscure_factor = <U48 as Readable>::read(reader)?.0;
@@ -2979,6 +2974,8 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 		let lockdown_from_offchain = Readable::read(reader)?;
 		let holder_tx_signed = Readable::read(reader)?;
 
+		read_tlv_fields!(reader, {}, {});
+
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 

lightning/src/chain/keysinterface.rs

@@ -704,10 +704,15 @@ impl BaseSign for InMemorySigner {
 	}
 }
 
+const SERIALIZATION_VERSION: u8 = 1;
+const MIN_SERIALIZATION_VERSION: u8 = 1;
+
 impl Sign for InMemorySigner {}
 
 impl Writeable for InMemorySigner {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
+		write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
+
 		self.funding_key.write(writer)?;
 		self.revocation_base_key.write(writer)?;
 		self.payment_key.write(writer)?;
@@ -718,12 +723,16 @@ impl Writeable for InMemorySigner {
 		self.channel_value_satoshis.write(writer)?;
 		self.channel_keys_id.write(writer)?;
 
+		write_tlv_fields!(writer, {}, {});
+
 		Ok(())
 	}
 }
 
 impl Readable for InMemorySigner {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
+
 		let funding_key = Readable::read(reader)?;
 		let revocation_base_key = Readable::read(reader)?;
 		let payment_key = Readable::read(reader)?;
@@ -739,6 +748,8 @@ impl Readable for InMemorySigner {
 			                                     &htlc_base_key);
 		let keys_id = Readable::read(reader)?;
 
+		read_tlv_fields!(reader, {}, {});
+
 		Ok(InMemorySigner {
 			funding_key,
 			revocation_base_key,

lightning/src/ln/channel.rs

@@ -15,6 +15,7 @@ use bitcoin::consensus::encode;
 
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hashes::sha256d::Hash as Sha256d;
 use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
 
 use bitcoin::secp256k1::key::{PublicKey,SecretKey};
@@ -420,6 +421,10 @@ pub(super) struct Channel<Signer: Sign> {
 
 	channel_update_status: ChannelUpdateStatus,
 
+	/// Our counterparty's channel_announcement signatures provided in announcement_signatures.
+	/// This can be used to rebroadcast the channel_announcement message later.
+	announcement_sigs: Option<(Signature, Signature)>,
+
 	// We save these values so we can make sure `next_local_commit_tx_fee_msat` and
 	// `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will
 	// be, by comparing the cached values to the fee of the tranaction generated by
@@ -621,6 +626,8 @@ impl<Signer: Sign> Channel<Signer> {
 
 			channel_update_status: ChannelUpdateStatus::Enabled,
 
+			announcement_sigs: None,
+
 			#[cfg(any(test, feature = "fuzztarget"))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
 			#[cfg(any(test, feature = "fuzztarget"))]
@@ -862,6 +869,8 @@ impl<Signer: Sign> Channel<Signer> {
 
 			channel_update_status: ChannelUpdateStatus::Enabled,
 
+			announcement_sigs: None,
+
 			#[cfg(any(test, feature = "fuzztarget"))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
 			#[cfg(any(test, feature = "fuzztarget"))]
@@ -3822,6 +3831,8 @@ impl<Signer: Sign> Channel<Signer> {
 	/// closing).
 	/// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
 	/// https://github.com/lightningnetwork/lightning-rfc/issues/468
+	///
+	/// This will only return ChannelError::Ignore upon failure.
 	pub fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
 		if !self.config.announced_channel {
 			return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned()));
@@ -3852,6 +3863,63 @@ impl<Signer: Sign> Channel<Signer> {
 		Ok((msg, sig))
 	}
 
+	/// Signs the given channel announcement, returning a ChannelError::Ignore if no keys are
+	/// available.
+	fn sign_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, msghash: secp256k1::Message, announcement: msgs::UnsignedChannelAnnouncement, our_bitcoin_sig: Signature) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+		if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
+			let were_node_one = announcement.node_id_1 == our_node_id;
+
+			let our_node_sig = self.secp_ctx.sign(&msghash, our_node_secret);
+			Ok(msgs::ChannelAnnouncement {
+				node_signature_1: if were_node_one { our_node_sig } else { their_node_sig },
+				node_signature_2: if were_node_one { their_node_sig } else { our_node_sig },
+				bitcoin_signature_1: if were_node_one { our_bitcoin_sig } else { their_bitcoin_sig },
+				bitcoin_signature_2: if were_node_one { their_bitcoin_sig } else { our_bitcoin_sig },
+				contents: announcement,
+			})
+		} else {
+			Err(ChannelError::Ignore("Attempted to sign channel announcement before we'd received announcement_signatures".to_string()))
+		}
+	}
+
+	/// Processes an incoming announcement_signatures message, providing a fully-signed
+	/// channel_announcement message which we can broadcast and storing our counterparty's
+	/// signatures for later reconstruction/rebroadcast of the channel_announcement.
+	pub fn announcement_signatures(&mut self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash, msg: &msgs::AnnouncementSignatures) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+		let (announcement, our_bitcoin_sig) = self.get_channel_announcement(our_node_id.clone(), chain_hash)?;
+
+		let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
+
+		if self.secp_ctx.verify(&msghash, &msg.node_signature, &self.get_counterparty_node_id()).is_err() {
+			return Err(ChannelError::Close(format!(
+				"Bad announcement_signatures. Failed to verify node_signature. UnsignedChannelAnnouncement used for verification is {:?}. their_node_key is {:?}",
+				 &announcement, self.get_counterparty_node_id())));
+		}
+		if self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, self.counterparty_funding_pubkey()).is_err() {
+			return Err(ChannelError::Close(format!(
+				"Bad announcement_signatures. Failed to verify bitcoin_signature. UnsignedChannelAnnouncement used for verification is {:?}. their_bitcoin_key is ({:?})",
+				&announcement, self.counterparty_funding_pubkey())));
+		}
+
+		self.announcement_sigs = Some((msg.node_signature, msg.bitcoin_signature));
+
+		self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig)
+	}
+
+	/// Gets a signed channel_announcement for this channel, if we previously received an
+	/// announcement_signatures from our counterparty.
+	pub fn get_signed_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash) -> Option<msgs::ChannelAnnouncement> {
+		let (announcement, our_bitcoin_sig) = match self.get_channel_announcement(our_node_id.clone(), chain_hash) {
+			Ok(res) => res,
+			Err(_) => return None,
+		};
+		let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
+		match self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig) {
+			Ok(res) => Some(res),
+			Err(_) => None,
+		}
+	}
+
 	/// May panic if called on a channel that wasn't immediately-previously
 	/// self.remove_uncommitted_htlcs_and_mark_paused()'d
 	pub fn get_channel_reestablish<L: Deref>(&self, logger: &L) -> msgs::ChannelReestablish where L::Target: Logger {
@@ -4375,8 +4443,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 		// Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been
 		// called.
 
-		writer.write_all(&[SERIALIZATION_VERSION; 1])?;
-		writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
+		write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
 		self.user_id.write(writer)?;
 		self.config.write(writer)?;
@@ -4565,6 +4632,9 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 		self.commitment_secrets.write(writer)?;
 
 		self.channel_update_status.write(writer)?;
+
+		write_tlv_fields!(writer, {}, {(0, self.announcement_sigs)});
+
 		Ok(())
 	}
 }
@@ -4573,11 +4643,7 @@ const MAX_ALLOC_SIZE: usize = 64*1024;
 impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 		where K::Target: KeysInterface<Signer = Signer> {
 	fn read<R : ::std::io::Read>(reader: &mut R, keys_source: &'a K) -> Result<Self, DecodeError> {
-		let _ver: u8 = Readable::read(reader)?;
-		let min_ver: u8 = Readable::read(reader)?;
-		if min_ver > SERIALIZATION_VERSION {
-			return Err(DecodeError::UnknownVersion);
-		}
+		let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
 		let user_id = Readable::read(reader)?;
 		let config: ChannelConfig = Readable::read(reader)?;
@@ -4739,6 +4805,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
 		let channel_update_status = Readable::read(reader)?;
 
+		let mut announcement_sigs = None;
+		read_tlv_fields!(reader, {}, {(0, announcement_sigs)});
+
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
 
@@ -4815,6 +4884,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
 			channel_update_status,
 
+			announcement_sigs,
+
 			#[cfg(any(test, feature = "fuzztarget"))]
 			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
 			#[cfg(any(test, feature = "fuzztarget"))]