Add assertions for in-order block [dis]connection in ChannelManager

Sadly the connected-in-order tests have to be skipped in our normal
test suite as many tests violate it. Luckily we can still enforce
it in the tests which run in other crates.

Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>

diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index 3e205c07183d402fe045110d562f94b084224155..7c5875ac1ef62d31590241228027fd75b954aba2 100644 (file)
--- a/fuzz/src/chanmon_consistency.rs
+++ b/fuzz/src/chanmon_consistency.rs
@@ -432,7 +432,8 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 
        macro_rules! confirm_txn {
                ($node: expr) => { {
-                       let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+                       let chain_hash = genesis_block(Network::Bitcoin).block_hash();
+                       let mut header = BlockHeader { version: 0x20000000, prev_blockhash: chain_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
                        let txdata: Vec<_> = channel_txn.iter().enumerate().map(|(i, tx)| (i + 1, tx)).collect();
                        $node.block_connected(&header, &txdata, 1);
                        for i in 2..100 {

diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index 132ff95f6bbb6ccafe1f18a893f05b625ad4120c..da2b015d2a7e26c13c321677cb4689d19c023151 100644 (file)
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -161,7 +161,7 @@ struct MoneyLossDetector<'a> {
        peers: &'a RefCell<[bool; 256]>,
        funding_txn: Vec<Transaction>,
        txids_confirmed: HashMap<Txid, usize>,
-       header_hashes: Vec<BlockHash>,
+       header_hashes: Vec<(BlockHash, u32)>,
        height: usize,
        max_height: usize,
        blocks_connected: u32,
@@ -179,7 +179,7 @@ impl<'a> MoneyLossDetector<'a> {
                        peers,
                        funding_txn: Vec::new(),
                        txids_confirmed: HashMap::new(),
-                       header_hashes: vec![Default::default()],
+                       header_hashes: vec![(genesis_block(Network::Bitcoin).block_hash(), 0)],
                        height: 0,
                        max_height: 0,
                        blocks_connected: 0,
@@ -199,23 +199,23 @@ impl<'a> MoneyLossDetector<'a> {
                        }
                }
 
-               let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height], merkle_root: Default::default(), time: self.blocks_connected, bits: 42, nonce: 42 };
-               self.height += 1;
                self.blocks_connected += 1;
+               let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height].0, merkle_root: Default::default(), time: self.blocks_connected, bits: 42, nonce: 42 };
+               self.height += 1;
                self.manager.block_connected(&header, &txdata, self.height as u32);
                (*self.monitor).block_connected(&header, &txdata, self.height as u32);
                if self.header_hashes.len() > self.height {
-                       self.header_hashes[self.height] = header.block_hash();
+                       self.header_hashes[self.height] = (header.block_hash(), self.blocks_connected);
                } else {
                        assert_eq!(self.header_hashes.len(), self.height);
-                       self.header_hashes.push(header.block_hash());
+                       self.header_hashes.push((header.block_hash(), self.blocks_connected));
                }
                self.max_height = cmp::max(self.height, self.max_height);
        }
 
        fn disconnect_block(&mut self) {
                if self.height > 0 && (self.max_height < 6 || self.height >= self.max_height - 6) {
-                       let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height], merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+                       let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height - 1].0, merkle_root: Default::default(), time: self.header_hashes[self.height].1, bits: 42, nonce: 42 };
                        self.manager.block_disconnected(&header);
                        self.monitor.block_disconnected(&header, self.height as u32);
                        self.height -= 1;

diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs
index 2226bcba6095373f7ae3dc2f7b3db2f7cf5e034c..e42893462eed7965b0b11081f8b1547af6570cef 100644 (file)
--- a/lightning-persister/src/lib.rs
+++ b/lightning-persister/src/lib.rs
@@ -241,8 +241,8 @@ mod tests {
                let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 1);
 
-               let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-               connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[0].clone()]}, 1);
+               let header = BlockHeader { version: 0x20000000, prev_blockhash: *nodes[0].last_block_hash.lock().unwrap(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+               connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[0].clone()]}, CHAN_CONFIRM_DEPTH);
                check_closed_broadcast!(nodes[1], false);
                check_added_monitors!(nodes[1], 1);
 

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 6e46d79fb08a76624e57bb2b25bac77834941be3..147d4aa4a7a28e23ddd76c032287154af77dea6f 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3260,6 +3260,16 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
 
+
+               // This assertion should be enforced in tests, however we have a number of tests that
+               // were written before this requirement and do not meet it.
+               #[cfg(not(test))]
+               {
+                       assert_eq!(*self.last_block_hash.read().unwrap(), header.prev_blockhash,
+                               "Blocks must be connected in chain-order - the connected header must build on the last connected header");
+                       assert_eq!(self.latest_block_height.load(Ordering::Acquire) as u64, height as u64 - 1,
+                               "Blocks must be connected in chain-order - the connected header must build on the last connected header");
+               }
                self.latest_block_height.store(height as usize, Ordering::Release);
                *self.last_block_hash.write().unwrap() = block_hash;
 
@@ -3376,6 +3386,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                // See the docs for `ChannelManagerReadArgs` for more.
                let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
 
+               assert_eq!(*self.last_block_hash.read().unwrap(), header.block_hash(),
+                       "Blocks must be disconnected in chain-order - the disconnected header must be the last connected header");
                self.latest_block_height.fetch_sub(1, Ordering::AcqRel);
                *self.last_block_hash.write().unwrap() = header.prev_blockhash;
 

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index e7204a045abd8a673bc49f0f782b8c06900c2b3f..dccbea333ed6a41a48fc673ec7173f8ea0054a99 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -50,7 +50,7 @@ pub fn confirm_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Tran
        let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
        let dummy_tx_count = tx.version as usize;
        let mut block = Block {
-               header: BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
+               header: BlockHeader { version: 0x20000000, prev_blockhash: genesis_block(Network::Testnet).header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 },
                txdata: vec![dummy_tx; dummy_tx_count],
        };
        block.txdata.push(tx.clone());
@@ -85,12 +85,14 @@ pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block,
        node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height);
        node.node.block_connected(&block.header, &txdata, height);
        node.node.test_process_background_events();
+       *node.last_block_hash.lock().unwrap() = block.header.block_hash();
 }
 
 pub fn disconnect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, header: &BlockHeader, height: u32) {
        node.chain_monitor.chain_monitor.block_disconnected(header, height);
        node.node.block_disconnected(header);
        node.node.test_process_background_events();
+       *node.last_block_hash.lock().unwrap() = header.prev_blockhash;
 }
 
 pub struct TestChanMonCfg {
@@ -123,6 +125,7 @@ pub struct Node<'a, 'b: 'a, 'c: 'b> {
        pub network_payment_count: Rc<RefCell<u8>>,
        pub network_chan_count: Rc<RefCell<u32>>,
        pub logger: &'c test_utils::TestLogger,
+       pub last_block_hash: Mutex<BlockHash>,
 }
 
 impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
@@ -1189,6 +1192,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
                                 keys_manager: &cfgs[i].keys_manager, node: &chan_mgrs[i], net_graph_msg_handler,
                                 node_seed: cfgs[i].node_seed, network_chan_count: chan_count.clone(),
                                 network_payment_count: payment_count.clone(), logger: cfgs[i].logger,
+                                last_block_hash: Mutex::new(genesis_block(Network::Testnet).header.block_hash()),
                })
        }
 

diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index b19a355d8f1acd6eca7115c79334f4788ee7fed6..7a317f65b9f32b5436be86e182b4ecfc759a454d 100644 (file)
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -21,7 +21,9 @@ use util::test_utils;
 use util::ser::{ReadableArgs, Writeable};
 
 use bitcoin::blockdata::block::{Block, BlockHeader};
+use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::hash_types::BlockHash;
+use bitcoin::network::constants::Network;
 
 use std::collections::HashMap;
 use std::mem;
@@ -208,7 +210,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool) {
        mem::drop(channel_state);
 
        let mut headers = Vec::new();
-       let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       let mut header = BlockHeader { version: 0x20000000, prev_blockhash: genesis_block(Network::Testnet).header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
        headers.push(header.clone());
        for _i in 2..100 {
                header = BlockHeader { version: 0x20000000, prev_blockhash: header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
@@ -319,7 +321,7 @@ fn test_set_outpoints_partial_claiming() {
        check_spends!(remote_txn[2], remote_txn[0]);
 
        // Connect blocks on node A to advance height towards TEST_FINAL_CLTV
-       let prev_header_100 = connect_blocks(&nodes[1], 100, 0, false, Default::default());
+       let block_hash_100 = connect_blocks(&nodes[1], 100, 0, false, genesis_block(Network::Testnet).header.block_hash());
        // Provide node A with both preimage
        nodes[0].node.claim_funds(payment_preimage_1, &None, 3_000_000);
        nodes[0].node.claim_funds(payment_preimage_2, &None, 3_000_000);
@@ -328,8 +330,8 @@ fn test_set_outpoints_partial_claiming() {
        nodes[0].node.get_and_clear_pending_msg_events();
 
        // Connect blocks on node A commitment transaction
-       let header = BlockHeader { version: 0x20000000, prev_blockhash: prev_header_100, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       connect_block(&nodes[0], &Block { header, txdata: vec![remote_txn[0].clone()] }, 101);
+       let header_101 = BlockHeader { version: 0x20000000, prev_blockhash: block_hash_100, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       connect_block(&nodes[0], &Block { header: header_101, txdata: vec![remote_txn[0].clone()] }, 101);
        check_closed_broadcast!(nodes[0], false);
        check_added_monitors!(nodes[0], 1);
        // Verify node A broadcast tx claiming both HTLCs
@@ -361,8 +363,8 @@ fn test_set_outpoints_partial_claiming() {
        };
 
        // Broadcast partial claim on node A, should regenerate a claiming tx with HTLC dropped
-       let header = BlockHeader { version: 0x20000000, prev_blockhash: header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       connect_block(&nodes[0], &Block { header, txdata: vec![partial_claim_tx.clone()] }, 102);
+       let header_102 = BlockHeader { version: 0x20000000, prev_blockhash: header_101.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       connect_block(&nodes[0], &Block { header: header_102, txdata: vec![partial_claim_tx.clone()] }, 102);
        {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 1);
@@ -373,7 +375,7 @@ fn test_set_outpoints_partial_claiming() {
        nodes[0].node.get_and_clear_pending_msg_events();
 
        // Disconnect last block on node A, should regenerate a claiming tx with HTLC dropped
-       disconnect_block(&nodes[0], &header, 102);
+       disconnect_block(&nodes[0], &header_102, 102);
        {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 1);
@@ -383,8 +385,8 @@ fn test_set_outpoints_partial_claiming() {
        }
 
        //// Disconnect one more block and then reconnect multiple no transaction should be generated
-       disconnect_block(&nodes[0], &header, 101);
-       connect_blocks(&nodes[1], 15, 101, false, prev_header_100);
+       disconnect_block(&nodes[0], &header_101, 101);
+       connect_blocks(&nodes[1], 15, 101, false, block_hash_100);
        {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 0);