From e296b743bc5547cf26bca4bbd9a0c8e171db3bd1 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Mon, 1 Jul 2024 18:33:25 -0700 Subject: [PATCH] Test fallible commitment secret --- lightning/src/ln/async_signer_tests.rs | 83 ++++++++++++++--------- lightning/src/util/test_channel_signer.rs | 3 + 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index c90db503aea..7a87f7cc9db 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -10,6 +10,8 @@ //! Tests for asynchronous signing. These tests verify that the channel state machine behaves //! properly with a signer implementation that asynchronously derives signatures. +use std::collections::HashSet; + use bitcoin::{Transaction, TxOut, TxIn, Amount}; use bitcoin::blockdata::locktime::absolute::LockTime; use bitcoin::transaction::Version; @@ -17,11 +19,12 @@ use bitcoin::transaction::Version; use crate::chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS; use crate::chain::ChannelMonitorUpdateStatus; use crate::events::bump_transaction::WalletSource; -use crate::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; +use crate::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider}; use crate::ln::{functional_test_utils::*, msgs}; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, RecipientOnionFields}; use crate::util::test_channel_signer::SignerOp; +use crate::util::logger::Logger; #[test] fn test_async_commitment_signature_for_funding_created() { @@ -127,11 +130,17 @@ fn test_async_commitment_signature_for_funding_signed() { #[test] fn test_async_commitment_signature_for_commitment_signed() { - do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(true); - do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(false); + for i in 0..=8 { + let enable_signer_op_order = vec![ + SignerOp::GetPerCommitmentPoint, + SignerOp::ReleaseCommitmentSecret, + SignerOp::SignCounterpartyCommitment, + ].into_iter().filter(|&op| i & (1 << op as u8) != 0).collect(); + do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(enable_signer_op_order); + } } -fn do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(enable_sign_counterparty_commit_first: bool) { +fn do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(enable_signer_op_order: Vec) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -160,31 +169,33 @@ fn do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(enabl // Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a // `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`. dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); + dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::ReleaseCommitmentSecret); dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg); check_added_monitors(dst, 1); - if enable_sign_counterparty_commit_first { - // Unblock CS -> no messages should be sent, since we must send RAA first. - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + let mut enabled_signer_ops = HashSet::new(); + log_trace!(dst.logger, "enable_signer_op_order={:?}", enable_signer_op_order); + for op in enable_signer_op_order { + enabled_signer_ops.insert(op); + dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, op); dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); - let events = dst.node.get_and_clear_pending_msg_events(); - assert!(events.is_empty(), "expected no message, got {}", events.len()); - // Unblock revoke_and_ack -> we should send both RAA + CS. - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); - dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); - get_revoke_commit_msgs(&dst, &src.node.get_our_node_id()); - } else { - // Unblock revoke_and_ack -> we should send just RAA. - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); - dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); - get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id()); - - // Unblock commitment signed -> we should send CS. - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); - dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); - get_htlc_update_msgs(dst, &src.node.get_our_node_id()); + if enabled_signer_ops.contains(&SignerOp::GetPerCommitmentPoint) && enabled_signer_ops.contains(&SignerOp::ReleaseCommitmentSecret) { + // We are just able to send revoke_and_ack + if op == SignerOp::GetPerCommitmentPoint || op == SignerOp::ReleaseCommitmentSecret { + get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id()); + } + // We either just sent or previously sent revoke_and_ack + // and now we are able to send commitment_signed + if op == SignerOp::SignCounterpartyCommitment { + get_htlc_update_msgs(dst, &src.node.get_our_node_id()); + } + } else { + // We can't send either message until RAA is unblocked + let events = dst.node.get_and_clear_pending_msg_events(); + assert!(events.is_empty(), "expected no message, got {}", events.len()); + } } } @@ -296,12 +307,22 @@ enum UnblockSignerAcrossDisconnectCase { #[test] fn test_async_raa_peer_disconnect() { - do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::AtEnd); - do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored); - do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::BeforeReestablish); + do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::AtEnd, true); + do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::AtEnd, false); + do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored, true); + do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored, false); + do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::BeforeReestablish, true); + do_test_async_raa_peer_disconnect(UnblockSignerAcrossDisconnectCase::BeforeReestablish, false); } -fn do_test_async_raa_peer_disconnect(test_case: UnblockSignerAcrossDisconnectCase) { +fn do_test_async_raa_peer_disconnect(test_case: UnblockSignerAcrossDisconnectCase, raa_blocked_by_commit_point: bool) { + // `raa_blocked_by_commit_point` determines whether we block the RAA by blocking the + // signer on `GetPerCommitmentPoint` or `ReleaseCommitmentSecret`. + let block_raa_signer_op = if raa_blocked_by_commit_point { + SignerOp::GetPerCommitmentPoint + } else { + SignerOp::ReleaseCommitmentSecret + }; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -334,7 +355,7 @@ fn do_test_async_raa_peer_disconnect(test_case: UnblockSignerAcrossDisconnectCas // Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a // `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`. - dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); + dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op); dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg); check_added_monitors(dst, 1); @@ -359,13 +380,13 @@ fn do_test_async_raa_peer_disconnect(test_case: UnblockSignerAcrossDisconnectCas if test_case == UnblockSignerAcrossDisconnectCase::BeforeReestablish { // Reenable the signer before the reestablish. - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); + dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op); } dst.node.handle_channel_reestablish(&src.node.get_our_node_id(), &reestablish_1[0]); if test_case == UnblockSignerAcrossDisconnectCase::BeforeMonitorRestored { - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); + dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op); chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); let (outpoint, latest_update, _) = dst.chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id).unwrap().clone(); dst.chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update); @@ -384,7 +405,7 @@ fn do_test_async_raa_peer_disconnect(test_case: UnblockSignerAcrossDisconnectCas } // Mark dst's signer as available and retry: we now expect to see dst's RAA + CS. - dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::GetPerCommitmentPoint); + dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, block_raa_signer_op); dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index fb20528f9ee..48ce7d26833 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -173,6 +173,9 @@ impl ChannelSigner for TestChannelSigner { } fn release_commitment_secret(&self, idx: u64) -> Result<[u8; 32], ()> { + if !self.is_signer_available(SignerOp::ReleaseCommitmentSecret) { + return Err(()); + } { let mut state = self.state.lock().unwrap(); assert!(idx == state.last_holder_revoked_commitment || idx == state.last_holder_revoked_commitment - 1, "can only revoke the current or next unrevoked commitment - trying {}, last revoked {}", idx, state.last_holder_revoked_commitment);