From: Matt Corallo Date: Fri, 5 Mar 2021 16:02:42 +0000 (-0500) Subject: Add assertions for in-order block [dis]connection in ChannelManager X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=refs%2Fheads%2F2021-03-823-extra-test;p=rust-lightning 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 2226bcba6..6308dd626 100644 --- 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 ded254d06..8c3ef3861 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3272,6 +3272,15 @@ impl 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 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 298e45c1c..c38da7a6f 100644 --- 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>, pub network_chan_count: Rc>, pub logger: &'c test_utils::TestLogger, + pub last_blockhash: Mutex, } 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