mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-24 23:08:36 +01:00
Code Issues Projects Releases Packages Wiki Activity

Fix serialization rt bug in Channel and test in functional_tests

Browse source 
Previously, when attempting to write out a channel with some
RemoteAnnounced pending inbound HTLCs, we'd write out the count
without them, but write out some of their fields. We should drop
them as intended as they will need to be reannounced upon
reconnection.

This was found while attempting to simply reproduce a different
bug by adding tests for ChannelManager serialization rount-trip at
the end of each functional_test (in Node::drop). That test is
included here to prevent some classes of similar bugs in the future.
This commit is contained in:
Matt Corallo 2020-02-17 13:50:46 -05:00
parent 5fceb0ff4f
commit ed2a5fdab9
2 changed files with 35 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
lightning/src/ln/channel.rs
View file

@ -3670,12 +3670,15 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
}
(self.pending_inbound_htlcs.len() as u64 - dropped_inbound_htlcs).write(writer)?;
for htlc in self.pending_inbound_htlcs.iter() {
if let &InboundHTLCState::RemoteAnnounced(_) = &htlc.state {
continue; // Drop
}
htlc.htlc_id.write(writer)?;
htlc.amount_msat.write(writer)?;
htlc.cltv_expiry.write(writer)?;
htlc.payment_hash.write(writer)?;
match &htlc.state {
&InboundHTLCState::RemoteAnnounced(_) => {}, // Drop
&InboundHTLCState::RemoteAnnounced(_) => unreachable!(),
&InboundHTLCState::AwaitingRemoteRevokeToAnnounce(ref htlc_state) => {
1u8.write(writer)?;
htlc_state.write(writer)?;

37
lightning/src/ln/functional_test_utils.rs
View file

@ -4,7 +4,7 @@
use chain::chaininterface;
use chain::transaction::OutPoint;
use chain::keysinterface::KeysInterface;
use ln::channelmanager::{ChannelManager,RAACommitmentOrder, PaymentPreimage, PaymentHash};
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash};
use ln::channelmonitor::{ChannelMonitor, ManyChannelMonitor};
use ln::router::{Route, Router};
use ln::features::InitFeatures;
@ -17,7 +17,7 @@ use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsPro
use util::errors::APIError;
use util::logger::Logger;
use util::config::UserConfig;
use util::ser::ReadableArgs;
use util::ser::{ReadableArgs, Writeable};
use bitcoin::util::hash::BitcoinHash;
use bitcoin::blockdata::block::BlockHeader;
@ -37,7 +37,7 @@ use std::cell::RefCell;
use std::rc::Rc;
use std::sync::{Arc, Mutex};
use std::mem;
use std::collections::HashSet;
use std::collections::{HashSet, HashMap};
pub const CHAN_CONFIRM_DEPTH: u32 = 100;
pub fn confirm_transaction<'a, 'b: 'a>(notifier: &'a chaininterface::BlockNotifierRef<'b>, chain: &chaininterface::ChainWatchInterfaceUtil, tx: &Transaction, chan_id: u32) {
@ -95,20 +95,45 @@ impl<'a, 'b> Drop for Node<'a, 'b> {
// Check that if we serialize and then deserialize all our channel monitors we get the
// same set of outputs to watch for on chain as we have now. Note that if we write
// tests that fully close channels and remove the monitors at some point this may break.
let chain_watch = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&self.logger) as Arc<Logger>));
let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
let channel_monitor = test_utils::TestChannelMonitor::new(chain_watch.clone(), self.tx_broadcaster.clone(), self.logger.clone(), feeest);
let old_monitors = self.chan_monitor.simple_monitor.monitors.lock().unwrap();
let mut deserialized_monitors = Vec::new();
for (_, old_monitor) in old_monitors.iter() {
let mut w = test_utils::TestVecWriter(Vec::new());
old_monitor.write_for_disk(&mut w).unwrap();
let (_, deserialized_monitor) = <(Sha256d, ChannelMonitor<EnforcingChannelKeys>)>::read(
&mut ::std::io::Cursor::new(&w.0), Arc::clone(&self.logger) as Arc<Logger>).unwrap();
deserialized_monitors.push(deserialized_monitor);
}
// Before using all the new monitors to check the watch outpoints, use the full set of
// them to ensure we can write and reload our ChannelManager.
{
let mut channel_monitors = HashMap::new();
for monitor in deserialized_monitors.iter_mut() {
channel_monitors.insert(monitor.get_funding_txo().unwrap(), monitor);
}
let mut w = test_utils::TestVecWriter(Vec::new());
self.node.write(&mut w).unwrap();
<(Sha256d, ChannelManager<EnforcingChannelKeys, &test_utils::TestChannelMonitor>)>::read(&mut ::std::io::Cursor::new(w.0), ChannelManagerReadArgs {
default_config: UserConfig::default(),
keys_manager: self.keys_manager.clone(),
fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }),
monitor: self.chan_monitor,
tx_broadcaster: self.tx_broadcaster.clone(),
logger: Arc::new(test_utils::TestLogger::new()),
channel_monitors: &mut channel_monitors,
}).unwrap();
}
let chain_watch = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&self.logger) as Arc<Logger>));
let channel_monitor = test_utils::TestChannelMonitor::new(chain_watch.clone(), self.tx_broadcaster.clone(), self.logger.clone(), feeest);
for deserialized_monitor in deserialized_monitors.drain(..) {
if let Err(_) = channel_monitor.add_update_monitor(deserialized_monitor.get_funding_txo().unwrap(), deserialized_monitor) {
panic!();
}
}
if *chain_watch != *self.chain_monitor {
panic!();
}