Commit Graph

114 Commits

Author SHA1 Message Date
Rusty Russell 0dcd66880c Rename `struct json_result` to `struct json_stream` (RENAMEONLY)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-19 22:02:11 +00:00
Rusty Russell e46ce0fc84 jsonrpc: declare up front whether a response is success or fail.
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>
2018-10-19 22:02:11 +00:00
Rusty Russell 96f05549b2 common/utils.h: add tal_arr_expand helper.
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>
2018-09-27 22:57:19 +02:00
Rusty Russell ae61f645ab chaintopology: don't "fix" unknown feerate against known one.
This was found because it means we have a non-zero feerate without
filling in the history of that feerate:

==15895== Conditional jump or move depends on uninitialised value(s)
==15895==    at 0x408699: feerate_max (chaintopology.c:828)
==15895==    by 0x41BE49: peer_start_openingd (opening_control.c:733)
==15895==    by 0x425FE9: peer_connected (peer_control.c:515)
==15895==    by 0x40CB8F: connectd_msg (connect_control.c:304)
==15895==    by 0x42DB4E: sd_msg_read (subd.c:475)
==15895==    by 0x42D499: read_fds (subd.c:302)
==15895==    by 0x46EB18: next_plan (io.c:59)
==15895==    by 0x46F5E9: do_plan (io.c:387)
==15895==    by 0x46F627: io_ready (io.c:397)
==15895==    by 0x471187: io_loop (poll.c:310)
==15895==    by 0x41683D: main (lightningd.c:732)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-03 15:55:42 +02:00
Rusty Russell 2af94f1817 chaintopology: remove redundant wallet pointer.
We already have access via the ld object, and we initialized this one
twice anyway.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-03 05:01:40 +00:00
Rusty Russell 33c6285787 feerates: turn it into a simple query API, remove setting.
It's probably unnecessary to have this weird way of injecting results
now we have explicit feerate args.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-30 16:33:35 +02:00
Rusty Russell db3c387264 feerate: allow names 'urgent' 'normal' and 'slow'.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-30 16:33:35 +02:00
Rusty Russell e0952ceff2 feerate: use suffix, not separate argument.
And, reluctantly, default to bitcoind style.
"It's wrong to be right too soon."

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-30 16:33:35 +02:00
Rusty Russell 14dc1c37ab fundchannel / withdraw: allow explicit feerate setting.
These are the two cases where we'll refuse without a fee estimate.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-30 16:33:35 +02:00
Rusty Russell e2d4b7cc8d cleanup: extract and formalize feerate conversion.
I didn't want to create a new file for this now, as that would totally
break #1880.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-30 16:33:35 +02:00
Rusty Russell af4fa9a359 feerates: rename sipa/bitcoind to perkw/perkb.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell a4b952ebc7 feerate: include rough estimates of actual tx costs.
We could refine this later (based on existing wallet, for example), but
this gives some estimate.

[ Rename onchain_estimates -> onchain_fee_estimates Suggested-by: @SimonVrouwe ]
[ Factor of 1000 fix Reported-by: @SimonVrouwe ]
Suggested-by: @molxyz
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell 14294642d2 feerates: consider last three raw values for min/max.
We don't know what our peer is doing, but if we see those values, maybe
they did too, and for longer.  And add the min/max acceptable values
into our JSON API.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell a260849870 moveonly: feerate_min and feerate_max belong in chaintopology.c
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell c7c5affa3f feerates: new command to inject/query fee estimates.
This is useful mainly in the case where bitcoind is not giving estimates,
but can also be used to bias results if you want.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell 9d37b78088 cleanup: lowercase name of feerates, immediate -> urgent.
This is only used for logging now, but it gets more important as it
enters the RPC API.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell 9d517ddc1d options: remove default-fee-rate now we don't use it.
And no more filtering out messages, as we should no longer spam the
logs with them (the 'Connected json input' one was removed some time
ago).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-25 00:33:12 +00:00
Rusty Russell 112b7336a3 memleak: create and use a generic htable helper and generic intmap helper.
memleak can't see into htables, as it overloads unused pointer bits.
And it can't see into intmap, since they use malloc (it only looks for tal
pointers).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 19:54:32 +02:00
Rusty Russell 898655f40c chaintopology: fix outdated comment.
Both @cdecker and @SimonVrouwe noted this.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell 175db926c2 chaintopology: expose when we don't actually know feerate.
We use feerate in several places, and each one really should react
differently when it's not available (such as when bitcoind is still
catching up):

1. For general fee-enforcement, we use the broadest possible limits.
2. For closingd, we use it as our opening negotiation point: just use half
   the last tx feerate.
3. For onchaind, we can use the last tx feerate as a guide for our own txs;
   it might be too high, but at least we know it was sufficient to be mined.
4. For withdraw and fund_channel, we can simply refuse.

Fixes: #1836
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell d93be58bd0 pytest: remove use dev-override-feerates.
Manipulate fees via fake-bitcoin-cli.  It's not quite the same, as
these are pre-smoothing, so we need a restart to override that where
we really need an exact change.  Or we can wait until it reaches a
certain value in cases we don't care about exact amounts.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell a75de62477 chaintopology: always initialize smoothed feerate if it's the first entry.
Not just during startup: we could have bitcoind not give estimates until
later, but we don't want to smooth with zero.

The test changes in next patch trigger this, so I didn't write a test
with this patch.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell 43fe7f034e chaintopology: try to get a feerate estimate before we complete startup.
It may fail, but it's better than having a window where we're using
the default feerate.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell ab0fa7a1bd chaintopology: always cap max block to bitcoind's block height.
We only did this when we were first creating a wallet, or when we
asked for a relative rescan, not in the normal case!

Fixes: #1843
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-21 01:00:37 +02:00
Rusty Russell f8052a6c1a chaintopology: watch UTXOs which need closeinfo when we remove blocks.
Normal wallet txs get reconfirmed as blocks come in, but ones which need
closeinfo are more fragile, so we do it manually using txwatch for them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-21 00:48:02 +02:00
Mark Beckwith a3178b8177 param: remove old callback code
Cleaned up remaining code. Reduced comment noise. Reverted
macro names back to the original.

Signed-off-by: Mark Beckwith <wythe@intrig.com>
2018-08-20 01:02:25 +00:00
Mark Beckwith 294dc06de9 param: upgraded json_tok_number
Also renamed old version to json_to_number for use as a utility function.

Signed-off-by: Mark Beckwith <wythe@intrig.com>
2018-08-20 01:02:25 +00:00
Simon Vrouwe 795e0e1b21 do not start fee estimation loop with option: --dev-override-fee-rates 2018-08-14 06:52:57 +00:00
Simon Vrouwe a235c1fa67 add more detail to log messages about feerate estimates 2018-07-29 20:12:21 +02:00
Simon Vrouwe a80622edab at startup initialize smoothed feerate to polled feerate
fix indentation
2018-07-26 19:08:13 +02:00
Mark Beckwith b876c601a6 Modern param style for chaintopology.c, ...
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>
2018-07-17 14:33:14 +02:00
Hiroki Gondo 070aa08709 fix: compile error with clang 2018-07-17 05:08:29 +00:00
SimonVrouwe f2ffb6d03e improves exponential smoothing of feerate estimates (#1699)
- fixes problem with polling interval > 150 * 0.9
- fixes log message 'feerate hit floor' at every feerate change
- smoothed fee now reaches 90% of (exp weighted) fee estimates polled in last
120s, independent of polling interval
- only apply smoothing when effect > 10 percent so it doesn't correct forever
- fix indentation
2018-07-15 18:30:43 +02:00
Rusty Russell fed5a117e7 Update ccan/structeq.
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>
2018-07-04 23:57:00 +02:00
Christian Decker 582ea1a33b jsonrpc: Remove `dev-blockheight` in favor of `getinfo`
`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.
2018-07-04 00:08:14 +00:00
Christian Decker fe405f49be bitcoind: Smooth fee changes over a number of estimates
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.
2018-07-02 01:41:42 +00:00
Rusty Russell 0e6c0dbba2 bitcoin: expose feerate_floor.
Onchaind will want it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-06-21 13:43:32 +02:00
Christian Decker 37327d31de topo: Remove obsolete FIXME marker
This was addressed in bdb87aa994

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2018-06-09 13:39:27 +02:00
Christian Decker 2415f48723 topo: Tell chain_topology about the min and max block height
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2018-06-06 03:30:02 +00:00
Christian Decker 0d4b7eaa2c topo: Have chain_topology track both min and max block heights
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2018-06-06 03:30:02 +00:00
Mark Beckwith 7f437715d5 Added error code parameter to command_fail
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).
2018-05-26 12:17:36 +02:00
ZmnSCPxj 097a8e72d1 channel_control: Forget if unconfirmed for a long time and we are fundee.
We should forget this as it is a potential DoS if we remember every
funding txid that an attacker gave in a `funding_created` but never
broadcasted.
2018-05-23 14:37:32 -07:00
Rusty Russell 0aa22741df option cleanup: --dev-override-fee-rates
Make --override-fee-rates a dev option.  We use default-fee-rate in
its place, which (since bitcoind won't give fee estimates in regtest
mode for short chains) gives an effective feerate of 15000/7500/3750.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-05-20 02:32:42 +00:00
Rusty Russell ceb1ce0ef5 chaintopology: fix default feerate.
We never hit the guess_feerate() path, because we turned a 0 ("can't
estimate fee") into 253.

This also revealed that we weren't initializing topo->feerate, and
that we were giving spurious updates even if we were using override-fee-rates.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-05-20 02:32:42 +00:00
Rusty Russell c8cc8fd83f option cleanup: --dev-bitcoind-poll
Make --bitcoind-poll a dev option.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-05-20 02:32:42 +00:00
Rusty Russell c6af2a8cb2 lightningd: loosen feerate minimum.
We're getting spurious closures, even on mainnet.  Using --ignore-fee-limits
is dangerous; it's slightly less so to lower the minimum (which is the
usual cause of problems).

So let's halve it, but beware the floor.

This is a workaround, until we get independent feerates in the spec.

Fixes: #613
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-05-01 18:45:04 +02:00
ZmnSCPxj d5a67ec87a chaintopology: Protect against underflow when computing first_blocknum.
Fixes: #1423

(Hopefully)

Reported-by: @NicolasDorier
2018-04-26 11:40:43 +00:00
Christian Decker 96352858d6 chaintopology: Simplify rescan offset computation
Simplification of the offset calculation to use the rescan parameter, and rename
of `wallet_first_blocknum`. We now use either relative rescan from our last
known location, or absolute if a negative rescan was given. It's all handled in
a single location (except the case in which the blockcount is below our
precomputed offset), so this should reduce surprises.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2018-04-25 14:33:38 +02:00
Christian Decker 244d4e49e1 onchaind: Store channeltxs so we can restore later
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2018-04-25 14:33:38 +02:00
Christian Decker f27cd3e43f topo: Remove in-memory txs from the block struct
The only use for these was to compute their txids so we could notify depth
in case of reorgs.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2018-04-13 00:04:37 +02:00