Skip to content

Commit 9943fb1

Browse files
watersonwaterson
committed
Support normal channel operation with async signing
This is a do-over of #2653, wherein we support asynchronous signing for 'normal' channel operation. This involves allowing the following `ChannelSigner` methods to return an `Err` result, indicating that the requested value is not available: - get_per_commitment_point - release_commitment_secret - sign_counterparty_commitment When the value does become available, channel operation can be resumed by invoking `signer_unblocked`. Note that this adds the current and next per-commitment point to the state that is persisted by the channel monitor.
1 parent 4deb263 commit 9943fb1

File tree

10 files changed

+1777
-267
lines changed

10 files changed

+1777
-267
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 175 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use lightning::ln::functional_test_utils::*;
4949
use lightning::offers::invoice::{BlindedPayInfo, UnsignedBolt12Invoice};
5050
use lightning::offers::invoice_request::UnsignedInvoiceRequest;
5151
use lightning::onion_message::{Destination, MessageRouter, OnionMessagePath};
52-
use lightning::util::test_channel_signer::{TestChannelSigner, EnforcementState};
52+
use lightning::util::test_channel_signer::{TestChannelSigner, EnforcementState, ops};
5353
use lightning::util::errors::APIError;
5454
use lightning::util::logger::Logger;
5555
use lightning::util::config::UserConfig;
@@ -72,6 +72,8 @@ use std::sync::atomic;
7272
use std::io::Cursor;
7373
use bitcoin::bech32::u5;
7474

75+
#[allow(unused)]
76+
const ASYNC_OPS: u32 = ops::GET_PER_COMMITMENT_POINT | ops::RELEASE_COMMITMENT_SECRET | ops::SIGN_COUNTERPARTY_COMMITMENT;
7577
const MAX_FEE: u32 = 10_000;
7678
struct FuzzEstimator {
7779
ret_val: atomic::AtomicU32,
@@ -297,7 +299,6 @@ impl SignerProvider for KeyProvider {
297299
inner,
298300
state,
299301
disable_revocation_policy_check: false,
300-
available: Arc::new(Mutex::new(true)),
301302
})
302303
}
303304

@@ -829,7 +830,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
829830
for (idx, dest) in nodes.iter().enumerate() {
830831
if dest.get_our_node_id() == node_id {
831832
for update_add in update_add_htlcs.iter() {
832-
out.locked_write(format!("Delivering update_add_htlc to node {}.\n", idx).as_bytes());
833+
out.locked_write(format!("Delivering update_add_htlc to node {} from node {}.\n", idx, $node).as_bytes());
833834
if !$corrupt_forward {
834835
dest.handle_update_add_htlc(&nodes[$node].get_our_node_id(), update_add);
835836
} else {
@@ -844,19 +845,19 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
844845
}
845846
}
846847
for update_fulfill in update_fulfill_htlcs.iter() {
847-
out.locked_write(format!("Delivering update_fulfill_htlc to node {}.\n", idx).as_bytes());
848+
out.locked_write(format!("Delivering update_fulfill_htlc to node {} from node {}.\n", idx, $node).as_bytes());
848849
dest.handle_update_fulfill_htlc(&nodes[$node].get_our_node_id(), update_fulfill);
849850
}
850851
for update_fail in update_fail_htlcs.iter() {
851-
out.locked_write(format!("Delivering update_fail_htlc to node {}.\n", idx).as_bytes());
852+
out.locked_write(format!("Delivering update_fail_htlc to node {} from node {}.\n", idx, $node).as_bytes());
852853
dest.handle_update_fail_htlc(&nodes[$node].get_our_node_id(), update_fail);
853854
}
854855
for update_fail_malformed in update_fail_malformed_htlcs.iter() {
855-
out.locked_write(format!("Delivering update_fail_malformed_htlc to node {}.\n", idx).as_bytes());
856+
out.locked_write(format!("Delivering update_fail_malformed_htlc to node {} from node {}.\n", idx, $node).as_bytes());
856857
dest.handle_update_fail_malformed_htlc(&nodes[$node].get_our_node_id(), update_fail_malformed);
857858
}
858859
if let Some(msg) = update_fee {
859-
out.locked_write(format!("Delivering update_fee to node {}.\n", idx).as_bytes());
860+
out.locked_write(format!("Delivering update_fee to node {} from node {}.\n", idx, $node).as_bytes());
860861
dest.handle_update_fee(&nodes[$node].get_our_node_id(), &msg);
861862
}
862863
let processed_change = !update_add_htlcs.is_empty() || !update_fulfill_htlcs.is_empty() ||
@@ -873,7 +874,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
873874
} });
874875
break;
875876
}
876-
out.locked_write(format!("Delivering commitment_signed to node {}.\n", idx).as_bytes());
877+
out.locked_write(format!("Delivering commitment_signed to node {} from node {}.\n", idx, $node).as_bytes());
877878
dest.handle_commitment_signed(&nodes[$node].get_our_node_id(), &commitment_signed);
878879
break;
879880
}
@@ -882,15 +883,15 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
882883
events::MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
883884
for (idx, dest) in nodes.iter().enumerate() {
884885
if dest.get_our_node_id() == *node_id {
885-
out.locked_write(format!("Delivering revoke_and_ack to node {}.\n", idx).as_bytes());
886+
out.locked_write(format!("Delivering revoke_and_ack to node {} from node {}.\n", idx, $node).as_bytes());
886887
dest.handle_revoke_and_ack(&nodes[$node].get_our_node_id(), msg);
887888
}
888889
}
889890
},
890891
events::MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } => {
891892
for (idx, dest) in nodes.iter().enumerate() {
892893
if dest.get_our_node_id() == *node_id {
893-
out.locked_write(format!("Delivering channel_reestablish to node {}.\n", idx).as_bytes());
894+
out.locked_write(format!("Delivering channel_reestablish to node {} from node {}.\n", idx, $node).as_bytes());
894895
dest.handle_channel_reestablish(&nodes[$node].get_our_node_id(), msg);
895896
}
896897
}
@@ -913,7 +914,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
913914
_ => if out.may_fail.load(atomic::Ordering::Acquire) {
914915
return;
915916
} else {
916-
panic!("Unhandled message event {:?}", event)
917+
panic!("Unhandled message event on node {}, {:?}", $node, event)
917918
},
918919
}
919920
if $limit_events != ProcessMessages::AllMessages {
@@ -1289,15 +1290,124 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
12891290
},
12901291
0x89 => { fee_est_c.ret_val.store(253, atomic::Ordering::Release); nodes[2].maybe_update_chan_fees(); },
12911292

1293+
#[cfg(async_signing)]
1294+
0xa0 => {
1295+
let states = keys_manager_a.enforcement_states.lock().unwrap();
1296+
assert_eq!(states.len(), 1);
1297+
states.values().next().unwrap().lock().unwrap().set_signer_unavailable(ASYNC_OPS);
1298+
}
1299+
#[cfg(async_signing)]
1300+
0xa1 => {
1301+
let states = keys_manager_a.enforcement_states.lock().unwrap();
1302+
assert_eq!(states.len(), 1);
1303+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::GET_PER_COMMITMENT_POINT);
1304+
nodes[0].signer_unblocked(None);
1305+
}
1306+
#[cfg(async_signing)]
1307+
0xa2 => {
1308+
let states = keys_manager_a.enforcement_states.lock().unwrap();
1309+
assert_eq!(states.len(), 1);
1310+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::RELEASE_COMMITMENT_SECRET);
1311+
nodes[0].signer_unblocked(None);
1312+
}
1313+
#[cfg(async_signing)]
1314+
0xa3 => {
1315+
let states = keys_manager_a.enforcement_states.lock().unwrap();
1316+
assert_eq!(states.len(), 1);
1317+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::SIGN_COUNTERPARTY_COMMITMENT);
1318+
nodes[0].signer_unblocked(None);
1319+
}
1320+
1321+
#[cfg(async_signing)]
1322+
0xa4 => {
1323+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1324+
assert_eq!(states.len(), 2);
1325+
states.values().next().unwrap().lock().unwrap().set_signer_unavailable(ASYNC_OPS);
1326+
}
1327+
#[cfg(async_signing)]
1328+
0xa5 => {
1329+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1330+
assert_eq!(states.len(), 2);
1331+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::GET_PER_COMMITMENT_POINT);
1332+
nodes[1].signer_unblocked(None);
1333+
}
1334+
#[cfg(async_signing)]
1335+
0xa6 => {
1336+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1337+
assert_eq!(states.len(), 2);
1338+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::RELEASE_COMMITMENT_SECRET);
1339+
nodes[1].signer_unblocked(None);
1340+
}
1341+
#[cfg(async_signing)]
1342+
0xa7 => {
1343+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1344+
assert_eq!(states.len(), 2);
1345+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::SIGN_COUNTERPARTY_COMMITMENT);
1346+
nodes[1].signer_unblocked(None);
1347+
}
1348+
1349+
#[cfg(async_signing)]
1350+
0xa8 => {
1351+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1352+
assert_eq!(states.len(), 2);
1353+
states.values().last().unwrap().lock().unwrap().set_signer_unavailable(ASYNC_OPS);
1354+
}
1355+
#[cfg(async_signing)]
1356+
0xa9 => {
1357+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1358+
assert_eq!(states.len(), 2);
1359+
states.values().last().unwrap().lock().unwrap().set_signer_available(ops::GET_PER_COMMITMENT_POINT);
1360+
nodes[1].signer_unblocked(None);
1361+
}
1362+
#[cfg(async_signing)]
1363+
0xaa => {
1364+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1365+
assert_eq!(states.len(), 2);
1366+
states.values().last().unwrap().lock().unwrap().set_signer_available(ops::RELEASE_COMMITMENT_SECRET);
1367+
nodes[1].signer_unblocked(None);
1368+
}
1369+
#[cfg(async_signing)]
1370+
0xab => {
1371+
let states = keys_manager_b.enforcement_states.lock().unwrap();
1372+
assert_eq!(states.len(), 2);
1373+
states.values().last().unwrap().lock().unwrap().set_signer_available(ops::SIGN_COUNTERPARTY_COMMITMENT);
1374+
nodes[1].signer_unblocked(None);
1375+
}
1376+
1377+
#[cfg(async_signing)]
1378+
0xac => {
1379+
let states = keys_manager_c.enforcement_states.lock().unwrap();
1380+
assert_eq!(states.len(), 1);
1381+
states.values().next().unwrap().lock().unwrap().set_signer_unavailable(ASYNC_OPS);
1382+
}
1383+
#[cfg(async_signing)]
1384+
0xad => {
1385+
let states = keys_manager_c.enforcement_states.lock().unwrap();
1386+
assert_eq!(states.len(), 1);
1387+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::GET_PER_COMMITMENT_POINT);
1388+
nodes[2].signer_unblocked(None);
1389+
}
1390+
#[cfg(async_signing)]
1391+
0xae => {
1392+
let states = keys_manager_c.enforcement_states.lock().unwrap();
1393+
assert_eq!(states.len(), 1);
1394+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::RELEASE_COMMITMENT_SECRET);
1395+
nodes[2].signer_unblocked(None);
1396+
}
1397+
#[cfg(async_signing)]
1398+
0xaf => {
1399+
let states = keys_manager_c.enforcement_states.lock().unwrap();
1400+
assert_eq!(states.len(), 1);
1401+
states.values().next().unwrap().lock().unwrap().set_signer_available(ops::SIGN_COUNTERPARTY_COMMITMENT);
1402+
nodes[2].signer_unblocked(None);
1403+
}
1404+
12921405
0xff => {
12931406
// Test that no channel is in a stuck state where neither party can send funds even
12941407
// after we resolve all pending events.
12951408
// First make sure there are no pending monitor updates, resetting the error state
12961409
// and calling force_channel_monitor_updated for each monitor.
1297-
*monitor_a.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1298-
*monitor_b.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1299-
*monitor_c.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1300-
1410+
out.locked_write(b"Restoring monitors...\n");
13011411
if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
13021412
monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
13031413
nodes[0].process_monitor_events();
@@ -1316,7 +1426,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13161426
}
13171427

13181428
// Next, make sure peers are all connected to each other
1429+
out.locked_write(b"Reconnecting peers...\n");
1430+
13191431
if chan_a_disconnected {
1432+
out.locked_write(b"Reconnecting node 0 and node 1...\n");
13201433
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init {
13211434
features: nodes[1].init_features(), networks: None, remote_network_address: None
13221435
}, true).unwrap();
@@ -1326,6 +1439,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13261439
chan_a_disconnected = false;
13271440
}
13281441
if chan_b_disconnected {
1442+
out.locked_write(b"Reconnecting node 1 and node 2...\n");
13291443
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init {
13301444
features: nodes[2].init_features(), networks: None, remote_network_address: None
13311445
}, true).unwrap();
@@ -1335,8 +1449,33 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13351449
chan_b_disconnected = false;
13361450
}
13371451

1452+
out.locked_write(b"Restoring signers...\n");
1453+
1454+
*monitor_a.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1455+
*monitor_b.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1456+
*monitor_c.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1457+
1458+
#[cfg(async_signing)]
1459+
{
1460+
for state in keys_manager_a.enforcement_states.lock().unwrap().values() {
1461+
state.lock().unwrap().set_signer_available(!0);
1462+
}
1463+
for state in keys_manager_b.enforcement_states.lock().unwrap().values() {
1464+
state.lock().unwrap().set_signer_available(!0);
1465+
}
1466+
for state in keys_manager_c.enforcement_states.lock().unwrap().values() {
1467+
state.lock().unwrap().set_signer_available(!0);
1468+
}
1469+
nodes[0].signer_unblocked(None);
1470+
nodes[1].signer_unblocked(None);
1471+
nodes[2].signer_unblocked(None);
1472+
}
1473+
1474+
out.locked_write(b"Running event queues to quiescence...\n");
1475+
13381476
for i in 0..std::usize::MAX {
13391477
if i == 100 { panic!("It may take may iterations to settle the state, but it should not take forever"); }
1478+
13401479
// Then, make sure any current forwards make their way to their destination
13411480
if process_msg_events!(0, false, ProcessMessages::AllMessages) { continue; }
13421481
if process_msg_events!(1, false, ProcessMessages::AllMessages) { continue; }
@@ -1349,13 +1488,34 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13491488
break;
13501489
}
13511490

1491+
out.locked_write(b"All channels restored to normal operation.\n");
1492+
13521493
// Finally, make sure that at least one end of each channel can make a substantial payment
13531494
assert!(
13541495
send_payment(&nodes[0], &nodes[1], chan_a, 10_000_000, &mut payment_id, &mut payment_idx) ||
13551496
send_payment(&nodes[1], &nodes[0], chan_a, 10_000_000, &mut payment_id, &mut payment_idx));
1497+
out.locked_write(b"Successfully sent a payment between node 0 and node 1.\n");
1498+
13561499
assert!(
13571500
send_payment(&nodes[1], &nodes[2], chan_b, 10_000_000, &mut payment_id, &mut payment_idx) ||
13581501
send_payment(&nodes[2], &nodes[1], chan_b, 10_000_000, &mut payment_id, &mut payment_idx));
1502+
out.locked_write(b"Successfully sent a payment between node 1 and node 2.\n");
1503+
1504+
out.locked_write(b"Flushing pending messages.\n");
1505+
for i in 0..std::usize::MAX {
1506+
if i == 100 { panic!("It may take may iterations to settle the state, but it should not take forever"); }
1507+
1508+
// Then, make sure any current forwards make their way to their destination
1509+
if process_msg_events!(0, false, ProcessMessages::AllMessages) { continue; }
1510+
if process_msg_events!(1, false, ProcessMessages::AllMessages) { continue; }
1511+
if process_msg_events!(2, false, ProcessMessages::AllMessages) { continue; }
1512+
// ...making sure any pending PendingHTLCsForwardable events are handled and
1513+
// payments claimed.
1514+
if process_events!(0, false) { continue; }
1515+
if process_events!(1, false) { continue; }
1516+
if process_events!(2, false) { continue; }
1517+
break;
1518+
}
13591519

13601520
last_htlc_clear_fee_a = fee_est_a.ret_val.load(atomic::Ordering::Acquire);
13611521
last_htlc_clear_fee_b = fee_est_b.ret_val.load(atomic::Ordering::Acquire);

‎lightning/src/chain/channelmonitor.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2944,9 +2944,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
29442944
},
29452945
commitment_txid: htlc.commitment_txid,
29462946
per_commitment_number: htlc.per_commitment_number,
2947-
per_commitment_point: self.onchain_tx_handler.signer.get_per_commitment_point(
2948-
htlc.per_commitment_number, &self.onchain_tx_handler.secp_ctx,
2949-
),
2947+
per_commitment_point: htlc.per_commitment_point,
29502948
feerate_per_kw: 0,
29512949
htlc: htlc.htlc,
29522950
preimage: htlc.preimage,

‎lightning/src/chain/onchaintx.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ pub(crate) struct ExternalHTLCClaim {
178178
pub(crate) htlc: HTLCOutputInCommitment,
179179
pub(crate) preimage: Option<PaymentPreimage>,
180180
pub(crate) counterparty_sig: Signature,
181+
pub(crate) per_commitment_point: bitcoin::secp256k1::PublicKey,
181182
}
182183

183184
// Represents the different types of claims for which events are yielded externally to satisfy said
@@ -1177,9 +1178,11 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
11771178
})
11781179
.map(|(htlc_idx, htlc)| {
11791180
let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[htlc_idx];
1181+
11801182
ExternalHTLCClaim {
11811183
commitment_txid: trusted_tx.txid(),
11821184
per_commitment_number: trusted_tx.commitment_number(),
1185+
per_commitment_point: trusted_tx.per_commitment_point(),
11831186
htlc: htlc.clone(),
11841187
preimage: *preimage,
11851188
counterparty_sig: counterparty_htlc_sig,

0 commit comments

Comments
 (0)