From 2b7ef4762f04823ddc4fe9269f95ae222d7b450c Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 9 Jun 2022 17:31:04 -0400 Subject: [PATCH 1/2] Check if funding transaction is final for propagation If the funding transaction is timelocked beyond the next block of our best known chain tip, return an APIError instead of silently failing at broadcast attempt. --- lightning/src/ln/channelmanager.rs | 15 ++++++++++ lightning/src/ln/functional_tests.rs | 44 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3fa136e9af8..b9971d8148c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2783,6 +2783,9 @@ impl ChannelMana /// Returns an [`APIError::APIMisuseError`] if the funding_transaction spent non-SegWit outputs /// or if no output was found which matches the parameters in [`Event::FundingGenerationReady`]. /// + /// Returns [`APIError::APIMisuseError`] if the funding transaction is not final for propagation + /// across the p2p network. + /// /// Returns [`APIError::ChannelUnavailable`] if a funding transaction has already been provided /// for the channel or if the channel has been closed as indicated by [`Event::ChannelClosed`]. /// @@ -2810,6 +2813,18 @@ impl ChannelMana }); } } + { + let height = self.best_block.read().unwrap().height(); + // Transactions are evaluated as final by network mempools at the next block. However, the modules + // constituting our Lightning node might not have perfect sync about their blockchain views. Thus, if + // the wallet module is in advance on the LDK view, allow one more block of headroom. + // TODO: updated if/when https://github.com/rust-bitcoin/rust-bitcoin/pull/994 landed and rust-bitcoin bumped. + if !funding_transaction.input.iter().all(|input| input.sequence == 0xffffffff) && funding_transaction.lock_time < 500_000_000 && funding_transaction.lock_time > height + 2 { + return Err(APIError::APIMisuseError { + err: "Funding transaction absolute timelock is non-final".to_owned() + }); + } + } self.funding_transaction_generated_intern(temporary_channel_id, counterparty_node_id, funding_transaction, |chan, tx| { let mut output_index = None; let expected_spk = chan.get_funding_redeemscript().to_v0_p2wsh(); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 3298db0dbd4..f82b8d08811 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -41,6 +41,8 @@ use bitcoin::blockdata::script::Builder; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::constants::genesis_block; use bitcoin::network::constants::Network; +use bitcoin::{Transaction, TxIn, TxOut, Witness}; +use bitcoin::OutPoint as BitcoinOutPoint; use bitcoin::secp256k1::Secp256k1; use bitcoin::secp256k1::{PublicKey,SecretKey}; @@ -10329,3 +10331,45 @@ fn test_max_dust_htlc_exposure() { do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, false); do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, true); } + +#[test] +fn test_non_final_funding_tx() { + 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 temp_channel_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None).unwrap(); + let open_channel_message = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &open_channel_message); + let accept_channel_message = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_channel_message); + + let best_height = nodes[0].node.best_block.read().unwrap().height(); + + let chan_id = *nodes[0].network_chan_count.borrow(); + let events = nodes[0].node.get_and_clear_pending_events(); + let input = TxIn { previous_output: BitcoinOutPoint::null(), script_sig: bitcoin::Script::new(), sequence: 0x1, witness: Witness::from_vec(vec!(vec!(1))) }; + assert_eq!(events.len(), 1); + let mut tx = match events[0] { + Event::FundingGenerationReady { ref channel_value_satoshis, ref output_script, .. } => { + // Timelock the transaction _beyond_ the best client height + 2. + Transaction { version: chan_id as i32, lock_time: best_height + 3, input: vec![input], output: vec![TxOut { + value: *channel_value_satoshis, script_pubkey: output_script.clone(), + }]} + }, + _ => panic!("Unexpected event"), + }; + // Transaction should fail as it's evaluated as non-final for propagation. + match nodes[0].node.funding_transaction_generated(&temp_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()) { + Err(APIError::APIMisuseError { err }) => { + assert_eq!(format!("Funding transaction absolute timelock is non-final"), err); + }, + _ => panic!() + } + + // However, transaction should be accepted if it's in a +2 headroom from best block. + tx.lock_time -= 1; + assert!(nodes[0].node.funding_transaction_generated(&temp_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).is_ok()); + get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); +} From 69344fab6194c76bb8e9cf8dd52808e5bc8abfcb Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Thu, 9 Jun 2022 17:40:42 -0400 Subject: [PATCH 2/2] Recommend funding_tx to apply anti-fee sniping --- lightning/src/ln/channelmanager.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b9971d8148c..9fd4a24f444 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2801,6 +2801,11 @@ impl ChannelMana /// not currently support replacing a funding transaction on an existing channel. Instead, /// create a new channel with a conflicting funding transaction. /// + /// Note to keep the miner incentives aligned in moving the blockchain forward, we recommend + /// the wallet software generating the funding transaction to apply anti-fee sniping as + /// implemented by Bitcoin Core wallet. See + /// for more details. + /// /// [`Event::FundingGenerationReady`]: crate::util::events::Event::FundingGenerationReady /// [`Event::ChannelClosed`]: crate::util::events::Event::ChannelClosed pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {