Commit Graph

254 Commits

Author SHA1 Message Date
Rusty Russell 262e4c840f sphinx: use struct secret for shared secret.
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>
2020-01-24 10:01:44 +10:30
Rusty Russell 1099f6a5e1 common: use struct onionreply.
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>
2020-01-23 16:17:42 +10:30
Rusty Russell ddce5573c7 channeld: use wirestring for failure strings.
I think this code predated wirestring.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-01-23 16:17:42 +10:30
Rusty Russell 9a72016640 lightningd: remove unused allocation.
Not a leak, since it's off tmpctx, but send_htlc_out allocates this itself.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-01-23 16:17:42 +10:30
Christian Decker 4be1868b8a pay: Invert ownership of wallet_payment
`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.
2020-01-13 23:34:46 +01:00
Vasil Dimov 2ea91f834c Add the missing space between "if" and "("
Changelog-None
2020-01-06 12:57:59 +01:00
Rusty Russell 72aa315b5e lightningd: save the fee_states into the database.
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>
2019-12-12 22:15:48 +01:00
Rusty Russell 8e3234e67a lightningd: sew in htlc set.
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>
2019-12-12 15:03:53 +01:00
Rusty Russell 8b129b439b lightningd: cleanup redundant args from handle_localpay
The cltv_expiry and payment_hash are in hin, so no need to hand them
in here.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-12-12 15:03:53 +01:00
Rusty Russell 0e4a30c635 doc: update experimental bolt version quotes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-12-12 15:03:53 +01:00
Rusty Russell 12985331f7 htlcs: remove origin_htlc_id from htlc_out.
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>
2019-12-12 15:03:53 +01:00
Rusty Russell 345ca9b122 db: add partid field to htlc_out.
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>
2019-12-12 15:03:53 +01:00
Rusty Russell 2d18c3a209 db: add partid, total_msat fields to payment entries.
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>
2019-12-12 15:03:53 +01:00
Rusty Russell d56513362a lightningd: don't always defer commitment_signed if we're not synced.
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>
2019-12-11 16:20:50 +01:00
Christian Decker ff5f7b194f sphinx: Return the error in parse_onionpacket
As suggested by @niftynei here: https://github.com/ElementsProject/lightning/pull/3260#discussion_r347543999

Suggested-by: Lisa Neigut <@niftynei>
Suggested-by: Rusty Russell <@rustyrussell>
Signed-off-by: Christian Decker <@cdecker>
2019-12-11 16:18:34 +01:00
Rusty Russell f7ebbb2ec5 common: make sphinx code ignorant of payload format.
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>
2019-12-09 14:33:31 +01:00
Rusty Russell bb538a1862 common: don't crash on bad sphinx payload.
It's cleanest to eliminate the SPHINX_INVALID_PAYLOAD altogether.

lightning_channeld: FATAL SIGNAL (version v0.7.3-242-gb1583bb-modded)
0x55a8169eed08 send_backtrace
	common/daemon.c:41
0x55a8169fc3eb status_failed
	common/status.c:206
0x55a8169fc657 status_backtrace_exit
	common/subdaemon.c:25
0x55a8169eedbb crashdump
	common/daemon.c:57
0x7f0eaff8446f ???
	???:0
0x7f0eaff843eb ???
	???:0
0x7f0eaff63898 ???
	???:0
0x55a8169fb29f route_step_decode
	common/sphinx.c:759
0x55a8169fb60a process_onionpacket
	common/sphinx.c:834
0x55a8169d9b34 get_shared_secret
	channeld/channeld.c:605
0x55a8169d9d35 handle_peer_add_htlc
	channeld/channeld.c:649
0x55a8169dd88d peer_in
	channeld/channeld.c:1838
0x55a8169e11a8 main
	channeld/channeld.c:3233
0x7f0eaff651e2 ???

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-12-09 14:33:31 +01:00
Christian Decker 69c17d2d31 wire: Let the TLV _is_valid function actually return validity
I got this one wrong myself, since the function name implied a boolean
result. So I changed it to take the optional err_index as argument.
2019-12-03 00:37:15 +00:00
Rusty Russell e5247a68b6 lightningd: check payment secret on htlc receipt.
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>
2019-11-24 23:33:17 +00:00
Rusty Russell ebac3d2a85 spec: update to experimental BOLTs with secret/total_amount.
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>
2019-11-24 23:33:17 +00:00
Rusty Russell 50d6941e89 lightningd: remove redundant htlc_accepted_hook_payload fields
Now we cache them in the route_step, don't need to copy them here.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-24 23:33:17 +00:00
Christian Decker d1df4d6959 htlc: Add a checker function tellung us whether we can continue
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>
2019-11-22 04:40:25 +00:00
Christian Decker d7b28ac480 htlc: Consolidate validation after the htlc_accepted hook returns
This now enforces all rules for validity, both for the TLV format and checking
that the required fields have been provided.
2019-11-22 04:40:25 +00:00
Christian Decker fc14e5eab0 htlcs: Make necessary payload fields optional and derfer validation
We make the fields in `htlc_accepted_payload` optional (NULL if not present in
the payload) and defer validation till after the hook call.
2019-11-22 04:40:25 +00:00
Christian Decker d69a43780c sphinx: Use the new `fromwire_tlv_payload` function
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.
2019-11-22 04:40:25 +00:00
darosior f075b87137 bitcoind: remove the chainparams member
We now have a global constant, prefer to use it instead of having
two variables with the same utility.
2019-11-15 13:14:08 +01:00
Rusty Russell 323e4f6288 dev: add option to prevent HTLC timeouts.
This is required for the protocol tests, which can be slow.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-14 10:19:01 +01:00
Rusty Russell 2a2259083a lightningd: handle tlv-style payloads.
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.
2019-11-14 10:15:33 +01:00
Rusty Russell a76518a029 common/sphinx: rename hop_data to hop_data_legacy.
This highlights the various places we need to change.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-14 10:15:33 +01:00
Rusty Russell 21d2cc663b lightningd: apply feerate changes correctly.
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>
2019-10-28 13:07:41 -05:00
Rusty Russell f019dc3d71 lightningd: fix sizeof() argument correctly.
c25ce826ab claimed to fix this, but didn't;
this is the correct fix.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-10-08 09:51:22 -05:00
Yash Bhutwala c25ce826ab take the size of 'shared_secret' itself rather than its address 2019-10-07 11:32:33 -05:00
Rusty Russell fa686c5ca7 channeld: reject wumbo payments with more style.
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>
2019-10-03 23:27:23 +00:00
Rusty Russell 049529542a lightningd: delay reprocessing of incoming htlcs at startup until plugins ready.
Fixes: #2923
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-01 16:55:32 +02:00
Rusty Russell 189b2f1313 BOLT: update CSV to latest bolt version.
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>
2019-08-29 09:01:48 +02:00
Rusty Russell 2600a6ed2e channeld: get current block height when an HTLC fails.
We need it to put in the error code for
WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-29 09:01:48 +02:00
Rusty Russell 6349222ea2 Spec: Update to latest BOLT, include our first global feature definition.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-29 09:01:48 +02:00
trueptolemy 4929034a40 json: Make payment_hash use `json_add_sha256` 2019-08-21 09:32:21 +08:00
Rusty Russell f18b911032 lightningd: listforwards shouldn't put in zero fields for fields we don't know.
Technically, this is an API change :(  So I made it conditional.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-15 03:12:56 +00:00
lisa neigut 802ebe768c rpc: fix crash 'listforwards' when payment_hash is empty
```
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): FATAL SIGNAL 11 (version v0.7.2rc1)
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: common/daemon.c:45 (send_backtrace) 0x563349d07879
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: common/daemon.c:53 (crashdump) 0x563349d078c9
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: (null):0 ((null)) 0x7efd7b996f1f
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: ccan/ccan/str/hex/hex.c:59 (hex_encode) 0x563349d57fec
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: lightningd/json.c:380 (json_add_hex) 0x563349cd9dd3
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: lightningd/peer_htlcs.c:2151 (json_format_forwarding_object) 0x563349cfa7ac
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: lightningd/peer_htlcs.c:2198 (listforwardings_add_forwardings) 0x563349cfa99d
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: lightningd/peer_htlcs.c:2216 (json_listforwards) 0x563349cfaa55
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: lightningd/jsonrpc.c:650 (parse_request) 0x563349cdc184
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: lightningd/jsonrpc.c:748 (read_json) 0x563349cdc5ae
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: ccan/ccan/io/io.c:59 (next_plan) 0x563349d4bbe5
2019-08-14T17:50:39.100Z **BROKEN** lightningd(11355): backtrace: ccan/ccan/io/io.c:407 (do_plan) 0x563349d4c762
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: ccan/ccan/io/io.c:417 (io_ready) 0x563349d4c7a0
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: ccan/ccan/io/poll.c:445 (io_loop) 0x563349d4e7f5
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: lightningd/io_loop_with_timers.c:24 (io_loop_with_timers) 0x563349cd8afe
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: lightningd/lightningd.c:834 (main) 0x563349cded3a
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: (null):0 ((null)) 0x7efd7b979b96
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: (null):0 ((null)) 0x563349cc5909
2019-08-14T17:50:39.101Z **BROKEN** lightningd(11355): backtrace: (null):0 ((null)) 0xffffffffffffffff
```

[ Modified to simply omit field --RR ]
2019-08-15 03:12:56 +00:00
lisa neigut 58fb1528dd add_htlc hook: fix crash when failing UPDATE failcode
Passing in an UPDATE failcode crashes, since the next hop's channel id
was passed in as NULL. Fixed by passing in id.

```
2019-08-15T00:19:49.639Z **BROKEN** lightningd(17070): FATAL SIGNAL 11 (version v0.7.2rc1-8-gbf3b77a-modded)
2019-08-15T00:19:49.639Z **BROKEN** lightningd(17070): backtrace: common/daemon.c:45 (send_backtrace) 0x55fef4ef036f
2019-08-15T00:19:49.639Z **BROKEN** lightningd(17070): backtrace: common/daemon.c:53 (crashdump) 0x55fef4ef03bf
2019-08-15T00:19:49.639Z **BROKEN** lightningd(17070): backtrace: (null):0 ((null)) 0x7f7762401f1f
2019-08-15T00:19:49.639Z **BROKEN** lightningd(17070): backtrace: lightningd/peer_htlcs.c:104 (fail_in_htlc) 0x55fef4edd9d7
2019-08-15T00:19:49.639Z **BROKEN** lightningd(17070): backtrace: lightningd/peer_htlcs.c:785 (htlc_accepted_hook_callback) 0x55fef4edf2c7
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: lightningd/plugin_hook.c:86 (plugin_hook_callback) 0x55fef4ee765f
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: lightningd/plugin.c:251 (plugin_response_handle) 0x55fef4ee44b2
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: lightningd/plugin.c:341 (plugin_read_json_one) 0x55fef4ee4637
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: lightningd/plugin.c:366 (plugin_read_json) 0x55fef4ee4764
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: ccan/ccan/io/io.c:59 (next_plan) 0x55fef4f38c7a
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: ccan/ccan/io/io.c:407 (do_plan) 0x55fef4f397f7
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: ccan/ccan/io/io.c:417 (io_ready) 0x55fef4f39835
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: ccan/ccan/io/poll.c:445 (io_loop) 0x55fef4f3b88a
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: lightningd/io_loop_with_timers.c:24 (io_loop_with_timers) 0x55fef4ec0afe
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: lightningd/lightningd.c:834 (main) 0x55fef4ec6f5a
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: (null):0 ((null)) 0x7f77623e4b96
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: (null):0 ((null)) 0x55fef4ead909
2019-08-15T00:19:49.640Z **BROKEN** lightningd(17070): backtrace: (null):0 ((null)) 0xffffffffffffffff
```
2019-08-15 02:24:18 +00:00
Rusty Russell c3a35416da lightningd: don't allow channeld to accept HTLCs if we're not synced.
We want to still allow incoming connections, and reestablishment of
channels, but if one tries to give us an HTLC, stall until we're
synced.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-10 22:09:09 +02:00
Rusty Russell 6195a878f7 lightningd: don't allow sending of HTLCs while still syncing.
If we don't know block height, we shouldn't be sending HTLCs.  This
stops us forwarding HTLCs as well as new payments.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-10 22:09:09 +02:00
Rusty Russell b73a85a75e lightningd: don't say 'killing channel' when HTLC times out.
We're actually only killing the connection.  I saw this in my logs,
but it was all OK.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-07 21:12:52 +08:00
Rusty Russell 02609773c0 lightningd: suppress gcc-7.4.0 error
In file included from wallet/test/run-wallet.c:15:0:
./lightningd/peer_htlcs.c: In function ‘htlcs_reconnect’:
./lightningd/peer_htlcs.c:2060:15: error: ‘failcode’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   }  else if (failcode) {
               ^~~~~~~~
./lightningd/peer_htlcs.c:2056:19: error: ‘failcode’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
          failcode != 0
          ~~~~~~~~~^~~~

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-02 15:56:15 +02:00
trueptolemy a449a91ae2 JSON: Warp the process of forward payment json object
Warp this process as a new function: 'void json_format_forwarding_object()'. This function will be used in 'forward_event' next, and can ensure the consistent json object structure for forward_payment between 'listforwards' API and 'forward_event' notification.
2019-08-01 18:49:25 +08:00
trueptolemy bcec6bb6cc API: 'listforwards' now include 'payment_hash' field
'payment_hash' can help users learn more about the forward payment.
2019-08-01 18:49:25 +08:00
Christian Decker 5dff67900e tx: Add chainparams when deserializing transactions from wire msgs
This is the other origin, besides `bitcoin_tx`, where we create `bitcoin_tx`
instances, so add the context as soon as possible. Sadly I can't weave the
chainparams into the deserialization code since that'd need to change all the
generated wire code as well.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-07-31 23:22:54 +00:00
Christian Decker b83d15ea4a sphinx: Remove standalone v0 payload in favor of the unionized one
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-07-30 02:14:49 +00:00
lisa neigut 5c07afac7d bolt: update to BOLT spec changes (extract format + type specifications)
updates the bolt version to 6639cef095a2ecc7b8f0c48c6e7f2f906fbfbc58.

this requires us to use the new bolt parser at generate-bolt.py
and updates to all of the type specifications (ie. from u8 -> byte)
2019-07-16 06:10:58 +00:00