Commit Graph

5199 Commits

Author SHA1 Message Date
Christian Decker 59fa47bf64 pytest: Mark the worst gossip offenders as developer-only tests
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-02-18 00:06:02 +00:00
Simon Vrouwe 201dd3ce0c doc: improve explanation of getroute command 2019-02-17 21:48:24 +01:00
Simon Vrouwe 4fe60ee769 doc: fundchannel command, clarify optional `feerate`
mentioned in issue #2354
2019-02-17 21:48:24 +01:00
Simon Vrouwe 872c0c90b9 remove trailing space in logline and small coding style fix 2019-02-17 21:48:24 +01:00
Simon Vrouwe 34e40b9383 lightningd/json_dev_forget_channel: clarify message when both peer_id and scid are given
mentioned in issue #2298
2019-02-17 21:48:24 +01:00
Simon Vrouwe b66d6a0d55 lightningd/json_fundchannel: use json_add_string to add hex_tx to response, similar as in json_withdraw 2019-02-17 21:48:24 +01:00
Richard Bondi 503360143a fix crash with lightning charge and plugin opts (#2358)
* fix crash with lightning charge and plugin opts
2019-02-17 21:44:10 +01:00
Mark Beckwith 3e4a3bafc3 doc: fix connect manpage parameter names and...
also fixed grammar and consistency with other manpages.

The names are now the same as what json_connect() expects.

Signed-off-by: Mark Beckwith <wythe@intrig.com>
2019-02-11 18:28:45 +01:00
Rusty Russell b99293fbb6 short_channel_id: don't accept :-separated in JSON if --allow-deprecated-apis=false
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>
2019-02-08 16:52:30 -08:00
Rusty Russell d413fc7e9b configure: use system libbase58 if available.
Also one less headache for reproducible builds.  But unlike
libsodium, this only seems common in Ubuntu.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-08 01:10:17 +00:00
Rusty Russell 21fd8f7eaa configure: use system libsodium if available and modern.
Also one less headache for reproducible builds.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-08 01:10:17 +00:00
Rusty Russell 2db84d6653 tools/check-setup_locale.sh: don't get caught by main in non-C files.
We're about to put one in configure.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-08 01:10:17 +00:00
Rusty Russell e857229de4 tools: make build-release more friendly.
You can now override sanity checks for testing, and also
specify exactly what to build.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-08 01:10:17 +00:00
Christian Decker b7222531fe pytest: Stabilize the test_pay_direct test
It was waiting for a remote channel, but not for all the interesting
channels we want to check. It can sometimes happen that further away
channels are added before closer ones are added, depending on
propagation path, flush timers and bitcoind poll timers. This now just
checks for all channels, which also reduces the ambiguity of whether
we selected a path solely because we were lacking alternatives.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-02-08 01:08:44 +00:00
Conor Scott 960664337f pylightning: handle method introspection more generally.
[ I just cut & paste from @conscott's comment on GitHub -- RR ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell 53c0a21d2c plugins: get usage from plugins (required unless deprecated_apis == True).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell a8d1588154 libplugin: provide usage in getmanifest.
Same method as lightningd: call the handlers up-front to get their param()
usage.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell c58a4b9b42 libplugin: put method name into command struct.
We'll need this for the usage map coming in the next patch.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Conor Scott 6e4eae372d [pylightning] add args list to getmanifest
[ Shamelessly stolen by Rusty, tweaked parameter name ]
2019-02-07 20:33:50 +00:00
Rusty Russell 0c89fc5d70 pylightning: use different decoration for init msg.
The next patch wants to decorate the methods with a compulsory
'usage' option, which doesn't make sense for init.  So I wanted
to change the init to its own decoration.

Made-to-work-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell 5770e0c700 jsonrpc: probe sites for usage information once, at start.
We store it in a strmap.  This means we call the jsonrpc handler earlier,
so all callers need to call param() before they do anything else; only
json_listaddrs and json_help needed fixing.

Plugins still use '[usage]' for now.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell e5c80f63d7 lightningd: add code to search strmaps for memleak detection.
Didn't put this in common/memleak because only lightningd currently needs it
(as of next patch).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell c506d42679 plugins: don't keep redundant jsonrpc pointer.
We have ld already, just use that in the one place we need.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell dabdefefae jsonrpc: populate ld->jsonrpc ourselves, so we can use it.
Next patch will call commands to get usage inside jsonrpc_new(): to do
this it will need access to ld->jsonrpc, so we can't use the current
pattern.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell cc76416447 jsonrpc: use tal destructor to remove json commands when required.
This fixes a bug with a plugin duplicating an existing name
where we'd crash, too.

This doesn't work for builtins, which aren't tal objects, so
create a separate path for them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell da355284de jsonrpc: help, even for a single item, should be in an array.
This is what we do for every other can-be-single JSON API.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell a1ebfc9e5e pylightning: help() accepts an argument.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 20:33:50 +00:00
Rusty Russell 0361f43ec3 ccan: update
Brings in fixes for closing stderr in parent for pipecmd (oops!)
and configurator fix (which we don't need yet)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-07 13:05:59 +01:00
fanquake 132d8460fc doc: skip building bitcoin-qt on macOS
Skip installing GUI dependancies and building bitcoin-qt.
2019-02-07 03:23:03 +00:00
Christian Decker 35e4227015 docker: Do not use the copy of a clone
This was failing the docker hub builds, since the git-config retains
an absolute path to the worktree location when cloning. Copying it
over from the host system means that this path now points to a
non-existent location, which then interfered with the submodule
initialization.

This fixes it by not using the copy directly, but rather it creates a
clean clone from the copied location, including a submodule init.

Signed-off-by: Christian Decker <@cdecker>
2019-02-07 03:07:18 +00:00
fanquake 01c9cdec97 travis: fix cache directories syntax 2019-02-06 18:40:38 +01:00
Rusty Russell fbf2168902 pytest: don't time out on test_pay_direct !DEVELOPER
Travis timed out.

Waiting for three fundchannel commands depends on the bitcoind polling
interval (30 seconds), and then waiting for gossip propagation
requires two propagation intervals (120 seconds).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
Rusty Russell 3ae0c20026 getroute: change definition (and pay default) for riskfactor.
Up until now, riskfactor was useless due to implementation bugs, and
also the default setting is wrong (too low to have an effect on
reasonable payment scenarios).

Let's simplify the definition (by assuming that P(failure) of a node
is 1), to make it a simple percentage.  I examined the current network
fees to see what would work, and under this definition, a default of
10 seems reasonable (equivalent to 1000 under the old definition).

It is *this* change which finally fixes our test case!  The riskfactor
is now 40msat (1500000 * 14 * 10 / 5259600 = 39.9), comparable with
worst-case fuzz is 50msat (1001 * 0.05 = 50).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
Rusty Russell 05f95b59c1 gossipd: take into account risk in final route comparison.
We were only comparing by total msatoshis.

Note, this *still* isn't sufficient to fix our indirect problem, as
our risk values are all 1 (the minimum):

	lightning_gossipd(25480): 2 hop solution: 1501990 + 2
	lightning_gossipd(25480): 3 hop solution: 1501971 + 3
	...
	lightning_gossipd(25480): => chose 3 hop solution

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
Rusty Russell 662bb0c565 gossipd: fix riskfactor passing.
We used a u16, and a 1000 multiplier, which meant we wrapped at
riskfactor 66.  We also never undid the multiplier, so we ended up
applying 1000x the riskfactor they specified.

This changes us to pass the riskfactor with a 1M multiplier.  The next
patch changes the definition of riskfactor to be more useful.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
Rusty Russell 706debf3d4 pytest: make test_pay_direct more effective.
The test sometimes passes: our routing logic always chooses between
the shorter of two equal-cost routes (because we compare best with <
not <=).

By adding another hop, we add more noise, and by making the alternate
route fee 0 we provide the worst case.

But to be fair, we make the amount of the payment ~50c (15,000,000
msat), and increase our cltv-delay to 14 and fee-base 1000 to match
mainnet.  The final patch shows the effect of this choice.

Otherwise our risk penalty is completely in the noise on
mainnet which has the vast majority of fees set at 1000msat + 1ppm.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
Rusty Russell 5d658012d6 plugins/pay: try without routehints first.
This is the direct cause of the failure of the original
test_pay_direct test and it makes sense: invoice routehints may not be
necessary, so try without them *first* rather than last.

We didn't mention the use of routehints in CHANGELOG at all yet, so
do that now.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
Rusty Russell 6a26b0c18d gossipd: increase randomness in route selection.
We have a seed, which is for (future!) unit testing consistency.  This
makes it change every time, so our pay_direct_test is more useful.

I tried restarting the noed around the loop, but it tended to fail
rebinding to the same port for some reason?

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-06 18:39:52 +01:00
ZmnSCPxj 38a2f6c616 test_pay.py: Add test that we prefer direct route. 2019-02-06 18:39:52 +01:00
Christian Decker 7ac0f53da3 travis: Attempt to setup travis without docker and without sudo
This should speed up testing since it no longer requires virtualization.
2019-02-04 17:10:24 +01:00
Simon Vrouwe 35545f705f lightningd/json_withdraw:
Add the change output to owned_txfilter so its entry in db will
get a confirmation_height when detected in a block by filter_block_txs

before this commit, after a 'withdraw' command, 'listfunds' would
not show our change outputs as confirmed

Modified the log message in wallet_extract_owned_outputs to
append 'CONFIRMED' when it is called with a blockheight arg.
To make distinction between (1st call) when adding owned output to the
db and (2th call) when confirmed in block.
2019-02-04 12:52:57 +01:00
Christian Decker 7eaf5b55ff make: Add an option to compile with address sanitizer
Currently only works with `gcc` due to google/sanitizers#1028, so
configure makes sure we warn if clang with ASAN is attempted.

According to [my benchmarks][benchmarks] the performance degradation
is small enough to have it active always.

[benchmarks]: https://github.com/ElementsProject/lightning/issues/2277#issuecomment-455897417

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-02-03 23:50:18 +00:00
Rusty Russell 0da4054045 Makefile: fix make install to depend on plugins.
Otherwise a straight "make install" gives:
	install: cannot stat 'plugins/pay': No such file or directory
	make: *** [Makefile:482: install-program] Error 1

Fixes: #2288
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-01-31 15:30:56 +01:00
Rusty Russell 6bd1e46b25 invoice: don't allow creation of unpayable invoices.
Fixes: #2301
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-01-31 15:30:12 +01:00
Rusty Russell f1a837e091 CHANGELOG.md: document deadlock fix.
Useful if others hit it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-01-29 11:45:17 +01:00
Rusty Russell c9a907cd71 common: handle peer input before gossipd input (for closingd, openingd)
Similar to the previous "handle peer input before gossip input", this
fixes similar potential deadlock for closingd and openingd which use
peer_or_gossip_sync_read.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-01-29 11:45:17 +01:00
Simon Vrouwe 6e4d9acac3 channeld: prioritize read from peer over (read from gossipd and) write to peer
This solves (or at least reduces probability of) a deadlock in channeld
when there is lot of gossip traffic, see issue #2286. That issue is
almost identical to #1943 (deadlock in openingd) and so is the fix.
2019-01-29 11:45:17 +01:00
Simon Vrouwe 10057c8335 openingd/json_fund_channel:
- result fundchannel command now depends on successful or failed broadcast of the funding tx
- failure returns error code FUNDING_BROADCAST_FAIL
- don't fail the channel when broadcast failed, but keep in CHANNELD_AWAITING_LOCKIN
- after fixing the initial broadcast failure, the user could manually rebroadcast the tx and
  keep the channel

openingd/opening_funder_finished:
- broadcast_tx callback function now handles both success and failure

jsonrpc: added error code FUNDING_BROADCAST_FAIL
manpage: added error code returned by fundchannel command

This makes the user more aware of broadcast failure, so it hopefully doesn't
try to broadcast new tx's that depend on its change_outputs. Some users have reported (see
issue #2171) a whole sequence of fundings failing, because each funding was using the change
output of the previous one, which would not confirm.
2019-01-29 04:50:01 +00:00
Rusty Russell 2d7c1ed0cf pytest: create proper mock failures.
We actually produce an invalid JSON error at the moment: bitcoin-cli
complains "JSON value is not an integer as expected" rather than returning
the given error.  Make our error a valid JSON RPC error.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-01-29 04:50:01 +00:00
Christian Decker 27b66997da pytest: Temporarily disable test_htlcs_cltv_only_difference with VG
It is suddenly timing out a lot and is breaking master, so we
temporarily disable it until it is fixed.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-01-29 00:50:09 +00:00