Skip to content

Commit 40c918d

Browse files
committed
Support async get_per_commitment_point, release_commitment_secret
This PR adds state to the context to allow a `ChannelSigner` implementation to respond asynchronously to the `get_per_commitment_point`, `release_commitment_secret`, and `sign_counterparty_commitment` methods, which are the main signer methods that are called during channel setup and normal operation.
1 parent 4deb263 commit 40c918d

10 files changed

+1937
-423
lines changed

fuzz/src/chanmon_consistency.rs

+150-27
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,7 @@ use std::sync::atomic;
7272
use std::io::Cursor;
7373
use bitcoin::bech32::u5;
7474

75+
const ASYNC_OPS: u32 = ops::GET_PER_COMMITMENT_POINT | ops::RELEASE_COMMITMENT_SECRET | ops::SIGN_COUNTERPARTY_COMMITMENT;
7576
const MAX_FEE: u32 = 10_000;
7677
struct FuzzEstimator {
7778
ret_val: atomic::AtomicU32,
@@ -196,10 +197,15 @@ impl chain::Watch<TestChannelSigner> for TestChainMonitor {
196197
}
197198
}
198199

200+
struct SignerState {
201+
enforcement: Arc<Mutex<EnforcementState>>,
202+
unavailable: Arc<Mutex<u32>>,
203+
}
204+
199205
struct KeyProvider {
200206
node_secret: SecretKey,
201207
rand_bytes_id: atomic::AtomicU32,
202-
enforcement_states: Mutex<HashMap<[u8;32], Arc<Mutex<EnforcementState>>>>,
208+
enforcement_states: Mutex<HashMap<[u8;32], SignerState>>,
203209
}
204210

205211
impl EntropySource for KeyProvider {
@@ -283,8 +289,15 @@ impl SignerProvider for KeyProvider {
283289
channel_keys_id,
284290
channel_keys_id,
285291
);
286-
let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
287-
TestChannelSigner::new_with_revoked(keys, revoked_commitment, false)
292+
let mut revoked_commitments = self.enforcement_states.lock().unwrap();
293+
let new_state = revoked_commitments.entry(keys.commitment_seed)
294+
.or_insert(SignerState {
295+
enforcement: Arc::new(Mutex::new(EnforcementState::new())),
296+
unavailable: Arc::new(Mutex::new(0)),
297+
});
298+
let mut ret = TestChannelSigner::new_with_revoked(keys, Arc::clone(&new_state.enforcement), false);
299+
ret.unavailable = Arc::clone(&new_state.unavailable);
300+
ret
288301
}
289302

290303
fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::EcdsaSigner, DecodeError> {
@@ -316,17 +329,6 @@ impl SignerProvider for KeyProvider {
316329
}
317330
}
318331

319-
impl KeyProvider {
320-
fn make_enforcement_state_cell(&self, commitment_seed: [u8; 32]) -> Arc<Mutex<EnforcementState>> {
321-
let mut revoked_commitments = self.enforcement_states.lock().unwrap();
322-
if !revoked_commitments.contains_key(&commitment_seed) {
323-
revoked_commitments.insert(commitment_seed, Arc::new(Mutex::new(EnforcementState::new())));
324-
}
325-
let cell = revoked_commitments.get(&commitment_seed).unwrap();
326-
Arc::clone(cell)
327-
}
328-
}
329-
330332
#[inline]
331333
fn check_api_err(api_err: APIError, sendable_bounds_violated: bool) {
332334
match api_err {
@@ -829,7 +831,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
829831
for (idx, dest) in nodes.iter().enumerate() {
830832
if dest.get_our_node_id() == node_id {
831833
for update_add in update_add_htlcs.iter() {
832-
out.locked_write(format!("Delivering update_add_htlc to node {}.\n", idx).as_bytes());
834+
out.locked_write(format!("Delivering update_add_htlc to node {} from node {}.\n", idx, $node).as_bytes());
833835
if !$corrupt_forward {
834836
dest.handle_update_add_htlc(&nodes[$node].get_our_node_id(), update_add);
835837
} else {
@@ -844,19 +846,19 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
844846
}
845847
}
846848
for update_fulfill in update_fulfill_htlcs.iter() {
847-
out.locked_write(format!("Delivering update_fulfill_htlc to node {}.\n", idx).as_bytes());
849+
out.locked_write(format!("Delivering update_fulfill_htlc to node {} from node {}.\n", idx, $node).as_bytes());
848850
dest.handle_update_fulfill_htlc(&nodes[$node].get_our_node_id(), update_fulfill);
849851
}
850852
for update_fail in update_fail_htlcs.iter() {
851-
out.locked_write(format!("Delivering update_fail_htlc to node {}.\n", idx).as_bytes());
853+
out.locked_write(format!("Delivering update_fail_htlc to node {} from node {}.\n", idx, $node).as_bytes());
852854
dest.handle_update_fail_htlc(&nodes[$node].get_our_node_id(), update_fail);
853855
}
854856
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());
857+
out.locked_write(format!("Delivering update_fail_malformed_htlc to node {} from node {}.\n", idx, $node).as_bytes());
856858
dest.handle_update_fail_malformed_htlc(&nodes[$node].get_our_node_id(), update_fail_malformed);
857859
}
858860
if let Some(msg) = update_fee {
859-
out.locked_write(format!("Delivering update_fee to node {}.\n", idx).as_bytes());
861+
out.locked_write(format!("Delivering update_fee to node {} from node {}.\n", idx, $node).as_bytes());
860862
dest.handle_update_fee(&nodes[$node].get_our_node_id(), &msg);
861863
}
862864
let processed_change = !update_add_htlcs.is_empty() || !update_fulfill_htlcs.is_empty() ||
@@ -873,7 +875,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
873875
} });
874876
break;
875877
}
876-
out.locked_write(format!("Delivering commitment_signed to node {}.\n", idx).as_bytes());
878+
out.locked_write(format!("Delivering commitment_signed to node {} from node {}.\n", idx, $node).as_bytes());
877879
dest.handle_commitment_signed(&nodes[$node].get_our_node_id(), &commitment_signed);
878880
break;
879881
}
@@ -882,15 +884,15 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
882884
events::MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
883885
for (idx, dest) in nodes.iter().enumerate() {
884886
if dest.get_our_node_id() == *node_id {
885-
out.locked_write(format!("Delivering revoke_and_ack to node {}.\n", idx).as_bytes());
887+
out.locked_write(format!("Delivering revoke_and_ack to node {} from node {}.\n", idx, $node).as_bytes());
886888
dest.handle_revoke_and_ack(&nodes[$node].get_our_node_id(), msg);
887889
}
888890
}
889891
},
890892
events::MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } => {
891893
for (idx, dest) in nodes.iter().enumerate() {
892894
if dest.get_our_node_id() == *node_id {
893-
out.locked_write(format!("Delivering channel_reestablish to node {}.\n", idx).as_bytes());
895+
out.locked_write(format!("Delivering channel_reestablish to node {} from node {}.\n", idx, $node).as_bytes());
894896
dest.handle_channel_reestablish(&nodes[$node].get_our_node_id(), msg);
895897
}
896898
}
@@ -1289,15 +1291,108 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
12891291
},
12901292
0x89 => { fee_est_c.ret_val.store(253, atomic::Ordering::Release); nodes[2].maybe_update_chan_fees(); },
12911293

1294+
0xa0 => {
1295+
let signer_states = keys_manager_a.enforcement_states.lock().unwrap();
1296+
assert_eq!(signer_states.len(), 1);
1297+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() = ASYNC_OPS;
1298+
}
1299+
0xa1 => {
1300+
let signer_states = keys_manager_a.enforcement_states.lock().unwrap();
1301+
assert_eq!(signer_states.len(), 1);
1302+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::GET_PER_COMMITMENT_POINT;
1303+
nodes[0].signer_unblocked(None);
1304+
}
1305+
0xa2 => {
1306+
let signer_states = keys_manager_a.enforcement_states.lock().unwrap();
1307+
assert_eq!(signer_states.len(), 1);
1308+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::RELEASE_COMMITMENT_SECRET;
1309+
nodes[0].signer_unblocked(None);
1310+
}
1311+
0xa3 => {
1312+
let signer_states = keys_manager_a.enforcement_states.lock().unwrap();
1313+
assert_eq!(signer_states.len(), 1);
1314+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::SIGN_COUNTERPARTY_COMMITMENT;
1315+
nodes[0].signer_unblocked(None);
1316+
}
1317+
1318+
0xa4 => {
1319+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1320+
assert_eq!(signer_states.len(), 2);
1321+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() = ASYNC_OPS;
1322+
}
1323+
0xa5 => {
1324+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1325+
assert_eq!(signer_states.len(), 2);
1326+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::GET_PER_COMMITMENT_POINT;
1327+
nodes[1].signer_unblocked(None);
1328+
}
1329+
0xa6 => {
1330+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1331+
assert_eq!(signer_states.len(), 2);
1332+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::RELEASE_COMMITMENT_SECRET;
1333+
nodes[1].signer_unblocked(None);
1334+
}
1335+
0xa7 => {
1336+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1337+
assert_eq!(signer_states.len(), 2);
1338+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::SIGN_COUNTERPARTY_COMMITMENT;
1339+
nodes[1].signer_unblocked(None);
1340+
}
1341+
1342+
0xa8 => {
1343+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1344+
assert_eq!(signer_states.len(), 2);
1345+
*signer_states.values().last().unwrap().unavailable.lock().unwrap() = ASYNC_OPS;
1346+
}
1347+
0xa9 => {
1348+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1349+
assert_eq!(signer_states.len(), 2);
1350+
*signer_states.values().last().unwrap().unavailable.lock().unwrap() &= !ops::GET_PER_COMMITMENT_POINT;
1351+
nodes[1].signer_unblocked(None);
1352+
}
1353+
0xaa => {
1354+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1355+
assert_eq!(signer_states.len(), 2);
1356+
*signer_states.values().last().unwrap().unavailable.lock().unwrap() &= !ops::RELEASE_COMMITMENT_SECRET;
1357+
nodes[1].signer_unblocked(None);
1358+
}
1359+
0xab => {
1360+
let signer_states = keys_manager_b.enforcement_states.lock().unwrap();
1361+
assert_eq!(signer_states.len(), 2);
1362+
*signer_states.values().last().unwrap().unavailable.lock().unwrap() &= !ops::SIGN_COUNTERPARTY_COMMITMENT;
1363+
nodes[1].signer_unblocked(None);
1364+
}
1365+
1366+
0xac => {
1367+
let signer_states = keys_manager_c.enforcement_states.lock().unwrap();
1368+
assert_eq!(signer_states.len(), 1);
1369+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() = ASYNC_OPS;
1370+
}
1371+
0xad => {
1372+
let signer_states = keys_manager_c.enforcement_states.lock().unwrap();
1373+
assert_eq!(signer_states.len(), 1);
1374+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::GET_PER_COMMITMENT_POINT;
1375+
nodes[2].signer_unblocked(None);
1376+
}
1377+
0xae => {
1378+
let signer_states = keys_manager_c.enforcement_states.lock().unwrap();
1379+
assert_eq!(signer_states.len(), 1);
1380+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::RELEASE_COMMITMENT_SECRET;
1381+
nodes[2].signer_unblocked(None);
1382+
}
1383+
0xaf => {
1384+
let signer_states = keys_manager_c.enforcement_states.lock().unwrap();
1385+
assert_eq!(signer_states.len(), 1);
1386+
*signer_states.values().next().unwrap().unavailable.lock().unwrap() &= !ops::SIGN_COUNTERPARTY_COMMITMENT;
1387+
nodes[2].signer_unblocked(None);
1388+
}
1389+
12921390
0xff => {
12931391
// Test that no channel is in a stuck state where neither party can send funds even
12941392
// after we resolve all pending events.
12951393
// First make sure there are no pending monitor updates, resetting the error state
12961394
// 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-
1395+
out.locked_write(format!("Restoring monitors...\n").as_bytes());
13011396
if let Some((id, _)) = monitor_a.latest_monitors.lock().unwrap().get(&chan_1_funding) {
13021397
monitor_a.chain_monitor.force_channel_monitor_updated(chan_1_funding, *id);
13031398
nodes[0].process_monitor_events();
@@ -1316,7 +1411,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13161411
}
13171412

13181413
// Next, make sure peers are all connected to each other
1414+
out.locked_write(format!("Reconnecting peers...\n").as_bytes());
1415+
13191416
if chan_a_disconnected {
1417+
out.locked_write(format!("Reconnecting node 0 and node 1...\n").as_bytes());
13201418
nodes[0].peer_connected(&nodes[1].get_our_node_id(), &Init {
13211419
features: nodes[1].init_features(), networks: None, remote_network_address: None
13221420
}, true).unwrap();
@@ -1326,6 +1424,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13261424
chan_a_disconnected = false;
13271425
}
13281426
if chan_b_disconnected {
1427+
out.locked_write(format!("Reconnecting node 1 and node 2...\n").as_bytes());
13291428
nodes[1].peer_connected(&nodes[2].get_our_node_id(), &Init {
13301429
features: nodes[2].init_features(), networks: None, remote_network_address: None
13311430
}, true).unwrap();
@@ -1335,8 +1434,30 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13351434
chan_b_disconnected = false;
13361435
}
13371436

1437+
out.locked_write(format!("Restoring signers...\n").as_bytes());
1438+
1439+
*monitor_a.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1440+
*monitor_b.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1441+
*monitor_c.persister.update_ret.lock().unwrap() = ChannelMonitorUpdateStatus::Completed;
1442+
1443+
for signer_state in keys_manager_a.enforcement_states.lock().unwrap().values() {
1444+
*signer_state.unavailable.lock().unwrap() = 0;
1445+
}
1446+
for signer_state in keys_manager_b.enforcement_states.lock().unwrap().values() {
1447+
*signer_state.unavailable.lock().unwrap() = 0;
1448+
}
1449+
for signer_state in keys_manager_c.enforcement_states.lock().unwrap().values() {
1450+
*signer_state.unavailable.lock().unwrap() = 0;
1451+
}
1452+
nodes[0].signer_unblocked(None);
1453+
nodes[1].signer_unblocked(None);
1454+
nodes[2].signer_unblocked(None);
1455+
1456+
out.locked_write(format!("Running event queues to quiescence...\n").as_bytes());
1457+
13381458
for i in 0..std::usize::MAX {
13391459
if i == 100 { panic!("It may take may iterations to settle the state, but it should not take forever"); }
1460+
13401461
// Then, make sure any current forwards make their way to their destination
13411462
if process_msg_events!(0, false, ProcessMessages::AllMessages) { continue; }
13421463
if process_msg_events!(1, false, ProcessMessages::AllMessages) { continue; }
@@ -1349,6 +1470,8 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
13491470
break;
13501471
}
13511472

1473+
out.locked_write(format!("All channels restored to normal operation.\n").as_bytes());
1474+
13521475
// Finally, make sure that at least one end of each channel can make a substantial payment
13531476
assert!(
13541477
send_payment(&nodes[0], &nodes[1], chan_a, 10_000_000, &mut payment_id, &mut payment_idx) ||

lightning/src/chain/channelmonitor.rs

+1-3
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

+3
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)