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

Drop the redundant/broken ChannelMonitor::get_monitored_outpoints

Browse source 
In review of the final doc changes in #649, I noticed there
appeared to be redundant monitored-outpoints function in
`ChannelMonitor` - `get_monitored_outpoints()` and
`get_outputs_to_watch()`.

In 6f08779b04,
get_monitored_outpoints() was added, with its behavior largely the
same as today's - only returning the set of remote commitment txn
outputs that we've learned about on-chain. This is clearly not
sufficient, and in 73dce207dd,
`get_outputs_to_watch` was added which was overly cautious to
ensure nothing was missed. Still, the author of 73dce207dd
(me) seemed entirely unaware of the work in 6f08779b04
(also me), despite the function being the literal next function in
the same file. This is presumably because it was assumed that
`get_monitored_outpoints` referred to oupoints for which we should
monitor for spends of (which is true), while `get_outputs_to_watch`
referred to outpouts which we should monitor for the transaction
containing said output (which is not true), or something of that
nature. Specifically, it is the expected behavior that the only
time we care about `Filter::register_tx` is for the funding
transaction (which we aren't aware of the inputs of), but for all
other transactions we register interest on the basis of an outpoint
in the previous transaction (ie via `Filter::register_output`).

Here we drop the broken-on-day-one `get_monitored_outpoints()`
version, but assert in testing that the values which it would return
are all present in `get_outputs_to_watch()`.
This commit is contained in:
Matt Corallo 2020-09-27 17:52:09 -04:00
parent 45a558b170
commit a162840ffd
2 changed files with 10 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
lightning/src/chain/channelmonitor.rs
View file

@ -1214,23 +1214,17 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
/// ///
/// (C-not exported) because we have no HashMap bindings /// (C-not exported) because we have no HashMap bindings
pub fn get_outputs_to_watch(&self) -> &HashMap<Txid, Vec<Script>> { pub fn get_outputs_to_watch(&self) -> &HashMap<Txid, Vec<Script>> {
&self.outputs_to_watch // If we've detected a counterparty commitment tx on chain, we must include it in the set
} // of outputs to watch for spends of, otherwise we're likely to lose user funds. Because
// its trivial to do, double-check that here.
/// Gets the sets of all outpoints which this ChannelMonitor expects to hear about spends of. for (txid, &(_, ref outputs)) in self.counterparty_commitment_txn_on_chain.iter() {
/// Generally useful when deserializing as during normal operation the return values of let watched_outputs = self.outputs_to_watch.get(txid).expect("Counterparty commitment txn which have been broadcast should have outputs registered");
/// block_connected are sufficient to ensure all relevant outpoints are being monitored (note assert_eq!(watched_outputs.len(), outputs.len());
/// that the get_funding_txo outpoint and transaction must also be monitored for!). for (watched, output) in watched_outputs.iter().zip(outputs.iter()) {
/// assert_eq!(watched, output);
/// (C-not exported) as there is no practical way to track lifetimes of returned values.
pub fn get_monitored_outpoints(&self) -> Vec<(Txid, u32, &Script)> {
let mut res = Vec::with_capacity(self.counterparty_commitment_txn_on_chain.len() * 2);
for (ref txid, &(_, ref outputs)) in self.counterparty_commitment_txn_on_chain.iter() {
for (idx, output) in outputs.iter().enumerate() {
res.push(((*txid).clone(), idx as u32, output));
} }
} }
res &self.outputs_to_watch
} }
/// Get the list of HTLCs who's status has been updated on chain. This should be called by /// Get the list of HTLCs who's status has been updated on chain. This should be called by

2
lightning/src/ln/channelmanager.rs
View file

@ -3730,7 +3730,7 @@ impl<ChanSigner: ChannelKeys + Writeable, M: Deref, T: Deref, K: Deref, F: Deref
/// This may result in closing some Channels if the ChannelMonitor is newer than the stored /// This may result in closing some Channels if the ChannelMonitor is newer than the stored
/// ChannelManager state to ensure no loss of funds. Thus, transactions may be broadcasted. /// ChannelManager state to ensure no loss of funds. Thus, transactions may be broadcasted.
/// 3) Register all relevant ChannelMonitor outpoints with your chain watch mechanism using /// 3) Register all relevant ChannelMonitor outpoints with your chain watch mechanism using
/// ChannelMonitor::get_monitored_outpoints and ChannelMonitor::get_funding_txo(). /// ChannelMonitor::get_outputs_to_watch() and ChannelMonitor::get_funding_txo().
/// 4) Reconnect blocks on your ChannelMonitors. /// 4) Reconnect blocks on your ChannelMonitors.
/// 5) Move the ChannelMonitors into your local chain::Watch. /// 5) Move the ChannelMonitors into your local chain::Watch.
/// 6) Disconnect/connect blocks on the ChannelManager. /// 6) Disconnect/connect blocks on the ChannelManager.