Skip to content

Funding_tx: add anti-fee sniping recommendation and check if final #1531

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 2 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2783,6 +2783,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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`].
///
Expand All @@ -2798,6 +2801,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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 <https://bitcoinops.org/en/topics/fee-sniping/>
/// 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> {
Expand All @@ -2810,6 +2818,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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();
Expand Down
44 changes: 44 additions & 0 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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());
}