There doesn't seeem to be a need for this anymore (unless I'm missing something).
I added the sendpay_nulltok() unit test to confirm.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
@rustyrussell showed we don't need temporary objects for params.
This means params no longer need a tal context.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
@wythe points out we don't need to keep the around now param_is_set()
is removed. We can in fact go further and avoid marshalling them into
temporary objects at the caller altogether.
This means internally we have an array of struct param, rather than an
array of 'struct param *', which causes most of the noise in this
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're using a macro anyway, so appending "" make it a compile-time check.
Complicates testing a bit, since we actually use generated names there.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a bit more natural, IMHO. The only issue is that json_tok_tok is
special, so we end up with param_opt_tok() if you really want an optional
generic token.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is part of #1464 and incorporates Rusty's suggested updates from #1569.
See comment in param.h for description, here's the basics:
unsigned cltv;
const jsmntok_t *note;
u64 msatoshi;
struct param * mp;
if (!param_parse(cmd, buffer, tokens,
param_req("cltv", json_tok_number, &cltv),
param_opt("note", json_tok_tok, ¬e),
mp = param_opt("msatoshi", json_tok_u64, &msatoshi),
NULL))
return;
if (param_is_set(mp))
do_something()
There is a lot of developer mode code to make sure we don't make mistakes,
like trying to unmarshal into the same variable twice or adding a required param
after optional.
During testing, I found a bug (of sorts) in the current system. It allows you
to provide two named parameters with the same name without error; e.g.:
# cli/lightning-cli -k newaddr addresstype=p2sh-segwit addresstype=bech32
{
"address": "2N3r6fT65PhfhE1mcMS6TtcdaEurud6M7pA"
}
It just takes the first and ignores the second. The new system reports this as an
error for now. We can always change this later.
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>
`getinfo` has been providing the blockheight for a good while and doesn't
require the `DEVELOPER=1` flag during compilation, so it should be the preferred
method to retrieve the blockchain height.
Implements an EWMA for the fee estimation. Achieves 90% influence of the newer
fee after 5 minutes, and adjusts to the polling rate that is configured.
Gossipd will ignore the second one, but doing it in the front end
gives an explicit error message.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
New codes: FUND_MAX_EXCEEDED, FUND_CANNOT_AFFORD, FUND_DUST_LIMIT_UNMET.
The error message "Cannot afford fee" was not exactly correct because
it would also occur if the amount requested could not be afforded. So
I changed it to the more generic "Cannot afford transaction".
Other things:
* Fixed off-by-one satoshi in fundchannel manpage.
* Changed 'arror' to 'error' because we are not pirates.
Turn req_running into a pointer to the current bcli structure, which means
the leak detection can find it.
Also suppress leaks in the case where we're only attached to a timer
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
During a meeting earlier this week we agreed with Eclair to temporarily
increase the final CLTV delta in our invoices to establish
compatibility with the already deployed Eclair wallets. They in turn
agreed to remove the enforcement of higher final CLTV deltas, or bump
it locally should it not match their expectations as allowed by
BOLT 11. This has since been implemented in ACINQ/eclair#627.
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>
The fee range can sometimes cause channels to be closed when the estimator
jumps. This has been the case a few times in the last months, and causes a
number of channels to be closed, and issue reports to be filed.
Increasing this from 5x to 10x should get rid of 84%+ of these
closures (measured based on 1h windows over the last 6 months and assuming
worst case situations).
Signed-off-by: Christian Decker <decker.christian@gmail.com>
I still believe that 2 weeks is way too much, but we were promised that these
defaults would be slowly reduced to saner values as the stability increases.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Compares the `blocknum` in the `short_channel_id` with the range of blocks we
store in the database and abort if we should have known about it. Avoids
bombarding `bitcoind` with requests for channels that have already been spent or
were invalid in the first place.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Since we currently only (ab)use it to send everything, we need a way to
generate boutique queries for testing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're telling gossipd about disconnections anyway, so let's just use that signal
to disable both sides of the channel.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This was failing some of our integration tests, i.e., the ones closing a channel
and not waiting for sigexchange. The remote node would often not be quick enough
to send us its disabling channel_update, and hence we'd still remember the
incoming direction. That could then be sent out as part of an invoice, and fail
subsequently. So just set both directions to be disabled and let the onchain
spend clean up once it happens.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
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).
This resolves the problem where both channeld and gossipd can generate
updates, and they can have the same timestamp. gossipd is always able
to generate them, so can ensure timestamp moves forward.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
