Turn it into temporary node failure: this only happens if we restart
with a failed htlc in, but it's clearer and more robust to handle it
generically.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For incoming htlcs, we need failure details in case we need to
re-xmit them. But for outgoing htlcs, lightningd is telling us it
already knows they've failed, so we just need to flag them failed
and don't need the details.
Internally, we set the ->fail to a dummy non-NULL value; this is
cleaned up next.
This matters for the next patch, which moves onion handling into
lightningd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. forward_htlc sets hout to NULL.
2. forward_htlc passes &hout to send_htlc_out.
3. forward_htlc checks the failcode and frees(NULL) and sets hout to NULL
(again). This in fact covers every failcode which send_htlc_out returns.
We should ensure send_htlc_out sets *houtp to NULL on failure; in fact,
both callers pass houtp, so we can make it unconditional.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the peer is not connected, or other error which means we don't
actually create an outgoing HTLC, we don't record the
short_channel_id. This is unhelpful!
Pass the scid down to the wallet code, and explicitly hand the
scid and amount down to the notification code rather than handing it
the htlc_out (which it doesn't need).
Changelog-Changed: JSON API: `listforwards` now shows `out_channel` even if we couldn't forward.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make the `htlc_accepted` hook the first chained hook in our repertoire. The
plugins are called one after the other in order until we have no more plugins
or the HTLC was handled by one of the plugins. If no plugins handles the HTLC
we continue to handle it internally like always.
Handling in this case means the plugin returns either `{"result": "resolve",
...}` or `{"result": "fail", ...}`.
Changelog-Changed: plugin: Multiple plugins can now register for the htlc_accepted hook.
The newly introduced type is used to determine what the call semantics of the
hook are. We have `single` corresponding to the old behavior, as well as
`chain` which allows multiple plugins to register for the hook, and they are
then called sequentially (if all plugins return `{"result": "continue"}`) or
exit the chain if the hook event was handled.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Relative plugin paths are not relative to startup (deprecated v0.7.2.1)
Changelog-Removed: Dummy fields in listforwards (deprecated v0.7.2.1)
This shouldn't happen if channeld is working properly, but I'm going to
change that, and this current code means we stop responding at that point
(not every failpath in peer_accepted_htlc() called channel_internal_error).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Generally I prefer structures over u8, since the size is enforced at
runtime; and in several places we were doing conversions as the code
using Sphinx does treat struct secret as type of the secret.
Note that passing an array is the same as passing the address, so
changing from 'u8 secret[32]' to 'struct secret secret' means various
'secret' parameters change to '&secret'. Technically, '&secret' also
would have worked before, since '&' is a noop on array, but that's
always seemed a bit weird.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes it clear we're dealing with a message which is a wrapped error
reply (needing unwrap_onionreply), not an already-wrapped one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`wallet_payment_store` would free the `wallet_payment` instance which would
then cause us to reload it from the DB. Instead of doing the store->free->load
dance we now tell `wallet_payment_store` whether it should take ownership and
leave it alone if not.
Passing the payment around instead of referencing it through payment_hash and
partid is a nice side-effect.
This is the final step: we pass the complete fee_states to and from
channeld.
Changelog-Fixed: "Bad commitment signature" closing channels when we sent back-to-back update_fee messages across multiple reconnects.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The invoice_try_pay code now takes a set, rather than a single htlc, but
it's basically the same thing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a transient field, so rework things so we don't leave it in
struct htlc_out. Instead, load htlc_in first and connect htlc_out to
them as we go.
This also changes one place where we use it instead of the am_origin
flag.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is in preparation for partial payments. For existing payments,
partid is 0 (to match the corresponding payment).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is in preparation for partial payments. For existing payments,
partid is 0 (arbitrarity) and total_msat is msatoshi.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Because my node runs under valgrind, it can take quite a while to
sync; nodes tend to disconnect and reconnect if you block too long.
This is particularly problematic since we often update fees: when the
other side sends its commitment_signed we block.
In particular, this triggers the corner case we have where we
update_fee twice, disconnecting each time, and our state machine gets
confused (which is why we never saw this exact corner case before this
change in 0.7.3!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now "raw_payload" is always the complete string (including realm or length
bytes at the front).
This has several effects:
1. We can receive an decrypt an onion which is grossly malformed.
2. We can still hand this to the htlc_accepted hook.
3. We then fail it unless the htlc_accepted accepts it manually.
4. The createonion API now takes the raw payload, and does not know
anything about "style".
The only caveat is that the sphinx code needs to know the payload
length: we have a call for that, which simply tells it to copy the
entire onion (and treat us as the final node) if it's invalid.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't set the secret to compulsory (yet!) but put code in for the
future. Meanwhile, if there is a secret, check it is correct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Also pulls in a new onion error (mpp_timeout). We change our
route_step_decode_end() to always return the total_msat and optional
secret.
We check total_amount (to prohibit mpp), but we do nothing with
secret for now other than hand it to the htlc_accepted hook.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This function ensures we have all the infos we need to continue if the
htlc_accepted hook tells us to. It also enforces well-formedness of the TLV
payload if we have a TLV payload.
Suggested-by: List Neigut <@niftynei>
Signed-off-by: Christian Decker <@cdecker>
We wire in the code-generated function, which removes the upfront validation
and add the validation back after the `htlc_accepted` hook returns. If a
plugin wanted to handle the onion in a special way it'll not have told us to
just continue.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-changed: JSON API: `htlc_accepted` hook has `type` (currently `legacy` or `tlv`) and other fields directly inside `onion`.
Changelog-deprecated: JSON API: `htlc_accepted` hook `per_hop_v0` object deprecated, as is `short_channel_id` for the final hop.
Feerate changes are asymmetric, as they can only be sent by the funder.
For FUNDER, the remote feerate is set when upon send of
commitment_signed, and the local feerate is set on receipt of
revoke_and_ack.
For non-funder, the local feerate is set on receipt of
commitment_signed, and the remote feerate set on send of
revoke_and_ack. In our code, these two happen together.
channeld gets this right, but lightningd ignored the funder/fundee
distinction, and as a result, receipt of a commitment_signed by the
funder altered fees in the database. If there was a reconnection
event or restart, then these (incorrect) values would be used, causing
us to complain about a 'Bad commit_sig signature' and close the
channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
WIRE_REQUIRED_CHANNEL_FEATURE_MISSING anticipates a glorious Wumbo future,
and is closer to correct (it's a PERM failure).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This removes the WIRE_FINAL_EXPIRY_TOO_SOON which leaked too much info,
and adds the blockheight to WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
