fixup! more cleanup

Author: devrandom

lightning/src/chain/channelmonitor.rs

@@ -969,12 +969,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
 		let secp_ctx = Secp256k1::new();
 
-		let txid = initial_holder_commitment_tx.trust_txid();
+		let txid = initial_holder_commitment_tx.untrusted_txid();
 
 		// block for Rust 1.34 compat
 		let (holder_commitment_tx, current_holder_commitment_number) = {
 			let commitment_tx = &initial_holder_commitment_tx.inner;
-			let tx_keys = commitment_tx.trust_key_derivation();
+			let tx_keys = commitment_tx.untrusted_key_derivation();
 			let holder_commitment_tx = HolderSignedTx {
 				txid,
 				revocation_key: tx_keys.revocation_key,
@@ -1143,12 +1143,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// 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> {
-		let txid = holder_commitment_tx.trust_txid();
+		let txid = holder_commitment_tx.untrusted_txid();
 
 		// block for Rust 1.34 compat
 		let mut new_holder_commitment_tx = {
 			let commitment_tx = &holder_commitment_tx.inner;
-			let tx_keys = &commitment_tx.trust_key_derivation();
+			let tx_keys = &commitment_tx.untrusted_key_derivation();
 			self.current_holder_commitment_number = commitment_tx.commitment_number();
 			HolderSignedTx {
 				txid,

lightning/src/chain/keysinterface.rs

@@ -463,15 +463,15 @@ impl ChannelKeys for InMemoryChannelKeys {
 	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
 
 	fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
-		let keys = commitment_tx.trust_key_derivation();
+		let keys = commitment_tx.untrusted_key_derivation();
 
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
-		let built_tx = commitment_tx.trust_built_transaction();
+		let built_tx = commitment_tx.untrusted_built_transaction();
 		let commitment_sig = built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
 
-		let commitment_txid = commitment_tx.trust_txid();
+		let commitment_txid = commitment_tx.untrusted_txid();
 
 		let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
 		for htlc in commitment_tx.htlcs() {
@@ -492,7 +492,7 @@ impl ChannelKeys for InMemoryChannelKeys {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
-		let built_tx = commitment_tx.inner.trust_built_transaction();
+		let built_tx = commitment_tx.inner.untrusted_built_transaction();
 		let sig = built_tx.sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
 		let htlc_sigs_o = self.sign_holder_commitment_htlc_transactions(&commitment_tx, secp_ctx)?;
 		let htlc_sigs = htlc_sigs_o.iter().map(|o| o.unwrap()).collect();
@@ -505,13 +505,13 @@ impl ChannelKeys for InMemoryChannelKeys {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
-		let built_tx = holder_commitment_tx.inner.trust_built_transaction();
+		let built_tx = holder_commitment_tx.inner.untrusted_built_transaction();
 		Ok(built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
 	}
 
 	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
 		let channel_parameters = self.make_channel_parameters();
-		let channel_parameters = channel_parameters.as_holder_directed();
+		let channel_parameters = channel_parameters.as_holder_broadcastable();
 		commitment_tx.inner.get_htlc_sigs(&self.htlc_base_key, &channel_parameters, secp_ctx)
 	}
 

lightning/src/ln/chan_utils.rs

@@ -41,6 +41,11 @@ use chain;
 
 const HTLC_OUTPUT_IN_COMMITMENT_SIZE: usize = 1 + 8 + 4 + 32 + 5;
 
+pub(crate) const MAX_HTLCS: u16 = 483;
+
+// This checks that the buffer size is greater than the maximum possible size for serialized HTLCS
+const _EXCESS_BUFFER_SIZE: usize = MAX_BUF_SIZE - MAX_HTLCS as usize * HTLC_OUTPUT_IN_COMMITMENT_SIZE;
+
 pub(super) const HTLC_SUCCESS_TX_WEIGHT: u64 = 703;
 pub(super) const HTLC_TIMEOUT_TX_WEIGHT: u64 = 663;
 
@@ -298,7 +303,7 @@ pub fn derive_public_revocation_key<T: secp256k1::Verification>(secp_ctx: &Secp2
 ///
 /// These keys are assumed to be good, either because the code derived them from
 /// channel basepoints via the new function, or they were obtained via
-/// PreCalculatedTxCreationKeys.trust_key_derivation because we trusted the source of the
+/// CommitmentTransaction.untrusted_key_derivation because we trusted the source of the
 /// pre-calculated keys.
 #[derive(PartialEq, Clone)]
 pub struct TxCreationKeys {
@@ -545,9 +550,10 @@ pub fn build_htlc_transaction(prev_hash: &Txid, feerate_per_kw: u32, contest_del
 }
 
 /// Per-channel data used to build transactions in conjunction with the per-commitment data (CommitmentTransaction).
+/// The fields are organized by holder/counterparty.
 ///
-/// Normally, this is converted to the owner-independent DirectedChannelTransactionParameters
-/// before use, via the as_holder_directed and as_counterparty_directed functions.
+/// Normally, this is converted to the broadcaster/countersignatory-organized DirectedChannelTransactionParameters
+/// before use, via the as_holder_broadcastable and as_counterparty_broadcastable functions.
 #[derive(Clone)]
 pub struct ChannelTransactionParameters {
 	/// Holder public keys
@@ -583,8 +589,8 @@ impl ChannelTransactionParameters {
 	/// given that the holder is the broadcaster.
 	///
 	/// self.is_populated() must be true before calling this function.
-	pub fn as_holder_directed(&self) -> DirectedChannelTransactionParameters {
-		assert!(self.is_populated(), "self.late_parameters must be set before using as_holder_directed");
+	pub fn as_holder_broadcastable(&self) -> DirectedChannelTransactionParameters {
+		assert!(self.is_populated(), "self.late_parameters must be set before using as_holder_broadcastable");
 		DirectedChannelTransactionParameters {
 			inner: self,
 			holder_is_broadcaster: true
@@ -595,8 +601,8 @@ impl ChannelTransactionParameters {
 	/// given that the counterparty is the broadcaster.
 	///
 	/// self.is_populated() must be true before calling this function.
-	pub fn as_counterparty_directed(&self) -> DirectedChannelTransactionParameters {
-		assert!(self.is_populated(), "self.late_parameters must be set before using as_counterparty_directed");
+	pub fn as_counterparty_broadcastable(&self) -> DirectedChannelTransactionParameters {
+		assert!(self.is_populated(), "self.late_parameters must be set before using as_counterparty_broadcastable");
 		DirectedChannelTransactionParameters {
 			inner: self,
 			holder_is_broadcaster: false
@@ -617,11 +623,11 @@ impl_writeable!(ChannelTransactionParameters, 0, {
 	funding_outpoint
 });
 
-/// Static channel fields used to build transactions given per-commitment fields, independent of
-/// transaction ownership.
+/// Static channel fields used to build transactions given per-commitment fields, organized by
+/// broadcaster/countersignatory.
 ///
 /// This is derived from the owner-specific ChannelTransactionParameters via the
-/// as_holder_directed and as_counterparty_directed functions.
+/// as_holder_broadcastable and as_counterparty_broadcastable functions.
 pub struct DirectedChannelTransactionParameters<'a> {
 	/// The holder's channel static parameters
 	inner: &'a ChannelTransactionParameters,
@@ -725,7 +731,7 @@ impl HolderCommitmentTransaction {
 			funding_outpoint: Some(chain::transaction::OutPoint { txid: Default::default(), index: 0 })
 		};
 		let aux: Vec<()> = Vec::new();
-		let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, 0, keys, 0, Vec::new(), aux, &channel_parameters.as_counterparty_directed()).0;
+		let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, 0, keys, 0, Vec::new(), aux, &channel_parameters.as_counterparty_broadcastable()).0;
 		HolderCommitmentTransaction {
 			inner,
 			counterparty_sig: dummy_sig,
@@ -734,9 +740,9 @@ impl HolderCommitmentTransaction {
 		}
 	}
 
-	// Trust the pre-built commitment transaction and return its ID.
-	pub(crate) fn trust_txid(&self) -> Txid {
-		self.inner.trust_txid()
+	// Get pre-built commitment transaction and return its ID.
+	pub(crate) fn untrusted_txid(&self) -> Txid {
+		self.inner.untrusted_txid()
 	}
 
 	/// Gets a signed HTLC transaction given a preimage (for !htlc.offered) and the holder HTLC transaction signature.
@@ -757,8 +763,8 @@ impl HolderCommitmentTransaction {
 
 	/// The pre-calculated transaction creation public keys.
 	/// An external validating signer should not trust these keys.
-	pub fn trust_key_derivation(&self) -> &TxCreationKeys {
-		self.inner.trust_key_derivation()
+	pub fn untrusted_key_derivation(&self) -> &TxCreationKeys {
+		self.inner.untrusted_key_derivation()
 	}
 }
 
@@ -798,7 +804,7 @@ pub struct BuiltCommitmentTransaction {
 impl_writeable!(BuiltCommitmentTransaction, 0, { transaction, txid });
 
 impl BuiltCommitmentTransaction {
-	/// Get the SIGHASH_ALL sighash value and the transaction.
+	/// Get the SIGHASH_ALL sighash value of the transaction.
 	///
 	/// This can be used to verify a signature.
 	pub fn get_sighash_all(&self, funding_redeemscript: &Script, channel_value_satoshis: u64) -> Message {
@@ -808,7 +814,7 @@ impl BuiltCommitmentTransaction {
 
 	/// Sign a transaction, either because we are counter-signing the counterparty's transaction or
 	/// because we are about to broadcast a holder transaction.
-	pub fn sign<T: secp256k1::Signing + secp256k1::Verification>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
+	pub fn sign<T: secp256k1::Signing>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
 		let sighash = self.get_sighash_all(funding_redeemscript, channel_value_satoshis);
 		secp_ctx.sign(&sighash, funding_key)
 	}
@@ -829,7 +835,7 @@ pub struct CommitmentTransaction {
 	htlcs: Vec<HTLCOutputInCommitment>,
 	// A cache of the parties' pubkeys required to construct the transaction
 	keys: TxCreationKeys,
-	// For access to the built transaction, see doc for trust_built_transaction
+	// For access to the pre-built transaction, see doc for untrusted_built_transaction
 	built: BuiltCommitmentTransaction,
 }
 
@@ -888,6 +894,10 @@ impl CommitmentTransaction {
 	/// Also keeps track of auxiliary HTLC data and returns it along with the mutated and sorted HTLCs.
 	/// This allows the caller to match the HTLC output index with the auxiliary data.
 	/// This auxiliary data is not stored in this object.
+	///
+	/// Only include HTLCs that are above the dust limit for the channel.
+	///
+	/// Panics if the length of htlcs and aux are different.
 	pub fn new_with_auxiliary_htlc_data<T>(commitment_number: u64, to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, keys: TxCreationKeys, feerate_per_kw: u32, htlcs: Vec<HTLCOutputInCommitment>, aux: Vec<T>, channel_parameters: &DirectedChannelTransactionParameters) -> (CommitmentTransaction, Vec<(HTLCOutputInCommitment, T)>) {
 		// Sort outputs and populate output indices while keeping track of the auxiliary data
 		let mut txouts = Self::internal_build_outputs(&keys, to_broadcaster_value_sat, to_countersignatory_value_sat, &htlcs, aux, channel_parameters).unwrap();
@@ -921,16 +931,16 @@ impl CommitmentTransaction {
 		(info, result_htlcs_with_aux)
 	}
 
-	// Trust the pre-built commitment transaction and return its ID.
-	pub(crate) fn trust_txid(&self) -> Txid {
+	// Get the pre-built commitment transaction's ID.
+	pub(crate) fn untrusted_txid(&self) -> Txid {
 		self.built.txid
 	}
 
-	/// Trust the pre-built commitment transaction.
+	/// Get the pre-built commitment transaction.
 	///
 	/// This should only be used if you fully trust the builder of this object.  It should not
 	/// be used by an external signer - instead use the build function.
-	pub fn trust_built_transaction(&self) -> &BuiltCommitmentTransaction {
+	pub fn untrusted_built_transaction(&self) -> &BuiltCommitmentTransaction {
 		&self.built
 	}
 
@@ -1068,8 +1078,8 @@ impl CommitmentTransaction {
 	/// The returned Vec has one entry for each HTLC, and in the same order. For HTLCs which were
 	/// considered dust and not included, a None entry exists, for all others a signature is
 	/// included.
-	pub fn get_htlc_sigs<T: secp256k1::Signing + secp256k1::Verification>(&self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
-		let txid = self.trust_txid();
+	pub fn get_htlc_sigs<T: secp256k1::Signing>(&self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
+		let txid = self.untrusted_txid();
 		let mut ret = Vec::with_capacity(self.htlcs.len());
 		let holder_htlc_key = derive_private_key(secp_ctx, &self.keys.per_commitment_point, htlc_base_key).map_err(|_| ())?;
 
@@ -1090,7 +1100,7 @@ impl CommitmentTransaction {
 
 	/// Gets a signed HTLC transaction given a preimage (for !htlc.offered) and the holder HTLC transaction signature.
 	pub(crate) fn get_signed_htlc_tx(&self, channel_parameters: &DirectedChannelTransactionParameters, htlc_index: usize, counterparty_signature: &Signature, signature: &Signature, preimage: &Option<PaymentPreimage>) -> Transaction {
-		let txid = self.trust_txid();
+		let txid = self.untrusted_txid();
 		let this_htlc = &self.htlcs[htlc_index];
 		assert!(this_htlc.transaction_output_index.is_some());
 		// if we don't have preimage for an HTLC-Success, we can't generate an HTLC transaction.
@@ -1123,7 +1133,7 @@ impl CommitmentTransaction {
 
 	/// The pre-calculated transaction creation public keys.
 	/// An external validating signer should not trust these keys.
-	pub fn trust_key_derivation(&self) -> &TxCreationKeys {
+	pub fn untrusted_key_derivation(&self) -> &TxCreationKeys {
 		&self.keys
 	}