Merge pull request #1867 from wpaulino/remove-signer-persistence

Re-derive signers instead of persisting them

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -193,9 +193,14 @@ impl KeysInterface for KeyProvider {
 		ShutdownScript::new_p2wpkh(&pubkey_hash)
 	}
 
-	fn get_channel_signer(&self, _inbound: bool, channel_value_satoshis: u64) -> EnforcingSigner {
+	fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] {
+		let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed) as u8;
+		[id; 32]
+	}
+
+	fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer {
 		let secp_ctx = Secp256k1::signing_only();
-		let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed);
+		let id = channel_keys_id[0];
 		let keys = InMemorySigner::new(
 			&secp_ctx,
 			self.get_node_secret(Recipient::Node).unwrap(),
@@ -204,9 +209,9 @@ impl KeysInterface for KeyProvider {
 			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(),
 			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, self.node_id]).unwrap(),
 			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, self.node_id]).unwrap(),
-			[id as u8, 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, self.node_id],
+			[id, 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, self.node_id],
 			channel_value_satoshis,
-			[0; 32],
+			channel_keys_id,
 		);
 		let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
 		EnforcingSigner::new_with_revoked(keys, revoked_commitment, false)

fuzz/src/full_stack.rs

@@ -263,6 +263,7 @@ struct KeyProvider {
 	node_secret: SecretKey,
 	inbound_payment_key: KeyMaterial,
 	counter: AtomicU64,
+	signer_state: RefCell<HashMap<u8, (bool, Arc<Mutex<EnforcementState>>)>>
 }
 impl KeysInterface for KeyProvider {
 	type Signer = EnforcingSigner;
@@ -297,10 +298,17 @@ impl KeysInterface for KeyProvider {
 		ShutdownScript::new_p2wpkh(&pubkey_hash)
 	}
 
-	fn get_channel_signer(&self, inbound: bool, channel_value_satoshis: u64) -> EnforcingSigner {
+	fn generate_channel_keys_id(&self, inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] {
 		let ctr = self.counter.fetch_add(1, Ordering::Relaxed) as u8;
+		self.signer_state.borrow_mut().insert(ctr, (inbound, Arc::new(Mutex::new(EnforcementState::new()))));
+		[ctr; 32]
+	}
+
+	fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer {
 		let secp_ctx = Secp256k1::signing_only();
-		EnforcingSigner::new(if inbound {
+		let ctr = channel_keys_id[0];
+		let (inbound, state) = self.signer_state.borrow().get(&ctr).unwrap().clone();
+		EnforcingSigner::new_with_revoked(if inbound {
 			InMemorySigner::new(
 				&secp_ctx,
 				self.node_secret.clone(),
@@ -311,7 +319,7 @@ impl KeysInterface for KeyProvider {
 				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, ctr]).unwrap(),
 				[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],
 				channel_value_satoshis,
-				[0; 32],
+				channel_keys_id,
 			)
 		} else {
 			InMemorySigner::new(
@@ -324,9 +332,9 @@ impl KeysInterface for KeyProvider {
 				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, 11, ctr]).unwrap(),
 				[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, 12, ctr],
 				channel_value_satoshis,
-				[0; 32],
+				channel_keys_id,
 			)
-		})
+		}, state, false)
 	}
 
 	fn get_secure_random_bytes(&self) -> [u8; 32] {
@@ -390,7 +398,12 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 	let monitor = Arc::new(chainmonitor::ChainMonitor::new(None, broadcast.clone(), Arc::clone(&logger), fee_est.clone(),
 		Arc::new(TestPersister { update_ret: Mutex::new(ChannelMonitorUpdateStatus::Completed) })));
 
-	let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()), counter: AtomicU64::new(0) });
+	let keys_manager = Arc::new(KeyProvider {
+		node_secret: our_network_key.clone(),
+		inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()),
+		counter: AtomicU64::new(0),
+		signer_state: RefCell::new(HashMap::new())
+	});
 	let mut config = UserConfig::default();
 	config.channel_config.forwarding_fee_proportional_millionths =  slice_to_be32(get_slice!(4));
 	config.channel_handshake_config.announced_channel = get_slice!(1)[0] != 0;

fuzz/src/onion_message.rs

@@ -111,7 +111,9 @@ impl KeysInterface for KeyProvider {
 
 	fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { unreachable!() }
 
-	fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> EnforcingSigner {
+	fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128) -> [u8; 32] { unreachable!() }
+
+	fn derive_channel_signer(&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32]) -> Self::Signer {
 		unreachable!()
 	}
 

lightning/src/chain/channelmonitor.rs

@@ -291,7 +291,7 @@ struct CounterpartyCommitmentParameters {
 
 impl Writeable for CounterpartyCommitmentParameters {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-		w.write_all(&byte_utils::be64_to_array(0))?;
+		w.write_all(&(0 as u64).to_be_bytes())?;
 		write_tlv_fields!(w, {
 			(0, self.counterparty_delayed_payment_base_key, required),
 			(2, self.counterparty_htlc_base_key, required),
@@ -945,7 +945,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 		self.channel_keys_id.write(writer)?;
 		self.holder_revocation_basepoint.write(writer)?;
 		writer.write_all(&self.funding_info.0.txid[..])?;
-		writer.write_all(&byte_utils::be16_to_array(self.funding_info.0.index))?;
+		writer.write_all(&self.funding_info.0.index.to_be_bytes())?;
 		self.funding_info.1.write(writer)?;
 		self.current_counterparty_commitment_txid.write(writer)?;
 		self.prev_counterparty_commitment_txid.write(writer)?;
@@ -972,24 +972,24 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 			},
 		}
 
-		writer.write_all(&byte_utils::be16_to_array(self.on_holder_tx_csv))?;
+		writer.write_all(&self.on_holder_tx_csv.to_be_bytes())?;
 
 		self.commitment_secrets.write(writer)?;
 
 		macro_rules! serialize_htlc_in_commitment {
 			($htlc_output: expr) => {
 				writer.write_all(&[$htlc_output.offered as u8; 1])?;
-				writer.write_all(&byte_utils::be64_to_array($htlc_output.amount_msat))?;
-				writer.write_all(&byte_utils::be32_to_array($htlc_output.cltv_expiry))?;
+				writer.write_all(&$htlc_output.amount_msat.to_be_bytes())?;
+				writer.write_all(&$htlc_output.cltv_expiry.to_be_bytes())?;
 				writer.write_all(&$htlc_output.payment_hash.0[..])?;
 				$htlc_output.transaction_output_index.write(writer)?;
 			}
 		}
 
-		writer.write_all(&byte_utils::be64_to_array(self.counterparty_claimable_outpoints.len() as u64))?;
+		writer.write_all(&(self.counterparty_claimable_outpoints.len() as u64).to_be_bytes())?;
 		for (ref txid, ref htlc_infos) in self.counterparty_claimable_outpoints.iter() {
 			writer.write_all(&txid[..])?;
-			writer.write_all(&byte_utils::be64_to_array(htlc_infos.len() as u64))?;
+			writer.write_all(&(htlc_infos.len() as u64).to_be_bytes())?;
 			for &(ref htlc_output, ref htlc_source) in htlc_infos.iter() {
 				debug_assert!(htlc_source.is_none() || Some(**txid) == self.current_counterparty_commitment_txid
 						|| Some(**txid) == self.prev_counterparty_commitment_txid,
@@ -999,13 +999,13 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 			}
 		}
 
-		writer.write_all(&byte_utils::be64_to_array(self.counterparty_commitment_txn_on_chain.len() as u64))?;
+		writer.write_all(&(self.counterparty_commitment_txn_on_chain.len() as u64).to_be_bytes())?;
 		for (ref txid, commitment_number) in self.counterparty_commitment_txn_on_chain.iter() {
 			writer.write_all(&txid[..])?;
 			writer.write_all(&byte_utils::be48_to_array(*commitment_number))?;
 		}
 
-		writer.write_all(&byte_utils::be64_to_array(self.counterparty_hash_commitment_number.len() as u64))?;
+		writer.write_all(&(self.counterparty_hash_commitment_number.len() as u64).to_be_bytes())?;
 		for (ref payment_hash, commitment_number) in self.counterparty_hash_commitment_number.iter() {
 			writer.write_all(&payment_hash.0[..])?;
 			writer.write_all(&byte_utils::be48_to_array(*commitment_number))?;
@@ -1023,7 +1023,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 		writer.write_all(&byte_utils::be48_to_array(self.current_counterparty_commitment_number))?;
 		writer.write_all(&byte_utils::be48_to_array(self.current_holder_commitment_number))?;
 
-		writer.write_all(&byte_utils::be64_to_array(self.payment_preimages.len() as u64))?;
+		writer.write_all(&(self.payment_preimages.len() as u64).to_be_bytes())?;
 		for payment_preimage in self.payment_preimages.values() {
 			writer.write_all(&payment_preimage.0[..])?;
 		}
@@ -1044,15 +1044,15 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 			}
 		}
 
-		writer.write_all(&byte_utils::be64_to_array(self.pending_events.len() as u64))?;
+		writer.write_all(&(self.pending_events.len() as u64).to_be_bytes())?;
 		for event in self.pending_events.iter() {
 			event.write(writer)?;
 		}
 
 		self.best_block.block_hash().write(writer)?;
-		writer.write_all(&byte_utils::be32_to_array(self.best_block.height()))?;
+		writer.write_all(&self.best_block.height().to_be_bytes())?;
 
-		writer.write_all(&byte_utils::be64_to_array(self.onchain_events_awaiting_threshold_conf.len() as u64))?;
+		writer.write_all(&(self.onchain_events_awaiting_threshold_conf.len() as u64).to_be_bytes())?;
 		for ref entry in self.onchain_events_awaiting_threshold_conf.iter() {
 			entry.write(writer)?;
 		}
@@ -3792,7 +3792,9 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K>
 				return Err(DecodeError::InvalidValue);
 			}
 		}
-		let onchain_tx_handler: OnchainTxHandler<K::Signer> = ReadableArgs::read(reader, keys_manager)?;
+		let onchain_tx_handler: OnchainTxHandler<K::Signer> = ReadableArgs::read(
+			reader, (keys_manager, channel_value_satoshis, channel_keys_id)
+		)?;
 
 		let lockdown_from_offchain = Readable::read(reader)?;
 		let holder_tx_signed = Readable::read(reader)?;