Update ChannelMonitor::best_block before calling block_confirmed

No matter the context, if we're told about a block which is
guaranteed by our API semantics to be on the best chain, and it has
a higher height than our current understanding of the best chain,
we should update our understanding. This avoids complexity
in `block_confirmed` by never having a height set which is *higher*
than our current best chain, potentially avoiding some bugs in the
rather-complicated code.

It also requires a minor test tweak as we in some cases now no
longer broadcast a conflicting transaction after the original has
reached the ANTI_REORG_DELAY.
This commit is contained in:
Matt Corallo 2021-06-28 16:23:19 +00:00
parent 697033342e
commit 599c74cd42
2 changed files with 13 additions and 3 deletions

View file

@ -2004,6 +2004,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
self.is_paying_spendable_output(&tx, height, &logger);
}
if height > self.best_block.height() {
self.best_block = BestBlock::new(block_hash, height);
}
self.block_confirmed(height, txn_matched, watch_outputs, claimable_outpoints, broadcaster, fee_estimator, logger)
}

View file

@ -3001,10 +3001,16 @@ fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) {
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
{
// B will rebroadcast its own holder commitment transaction here...just because
// B may rebroadcast its own holder commitment transaction here, as a safeguard against
// some incredibly unlikely partial-eclipse-attack scenarios. That said, because the
// original commitment_tx[0] (also spending chan_2.3) has reached ANTI_REORG_DELAY B really
// shouldn't broadcast anything here, and in some connect style scenarios we do not.
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
assert_eq!(node_txn.len(), 1);
check_spends!(node_txn[0], chan_2.3);
if node_txn.len() == 1 {
check_spends!(node_txn[0], chan_2.3);
} else {
assert_eq!(node_txn.len(), 0);
}
}
expect_pending_htlcs_forwardable!(nodes[1]);