WIP: revocation enforcement in signer

Author: devrandom

lightning/src/chain/keysinterface.rs

@@ -202,7 +202,8 @@ pub trait ChannelKeys : Send+Clone {
 	/// Gets the commitment seed for a specific commitment number, thereby revoking the commitment
 	///
 	/// After this is called, it is an error to try to sign the relevant local commitment transaction.
-	/// May be called more than once (e.g. if re-establishing the channel).
+	/// Must be called in monotonic order, without gaps. May be called more than once (e.g. if
+	/// re-establishing the channel).
 	/// Note that the commitment number starts at (1 << 48) - 1 and counts backwards.
 	fn get_revoke_commitment_secret(&self, idx: u64) -> [u8; 32];
 	/// Gets the local channel public keys and basepoints

lightning/src/ln/functional_tests.rs

@@ -1608,14 +1608,20 @@ fn test_fee_spike_violation_fails_htlc() {
 		let local_chan = chan_lock.by_id.get(&chan.2).unwrap();
 		let chan_keys = local_chan.get_local_keys();
 		let pubkeys = chan_keys.pubkeys();
+		let local_secret = chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER);
+		// Must revoke without gaps
+		chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1);
+		let local_secret2 = chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2);
 		(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
-		 chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
+		 local_secret, local_secret2)
 	};
 	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_secret1) = {
 		let chan_lock = nodes[1].node.channel_state.lock().unwrap();
 		let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
 		let chan_keys = remote_chan.get_local_keys();
 		let pubkeys = chan_keys.pubkeys();
+		// Must revoke without gaps
+		chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER);
 		(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
 		 chan_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
 	};
@@ -8128,9 +8134,11 @@ fn test_counterparty_raa_skip_no_crash() {
 	let mut guard = nodes[0].node.channel_state.lock().unwrap();
 	let local_keys = &guard.by_id.get_mut(&channel_id).unwrap().local_keys;
 	const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+	let per_commitment_secret = local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER);
+	// Must revoke without gaps
+	local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1);
 	let next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
 		&SecretKey::from_slice(&local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
-	let per_commitment_secret = local_keys.get_revoke_commitment_secret(INITIAL_COMMITMENT_NUMBER);
 
 	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
 		&msgs::RevokeAndACK { channel_id, per_commitment_secret, next_per_commitment_point });

lightning/src/util/enforcing_trait_impls.rs

@@ -15,11 +15,14 @@ use util::ser::{Writeable, Writer, Readable};
 use std::io::Error;
 use ln::msgs::DecodeError;
 
+const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+
 /// 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)>>,
 }
 
@@ -28,6 +31,7 @@ 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)),
 		}
 	}
 }
@@ -52,7 +56,14 @@ impl ChannelKeys for EnforcingChannelKeys {
 		self.inner.get_per_commitment_point(idx, secp_ctx)
 	}
 
-	fn get_revoke_commitment_secret(&self, idx: u64) -> [u8; 32] { self.inner.get_revoke_commitment_secret(idx) }
+	fn get_revoke_commitment_secret(&self, idx: u64) -> [u8; 32] {
+		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.get_revoke_commitment_secret(idx)
+	}
 	fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
 	fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() }
 
@@ -124,6 +135,8 @@ 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)?;
 		let (obscure, last) = *self.commitment_number_obscure_and_last.lock().unwrap();
 		obscure.write(writer)?;
 		last.write(writer)?;
@@ -134,9 +147,11 @@ impl Writeable for EnforcingChannelKeys {
 impl Readable for EnforcingChannelKeys {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
 		let inner = Readable::read(reader)?;
+		let revoked = Readable::read(reader)?;
 		let obscure_and_last = Readable::read(reader)?;
 		Ok(EnforcingChannelKeys {
 			inner: inner,
+			revoked_commitment: Arc::new(Mutex::new(revoked)),
 			commitment_number_obscure_and_last: Arc::new(Mutex::new(obscure_and_last))
 		})
 	}