Commit Graph

96 Commits

Author SHA1 Message Date
Rusty Russell 6195d953cc plugins: use "slow" feerate for mutual close negotiation.
We're rarely in a hurry here, and bitcoind is aggressive with fees.
You can always spend this output if you really have to, using CPFP.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: mutual closing feerate reduced to "slow" to avoid overpaying.
2020-10-13 20:53:34 +02:00
Rusty Russell 711133e3c7 pytest: fix flaky test.
l2 only gives up 100 blocks after it ignores the HTLC, so sometimes
this test fails.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-10-09 15:20:03 +10:30
Christian Decker ee0ee046ff pytest: Stabilize test_penalty_htlc_tx_timeout
lightning-5 can sometimes see itself sweeping the unilateral output resulting
in this weird line:

```log
HTLC already resolved by SELF when we found preimage
```
2020-10-07 09:36:30 +10:30
Jan Sarenik a777d21fb7 pytest: Rename all fund_channel to fundchannel 2020-09-25 21:04:56 +02:00
ZmnSCPxj jxPCSnmZ 9587425f2d plugins/spender/fundchannel.c: Make fundchannel a thin layer around multifundchannel.
Changelog-Changed: protocol: `fundchannel` now shuffles inputs and outputs, and no longer follows BIP69.
2020-09-10 15:28:50 +09:30
ZmnSCPxj jxPCSnmZ 5bac63c6a3 tests/test_closing.py: Test new onchaind earth-scorching behavior. 2020-09-09 12:38:19 +09:30
Rusty Russell 3b8c0a7397 sendpsbt: just reserve (maybe bump) inputs on send, don't mark spent.
Marking spent means if the transaction doesn't confirm for some
reason, the user will need to force a rescan to find the funds.  Now
we have timed reservations, reserving for (an additional) 12 hours
should be sufficient.

We also take this opportunity (now we have our own callback path)
to record the tx in the wallet only on success.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-09-08 10:14:42 +09:30
Christian Decker bdad5a6f94 pytest: Stabilize test_onchain_middleman
It wasn't waiting for the tx to be in the mempool
2020-08-27 09:45:35 +09:30
Christian Decker 8f05d28049 pytest: Stabilize test_onchaind_replay 2020-08-27 09:45:35 +09:30
Rusty Russell 8cf2fff91b pytest: generalize tests.
Anchor outputs break many assumptions in our tests:

1. Remove some hardcoded numbers in favor of a fee calc, so we only have to
   change in one place.
   FIXME: This should also be done for elements!
2. Do binary search to get feerate for a given closing fee.
3. Don't assume output #0: anchor outputs perturb them.
4. Don't assume we can make 1ksat channels (anchors cost 660 sats!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-14 11:51:14 +09:30
Rusty Russell 185b377551 pytest: don't wait for channel active message in test_closing.py::test_penalty_htlc_tx_timeout
It might have already happened, and anyway, we wait for the actual state
below.

```
        # make database snapshot of l2
        l2.stop()
        l2_db_path = os.path.join(l2.daemon.lightning_dir, chainparams['name'], 'lightningd.sqlite3')
        l2_db_path_bak = os.path.join(l2.daemon.lightning_dir, chainparams['name'], 'lightningd.sqlite3.bak')
        copyfile(l2_db_path, l2_db_path_bak)
        l2.start()
        sync_blockheight(bitcoind, [l2])

        # push some money from l3->l2, so that the commit counter advances
        l2.rpc.connect(l3.info['id'], 'localhost', l3.port)
>       l2.daemon.wait_for_log('now ACTIVE')

tests/test_closing.py:908:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
contrib/pyln-testing/pyln/testing/utils.py:288: in wait_for_log
    return self.wait_for_logs([regex], timeout)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyln.testing.utils.LightningD object at 0x7f0c145a32d0>
regexs = ['now ACTIVE'], timeout = 60

    def wait_for_logs(self, regexs, timeout=TIMEOUT):
        """Look for `regexs` in the logs.

        We tail the stdout of the process and look for each regex in `regexs`,
        starting from last of the previous waited-for log entries (if any).  We
        fail if the timeout is exceeded or if the underlying process
        exits before all the `regexs` were found.

        If timeout is None, no time-out is applied.
        """
        logging.debug("Waiting for {} in the logs".format(regexs))
        exs = [re.compile(r) for r in regexs]
        start_time = time.time()
        pos = self.logsearch_start
        while True:
            if timeout is not None and time.time() > start_time + timeout:
                print("Time-out: can't find {} in logs".format(exs))
                for r in exs:
                    if self.is_in_log(r):
                        print("({} was previously in logs!)".format(r))
>               raise TimeoutError('Unable to find "{}" in logs.'.format(exs))
E               TimeoutError: Unable to find "[re.compile('now ACTIVE')]" in logs.

contrib/pyln-testing/pyln/testing/utils.py:264: TimeoutError
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-13 10:39:56 +09:30
Rusty Russell 01a82d38f7 pytest: add slow_test marker.
And when it's set, and we're SLOW_MACHINE, simply disable valgrind.

Since Travis (SLOW_MACHINE=1) only does VALGRIND=1 DEVELOPER=1 tests,
and VALGRIND=0 DEVELOPER=0 tests, it was missing tests which needed
DEVELOPER and !VALGRIND.

Instead, this demotes them to non-valgrind tests for SLOW_MACHINEs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-07 13:57:31 +02:00
Rusty Russell fde353ab00 pytest: use get_nodes more widely.
I started replacing all get_node() calls, but got bored, so then just did the
tests which call get_node() 3 times or more.

Ends up not making a measurable speed difference, but it does make some
things neater and more standard.

Times with SLOW_MACHINE=1 (given that's how Travis tests):

Time before (non-valgrind):
	393 sec (had 3 failures?)
Time after (non-valgrind):
	410 sec

Time before (valgrind):
	890 seconds (had 2 failures)
Time after (valgrind):
	892 sec

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-07 13:57:31 +02:00
Rusty Russell 7aa40b6bae pytest: fix assumption in test_onchain_their_unilateral_out
In fact, the 5 blocks generate above were not seen by nodes until
later, making me very confused.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-07 13:57:31 +02:00
Christian Decker dad23066dc pytest: Disable MPP sending for 4 tests
These mostly deal with exact HTLC counts, and fixed number of attempts to
conclusion, so the randomization that MPP adds is not desirable.
2020-07-15 11:32:58 +02:00
Rusty Russell 1708fb91e5 wallet: clean up json output creation, part 2.
It looked like we weren't printing the address on closing outputs.

But we are, because the 'scriptPubkey' field is in the 'outputs' db
table since 0.7.3 (66a47d2761).

So make the logic clearer, and remove a completely bogus comment (UTXOs
with closing_info are definitely spendable!).

We export the json_add_utxos() for future use, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-07-06 19:25:05 +02:00
niftynei 20193496bb tests: use chain's directory for finding sqlite3 file (works for liquid-regtests)
Move hardcoded 'regtest' to chainparams one
2020-07-01 19:50:02 -05:00
Rusty Russell 6b3bcd49d2 tests: fix flake in test_penalty_htlc_tx_fulfill
If the daemon already knows about the channel before it was stopped,
it won't get this message from gossipd.  That's OK, since we explicitly
test for the channel being active two lines down.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-06-30 13:50:30 +00:00
lisa neigut 487771aee5 tests: skip longer htlc tests on valgring+slow machines
These guys take a while to run, so let's just skip them on the
valgrind/slow-machine combos :/
2020-05-12 15:46:58 +09:30
lisa neigut 6e4b4c50f0 coin tests: try to make a bit more travis robust 2020-05-12 15:46:58 +09:30
lisa neigut 087ab166b3 coins, fix: don't crash if the to_us amount is greater than our_msat
It's possible for our peer to publish a commitment tx that has already
updated our balance for an htlc before we've completed removing it from
our commitment tx (aka before we've updated our balance). This used to
crash, now we just update our balance (and the channel balance logs!)
and keep going.

If they've removed anything from our balance, we'll end up counting it
as chain_fees below. Not ideal but fine... probably.
2020-05-12 15:46:58 +09:30
lisa neigut 9caf20f636 coin moves: don't log coin moves in onchaind if we're replaying
On node start we replay onchaind's transactions from the database/from
our loaded htlc table.  To keep things tidy, we shouldn't notify the
ledger about these, so we wrap pretty much everything in a flag that
tells us whether or not this is a replay.

There's a very small corner case where dust transactions will get missed
if the node crashes after the htlc has been added to the database but
before we've successfully notified onchaind about it.

Notably, most of the obtrusive updates to onchaind wrappings are due to
the fact that we record dust (ignored outputs) before we receive
confirmation of its confirmation.
2020-05-12 15:46:58 +09:30
lisa neigut 8c986d67db coin moves: test all of the onchaind variants
Mostly we update existing tests to account for channel balances. In a
few places, new tests were needed as there wasn't an existing pathway
that tested the chain-fees for a few penalty cases
2020-05-12 15:46:58 +09:30
lisa neigut 6ee6cdc280 coin moves: record their attempted cheat (and our handling thereof)
For cheats, we do a little bit of weird accounting. First we 'update'
our on-ledger balance to be the entirety of the channel's balance. Then,
as outputs get resolved, we record the fees and outputs as withdrawals
from this amount.

It's possible that they might successfully 'cheat', in which case we
record those as 'penalty' but debits (not credits).
2020-05-12 15:46:58 +09:30
Rusty Russell 4df9b2c5f2 pytest: fix flake in test_onchain_different_fees()
We didn't wait until l2 processed the final state of HTLC #2, so
it might not include it when it drops onchain, leading to us only
getting 3 (not 4) sendrawtx calls.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-05-04 18:56:33 +09:30
lisa neigut 0e20e3c5e7 df: rename 'funder' to 'opener'
Previously we've used the term 'funder' to refer to the peer
paying the fees for a transaction; v2 of openchannel will make
this no longer true. Instead we rename this to 'opener', or the
peer sending the 'open_channel' message, since this will be universally
true in a dual-funding world.
2020-05-04 10:22:26 +09:30
Christian Decker e7b555dd90 pytest: Stabilize test_penalty_{in,out}htlc tests
They were looking for specific amounts which proved to be rather flaky. Now
they look for specific outputs being available in the `listfunds` result after
everything was settled.
2020-05-01 18:20:33 -05:00
Vasil Dimov 158d2212c2 closingd: configurable closing fee negotiation step
When negotiating the transaction fee for closing a channel [1], we used
to always pick the middle of the range between our proposal and the
peer's proposal.

Introduce a new option `fee_negotiation_step` to the close command, so
the peer who initiates the close can choose his back off step.

Partially resolves https://github.com/ElementsProject/lightning/issues/3270

[1] https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed

Changelog-Added: New optional parameter to the `close` command to control the closing transaction fee negotiation back off step
2020-04-07 13:52:48 +09:30
darosior 7c0af81c21 bcli: use a more urgent feerate for HTLCs and penalty transactions
A CONSERVATIVE/3 target for them.

Some noisy changes to the tests as we had to update the estimatesmartfee
mock.

Changelog-Changed: We now use a higher feerate for resolving onchain HTLCs and for penalty transactions
2020-04-01 23:02:47 -05:00
Rusty Russell 5d4620484c lightningd: remove json_close `force` option.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSON: `close` `force` parameter removed (deprecated in 0.7.2.1)
2020-03-30 12:47:01 +02:00
darosior dce2e87928 chaintopology: better feerate targets differentiation
We kept track of an URGENT, a NORMAL, and a SLOW feerate. They were used
for opening (NORMAL), mutual (NORMAL), UNILATERAL (URGENT) transactions
as well as minimum and maximum estimations, and onchain resolution.

We now keep track of more fine-grained feerates:
- `opening` used for funding and also misc transactions
- `mutual_close` used for the mutual close transaction
- `unilateral_close` used for unilateral close (commitment transactions)
- `delayed_to_us` used for resolving our output from our unilateral close
- `htlc_resolution` used for resolving onchain HTLCs
- `penalty` used for resolving revoked transactions

We don't modify our requests to our Bitcoin backend, as the next commit
will batch them !

Changelog-deprecated: The "urgent", "slow", and "normal" field of the `feerates` command are now deprecated.
Changelog-added: The fields "opening", "mutual_close", "unilateral_close", "delayed_to_us", "htlc_resolution" and "penalty" have been added to the `feerates` command.
2020-03-30 20:17:18 +10:30
Christian Decker e99ab03f79 pytest: Fix test_closing_fee regression in elements
Constants once again.
2020-03-24 09:52:33 +10:30
Vasil Dimov 66eca2ff3f lightningd: always broadcast the latest close tx
Before this patch we would only update `channel->last_tx` with the newly
proposed closure tx from the peer if the fee of the new one was lower.

In negotiations where we are at the higher end and the peer starts
lower, all peer's subsequent proposals will be higher than his initial
proposal and in this case we would never update `channel->last_tx`
and would wrongly broadcast his initial proposal at the end of the
negotiation.

Fixes https://github.com/ElementsProject/lightning/issues/3549

Changelog-Fixed: Always broadcast the latest close transaction at the end of the close fee negotiation, instead of sometimes broadcasting the peer's initial closing proposal.
2020-03-10 11:40:19 +10:30
darosior 4c5862eab1 pytest: fixup flaky test_closing_specified_destination 2020-02-12 11:45:07 +10:30
darosior dc3aa33927 pytest: use pyln.client for functional tests 2020-01-21 22:38:14 +01:00
Vasil Dimov 02dc261d73 tests: fix comments about the number of nodes
A fully connected network of 10 nodes has 45 connections
in total, not 55.

Changelog-None
2020-01-06 12:57:59 +01:00
Christian Decker 483b00392e pytest: Skip test_closing_torture under valgrind
It's really too slow under valgrind, so skip it. Otherwise it'll just fail the
entire run in most cases.
2020-01-02 16:05:52 +01: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
Christian Decker 6cc60f57a4 pytest: Parametrize test_deprecated_closing_compat for elements 2019-12-11 16:18:34 +01:00
Christian Decker 43dfb4dae7 pytest: Stabilize and parameterize closing_specified_destination
This test was doing a few contortions, so I disentangled it a bit and
parametrized it
2019-12-11 16:18:34 +01: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 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
trueptolemy 9d4cccda19 pytest: Add the test for compact of deprecated close style 2019-10-09 21:04:16 -05:00
trueptolemy 63af8632b7 pytest: Test `close` to the specified destination 2019-10-09 21:04:16 -05:00
Christian Decker 465a12ffbe elements: Parametrize tests to consider chainparams 2019-10-03 04:32:57 +00:00
Christian Decker d89962213c pytest: Allow a bit more slack when running in elements
The fee outputs contributes to our fee, so allow for an extra 5k to go to
fees.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-10-03 04:32:57 +00:00
Christian Decker 67fc8ffbcf elements: Use chainparams when the test relies on addresses
We were checking against hardcoded hrp and prefixes. Now we parametrize via
the chainparams.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-10-03 04:32:57 +00:00
Christian Decker 6b26ce5764 db: Split the vars table to have type-specific columns
This was weird right from the start, so we just split the table into integers
and blobs, so each column has a well-defined format. It is also required for
postgres not to cry about explicit casts in the `paramTypes` array.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2019-09-22 02:03:43 +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 a35677fa0f fundchannel: use plugin; delete interior impl
Switch over to using the fundchannel plugin.
2019-09-11 23:56:27 +00:00