Skip to content

Expose new BumpChannelClose event for channels with anchor outputs #1689

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.

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
24 changes: 24 additions & 0 deletions lightning/src/chain/chainmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
L::Target: Logger,
P::Target: Persist<ChannelSigner>,
{
#[cfg(not(anchors))]
/// Processes [`SpendableOutputs`] events produced from each [`ChannelMonitor`] upon maturity.
///
/// An [`EventHandler`] may safely call back to the provider, though this shouldn't be needed in
Expand All @@ -722,6 +723,29 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
handler.handle_event(&event);
}
}
#[cfg(anchors)]
/// Processes [`SpendableOutputs`] events produced from each [`ChannelMonitor`] upon maturity.
///
/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
/// within each channel. As the confirmation of a commitment transaction may be critical to the
/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
/// update (block connected or disconnected).
///
/// An [`EventHandler`] may safely call back to the provider, though this shouldn't be needed in
/// order to handle these events.
///
/// [`SpendableOutputs`]: events::Event::SpendableOutputs
/// [`BumpTransaction`]: events::Event::BumpTransaction
fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler {
let mut pending_events = Vec::new();
for monitor_state in self.monitors.read().unwrap().values() {
pending_events.append(&mut monitor_state.monitor.get_and_clear_pending_events());
}
for event in pending_events.drain(..) {
handler.handle_event(&event);
}
}
}

#[cfg(test)]
Expand Down
99 changes: 84 additions & 15 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
//! ChannelMonitors to get out of the HSM and onto monitoring devices.

use bitcoin::blockdata::block::BlockHeader;
use bitcoin::blockdata::transaction::{TxOut,Transaction};
use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
use bitcoin::blockdata::transaction::{OutPoint as BitcoinOutPoint, TxOut, Transaction};
use bitcoin::blockdata::script::{Script, Builder};
use bitcoin::blockdata::opcodes;

Expand All @@ -44,13 +43,17 @@ use chain::{BestBlock, WatchedOutput};
use chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
use chain::transaction::{OutPoint, TransactionData};
use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface};
#[cfg(anchors)]
use chain::onchaintx::ClaimEvent;
use chain::onchaintx::OnchainTxHandler;
use chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
use chain::Filter;
use util::logger::Logger;
use util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, U48, OptionDeserWrapper};
use util::byte_utils;
use util::events::Event;
#[cfg(anchors)]
use util::events::{AnchorDescriptor, BumpTransactionEvent};

use prelude::*;
use core::{cmp, mem};
Expand Down Expand Up @@ -263,6 +266,20 @@ impl_writeable_tlv_based!(HolderSignedTx, {
(14, htlc_outputs, vec_type)
});

#[cfg(anchors)]
impl HolderSignedTx {
fn non_dust_htlcs(&self) -> Vec<HTLCOutputInCommitment> {
self.htlc_outputs.iter().filter_map(|(htlc, _, _)| {
if let Some(_) = htlc.transaction_output_index {
Some(htlc.clone())
} else {
None
}
})
.collect()
}
}

/// We use this to track static counterparty commitment transaction data and to generate any
/// justice or 2nd-stage preimage/timeout transactions.
#[derive(PartialEq, Eq)]
Expand Down Expand Up @@ -1221,7 +1238,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
B::Target: BroadcasterInterface,
L::Target: Logger,
{
self.inner.lock().unwrap().broadcast_latest_holder_commitment_txn(broadcaster, logger)
self.inner.lock().unwrap().broadcast_latest_holder_commitment_txn(broadcaster, logger);
}

/// Updates a ChannelMonitor on the basis of some new information provided by the Channel
Expand Down Expand Up @@ -2222,6 +2239,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!");
}
let mut ret = Ok(());
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&*fee_estimator);
for update in updates.updates.iter() {
match update {
ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
Expand All @@ -2239,7 +2257,6 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
},
ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => {
log_trace!(logger, "Updating ChannelMonitor with payment preimage");
let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&*fee_estimator);
self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, &bounded_fee_estimator, logger)
},
ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => {
Expand All @@ -2255,6 +2272,25 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
self.lockdown_from_offchain = true;
if *should_broadcast {
self.broadcast_latest_holder_commitment_txn(broadcaster, logger);
// If the channel supports anchor outputs, we'll need to emit an external
// event to be consumed such that a child transaction is broadcast with a
// high enough feerate for the parent commitment transaction to confirm.
if self.onchain_tx_handler.opt_anchors() {
let funding_output = HolderFundingOutput::build(
self.funding_redeemscript.clone(), self.channel_value_satoshis,
self.onchain_tx_handler.opt_anchors(),
);
let best_block_height = self.best_block.height();
let commitment_package = PackageTemplate::build_package(
self.funding_info.0.txid.clone(), self.funding_info.0.index as u32,
PackageSolvingData::HolderFundingOutput(funding_output),
best_block_height, false, best_block_height,
Copy link

Choose a reason for hiding this comment

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

For builders/helpers in safety-critical parts, we could start to document the boolean argument at the caller call-site, i.e "/* aggregable */ false". It sounds a code style sophistication, though we already do that on the Core-side. As the codebase keeps growing I think this could be a good practice as a argument misusage could introduce a logical bug. E.g, aggregating a PackageMalleability::Untractable and a PackageMalleability::Malleable together triggering a panic!(). Already seen that type of bug sticking for years in Core.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will address this as a separate PR.

);
self.onchain_tx_handler.update_claims_view(
&[], vec![commitment_package], best_block_height, best_block_height,
broadcaster, &bounded_fee_estimator, logger,
);
}
} else if !self.holder_tx_signed {
log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast");
log_error!(logger, " in channel monitor for channel {}!", log_bytes!(self.funding_info.0.to_channel_id()));
Expand Down Expand Up @@ -2309,6 +2345,34 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
pub fn get_and_clear_pending_events(&mut self) -> Vec<Event> {
Copy link

Choose a reason for hiding this comment

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

I think we should update ChainMonitor::process_pending_events in consequence, noticing the user that this call is from now on important for the funds safety and call frequence should at least match every 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.

Done. I wonder if the ChainMonitor should just do this itself if any anchor channels are present. Would it be considered a violation of the events API?

Copy link

Choose a reason for hiding this comment

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

Yeah we can do in a follow-up.

let mut ret = Vec::new();
mem::swap(&mut ret, &mut self.pending_events);
#[cfg(anchors)]
for claim_event in self.onchain_tx_handler.get_and_clear_pending_claim_events().drain(..) {
match claim_event {
ClaimEvent::BumpCommitment {
package_target_feerate_sat_per_1000_weight, commitment_tx, anchor_output_idx,
} => {
let commitment_txid = commitment_tx.txid();
debug_assert_eq!(self.current_holder_commitment_tx.txid, commitment_txid);
let pending_htlcs = self.current_holder_commitment_tx.non_dust_htlcs();
let commitment_tx_fee_satoshis = self.channel_value_satoshis -
commitment_tx.output.iter().fold(0u64, |sum, output| sum + output.value);
ret.push(Event::BumpTransaction(BumpTransactionEvent::ChannelClose {
package_target_feerate_sat_per_1000_weight,
commitment_tx,
commitment_tx_fee_satoshis,
anchor_descriptor: AnchorDescriptor {
channel_keys_id: self.channel_keys_id,
channel_value_satoshis: self.channel_value_satoshis,
outpoint: BitcoinOutPoint {
txid: commitment_txid,
vout: anchor_output_idx,
},
},
pending_htlcs,
}));
},
}
}
ret
}

Expand Down Expand Up @@ -2521,13 +2585,13 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
CounterpartyOfferedHTLCOutput::build(*per_commitment_point,
self.counterparty_commitment_params.counterparty_delayed_payment_base_key,
self.counterparty_commitment_params.counterparty_htlc_base_key,
preimage.unwrap(), htlc.clone()))
preimage.unwrap(), htlc.clone(), self.onchain_tx_handler.opt_anchors()))
} else {
PackageSolvingData::CounterpartyReceivedHTLCOutput(
CounterpartyReceivedHTLCOutput::build(*per_commitment_point,
self.counterparty_commitment_params.counterparty_delayed_payment_base_key,
self.counterparty_commitment_params.counterparty_htlc_base_key,
htlc.clone()))
htlc.clone(), self.onchain_tx_handler.opt_anchors()))
};
let aggregation = if !htlc.offered { false } else { true };
let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, counterparty_htlc_outp, htlc.cltv_expiry,aggregation, 0);
Expand Down Expand Up @@ -2884,21 +2948,26 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {

let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
if should_broadcast {
let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone());
let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone(), self.channel_value_satoshis, self.onchain_tx_handler.opt_anchors());
let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), false, self.best_block.height());
claimable_outpoints.push(commitment_package);
self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
self.holder_tx_signed = true;
// Because we're broadcasting a commitment transaction, we should construct the package
// assuming it gets confirmed in the next block. Sadly, we have code which considers
// "not yet confirmed" things as discardable, so we cannot do that here.
let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
if !new_outputs.is_empty() {
watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
// We can't broadcast our HTLC transactions while the commitment transaction is
// unconfirmed. We'll delay doing so until we detect the confirmed commitment in
// `transactions_confirmed`.
if !self.onchain_tx_handler.opt_anchors() {
// Because we're broadcasting a commitment transaction, we should construct the package
// assuming it gets confirmed in the next block. Sadly, we have code which considers
// "not yet confirmed" things as discardable, so we cannot do that here.
let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
if !new_outputs.is_empty() {
watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
}
claimable_outpoints.append(&mut new_outpoints);
}
claimable_outpoints.append(&mut new_outpoints);
}

// Find which on-chain events have reached their confirmation threshold.
Expand Down
18 changes: 18 additions & 0 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use util::crypto::{hkdf_extract_expand_twice, sign};
use util::ser::{Writeable, Writer, Readable, ReadableArgs};

use chain::transaction::OutPoint;
use ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI;
use ln::{chan_utils, PaymentPreimage};
use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction, ClosingTransaction};
use ln::msgs::UnsignedChannelAnnouncement;
Expand Down Expand Up @@ -348,6 +349,12 @@ pub trait BaseSign {
/// chosen to forgo their output as dust.
fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;

/// Computes the signature for a commitment transaction's anchor output used as an
/// input within `anchor_tx`, which spends the commitment transaction, at index `input`.
fn sign_holder_anchor_input(
&self, anchor_tx: &mut Transaction, input: usize, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<Signature, ()>;

/// Signs a channel announcement message with our funding key and our node secret key (aka
/// node_id or network_key), proving it comes from one of the channel participants.
///
Expand Down Expand Up @@ -645,6 +652,7 @@ impl InMemorySigner {
witness.push(witness_script.clone().into_bytes());
Ok(witness)
}

}

impl BaseSign for InMemorySigner {
Expand Down Expand Up @@ -762,6 +770,16 @@ impl BaseSign for InMemorySigner {
Ok(closing_tx.trust().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
}

fn sign_holder_anchor_input(
&self, anchor_tx: &mut Transaction, input: usize, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<Signature, ()> {
let witness_script = chan_utils::get_anchor_redeemscript(&self.holder_channel_pubkeys.funding_pubkey);
let sighash = sighash::SighashCache::new(&*anchor_tx).segwit_signature_hash(
input, &witness_script, ANCHOR_OUTPUT_VALUE_SATOSHI, EcdsaSighashType::All,
).unwrap();
Ok(sign(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key))
}

fn sign_channel_announcement(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>)
-> Result<(Signature, Signature), ()> {
let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
Expand Down
Loading