Test that EnforcingChannelKeys doesn't panic on duplicate RAAs

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -240,7 +240,10 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
 	secp_ctx: Secp256k1<secp256k1::All>,
 	channel_value_satoshis: u64,
 
+	#[cfg(not(test))]
 	local_keys: ChanSigner,
+	#[cfg(test)]
+	pub(super) local_keys: ChanSigner,
 	shutdown_pubkey: PublicKey,
 
 	// Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction

lightning/src/ln/functional_tests.rs

@@ -4,12 +4,12 @@
 
 use chain::transaction::OutPoint;
 use chain::chaininterface::{ChainListener, ChainWatchInterfaceUtil};
-use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor};
+use chain::keysinterface::{ChannelKeys, KeysInterface, SpendableOutputDescriptor};
 use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
 use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT};
 use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ManyChannelMonitor, ANTI_REORG_DELAY};
 use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT, Channel, ChannelError};
-use ln::onion_utils;
+use ln::{chan_utils, onion_utils};
 use ln::router::{Route, RouteHop};
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
@@ -6767,6 +6767,29 @@ fn test_set_outpoints_partial_claiming() {
 	}
 }
 
+#[test]
+fn test_counterparty_raa_skip_no_crash() {
+	// Previously, if our counterparty sent two RAAs in a row without us having provided a
+	// commitment transaction, we would have happily carried on and provided them the next
+	// commitment transaction based on one RAA forward. This would probably eventually have led to
+	// channel closure, but it would not have resulted in funds loss. Still, our
+	// EnforcingChannelKeys would have paniced as it doesn't like jumps into the future. Here, we
+	// check simply that the channel is closed in response to such an RAA, but don't check whether
+	// we decide to punish our counterparty for revoking their funds (as we don't currently
+	// implement that).
+	let nodes = create_network(2, &[None, None]);
+	let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::supported(), InitFeatures::supported());
+
+	let commitment_seed = nodes[0].node.channel_state.lock().unwrap().by_id.get_mut(&chan.2).unwrap().local_keys.commitment_seed().clone();
+	let next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
+		&SecretKey::from_slice(&chan_utils::build_commitment_secret(&commitment_seed, (1 << 48) - 3)).unwrap());
+	let per_commitment_secret = chan_utils::build_commitment_secret(&commitment_seed, (1 << 48) - 1);
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
+		&msgs::RevokeAndACK { channel_id: chan.2, per_commitment_secret, next_per_commitment_point });
+	assert_eq!(check_closed_broadcast!(nodes[1], true).unwrap().data, "Got a revoke when we weren't expecting one");
+}
+
 #[test]
 fn test_bump_txn_sanitize_tracking_maps() {
 	// Sanitizing pendning_claim_request and claimable_outpoints used to be buggy,