Commit Graph

2012 Commits

Author SHA1 Message Date
Rusty Russell
e8d2176e6b pytest: protect against bad gossip messages from mining confirms too fast.
If we fund a channel between two nodes, then mine all the blocks to
announce it, any other nodes may see the announcement before the
blocks, causing CI to complain about "bad gossip":

```
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG   032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Ignoring future channel_announcment for 113x1x1 (current block 112)
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG   032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 113x1x1/0
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG   032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 113x1x1/1
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG   032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Bad gossip order: WIRE_NODE_ANNOUNCEMENT before announcement 032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e
```

Add a new helper for this case, and use it where there are more than 2 nodes.

Cleans up test_routing_gossip and a few other places which did this manually.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell
6d9f6ffd67 pytest: make test_gossip_no_empty_announcements more robust.
Don't assume gossip send order: explicitly disconnect and reconnect.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell
f7f9f35f2a pytest: remove flake in test_upgrade_statickey_onchaind
We were relying on the fee update to create an additional tx.  That's
ugly; do an actual payment and make sure we definitely complete a new
tx by waiting for that *then* both revoke_and_ack.

(Without this, we could get a unilateral close instead of a penalty).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell
1c71c9849b connectd: handle custom messages.
This is neater than what we had before, and slightly more general.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON_RPC: `sendcustommsg` now works with any connected peer, even when shutting down a channel.
2022-02-08 11:15:52 +10:30
Rusty Russell
0ac5c4f8df pytest: ignore pings when doing query_gossip.
Next patch starts a timeout ping, which can interfere with results.

In theory, we should reply, but in practice (so far!) we seem to get enough
time that it doesn't hang up on us.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell
5065bd6fc2 lightningd: use our cached channel_update for errors instead of asking gossipd.
We also no longer strip the type off: everyone handles both forms, and
Eclair doesn't strip (and it's easier!).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell
430a380e35 gossipd: feed lightningd the channel_updates as soon as we make them.
Even if we're deferring putting them in the store and broadcasting them,
we tell lightningd so it will use it in any error messages.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-02-08 11:15:52 +10:30
Rusty Russell
c0e3155bb6 pytest: update elements for new, more accurate feerate calc.
And skip some tests: I'd simply be pasting in the results, which is
not very useful.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-27 12:22:36 +01:00
Rusty Russell
de28bbd792 closingd, lightningd: use bitcoin_tx_2of2_input_witness_weight
This fixes lightningd's chronic weight underestimate.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: closingd: more accurate weight estimation helps mutual closing near min/max feerates.
2022-01-27 12:22:36 +01:00
Rusty Russell
169c113d15 pytest: test that our closingd tx weight estimate is correct.
Compare against lightningd's, and the actual tx.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-27 12:22:36 +01:00
niftynei
4dafeede5c coin moves: notify when we make deposits to external accounts
The blockheight is zero though, since these aren't included in a block
yet.

We also don't issue an 'external' deposit event if we can tell that the
address you're sending to actually belongs to our wallet (we'll issue a
deposit event when it gets included in a block)
2022-01-26 13:34:45 +10:30
Rusty Russell
16dd091d8b pytest: fix flake in test_upgrade_statickey_fail
```
        l1.rpc.disconnect(l2.info['id'], force=True)
        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
    
>       l1.daemon.wait_for_log('option_static_remotekey enabled at 2/2')

tests/test_connection.py:3653: 
```

If l2's channeld gets killed (due to reconnect) before it tells
lightningd it got the revoke_and_ack it will need a retransmission
*again*.

This makes the test more robust, and does more checks too. 

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-25 06:26:52 +10:30
Rusty Russell
3281d72169 pytest: clean up test_channel_state_changed_unilateral
OK, now this test makes more sense!  Now we don't ignore errors, we
*will* drop to chain if we reconnect after one side has dropped to
chain.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-25 06:26:52 +10:30
Rusty Russell
fc44846e10 pytest: disable test_closing_different_fees for elements temporarily.
There's actually a bug in our closing tx size estimation; I'll do
a separate patch for this, though.

Seems this used to be flaky, now we always flush queues, so it's
more reliably caught.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
3ccb3da2c5 pytest: disable automatic reconnection.
We seem to hit a race between manual reconnect (with address hint) and an automatic
reconnection attempt which fails:

```
 >       l4.rpc.connect(l3.info['id'], 'localhost', l3.port)
...
 E           pyln.client.lightning.RpcError: RPC call failed: method: connect, payload: {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'host': 'localhost', 'port': 41285}, error: {'code': 401, 'message': 'All addresses failed: 127.0.0.1:36678: Connection establishment: Connection refused. '}
```

See how it didn't even try the given address?

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
109b3e62e9 pytest: disable tests/test_closing.py::test_onchain_all_dust
Here's the "Normal Tet Config clang-fuzzing" setup where it fails:

```
CC=clang
CONFIGURATOR_CC=clang
CWARNFLAGS=-Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -Werror
CDEBUGFLAGS=-std=gnu11 -g -fstack-protector-strong
COPTFLAGS=
SQLITE3_CFLAGS=
SQLITE3_LDLIBS=-lsqlite3
POSTGRES_INCLUDE=-I/usr/include/postgresql
POSTGRES_LDLIBS=-L/usr/lib/x86_64-linux-gnu -lpq
VALGRIND=0
DEVELOPER=1
EXPERIMENTAL_FEATURES=0
COMPAT=1
```

Here's the truncated test output:

```
        if anchor_expected():
            expected_1['B'].append(('external', ['anchor'], None, None))
            expected_2['B'].append(('external', ['anchor'], None, None))
            expected_1['B'].append(('wallet', ['anchor'], None, None))
            expected_2['B'].append(('wallet', ['anchor'], None, None))
    
        tags = check_utxos_channel(l1, [channel_id], expected_1)
>       check_utxos_channel(l2, [channel_id], expected_2, tags)

tests/test_closing.py:2662: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/utils.py:321: in check_utxos_channel
    txid = matchup_events(u_set, evs, chans, tag_list)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

u_set = [[{'account_id': 'external', 'blockheight': 104, 'coin_type': 'bcrt', 'credit': '0msat', ...}, {'account_id': 'external', 'blockheight': 110, 'coin_type': 'bcrt', 'credit': '0msat', ...}]]
evs = [('external', ['to_them'], None, None), ('external', ['htlc_timeout'], None, None)]
chans = ['8ede62cea34c5196467c68175f70b8915f0edda421c5087a99584d4197cfb6c4']
tag_list = {'A': 'c5b6cf97414d58997a08c521a4dd0e5f91b8705f17687c4696514ca3ce62de8e', 'B': 'bb09b25d6653aeeec188961347ff80e90dca6f4a29cc017856f6585adb8cb468'}

    def matchup_events(u_set, evs, chans, tag_list):
        assert len(u_set) == len(evs) and len(u_set) > 0
    
        txid = u_set[0][0]['utxo_txid']
        for ev in evs:
            found = False
            for u in u_set:
                # We use 'cid' as a placeholder for the channel id, since it's
                # dyanmic, but we need to sub it in. 'chans' is a list of cids,
                # which are mapped to `cid` tags' suffixes. eg. 'cid1' is the
                # first cid in the chans list
                if ev[0][:3] == 'cid':
                    idx = int(ev[0][3:])
                    acct = chans[idx - 1]
                else:
                    acct = ev[0]
    
                if u[0]['account_id'] != acct or u[0]['tags'] != ev[1]:
                    continue
    
                if ev[2] is None:
>                   assert u[1] is None
E                   AssertionError
```
2022-01-20 15:24:06 +10:30
Rusty Russell
e366cb17f6 pytest: fix flake in test_reconnect_sender_add1
l1 might split in a commitment_signed before it notices the disconnect, and this test fails:

```
        for i in range(0, len(disconnects)):
            with pytest.raises(RpcError):
                l1.rpc.sendpay(route, rhash, payment_secret=inv['payment_secret'])
>               l1.rpc.waitsendpay(rhash)
E               Failed: DID NOT RAISE <class 'pyln.client.lightning.RpcError'>
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
6115ed02e8 subdaemons: don't stream gossip_store at all.
We now let gossipd do it.

This also means there's nothing left in 'struct per_peer_state' to
send across the wire (the fds are sent separately), so that gets
removed from wire messages too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
029d65cf2e connectd: serve gossip_store file for the peer.
We actually intercept the gossip_timestamp_filter, so the gossip_store
mechanism inside the per-peer daemon never kicks off for normal connections.

The gossipwith tool doesn't set OPT_GOSSIP_QUERIES, so it gets both, but
that only effects one place.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
e37a638c0c connectd: do nagle by packet type.
channeld can't do it any more: it's using local sockets.  Connectd
can do it, and simply does it by type.

Amazingly, on my machine the timing change *always* caused
test_channel_receivable() to fail, due to a latent race.

Includes feedback from @cdecker.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
7a514112ec connectd: do dev_disconnect logic.
As connectd handles more packets itself, or diverts them to/from gossipd,
it's the only place we can implement the dev_disconnect logic.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Rusty Russell
8f3b390356 pytest: make test_channel_state_changed_unilateral more robust.
This test started mostly failing (in non-DEVELOPER mode) after the
next patch, due to timing issues.

Handle both cases for now, and we'll add more enhancements later to
things we should be handling more consistently.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-01-20 15:24:06 +10:30
Michael Schmoock
019b31c522 options: only allow one DNS announcement 2022-01-02 16:01:38 +01:00
Rusty Russell
b659fbbdf7 pytest: further deflake test_funding_push.
I also got an error under CI; it seems the sleep() was insufficient.
So try adding a sleep inside the check_coin_moves, which should cover
everyone.

```
                acct_moves = acct_moves[number_moves:]
            else:
>               if not move_matches(m, acct_moves[0]):
E               IndexError: list index out of range
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-30 14:36:55 +10:30
Rusty Russell
70ed47d77a channeld: add dev-disable-commit-after instead of dev-disconnect -nocommit
It was always a hack, but an impossible one once connectd will be
interpreting dev-disconnect!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-30 09:50:40 +10:30
Rusty Russell
888745be16 dev_disconnect: remove @ marker.
Once connectd is doing this, we can't close as soon as we send,
and in fact we can't do 'fail write' either.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-30 09:50:40 +10:30
Rusty Russell
560fa06f42 pytest: disable dev-disconnect blackhole tests.
These would have to be done by connectd, not the local daemon, once it's
intermediating.  Otherwise the remote peer won't see any change.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-30 09:50:40 +10:30
niftynei
70a73928cb balance-snaps: add a balance_snapshot event; fires after first catchup
Fire off a snapshot of current account balances (node wallet + every
'active' channel) after we've caught up to the chain tip for the *first*
time (in other words, on start).
2021-12-28 04:42:42 +10:30
niftynei
f169597a02 test plugins/coin_moves: clean up/tidy coin moves plugin logic
removes unused in-memory stash of coin-moves; don't write the entire set
of coinmoves out on every update.
2021-12-28 04:42:42 +10:30
niftynei
8225a9decf coin_mvt: log events for pushes/lease_fees for leased channels
We need to stash/save the amount of the lease fees on a leased channel,
we do this by re-using the 'push' amount field on channel (which is
technically correct, since we're essentially pushing the fee amount to
the peer).

Also updates a bit of how the pushes are accounted for (pushed to now
has an event; their channel will open at zero but then they'll
immediately register a push event).

Leases fees are treated exactly the same as pushes, except labeled
differently.

Required adding a 'lease_fee' field to the inflights so we keep track of
the fee for the lease until the open happens.
2021-12-28 04:42:42 +10:30
niftynei
bddd3694fa coin_mvt: record fees for an outbound htlc
If we initialized the payment, the fees are the entire fee-chain
(final hop amount - starting hop amount)

If it's a payment we routed, the fees are the diff between the
inbound htlc and the outbound (net gain by this routing)

Added to database so data persists nicely.
2021-12-28 04:42:42 +10:30
niftynei
29c6718297 coin_mvt: record new 'fees' field on htlc channel moves
We record the amount of fees collected for a routed payment. For
simplicity's sake on the data agg side, we record the fee payment on
*BOTH* the incoming htlc and the outgoing htlc. Note that this results
in double counting if you add up the fees from both an in-routed and
out-routed payment.
2021-12-28 04:42:42 +10:30
niftynei
b6463174d6 coin moves: turn 'tag' into 'tags' array, add OPENER tag
Channels that the node has hopened will now be tagged with 'opener' in a
list of tags.
2021-12-28 04:42:42 +10:30
niftynei
4506f639fa coin_moves: remove 'index' for moves; bump version
Get rid of the 'movement_idx', since we replay events now.

Since we're removing a field from the 'coin_movement' event emission, we
bump the version type.

Changelog-Updated: `coin_movements` events have been revamped and are now on version 2.
2021-12-28 04:42:42 +10:30
niftynei
27ff87c045 coin_moves: de-dupe moves as they come in
test_onchain_dust_out restarts a node, which produces duplicate events.
this is expected, but we need to de-duplicate the events stream to get
accurate results
2021-12-28 04:42:42 +10:30
niftynei
d2c4d4aec2 coin_mvts: rewrite how onchain events are recorded, update tests
The old model of coin movements attempted to compute fees etc and log
amounts, not utxos. This is not as robust, as multi-party opens and dual
funded channels make it hard to account for fees etc correctly.

Instead, we move towards a 'utxo' view of the onchain events. Every
event is either the creation or 'destruction' of a utxo. For cases where
the value of the utxo is not (fully) debited/credited to our account, we
also record the output_value. E.g. channel closings spend a utxo who's
entire value we may not own.

Since we're now tracking UTXOs onchain, we can now do more complex
assertions about the onchain footprint of them. The integration tests
have been updated to now use more 'chain aware' assertions about the
ending state.
2021-12-28 04:42:42 +10:30
Vincenzo Palazzo
43ff949ea7 lightningd: support hsm error code
Suggested-by: Rusty Russell
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Changed: Support hsm specific error error code in lightning-cli
2021-12-15 12:24:54 +10:30
Rusty Russell
1ac5a1b7e2 pytest: restore gossip advertizement tests for default port.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-14 13:41:31 +10:30
Simon Vrouwe
28816c387c testing: remove test_htlc_accepted_hook_shutdown, move relevant parts
into test_plugin_shutdown

The fact that plugins are kept alive untill the very end also ensures
their hooks are not silently unregistered.
2021-12-14 09:33:10 +10:30
Simon Vrouwe
9b70c9d63b testing: fix flake in test_fetchinvoice_3hop 2021-12-14 09:33:10 +10:30
Michael Schmoock
c2d2cc1274 connectd: fix empty error message
1. Adds the missing DNS error massages so they can be handled by
   connect_control.
2. Prepends a 'All addresses failed' to code 401 message, so we
   always have at least some error message to the user.

Changelog-None
2021-12-08 13:52:24 +01:00
Michael Schmoock
bad09887e0 pytest: fix test_pluginconnected_hook_chaining
The last line of the testcase was checking on the wrong node l3
instead of l1. l3 didn't had the plugins configured that would
produce the log entry we were looking for not to be present.

Changelog-None
2021-12-06 14:53:37 +01:00
Jan Sarenik
96f28323bd Set default port according to network
The idea is to have different default ports for different networks.

Current default port is `9735` for everything. Let's use it for
the mainnet and reuse the difference added to the default port
from `rpc_port` values in `bitcoin/chainstate.c`.

Testnet would be `19735` (adding rpc_port - 8332 = `10000`).

Signet would be `39735` (adding rpc_port - 8332 = `30000`).

Regtest would be `19846` (adding rpc_port - 8332 = `10111`).

With Vincenzo's kind pair-programming help over tmate.

Two other commits were squashed into this one so that bisecting
never ends up in half-baked state:

1. chainparams: Fix regtest default rpc_port

   bitcoind -help says this:

    -rpcport=<port>
         Listen for JSON-RPC connections on <port> (default: 8332, testnet:
         18332, signet: 38332, regtest: 18443)

2. test_gossip: Default port for regtest

   hex: 2607 is now .... (could be 4d86 but Elements uses another port)
   dec: 9735 is now any port (could be 19846 ^^ but now is for any port)

   The lines which were binding to default port were removed as the
   default port is different on each network.

NOTE: Remember not to modify gossip_store tests which loads everything raw
      including the checksums.

Changelog-Changed: If the port is unspecified, the default port is chosen according to used network similarly to Bitcoin Core.
2021-12-06 17:10:08 +10:30
Rusty Russell
4ffda340d3 check: make sure all files outside contrib/ include "config.h" first.
And turn "" includes into full-path (which makes it easier to put
config.h first, and finds some cases check-includes.sh missed
previously).

config.h sets _GNU_SOURCE which really needs to be done before any
'#includes': we mainly got away with it with glibc, but other platforms
like Alpine may have stricter requirements.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-06 10:05:39 +10:30
Rusty Russell
bdabef9a9b pytest: fix occasional flake in test_announce_address
We make sure the gossip msg was sent, but the other node might not
have digested it yet:

```
        # Check other node can parse these
>       addresses = l2.rpc.listnodes(l1.info['id'])['nodes'][0]['addresses']
E       KeyError: 'addresses'
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-06 10:05:18 +10:30
Rusty Russell
b45544c659 options: fix handling of wildcard (allproto) address.
We treated ':' as an empty DNS name in EXPERIMENTAL, which is wrong.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-05 16:43:09 +01:00
Rusty Russell
cf40e585c3 pytest: test to show conflict between websocket and wildcard addresses.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-05 16:43:09 +01:00
Andrew Toth
69bc1191cb tests: add pay test for exclude arg 2021-12-04 21:37:42 +10:30
Rusty Russell
5a5cf8c696 pytest: fix flake in testing.
As noted in 0a406230d0 (diff-5871d4c569454db5e625383975462132da0bd03d32df145d8d72d8fafd86d952R3544-R3546)

Turns out we sometimes hang up before l2 sees the previous tx revoked,
so we get a normal unilateral close, not a cheat.

Reported-by: Simon Vrouwe
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-01 09:48:00 +10:30
Rusty Russell
605b3bf985 pytest: re-enable modern/obsolete fetchonion tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2021-12-01 05:44:28 +10:30