Skip to content

Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheBlueMatt merged 2 commits into from
Aug 15, 2022
Merged

Drop return value from Filter::register_output#1663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f4457f
Drop return value from `Filter::register_output`
tnull Aug 11, 2022
c562f33
Clarify 'should' vs 'will' in `get_relevant_txids`
tnull Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 51 additions & 110 deletions lightning/src/chain/chainmonitor.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
where
FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
{
let mut dependent_txdata = Vec::new();
{
let monitor_states = self.monitors.write().unwrap();
if let Some(height) = best_height {
// If the best block height is being updated, update highest_chain_height under the
// monitors write lock.
let old_height = self.highest_chain_height.load(Ordering::Acquire);
let new_height = height as usize;
if new_height > old_height {
self.highest_chain_height.store(new_height, Ordering::Release);
}
let monitor_states = self.monitors.write().unwrap();
if let Some(height) = best_height {
// If the best block height is being updated, update highest_chain_height under the
// monitors write lock.
let old_height = self.highest_chain_height.load(Ordering::Acquire);
let new_height = height as usize;
if new_height > old_height {
self.highest_chain_height.store(new_height, Ordering::Release);
}
}

for (funding_outpoint, monitor_state) in monitor_states.iter() {
let monitor = &monitor_state.monitor;
let mut txn_outputs;
{
txn_outputs = process(monitor, txdata);
let update_id = MonitorUpdateId {
contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
};
let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
if let Some(height) = best_height {
if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
// If there are not ChainSync persists awaiting completion, go ahead and
// set last_chain_persist_height here - we wouldn't want the first
// TemporaryFailure to always immediately be considered "overly delayed".
monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
}
for (funding_outpoint, monitor_state) in monitor_states.iter() {
let monitor = &monitor_state.monitor;
let mut txn_outputs;
{
txn_outputs = process(monitor, txdata);
let update_id = MonitorUpdateId {
contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
};
let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
if let Some(height) = best_height {
if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
// If there are not ChainSync persists awaiting completion, go ahead and
// set last_chain_persist_height here - we wouldn't want the first
// TemporaryFailure to always immediately be considered "overly delayed".
monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
}
}

log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
Ok(()) =>
log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
Err(ChannelMonitorUpdateErr::PermanentFailure) => {
monitor_state.channel_perm_failed.store(true, Ordering::Release);
self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
},
Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
pending_monitor_updates.push(update_id);
},
}
log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
Ok(()) =>
log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
Err(ChannelMonitorUpdateErr::PermanentFailure) => {
monitor_state.channel_perm_failed.store(true, Ordering::Release);
self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
},
Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
pending_monitor_updates.push(update_id);
},
}
}

// Register any new outputs with the chain source for filtering, storing any dependent
// transactions from within the block that previously had not been included in txdata.
if let Some(ref chain_source) = self.chain_source {
let block_hash = header.block_hash();
for (txid, mut outputs) in txn_outputs.drain(..) {
for (idx, output) in outputs.drain(..) {
// Register any new outputs with the chain source for filtering and recurse
// if it indicates that there are dependent transactions within the block
// that had not been previously included in txdata.
let output = WatchedOutput {
block_hash: Some(block_hash),
outpoint: OutPoint { txid, index: idx as u16 },
script_pubkey: output.script_pubkey,
};
if let Some(tx) = chain_source.register_output(output) {
dependent_txdata.push(tx);
}
}
// Register any new outputs with the chain source for filtering, storing any dependent
// transactions from within the block that previously had not been included in txdata.
if let Some(ref chain_source) = self.chain_source {
let block_hash = header.block_hash();
for (txid, mut outputs) in txn_outputs.drain(..) {
for (idx, output) in outputs.drain(..) {
// Register any new outputs with the chain source for filtering
let output = WatchedOutput {
block_hash: Some(block_hash),
outpoint: OutPoint { txid, index: idx as u16 },
script_pubkey: output.script_pubkey,
};
chain_source.register_output(output)
}
}
}
}

// Recursively call for any dependent transactions that were identified by the chain source.
if !dependent_txdata.is_empty() {
dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
dependent_txdata.dedup_by_key(|(index, _tx)| *index);
let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
}
}

/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
use ln::msgs::ChannelMessageHandler;
use util::errors::APIError;
use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
use util::test_utils::{OnRegisterOutput, TxOutReference};

/// Tests that in-block dependent transactions are processed by `block_connected` when not
/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
/// commitment transaction itself. An Electrum client may filter the commitment transaction but
/// needs to return the HTLC transaction so it can be processed.
#[test]
fn connect_block_checks_dependent_transactions() {
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
let channel = create_announced_chan_between_nodes(
&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

// Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
let (commitment_tx, htlc_tx) = {
let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

assert_eq!(txn.len(), 2);
(txn.remove(0), txn.remove(0))
};

// Set expectations on nodes[1]'s chain source to return dependent transactions.
let htlc_output = TxOutReference(commitment_tx.clone(), 0);
let to_local_output = TxOutReference(commitment_tx.clone(), 1);
let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
nodes[1].chain_source
.expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
.expect(OnRegisterOutput { with: to_local_output, returns: None })
.expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

// Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
// source should return the dependent HTLC transaction when the HTLC output is registered.
mine_transaction(&nodes[1], &commitment_tx);

// Clean up so uninteresting assertions don't fail.
check_added_monitors!(nodes[1], 1);
nodes[1].node.get_and_clear_pending_msg_events();
nodes[1].node.get_and_clear_pending_events();
}

#[test]
fn test_async_ooo_offchain_updates() {
Expand Down
29 changes: 13 additions & 16 deletions lightning/src/chain/mod.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
/// in the event of a chain reorganization, it must not be called with a `header` that is no
/// longer in the chain as of the last call to [`best_block_updated`].
///
/// [chain order]: Confirm#Order
/// [chain order]: Confirm#order
/// [`best_block_updated`]: Self::best_block_updated
fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
///
/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
/// reorganized out of the best chain. Once called, the given transaction should not be returned
/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
/// reorganized out of the best chain. Once called, the given transaction will not be returned
/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
///
/// [`get_relevant_txids`]: Self::get_relevant_txids
/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

/// Returns transactions that should be monitored for reorganization out of the chain.
///
/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
/// confirmations to be safe from a chain reorganization. Should not include any transactions
/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
/// confirmations to be safe from a chain reorganization. Will not include any transactions
/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
///
/// May be called to determine the subset of transactions that must still be monitored for
/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

/// Registers interest in spends of a transaction output.
///
/// Optionally, when `output.block_hash` is set, should return any transaction spending the
/// output that is found in the corresponding block along with its index.
///
/// This return value is useful for Electrum clients in order to supply in-block descendant
/// transactions which otherwise were not included. This is not necessary for other clients if
/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
/// full block).
fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.
/// Note that this method might be called during processing of a new block. You therefore need
/// to ensure that also dependent output spents within an already connected block are correctly
/// handled, e.g., by re-scanning the block in question whenever new outputs have been
/// registered mid-processing.
fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.
Expand Down
69 changes: 1 addition & 68 deletions lightning/src/util/test_utils.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
watched_txn: Mutex::new(HashSet::new()),
watched_outputs: Mutex::new(HashSet::new()),
expectations: Mutex::new(None),
}
}

/// Sets an expectation that [`chain::Filter::register_output`] is called.
pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
self.expectations.lock().unwrap()
.get_or_insert_with(|| VecDeque::new())
.push_back(expectation);
self
}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
}

fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
let dependent_tx = match &mut *self.expectations.lock().unwrap() {
None => None,
Some(expectations) => match expectations.pop_front() {
None => {
panic!("Unexpected register_output: {:?}",
(output.outpoint, output.script_pubkey));
},
Some(expectation) => {
assert_eq!(output.outpoint, expectation.outpoint());
assert_eq!(&output.script_pubkey, expectation.script_pubkey());
expectation.returns
},
},
};

fn register_output(&self, output: WatchedOutput) {
self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
dependent_tx
}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
if panicking() {
return;
}

if let Some(expectations) = &*self.expectations.lock().unwrap() {
if !expectations.is_empty() {
panic!("Unsatisfied expectations: {:?}", expectations);
}
}
}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
/// The transaction output to register.
pub with: TxOutReference,

/// A dependent transaction spending the output along with its position in the block.
pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
fn outpoint(&self) -> OutPoint {
let txid = self.with.0.txid();
let index = self.with.1 as u16;
OutPoint { txid, index }
}

fn script_pubkey(&self) -> &Script {
let index = self.with.1;
&self.with.0.output[index].script_pubkey
}
}

impl core::fmt::Debug for OnRegisterOutput {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("OnRegisterOutput")
.field("outpoint", &self.outpoint())
.field("script_pubkey", self.script_pubkey())
.finish()
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Drop return value from `Filter::register_output` by tnull · Pull Request #1663 · lightningdevkit/rust-lightning · GitHub
Skip to content
Drop return value from Filter::register_output#1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 2 commits into

from

Aug 15, 2022
 Merged


Drop return value from Filter::register_output#1663
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
2f4457f

Drop return value from `Filter::register_output`


tnull Aug 11, 2022
c562f33

Clarify 'should' vs 'will' in `get_relevant_txids`


tnull Aug 11, 2022

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


161 changes: 51 additions & 110 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -263,82 +263,67 @@ where C::Target: chain::Filter,
	where
 FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
	{
 let mut dependent_txdata = Vec::new();
 {
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 let monitor_states = self.monitors.write().unwrap();
 if let Some(height) = best_height {
 // If the best block height is being updated, update highest_chain_height under the
 // monitors write lock.
 let old_height = self.highest_chain_height.load(Ordering::Acquire);
 let new_height = height as usize;
 if new_height > old_height {
 self.highest_chain_height.store(new_height, Ordering::Release);
 }
 }

 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 for (funding_outpoint, monitor_state) in monitor_states.iter() {
 let monitor = &monitor_state.monitor;
 let mut txn_outputs;
 {
 txn_outputs = process(monitor, txdata);
 let update_id = MonitorUpdateId {
 contents: UpdateOrigin::ChainSync(self.sync_persistence_id.get_increment()),
 };
 let mut pending_monitor_updates = monitor_state.pending_monitor_updates.lock().unwrap();
 if let Some(height) = best_height {
 if !monitor_state.has_pending_chainsync_updates(&pending_monitor_updates) {
 // If there are not ChainSync persists awaiting completion, go ahead and
 // set last_chain_persist_height here - we wouldn't want the first
 // TemporaryFailure to always immediately be considered "overly delayed".
 monitor_state.last_chain_persist_height.store(height as usize, Ordering::Release);
 }
 }

 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 log_trace!(self.logger, "Syncing Channel Monitor for channel {}", log_funding_info!(monitor));
 match self.persister.update_persisted_channel(*funding_outpoint, &None, monitor, update_id) {
 Ok(()) =>
 log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
 Err(ChannelMonitorUpdateErr::PermanentFailure) => {
 monitor_state.channel_perm_failed.store(true, Ordering::Release);
 self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
 },
 Err(ChannelMonitorUpdateErr::TemporaryFailure) => {
 log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
 pending_monitor_updates.push(update_id);
 },
 }
 }

 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering and recurse
 // if it indicates that there are dependent transactions within the block
 // that had not been previously included in txdata.
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 if let Some(tx) = chain_source.register_output(output) {
 dependent_txdata.push(tx);
 }
 }
 // Register any new outputs with the chain source for filtering, storing any dependent
 // transactions from within the block that previously had not been included in txdata.
 if let Some(ref chain_source) = self.chain_source {
 let block_hash = header.block_hash();
 for (txid, mut outputs) in txn_outputs.drain(..) {
 for (idx, output) in outputs.drain(..) {
 // Register any new outputs with the chain source for filtering
 let output = WatchedOutput {
 block_hash: Some(block_hash),
 outpoint: OutPoint { txid, index: idx as u16 },
 script_pubkey: output.script_pubkey,
 };
 chain_source.register_output(output)
 }
 }
 }
 }

 // Recursively call for any dependent transactions that were identified by the chain source.
 if !dependent_txdata.is_empty() {
 dependent_txdata.sort_unstable_by_key(|(index, _tx)| *index);
 dependent_txdata.dedup_by_key(|(index, _tx)| *index);
 let txdata: Vec<_> = dependent_txdata.iter().map(|(index, tx)| (*index, tx)).collect();
 self.process_chain_data(header, None, &txdata, process); // We skip the best height the second go-around
 }
	}

	/// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
Expand DownExpand Up@@ -745,50 +730,6 @@ mod tests {
	use ln::msgs::ChannelMessageHandler;
	use util::errors::APIError;
	use util::events::{ClosureReason, MessageSendEvent, MessageSendEventsProvider};
	use util::test_utils::{OnRegisterOutput, TxOutReference};

	/// Tests that in-block dependent transactions are processed by `block_connected` when not
	/// included in `txdata` but returned by [`chain::Filter::register_output`]. For instance,
	/// a (non-anchor) commitment transaction's HTLC output may be spent in the same block as the
	/// commitment transaction itself. An Electrum client may filter the commitment transaction but
	/// needs to return the HTLC transaction so it can be processed.
	#[test]
	fn connect_block_checks_dependent_transactions() {
 let chanmon_cfgs = create_chanmon_cfgs(2);
 let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 let channel = create_announced_chan_between_nodes(
 &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Send a payment, saving nodes[0]'s revoked commitment and HTLC-Timeout transactions.
 let (commitment_tx, htlc_tx) = {
 let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
 claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);

 assert_eq!(txn.len(), 2);
 (txn.remove(0), txn.remove(0))
 };

 // Set expectations on nodes[1]'s chain source to return dependent transactions.
 let htlc_output = TxOutReference(commitment_tx.clone(), 0);
 let to_local_output = TxOutReference(commitment_tx.clone(), 1);
 let htlc_timeout_output = TxOutReference(htlc_tx.clone(), 0);
 nodes[1].chain_source
 .expect(OnRegisterOutput { with: htlc_output, returns: Some((1, htlc_tx)) })
 .expect(OnRegisterOutput { with: to_local_output, returns: None })
 .expect(OnRegisterOutput { with: htlc_timeout_output, returns: None });

 // Notify nodes[1] that nodes[0]'s revoked commitment transaction was mined. The chain
 // source should return the dependent HTLC transaction when the HTLC output is registered.
 mine_transaction(&nodes[1], &commitment_tx);

 // Clean up so uninteresting assertions don't fail.
 check_added_monitors!(nodes[1], 1);
 nodes[1].node.get_and_clear_pending_msg_events();
 nodes[1].node.get_and_clear_pending_events();
	}

	#[test]
	fn test_async_ooo_offchain_updates() {
Expand Down

29 changes: 13 additions & 16 deletions
lightning/src/chain/mod.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -12,7 +12,7 @@
use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::script::Script;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::blockdata::transaction::TxOut;
use bitcoin::hash_types::{BlockHash, Txid};
use bitcoin::network::constants::Network;
use bitcoin::secp256k1::PublicKey;
Expand DownExpand Up@@ -151,15 +151,15 @@ pub trait Confirm {
	/// in the event of a chain reorganization, it must not be called with a `header` that is no
	/// longer in the chain as of the last call to [`best_block_updated`].
	///
	/// [chain order]: Confirm#Order
	/// [chain order]: Confirm#order
	/// [`best_block_updated`]: Self::best_block_updated
	fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);

	/// Processes a transaction that is no longer confirmed as result of a chain reorganization.
	///
	/// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
	/// reorganized out of the best chain. Once called, the given transaction should not be returned
	/// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
	/// reorganized out of the best chain. Once called, the given transaction will not be returned
	/// by [`get_relevant_txids`], unless it has been reconfirmed via [`transactions_confirmed`].
	///
	/// [`get_relevant_txids`]: Self::get_relevant_txids
	/// [`transactions_confirmed`]: Self::transactions_confirmed
Expand All@@ -173,9 +173,9 @@ pub trait Confirm {

	/// Returns transactions that should be monitored for reorganization out of the chain.
	///
	/// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Should not include any transactions
	/// passed to [`transaction_unconfirmed`] unless later reconfirmed.
	/// Will include any transactions passed to [`transactions_confirmed`] that have insufficient
	/// confirmations to be safe from a chain reorganization. Will not include any transactions
	/// passed to [`transaction_unconfirmed`], unless later reconfirmed.
	///
	/// May be called to determine the subset of transactions that must still be monitored for
	/// reorganization. Will be idempotent between calls but may change as a result of calls to the
Expand DownExpand Up@@ -333,21 +333,18 @@ pub trait Filter {

	/// Registers interest in spends of a transaction output.
	///
	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
	/// output that is found in the corresponding block along with its index.
	///
	/// This return value is useful for Electrum clients in order to supply in-block descendant
	/// transactions which otherwise were not included. This is not necessary for other clients if
	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
	/// full block).
	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
Comment thread
valentinewallace marked this conversation as resolved.

	/// Note that this method might be called during processing of a new block. You therefore need
	/// to ensure that also dependent output spents within an already connected block are correctly
	/// handled, e.g., by re-scanning the block in question whenever new outputs have been
	/// registered mid-processing.
	fn register_output(&self, output: WatchedOutput);
}

/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
///
/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
/// the return value of [`Filter::register_output`].
/// [`Confirm::transactions_confirmed`].
///
/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
/// may have been spent there. See [`Filter::register_output`] for details.
Comment thread
tnull marked this conversation as resolved.

Expand Down

69 changes: 1 addition & 68 deletions
lightning/src/util/test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -728,7 +728,6 @@ pub struct TestChainSource {
	pub utxo_ret: Mutex<Result<TxOut, chain::AccessError>>,
	pub watched_txn: Mutex<HashSet<(Txid, Script)>>,
	pub watched_outputs: Mutex<HashSet<(OutPoint, Script)>>,
	expectations: Mutex<Option<VecDeque<OnRegisterOutput>>>,
}

impl TestChainSource {
Expand All@@ -739,17 +738,8 @@ impl TestChainSource {
 utxo_ret: Mutex::new(Ok(TxOut { value: u64::max_value(), script_pubkey })),
 watched_txn: Mutex::new(HashSet::new()),
 watched_outputs: Mutex::new(HashSet::new()),
 expectations: Mutex::new(None),
 }
	}

	/// Sets an expectation that [`chain::Filter::register_output`] is called.
	pub fn expect(&self, expectation: OnRegisterOutput) -> &Self {
 self.expectations.lock().unwrap()
 .get_or_insert_with(|| VecDeque::new())
 .push_back(expectation);
 self
	}
}

impl chain::Access for TestChainSource {
Expand All@@ -767,24 +757,8 @@ impl chain::Filter for TestChainSource {
 self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
	}

	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
 let dependent_tx = match &mut *self.expectations.lock().unwrap() {
 None => None,
 Some(expectations) => match expectations.pop_front() {
 None => {
 panic!("Unexpected register_output: {:?}",
 (output.outpoint, output.script_pubkey));
 },
 Some(expectation) => {
 assert_eq!(output.outpoint, expectation.outpoint());
 assert_eq!(&output.script_pubkey, expectation.script_pubkey());
 expectation.returns
 },
 },
 };

	fn register_output(&self, output: WatchedOutput) {
 self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 dependent_tx
	}
}

Expand All@@ -793,47 +767,6 @@ impl Drop for TestChainSource {
 if panicking() {
 return;
 }

 if let Some(expectations) = &*self.expectations.lock().unwrap() {
 if !expectations.is_empty() {
 panic!("Unsatisfied expectations: {:?}", expectations);
 }
 }
	}
}

/// An expectation that [`chain::Filter::register_output`] was called with a transaction output and
/// returns an optional dependent transaction that spends the output in the same block.
pub struct OnRegisterOutput {
	/// The transaction output to register.
	pub with: TxOutReference,

	/// A dependent transaction spending the output along with its position in the block.
	pub returns: Option<(usize, Transaction)>,
}

/// A transaction output as identified by an index into a transaction's output list.
pub struct TxOutReference(pub Transaction, pub usize);

impl OnRegisterOutput {
	fn outpoint(&self) -> OutPoint {
 let txid = self.with.0.txid();
 let index = self.with.1 as u16;
 OutPoint { txid, index }
	}

	fn script_pubkey(&self) -> &Script {
 let index = self.with.1;
 &self.with.0.output[index].script_pubkey
	}
}

impl core::fmt::Debug for OnRegisterOutput {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 f.debug_struct("OnRegisterOutput")
 .field("outpoint", &self.outpoint())
 .field("script_pubkey", self.script_pubkey())
 .finish()
	}
}

Expand Down