Make ChannelKeys an API and template Channel with it.

Instead of having in-memory access to the list of private keys
associated with a channel, we should have a generic API which
allows us to request signing, allowing the user to store private
keys any way they like.

The first step is the (rather mechanical) process of templating
the entire tree of ChannelManager -> Channel impls by the
key-providing type. In a later commit we should expose only public
keys where possible.

Author: TheBlueMatt

fuzz/fuzz_targets/chanmon_fail_consistency.rs

@@ -32,7 +32,7 @@ use bitcoin_hashes::sha256d::Hash as Sha256d;
 use lightning::chain::chaininterface;
 use lightning::chain::transaction::OutPoint;
 use lightning::chain::chaininterface::{BroadcasterInterface,ConfirmationTarget,ChainListener,FeeEstimator,ChainWatchInterfaceUtil};
-use lightning::chain::keysinterface::{ChannelKeys, KeysInterface};
+use lightning::chain::keysinterface::{InMemoryChannelKeys, KeysInterface};
 use lightning::ln::channelmonitor;
 use lightning::ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, HTLCUpdate};
 use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage, ChannelManagerReadArgs};
@@ -139,6 +139,8 @@ struct KeyProvider {
 	channel_id: atomic::AtomicU8,
 }
 impl KeysInterface for KeyProvider {
+	type ChanKeySigner = InMemoryChannelKeys;
+
 	fn get_node_secret(&self) -> SecretKey {
 		SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, self.node_id]).unwrap()
 	}
@@ -155,8 +157,8 @@ impl KeysInterface for KeyProvider {
 		PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, self.node_id]).unwrap())
 	}
 
-	fn get_channel_keys(&self, _inbound: bool) -> ChannelKeys {
-		ChannelKeys {
+	fn get_channel_keys(&self, _inbound: bool) -> InMemoryChannelKeys {
+		InMemoryChannelKeys {
 			funding_key:               SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, self.node_id]).unwrap(),
 			revocation_base_key:       SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, self.node_id]).unwrap(),
 			payment_base_key:          SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, self.node_id]).unwrap(),
@@ -232,7 +234,7 @@ pub fn do_test(data: &[u8]) {
 				channel_monitors: &monitor_refs,
 			};
 
-			let res = (<(Sha256d, ChannelManager)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, monitor);
+			let res = (<(Sha256d, ChannelManager<InMemoryChannelKeys>)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, monitor);
 			for (_, was_good) in $old_monitors.latest_updates_good_at_last_ser.lock().unwrap().iter() {
 				if !was_good {
 					// If the last time we updated a monitor we didn't successfully update (and we

fuzz/fuzz_targets/full_stack_target.rs

@@ -28,7 +28,7 @@ use bitcoin_hashes::sha256d::Hash as Sha256dHash;
 
 use lightning::chain::chaininterface::{BroadcasterInterface,ConfirmationTarget,ChainListener,FeeEstimator,ChainWatchInterfaceUtil};
 use lightning::chain::transaction::OutPoint;
-use lightning::chain::keysinterface::{ChannelKeys, KeysInterface};
+use lightning::chain::keysinterface::{InMemoryChannelKeys, KeysInterface};
 use lightning::ln::channelmonitor;
 use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage};
 use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor};
@@ -145,7 +145,7 @@ impl<'a> Hash for Peer<'a> {
 }
 
 struct MoneyLossDetector<'a, 'b> {
-	manager: Arc<ChannelManager<'b>>,
+	manager: Arc<ChannelManager<'b, InMemoryChannelKeys>>,
 	monitor: Arc<channelmonitor::SimpleManyChannelMonitor<OutPoint>>,
 	handler: PeerManager<Peer<'a>>,
 
@@ -158,7 +158,7 @@ struct MoneyLossDetector<'a, 'b> {
 	blocks_connected: u32,
 }
 impl<'a, 'b> MoneyLossDetector<'a, 'b> {
-	pub fn new(peers: &'a RefCell<[bool; 256]>, manager: Arc<ChannelManager<'b>>, monitor: Arc<channelmonitor::SimpleManyChannelMonitor<OutPoint>>, handler: PeerManager<Peer<'a>>) -> Self {
+	pub fn new(peers: &'a RefCell<[bool; 256]>, manager: Arc<ChannelManager<'b, InMemoryChannelKeys>>, monitor: Arc<channelmonitor::SimpleManyChannelMonitor<OutPoint>>, handler: PeerManager<Peer<'a>>) -> Self {
 		MoneyLossDetector {
 			manager,
 			monitor,
@@ -238,6 +238,8 @@ struct KeyProvider {
 	counter: AtomicU64,
 }
 impl KeysInterface for KeyProvider {
+	type ChanKeySigner = InMemoryChannelKeys;
+
 	fn get_node_secret(&self) -> SecretKey {
 		self.node_secret.clone()
 	}
@@ -254,10 +256,10 @@ impl KeysInterface for KeyProvider {
 		PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]).unwrap())
 	}
 
-	fn get_channel_keys(&self, inbound: bool) -> ChannelKeys {
+	fn get_channel_keys(&self, inbound: bool) -> InMemoryChannelKeys {
 		let ctr = self.counter.fetch_add(1, Ordering::Relaxed) as u8;
 		if inbound {
-			ChannelKeys {
+			InMemoryChannelKeys {
 				funding_key:               SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, ctr]).unwrap(),
 				revocation_base_key:       SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, ctr]).unwrap(),
 				payment_base_key:          SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, ctr]).unwrap(),
@@ -266,7 +268,7 @@ impl KeysInterface for KeyProvider {
 				commitment_seed: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, ctr],
 			}
 		} else {
-			ChannelKeys {
+			InMemoryChannelKeys {
 				funding_key:               SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, ctr]).unwrap(),
 				revocation_base_key:       SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, ctr]).unwrap(),
 				payment_base_key:          SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, ctr]).unwrap(),

lightning/src/chain/keysinterface.rs

@@ -68,6 +68,9 @@ pub enum SpendableOutputDescriptor {
 
 /// A trait to describe an object which can get user secrets and key material.
 pub trait KeysInterface: Send + Sync {
+	/// A type which implements ChannelKeys which will be returned by get_channel_keys.
+	type ChanKeySigner : ChannelKeys;
+
 	/// Get node secret key (aka node_id or network_key)
 	fn get_node_secret(&self) -> SecretKey;
 	/// Get destination redeemScript to encumber static protocol exit points.
@@ -76,7 +79,7 @@ pub trait KeysInterface: Send + Sync {
 	fn get_shutdown_pubkey(&self) -> PublicKey;
 	/// Get a new set of ChannelKeys for per-channel secrets. These MUST be unique even if you
 	/// restarted with some stale data!
-	fn get_channel_keys(&self, inbound: bool) -> ChannelKeys;
+	fn get_channel_keys(&self, inbound: bool) -> Self::ChanKeySigner;
 	/// Get a secret and PRNG seed for construting an onion packet
 	fn get_onion_rand(&self) -> (SecretKey, [u8; 32]);
 	/// Get a unique temporary channel id. Channels will be referred to by this until the funding
@@ -85,9 +88,33 @@ pub trait KeysInterface: Send + Sync {
 	fn get_channel_id(&self) -> [u8; 32];
 }
 
-/// Set of lightning keys needed to operate a channel as described in BOLT 3
+/// Set of lightning keys needed to operate a channel as described in BOLT 3.
+///
+/// If you're implementing a custom signer, you almost certainly want to implement
+/// Readable/Writable to serialize out a unique reference to this set of keys so
+/// that you can serialize the full ChannelManager object.
+///
+/// (TODO: We shouldn't require that, and should have an API to get them at deser time, due mostly
+/// to the possibility of reentrancy issues by calling the user's code during our deserialization
+/// routine).
+pub trait ChannelKeys : Clone + Send {
+	/// Gets the private key for the anchor tx
+	fn funding_key<'a>(&'a self) -> &'a SecretKey;
+	/// Gets the local secret key for blinded revocation pubkey
+	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
+	/// Gets the local secret key used in commitment tx htlc outputs
+	fn payment_base_key<'a>(&'a self) -> &'a SecretKey;
+	/// Gets the local secret key used in HTLC tx
+	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
+	/// Gets the local htlc secret key used in commitment tx htlc outputs
+	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
+	/// Gets the commitment seed
+	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
+}
+
 #[derive(Clone)]
-pub struct ChannelKeys {
+/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
+pub struct InMemoryChannelKeys {
 	/// Private key of anchor tx
 	pub funding_key: SecretKey,
 	/// Local secret key for blinded revocation pubkey
@@ -102,7 +129,16 @@ pub struct ChannelKeys {
 	pub commitment_seed: [u8; 32],
 }
 
-impl_writeable!(ChannelKeys, 0, {
+impl ChannelKeys for InMemoryChannelKeys {
+	fn funding_key(&self) -> &SecretKey { &self.funding_key }
+	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
+	fn payment_base_key(&self) -> &SecretKey { &self.payment_base_key }
+	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
+	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
+	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
+}
+
+impl_writeable!(InMemoryChannelKeys, 0, {
 	funding_key,
 	revocation_base_key,
 	payment_base_key,
@@ -203,6 +239,8 @@ impl KeysManager {
 }
 
 impl KeysInterface for KeysManager {
+	type ChanKeySigner = InMemoryChannelKeys;
+
 	fn get_node_secret(&self) -> SecretKey {
 		self.node_secret.clone()
 	}
@@ -215,7 +253,7 @@ impl KeysInterface for KeysManager {
 		self.shutdown_pubkey.clone()
 	}
 
-	fn get_channel_keys(&self, _inbound: bool) -> ChannelKeys {
+	fn get_channel_keys(&self, _inbound: bool) -> InMemoryChannelKeys {
 		// We only seriously intend to rely on the channel_master_key for true secure
 		// entropy, everything else just ensures uniqueness. We rely on the unique_start (ie
 		// starting_time provided in the constructor) to be unique.
@@ -248,7 +286,7 @@ impl KeysInterface for KeysManager {
 		let delayed_payment_base_key = key_step!(b"delayed payment base key", payment_base_key);
 		let htlc_base_key = key_step!(b"HTLC base key", delayed_payment_base_key);
 
-		ChannelKeys {
+		InMemoryChannelKeys {
 			funding_key,
 			revocation_base_key,
 			payment_base_key,

lightning/src/ln/chan_utils.rs

@@ -20,8 +20,8 @@ pub const HTLC_TIMEOUT_TX_WEIGHT: u64 = 663;
 // Various functions for key derivation and transaction creation for use within channels. Primarily
 // used in Channel and ChannelMonitor.
 
-pub fn build_commitment_secret(commitment_seed: [u8; 32], idx: u64) -> [u8; 32] {
-	let mut res: [u8; 32] = commitment_seed;
+pub fn build_commitment_secret(commitment_seed: &[u8; 32], idx: u64) -> [u8; 32] {
+	let mut res: [u8; 32] = commitment_seed.clone();
 	for i in 0..48 {
 		let bitpos = 47 - i;
 		if idx & (1 << bitpos) == (1 << bitpos) {