Skip to content

Handle if funding output is in a coinbase transaction #1924

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
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
33 changes: 27 additions & 6 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,9 @@ pub(crate) const DISCONNECT_PEER_AWAITING_RESPONSE_TICKS: usize = 2;
/// exceeding this age limit will be force-closed and purged from memory.
pub(crate) const UNFUNDED_CHANNEL_AGE_LIMIT_TICKS: usize = 60;

/// Number of blocks needed for an output from a coinbase transaction to be spendable.
pub(crate) const COINBASE_MATURITY: u32 = 100;

struct PendingChannelMonitorUpdate {
update: ChannelMonitorUpdate,
}
Expand Down Expand Up @@ -4734,12 +4737,14 @@ impl<SP: Deref> Channel<SP> where
return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
} else {
if self.context.is_outbound() {
for input in tx.input.iter() {
if input.witness.is_empty() {
// We generated a malleable funding transaction, implying we've
// just exposed ourselves to funds loss to our counterparty.
#[cfg(not(fuzzing))]
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
if !tx.is_coin_base() {
for input in tx.input.iter() {
if input.witness.is_empty() {
// We generated a malleable funding transaction, implying we've
// just exposed ourselves to funds loss to our counterparty.
#[cfg(not(fuzzing))]
panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
}
}
}
}
Expand All @@ -4750,6 +4755,13 @@ impl<SP: Deref> Channel<SP> where
Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"),
}
}
// If this is a coinbase transaction and not a 0-conf channel
// we should update our min_depth to 100 to handle coinbase maturity
if tx.is_coin_base() &&
self.context.minimum_depth.unwrap_or(0) > 0 &&
self.context.minimum_depth.unwrap_or(0) < COINBASE_MATURITY {
self.context.minimum_depth = Some(COINBASE_MATURITY);
}
}
// If we allow 1-conf funding, we may need to check for channel_ready here and
// send it immediately instead of waiting for a best_block_updated call (which
Expand Down Expand Up @@ -5821,6 +5833,15 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {

self.context.channel_state = ChannelState::FundingCreated as u32;
self.context.channel_id = funding_txo.to_channel_id();

// If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100.
// We can skip this if it is a zero-conf channel.
if funding_transaction.is_coin_base() &&
self.context.minimum_depth.unwrap_or(0) > 0 &&
self.context.minimum_depth.unwrap_or(0) < COINBASE_MATURITY {
self.context.minimum_depth = Some(COINBASE_MATURITY);
}

self.context.funding_transaction = Some(funding_transaction);

let channel = Channel {
Expand Down
12 changes: 7 additions & 5 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3561,11 +3561,13 @@ where
pub fn funding_transaction_generated(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, funding_transaction: Transaction) -> Result<(), APIError> {
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);

for inp in funding_transaction.input.iter() {
if inp.witness.is_empty() {
return Err(APIError::APIMisuseError {
err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
});
if !funding_transaction.is_coin_base() {
for inp in funding_transaction.input.iter() {
if inp.witness.is_empty() {
return Err(APIError::APIMisuseError {
err: "Funding transaction must be fully signed and spend Segwit outputs".to_owned()
});
}
}
}
{
Expand Down
32 changes: 29 additions & 3 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use alloc::rc::Rc;
use crate::sync::{Arc, Mutex, LockTestExt, RwLock};
use core::mem;
use core::iter::repeat;
use bitcoin::{PackedLockTime, TxMerkleNode};
use bitcoin::{PackedLockTime, TxIn, TxMerkleNode};

pub const CHAN_CONFIRM_DEPTH: u32 = 10;

Expand Down Expand Up @@ -1005,7 +1005,23 @@ macro_rules! reload_node {
};
}

pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128) -> (ChannelId, Transaction, OutPoint) {
pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>,
expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128)
-> (ChannelId, Transaction, OutPoint)
{
internal_create_funding_transaction(node, expected_counterparty_node_id, expected_chan_value, expected_user_chan_id, false)
}

pub fn create_coinbase_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>,
expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128)
-> (ChannelId, Transaction, OutPoint)
{
internal_create_funding_transaction(node, expected_counterparty_node_id, expected_chan_value, expected_user_chan_id, true)
}

fn internal_create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>,
expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128,
coinbase: bool) -> (ChannelId, Transaction, OutPoint) {
let chan_id = *node.network_chan_count.borrow();

let events = node.node.get_and_clear_pending_events();
Expand All @@ -1016,7 +1032,16 @@ pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_
assert_eq!(*channel_value_satoshis, expected_chan_value);
assert_eq!(user_channel_id, expected_user_chan_id);

let tx = Transaction { version: chan_id as i32, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: vec![TxOut {
let input = if coinbase {
vec![TxIn {
previous_output: bitcoin::OutPoint::null(),
..Default::default()
}]
} else {
Vec::new()
};

let tx = Transaction { version: chan_id as i32, lock_time: PackedLockTime::ZERO, input, output: vec![TxOut {
value: *channel_value_satoshis, script_pubkey: output_script.clone(),
}]};
let funding_outpoint = OutPoint { txid: tx.txid(), index: 0 };
Expand All @@ -1025,6 +1050,7 @@ pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_
_ => panic!("Unexpected event"),
}
}

pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, expected_temporary_channel_id: ChannelId) -> Transaction {
let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, &node_b.node.get_our_node_id(), channel_value, 42);
assert_eq!(temporary_channel_id, expected_temporary_channel_id);
Expand Down
62 changes: 61 additions & 1 deletion lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::chain::transaction::OutPoint;
use crate::sign::{ChannelSigner, EcdsaChannelSigner, EntropySource, SignerProvider};
use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason};
use crate::ln::{ChannelId, PaymentPreimage, PaymentSecret, PaymentHash};
use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel};
use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY};
use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA};
use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError};
use crate::ln::{chan_utils, onion_utils};
Expand Down Expand Up @@ -9133,6 +9133,66 @@ fn test_invalid_funding_tx() {
mine_transaction(&nodes[1], &spend_tx);
}

#[test]
fn test_coinbase_funding_tx() {
// Miners are able to fund channels directly from coinbase transactions, however
// by consensus rules, outputs of a coinbase transaction are encumbered by a 100
// block maturity timelock. To ensure that a (non-0conf) channel like this is enforceable
// on-chain, the minimum depth is updated to 100 blocks for coinbase funding transactions.
//
// Note that 0conf channels with coinbase funding transactions are unaffected and are
// immediately operational after opening.
Comment on lines +9143 to +9144
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth adding the coinbase case to the 0conf test? FWIW it doesn't look like too much work since we already have functional_test_utils::open_zero_conf_channel

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we thought it would be good to add them, so will be doing that definitely in this PR. Maybe some closes on those channels before maturity too.

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);

nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, None).unwrap();
let open_channel = 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(), &open_channel);
let accept_channel = 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(), &accept_channel);

// Create the coinbase funding transaction.
let (temporary_channel_id, tx, _) = create_coinbase_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);

nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
check_added_monitors!(nodes[0], 0);
let funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());

nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created);
check_added_monitors!(nodes[1], 1);
expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());

let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());

nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed);
check_added_monitors!(nodes[0], 1);

expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
assert!(nodes[0].node.get_and_clear_pending_events().is_empty());

// Starting at height 0, we "confirm" the coinbase at height 1.
confirm_transaction_at(&nodes[0], &tx, 1);
// We connect 98 more blocks to have 99 confirmations for the coinbase transaction.
connect_blocks(&nodes[0], COINBASE_MATURITY - 2);
// Check that we have no pending message events (we have not queued a `channel_ready` yet).
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
// Now connect one more block which results in 100 confirmations of the coinbase transaction.
connect_blocks(&nodes[0], 1);
// There should now be a `channel_ready` which can be handled.
let _ = &nodes[1].node.handle_channel_ready(&nodes[0].node.get_our_node_id(), &get_event_msg!(&nodes[0], MessageSendEvent::SendChannelReady, nodes[1].node.get_our_node_id()));

confirm_transaction_at(&nodes[1], &tx, 1);
connect_blocks(&nodes[1], COINBASE_MATURITY - 2);
assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
connect_blocks(&nodes[1], 1);
expect_channel_ready_event(&nodes[1], &nodes[0].node.get_our_node_id());
create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]);
}

fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_timelock: bool) {
// In the first version of the chain::Confirm interface, after a refactor was made to not
// broadcast CSV-locked transactions until their CSV lock is up, we wouldn't reliably broadcast
Expand Down