Skip to content

Commit

Permalink
Use block timestamps as the min for generated update messages.
Browse files Browse the repository at this point in the history 
Fixes issue #493 and should resolve some issues where other nodes
(incorrectly) reject channel_update/node_announcement messages
which have a serial number that is not a relatively recent
timestamp.
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Mar 6, 2020
1 parent c2ca6d3 commit 78c48f7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 19 deletions.
37 changes: 19 additions & 18 deletions lightning/src/ln/channel.rs
View file
Expand Up @@ -295,7 +295,7 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
holding_cell_update_fee: Option<u64>,
next_local_htlc_id: u64,
next_remote_htlc_id: u64,
channel_update_count: u32,
update_time_counter: u32,
feerate_per_kw: u64,

#[cfg(debug_assertions)]
Expand Down Expand Up @@ -490,7 +490,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
holding_cell_update_fee: None,
next_local_htlc_id: 0,
next_remote_htlc_id: 0,
channel_update_count: 1,
update_time_counter: 1,

resend_order: RAACommitmentOrder::CommitmentFirst,

Expand Down Expand Up @@ -714,7 +714,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
holding_cell_update_fee: None,
next_local_htlc_id: 0,
next_remote_htlc_id: 0,
channel_update_count: 1,
update_time_counter: 1,

resend_order: RAACommitmentOrder::CommitmentFirst,

Expand Down Expand Up @@ -1586,7 +1586,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
self.channel_state |= ChannelState::TheirFundingLocked as u32;
} else if non_shutdown_state == (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) {
self.channel_state = ChannelState::ChannelFunded as u32 | (self.channel_state & MULTI_STATE_FLAGS);
self.channel_update_count += 1;
self.update_time_counter += 1;
} else if (self.channel_state & (ChannelState::ChannelFunded as u32) != 0 &&
// Note that funding_signed/funding_created will have decremented both by 1!
self.cur_local_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 1 &&
Expand Down Expand Up @@ -2480,7 +2480,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}
Channel::<ChanSigner>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
self.pending_update_fee = Some(msg.feerate_per_kw as u64);
self.channel_update_count += 1;
self.update_time_counter += 1;
Ok(())
}

Expand Down Expand Up @@ -2763,7 +2763,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
// From here on out, we may not fail!

self.channel_state |= ChannelState::RemoteShutdownSent as u32;
self.channel_update_count += 1;
self.update_time_counter += 1;

// We can't send our shutdown until we've committed all of our pending HTLCs, but the
// remote side is unlikely to accept any new HTLCs, so we go ahead and "free" any holding
Expand Down Expand Up @@ -2793,7 +2793,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
};

self.channel_state |= ChannelState::LocalShutdownSent as u32;
self.channel_update_count += 1;
self.update_time_counter += 1;

Ok((our_shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs))
}
Expand Down Expand Up @@ -2860,7 +2860,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
if last_fee == msg.fee_satoshis {
self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);
self.channel_state = ChannelState::ShutdownComplete as u32;
self.channel_update_count += 1;
self.update_time_counter += 1;
return Ok((None, Some(closing_tx)));
}
}
Expand Down Expand Up @@ -2910,7 +2910,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &our_sig);

self.channel_state = ChannelState::ShutdownComplete as u32;
self.channel_update_count += 1;
self.update_time_counter += 1;

Ok((Some(msgs::ClosingSigned {
channel_id: self.channel_id,
Expand Down Expand Up @@ -3022,8 +3022,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

/// Allowed in any state (including after shutdown)
pub fn get_channel_update_count(&self) -> u32 {
self.channel_update_count
pub fn get_update_time_counter(&self) -> u32 {
self.update_time_counter
}

pub fn get_latest_monitor_update_id(&self) -> u64 {
Expand Down Expand Up @@ -3149,7 +3149,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
}
self.channel_state = ChannelState::ShutdownComplete as u32;
self.channel_update_count += 1;
self.update_time_counter += 1;
return Err(msgs::ErrorMessage {
channel_id: self.channel_id(),
data: "funding tx had wrong script/value".to_owned()
Expand All @@ -3175,6 +3175,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}
if header.bitcoin_hash() != self.last_block_connected {
self.last_block_connected = header.bitcoin_hash();
self.update_time_counter = cmp::max(self.update_time_counter, header.time);
if let Some(channel_monitor) = self.channel_monitor.as_mut() {
channel_monitor.last_block_hash = self.last_block_connected;
}
Expand All @@ -3185,7 +3186,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
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;
self.update_time_counter += 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
Expand Down Expand Up @@ -3728,7 +3729,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
} else {
self.channel_state |= ChannelState::LocalShutdownSent as u32;
}
self.channel_update_count += 1;
self.update_time_counter += 1;

// Go ahead and drop holding cell updates as we'd rather fail payments than wait to send
// our shutdown until we've committed all of the pending changes.
Expand Down Expand Up @@ -3777,7 +3778,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

self.channel_state = ChannelState::ShutdownComplete as u32;
self.channel_update_count += 1;
self.update_time_counter += 1;
if self.channel_monitor.is_some() {
(self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
} else {
Expand Down Expand Up @@ -3964,7 +3965,7 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {

self.next_local_htlc_id.write(writer)?;
(self.next_remote_htlc_id - dropped_inbound_htlcs).write(writer)?;
self.channel_update_count.write(writer)?;
self.update_time_counter.write(writer)?;
self.feerate_per_kw.write(writer)?;

match self.last_sent_closing_fee {
Expand Down Expand Up @@ -4124,7 +4125,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C

let next_local_htlc_id = Readable::read(reader)?;
let next_remote_htlc_id = Readable::read(reader)?;
let channel_update_count = Readable::read(reader)?;
let update_time_counter = Readable::read(reader)?;
let feerate_per_kw = Readable::read(reader)?;

let last_sent_closing_fee = match <u8 as Readable>::read(reader)? {
Expand Down Expand Up @@ -4203,7 +4204,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<C
holding_cell_update_fee,
next_local_htlc_id,
next_remote_htlc_id,
channel_update_count,
update_time_counter,
feerate_per_kw,

#[cfg(debug_assertions)]
Expand Down
14 changes: 13 additions & 1 deletion lightning/src/ln/channelmanager.rs
View file
Expand Up @@ -1124,7 +1124,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
let unsigned = msgs::UnsignedChannelUpdate {
chain_hash: self.genesis_hash,
short_channel_id: short_channel_id,
timestamp: chan.get_channel_update_count(),
timestamp: chan.get_update_time_counter(),
flags: (!were_node_one) as u16 | ((!chan.is_live() as u16) << 1),
cltv_expiry_delta: CLTV_EXPIRY_DELTA,
htlc_minimum_msat: chan.get_our_htlc_minimum_msat(),
Expand Down Expand Up @@ -2776,6 +2776,18 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
}
self.latest_block_height.store(height as usize, Ordering::Release);
*self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header_hash;
loop {
// Update last_node_announcement_serial to be the max of its current value and the
// block timestamp. This should keep us close to the current time without relying on
// having an explicit local time source.
// Just in case we end up in a race, we loop until we either successfully update
// last_node_announcement_serial or decide we don't need to.
let old_serial = self.last_node_announcement_serial.load(Ordering::Acquire);
if old_serial >= header.time as usize { break; }
if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() {
break;
}
}
}

/// We force-close the channel without letting our counterparty participate in the shutdown
Expand Down

0 comments on commit 78c48f7

Please sign in to comment.