We're going to want this for bolt13 formation as well.
As a result of reworking the logic into "candidate selection" then
"route hint selection", we need to change the way round-robin works.
We use a simple incrementing index now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that other directories were explicitly depending on the generated
file, instead of relying on their (already existing) dependency on
$(LIGHTNINGD_HSM_CLIENT_OBJS), so we remove that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You can't pay them anyway, and at least one person used 0 instead of "any".
Closes: #3808
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `invoice` no longer accepts zero amounts (did you mean "any"?)
It's not all that rare to do these operations, and requiring annotations
for it is a little painful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `delinvoice` will now report specific error codes: 905 for failing to find the invoice, 906 for the invoice status not matching the parameter.
One is called on every plugin return, and tells us whether to continue;
the other is only called if every plugin says ok.
This works for things like payload replacement, where we need to process
the results from each plugin, not just the final one!
We should probably turn everything into a chained callback next
release.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They callback must take ownership of the payload (almost all do, but
now it's explicit).
And since the payload and cb_arg arguments to plugin_hook_call_() are
always identical, make them a single parameter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes testing easier, and makes sense: lightningd might not
*know* about other connected channels, depending on gossip, but if the
user specifies it we should obey it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON: `invoice` `exposeprivatechannels` now includes explicitly named channels even if they seem like dead-ends.
It's almost always "their_features" and "our_features" respectively, so
make those names clear.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Turns out that unnecessary: all callers can access the feature_set,
so make it much more like a normal primitive.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Unfortunately the invoice_payment_hook can give us a failcode, so I simply
restrict it to the two sensible ones.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-deprecated: plugins: invoice_payment_hook "failure_code" only handles simple cases now, use "failure_message".
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.
Changelog-Changed: The hooks `db_write`, `invoice_payment`, and `rpc_command` now accept `{ "result": "continue" }` to mean "do default action", in addition to `true` (`db_write`), `{}` (`invoice_payment`), and `{"continue": true}` (`rpc_command`). The older "default" indicators are now deprecated and are now recognized only if `--deprecated-apis` is set.
We tag them with specific versions when they're experimental,
but do a poor job of cleaning them up (and thus ensuring they're
checked!) afterwards.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Before this patch we used `int` for error codes. The problem with
`int` is that we try to pass it to/from wire and the size of `int` is
not defined by the standard. So a sender with 4-byte `int` would write
4 bytes to the wire and a receiver with 2-byte `int` (for example) would
read just 2 bytes from the wire.
To resolve this:
* Introduce an error code type with a known size:
`typedef s32 errcode_t`.
* Change all error code macros to constants of type `errcode_t`.
Constants also play better with gdb - it would visualize the name of
the constant instead of the numeric value.
* Change all functions that take error codes to take the new type
`errcode_t` instead of `int`.
* Introduce towire / fromwire functions to send / receive the newly added
type `errcode_t` and use it instead of `towire_int()`.
In addition:
* Remove the now unneeded `towire_int()`.
* Replace a hardcoded error code `-2` with a new constant
`INVOICE_EXPIRED_DURING_WAIT` (903).
Changelog-Changed: The waitinvoice command would now return error code 903 to designate that the invoice expired during wait, instead of the previous -2
Fixes: #3192
Changelog-Added: `waitanyinvoice` now supports a `timeout` parameter, which when set will cause the command to fail when the timeout is reached; can set this to 0 to fail immediately if no new invoice has been paid yet.
This lets us do more flexible filtering in the next patch. But it also
keeps some weird logic out of gossipd.
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>
We now return the same error for various "does not match this
invoice", so it makes sense to encapsulate these checks. We'll also
want to expose this for multi-part payments.
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>
In a future version, we will use features to insist that payers
provide the secret. In transition, we may have old invoices which
didn't insist on that, so we need to know this on a per-invoice basis.
Not sure if I got the right syntax for adding an empty blob though!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The spec says to close the channel if they send us an error, but we
need to be more lenient to preserve channels with other
implementations.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"result" should always be an object (so that we can add new fields),
so make that implicit in json_stream_success.
This makes our primitives well-formed: we previously used NULL as our
fieldname when calling the first json_object_start, which is a hack
since we're actually in an object and the fieldname is 'result' (which
was already written by json_object_start).
There were only two cases which didn't do this:
1. dev-memdump returned an array. No API guarantees on this.
2. shutdown returned a string.
I temporarily made shutdown return an empty object, which shouldn't
break anything, but I want to fix that later anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are generalized from our internal implementations.
The main difference is that 'struct json_escaped' is now 'struct
json_escape', so we replace that immediately.
The difference between lightningd's json-writing ringbuffer and the
more generic ccan/json_out is that the latter has a better API and
handles escaping transparently if something slips through (though
it does offer direct accessors so you can mess things up yourself!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
A new string field is added to the command structure and is specified at the creation of each native command, and in the JSON created by 'json_add_help_command()'.
New fields don't have to be spelled out twice.
The raw version are called _only, so we don't miss a call
accidentally. We can rename them when we finally deprecated old
fields.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The chainparams are needed to know the prefixes, so instead of passing down
the testnet, we pass the entire params struct.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
I misunderstood the API, this ended up nesting a result inside the JSON-RPC
result.
No concerns about backwards compatibility since this is so new.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This seems like overkill, at least for now. Handling the JSON
inline is clearer, for the existing examples at least.
We also remove the dummy hook, rather than fix it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Refactored the weighted-reservoir-sampling algo to make it more straightforward.
It now uses the excess as fraction of capacity as weight. This favors channels that
are more _relatively_ unbalanced to be used for incoming payment.
Now passes test_invoice_routeboost_private() when using max fundamount=16777215.
This also allows plugins to do "hold invoices" a-la LND, useful for
just-in-time inventory handling.
We're careful to handle the invoice getting paid behind our backs, and
the incoming HTLC going away.
Once @cdecker's sphinx rework is in, we can also hand the raw payload
to the invoice_payment_hook, for special effects.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to make it async, so start by moving the core code into
invoice.c and having that directly call fail/success functions for the
htlc.
We add an extra check in fulfill_htlc() that the HTLC state is correct:
that can't happen now, but may once we're async.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For online services, shorter may be fine, but for casual use I'm usually
in a different timezone than the payer, so needs to be at least 1 day.
Certainly 1 hr is short if they have to open a channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I tried to just do gossipd, but it was uncontainable, so this ended up being
a complete sweep.
We didn't get much space saving in gossipd, even though we should save
24 bytes per node.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to do it in various places, but we shouldn't do it lightly:
the primitives are there to help us get overflow handling correct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As a side-effect of using amount_msat in gossipd/routing.c, we explicitly
handle overflows and don't need to pre-prune ridiculous-fee channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current param_sat accepts "any": rename and move that to invoice.c
where it's called. We rename it to param_msat_or_any and invoice.c
is our first (trivial) amount_msat user.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to still accept it when parsing the database, but this flag
should allow upgrade testing for devs building on top
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian points out that we can iterate by ->size rather than calling
json_next() to find the end (which traverses the entire object!).
Now ->size is reliable (since previous patch), this is OK.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian and I both unwittingly used it in form:
*tal_arr_expand(&x) = tal(x, ...)
Since '=' isn't a sequence point, the compiler can (and does!) cache
the value of x, handing it to tal *after* tal_arr_expand() moves it
due to tal_resize().
The new version is somewhat less convenient to use, but doesn't have
this problem, since the assignment is always evaluated after the
resize.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Obviously the Facebook relationship status joke was a bit subtle, but I've
continued it anyway because I'm especially susceptible to Dad jokes.
Suggested-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This causes a compiler warning if we don't do something with the
result (hopefully return immediately!).
We use was_pending() to ignore the result in the case where we
complete a command in a callback (thus really do want to ignore
the result).
This actually fixes one bug: we didn't return after command_fail
in json_getroute with a bad seed value.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Usually, this means they return 'command_param_failed()' if param()
fails, and changing 'command_success(); return;' to 'return
command_success()'.
Occasionally, it's more complex: there's a command_its_complicated()
for the case where we can't exactly determine what the status is,
but it should be considered a last resort.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Handers of a specific form are both designed to be used as callbacks
for param(), and also dispose of the command if something goes wrong.
Make them return the 'struct command_result *' from command_failed(),
or NULL.
Renaming them just makes sense: json_tok_XXX is used for non-command-freeing
parsers too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_escaped.[ch], param.[ch] and jsonrpc_errors.h move from lightningd/
to common/. Tests moved too.
We add a new 'common/json_tok.[ch]' for the common parameter parsing
routines which a plugin might want, taking them out of
lightningd/json.c (which now only contains the lightningd-specific
ones).
The rest is mainly fixing up includes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This (will) avoid the plugin having to walk back from the params object
as it currently does.
No code changes; I removed UNUSED and UNNEEDED labels from the other
parameters though (as *every* json_rpc callback needs to call param()
these days, they're *always* used).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Such an API is required for when we stream it directly. Almost all our
handlers fit this pattern already, or nearly do.
We remove new_json_result() in favor of explicit json_stream_success()
and json_stream_fail(), but still allowing command_fail() if you just
want a simple all-in-one fail wrapper.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's the only user of them, and it's going to get optimized.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossip.pydiff --git a/common/test/run-json.c b/common/test/run-json.c
index 956fdda35..db52d6b01 100644
We split json_invoice(), as it now needs to round-trip to the gossipd,
and uniqueness checks need to happen *after* gossipd replies to avoid
a race.
For every candidate channel gossipd gives us, we check that it's in
state NORMAL (not shutting down, not still waiting for lockin), that
it's connected, and that it has capacity. We then choose one with
probability weighted by excess capacity, so larger channels are more
likely.
As a side effect of this, we can tell if an invoice is unpayble (no
channels have sufficient incoming capacity) or difficuly (no *online*
channels have sufficient capacity), so we add those warnings.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do this a lot, and had boutique helpers in various places. So add
a more generic one; for convenience it returns a pointer to the new
end element.
I prefer the name tal_arr_expand to tal_arr_append, since it's up to
the caller to populate the new array entry.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
That matches the other CSV names (HSM was the first, so it was written
before the pattern emerged).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This could have been a local static but its used by the run-param test,
so putting it in json.c made things easier.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
This was a very simple change and allowed us to remove the special
`json_opt_tok` macro.
Moved the callback out of `common/json.c` to `lightningd/json.c` because the new
callbacks are dependent on `struct command` etc.
(I already started on `json_tok_number`)
My plan is to:
1. upgrade json_tok_X one a time, maybe a PR for each one.
2. When done, rename macros (i.e, remove "_tal").
3. Remove all vestiges of the old callbacks
4. Add new callbacks so that we no longer need json_tok_tok!
(e.g., json_tok_label, json_tok_str, json_tok_msat)
Signed-off-by: Mark Beckwith <wythe@intrig.com>
tal_count() is used where there's a type, even if it's char or u8, and
tal_bytelen() is going to replace tal_len() for clarity: it's only needed
where a pointer is void.
We shim tal_bytelen() for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This seems like a premature optimization: it tried to cut down the number of
allocations by reusing the same `struct invoice_details` while iterating through
a number of results. But this sidesteps the checks by `valgrind` and we'd miss a
missing field that was set by the previous iteration.
Reported-by: @rustyrussell
Signed-off-by: Christian Decker <@cdecker>
connect_control.c, dev_ping.c, gossip_control.c, invoice.c.
This converts about 50% of all calls of `json_get_params` to `param`.
After trying (and failing) to squash and rebase #1682 I just made a new branch
from a patch file and closed#1682.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
I crashed the HSMD, and it gave no output at all. That's because we
were only reading the status fd when we were waiting for a reply.
Fix this by using a separate request fd and status fd, which also means
that hsm_sync_read() is no longer required.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
structeq() is too dangerous: if a structure has padding, it can fail
silently.
The new ccan/structeq instead provides a macro to define foo_eq(),
which does the right thing in case of padding (which none of our
structures currently have anyway).
Upgrade ccan, and use it everywhere. Except run-peer-wire.c, which
is only testing code and can use raw memcmp(): valgrind will tell us
if padding exists.
Interestingly, we still declared short_channel_id_eq, even though
we didn't define it any more!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
satoshis.place was slowing to a crawl, c-lightning was unresponsive.
Logs revealed charged doing many, many listinvoice <label> RPCs.
We were iterating the entire db every time: stop that!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Until now, `command_fail()` reported an error code of -1 for all uses.
This PR adds an `int code` parameter to `command_fail()`, requiring the
caller to explicitly include the error code.
This is part of #1464.
The majority of the calls are used during parameter validation and
their error code is now JSONRPC2_INVALID_PARAMS.
The rest of the calls report an error code of LIGHTNINGD, which I defined to
-1 in `jsonrpc_errors.h`. The intention here is that as we improve our error
reporting, all occurenaces of LIGHTNINGD will go away and we can eventually
remove it.
I also converted calls to `command_fail_detailed()` that took a `NULL` `data`
parameter to use the new `command_fail()`.
The only difference from an end user perspecive is that bad input errors that
used to be -1 will now be -32602 (JSONRPC2_INVALID_PARAMS).
We can have more than one; eg we might offer both bech32 and a p2sh
address, and in future we might offer v1 segwit, etc.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't handle \u, since we assume everyone sane is using UTF-8. We'd
still have to reject '\u0000' and maybe other weird cases if we did.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* Modifies invoice command to have the following format
invoice <msatoshi> <label> <desc> <?expiry> <?fallbackaddr>
* Adds support for Segwit bcrt1 addresses for withdraw
* Add test case for fallback address in invoice creation
* Create a common json_tok_address_scriptpubkey to be used
by invoice and withdraw commands.
We always hand in "NULL" (which means use tal_len on the msg), except
for two places which do that manually for no good reason.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We move it into jsonrpc where it belongs, and make it fail the command.
This means it can tell us exactly what was wrong.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
delinvoice was orginally documented to only allow deletion of unpaid
invoices, but there might be reasons to delete paid ones or unexpired ones.
But we have to avoid the race where someone pays as it's deleted: the
easiest way is to have the caller tell us the status, and fail if
it's wrong.
Fixes: #477
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Error code is inverted (which makes sense: who returns 'true' on
error?), and anyway there's a leak if we do error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows us to add other fields, such as version information,
warnings or invoiceless payments, later.
(Note: the deprecated listinvoice is unchanged)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This matches the other names, and also the return value is about to change.
This will be removed before release!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Paid invoices need to know how much was actually paid: both for the case
where no 'msatoshi' amount was specified, and for the normal case, where
clients are permitted to overpay in order to help them disguise their
payments.
While we migrate the db, we leave this field as 0 for old paid
invoices. This is unhelpful for accounting, but at least clearly
indicates what happened if we find this in the wild.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
