Skip to content

Fix one-confirmation funding_locked generation #491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 11, 2020
Merged

Fix one-confirmation funding_locked generation #491

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b02ccbb
Fix sending funding_locked with 1 conf.
TheBlueMatt Feb 10, 2020
8c0ec5c
Add test for 1-conf channels
TheBlueMatt Feb 10, 2020
ff24530
Add some trace logging for funding_locked and announcement_sigs
TheBlueMatt Feb 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 45 additions & 41 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2987,49 +2987,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<Option<msgs::FundingLocked>, msgs::ErrorMessage> {
let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
if header.bitcoin_hash() != self.last_block_connected {
self.last_block_connected = header.bitcoin_hash();
self.channel_monitor.last_block_hash = self.last_block_connected;
if self.funding_tx_confirmations > 0 {
self.funding_tx_confirmations += 1;
if self.funding_tx_confirmations == self.minimum_depth as u64 {
let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
self.channel_state |= ChannelState::OurFundingLocked as u32;
true
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
self.channel_update_count += 1;
true
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
// We got a reorg but not enough to trigger a force close, just update
// funding_tx_confirmed_in and return.
false
} else if self.channel_state < ChannelState::ChannelFunded as u32 {
panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state);
} else {
// We got a reorg but not enough to trigger a force close, just update
// funding_tx_confirmed_in and return.
false
};
self.funding_tx_confirmed_in = Some(header.bitcoin_hash());

//TODO: Note that this must be a duplicate of the previous commitment point they sent us,
//as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
//they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
//a protocol oversight, but I assume I'm just missing something.
if need_commitment_update {
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
return Ok(Some(msgs::FundingLocked {
channel_id: self.channel_id,
next_per_commitment_point: next_per_commitment_point,
}));
} else {
self.monitor_pending_funding_locked = true;
return Ok(None);
}
}
}
}
}
if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
Expand Down Expand Up @@ -3072,6 +3031,51 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}
}
}
if header.bitcoin_hash() != self.last_block_connected {
self.last_block_connected = header.bitcoin_hash();
self.channel_monitor.last_block_hash = self.last_block_connected;
if self.funding_tx_confirmations > 0 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support zero-conf channels ? If so we should remove this check I guess.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in theory I think we do, but it probably shouldnt be with this API - we should do some kind of risk API that lets you limit exposure to 0confs and such...I havent thought much about how to structure it, yet, though.

if self.funding_tx_confirmations == self.minimum_depth as u64 {
let need_commitment_update = if non_shutdown_state == ChannelState::FundingSent as u32 {
self.channel_state |= ChannelState::OurFundingLocked as u32;
true
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::TheirFundingLocked as u32) {
self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
self.channel_update_count += 1;
true
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
// We got a reorg but not enough to trigger a force close, just update
// funding_tx_confirmed_in and return.
false
} else if self.channel_state < ChannelState::ChannelFunded as u32 {
panic!("Started confirming a channel in a state pre-FundingSent?: {}", self.channel_state);
} else {
// We got a reorg but not enough to trigger a force close, just update
// funding_tx_confirmed_in and return.
false
};
self.funding_tx_confirmed_in = Some(header.bitcoin_hash());

//TODO: Note that this must be a duplicate of the previous commitment point they sent us,
//as otherwise we will have a commitment transaction that they can't revoke (well, kinda,
//they can by sending two revoke_and_acks back-to-back, but not really). This appears to be
//a protocol oversight, but I assume I'm just missing something.
if need_commitment_update {
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
return Ok(Some(msgs::FundingLocked {
channel_id: self.channel_id,
next_per_commitment_point: next_per_commitment_point,
}));
} else {
self.monitor_pending_funding_locked = true;
return Ok(None);
}
}
}
}
}
Ok(None)
}

Expand Down