Skip to content

Commit 4d5cf4b

Browse files
TheBlueMattTheBlueMatt
committed
Move ChannelMonitorUpdateErr to chain as it is a chain::Watch val
1 parent 3a24ffb commit 4d5cf4b

File tree

9 files changed

+97
-96
lines changed

9 files changed

+97
-96
lines changed

‎fuzz/src/chanmon_consistency.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ use bitcoin::hashes::sha256::Hash as Sha256;
3030
use bitcoin::hash_types::{BlockHash, WPubkeyHash};
3131

3232
use lightning::chain;
33-
use lightning::chain::{BestBlock, chainmonitor, channelmonitor, Confirm, Watch};
34-
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, MonitorEvent};
33+
use lightning::chain::{BestBlock, ChannelMonitorUpdateErr, chainmonitor, channelmonitor, Confirm, Watch};
34+
use lightning::chain::channelmonitor::{ChannelMonitor, MonitorEvent};
3535
use lightning::chain::transaction::OutPoint;
3636
use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
3737
use lightning::chain::keysinterface::{KeysInterface, InMemorySigner};
@@ -100,7 +100,7 @@ struct TestChainMonitor {
100100
pub logger: Arc<dyn Logger>,
101101
pub keys: Arc<KeyProvider>,
102102
pub chain_monitor: Arc<chainmonitor::ChainMonitor<EnforcingSigner, Arc<dyn chain::Filter>, Arc<TestBroadcaster>, Arc<FuzzEstimator>, Arc<dyn Logger>, Arc<TestPersister>>>,
103-
pub update_ret: Mutex<Result<(), channelmonitor::ChannelMonitorUpdateErr>>,
103+
pub update_ret: Mutex<Result<(), chain::ChannelMonitorUpdateErr>>,
104104
// If we reload a node with an old copy of ChannelMonitors, the ChannelManager deserialization
105105
// logic will automatically force-close our channels for us (as we don't have an up-to-date
106106
// monitor implying we are not able to punish misbehaving counterparties). Because this test
@@ -122,7 +122,7 @@ impl TestChainMonitor {
122122
}
123123
}
124124
impl chain::Watch<EnforcingSigner> for TestChainMonitor {
125-
fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
125+
fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
126126
let mut ser = VecWriter(Vec::new());
127127
monitor.write(&mut ser).unwrap();
128128
if let Some(_) = self.latest_monitors.lock().unwrap().insert(funding_txo, (monitor.get_latest_update_id(), ser.0)) {
@@ -133,7 +133,7 @@ impl chain::Watch<EnforcingSigner> for TestChainMonitor {
133133
self.update_ret.lock().unwrap().clone()
134134
}
135135

136-
fn update_channel(&self, funding_txo: OutPoint, update: channelmonitor::ChannelMonitorUpdate) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
136+
fn update_channel(&self, funding_txo: OutPoint, update: channelmonitor::ChannelMonitorUpdate) -> Result<(), chain::ChannelMonitorUpdateErr> {
137137
let mut map_lock = self.latest_monitors.lock().unwrap();
138138
let mut map_entry = match map_lock.entry(funding_txo) {
139139
hash_map::Entry::Occupied(entry) => entry,

‎fuzz/src/utils/test_persister.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1-
use lightning::chain::{chainmonitor, channelmonitor};
1+
use lightning::chain;
2+
use lightning::chain::chainmonitor;
23
use lightning::chain::transaction::OutPoint;
34
use lightning::util::enforcing_trait_impls::EnforcingSigner;
45

56
pub struct TestPersister {}
67
impl chainmonitor::Persist<EnforcingSigner> for TestPersister {
7-
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
8+
fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
89
Ok(())
910
}
1011

11-
fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: &channelmonitor::ChannelMonitorUpdate, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> {
12+
fn update_persisted_channel(&self, _funding_txo: OutPoint, _update: &channelmonitor::ChannelMonitorUpdate, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
13+
Ok(())
14+
}
15+
16+
fn sync_persisted_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<EnforcingSigner>) -> Result<(), ()> {
1217
Ok(())
1318
}
1419
}

‎lightning-persister/src/lib.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use bitcoin::hashes::hex::{FromHex, ToHex};
1717
use crate::util::DiskWriteable;
1818
use lightning::chain;
1919
use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
20-
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr};
20+
use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate};
2121
use lightning::chain::chainmonitor;
2222
use lightning::chain::keysinterface::{Sign, KeysInterface};
2323
use lightning::chain::transaction::OutPoint;
@@ -159,16 +159,16 @@ impl FilesystemPersister {
159159
}
160160

161161
impl<ChannelSigner: Sign> chainmonitor::Persist<ChannelSigner> for FilesystemPersister {
162-
fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr> {
162+
fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
163163
let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
164164
util::write_to_file(self.path_to_monitor_data(), filename, monitor)
165-
.map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
165+
.map_err(|_| chain::ChannelMonitorUpdateErr::PermanentFailure)
166166
}
167167

168-
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: &ChannelMonitorUpdate, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), ChannelMonitorUpdateErr> {
168+
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: &ChannelMonitorUpdate, monitor: &ChannelMonitor<ChannelSigner>) -> Result<(), chain::ChannelMonitorUpdateErr> {
169169
let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
170170
util::write_to_file(self.path_to_monitor_data(), filename, monitor)
171-
.map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
171+
.map_err(|_| chain::ChannelMonitorUpdateErr::PermanentFailure)
172172
}
173173
}
174174

@@ -180,8 +180,8 @@ mod tests {
180180
use bitcoin::blockdata::block::{Block, BlockHeader};
181181
use bitcoin::hashes::hex::FromHex;
182182
use bitcoin::Txid;
183+
use lightning::chain::ChannelMonitorUpdateErr;
183184
use lightning::chain::chainmonitor::Persist;
184-
use lightning::chain::channelmonitor::ChannelMonitorUpdateErr;
185185
use lightning::chain::transaction::OutPoint;
186186
use lightning::{check_closed_broadcast, check_closed_event, check_added_monitors};
187187
use lightning::ln::features::InitFeatures;

‎lightning/src/chain/chainmonitor.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ use bitcoin::blockdata::block::{Block, BlockHeader};
2727
use bitcoin::hash_types::Txid;
2828

2929
use chain;
30-
use chain::{Filter, WatchedOutput};
30+
use chain::{ChannelMonitorUpdateErr, Filter, WatchedOutput};
3131
use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
32-
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, Balance, MonitorEvent, TransactionOutputs};
32+
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Balance, MonitorEvent, TransactionOutputs};
3333
use chain::transaction::{OutPoint, TransactionData};
3434
use chain::keysinterface::Sign;
3535
use util::logger::Logger;

‎lightning/src/chain/channelmonitor.rs

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -115,67 +115,6 @@ impl Readable for ChannelMonitorUpdate {
115115
}
116116
}
117117

118-
/// An error enum representing a failure to persist a channel monitor update.
119-
#[derive(Clone, Copy, Debug, PartialEq)]
120-
pub enum ChannelMonitorUpdateErr {
121-
/// Used to indicate a temporary failure (eg connection to a watchtower or remote backup of
122-
/// our state failed, but is expected to succeed at some point in the future).
123-
///
124-
/// Such a failure will "freeze" a channel, preventing us from revoking old states or
125-
/// submitting new commitment transactions to the counterparty. Once the update(s) which failed
126-
/// have been successfully applied, ChannelManager::channel_monitor_updated can be used to
127-
/// restore the channel to an operational state.
128-
///
129-
/// Note that a given ChannelManager will *never* re-generate a given ChannelMonitorUpdate. If
130-
/// you return a TemporaryFailure you must ensure that it is written to disk safely before
131-
/// writing out the latest ChannelManager state.
132-
///
133-
/// Even when a channel has been "frozen" updates to the ChannelMonitor can continue to occur
134-
/// (eg if an inbound HTLC which we forwarded was claimed upstream resulting in us attempting
135-
/// to claim it on this channel) and those updates must be applied wherever they can be. At
136-
/// least one such updated ChannelMonitor must be persisted otherwise PermanentFailure should
137-
/// be returned to get things on-chain ASAP using only the in-memory copy. Obviously updates to
138-
/// the channel which would invalidate previous ChannelMonitors are not made when a channel has
139-
/// been "frozen".
140-
///
141-
/// Note that even if updates made after TemporaryFailure succeed you must still call
142-
/// channel_monitor_updated to ensure you have the latest monitor and re-enable normal channel
143-
/// operation.
144-
///
145-
/// Note that the update being processed here will not be replayed for you when you call
146-
/// ChannelManager::channel_monitor_updated, so you must store the update itself along
147-
/// with the persisted ChannelMonitor on your own local disk prior to returning a
148-
/// TemporaryFailure. You may, of course, employ a journaling approach, storing only the
149-
/// ChannelMonitorUpdate on disk without updating the monitor itself, replaying the journal at
150-
/// reload-time.
151-
///
152-
/// For deployments where a copy of ChannelMonitors and other local state are backed up in a
153-
/// remote location (with local copies persisted immediately), it is anticipated that all
154-
/// updates will return TemporaryFailure until the remote copies could be updated.
155-
TemporaryFailure,
156-
/// Used to indicate no further channel monitor updates will be allowed (eg we've moved on to a
157-
/// different watchtower and cannot update with all watchtowers that were previously informed
158-
/// of this channel).
159-
///
160-
/// At reception of this error, ChannelManager will force-close the channel and return at
161-
/// least a final ChannelMonitorUpdate::ChannelForceClosed which must be delivered to at
162-
/// least one ChannelMonitor copy. Revocation secret MUST NOT be released and offchain channel
163-
/// update must be rejected.
164-
///
165-
/// This failure may also signal a failure to update the local persisted copy of one of
166-
/// the channel monitor instance.
167-
///
168-
/// Note that even when you fail a holder commitment transaction update, you must store the
169-
/// update to ensure you can claim from it in case of a duplicate copy of this ChannelMonitor
170-
/// broadcasts it (e.g distributed channel-monitor deployment)
171-
///
172-
/// In case of distributed watchtowers deployment, the new version must be written to disk, as
173-
/// state may have been stored but rejected due to a block forcing a commitment broadcast. This
174-
/// storage is used to claim outputs of rejected state confirmed onchain by another watchtower,
175-
/// lagging behind on block processing.
176-
PermanentFailure,
177-
}
178-
179118
/// General Err type for ChannelMonitor actions. Generally, this implies that the data provided is
180119
/// inconsistent with the ChannelMonitor being called. eg for ChannelMonitor::update_monitor this
181120
/// means you tried to update a monitor for a different channel or the ChannelMonitorUpdate was

‎lightning/src/chain/mod.rs

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use bitcoin::blockdata::transaction::{Transaction, TxOut};
1616
use bitcoin::hash_types::{BlockHash, Txid};
1717
use bitcoin::network::constants::Network;
1818

19-
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, MonitorEvent};
19+
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, MonitorEvent};
2020
use chain::keysinterface::Sign;
2121
use chain::transaction::{OutPoint, TransactionData};
2222

@@ -175,6 +175,67 @@ pub trait Confirm {
175175
fn get_relevant_txids(&self) -> Vec<Txid>;
176176
}
177177

178+
/// An error enum representing a failure to persist a channel monitor update.
179+
#[derive(Clone, Copy, Debug, PartialEq)]
180+
pub enum ChannelMonitorUpdateErr {
181+
/// Used to indicate a temporary failure (eg connection to a watchtower or remote backup of
182+
/// our state failed, but is expected to succeed at some point in the future).
183+
///
184+
/// Such a failure will "freeze" a channel, preventing us from revoking old states or
185+
/// submitting new commitment transactions to the counterparty. Once the update(s) which failed
186+
/// have been successfully applied, ChannelManager::channel_monitor_updated can be used to
187+
/// restore the channel to an operational state.
188+
///
189+
/// Note that a given ChannelManager will *never* re-generate a given ChannelMonitorUpdate. If
190+
/// you return a TemporaryFailure you must ensure that it is written to disk safely before
191+
/// writing out the latest ChannelManager state.
192+
///
193+
/// Even when a channel has been "frozen" updates to the ChannelMonitor can continue to occur
194+
/// (eg if an inbound HTLC which we forwarded was claimed upstream resulting in us attempting
195+
/// to claim it on this channel) and those updates must be applied wherever they can be. At
196+
/// least one such updated ChannelMonitor must be persisted otherwise PermanentFailure should
197+
/// be returned to get things on-chain ASAP using only the in-memory copy. Obviously updates to
198+
/// the channel which would invalidate previous ChannelMonitors are not made when a channel has
199+
/// been "frozen".
200+
///
201+
/// Note that even if updates made after TemporaryFailure succeed you must still call
202+
/// channel_monitor_updated to ensure you have the latest monitor and re-enable normal channel
203+
/// operation.
204+
///
205+
/// Note that the update being processed here will not be replayed for you when you call
206+
/// ChannelManager::channel_monitor_updated, so you must store the update itself along
207+
/// with the persisted ChannelMonitor on your own local disk prior to returning a
208+
/// TemporaryFailure. You may, of course, employ a journaling approach, storing only the
209+
/// ChannelMonitorUpdate on disk without updating the monitor itself, replaying the journal at
210+
/// reload-time.
211+
///
212+
/// For deployments where a copy of ChannelMonitors and other local state are backed up in a
213+
/// remote location (with local copies persisted immediately), it is anticipated that all
214+
/// updates will return TemporaryFailure until the remote copies could be updated.
215+
TemporaryFailure,
216+
/// Used to indicate no further channel monitor updates will be allowed (eg we've moved on to a
217+
/// different watchtower and cannot update with all watchtowers that were previously informed
218+
/// of this channel).
219+
///
220+
/// At reception of this error, ChannelManager will force-close the channel and return at
221+
/// least a final ChannelMonitorUpdate::ChannelForceClosed which must be delivered to at
222+
/// least one ChannelMonitor copy. Revocation secret MUST NOT be released and offchain channel
223+
/// update must be rejected.
224+
///
225+
/// This failure may also signal a failure to update the local persisted copy of one of
226+
/// the channel monitor instance.
227+
///
228+
/// Note that even when you fail a holder commitment transaction update, you must store the
229+
/// update to ensure you can claim from it in case of a duplicate copy of this ChannelMonitor
230+
/// broadcasts it (e.g distributed channel-monitor deployment)
231+
///
232+
/// In case of distributed watchtowers deployment, the new version must be written to disk, as
233+
/// state may have been stored but rejected due to a block forcing a commitment broadcast. This
234+
/// storage is used to claim outputs of rejected state confirmed onchain by another watchtower,
235+
/// lagging behind on block processing.
236+
PermanentFailure,
237+
}
238+
178239
/// The `Watch` trait defines behavior for watching on-chain activity pertaining to channels as
179240
/// blocks are connected and disconnected.
180241
///
@@ -193,9 +254,7 @@ pub trait Confirm {
193254
/// funds in the channel. See [`ChannelMonitorUpdateErr`] for more details about how to handle
194255
/// multiple instances.
195256
///
196-
/// [`ChannelMonitor`]: channelmonitor::ChannelMonitor
197-
/// [`ChannelMonitorUpdateErr`]: channelmonitor::ChannelMonitorUpdateErr
198-
/// [`PermanentFailure`]: channelmonitor::ChannelMonitorUpdateErr::PermanentFailure
257+
/// [`PermanentFailure`]: ChannelMonitorUpdateErr::PermanentFailure
199258
pub trait Watch<ChannelSigner: Sign> {
200259
/// Watches a channel identified by `funding_txo` using `monitor`.
201260
///
@@ -217,7 +276,6 @@ pub trait Watch<ChannelSigner: Sign> {
217276
/// [`ChannelMonitorUpdateErr`] for invariants around returning an error.
218277
///
219278
/// [`update_monitor`]: channelmonitor::ChannelMonitor::update_monitor
220-
/// [`ChannelMonitorUpdateErr`]: channelmonitor::ChannelMonitorUpdateErr
221279
fn update_channel(&self, funding_txo: OutPoint, update: ChannelMonitorUpdate) -> Result<(), ChannelMonitorUpdateErr>;
222280

223281
/// Returns any monitor events since the last call. Subsequent calls must only return new
@@ -242,7 +300,7 @@ pub trait Watch<ChannelSigner: Sign> {
242300
/// processed later. Then, in order to block until the data has been processed, any [`Watch`]
243301
/// invocation that has called the `Filter` must return [`TemporaryFailure`].
244302
///
245-
/// [`TemporaryFailure`]: channelmonitor::ChannelMonitorUpdateErr::TemporaryFailure
303+
/// [`TemporaryFailure`]: ChannelMonitorUpdateErr::TemporaryFailure
246304
/// [BIP 157]: https://github.com/bitcoin/bips/blob/master/bip-0157.mediawiki
247305
/// [BIP 158]: https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki
248306
pub trait Filter {

‎lightning/src/ln/chanmon_update_fail_tests.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,9 @@ use bitcoin::blockdata::block::{Block, BlockHeader};
1616
use bitcoin::blockdata::constants::genesis_block;
1717
use bitcoin::hash_types::BlockHash;
1818
use bitcoin::network::constants::Network;
19-
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr};
19+
use chain::channelmonitor::ChannelMonitor;
2020
use chain::transaction::OutPoint;
21-
use chain::Listen;
22-
use chain::Watch;
21+
use chain::{ChannelMonitorUpdateErr, Listen, Watch};
2322
use ln::{PaymentPreimage, PaymentHash};
2423
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
2524
use ln::features::{InitFeatures, InvoiceFeatures};

‎lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
3636
use bitcoin::secp256k1;
3737

3838
use chain;
39-
use chain::{Confirm, Watch, BestBlock};
39+
use chain::{Confirm, ChannelMonitorUpdateErr, Watch, BestBlock};
4040
use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
41-
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
41+
use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
4242
use chain::transaction::{OutPoint, TransactionData};
4343
// Since this struct is returned in `list_channels` methods, expose it here in case users want to
4444
// construct one themselves.

0 commit comments

Comments
 (0)