Commit Graph

344 Commits

Author SHA1 Message Date
lisa neigut 27c7707874 Add note about rationale for check 2020-02-10 15:59:26 +10:30
lisa neigut ec1b86d9d5 channel: make error 'hard' if awaiting lockin
if the channel hasn't been locked in yet, allow for a 'hard' error
to kill the channel
2020-02-10 15:59:26 +10:30
lisa neigut 1e3a411b74 channel control: check that peer is connected before canceling
Use the new forget_channel method to cancel, which checks that
peer is still connected before attempting to send message.
2020-02-10 15:59:26 +10:30
lisa neigut 278b69dfbe channel control: don't allow peer to cancel channel
restrict fundchannel_cancel usage to only the opener side

Changelog-Changed: Only the opener of a fundchannel can cancel the channel open with fundchannel_cancel
2020-02-10 15:59:26 +10:30
darosior dc3aa33927 pytest: use pyln.client for functional tests 2020-01-21 22:38:14 +01:00
lisa neigut 689dd28ddd funding: enable push_msat
it's that time of year (merry xmas!)

enables the ability to push_msat on fundchannel

Changelog-Added: RPC: `fundchannel` and `fundchannel_start` can now accept an optional parameter, `push_msat`, which will gift that amount of satoshis to the peer at channel open.
2019-12-24 12:04:01 -06:00
Rusty Russell 1d0c433dc4 channeld: treat all incoming errors as "soft", so we retry.
We still close the channel if we *send* an error, but we seem to have hit
another case where LND sends an error which seems transient, so this will
make a best-effort attempt to preserve our channel in that case.

Some test have to be modified, since they don't terminate as they did
previously :(

Changelog-Changed: quirks: We'll now reconnect and retry if we get an error on an established channel. This works around lnd sending error messages that may be non-fatal.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-12-13 16:36:18 +01:00
Rusty Russell 24d54f98ad channeld: use fee_states internally.
This is an intermediary step: we still don't save it to the database,
but we do use the fee_states struct to track it internally.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-12-12 22:15:48 +01:00
lisa neigut f6ff5e5b19 connectd: make failure message more descriptive 2019-12-04 15:32:31 -06:00
Rusty Russell 3b37c9d426 common/features: clean up feature handling for different cases.
The spec is (RSN!) going to explicitly denote where each feature should
be presented, so create that infrastructure.

Incorporate the new proposed bolt11 features, which need this.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-24 23:33:17 +00:00
Rusty Russell aab83e729b lightningd: change config-dir from plugin / wallet / hsm POV into <network> subdir
Changelog-changed: .lightningd plugins and files moved into <network>/ subdir
Changelog-changed: WARNING: If you don't have a config file, you now may need to specify the network to lightning-cli
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-23 22:42:34 +00:00
lisa neigut de16d0f0b4 openchannel hook: add new `close_to` field
Rounds out the application of `upfront_shutdown_script`, allowing
an accepting node to specify a close_to address.

Prior to this, only the opening node could specify one.

Changelog-Added: Plugins: Allow the 'accepter' to specify an upfront_shutdown_script for a channel via a `close_to` field in the openchannel hook result
2019-11-22 00:41:15 +00:00
Rusty Russell 709c98f539 logging: remove spaces from subsystem names.
Spaces just make life a little harder for everyone.

(Plus, fix documentation: it's 'jsonrpc' not 'json' subsystem).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-18 04:50:22 +00:00
Rusty Russell 4fc498f901 lightningd: enable io logging on subdaemons iff we're going to print it.
This simplifies our tests, too, since we don't need a magic option to
enable io logging in subdaemons.

Note that test_bad_onion still takes too long, due to a separate minor
bug, so that's marked and left dev-only for now.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-18 04:50:22 +00:00
Rusty Russell ef7a820ab1 log: make formatting more consistent.
1. Printed form is always "[<nodeid>-]<prefix>: <string>"
2. "jcon fd %i" becomes "jsonrpc #%i".
3. "jsonrpc" log is only used once, and is removed.
4. "database" log prefix is use for db accesses.
5. "lightningd(%i)" becomes simply "lightningd" without the pid.
6. The "lightningd_" prefix is stripped from subd log prefixes, and pid removed.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-changed: Logging: formatting made uniform: [NODEID-]SUBSYSTEM: MESSAGE
Changelog-removed: `lightning_` prefixes removed from subdaemon names, including in listpeers `owner` field.
2019-11-18 04:50:22 +00:00
Rusty Russell eed654f684 connectd, gossipd: use per-peer logging.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-18 04:50:22 +00:00
Rusty Russell e433d4ddc1 lightningd: have logging include an optional node_id for each entry.
A log can have a default node_id, which can be overridden on a per-entry
basis.  This changes the format of logging, so some tests need rework.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-18 04:50:22 +00:00
Rusty Russell bb06bec891 features: set OPT_VAR_ONION (bit 9) iff EXPERIMENTAL_FEATURES
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-14 10:15:33 +01:00
lisa neigut a333df449a listpeers: show close_to address
If a 'upfront_shutdown_script' was specified, show the address +
scriptpubky in `listpeers`

Changelog-added: JSON API: `listpeers` channels now include `close_to` and `close_to_addr` iff a `close_to` address was specified at channel open
2019-11-13 03:31:20 +00:00
Rusty Russell a70f2dc650 pytest: two more timeouts on Travis.
Reduce test_feerate_stress iterations, and simply don't run
test_pay_retry under VALGRIND with SLOW_MACHINE at all.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-07 03:50:53 +00:00
Rusty Russell c96cee9b8d channeld: fix invalid assumption in htlc restore.
A long time ago (93dcd5fed7), I
simplified the htlc reload code so it adjusted the amounts for HTLCs
in id order.  As we presumably allowed them to be added in that order,
this avoided special-casing overflow (which was about to deliberately
be made harder by the new amount_msat code).

Unfortunately, htlc id order is not canonical, since htlc ids are
assigned consecutively in both directions!  Concretely, we can have two HTLCs:

	HTLC #0 LOCAL->REMOTE: 500,000,000 msat, state RCVD_REMOVE_REVOCATION
	HTLC #0 REMOTE->LOCAL: 10,000 msat, state SENT_ADD_COMMIT

On a new remote-funded channel, in which we have 0 balance, these
commits *only* work in this order.  Sorting by HTLC ID is not enough!
In fact, we'd have to worry about redemption order as well, as that
matters.

So, regretfully, we offset the balances halfway to UINT64_MAX, then check
they didn't underflow at the end.  This loses us this one sanity check,
but that's probably OK.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-05 22:38:07 +01:00
Rusty Russell 7b6a1c8c87 pytest: add test for bug found by Travis
We fail to restore HTLCs on reconnect sometimes, depending on traverse order:

	2019-10-30T18:39:40.012Z **BROKEN** lightningd(7652): lightning_channeld-0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518 chan #1: Cannot add htlc #0 10000msat to LOCAL
	2019-10-30T18:39:40.024Z **BROKEN** lightningd(7652): lightning_channeld-0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518 chan #1: Could not restore HTLCs (version v0.7.3-12-ga0a271a)

Or, alternatively:

lightning_channeld: Could not restore HTLCs (version v0.7.3-11-gd7838db-modded)
0x564d1c1b53bd send_backtrace
	common/daemon.c:41
0x564d1c1c23c9 status_failed
	common/status.c:199
0x564d1c1a7509 init_channel
	channeld/channeld.c:3073
0x564d1c1a7959 main
	channeld/channeld.c:3165
0x7fdc73be01e2 ???
	???:0
0x564d1c19ee5d ???
	???:0
0xffffffffffffffff ???
	???:0

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-11-05 22:38:07 +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 61e1d6431c pytest: stress fee_update code, trigger bug.
A 'Bad commit_sig signature' was reported by @Javier on Telegram and
@DarthCoin.  This was between two c-lightning peers, so definitely our fault.

Analysis of this message revealed the signature was using the wrong
feerate.  I finally managed to make a test case which triggered this.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-10-28 13:07:41 -05:00
Christian Decker 0a641c9443 pytest: Don't assume UTXO ordering when selecting coins
This was failing because the internal coin-selection doesn't go by insertion
order when using postgres.
2019-10-18 08:29:46 +02:00
lisa neigut 49d5a36751 funding: integration tests for close_to
Check behavior for user supplied upfront_shutdown_script via close_to


Header from folded patch 'fix__return__not__iff_well_close_to_the_provided_addr.patch':

fix: return  not  iff we'll close to the provided addr
2019-10-15 19:10:05 +02:00
Rusty Russell bd55f6d940
common/features: only support a single feature bitset.
This is mainly an internal-only change, especially since we don't
offer any globalfeatures.

However, LND (as of next release) will offer global features, and also
expect option_static_remotekey to be a *global* feature.  So we send
our (merged) feature bitset as both global and local in init, and fold
those bitsets together when we get an init msg.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-10-11 02:52:04 +00:00
Rusty Russell 8a3abdd5ff pytest: fix test_restart_many_payments
It sometimes fail with a bad_gossip error because the sending node
might not have found out about the channel when it gets a
channel_update.  Make sure the whole network knows everything before
we start.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-10-10 21:48:52 -05:00
trueptolemy fdb90a765e pytest: Test compact of the old-style `fundchannel_start` 2019-10-09 16:51:42 -05:00
trueptolemy 987e4e27e8 pytest: Test compact of the old-style `fundchannel` 2019-10-09 16:51:42 -05:00
Rusty Russell 33c658ecfb gossipd: advertize all our features in node_announcement.
This preempts the acceptance of
https://github.com/lightningnetwork/lightning-rfc/pull/666 but it's
clear that feature bits are going to be distinct, so this is safe to
do anyway.

See https://github.com/lightningnetwork/lightning-rfc/pull/680

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-10-07 05:51:05 +00:00
Christian Decker e23d8a4aff pytest: Mark some tests as skippable that are too bitcoin-specific 2019-10-03 04:32:57 +00:00
Christian Decker 8a69ea7bf5 elements: Parametrize the sample addresses we test against
Since elements addresses look quite different from the bitcoin mainnet
addresses I just added a sample to the chainparams fixture. In addition I
extracted some of the fixed strings to reference chainparams instead.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-10-03 04:32:57 +00:00
Rusty Russell 15612d269a Make option_static_remotekey non-EXPERIMENTAL now it's in spec.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-30 01:50:44 +00:00
Rusty Russell 4e8141aa36 features: remove INITIAL_ROUTING_SYNC.
It only had an effect if the peer didn't support option_gossip_queries, but
still, we don't want a gossip blast any more.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-25 04:01:56 +00:00
Christian Decker 9915386c4a pytest: Stabilize test_no_fee_estimate against UTXO selection issues
The test was implicitly relying on us selecting the larger output and then not
touching the smaller, leaving it there for the final `withdraw` to claim. This
ordering of UTXOs is not guaranteed, and in particular can fail when switching
DB backends. To stabilize we just need to make sure to select the change
output as well.
2019-09-22 02:03:43 +00:00
Christian Decker efc4aa94a3 pytest: Skip some tests that assume we have a sqlite3 db on postgres
These will not work since they touch the DB file itself.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-09-22 02:03:43 +00:00
Rusty Russell 27790832a5 gossipd: gossip_queries_ex is not longer experimental.
The master spec has some typos which make it not parse, so I created
a PR and generated the CSV from that:

https://github.com/lightningnetwork/lightning-rfc/pull/673

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-22 01:17:11 +00:00
Rusty Russell 147eaced2e developer: consolidiate gossip timing options into one --dev-fast-gossip.
It's generally clearer to have simple hardcoded numbers with an
#if DEVELOPER around it, than apparent variables which aren't, really.

Interestingly, our pruning test was always kinda broken: we have to pass
two cycles, since l2 will refresh the channel once to avoid pruning.

Do the more obvious thing, and cut the network in half and check that
l1 and l3 time out.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-20 06:55:00 +00:00
lisa neigut c261309f18 test: fund wallet before testing fundchannel
After switching to a plugin, we verify that we can fund a channel
before we check to contact a peer. We'll need to have a funded wallet
to pass the check in this test that verifies that 'fundchannel' cannot
be called for a peer after fundchannel_start is.
2019-09-11 23:56:27 +00:00
trueptolemy 8d28e52515 pytest: Test `fundchannel_cancel` can catch tx braodcast by bitcoind
For now, we can't fully ensure that the broadcast was catched from a third pary. Only when the transaction (broadcast by a third pary) is onchain, we can catch it.
2019-09-11 17:04:14 -05:00
trueptolemy f1822e3650 pytest: Test `fundchannel_cancel` can work after `fundchannel_complete` 2019-09-11 17:04:14 -05:00
Rusty Russell 226e2aee48 option_static_remotekey: update to latest draft.
531c8d7d9b

In this one, we always send my_current_per_commitment_point, though it's
ignored.  And we have our official feature numbers.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-10 16:18:25 -05:00
Rusty Russell 0861279b65 channeld: don't exchange my_current_per_commitment_point if option_static_remotekey
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-10 16:18:25 -05:00
Rusty Russell 5203847025 common/features: if EXPERIMENTAL_FEATURES, advertise `option_static_remotekey`
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-09-10 16:18:25 -05:00
darosior a7cbe93fb8 closingd: retransmit 'funding_locked' if we reconnect without any update
As per BOLT02 #message-retransmission :
if `next_commitment_number` is 1 in both the `channel_reestablish` it sent and received:
    - MUST retransmit `funding_locked`
2019-09-10 02:02:51 +00:00
Rusty Russell a38131f349 EXPERIMENTAL: advertize the extended_queries feature.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-29 09:01:48 +02:00
Rusty Russell bf3b77a947 Travis: skip testing VALGRIND=1 DEVELOPER=0, remove the slowest non-developer tests.
I don't remember ever seeing a bug which only showed up in VALGRIND=1 with developer
mode disabled, so don't test that, and spread out the other test more evenly.

In addition, disable the worst-performing tests in DEVELOPER=0 mode.

Here timings from my build machine: the worst 6 (- DEVELOPER=0 VALGRIND=0)
with the same tests (+ DEVELOPER=1 VALGRIND=1)

-452.42s call     tests/test_pay.py::test_channel_spendable
+87.69s call     tests/test_pay.py::test_channel_spendable
-335.66s call     tests/test_gossip.py::test_gossip_store_compact_on_load
+47.41s call     tests/test_gossip.py::test_gossip_store_compact_on_load
-332.07s call     tests/test_connection.py::test_opening_tiny_channel
+89.71s call     tests/test_connection.py::test_opening_tiny_channel
-331.97s call     tests/test_pay.py::test_channel_spendable_large
+56.23s call     tests/test_pay.py::test_channel_spendable_large
-305.28s call     tests/test_invoices.py::test_invoice_routeboost
+37.57s call     tests/test_invoices.py::test_invoice_routeboost
-284.28s call     tests/test_plugin.py::test_htlc_accepted_hook_forward_restart
+49.12s call     tests/test_plugin.py::test_htlc_accepted_hook_forward_restart

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-14 11:14:38 +00:00
darosior b966e5e783 Add a test for 'fundchannel_start' crash on deconnection 2019-08-10 00:04:01 +08:00
Rusty Russell 0edc0ae5e9 pytest: don't use deprecated options for close() in tests.
Only downside is you have to wait 1 second at least before
unilaterally closing.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-09 05:47:16 +00:00
Rusty Russell b35dc4689b pytest: enable deprecated APIs for multi-arg closes.
We're about to change the API, so this makes the tests still work
across the transition (and, as a bonus, tests our backwards compat
shim).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-08-09 05:47:16 +00:00
Christian Decker a85d3dc220 pytest: Make test_funding_cancel_race SLOW_MACHINE compatible
This test is spawning 100 nodes concurrently, which is a lot even when not
running with `valgrind`, especially when executing tests in parallel.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-08-07 01:54:43 +00:00
Simon Vrouwe af7044d09d pytest: test_fee_limits, add case where opening channel is denied when feerate is too low 2019-07-27 05:14:34 +00:00
Rusty Russell 789d14299b pytest: mix up order of cancel vs complete in test_funding_cancel_race
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-07-08 15:14:31 +00:00
Rusty Russell ee8edfe6a6 pytest: severely reduce scope of test_funding_cancel_race under valgrind.
Otherwise we get timeouts across the board.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-07-08 15:14:31 +00:00
Rusty Russell 280bd60988 lightningd: allow multiple cancels on a single fundchannel command.
Instead of taking over the ->cmd pointer, append ourselves to a list
of cancels.  This fixes the test_funding_cancel_race.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-07-08 15:14:31 +00:00
Rusty Russell 13dbe6e74b pytest: stress test funding_cancel vs funding_complete
This fails, because they fight over the fc->cmd pointer, leaving
others hanging.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-07-08 15:14:31 +00:00
Rusty Russell 54ce4ed1cf pytest: fail tests if we get any LOG_BROKEN level messages, unless flagged.
And clean up some dev ones which actually happen (mainly by calling
channel_fail_permanent which logs UNUSUAL, rather than
channel_internal_error which logs BROKEN).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-07-02 03:26:10 +00:00
Christian Decker 915c932e2d pytest: Make sure the channels entries don't get deleted anymore
Signed-off-by: Christian Decker <@cdecker>
2019-06-30 16:41:30 +09:30
lisa neigut 92bd3a3587 connect test: use wait_for_log in test_funding_external_wallet
test_funding_external_wallet is flakey because we don't wait for
the channel state to update before asserting that it's changed.
2019-06-18 06:54:21 +00:00
lisa neigut c00e0d2936 funding: rename fundchannel_continue -> _complete
Renaming. "complete" more accurately describes what we're doing here.
2019-06-12 02:22:43 +00:00
lisa neigut 3ae78a61fa tests: finish up test for external funding flow
Add to test for fundchannel with composing and broadcasting
an external transaction.
2019-06-12 02:22:43 +00:00
lisa neigut 5aad642c59 opening: add fundchannel_cancel command
Provide the option to cancel a funding-opening with a peer.
Must either call `fundchannel_cancel` or `fundchannel_continue`
2019-06-12 02:22:43 +00:00
lisa neigut 7ea21c36b1 fundchannel: add txout field to RPC/API
We'll need the outpoint for the funding output.
2019-06-12 02:22:43 +00:00
lisa neigut b0b813a171 test: add initial tests for starting an external fundchannel
Test for getting through the address generation portion.
2019-06-12 02:22:43 +00:00
darosior 2436214145 fundchannel: Add some tests to fund channels by utxos 2019-06-11 23:24:07 +00:00
Rusty Russell 6f015b69fd channeld: don't send feerate spam if we can't set it as high as we want.
@pm47 gave a great bug report showing c-lightning sending the same
UPDATE_FEE over and over, with the final surprise result being that we
blamed the peer for sending us multiple empty commits!

The spam is caused by us checking "are we at the desired feerate?" but
then if we can't afford the desired feerate, setting the feerate we
can afford, even though it's a duplicate.  Doing the feerate cap before
we test if it's what we have already eliminates this.

But the empty commits was harder to find: it's caused by a heuristic in
channel_rcvd_revoke_and_ack:

```
	/* For funder, ack also means time to apply new feerate locally. */
	if (channel->funder == LOCAL &&
	    (channel->view[LOCAL].feerate_per_kw
	     != channel->view[REMOTE].feerate_per_kw)) {
		status_trace("Applying feerate %u to LOCAL (was %u)",
			     channel->view[REMOTE].feerate_per_kw,
			     channel->view[LOCAL].feerate_per_kw);
		channel->view[LOCAL].feerate_per_kw
			= channel->view[REMOTE].feerate_per_kw;
		channel->changes_pending[LOCAL] = true;
	}
```

We assume we never send duplicates, so we detect an otherwise-empty
change using the difference in feerates.  If we don't set this flag,
we will get upset if we receive a commitment_signed since we consider
there to be no changes to commit.

This is actually hard to test: the previous commit adds a test which
spams update_fee and doesn't trigger this bug, because both sides
use the same "there's nothing outstanding" logic.

Fixes: #2701
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-06-09 02:39:05 +00:00
Rusty Russell d344f9d267 pytest: test for duplicate update_fee spam.
Reported-by: @pm47
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-06-09 02:39:05 +00:00
Simon Vrouwe b1058dfaca pytest: add test_reconnect_remote_sends_no_sigs, which tests PR2619
Remote node may (incorrectly) not send announcement_signatures when
reconnecting, so we we use a copy and can still re-announce.
Also checks that we still send our announcement_signatures when reconnecting.
2019-06-07 10:34:34 +02:00
Michael Schmoock 45645e3a85 test: fix flakyness test_no_fee_estimate 2019-06-03 19:30:19 +02:00
Christian Decker 8feb05aef4 subd: Remove overly verbose log when receiving a message
These tend to spam the logs, so I removed them :-)

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-05-26 16:19:10 +02:00
Christian Decker ced2ff8aa7 pytest: Mark repeat offenders as flaky
It's always the same few tests that fail, so I'm marking them as flaky and
will do a cleanup round later to individually address them.

I've been tracking the failures on the master branch for a few months now and
these are the worst offenders:

|-----------------------------------------|--------|--------|-------|
| test_name                               | state  | branch | count |
|-----------------------------------------|--------|--------|-------|
| test_pay_direct                         | FAILED | master | 31    |
| test_reconnect_gossiping                | FAILED | master | 20    |
| test_htlc_send_timeout                  | FAILED | master | 15    |
| test_pay_limits                         | FAILED | master | 13    |
| test_permfail                           | FAILED | master | 13    |
| test_funding_reorg_private              | FAILED | master | 12    |
| test_invoice                            | FAILED | master | 12    |
| test_invoice_preimage                   | FAILED | master | 12    |
| test_shutdown                           | FAILED | master | 12    |
| test_withdraw                           | FAILED | master | 12    |
| test_gossip_store_load_v3               | FAILED | master | 11    |
| test_onchain_multihtlc_their_unilateral | FAILED | master | 10    |
| test_opening_tiny_channel               | FAILED | master | 10    |
| test_channel_reenable                   | FAILED | master | 9     |
| test_crashlog                           | FAILED | master | 9     |
| test_gossip_weirdalias                  | FAILED | master | 9     |
| test_invoice_expiry                     | FAILED | master | 9     |
| test_onchain_multihtlc_our_unilateral   | FAILED | master | 9     |
| test_peerinfo                           | FAILED | master | 9     |
| test_private_channel                    | FAILED | master | 9     |
| test_shutdown_reconnect                 | FAILED | master | 9     |
| test_closing                            | FAILED | master | 8     |
| test_closing_different_fees             | FAILED | master | 8     |
| test_closing_while_disconnected         | FAILED | master | 8     |
| test_gossip_timestamp_filter            | FAILED | master | 8     |
| test_gossipwith                         | FAILED | master | 8     |
| test_invoice_routeboost                 | FAILED | master | 8     |
| test_onchain_middleman                  | FAILED | master | 8     |
| test_plugin_notifications               | FAILED | master | 8     |
| test_reconnect_channel_peers            | FAILED | master | 8     |
| test_bad_onion                          | FAILED | master | 7     |
| test_closing_torture                    | FAILED | master | 7     |
| test_fulfill_incoming_first             | FAILED | master | 7     |
| test_funding_reorg_disagree_scid_route  | FAILED | master | 7     |
| test_pay_disconnect                     | FAILED | master | 7     |
| test_balance                            | FAILED | master | 6     |
| test_check_command                      | FAILED | master | 6     |
| test_closing_id                         | FAILED | master | 6     |
| test_gossip_addresses                   | FAILED | master | 6     |
| test_gossip_notices_close               | FAILED | master | 6     |
| test_setchannelfee_usage                | FAILED | master | 6     |
| test_announce_address                   | FAILED | master | 5     |
| test_connect_by_gossip                  | FAILED | master | 5     |
| test_gossip_badsig                      | FAILED | master | 5     |
| test_gossip_disable_channels            | FAILED | master | 5     |
| test_gossip_jsonrpc                     | FAILED | master | 5     |
| test_gossip_persistence                 | FAILED | master | 5     |
| test_gossip_pruning                     | FAILED | master | 5     |
| test_onchain_different_fees             | FAILED | master | 5     |
| test_onchain_dust_out                   | FAILED | master | 5     |
|-----------------------------------------|--------|--------|-------|
2019-05-26 16:19:10 +02:00
Christian Decker 56f7efad46 pytest: Stabilize test_reconnect_gossiping
We weren't waiting for l2 to register the peer before asking it to ping it.
2019-05-26 16:19:10 +02:00
Rusty Russell cfebe66762 openingd: support receipt of upfront_shutdown_script.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-05-02 01:18:33 +00:00
Michael Schmoock 4985693bea feat: subtract dust reserves on the fly from min-capacity-sat 2019-04-16 15:01:28 -07:00
Michael Schmoock 3fa539fc1b chore: increase min-capacity-sat to 10k
The old value of 1000 sat was too small to cover the dust reserves.
This lead to the situation when trying to open a channel with minimal
amount, the channels got refused because they were not able cover the
commitment fees.

For this reason the minimal capacity should be increased to i.e. 10k
satoshi, as the technical minimum that also accounts for fees and
reserves is somewhere around 6k sat.
2019-04-16 15:01:28 -07:00
Michael Schmoock b0b86c9eb8 test: opening_tiny_channel with min_capacity_sat 2019-04-09 13:20:52 +02:00
Rusty Russell 049f2351d4 newaddr: deprecate 'address' output.
It doesn't make sense with 'all', and it's ok to be explicit.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-03-04 15:11:20 +01:00
Rusty Russell 203ef2ed0b listsendpays: updated version of listpayments.
New name is less confusing, and most people should be transitioning to
listpays rather than this anyway.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-23 05:45:25 +00:00
Christian Decker 72f1c78a1e jsonrpc: Arm the minconf=1 parameter and deal with the fallout
We want to disallow using unconfirmed outputs by default, so making the
default 1 confirmation seems a good idea. This also matches `bitcoind`s
minimum confirmation requirement.

Arming however breaks some of our tests, so I used `minconf=0` for the
breaking tests and added a new test specifically for the `minconf` parameter
for `fundchannel`.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-02-22 10:40:59 +00:00
Rusty Russell 3b587a1c6d lightningd: fix db error where we can have detached peer.
An uncommitted channel should not keep the peer in the db, since the
uncommitted channel isn't in the db itself.

Fixes: #2367
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-21 12:03:54 +01:00
Rusty Russell 26df586fbf pytest: add test for db constraint failure
wallet_channel_insert: UNIQUE constraint failed: peers.node_id
lightningd: Fatal signal 6 (version v0.6.3rc1-202-g4c8cb98)
0x555625b14261 crashdump
	common/daemon.c:40
0x7f8f7400e0ff ???
	???:0
0x7f8f7400e077 ???
	???:0
0x7f8f73fef534 ???
	???:0
0x555625af34f2 fatal
	lightningd/log.c:624
0x555625b3dd96 db_exec_prepared_
	wallet/db.c:448
0x555625b44fcd wallet_channel_insert
	wallet/wallet.c:1067
0x555625af64f7 wallet_commit_channel
	lightningd/opening_control.c:229
0x555625af6e86 opening_funder_finished
	lightningd/opening_control.c:394
0x555625af7bfa openingd_msg
	lightningd/opening_control.c:714
0x555625b0d421 sd_msg_read
	lightningd/subd.c:474
0x555625b0cd3b read_fds
	lightningd/subd.c:302
0x555625b548d3 next_plan
	ccan/ccan/io/io.c:59
0x555625b553ef do_plan
	ccan/ccan/io/io.c:395
0x555625b5542d io_ready
	ccan/ccan/io/io.c:405
0x555625b5700e io_loop
	ccan/ccan/io/poll.c:310
0x555625af1562 main
	lightningd/lightningd.c:827
0x7f8f73ff109a ???
	???:0
0x555625adaad9 ???
	???:0
0xffffffffffffffff ???
	???:0
Log dumped in crash.log.20190220020526

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-02-21 12:03:54 +01:00
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 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
lisa neigut 28699f0eca option_data_loss_protect: reenable by default 2019-01-21 00:48:25 +00:00
lisa neigut c45d034bc0 option_data_loss_protect: fixup commitment point check
Spurious errors were occuring around checking the provided
current commitment point from the peer on reconnect when
option_data_loss_protect is enabled. The problem was that
we were using an inaccurate measure to screen for which
commitment point to compare the peer's provided one to.

This fixes the problem with screening, plus makes our
data_loss test a teensy bit more robust.
2019-01-20 03:09:48 +00:00
Rusty Russell 9f1f79587e short_channel_id_dir: new primitive for one direction of short_channel_id
Currently only used by gossipd for channel elimination.

Also print them in canonical form (/[01]), so tests need to be
changed.

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2019-01-15 12:01:38 +01:00
Christian Decker 659a26ea5a misc: Update short_channel_id representation to use 'x' separators
Reported-by: Alex Bosworth <@alexbosworth>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-01-15 03:50:27 +00:00
lisa neigut efa38875b2 listpeers: include `private` field in `channels` output
Reveal channel's 'privacy' in `listpeers` output

Suggested-By: @shesek
2019-01-08 02:21:32 +00:00
Rusty Russell 6aa511fa7a channeld: only enable option_data_loss_protect if EXPERIMENTAL_FEATURES.
We have an incompatibility with lnd it seems: I've lost channels on
reconnect with 'sync error'.  Since I never got this code to be reliable,
disable it for next release since I suspect it's our fault :(

And reenable the check which didn't work, for others to untangle.

I couldn't get option_data_loss_protect to be reliable, and I disabled
the check.  This was a mistake, I should have either spent even more
time trying to get to the bottom of this (especially, writing test
vectors for the spec and testing against other implementations).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-12-10 22:25:32 +00:00
Rusty Russell 981b000949 pytest: add trivial test to make sure reconnect works with no channel activity.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-12-10 22:25:32 +00:00
lisa neigut a39c97c960 channeld: support private channel creation, fixes #2125
Adds a new 'announce' field for `fundchannel`, which if false
won't broadcast a `channel_announcement`.
2018-12-08 15:15:55 -08:00
lisa neigut eab992cecd py-tests: rename 'announce' to 'wait_for_announce'
Better description of what the option actually does -- if true
waits for the announcement messages to be generated and exchanged.
2018-12-08 15:15:55 -08:00
Rusty Russell 1d7b287439 pytest: speed up test_restart_many_payments when !DEVELOPER.
Because gossip in this case takes up to a minute, this test took 10
minutes.  The workaround is to do the waiting-for-gossip all at once.

Now it takes 362 seconds.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-11-22 05:15:42 +00:00
Rusty Russell 38e6aa66ff python: quieten modern flake8.
After Ubuntu 18.10 upgrade, lots of new flake8 warnings.

$ flake8 --version:
3.5.0 (mccabe: 0.6.1, pycodestyle: 2.4.0, pyflakes: 1.6.0) CPython 3.6.7rc1 on Linux

Note it seems that W503 warned about line breaks before binary
operators, and W504 complains about them after.  I prefer W504, so
disable W503.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-28 16:03:12 +01:00
Rusty Russell 09b33015c4 connectd: give user a hint when wrong key is used.
When the wrong key is used, the remote end simply hangs up.

We used to get a random errno, which tends to be "Operation now in progress."
Now it's defined to be 0, detect and provide a better error.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-19 00:53:00 +00:00
Rusty Russell fcb5310873 pytest: make wait_for do exponential backoff, start at 0.25 seconds.
This doesn't alter runtime very much, but does reduce log spam.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-10 06:10:42 +00:00
Rusty Russell a4730089e8 pytest: make test_restart_many_payments more Pythony!
IIUC, namedtuple is like tuple for grown-ups: Pythonify!

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell d85251ac6c db: fix up HTLCs which are missing failure information.
We don't save them to the database, so fix things up as we load them.

Next patch will actually save them into the db, and this will become
COMPAT code.

Also: call htlc_in_check() with NULL on db load, as otherwise it aborts
internally.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell 77be009354 pytest: add restart-during-n-way payment test.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell 65f6813706 lightningd: handle the case where the db contains a resolved HTLC without a preimage.
We need to handle this case (old db) before the next commit, which actually
fixes it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell c94ab7370c pytest: extend the test_fulfill_incoming_first case to cover reconnect.
Which we don't handle, due to a separate bug, so it's xfail.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell 4040c53258 lightningd: handle case where incoming HTLC vanished before fulfilled outgoing.
We now need an explicit 'local' flag, rather than relying on the existence
of the 'in' pointer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell 3643e1bd90 pytest: add test for when an incoming fulfilled HTLC expires before outgoing.
Usually, we only close an incoming HTLC once the outgoing HTLC is completely
resolved.  However, we short-cut this in the FULFILL case: we have the
preimage, so might as well use it immediately (in fact, we wait for it to
be committed, but we don't need to in theory).

As a side-effect of this, our assumption that every outgoing HTLC has
a corresponding incoming HTLC is incorrect, and this test (xfail) tickles
that corner case.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-10-09 23:17:54 +00:00
Rusty Russell 9455331575 json: use bolt naming for features arrays in listnodes, listpeers.
Deprecate the old names.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-28 04:14:28 +00:00
Rusty Russell 3358437062 connectd: don't log every time a peer disconnects.
Great for a few of our tests, but generally spammy.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-28 04:14:28 +00:00
Rusty Russell 252bbe1d2d pytest: don't wait for sendrawtx, wait for expected tx.
In particular, test_no_fee_estimate was flaky due to seeing the funding
tx being sent.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-19 13:04:01 +02:00
Christian Decker 9e5d7dacb0 pytest: Use the mock bitcoind everywhere 2018-09-16 00:05:34 +02:00
Christian Decker 16869e3fe6 pytest: Use the bitcoind proxy to mock feerates 2018-09-16 00:05:34 +02:00
Rusty Russell cefb6925b2 db: save and restore last_sent_commit correctly.
It's an array: we were only saving the single element; if there was more than
one changed HTLC we'd get a bad signature!

The report in #1907 is probably caused by the other side re-requesting
something we considered already finalized; to avoid this particular error,
we should set the field to NULL if there's no last_sent_commit.

I'm increasingly of the opinion we want to just save all the update
packets to the db and blast them out, instead of doing this
second-guessing dance.

Fixes: #1907
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-04 14:43:27 +02:00
Rusty Russell db12a1452f pytest: reproduce problem with restarting and retransmitting multiple outgoing htlcs
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-09-04 14:43:27 +02:00
Rusty Russell 7e1fd9c38a pytest: make test_no_fee_estimate more reliable.
1. Wait for a 'sendrawtransaction' *after* the dev-fail message; don't be
   fooled by a previous one.
2. Turning on estimate fee sets fees exactly; just wait for it to be processed.

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 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 b5ae4c12c7 pytest: fix flaky wait_for_log() in test_funder_feerate_reconnect.
The comment was wrong: the channel being locked in was triggering
the fee update and hence the disconnect.  But that can actually
happen before fund_channel returns, as that waits for the gossipd
to see the channel active.

Best to do the fee update manually, so it's exactly what we want.

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 607d4bf9d2 channel: update fees after lockin.
We don't respond to fee changes until we're locked in: make sure we catch
up at that point.

Note that we use NORMAL fees during opening, but IMMEDIATE after, so
this often sends a fee update.  The tests which break, we set those
feerates to be equal.

This (sometimes) changes the behavior of test_permfail, as we now
get an immediate commit, so that is fixed too so we always wait for
that to complete.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell 6338ae8a44 channeld: update fees if we're restarting.
This is a noop if we're opening a new channel (channel_fees_can_change(channel)
is false until funding locked in), but important if we're restarting.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-24 02:17:51 +00:00
Rusty Russell 36b1cac6e6 lightningd: new state AWAITING_UNILATERAL.
When in this state, we send a canned error "Awaiting unilateral close".
We enter this both when we drop to chain, and when we're trying to get
them to drop to chain due to option_data_loss_protect.

As this state (unlike channel errors) is saved to the database, it means
we will *never* talk to a peer again in this state, so they can't
confuse us.

Since we set this state in channel_fail_permanent() (which is the only
place we call drop_to_chain for a unilateral close), we don't need to
save to the db: channel_set_state() does that for us.

This state change has a subtle effect: we return WIRE_UNKNOWN_NEXT_PEER
instead of WIRE_TEMPORARY_CHANNEL_FAILURE as soon as we get a failure
with a peer.  To provoke a temporary failure in test_pay_disconnect we
take the node offline.

Reported-by: Christian Decker @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell a5ecc95c42 db: store claimed per_commitment_point from option_data_loss_protect.
This means we don't try to unilaterally close after a restart, *and*
we can tell onchaind to try to use the point to recover funds when the
peer unilaterally closes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell 1a4084442b onchaind: use a point-of-last-resort if we see an unknown transaction.
This may have been supplied by the peer if it's nice and supports
option_data_loss_protect.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell 6aed936799 channeld: check option_data_loss_protect fields.
Firstly, if they claim to know a future value, we ask the HSM; if
they're right, we tell master what the per-commitment-secret it gave
us (we have no way to validate this, though) and it will not broadcast
a unilateral (knowing it will cause them to use a penalty tx!).

Otherwise, we check the results they sent were valid.  The spec says
to do this (and close the channel if it's wrong!), because otherwise they
could continually lie and give us a bad per-commitment-secret when we
actually need it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell da8d620907 pytest: check that we advertise and send option_data_loss_protect.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell ebaf5eaf2e channeld: send option_data_loss_protect fields.
We ignore incoming for now, but this means we advertize the option and
we send the required fields.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell 162adfdf12 listpeers: correctly display features on reconnect.
peer features are only kept for connected peers (as they can change),
but we didn't update them on reconnect.  The main effect was that
after a restart we displayed the features as empty, even after
reconnect.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell 9f175deecd lightningd: update feerate upon receiving revoke_and_ack from fundee.
1. l1     update_fee ->    l2
2. l1 commitment_signed -> l2 (using new feerate)
3. l1  <- revoke_and_ack   l2
4. l1 <- commitment_signed l2 (using new feerate)
5. l1  -> revoke_and_ack   l2

When we break the connection after #3, the reconnection causes #4 to
be retransmitted, but it turns out l1 wasn't telling the master to set
the local feerate until it received the commitment_signed, so on
reconnect it uses the old feerate, with predictable results (bad
signature).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-22 18:54:53 +02:00
Rusty Russell c106fa1b4f pytest: add test for reconnect immediately after feerate change.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-22 18:54:53 +02:00
Rusty Russell 7a77b81dff pytest: always use the fake-bitcoin-cli, name it more appropriately.
We're going to use it to override specific commands.  It's non-valgrinded
already since we use '--trace-children-skip=*bitcoin-cli*' so the overhead
should be minimal.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-22 12:13:23 +02:00
Rusty Russell 5e20eedb41 pytest: allow more elaborate bitcoin-cli override.
In particular, this lets us intercept individual commands, such as
estimatesmartfee.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-22 12:13:23 +02:00
Rusty Russell b379bec4e4 pytest: fix flakiness in test_channel_reenable.
In one case, the channel_update which we expected to activate the channel
from l2 was suppressed as redundant.  This is certainly valid, so just
check the results.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-16 00:14:08 +00:00
Christian Decker a97955845f pytest: dev-ping was renamed to ping 2018-08-10 16:15:12 +02:00
Rusty Russell 35d7449259 connectd: initialize peer->conn.
It's only used in one place, but that's enough.

Fixes: #1434
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-10 16:15:12 +02:00
Rusty Russell f8aed1b4b0 pytest: add reconnection stress test.
It sometimes triggers a crash like #1434 (though never under valgrind).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-10 16:15:12 +02:00
Rusty Russell fefb7faba7 pytest: try a simple reconnection test.
This passes, but that's OK.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-10 16:15:12 +02:00
Rusty Russell 8939a5001b connectd: rely on the master to tell us to reconnect.
connectd tells master about every disconnection, and master knows
whether it's important to reconnect.  Just get the master to invoke a new
connect command if it considers the peer important!

The only twist is timeouts: we don't want to immediately reconnect if
we've failed to connect.  To solve this, connectd passes a 'delaytime'
to the master when a connection fails, and the master passes it back
when it asks for a connection.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-09 19:44:27 +02:00
Rusty Russell 035362e151 openingd: don't exit when we receive an error.
In particular, all opening_read_peer_msg() callers need to know there
was an error (presumably, negotiating) so they can stop, but we should
not exit.

This lets us reenable the final disabled test.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-09 19:44:27 +02:00
Rusty Russell 02966a4857 connectd: remove unused handback APIs and code.
We now simply maintain a pubkey set for connected peers (we only care
if there's a reconnect), not the entire peer structure.

lightningd no longer queries us for getpeers: it knows more than we do
already.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-09 19:44:27 +02:00
Rusty Russell e59cbb3e2c pytest: make sure receiving peer's openingd is ready.
There's now a potential race: the source peer connect returns, but in
destination peer the master hasn't read the connect message from
connectd, so the peer isn't in listpeers yet.

(Previously the connection stayed in connectd, so there was no such
window).

This is an occasional issue in a few places.

Note that we take the opportunity to speed up test_disconnectpeer too
while we're there.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-09 19:44:27 +02:00
Rusty Russell 50f5eb34b4 openingd: take peer before we're opening, wait for explicit funding msg.
Prior to this, lightningd would hand uninteresting peers back to connectd,
which would then return it to lightningd if it sent a non-gossip msg,
or if lightningd asked it to release the peer.

Now connectd hands the peer to lightningd once we've done the init
handshake, which hands it off to openingd.

This is a deep structural change, so we do the minimum here and cleanup
in the following patches.

Lightningd:
1. Remove peer_nongossip handling from connect_control and peer_control.
2. Remove list of outstanding fundchannel command; it was only needed to
   find the race between us asking connectd to release the peer and it
   reconnecting.
3. We can no longer tell if the remote end has started trying to fund a
   channel (until it has succeeded): it's very transitory anyway so not
   worth fixing.
4. We now always have a struct peer, and allocate an uncommitted_channel
   for it, though it may never be used if neither end funds a channel.
5. We start funding on messages for openingd: we can get a funder_reply
   or a fundee, or an error in response to our request to fund a channel.
   so we handle all of them.
6. A new peer_start_openingd() is called after connectd hands us a peer.
7. json_fund_channel just looks through local peers; there are none
   hidden in connectd any more.
8. We sometimes start a new openingd just to send an error message.

Openingd:
1. We always have information we need to accept them funding a channel (in
   the init message).
2. We have to listen for three fds: peer, gossip and master, so we opencode
   the poll.
3. We have an explicit message to start trying to fund a channel.
4. We can be told to send a message in our init message.

Testing:
1. We don't handle some things gracefully yet, so two tests are disabled.
2. 'hand_back_peer .*: now local again' from connectd is no longer a message,
   openingd says 'Handed peer, entering loop' once its managing it.
3. peer['state'] used to be set to 'GOSSIPING' (otherwise this field doesn't
   exist; 'state' is now per-channel.  It doesn't exist at all now.
4. Some tests now need to turn on IO logging in openingd, not connectd.
5. There's a gap between connecting on one node and having connectd on
   the peer hand over the connection to openingd.  Our tests sometimes
   checked getpeers() on the peer, and didn't see anything, so line_graph
   needed updating.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-09 19:44:27 +02:00
Christian Decker 58709cf190 pytest: Migrate connection tests to new fixture model 2018-08-07 00:54:19 +00:00