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.

diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs
index 2226bcba6095373f7ae3dc2f7b3db2f7cf5e034c..6308dd626af6d1d2e487e078173da772ec1cb60e 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_blockhash.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 ded254d06f900c839cac2bf7a6411fc72946a69e..8c3ef38619eccf1422fecb43d35a64ff851a5519 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3272,6 +3272,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                !htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
                        });
 
+                       // 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.lock().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.last_block_hash.lock().unwrap() = block_hash;
                        self.latest_block_height.store(height as usize, Ordering::Release);
                }
@@ -3327,6 +3336,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                }
                        });
 
+                       assert_eq!(*self.last_block_hash.lock().unwrap(), header.block_hash(),
+                               "Blocks must be disconnected in chain-order - the disconnected header must be the last connected header");
                        *self.last_block_hash.lock().unwrap() = header.prev_blockhash;
                        self.latest_block_height.fetch_sub(1, Ordering::AcqRel);
                }

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 298e45c1c8878202a2ae3cc573e95b7aeef5acf3..c38da7a6fd5b54be4a76bbdc726f486258ff0061 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());
@@ -84,11 +84,13 @@ pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block,
        let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
        node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height);
        node.node.block_connected(&block.header, &txdata, height);
+       *node.last_blockhash.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.last_blockhash.lock().unwrap() = header.prev_blockhash;
 }
 
 pub struct TestChanMonCfg {
@@ -121,6 +123,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_blockhash: Mutex<BlockHash>,
 }
 
 impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
@@ -1187,6 +1190,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_blockhash: Mutex::new(genesis_block(Network::Testnet).header.block_hash()),
                })
        }
 

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index f185f4fada5f1614dd39d3a7fa673a7666c5ce6b..9572e2d2c4855ec1df2b5c7eb05af8691bf29ebe 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -3548,7 +3548,7 @@ fn test_unconf_chan() {
        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 };
@@ -8125,8 +8125,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 new_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: new_header, 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);
@@ -8137,7 +8137,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], &new_header, 102);
        {
                let mut node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 1);