Browse Source

lightningd: don't report spurious temporary_node_failure on local failures.

I noticed the following in logs for tests/test_connection.py::test_feerate_stress:

```
DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: Failing HTLC 18446744073709551615 due to peer death
DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: local_routing_failure: 8194 (WIRE_TEMPORARY_NODE_FAILURE)
```

This is because it reports the (transient) node_failure error, because
our channel_failure message is incomplete.  Fix this wart up.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
nifty/pset-pre
Rusty Russell 5 years ago
parent
commit
4eb1233ccb
  1. 15
      lightningd/pay.c
  2. 4
      lightningd/pay.h
  3. 7
      lightningd/peer_htlcs.c
  4. 5
      lightningd/test/run-invoice-select-inchan.c
  5. 5
      lightningd/test/run-jsonrpc.c
  6. 2
      onchaind/test/run-grind_feerate-bug.c
  7. 2
      onchaind/test/run-grind_feerate.c
  8. 1
      tests/test_connection.py
  9. 7
      wallet/test/run-wallet.c

15
lightningd/pay.c

@ -340,13 +340,14 @@ static struct routing_failure*
local_routing_failure(const tal_t *ctx,
const struct lightningd *ld,
const struct htlc_out *hout,
enum onion_type failcode,
const struct wallet_payment *payment)
{
struct routing_failure *routing_failure;
routing_failure = tal(ctx, struct routing_failure);
routing_failure->erring_index = 0;
routing_failure->failcode = fromwire_peektype(hout->failmsg);
routing_failure->failcode = failcode;
routing_failure->erring_node =
tal_dup(routing_failure, struct node_id, &ld->id);
@ -511,7 +512,7 @@ void payment_store(struct lightningd *ld, struct wallet_payment *payment TAKES)
}
void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
const char *localfail)
const char *localfail, const u8 *failmsg_needs_update)
{
struct wallet_payment *payment;
struct routing_failure* fail = NULL;
@ -542,7 +543,15 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
/* This gives more details than a generic failure message */
if (localfail) {
fail = local_routing_failure(tmpctx, ld, hout, payment);
/* Use temporary_channel_failure if failmsg has it */
enum onion_type failcode;
if (failmsg_needs_update)
failcode = fromwire_peektype(failmsg_needs_update);
else
failcode = fromwire_peektype(hout->failmsg);
fail = local_routing_failure(tmpctx, ld, hout, failcode,
payment);
failstr = localfail;
pay_errcode = PAY_TRY_OTHER_ROUTE;
} else if (payment->path_secrets == NULL) {

4
lightningd/pay.h

@ -16,8 +16,10 @@ struct routing_failure;
void payment_succeeded(struct lightningd *ld, struct htlc_out *hout,
const struct preimage *rval);
/* failmsg_needs_update is if we actually wanted to temporary_channel_failure
* but we haven't got the update msg yet */
void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
const char *localfail);
const char *localfail, const u8 *failmsg_needs_update);
/* Inform payment system to save the payment. */
void payment_store(struct lightningd *ld, struct wallet_payment *payment);

7
lightningd/peer_htlcs.c

@ -295,7 +295,8 @@ static void fail_out_htlc(struct htlc_out *hout,
assert(hout->failmsg || hout->failonion);
if (hout->am_origin) {
payment_failed(hout->key.channel->peer->ld, hout, localfail);
payment_failed(hout->key.channel->peer->ld, hout, localfail,
failmsg_needs_update);
if (taken(failmsg_needs_update))
tal_free(failmsg_needs_update);
} else if (hout->in) {
@ -549,7 +550,7 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU
char *localfail = tal_fmt(msg, "%s: %s",
onion_type_name(fromwire_peektype(failmsg)),
failurestr);
payment_failed(ld, hout, localfail);
payment_failed(ld, hout, localfail, NULL);
} else if (hout->in) {
struct onionreply *failonion;
@ -1422,7 +1423,7 @@ void onchain_failed_our_htlc(const struct channel *channel,
char *localfail = tal_fmt(channel, "%s: %s",
onion_type_name(WIRE_PERMANENT_CHANNEL_FAILURE),
why);
payment_failed(ld, hout, localfail);
payment_failed(ld, hout, localfail, NULL);
tal_free(localfail);
} else if (hout->in) {
local_fail_in_htlc(hout->in,

5
lightningd/test/run-invoice-select-inchan.c

@ -375,11 +375,8 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED,
{ fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
tal_t *cb_arg UNNEEDED)
tal_t *cb_arg STEALS UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for plugin_hook_continue */
bool plugin_hook_continue(void *arg UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED)
{ fprintf(stderr, "plugin_hook_continue called!\n"); abort(); }
/* Generated stub for subd_release_channel */
void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED)
{ fprintf(stderr, "subd_release_channel called!\n"); abort(); }

5
lightningd/test/run-jsonrpc.c

@ -96,11 +96,8 @@ struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name
{ fprintf(stderr, "param_tok called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
tal_t *cb_arg UNNEEDED)
tal_t *cb_arg STEALS UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for plugin_hook_continue */
bool plugin_hook_continue(void *arg UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED)
{ fprintf(stderr, "plugin_hook_continue called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */
bool deprecated_apis;

2
onchaind/test/run-grind_feerate-bug.c

@ -43,7 +43,7 @@ bool fromwire_onchain_dev_memleak(const void *p UNNEEDED)
bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED)
{ fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); }
/* Generated stub for fromwire_onchain_init */
bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED)
bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED)
{ fprintf(stderr, "fromwire_onchain_init called!\n"); abort(); }
/* Generated stub for fromwire_onchain_known_preimage */
bool fromwire_onchain_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED)

2
onchaind/test/run-grind_feerate.c

@ -47,7 +47,7 @@ bool fromwire_onchain_dev_memleak(const void *p UNNEEDED)
bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED)
{ fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); }
/* Generated stub for fromwire_onchain_init */
bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED)
bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED)
{ fprintf(stderr, "fromwire_onchain_init called!\n"); abort(); }
/* Generated stub for fromwire_onchain_known_preimage */
bool fromwire_onchain_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED)

1
tests/test_connection.py

@ -2193,6 +2193,7 @@ def test_feerate_stress(node_factory, executor):
# Make sure it's reconnected, and wait for last payment.
wait_for(lambda: l1.rpc.getpeer(l2.info['id'])['connected'])
# We can get temporary NODE_
with pytest.raises(RpcError, match='WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS'):
l1.rpc.waitsendpay("{:064x}".format(l1done - 1))
with pytest.raises(RpcError, match='WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS'):

7
wallet/test/run-wallet.c

@ -509,7 +509,7 @@ enum onion_type parse_onionpacket(const u8 *src UNNEEDED,
{ fprintf(stderr, "parse_onionpacket called!\n"); abort(); }
/* Generated stub for payment_failed */
void payment_failed(struct lightningd *ld UNNEEDED, const struct htlc_out *hout UNNEEDED,
const char *localfail UNNEEDED)
const char *localfail UNNEEDED, const u8 *failmsg_needs_update UNNEEDED)
{ fprintf(stderr, "payment_failed called!\n"); abort(); }
/* Generated stub for payment_store */
void payment_store(struct lightningd *ld UNNEEDED, struct wallet_payment *payment UNNEEDED)
@ -547,11 +547,8 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED,
{ fprintf(stderr, "per_peer_state_set_fds called!\n"); abort(); }
/* Generated stub for plugin_hook_call_ */
void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED,
tal_t *cb_arg UNNEEDED)
tal_t *cb_arg STEALS UNNEEDED)
{ fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); }
/* Generated stub for plugin_hook_continue */
bool plugin_hook_continue(void *arg UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED)
{ fprintf(stderr, "plugin_hook_continue called!\n"); abort(); }
/* Generated stub for process_onionpacket */
struct route_step *process_onionpacket(
const tal_t * ctx UNNEEDED,

Loading…
Cancel
Save