diff --git a/channeld/channeld.c b/channeld/channeld.c index fe7f7707f..eb1b7a503 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1571,28 +1571,30 @@ static void marshall_htlc_info(const tal_t *ctx, struct changed_htlc **changed, struct fulfilled_htlc **fulfilled, const struct failed_htlc ***failed, - struct added_htlc **added) + const struct added_htlc ***added) { *changed = tal_arr(ctx, struct changed_htlc, 0); - *added = tal_arr(ctx, struct added_htlc, 0); + *added = tal_arr(ctx, const struct added_htlc *, 0); *failed = tal_arr(ctx, const struct failed_htlc *, 0); *fulfilled = tal_arr(ctx, struct fulfilled_htlc, 0); for (size_t i = 0; i < tal_count(changed_htlcs); i++) { const struct htlc *htlc = changed_htlcs[i]; if (htlc->state == RCVD_ADD_COMMIT) { - struct added_htlc a; + struct added_htlc *a = tal(*added, struct added_htlc); - a.id = htlc->id; - a.amount = htlc->amount; - a.payment_hash = htlc->rhash; - a.cltv_expiry = abs_locktime_to_blocks(&htlc->expiry); - memcpy(a.onion_routing_packet, + a->id = htlc->id; + a->amount = htlc->amount; + a->payment_hash = htlc->rhash; + a->cltv_expiry = abs_locktime_to_blocks(&htlc->expiry); + memcpy(a->onion_routing_packet, htlc->routing, - sizeof(a.onion_routing_packet)); - a.path_key = htlc->path_key; - a.extra_tlvs = htlc->extra_tlvs; - a.fail_immediate = htlc->fail_immediate; + sizeof(a->onion_routing_packet)); + /* Note: we assume htlc's lifetime is greater than ours, + * so we just share pointers and don't bother copying */ + a->path_key = htlc->path_key; + a->extra_tlvs = htlc->extra_tlvs; + a->fail_immediate = htlc->fail_immediate; tal_arr_expand(added, a); } else if (htlc->state == RCVD_REMOVE_COMMIT) { if (htlc->r) { @@ -1629,7 +1631,7 @@ static void send_revocation(struct peer *peer, struct changed_htlc *changed; struct fulfilled_htlc *fulfilled; const struct failed_htlc **failed; - struct added_htlc *added; + const struct added_htlc **added; const u8 *msg; const u8 *msg_for_master; diff --git a/common/htlc_wire.c b/common/htlc_wire.c index 49c8211de..bb17e738b 100644 --- a/common/htlc_wire.c +++ b/common/htlc_wire.c @@ -216,9 +216,10 @@ static struct tlv_field *fromwire_len_and_tlvstream(const tal_t *ctx, return tlvs; } -void fromwire_added_htlc(const u8 **cursor, size_t *max, - struct added_htlc *added) +struct added_htlc *fromwire_added_htlc(const tal_t *ctx, const u8 **cursor, + size_t *max) { + struct added_htlc *added = tal(ctx, struct added_htlc); added->id = fromwire_u64(cursor, max); added->amount = fromwire_amount_msat(cursor, max); fromwire_sha256(cursor, max, &added->payment_hash); @@ -235,6 +236,7 @@ void fromwire_added_htlc(const u8 **cursor, size_t *max, } else added->extra_tlvs = NULL; added->fail_immediate = fromwire_bool(cursor, max); + return added; } struct existing_htlc *fromwire_existing_htlc(const tal_t *ctx, diff --git a/common/htlc_wire.h b/common/htlc_wire.h index 5cc94a985..2ef90e9e8 100644 --- a/common/htlc_wire.h +++ b/common/htlc_wire.h @@ -86,8 +86,8 @@ void towire_changed_htlc(u8 **pptr, const struct changed_htlc *changed); void towire_side(u8 **pptr, const enum side side); void towire_shachain(u8 **pptr, const struct shachain *shachain); -void fromwire_added_htlc(const u8 **cursor, size_t *max, - struct added_htlc *added); +struct added_htlc *fromwire_added_htlc(const tal_t *ctx, const u8 **cursor, + size_t *max); struct existing_htlc *fromwire_existing_htlc(const tal_t *ctx, const u8 **cursor, size_t *max); void fromwire_fulfilled_htlc(const u8 **cursor, size_t *max, diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 454eb8dcf..27c3a4757 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -2309,7 +2309,7 @@ static bool channel_added_their_htlc(struct channel *channel, /* The peer doesn't tell us this separately, but logically it's a separate * step to receiving commitsig */ static bool peer_sending_revocation(struct channel *channel, - struct added_htlc *added, + struct added_htlc **added, struct fulfilled_htlc *fulfilled, struct failed_htlc **failed, struct changed_htlc *changed) @@ -2317,7 +2317,7 @@ static bool peer_sending_revocation(struct channel *channel, size_t i; for (i = 0; i < tal_count(added); i++) { - if (!update_in_htlc(channel, added[i].id, SENT_ADD_REVOCATION)) + if (!update_in_htlc(channel, added[i]->id, SENT_ADD_REVOCATION)) return false; } for (i = 0; i < tal_count(fulfilled); i++) { @@ -2364,7 +2364,7 @@ void peer_got_commitsig(struct channel *channel, const u8 *msg) struct fee_states *fee_states; struct height_states *blockheight_states; struct bitcoin_signature commit_sig, *htlc_sigs; - struct added_htlc *added; + struct added_htlc **added; struct fulfilled_htlc *fulfilled; struct failed_htlc **failed; struct changed_htlc *changed; @@ -2439,7 +2439,7 @@ void peer_got_commitsig(struct channel *channel, const u8 *msg) /* New HTLCs */ for (i = 0; i < tal_count(added); i++) { - if (!channel_added_their_htlc(channel, &added[i])) + if (!channel_added_their_htlc(channel, added[i])) return; } diff --git a/tests/test_pay.py b/tests/test_pay.py index 1d3b93538..151d56036 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -7016,7 +7016,6 @@ def test_sendonion_sendpay(node_factory, bitcoind): assert invoice["amount_received_msat"] == Millisatoshi(total_amount) -@pytest.mark.xfail(strict=True) def test_htlc_tlv_crash(node_factory): """Marshalling code treated an array of htlc_added as if they were tal objects, but only the head is a tal object so if we have more than one, BOOM!""" plugin = os.path.join(os.path.dirname(__file__), 'plugins/htlc_accepted-customtlv.py') diff --git a/tools/generate-wire.py b/tools/generate-wire.py index dd6c74249..e78d68c5a 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -230,6 +230,7 @@ class Type(FieldSet): 'gossip_getnodes_entry', 'gossip_getchannels_entry', 'failed_htlc', + 'added_htlc', 'existing_htlc', 'inflight', 'hsm_utxo', diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 92ec32bc3..c1cd3b066 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -300,7 +300,7 @@ struct channel_type *fromwire_channel_type(const tal_t *ctx UNNEEDED, const u8 * bool fromwire_channeld_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED) { fprintf(stderr, "fromwire_channeld_dev_memleak_reply called!\n"); abort(); } /* Generated stub for fromwire_channeld_got_commitsig */ -bool fromwire_channeld_got_commitsig(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *commitnum UNNEEDED, struct fee_states **fee_states UNNEEDED, struct height_states **blockheight_states UNNEEDED, struct bitcoin_signature *signature UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, struct added_htlc **added UNNEEDED, struct fulfilled_htlc **fulfilled UNNEEDED, struct failed_htlc ***failed UNNEEDED, struct changed_htlc **changed UNNEEDED, struct bitcoin_tx **tx UNNEEDED, struct commitsig ***inflight_commitsigs UNNEEDED) +bool fromwire_channeld_got_commitsig(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u64 *commitnum UNNEEDED, struct fee_states **fee_states UNNEEDED, struct height_states **blockheight_states UNNEEDED, struct bitcoin_signature *signature UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, struct added_htlc ***added UNNEEDED, struct fulfilled_htlc **fulfilled UNNEEDED, struct failed_htlc ***failed UNNEEDED, struct changed_htlc **changed UNNEEDED, struct bitcoin_tx **tx UNNEEDED, struct commitsig ***inflight_commitsigs UNNEEDED) { fprintf(stderr, "fromwire_channeld_got_commitsig called!\n"); abort(); } /* Generated stub for fromwire_channeld_got_revoke */ bool fromwire_channeld_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 height_states **blockheight_states UNNEEDED, struct changed_htlc **changed UNNEEDED, struct penalty_base **pbase UNNEEDED, struct bitcoin_tx **penalty_tx UNNEEDED)