Commit Graph

580 Commits

Author SHA1 Message Date
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 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 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 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 7925469f88 pytest: fix flaky assert in test_htlc_send_timeout.
If feerates change, L2 sends L3 a commit for that, which causes us to
fail the assert (which says we won't send a commitment_signed).

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 ede05ecd40 pytest: demonstrate feerate on restart bug.
We don't update a channel's feerate on reestablishment: we insert a restart
in test_onchain_different_fees() (which we'll need soon anyway) to show it.

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 4c891f4661 common: log when we toggle IO logging, don't edit env in tests!
Tests were failing when in the same thread after a test which set
log_all_io=True, because SIGUSR1 seemed to be turning logging *off*.

This is due to Python using references not copies for assignment.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Rusty Russell 9f044305db pytest: dev env var LIGHTNINGD_DEV_LOG_IO turns io logging on immediately.
This is required for the next test, which has to log messages from channeld
as soon as it starts (so might be too late if it sends SIGUSR1).

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 f5143d9549 pytest: fail if we see 'bad reestablish' in the logs.
Really, we should use log-level more cleverly here.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 14:46:22 +02:00
Christian Decker 8f56d64a1f log: Append the current time to the crash log filename
This should make it easier to identify the latest crash file and correlate
crashes with external monitoring tools.
2018-08-23 12:51:08 +02:00
Rusty Russell 00696277d2 logging: always dump a crash log, but make files per-pid.
Someone had a 21GB crash.log, which doesn't help anyone!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 12:51:08 +02:00
Rusty Russell 213be90e77 log: implement reopening log-file on SIGHUP
Closes: #1623
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-23 12:51:08 +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 cda6c97a57 pytest: allow NodeFactory to create non-started peers.
Useful it we want to intercept bitcoin-cli first.

We move the getinfo() caching into start(), as that's when we can actually
use RPC.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-22 12:13:23 +02:00
Rusty Russell 7eed9cba90 pytest: always override estimatesmartfee.
It (almost?) always fails for regtest; best to override it directly.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-22 12:13:23 +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 bc7ca34a38 pytest: fix race in test_closing_different_fees
We got an index error, because status had only one field (onchaind not
started yet).

    >   wait_for(lambda: only_one(p.rpc.listpeers(l1.info['id'])['peers'][0]['channels'])['status'][1] == 'ONCHAIN:Tracking mutual close transaction')
    E   IndexError: list index out of range1

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-21 00:48:02 +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
Rusty Russell d280eabc1e pytest: add a resart to test_permfail that we don't forget on restart.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-21 00:48:02 +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
Rusty Russell 3454ca57a2 pytest: remove DEVELOPER guard for test_ping
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-15 06:48:55 +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 9d8b3a070b pytest: make test_htlc_send_timeout more reliable.
Waiting for three node_announcments isn't always enough, since l2 can
publish two of them (an independent bug).  Do the more Right Thing and
just wait for 30 seconds of no input...

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-10 12:46:45 +02:00
Rusty Russell 65c882ca3a Minor cleanups.
1. connect convenience variable for improved readabilty.
2. a comment explaining that timer is on channel, not HTLC.
3. use modern python style in test_htlc_send_timeout

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-10 12:46:45 +02:00
Rusty Russell 63e4ea17af channeld: don't commit until we've seen recent incoming msg, ping if required.
Now sending a ping makes sense: it should force the other end to send
a reply, unblocking the commitment process.

Note that rather than waiting for a reply, we're actually spinning on
a 100ms loop in this case.  But it's simple and it works.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2018-08-10 12:46:45 +02:00