Skip to content

Improve Confirm docs #1900

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
merged 1 commit into from
Dec 12, 2022
Merged

Improve Confirm docs #1900

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 36 additions & 32 deletions lightning/src/chain/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,31 +107,35 @@ pub trait Listen {
fn block_disconnected(&self, header: &BlockHeader, height: u32);
}

/// The `Confirm` trait is used to notify when transactions have been confirmed on chain or
/// unconfirmed during a chain reorganization.
/// The `Confirm` trait is used to notify LDK when relevant transactions have been confirmed on
/// chain or unconfirmed during a chain reorganization.
///
/// Clients sourcing chain data using a transaction-oriented API should prefer this interface over
/// [`Listen`]. For instance, an Electrum client may implement [`Filter`] by subscribing to activity
/// related to registered transactions and outputs. Upon notification, it would pass along the
/// matching transactions using this interface.
/// [`Listen`]. For instance, an Electrum-based transaction sync implementation may implement
/// [`Filter`] to subscribe to relevant transactions and unspent outputs it should monitor for
/// on-chain activity. Then, it needs to notify LDK via this interface upon observing any changes
/// with reference to the confirmation status of the monitored objects.
///
/// # Use
///
/// The intended use is as follows:
/// - Call [`transactions_confirmed`] to process any on-chain activity of interest.
/// - Call [`transaction_unconfirmed`] to process any transaction returned by [`get_relevant_txids`]
/// that has been reorganized out of the chain.
/// - Call [`best_block_updated`] whenever a new chain tip becomes available.
/// - Call [`transactions_confirmed`] to notify LDK whenever any of the registered transactions or
/// outputs are, respectively, confirmed or spent on chain.
/// - Call [`transaction_unconfirmed`] to notify LDK whenever any transaction returned by
/// [`get_relevant_txids`] is no longer confirmed in the block with the given block hash.
/// - Call [`best_block_updated`] to notify LDK whenever a new chain tip becomes available.
///
/// # Order
///
/// Clients must call these methods in chain order. Specifically:
/// - Transactions confirmed in a block must be given before transactions confirmed in a later
/// block.
/// - Transactions which are confirmed in a particular block must be given before transactions
/// confirmed in a later block.
/// - Dependent transactions within the same block must be given in topological order, possibly in
/// separate calls.
/// - Unconfirmed transactions must be given after the original confirmations and before any
/// reconfirmation.
/// - All unconfirmed transactions must be given after the original confirmations and before *any*
/// reconfirmations, i.e., [`transactions_confirmed`] and [`transaction_unconfirmed`] calls should
/// never be interleaved, but always conduced *en bloc*.
/// - Any reconfirmed transactions need to be explicitly unconfirmed before they are reconfirmed
/// in regard to the new block.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// in regard to the new block.
/// in the new block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mh, likewise I added that to somewhat separate a) the event of a transaction being confirmed in a block on chain and b) us getting notice of that and calling transactions_confirmed. The former is literally happening in the new block, while the latter is happening in regard to the former, no?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess? I dunno it just read funny to me, it doesn't really matter.

///
/// See individual method documentation for further details.
///
Expand All @@ -140,9 +144,9 @@ pub trait Listen {
/// [`best_block_updated`]: Self::best_block_updated
/// [`get_relevant_txids`]: Self::get_relevant_txids
pub trait Confirm {
/// Processes transactions confirmed in a block with a given header and height.
/// Notifies LDK of transactions confirmed in a block with a given header and height.
///
/// Should be called for any transactions registered by [`Filter::register_tx`] or any
/// Must be called for any transactions registered by [`Filter::register_tx`] or any
/// transactions spending an output registered by [`Filter::register_output`]. Such transactions
/// appearing in the same block do not need to be included in the same call; instead, multiple
/// calls with additional transactions may be made so long as they are made in [chain order].
Expand All @@ -154,36 +158,36 @@ pub trait Confirm {
/// [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.
/// Notifies LDK of 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 will not be returned
/// Must be called for any transaction returned by [`get_relevant_txids`] if it has been
/// reorganized out of the best chain or if it is no longer confirmed in the block with the
/// given block hash. 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
fn transaction_unconfirmed(&self, txid: &Txid);

/// Processes an update to the best header connected at the given height.
/// Notifies LDK of an update to the best header connected at the given height.
///
/// Should be called when a new header is available but may be skipped for intermediary blocks
/// if they become available at the same time.
/// Must be called whenever a new chain tip becomes available. May be skipped for intermediary
/// blocks.
fn best_block_updated(&self, header: &BlockHeader, height: u32);

/// Returns transactions that should be monitored for reorganization out of the chain along
/// with the hash of the block as part of which had been previously confirmed.
/// Returns transactions that must be monitored for reorganization out of the chain along
/// with the hash of the block as part of which it had been previously confirmed.
///
/// 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
/// Must be called to determine the subset of transactions that must be monitored for
/// reorganization. Will be idempotent between calls but may change as a result of calls to the
/// other interface methods. Thus, this is useful to determine which transactions may need to be
/// given to [`transaction_unconfirmed`]. If any of the returned transactions are confirmed in
/// a block other than the one with the given hash, they need to be unconfirmed and reconfirmed
/// via [`transaction_unconfirmed`] and [`transactions_confirmed`], respectively.
/// other interface methods. Thus, this is useful to determine which transactions must be
/// given to [`transaction_unconfirmed`].
///
/// If any of the returned transactions are confirmed in a block other than the one with the
/// given hash, they need to be unconfirmed and reconfirmed via [`transaction_unconfirmed`] and
/// [`transactions_confirmed`], respectively.
///
/// [`transactions_confirmed`]: Self::transactions_confirmed
/// [`transaction_unconfirmed`]: Self::transaction_unconfirmed
Expand Down