Skip to content

Commit 4940ffa

Browse files
committed
(XXX: Add tests) Handle 1-conf funding_locked in channel no matter the event order
See comment in the diff for more details
1 parent 0c06b87 commit 4940ffa

File tree

2 files changed

+68
-45
lines changed

2 files changed

+68
-45
lines changed

lightning/src/ln/channel.rs

Lines changed: 67 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3501,11 +3501,63 @@ impl<Signer: Sign> Channel<Signer> {
35013501
self.network_sync == UpdateStatus::DisabledMarked
35023502
}
35033503

3504+
fn check_get_funding_locked(&mut self, height: u32) -> Option<msgs::FundingLocked> {
3505+
if self.funding_tx_confirmation_height <= 0 {
3506+
return None;
3507+
}
3508+
3509+
let funding_tx_confirmations = height as i64 - self.funding_tx_confirmation_height as i64 + 1;
3510+
if funding_tx_confirmations <= 0 {
3511+
self.funding_tx_confirmation_height = 0;
3512+
}
3513+
3514+
if funding_tx_confirmations < self.minimum_depth as i64 {
3515+
return None;
3516+
}
3517+
3518+
let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
3519+
let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
3520+
self.channel_state |= ChannelState::OurFundingLocked as u32;
3521+
true
3522+
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
3523+
self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
3524+
self.update_time_counter += 1;
3525+
true
3526+
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
3527+
// We got a reorg but not enough to trigger a force close, just update
3528+
// funding_tx_confirmed_in and return.
3529+
false
3530+
} else if self.channel_state < ChannelState::ChannelFunded as u32 {
3531+
panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state);
3532+
} else {
3533+
// We got a reorg but not enough to trigger a force close, just update
3534+
// funding_tx_confirmed_in and return.
3535+
false
3536+
};
3537+
3538+
//TODO: Note that this must be a duplicate of the previous commitment point they sent us,
3539+
//as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
3540+
//they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
3541+
//a protocol oversight, but I assume I'm just missing something.
3542+
if need_commitment_update {
3543+
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
3544+
let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
3545+
return Some(msgs::FundingLocked {
3546+
channel_id: self.channel_id,
3547+
next_per_commitment_point,
3548+
});
3549+
} else {
3550+
self.monitor_pending_funding_locked = true;
3551+
}
3552+
}
3553+
None
3554+
}
3555+
35043556
/// When a transaction is confirmed, we check whether it is or spends the funding transaction
35053557
/// In the first case, we store the confirmation height and calculating the short channel id.
35063558
/// In the second, we simply return an Err indicating we need to be force-closed now.
35073559
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 {
3560+
-> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> where L::Target: Logger {
35093561
let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
35103562
for &(index_in_block, tx) in txdata.iter() {
35113563
if let Some(funding_txo) = self.get_funding_txo() {
@@ -3548,6 +3600,13 @@ impl<Signer: Sign> Channel<Signer> {
35483600
}
35493601
}
35503602
}
3603+
// If we allow 1-conf funding, we may need to check for funding_locked here and
3604+
// send it immediately instead of waiting for an update_best_block call (which
3605+
// may have already happened for this block).
3606+
// XXX: Test this case!
3607+
if let Some(funding_locked) = self.check_get_funding_locked(height) {
3608+
return Ok(Some(funding_locked));
3609+
}
35513610
}
35523611
for inp in tx.input.iter() {
35533612
if inp.previous_output == funding_txo.into_bitcoin_outpoint() {
@@ -3560,7 +3619,7 @@ impl<Signer: Sign> Channel<Signer> {
35603619
}
35613620
}
35623621
}
3563-
Ok(())
3622+
Ok(None)
35643623
}
35653624

35663625
/// When a new block is connected, we check the height of the block against outbound holding
@@ -3590,13 +3649,15 @@ impl<Signer: Sign> Channel<Signer> {
35903649
});
35913650

35923651
self.update_time_counter = cmp::max(self.update_time_counter, highest_header_time);
3652+
3653+
if let Some(funding_locked) = self.check_get_funding_locked(height) {
3654+
return Ok((Some(funding_locked), timed_out_htlcs));
3655+
}
3656+
35933657
if self.funding_tx_confirmation_height > 0 {
35943658
let funding_tx_confirmations = height as i64 - self.funding_tx_confirmation_height as i64 + 1;
3595-
if funding_tx_confirmations <= 0 {
3596-
self.funding_tx_confirmation_height = 0;
3597-
}
3598-
35993659
let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
3660+
36003661
if (non_shutdown_state >= ChannelState::ChannelFunded as u32 ||
36013662
(non_shutdown_state & ChannelState::OurFundingLocked as u32) == ChannelState::OurFundingLocked as u32) &&
36023663
funding_tx_confirmations < self.minimum_depth as i64 / 2 {
@@ -3605,44 +3666,6 @@ impl<Signer: Sign> Channel<Signer> {
36053666
data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth, funding_tx_confirmations),
36063667
});
36073668
}
3608-
3609-
if funding_tx_confirmations == self.minimum_depth as i64 {
3610-
let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
3611-
self.channel_state |= ChannelState::OurFundingLocked as u32;
3612-
true
3613-
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
3614-
self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
3615-
self.update_time_counter += 1;
3616-
true
3617-
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
3618-
// We got a reorg but not enough to trigger a force close, just update
3619-
// funding_tx_confirmed_in and return.
3620-
false
3621-
} else if self.channel_state < ChannelState::ChannelFunded as u32 {
3622-
panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state);
3623-
} else {
3624-
// We got a reorg but not enough to trigger a force close, just update
3625-
// funding_tx_confirmed_in and return.
3626-
false
3627-
};
3628-
3629-
//TODO: Note that this must be a duplicate of the previous commitment point they sent us,
3630-
//as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
3631-
//they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
3632-
//a protocol oversight, but I assume I'm just missing something.
3633-
if need_commitment_update {
3634-
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
3635-
let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
3636-
return Ok((Some(msgs::FundingLocked {
3637-
channel_id: self.channel_id,
3638-
next_per_commitment_point,
3639-
}), timed_out_htlcs));
3640-
} else {
3641-
self.monitor_pending_funding_locked = true;
3642-
return Ok((None, timed_out_htlcs));
3643-
}
3644-
}
3645-
}
36463669
}
36473670

36483671
Ok((None, timed_out_htlcs))

lightning/src/ln/channelmanager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3427,7 +3427,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
34273427
log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height);
34283428

34293429
let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
3430-
self.do_chain_event(height, |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|_| (None, Vec::new())));
3430+
self.do_chain_event(height, |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new())));
34313431
}
34323432

34333433
/// Updates channel state with the current best blockchain tip. You should attempt to call this

0 commit comments

Comments
 (0)