keep track of revocations in KeysInterface

Author: devrandom

lightning/src/ln/onchaintx.rs

@@ -948,7 +948,7 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 	pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
 		if let Some(ref mut holder_commitment) = self.holder_commitment {
 			let holder_commitment = holder_commitment.clone();
-			match self.key_storage.sign_holder_commitment(&holder_commitment, &self.secp_ctx) {
+			match self.key_storage.unsafe_sign_holder_commitment(&holder_commitment, &self.secp_ctx) {
 				Ok(sig) => Some(holder_commitment.add_holder_sig(funding_redeemscript, sig)),
 				Err(_) => return None,
 			}

lightning/src/util/enforcing_trait_impls.rs

@@ -24,7 +24,8 @@ use util::ser::{Writeable, Writer, Readable};
 use std::io::Error;
 use ln::msgs::DecodeError;
 
-const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+/// Initial value for revoked commitment downward counter
+pub const INITIAL_REVOKED_COMMITMENT_NUMBER: u64 = 1 << 48;
 
 /// Enforces some rules on ChannelKeys calls. Eventually we will probably want to expose a variant
 /// of this which would essentially be what you'd want to run on a hardware wallet.
@@ -40,7 +41,15 @@ impl EnforcingChannelKeys {
 		Self {
 			inner,
 			commitment_number_obscure_and_last: Arc::new(Mutex::new((None, 0))),
-			revoked_commitment: Arc::new(Mutex::new(INITIAL_COMMITMENT_NUMBER + 1)),
+			revoked_commitment: Arc::new(Mutex::new(INITIAL_REVOKED_COMMITMENT_NUMBER))
+		}
+	}
+
+	pub fn new_with_revoked(inner: InMemoryChannelKeys, revoked_commitment: Arc<Mutex<u64>>) -> Self {
+		Self {
+			inner,
+			commitment_number_obscure_and_last: Arc::new(Mutex::new((None, 0))),
+			revoked_commitment
 		}
 	}
 }
@@ -69,20 +78,14 @@ impl ChannelKeys for EnforcingChannelKeys {
 
 	fn release_commitment_secret(&self, idx: u64) -> [u8; 32] {
 		println!("XXX revoke {} {}", self.inner.commitment_seed[0], idx);
-		let res = {
-			let mut revoked = self.revoked_commitment.lock().unwrap();
-			if idx != *revoked && idx != *revoked - 1 {
-				panic!("can only revoke the current or next unrevoked commitment - trying {}, revoked {}", idx, *revoked)
-			}
-			*revoked = idx;
-			self.inner.release_commitment_secret(idx)
-		};
-		{
-			let revoked = self.revoked_commitment.lock().unwrap();
-			assert_eq!(idx, *revoked);
+		let mut revoked = self.revoked_commitment.lock().unwrap();
+		if idx != *revoked && idx != *revoked - 1 {
+			panic!("can only revoke the current or next unrevoked commitment - trying {}, revoked {}", idx, *revoked)
 		}
-		res
+		*revoked = idx;
+		self.inner.release_commitment_secret(idx)
 	}
+
 	fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
 	fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() }
 
@@ -112,10 +115,12 @@ impl ChannelKeys for EnforcingChannelKeys {
 		if keys.per_commitment_point != self.inner.get_per_commitment_point(*revoked - 1, secp_ctx) {
 			if keys.per_commitment_point != self.inner.get_per_commitment_point(*revoked - 2, secp_ctx) {
 				if keys.per_commitment_point == self.inner.get_per_commitment_point(*revoked, secp_ctx) {
-					panic!("attempted to sign the latest revoked local commitment {}", self.inner.commitment_seed[0]);
+					println!("attempted to sign the latest revoked local commitment {}", self.inner.commitment_seed[0]);
+					return Err(())
 				} else {
-					panic!("can only sign the next two unrevoked commitment numbers, {} revoked={} point={}",
+					println!("can only sign the next two unrevoked commitment numbers, {} revoked={} point={}",
 					       self.inner.commitment_seed[0], *revoked, keys.per_commitment_point);
+					return Err(())
 				}
 			}
 		}
@@ -166,12 +171,11 @@ impl ChannelKeys for EnforcingChannelKeys {
 	}
 }
 
+
 impl Writeable for EnforcingChannelKeys {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
 		self.inner.write(writer)?;
-		let revoked = *self.revoked_commitment.lock().unwrap();
-		revoked.write(writer)?;
-		println!("XXX write {} {}", self.inner.commitment_seed[0], revoked);
+		println!("XXX write {}", self.inner.commitment_seed[0]);
 
 		let (obscure, last) = *self.commitment_number_obscure_and_last.lock().unwrap();
 		obscure.write(writer)?;
@@ -183,13 +187,12 @@ impl Writeable for EnforcingChannelKeys {
 impl Readable for EnforcingChannelKeys {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
 		let inner: InMemoryChannelKeys = Readable::read(reader)?;
-		let revoked = Readable::read(reader)?;
 		let obscure_and_last = Readable::read(reader)?;
-		println!("XXX read {} {}", inner.commitment_seed[0], revoked);
+		println!("XXX read {}", inner.commitment_seed[0]);
 		Ok(EnforcingChannelKeys {
-			inner: inner,
-			revoked_commitment: Arc::new(Mutex::new(revoked)),
-			commitment_number_obscure_and_last: Arc::new(Mutex::new(obscure_and_last))
+			inner,
+			commitment_number_obscure_and_last: Arc::new(Mutex::new(obscure_and_last)),
+			revoked_commitment: Arc::new(Mutex::new(INITIAL_REVOKED_COMMITMENT_NUMBER)),
 		})
 	}
 }

lightning/src/util/test_utils.rs

@@ -18,7 +18,7 @@ use chain::keysinterface;
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::OptionalField;
-use util::enforcing_trait_impls::EnforcingChannelKeys;
+use util::enforcing_trait_impls::{EnforcingChannelKeys, INITIAL_REVOKED_COMMITMENT_NUMBER};
 use util::events;
 use util::logger::{Logger, Level, Record};
 use util::ser::{Readable, Writer, Writeable};
@@ -35,7 +35,7 @@ use bitcoin::secp256k1::{SecretKey, PublicKey, Secp256k1, Signature};
 use regex;
 
 use std::time::Duration;
-use std::sync::Mutex;
+use std::sync::{Mutex, Arc};
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::{cmp, mem};
 use std::collections::{HashMap, HashSet};
@@ -384,6 +384,7 @@ pub struct TestKeysInterface {
 	backing: keysinterface::KeysManager,
 	pub override_session_priv: Mutex<Option<[u8; 32]>>,
 	pub override_channel_id_priv: Mutex<Option<[u8; 32]>>,
+	revoked_commitments: Mutex<HashMap<[u8;32], Arc<Mutex<u64>>>>,
 }
 
 impl keysinterface::KeysInterface for TestKeysInterface {
@@ -393,7 +394,9 @@ impl keysinterface::KeysInterface for TestKeysInterface {
 	fn get_destination_script(&self) -> Script { self.backing.get_destination_script() }
 	fn get_shutdown_pubkey(&self) -> PublicKey { self.backing.get_shutdown_pubkey() }
 	fn get_channel_keys(&self, inbound: bool, channel_value_satoshis: u64) -> EnforcingChannelKeys {
-		EnforcingChannelKeys::new(self.backing.get_channel_keys(inbound, channel_value_satoshis))
+		let keys = self.backing.get_channel_keys(inbound, channel_value_satoshis);
+		let revoked_commitment = self.make_revoked_commitment_cell(keys.commitment_seed);
+		EnforcingChannelKeys::new_with_revoked(keys, revoked_commitment)
 	}
 
 	fn get_secure_random_bytes(&self) -> [u8; 32] {
@@ -412,17 +415,21 @@ impl keysinterface::KeysInterface for TestKeysInterface {
 	}
 }
 
+
 impl TestKeysInterface {
 	pub fn new(seed: &[u8; 32], network: Network) -> Self {
 		let now = Duration::from_secs(genesis_block(network).header.time as u64);
 		Self {
 			backing: keysinterface::KeysManager::new(seed, network, now.as_secs(), now.subsec_nanos()),
 			override_session_priv: Mutex::new(None),
 			override_channel_id_priv: Mutex::new(None),
+			revoked_commitments: Mutex::new(HashMap::new()),
 		}
 	}
 	pub fn derive_channel_keys(&self, channel_value_satoshis: u64, user_id_1: u64, user_id_2: u64) -> EnforcingChannelKeys {
-		EnforcingChannelKeys::new(self.backing.derive_channel_keys(channel_value_satoshis, user_id_1, user_id_2))
+		let keys = self.backing.derive_channel_keys(channel_value_satoshis, user_id_1, user_id_2);
+		let revoked_commitment = self.make_revoked_commitment_cell(keys.commitment_seed);
+		EnforcingChannelKeys::new_with_revoked(keys, revoked_commitment)
 	}
 }
 
@@ -464,3 +471,13 @@ impl chain::Filter for TestChainSource {
 		self.watched_outputs.lock().unwrap().insert((*outpoint, script_pubkey.clone()));
 	}
 }
+
+impl TestKeysInterface {
+	fn make_revoked_commitment_cell(&self, commitment_seed: [u8; 32]) -> Arc<Mutex<u64>> {
+		let mut revoked_commitments = self.revoked_commitments.lock().unwrap();
+		if !revoked_commitments.contains_key(&commitment_seed) {
+			revoked_commitments.insert(commitment_seed, Arc::new(Mutex::new(INITIAL_REVOKED_COMMITMENT_NUMBER)));
+		}
+		Arc::clone(revoked_commitments.get(&commitment_seed).unwrap())
+	}
+}