From 41dd975aac2daf05931792744d2d2c61e6e87906 Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Wed, 3 Apr 2019 10:20:36 +0200 Subject: [PATCH] chore: err_reason for initial_channel_tx initial_commit_tx --- common/initial_channel.c | 10 +++++++--- common/initial_channel.h | 4 +++- common/initial_commit_tx.c | 6 +++++- common/initial_commit_tx.h | 4 +++- openingd/openingd.c | 18 ++++++++++-------- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/common/initial_channel.c b/common/initial_channel.c index d5983fec9..228587387 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -73,7 +73,8 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, const u8 **wscript, const struct channel *channel, const struct pubkey *per_commitment_point, - enum side side) + enum side side, + char** err_reason) { struct keyset keyset; @@ -83,8 +84,10 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, if (!derive_keyset(per_commitment_point, &channel->basepoints[side], &channel->basepoints[!side], - &keyset)) + &keyset)){ + *err_reason = "Cannot derive keyset"; return NULL; + } *wscript = bitcoin_redeem_2of2(ctx, &channel->funding_pubkey[side], @@ -103,7 +106,8 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, channel->view[side].owed[!side], channel->config[!side].channel_reserve, 0 ^ channel->commitment_number_obscurer, - side); + side, + err_reason); } static char *fmt_channel_view(const tal_t *ctx, const struct channel_view *view) diff --git a/common/initial_channel.h b/common/initial_channel.h index 6d5aa445a..b4a91d3f0 100644 --- a/common/initial_channel.h +++ b/common/initial_channel.h @@ -111,6 +111,7 @@ struct channel *new_initial_channel(const tal_t *ctx, * @channel: The channel to evaluate * @per_commitment_point: Per-commitment point to determine keys * @side: which side to get the commitment transaction for + * @err_reason: When NULL is returned, this will point to a human readable reason. * * Returns the unsigned initial commitment transaction for @side, or NULL * if the channel size was insufficient to cover fees or reserves. @@ -119,6 +120,7 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, const u8 **wscript, const struct channel *channel, const struct pubkey *per_commitment_point, - enum side side); + enum side side, + char** err_reason); #endif /* LIGHTNING_COMMON_INITIAL_CHANNEL_H */ diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index 479472df7..ac61658a5 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -72,7 +72,8 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, struct amount_msat other_pay, struct amount_sat self_reserve, u64 obscured_commitment_number, - enum side side) + enum side side, + char** err_reason) { struct amount_sat base_fee; struct bitcoin_tx *tx; @@ -112,6 +113,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, * - it considers `feerate_per_kw` too small for timely * processing or unreasonably large. */ + *err_reason = "Funder cannot afford fee on initial commitment transaction"; status_unusual("Funder cannot afford fee" " on initial commitment transaction"); return NULL; @@ -127,6 +129,8 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, */ if (!amount_msat_greater_sat(self_pay, self_reserve) && !amount_msat_greater_sat(other_pay, self_reserve)) { + *err_reason = "Neither self amount nor other amount exceed reserve on " + "initial commitment transaction"; status_unusual("Neither self amount %s" " nor other amount %s" " exceed reserve %s" diff --git a/common/initial_commit_tx.h b/common/initial_commit_tx.h index fd6830f61..9bbb01200 100644 --- a/common/initial_commit_tx.h +++ b/common/initial_commit_tx.h @@ -61,6 +61,7 @@ static inline struct amount_sat commit_tx_base_fee(u32 feerate_per_kw, * @self_reserve: reserve the other side insisted we have * @obscured_commitment_number: number to encode in commitment transaction * @side: side to generate commitment transaction for. + * @err_reason: When NULL is returned, this will point to a human readable reason. * * We need to be able to generate the remote side's tx to create signatures, * but the BOLT is expressed in terms of generating our local commitment @@ -79,7 +80,8 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, struct amount_msat other_pay, struct amount_sat self_reserve, u64 obscured_commitment_number, - enum side side); + enum side side, + char** err_reason); /* 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, diff --git a/openingd/openingd.c b/openingd/openingd.c index 291416d45..b127d7099 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -445,6 +445,7 @@ static u8 *funder_channel(struct state *state, struct bitcoin_tx *funding; const u8 *wscript; struct amount_msat local_msat; + char* err_reason; /*~ For symmetry, we calculate our own reserve even though lightningd * could do it for the we-are-funding case. */ @@ -668,12 +669,12 @@ static u8 *funder_channel(struct state *state, /* This gives us their first commitment transaction. */ tx = initial_channel_tx(state, &wscript, state->channel, &state->first_per_commitment_point[REMOTE], - REMOTE); + REMOTE, &err_reason); if (!tx) { /* This should not happen: we should never create channels we * can't afford the fees for after reserve. */ negotiation_failed(state, true, - "Could not meet their fees and reserve"); + "Could not meet their fees and reserve: %s", err_reason); goto fail_2; } @@ -775,10 +776,10 @@ static u8 *funder_channel(struct state *state, * signature they sent against that. */ tx = initial_channel_tx(state, &wscript, state->channel, &state->first_per_commitment_point[LOCAL], - LOCAL); + LOCAL, &err_reason); if (!tx) { negotiation_failed(state, true, - "Could not meet our fees and reserve"); + "Could not meet our fees and reserve: %s", err_reason); goto fail_2; } @@ -844,6 +845,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) u8 *msg; const u8 *wscript; u8 channel_flags; + char* err_reason; /* BOLT #2: * @@ -1070,11 +1072,11 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) */ local_commit = initial_channel_tx(state, &wscript, state->channel, &state->first_per_commitment_point[LOCAL], - LOCAL); + LOCAL, &err_reason); /* This shouldn't happen either, AFAICT. */ if (!local_commit) { negotiation_failed(state, false, - "Could not meet our fees and reserve"); + "Could not meet our fees and reserve: %s", err_reason); return NULL; } @@ -1130,10 +1132,10 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) */ remote_commit = initial_channel_tx(state, &wscript, state->channel, &state->first_per_commitment_point[REMOTE], - REMOTE); + REMOTE, &err_reason); if (!remote_commit) { negotiation_failed(state, false, - "Could not meet their fees and reserve"); + "Could not meet their fees and reserve: %s", err_reason); return NULL; }