[0.6] `unwrap_onion_reply` unable to decode remote failure?

[ZmnSCPxj]

@cdecker @rustyrussell help with #638 please!

I have a "mostly" working version of #638 in a fishing branch: https://github.com/ZmnSCPxj/lightning/commits/fish

On top of #638, branch fish has:

1. In the test, corrected order in which we wait for the channel to be updated to the payer. The version in pay hands back failures to gossipd #638 starts the wait well after the channel has already been gossiped to the payer.
2. Dumps path_secrets in send_payment and the path_secrets in payment_failed. Running this shows that the path_secrets created in send_payment are the same as what is processed in payment_failed and passed to unwrap_onion_reply.

---

First, the test setup. Arrows show the direction of the newly-created channels.

    #   l3-->l4
    #   ^   / |
    #   |  /  |
    #   | L   v
    #   l2<--l1

l1 wants to pay l4. First routing gives the route l1-l4, which fails since l1 has no money in that channel to pay l4 with.

The succeeding case is what is interesting. With l1-l4 channel deactivated due to routing failure, routing will then give the route l1-l2-l4.

When the HTLC reaches l2, it realizes it has no capacity to pay l4, and so generates a WIRE_TEMPORARY_CHANNEL_FAILURE error: https://travis-ci.org/ZmnSCPxj/lightning/jobs/335677831#L7843

l2 then fails the offered HTLC: https://travis-ci.org/ZmnSCPxj/lightning/jobs/335677831#L7852

l1 receives the failure of the HTLC: https://travis-ci.org/ZmnSCPxj/lightning/jobs/335677831#L7856

And then l1 calls into payment_failed but fails to decrypt the onion reply: https://travis-ci.org/ZmnSCPxj/lightning/jobs/335677831#L7973

And now I am stuck!

[ZmnSCPxj]

Okay, unwrap_onionreply expects the message to be 256 + 32 + 4 bytes in length, or 296 bytes.

But the message being passed in to payment_failed is only 134 bytes: https://travis-ci.org/ZmnSCPxj/lightning/jobs/335677831#L7973

Which is strange!

So I checked a little deeper into channeld. When channeld makes a routing failure onionreply, it goes through this code to create the message:

lightning/channeld/channel.c

Line 2337 in dd044f8

errpkt = make_failmsg(inmsg, peer, h,

Which calls this function:

lightning/channeld/channel.c

Lines 2212 to 2299 in dd044f8

	static u8 *make_failmsg(const tal_t *ctx,
	struct peer *peer,
	const struct htlc *htlc,
	enum onion_type failcode,
	const struct short_channel_id *scid)
	{
	u8 *msg, *channel_update = NULL;
	u32 cltv_expiry = abs_locktime_to_blocks(&htlc->expiry);
	switch (failcode) {
	case WIRE_INVALID_REALM:
	msg = towire_invalid_realm(ctx);
	goto done;
	case WIRE_TEMPORARY_NODE_FAILURE:
	msg = towire_temporary_node_failure(ctx);
	goto done;
	case WIRE_PERMANENT_NODE_FAILURE:
	msg = towire_permanent_node_failure(ctx);
	goto done;
	case WIRE_REQUIRED_NODE_FEATURE_MISSING:
	msg = towire_required_node_feature_missing(ctx);
	goto done;
	case WIRE_TEMPORARY_CHANNEL_FAILURE:
	channel_update = foreign_channel_update(ctx, peer, scid);
	msg = towire_temporary_channel_failure(ctx, channel_update);
	goto done;
	case WIRE_CHANNEL_DISABLED:
	msg = towire_channel_disabled(ctx);
	goto done;
	case WIRE_PERMANENT_CHANNEL_FAILURE:
	msg = towire_permanent_channel_failure(ctx);
	goto done;
	case WIRE_REQUIRED_CHANNEL_FEATURE_MISSING:
	msg = towire_required_channel_feature_missing(ctx);
	goto done;
	case WIRE_UNKNOWN_NEXT_PEER:
	msg = towire_unknown_next_peer(ctx);
	goto done;
	case WIRE_AMOUNT_BELOW_MINIMUM:
	channel_update = foreign_channel_update(ctx, peer, scid);
	msg = towire_amount_below_minimum(ctx, htlc->msatoshi,
	channel_update);
	goto done;
	case WIRE_FEE_INSUFFICIENT:
	channel_update = foreign_channel_update(ctx, peer, scid);
	msg = towire_fee_insufficient(ctx, htlc->msatoshi,
	channel_update);
	goto done;
	case WIRE_INCORRECT_CLTV_EXPIRY:
	channel_update = foreign_channel_update(ctx, peer, scid);
	msg = towire_incorrect_cltv_expiry(ctx, cltv_expiry,
	channel_update);
	goto done;
	case WIRE_EXPIRY_TOO_SOON:
	channel_update = foreign_channel_update(ctx, peer, scid);
	msg = towire_expiry_too_soon(ctx, channel_update);
	goto done;
	case WIRE_EXPIRY_TOO_FAR:
	msg = towire_expiry_too_far(ctx);
	goto done;
	case WIRE_UNKNOWN_PAYMENT_HASH:
	msg = towire_unknown_payment_hash(ctx);
	goto done;
	case WIRE_INCORRECT_PAYMENT_AMOUNT:
	msg = towire_incorrect_payment_amount(ctx);
	goto done;
	case WIRE_FINAL_EXPIRY_TOO_SOON:
	msg = towire_final_expiry_too_soon(ctx);
	goto done;
	case WIRE_FINAL_INCORRECT_CLTV_EXPIRY:
	msg = towire_final_incorrect_cltv_expiry(ctx, cltv_expiry);
	goto done;
	case WIRE_FINAL_INCORRECT_HTLC_AMOUNT:
	msg = towire_final_incorrect_htlc_amount(ctx, htlc->msatoshi);
	goto done;
	case WIRE_INVALID_ONION_VERSION:
	case WIRE_INVALID_ONION_HMAC:
	case WIRE_INVALID_ONION_KEY:
	break;
	}
	status_failed(STATUS_FAIL_INTERNAL_ERROR,
	"Asked to create failmsg %u (%s)",
	failcode, onion_type_name(failcode));
	done:
	tal_free(channel_update);
	return msg;
	}

Of note is that this function does not call create_onionreply, which pads the message and adds the hmac.

In fact, searching for create_onionreply:https://github.com/ElementsProject/lightning/search?utf8=%E2%9C%93&q=create_onionreply&type=

Shows we only call create_onionreply in the sphinx testing program... and not in the actual lightningd (particularly channeld) code!

So it seems we need to run create_onionreply inside make_failmsg!