Skip to content

Commit 7bccfd4

Browse files
TheBlueMattTheBlueMatt
committed
Move ChannelManager to Channel's new block data API
This also moves the scanning of the block for commitment transactions into channel, unifying the error path.
1 parent a583a0d commit 7bccfd4

File tree

2 files changed

+67
-94
lines changed

2 files changed

+67
-94
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 50 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
// You may not use this file except in accordance with one or both of these
88
// licenses.
99

10-
use bitcoin::blockdata::block::BlockHeader;
1110
use bitcoin::blockdata::script::{Script,Builder};
1211
use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
1312
use bitcoin::blockdata::opcodes;
@@ -3502,47 +3501,61 @@ impl<Signer: Sign> Channel<Signer> {
35023501
self.network_sync == UpdateStatus::DisabledMarked
35033502
}
35043503

3505-
pub fn transactions_confirmed(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData) -> Result<(), msgs::ErrorMessage> {
3504+
/// When a transaction is confirmed, we check whether it is or spends the funding transaction
3505+
/// In the first case, we store the confirmation height and calculating the short channel id.
3506+
/// In the second, we simply return an Err indicating we need to be force-closed now.
3507+
pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, logger: &L)
3508+
-> Result<(), msgs::ErrorMessage> where L::Target: Logger {
35063509
let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
3507-
if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
3508-
for &(index_in_block, tx) in txdata.iter() {
3509-
let funding_txo = self.get_funding_txo().unwrap();
3510-
if tx.txid() == funding_txo.txid {
3511-
let txo_idx = funding_txo.index as usize;
3512-
if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
3513-
tx.output[txo_idx].value != self.channel_value_satoshis {
3514-
if self.is_outbound() {
3515-
// If we generated the funding transaction and it doesn't match what it
3516-
// should, the client is really broken and we should just panic and
3517-
// tell them off. That said, because hash collisions happen with high
3518-
// probability in fuzztarget mode, if we're fuzzing we just close the
3519-
// channel and move on.
3520-
#[cfg(not(feature = "fuzztarget"))]
3521-
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
3510+
for &(index_in_block, tx) in txdata.iter() {
3511+
if let Some(funding_txo) = self.get_funding_txo() {
3512+
if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
3513+
if tx.txid() == funding_txo.txid {
3514+
let txo_idx = funding_txo.index as usize;
3515+
if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
3516+
tx.output[txo_idx].value != self.channel_value_satoshis {
3517+
if self.is_outbound() {
3518+
// If we generated the funding transaction and it doesn't match what it
3519+
// should, the client is really broken and we should just panic and
3520+
// tell them off. That said, because hash collisions happen with high
3521+
// probability in fuzztarget mode, if we're fuzzing we just close the
3522+
// channel and move on.
3523+
#[cfg(not(feature = "fuzztarget"))]
3524+
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
3525+
}
3526+
self.channel_state = ChannelState::ShutdownComplete as u32;
3527+
self.update_time_counter += 1;
3528+
return Err(msgs::ErrorMessage {
3529+
channel_id: self.channel_id(),
3530+
data: "funding tx had wrong script/value".to_owned()
3531+
});
3532+
} else {
3533+
if self.is_outbound() {
3534+
for input in tx.input.iter() {
3535+
if input.witness.is_empty() {
3536+
// We generated a malleable funding transaction, implying we've
3537+
// just exposed ourselves to funds loss to our counterparty.
3538+
#[cfg(not(feature = "fuzztarget"))]
3539+
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
3540+
}
3541+
}
3542+
}
3543+
self.funding_tx_confirmation_height = height as u64;
3544+
self.funding_tx_confirmed_in = Some(*block_hash);
3545+
self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
3546+
Ok(scid) => Some(scid),
3547+
Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
3548+
}
35223549
}
3523-
self.channel_state = ChannelState::ShutdownComplete as u32;
3524-
self.update_time_counter += 1;
3550+
}
3551+
}
3552+
for inp in tx.input.iter() {
3553+
if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
3554+
log_trace!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(self.channel_id()));
35253555
return Err(msgs::ErrorMessage {
35263556
channel_id: self.channel_id(),
3527-
data: "funding tx had wrong script/value".to_owned()
3557+
data: "Commitment transaction was confirmed on chain.".to_owned()
35283558
});
3529-
} else {
3530-
if self.is_outbound() {
3531-
for input in tx.input.iter() {
3532-
if input.witness.is_empty() {
3533-
// We generated a malleable funding transaction, implying we've
3534-
// just exposed ourselves to funds loss to our counterparty.
3535-
#[cfg(not(feature = "fuzztarget"))]
3536-
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
3537-
}
3538-
}
3539-
}
3540-
self.funding_tx_confirmation_height = height as u64;
3541-
self.funding_tx_confirmed_in = Some(*block_hash);
3542-
self.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) {
3543-
Ok(scid) => Some(scid),
3544-
Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
3545-
}
35463559
}
35473560
}
35483561
}
@@ -3635,35 +3648,6 @@ impl<Signer: Sign> Channel<Signer> {
36353648
Ok((None, timed_out_htlcs))
36363649
}
36373650

3638-
/// When we receive a new block, we (a) check whether the block contains the funding
3639-
/// transaction (which would start us counting blocks until we send the funding_signed), and
3640-
/// (b) check the height of the block against outbound holding cell HTLCs in case we need to
3641-
/// give up on them prematurely and time them out. Everything else (e.g. commitment
3642-
/// transaction broadcasts, channel closure detection, HTLC transaction broadcasting, etc) is
3643-
/// handled by the ChannelMonitor.
3644-
///
3645-
/// If we return Err, the channel may have been closed, at which point the standard
3646-
/// requirements apply - no calls may be made except those explicitly stated to be allowed
3647-
/// post-shutdown.
3648-
/// Only returns an ErrorAction of DisconnectPeer, if Err.
3649-
///
3650-
/// May return some HTLCs (and their payment_hash) which have timed out and should be failed
3651-
/// back.
3652-
pub fn block_connected(&mut self, header: &BlockHeader, txdata: &TransactionData, height: u32) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
3653-
self.transactions_confirmed(&header.block_hash(), height, txdata)?;
3654-
self.update_best_block(height, header.time)
3655-
}
3656-
3657-
/// Called by channelmanager based on chain blocks being disconnected.
3658-
/// Returns true if we need to close the channel now due to funding transaction
3659-
/// unconfirmation/reorg.
3660-
pub fn block_disconnected(&mut self, header: &BlockHeader, new_height: u32) -> bool {
3661-
if self.update_best_block(new_height, header.time).is_err() {
3662-
return true;
3663-
}
3664-
false
3665-
}
3666-
36673651
// Methods to get unprompted messages to send to the remote end (or where we already returned
36683652
// something in the handler for the message that prompted this message):
36693653

‎lightning/src/ln/channelmanager.rs

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,7 +3334,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
33343334
let short_to_id = &mut channel_state.short_to_id;
33353335
let pending_msg_events = &mut channel_state.pending_msg_events;
33363336
channel_state.by_id.retain(|_, channel| {
3337-
let res = channel.block_connected(header, txdata, height);
3337+
let res = channel.transactions_confirmed(&block_hash, height, txdata, &self.logger)
3338+
.and_then(|_| channel.update_best_block(height, header.time));
33383339
if let Ok((chan_res, mut timed_out_pending_htlcs)) = res {
33393340
for (source, payment_hash) in timed_out_pending_htlcs.drain(..) {
33403341
let chan_update = self.get_channel_update(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe
@@ -3360,38 +3361,22 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
33603361
short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
33613362
}
33623363
} else if let Err(e) = res {
3364+
if let Some(short_id) = channel.get_short_channel_id() {
3365+
short_to_id.remove(&short_id);
3366+
}
3367+
// It looks like our counterparty went on-chain. Close the channel.
3368+
failed_channels.push(channel.force_shutdown(true));
3369+
if let Ok(update) = self.get_channel_update(&channel) {
3370+
pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
3371+
msg: update
3372+
});
3373+
}
33633374
pending_msg_events.push(events::MessageSendEvent::HandleError {
33643375
node_id: channel.get_counterparty_node_id(),
33653376
action: msgs::ErrorAction::SendErrorMessage { msg: e },
33663377
});
33673378
return false;
33683379
}
3369-
if let Some(funding_txo) = channel.get_funding_txo() {
3370-
for &(_, tx) in txdata.iter() {
3371-
for inp in tx.input.iter() {
3372-
if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
3373-
log_trace!(self.logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(channel.channel_id()));
3374-
if let Some(short_id) = channel.get_short_channel_id() {
3375-
short_to_id.remove(&short_id);
3376-
}
3377-
// It looks like our counterparty went on-chain. Close the channel.
3378-
failed_channels.push(channel.force_shutdown(true));
3379-
if let Ok(update) = self.get_channel_update(&channel) {
3380-
pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
3381-
msg: update
3382-
});
3383-
}
3384-
pending_msg_events.push(events::MessageSendEvent::HandleError {
3385-
node_id: channel.get_counterparty_node_id(),
3386-
action: msgs::ErrorAction::SendErrorMessage {
3387-
msg: msgs::ErrorMessage { channel_id: channel.channel_id(), data: "Commitment transaction was confirmed on chain.".to_owned() }
3388-
},
3389-
});
3390-
return false;
3391-
}
3392-
}
3393-
}
3394-
}
33953380
true
33963381
});
33973382

@@ -3457,7 +3442,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
34573442
let short_to_id = &mut channel_state.short_to_id;
34583443
let pending_msg_events = &mut channel_state.pending_msg_events;
34593444
channel_state.by_id.retain(|_, v| {
3460-
if v.block_disconnected(header, new_height) {
3445+
if let Err(err_msg) = v.update_best_block(new_height, header.time) {
34613446
if let Some(short_id) = v.get_short_channel_id() {
34623447
short_to_id.remove(&short_id);
34633448
}
@@ -3467,6 +3452,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
34673452
msg: update
34683453
});
34693454
}
3455+
pending_msg_events.push(events::MessageSendEvent::HandleError {
3456+
node_id: v.get_counterparty_node_id(),
3457+
action: msgs::ErrorAction::SendErrorMessage { msg: err_msg },
3458+
});
34703459
false
34713460
} else {
34723461
true

0 commit comments

Comments
 (0)