Browse Source

lightningd: store raw msg rather than code for locally-failed outgoing HTLCs

At the moment, we store e.g. WIRE_TEMPORARY_CHANNEL_FAILURE, and then
lightningd has a large demux function which turns that into the correct
error message.

Such an enum demuxer is an anti-pattern.

Instead, store the message directly for output HTLCs; channeld now
sends us an error message rather than an error code.

For input HTLCs we will still need the failure code if the onion was
bad (since we need to prompt channeld to send a completely different
message than normal), though we can (and will!) eliminate its use in
non-BADONION failure cases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
travis-debug
Rusty Russell 5 years ago
parent
commit
72d55d3e3b
  1. 5
      channeld/channel_wire.csv
  2. 16
      channeld/channeld.c
  3. 14
      lightningd/htlc_end.c
  4. 5
      lightningd/htlc_end.h
  5. 73
      lightningd/pay.c
  6. 115
      lightningd/peer_htlcs.c
  7. 2
      wallet/test/run-wallet.c
  8. 7
      wallet/wallet.c

5
channeld/channel_wire.csv

@ -87,8 +87,9 @@ msgdata,channel_offer_htlc,onion_routing_packet,u8,1366
# Reply; synchronous since IDs have to increment.
msgtype,channel_offer_htlc_reply,1104
msgdata,channel_offer_htlc_reply,id,u64,
# Zero failure code means success.,
msgdata,channel_offer_htlc_reply,failure_code,u16,
# Empty failure message means success.
msgdata,channel_offer_htlc_reply,len,u16,
msgdata,channel_offer_htlc_reply,failuremsg,u8,len
msgdata,channel_offer_htlc_reply,failurestr,wirestring,
# Main daemon found out the preimage for an HTLC

Can't render this file because it has a wrong number of fields in line 8.

16
channeld/channeld.c

@ -308,7 +308,7 @@ static void send_channel_update(struct peer *peer, int disable_flag)
}
/* Get the latest channel update for this channel from gossipd */
static NEEDED const u8 *get_local_channel_update(const tal_t *ctx, struct peer *peer)
static const u8 *get_local_channel_update(const tal_t *ctx, struct peer *peer)
{
const u8 *msg;
@ -2407,7 +2407,7 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
struct sha256 payment_hash;
u8 onion_routing_packet[TOTAL_PACKET_SIZE];
enum channel_add_err e;
enum onion_type failcode;
const u8 *failwiremsg;
const char *failstr;
struct amount_sat htlc_fee;
@ -2445,7 +2445,7 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
peer->htlc_id++;
return;
case CHANNEL_ERR_INVALID_EXPIRY:
failcode = WIRE_INCORRECT_CLTV_EXPIRY;
failwiremsg = towire_incorrect_cltv_expiry(inmsg, cltv_expiry, get_local_channel_update(tmpctx, peer));
failstr = tal_fmt(inmsg, "Invalid cltv_expiry %u", cltv_expiry);
goto failed;
case CHANNEL_ERR_DUPLICATE:
@ -2454,23 +2454,23 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
"Duplicate HTLC %"PRIu64, peer->htlc_id);
case CHANNEL_ERR_MAX_HTLC_VALUE_EXCEEDED:
failcode = WIRE_REQUIRED_CHANNEL_FEATURE_MISSING;
failwiremsg = towire_required_node_feature_missing(inmsg);
failstr = "Mini mode: maximum value exceeded";
goto failed;
/* FIXME: Fuzz the boundaries a bit to avoid probing? */
case CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED:
failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
failwiremsg = towire_temporary_channel_failure(inmsg, get_local_channel_update(inmsg, peer));
failstr = tal_fmt(inmsg, "Capacity exceeded - HTLC fee: %s", fmt_amount_sat(inmsg, &htlc_fee));
goto failed;
case CHANNEL_ERR_HTLC_BELOW_MINIMUM:
failcode = WIRE_AMOUNT_BELOW_MINIMUM;
failwiremsg = towire_amount_below_minimum(inmsg, amount, get_local_channel_update(inmsg, peer));
failstr = tal_fmt(inmsg, "HTLC too small (%s minimum)",
type_to_string(tmpctx,
struct amount_msat,
&peer->channel->config[REMOTE].htlc_minimum));
goto failed;
case CHANNEL_ERR_TOO_MANY_HTLCS:
failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
failwiremsg = towire_temporary_channel_failure(inmsg, get_local_channel_update(inmsg, peer));
failstr = "Too many HTLCs";
goto failed;
}
@ -2478,7 +2478,7 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
abort();
failed:
msg = towire_channel_offer_htlc_reply(NULL, 0, failcode, failstr);
msg = towire_channel_offer_htlc_reply(NULL, 0, failwiremsg, failstr);
wire_sync_write(MASTER_FD, take(msg));
}

14
lightningd/htlc_end.c

@ -192,13 +192,13 @@ struct htlc_out *htlc_out_check(const struct htlc_out *hout,
if (hout->in->preimage)
return corrupt(abortstr,
"Output failmsg, input preimage");
} else if (hout->failcode) {
} else if (hout->failmsg) {
if (hout->in->failonion)
return corrupt(abortstr,
"Output failcode, input failonion");
"Output failmsg, input failonion");
if (hout->in->preimage)
return corrupt(abortstr,
"Output failcode, input preimage");
"Output failmsg, input preimage");
} else if (hout->preimage) {
if (hout->in->failonion)
return corrupt(abortstr,
@ -226,11 +226,11 @@ struct htlc_out *htlc_out_check(const struct htlc_out *hout,
return corrupt(abortstr, "Still adding, has preimage");
if (hout->failonion)
return corrupt(abortstr, "Still adding, has failmsg");
if (hout->failcode)
return corrupt(abortstr, "Still adding, has failcode");
if (hout->failmsg)
return corrupt(abortstr, "Still adding, has failmsg");
} else if (hout->hstate >= RCVD_REMOVE_HTLC
&& hout->hstate <= RCVD_REMOVE_ACK_REVOCATION) {
if (!hout->preimage && !hout->failonion && !hout->failcode)
if (!hout->preimage && !hout->failonion && !hout->failmsg)
return corrupt(abortstr, "Removing, no resolution");
} else
return corrupt(abortstr, "Bad state %s",
@ -285,7 +285,7 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
sizeof(hout->onion_routing_packet));
hout->hstate = SENT_ADD_HTLC;
hout->failcode = 0;
hout->failmsg = NULL;
hout->failonion = NULL;
hout->preimage = NULL;

5
lightningd/htlc_end.h

@ -69,9 +69,8 @@ struct htlc_out {
/* Onion information */
u8 onion_routing_packet[TOTAL_PACKET_SIZE];
/* FIXME: Use failed_htlc here */
/* If a local error, this is non-zero. */
enum onion_type failcode;
/* If a local error, this is non-NULL. */
const u8 *failmsg;
/* For a remote error. */
const struct onionreply *failonion;

73
lightningd/pay.c

@ -344,11 +344,10 @@ local_routing_failure(const tal_t *ctx,
{
struct routing_failure *routing_failure;
assert(hout->failcode);
routing_failure = tal(ctx, struct routing_failure);
routing_failure->erring_index = 0;
routing_failure->failcode = hout->failcode;
routing_failure->failcode = fromwire_peektype(hout->failmsg);
routing_failure->erring_node =
tal_dup(routing_failure, struct node_id, &ld->id);
@ -365,7 +364,8 @@ local_routing_failure(const tal_t *ctx,
routing_failure->msg = NULL;
log_debug(hout->key.channel->log, "local_routing_failure: %u (%s)",
hout->failcode, onion_type_name(hout->failcode));
routing_failure->failcode,
onion_type_name(routing_failure->failcode));
return routing_failure;
}
@ -427,7 +427,9 @@ remote_routing_failure(const tal_t *ctx,
* - if the PERM bit is set:
* - SHOULD fail the payment.
* */
if (failcode & PERM)
if (failcode & BADONION)
*pay_errcode = PAY_UNPARSEABLE_ONION;
else if (failcode & PERM)
*pay_errcode = PAY_DESTINATION_PERM_FAIL;
else
/* FIXME: not right for WIRE_FINAL_EXPIRY_TOO_SOON */
@ -487,30 +489,6 @@ remote_routing_failure(const tal_t *ctx,
return routing_failure;
}
/* If our immediate peer says WIRE_UPDATE_FAIL_MALFORMED_HTLC, we only get a
* code, no onion msg. */
static struct routing_failure *
badonion_routing_failure(const tal_t *ctx,
struct lightningd *ld,
const struct wallet_payment *payment,
enum onion_type failcode)
{
struct routing_failure *rf;
rf = tal(ctx, struct routing_failure);
rf->erring_index = 0;
rf->failcode = failcode;
rf->msg = NULL;
rf->erring_node = tal_dup(rf, struct node_id,
&payment->route_nodes[0]);
rf->erring_channel = tal_dup(rf, struct short_channel_id,
&payment->route_channels[0]);
rf->channel_dir = node_id_idx(&ld->id, &payment->route_nodes[0]);
return rf;
}
void payment_store(struct lightningd *ld, struct wallet_payment *payment TAKES)
{
struct sendpay_command *pc;
@ -540,6 +518,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
struct routing_failure* fail = NULL;
const char *failstr;
errcode_t pay_errcode;
const u8 *failmsg;
int origin_index;
payment = wallet_payment_by_hash(tmpctx, ld->wallet,
&hout->payment_hash,
@ -575,26 +555,24 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
pay_errcode = PAY_UNPARSEABLE_ONION;
fail = NULL;
failstr = NULL;
} else if (hout->failcode) {
/* Direct peer told channeld it's a malformed onion using
* update_fail_malformed_htlc. */
failstr = "malformed onion";
fail = badonion_routing_failure(tmpctx, ld, payment,
hout->failcode);
pay_errcode = PAY_UNPARSEABLE_ONION;
} else if (hout->failmsg) {
/* This can happen when a direct peer told channeld it's a
* malformed onion using update_fail_malformed_htlc. */
failstr = "local failure";
failmsg = hout->failmsg;
origin_index = 0;
pay_errcode = PAY_TRY_OTHER_ROUTE;
goto use_failmsg;
} else {
/* Must be normal remote fail with an onion-wrapped error. */
assert(!hout->failcode);
failstr = "reply from remote";
/* Try to parse reply. */
struct secret *path_secrets = payment->path_secrets;
u8 *reply;
int origin_index;
reply = unwrap_onionreply(tmpctx, path_secrets,
tal_count(path_secrets),
hout->failonion, &origin_index);
if (!reply) {
failmsg = unwrap_onionreply(tmpctx, path_secrets,
tal_count(path_secrets),
hout->failonion, &origin_index);
if (!failmsg) {
log_info(hout->key.channel->log,
"htlc %"PRIu64" failed with bad reply (%s)",
hout->key.id,
@ -603,7 +581,10 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
fail = NULL;
pay_errcode = PAY_UNPARSEABLE_ONION;
} else {
enum onion_type failcode = fromwire_peektype(reply);
enum onion_type failcode;
use_failmsg:
failcode = fromwire_peektype(failmsg);
log_info(hout->key.channel->log,
"htlc %"PRIu64" "
"failed from %ith node "
@ -611,10 +592,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
hout->key.id,
origin_index,
failcode, onion_type_name(failcode));
log_debug(hout->key.channel->log, "failmsg: %s",
tal_hex(tmpctx, reply));
fail = remote_routing_failure(tmpctx, ld,
payment, reply,
payment, failmsg,
origin_index,
hout->key.channel->log,
&pay_errcode);

115
lightningd/peer_htlcs.c

@ -91,8 +91,8 @@ static bool htlc_out_update_state(struct channel *channel,
return false;
wallet_htlc_update(channel->peer->ld->wallet, hout->dbid, newstate,
hout->preimage, hout->failcode, hout->failonion,
NULL);
hout->preimage, 0, hout->failonion,
hout->failmsg);
hout->hstate = newstate;
return true;
@ -273,12 +273,22 @@ void fail_htlc(struct htlc_in *hin, enum onion_type failcode)
static void fail_out_htlc(struct htlc_out *hout, const char *localfail)
{
htlc_out_check(hout, __func__);
assert(hout->failcode || hout->failonion);
assert(hout->failmsg || hout->failonion);
if (hout->am_origin) {
payment_failed(hout->key.channel->peer->ld, hout, localfail);
} else if (hout->in) {
fail_in_htlc(hout->in, hout->failcode, hout->failonion,
const struct onionreply *failonion;
/* If we have an onion, simply copy it. */
if (hout->failonion)
failonion = hout->failonion;
/* Otherwise, we need to onionize this local error. */
else
failonion = create_onionreply(hout,
hout->in->shared_secret,
hout->failmsg);
fail_in_htlc(hout->in, 0, failonion,
hout->key.channel->stripped_update);
}
}
@ -475,10 +485,11 @@ static void destroy_hout_subd_died(struct htlc_out *hout)
"Failing HTLC %"PRIu64" due to peer death",
hout->key.id);
hout->failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
hout->failmsg = towire_temporary_channel_failure(hout,
hout->key.channel->stripped_update);
/* Assign a temporary state (we're about to free it!) so checks
* are happy that it has a failure code */
* are happy that it has a failure message */
assert(hout->hstate == SENT_ADD_HTLC);
hout->hstate = RCVD_REMOVE_HTLC;
@ -490,13 +501,13 @@ static void destroy_hout_subd_died(struct htlc_out *hout)
static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED,
struct htlc_out *hout)
{
u16 failure_code;
u8 *failmsg;
char *failurestr;
struct lightningd *ld = subd->ld;
if (!fromwire_channel_offer_htlc_reply(msg, msg,
&hout->key.id,
&failure_code,
&failmsg,
&failurestr)) {
channel_internal_error(subd->channel,
"Bad channel_offer_htlc_reply");
@ -504,24 +515,29 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU
return;
}
if (failure_code) {
hout->failcode = (enum onion_type) failure_code;
if (tal_count(failmsg)) {
hout->failmsg = tal_steal(hout, failmsg);
if (hout->am_origin) {
char *localfail = tal_fmt(msg, "%s: %s",
onion_type_name(failure_code),
onion_type_name(fromwire_peektype(failmsg)),
failurestr);
payment_failed(ld, hout, localfail);
} else if (hout->in) {
local_fail_htlc(hout->in, failure_code,
hout->key.channel->stripped_update);
struct onionreply *failonion;
failonion = create_onionreply(hout,
hout->in->shared_secret,
hout->failmsg);
fail_in_htlc(hout->in, 0, failonion,
hout->key.channel->stripped_update);
/* here we haven't called connect_htlc_out(),
* so set htlc field with NULL */
wallet_forwarded_payment_add(ld->wallet,
hout->in, NULL, NULL,
FORWARD_LOCAL_FAILED,
failure_code);
fromwire_peektype(hout->failmsg));
}
/* Prevent hout from being failed twice. */
@ -1101,8 +1117,8 @@ static void fulfill_our_htlc_out(struct channel *channel, struct htlc_out *hout,
htlc_out_check(hout, __func__);
wallet_htlc_update(ld->wallet, hout->dbid, hout->hstate,
hout->preimage, hout->failcode, hout->failonion,
NULL);
hout->preimage, 0, hout->failonion,
hout->failmsg);
/* Update channel stats */
wallet_channel_stats_incr_out_fulfilled(ld->wallet,
channel->dbid,
@ -1158,7 +1174,7 @@ void onchain_fulfilled_htlc(struct channel *channel,
/* It's possible that we failed some and succeeded one,
* if we got multiple errors. */
if (hout->failcode != 0 || hout->failonion)
if (hout->failmsg || hout->failonion)
continue;
if (!sha256_eq(&hout->payment_hash, &payment_hash))
@ -1196,6 +1212,7 @@ static bool peer_failed_our_htlc(struct channel *channel,
if (failed->sha256_of_onion) {
struct sha256 our_sha256_of_onion;
u8 *failmsg;
/* BOLT #2:
*
@ -1211,19 +1228,6 @@ static bool peer_failed_our_htlc(struct channel *channel,
" for htlc with id %"PRIu64".",
hout->key.id);
/* We only handle these cases in make_failmsg, so convert any
* (future?) unknown one. */
if (failed->badonion != WIRE_INVALID_ONION_VERSION
&& failed->badonion != WIRE_INVALID_ONION_HMAC
&& failed->badonion != WIRE_INVALID_ONION_KEY) {
log_unusual(channel->log,
"Unknown update_fail_malformed_htlc code %u:"
" sending WIRE_INVALID_ONION_VERSION",
failed->badonion);
hout->failcode = WIRE_INVALID_ONION_VERSION;
} else
hout->failcode = failed->badonion;
/* BOLT #2:
*
* - otherwise, a receiving node which has an outgoing HTLC
@ -1234,30 +1238,29 @@ static bool peer_failed_our_htlc(struct channel *channel,
* `failure_code` given and setting the data to
* `sha256_of_onion`.
*/
if (hout->in) {
const u8 *f;
f = make_failmsg(tmpctx, hout->in, hout->failcode,
hout->key.channel->stripped_update,
failed->sha256_of_onion,
get_block_height(hout->key.channel->owner->ld->topology));
hout->failonion = create_onionreply(hout,
hout->in->shared_secret,
f);
hout->failcode = 0;
}
/* All badonion codes are the same form, so we make them
* manually, which covers any unknown cases too. Grep fodder:
* towire_invalid_onion_version, towire_invalid_onion_hmac,
* towire_invalid_onion_key. */
failmsg = tal_arr(hout, u8, 0);
towire_u16(&failmsg, failed->badonion);
towire_sha256(&failmsg, failed->sha256_of_onion);
hout->failmsg = failmsg;
} else {
hout->failonion = dup_onionreply(hout, failed->onion);
}
log_debug(channel->log, "Our HTLC %"PRIu64" failed (%u)", failed->id,
hout->failcode);
fromwire_peektype(hout->failmsg));
htlc_out_check(hout, __func__);
if (hout->in)
wallet_forwarded_payment_add(ld->wallet, hout->in,
channel->scid,
hout, FORWARD_FAILED, hout->failcode);
hout, FORWARD_FAILED,
hout->failmsg
? fromwire_peektype(hout->failmsg)
: 0);
return true;
}
@ -1274,17 +1277,17 @@ void onchain_failed_our_htlc(const struct channel *channel,
return;
/* Don't fail twice (or if already succeeded)! */
if (hout->failonion || hout->failcode || hout->preimage)
if (hout->failonion || hout->failmsg || hout->preimage)
return;
hout->failcode = WIRE_PERMANENT_CHANNEL_FAILURE;
hout->failmsg = towire_permanent_channel_failure(hout);
/* Force state to something which expects a failure, and save to db */
hout->hstate = RCVD_REMOVE_HTLC;
htlc_out_check(hout, __func__);
wallet_htlc_update(ld->wallet, hout->dbid, hout->hstate,
hout->preimage, hout->failcode, hout->failonion,
NULL);
hout->preimage, 0, hout->failonion,
hout->failmsg);
if (hout->am_origin) {
assert(why != NULL);
@ -1299,7 +1302,9 @@ void onchain_failed_our_htlc(const struct channel *channel,
wallet_forwarded_payment_add(hout->key.channel->peer->ld->wallet,
hout->in, channel->scid, hout,
FORWARD_LOCAL_FAILED,
hout->failcode);
hout->failmsg
? fromwire_peektype(hout->failmsg)
: 0);
}
}
@ -1343,11 +1348,11 @@ static void remove_htlc_in(struct channel *channel, struct htlc_in *hin)
static void remove_htlc_out(struct channel *channel, struct htlc_out *hout)
{
htlc_out_check(hout, __func__);
assert(hout->failonion || hout->preimage || hout->failcode);
assert(hout->failonion || hout->preimage || hout->failmsg);
log_debug(channel->log, "Removing out HTLC %"PRIu64" state %s %s",
hout->key.id, htlc_state_name(hout->hstate),
hout->preimage ? "FULFILLED"
: hout->failcode ? onion_type_name(hout->failcode)
: hout->failmsg ? onion_type_name(fromwire_peektype(hout->failmsg))
: "REMOTEFAIL");
/* If it's failed, now we can forward since it's completely locked-in */
@ -2043,7 +2048,7 @@ void peer_htlcs(const tal_t *ctx,
hout->cltv_expiry, hout->onion_routing_packet,
hout->hstate);
if (hout->failonion || hout->failcode)
if (hout->failonion || hout->failmsg)
tal_arr_expand(failed_out, hout->key.id);
if (hout->preimage)
@ -2227,7 +2232,7 @@ static void fixup_hout(struct lightningd *ld, struct htlc_out *hout)
return;
/* Failed ones (only happens after db fixed!) OK. */
if (hout->failcode || hout->failonion)
if (hout->failmsg || hout->failonion)
return;
/* payment_preimage for HTLC in *was* stored, so look for that. */
@ -2236,8 +2241,8 @@ static void fixup_hout(struct lightningd *ld, struct htlc_out *hout)
hout->in->preimage);
fix = "restoring preimage from incoming HTLC";
} else {
hout->failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
fix = "subsituting temporary channel failure";
hout->failmsg = towire_temporary_node_failure(hout);
fix = "subsituting temporary node failure";
}
log_broken(ld->log, "HTLC #%"PRIu64" (%s) "

2
wallet/test/run-wallet.c

@ -109,7 +109,7 @@ bool fromwire_channel_got_commitsig(const tal_t *ctx UNNEEDED, const void *p UNN
bool fromwire_channel_got_revoke(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *revokenum UNNEEDED, struct secret *per_commitment_secret UNNEEDED, struct pubkey *next_per_commit_point UNNEEDED, struct fee_states **fee_states UNNEEDED, struct changed_htlc **changed UNNEEDED)
{ fprintf(stderr, "fromwire_channel_got_revoke called!\n"); abort(); }
/* Generated stub for fromwire_channel_offer_htlc_reply */
bool fromwire_channel_offer_htlc_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *id UNNEEDED, u16 *failure_code UNNEEDED, wirestring **failurestr UNNEEDED)
bool fromwire_channel_offer_htlc_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *id UNNEEDED, u8 **failuremsg UNNEEDED, wirestring **failurestr UNNEEDED)
{ fprintf(stderr, "fromwire_channel_offer_htlc_reply called!\n"); abort(); }
/* Generated stub for fromwire_channel_sending_commitsig */
bool fromwire_channel_sending_commitsig(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *commitnum UNNEEDED, struct fee_states **fee_states UNNEEDED, struct changed_htlc **changed UNNEEDED, struct bitcoin_signature *commit_sig UNNEEDED, secp256k1_ecdsa_signature **htlc_sigs UNNEEDED)

7
wallet/wallet.c

@ -1882,7 +1882,12 @@ static bool wallet_stmt2htlc_out(struct wallet *wallet,
else
out->failonion = db_column_onionreply(out, stmt, 8);
out->failcode = db_column_int_or_default(stmt, 9, 0);
if (db_column_is_null(stmt, 14))
out->failmsg = NULL;
else
out->failmsg = tal_dup_arr(out, u8, db_column_blob(stmt, 14),
db_column_bytes(stmt, 14), 0);
out->in = NULL;
if (!db_column_is_null(stmt, 10)) {

Loading…
Cancel
Save