keep track of revocations in KeysInterface

Author: devrandom

lightning/src/ln/onchaintx.rs

@@ -962,7 +962,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,23 +24,32 @@ 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.
 #[derive(Clone)]
 pub struct EnforcingChannelKeys {
 	pub inner: InMemoryChannelKeys,
-	revoked_commitment: Arc<Mutex<u64>>,
-	commitment_number_obscure_and_last: Arc<Mutex<(Option<u64>, u64)>>,
+	pub(crate) revoked_commitment: Arc<Mutex<u64>>,
+	pub(crate) commitment_number_obscure_and_last: Arc<Mutex<(Option<u64>, u64)>>,
 }
 
 impl EnforcingChannelKeys {
 	pub fn new(inner: InMemoryChannelKeys) -> Self {
 		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,17 @@ 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);
 		}
-		res
+		println!("XXX revoke after {} {}", self.inner.commitment_seed[0], *self.revoked_commitment.lock().unwrap());
+		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 +118,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 +174,14 @@ 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);
+		{
+			let result = self.revoked_commitment.lock().unwrap();
+			println!("XXX write {} {}", self.inner.commitment_seed[0], result);
+		}
 
 		let (obscure, last) = *self.commitment_number_obscure_and_last.lock().unwrap();
 		obscure.write(writer)?;
@@ -183,13 +193,11 @@ 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);
 		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, ReadableArgs, 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};
@@ -402,6 +402,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 {
@@ -411,7 +412,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] {
@@ -429,22 +432,41 @@ impl keysinterface::KeysInterface for TestKeysInterface {
 		self.backing.get_secure_random_bytes()
 	}
 
-	fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::ChanKeySigner, msgs::DecodeError> {
-		EnforcingChannelKeys::read(&mut std::io::Cursor::new(reader))
+	fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::ChanKeySigner, msgs::DecodeError> {
+		let mut reader = std::io::Cursor::new(buffer);
+
+		let inner: InMemoryChannelKeys = Readable::read(&mut reader)?;
+		let revoked_commitment = self.make_revoked_commitment_cell(inner.commitment_seed);
+
+		{
+			let result = revoked_commitment.lock().unwrap();
+			println!("XXX read {} {}", inner.commitment_seed[0], result);
+		}
+		let obscure_and_last = Readable::read(&mut reader)?;
+
+		Ok(EnforcingChannelKeys {
+			inner,
+			commitment_number_obscure_and_last: Arc::new(Mutex::new(obscure_and_last)),
+			revoked_commitment,
+		})
 	}
 }
 
+
 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)
 	}
 }
 
@@ -486,3 +508,29 @@ 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)));
+		}
+		println!("XXX create {}", commitment_seed[0]);
+		let cell = revoked_commitments.get(&commitment_seed).unwrap();
+		Arc::clone(cell)
+	}
+}
+
+#[test]
+fn test_mutex() {
+	let x = Arc::new(Mutex::new(123));
+	let y = Arc::clone(&x);
+	{
+		let mut rx = x.lock().unwrap();
+		*rx = 124;
+	}
+	{
+		let ry = y.lock().unwrap();
+		println!("{}", *ry);
+	}
+}