From 21fbae6df8d10e846d0afaa3c79beeef9d9ae5ce Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 18 Mar 2018 15:11:32 +1030 Subject: [PATCH] openingd: ensure that initial channel can cover fees and reserve. This is probably covered by our "channel capacity" heuristic which requires the channel be significant, but best to be explicit and sure. Signed-off-by: Rusty Russell --- common/initial_channel.c | 1 + common/initial_channel.h | 3 +- common/initial_commit_tx.c | 49 +++++++++++++++++++++++++++++---- common/initial_commit_tx.h | 5 ++-- lightningd/test/run-commit_tx.c | 7 +++++ openingd/opening.c | 13 +++++++++ 6 files changed, 70 insertions(+), 8 deletions(-) diff --git a/common/initial_channel.c b/common/initial_channel.c index 4ac04dad6..dc8bd7407 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -98,6 +98,7 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, dust_limit_satoshis(channel, side), channel->view[side].owed_msat[side], channel->view[side].owed_msat[!side], + channel_reserve_msat(channel, side), 0 ^ channel->commitment_number_obscurer, side); } diff --git a/common/initial_channel.h b/common/initial_channel.h index 3354b01c2..ed7cab126 100644 --- a/common/initial_channel.h +++ b/common/initial_channel.h @@ -164,7 +164,8 @@ struct channel *new_initial_channel(const tal_t *ctx, * @per_commitment_point: Per-commitment point to determine keys * @side: which side to get the commitment transaction for * - * Returns the unsigned initial commitment transaction for @side. + * Returns the unsigned initial commitment transaction for @side, or NULL + * if the channel size was insufficient to cover fees or reserves. */ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, const u8 **wscript, diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index b9f562f7d..64e679f35 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -4,7 +4,9 @@ #include #include #include +#include #include +#include /* BOLT #3: * @@ -29,7 +31,7 @@ u64 commit_number_obscurer(const struct pubkey *opener_payment_basepoint, return be64_to_cpu(obscurer); } -void try_subtract_fee(enum side funder, enum side side, +bool try_subtract_fee(enum side funder, enum side side, u64 base_fee_msat, u64 *self_msat, u64 *other_msat) { u64 *funder_msat; @@ -39,10 +41,13 @@ void try_subtract_fee(enum side funder, enum side side, else funder_msat = other_msat; - if (*funder_msat >= base_fee_msat) + if (*funder_msat >= base_fee_msat) { *funder_msat -= base_fee_msat; - else + return true; + } else { *funder_msat = 0; + return false; + } } u8 *to_self_wscript(const tal_t *ctx, @@ -65,6 +70,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, u64 dust_limit_satoshis, u64 self_pay_msat, u64 other_pay_msat, + u64 self_reserve_msat, u64 obscured_commitment_number, enum side side) { @@ -93,8 +99,41 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, * 3. Subtract this base fee from the funder (either `to_local` or * `to_remote`), with a floor of zero (see [Fee Payment](#fee-payment)). */ - try_subtract_fee(funder, side, base_fee_msat, - &self_pay_msat, &other_pay_msat); + if (!try_subtract_fee(funder, side, base_fee_msat, + &self_pay_msat, &other_pay_msat)) { + /* BOLT #2: + * + * The receiving node MUST fail the channel if: + *... + * - the funder's amount for the initial commitment + * transaction is not sufficient for full [fee + * payment](03-transactions.md#fee-payment). + */ + status_unusual("Funder cannot afford fee" + " on initial commitment transaction"); + return NULL; + } + + /* BOLT #2: + * + * The receiving node MUST fail the channel if: + *... + * - both `to_local` and `to_remote` amounts for the initial + * commitment transaction are less than or equal to + * `channel_reserve_satoshis`. + */ + if (self_pay_msat <= self_reserve_msat + && other_pay_msat <= self_reserve_msat) { + status_unusual("Neither self amount %"PRIu64 + " nor other amount %"PRIu64 + " exceed reserve %"PRIu64 + " on initial commitment transaction", + self_pay_msat / 1000, + other_pay_msat / 1000, + self_reserve_msat / 1000); + return NULL; + } + /* Worst-case sizing: both to-local and to-remote outputs. */ tx = bitcoin_tx(ctx, 1, untrimmed + 2); diff --git a/common/initial_commit_tx.h b/common/initial_commit_tx.h index 858ede797..621ffc61b 100644 --- a/common/initial_commit_tx.h +++ b/common/initial_commit_tx.h @@ -76,11 +76,12 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, u64 dust_limit_satoshis, u64 self_pay_msat, u64 other_pay_msat, + u64 self_reserve_msat, u64 obscured_commitment_number, enum side side); -/* try_subtract_fee - take away this fee from the funder, or all if insufficient. */ -void try_subtract_fee(enum side funder, enum side side, +/* try_subtract_fee - take away this fee from the funder (and return true), or all if insufficient (and return false). */ +bool try_subtract_fee(enum side funder, enum side side, u64 base_fee_msat, u64 *self_msat, u64 *other_msat); /* Generate the witness script for the to-self output: diff --git a/lightningd/test/run-commit_tx.c b/lightningd/test/run-commit_tx.c index ef72e83f3..6612fb29b 100644 --- a/lightningd/test/run-commit_tx.c +++ b/lightningd/test/run-commit_tx.c @@ -19,6 +19,13 @@ static bool print_superverbose; /* Turn this on to brute-force fee values */ /*#define DEBUG */ +/* AUTOGENERATED MOCKS START */ +/* Generated stub for status_fmt */ +void status_fmt(enum log_level level UNNEEDED, const char *fmt UNNEEDED, ...) + +{ fprintf(stderr, "status_fmt called!\n"); abort(); } +/* AUTOGENERATED MOCKS END */ + /* bitcoind loves its backwards txids! */ static struct bitcoin_txid txid_from_hex(const char *hex) { diff --git a/openingd/opening.c b/openingd/opening.c index af11aea3b..bba6e693c 100644 --- a/openingd/opening.c +++ b/openingd/opening.c @@ -422,6 +422,9 @@ static u8 *funder_channel(struct state *state, */ tx = initial_channel_tx(state, &wscript, state->channel, &state->next_per_commit[REMOTE], REMOTE); + if (!tx) + negotiation_failed(state, + "Could not meet their fees and reserve"); sign_tx_input(tx, 0, NULL, wscript, &state->our_secrets.funding_privkey, @@ -480,6 +483,9 @@ static u8 *funder_channel(struct state *state, */ tx = initial_channel_tx(state, &wscript, state->channel, &state->next_per_commit[LOCAL], LOCAL); + if (!tx) + negotiation_failed(state, + "Could not meet our fees and reserve"); if (!check_tx_sig(tx, 0, NULL, wscript, &their_funding_pubkey, &sig)) { peer_failed(&state->cs, state->gossip_index, @@ -706,6 +712,9 @@ static u8 *fundee_channel(struct state *state, */ their_commit = initial_channel_tx(state, &wscript, state->channel, &state->next_per_commit[LOCAL], LOCAL); + if (!their_commit) + negotiation_failed(state, + "Could not meet our fees and reserve"); if (!check_tx_sig(their_commit, 0, NULL, wscript, &their_funding_pubkey, &theirsig)) { @@ -740,6 +749,10 @@ static u8 *fundee_channel(struct state *state, */ our_commit = initial_channel_tx(state, &wscript, state->channel, &state->next_per_commit[REMOTE], REMOTE); + if (!our_commit) + negotiation_failed(state, + "Could not meet their fees and reserve"); + sign_tx_input(our_commit, 0, NULL, wscript, &state->our_secrets.funding_privkey, our_funding_pubkey, &sig);