Commit Graph

2290 Commits

Author SHA1 Message Date
Alex Myers 0abe2e3af1 pytest: Add debugging to test_gossip_store_compact_on_load
This flake has been difficult to reproduce, so let's dump the gossip
store to aid in debugging. See issue #5410.

Changelog-None
2022-08-26 07:03:10 +09:30
adi2011 8f78a76d1a tests/test_misc.py: check logs for already existing channel. 2022-08-19 10:27:09 -04:00
Christian Decker 65549a2931 ld: Fix a log message assuming that the `channel->scid` was set
This is no longer true after we introduce `zeroconf`, so use the alias
local alias instead if not set.

Signed-off-by: Christian Decker <@cdecker>
2022-08-16 14:52:52 -05:00
Christian Decker ab1ca7f159 pytest: Reproduce a crash when we have multiple channels and 0conf
This is a case where we assume that the `channel->scid` is set, which
is no longer true with `zeroconf`

Changelog-None: Zeroconf was not yet released at the time the discovery was made

Reported-by: Yaacov Akiba Slama <@yaslama>
Reported-by: Roei Erez <@roeierez>
2022-08-16 14:52:52 -05:00
niftynei 4984014578 signpsbt: add utxo info to inputs
If you build a PSBT externally from CLN and attempt to sign for the
output, we would crash. Now we don't crash.

Changelog-Changed: JSON-RPC: `signpsbt` will now add redeemscript + witness-utxo to the PSBT for an input that we can sign for, before signing it.

Fixes #5499 ?
2022-08-13 12:57:03 -05:00
Ken Sedgwick 0878002fe6 Fix derived_secret, use correct size of secretstuff.derived secret
[ Updated tests to match -- RR]
2022-08-10 12:41:27 -05:00
Rusty Russell b173b29346 pytest: test loading lease_chan_max_msat from channel_funding_inflights
It crashes with:

```
s32 field doesn't match size: expected 4, actual 8
```

Reported-by: @zerofeerouting
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-10 10:34:12 -05:00
niftynei 9d3bf4a1b5 bkpr: let channel reconnect, flake? 2022-08-10 10:30:45 -05:00
niftynei 72a30fc750 bkpr: dont flake, wait til pay done before mining blocks 2022-08-10 10:30:45 -05:00
niftynei 23cd58402a bkpr: create accounts for zero sat channels
we weren't making records for 'missed' accounts that had a zero balance
at snapshot time (if peer opens channel and is unused)

Fixes: #5502
Reported-By: https://github.com/niftynei/cln-logmaid
2022-08-10 10:30:45 -05:00
Rusty Russell 9543204b79 pytest: don't use bogus scids for first hop of route.
This was a legacy from when it was redundant: with multiple channels, it
no longer is!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-09 16:33:24 -05:00
Rusty Russell 8a9ce55345 lightningd: don't route private channels via real scid.
Again, we should use the real channel_type, but we approximate.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: private channels will only route using short-channel-ids if channel opened with option_scid_alias-supporting peer.
2022-08-09 16:31:36 -05:00
Rusty Russell cfe6b06fb5 lightnind: use aliases in routehints for private channels.
We *should* remember the channel type, since this is only required
if they set the channel_type to include option_scid_alias.

However, since we support channel upgrade, channel_type really needs
a new table.  I have a patch for that, from my abandoned original
"fastopen" branch for aliases, but it's too big a chance for rc2 IMHO.

Meanwhile, we allow exposeprivatechannels's scids to be either real or
the aliases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: invoice routehints will use fake short-channel-ids for private channels if channel opened with option_scid_alias-supporting peer.
2022-08-09 16:31:36 -05:00
Rusty Russell b479e9a9fa pytest: test that we implement option_scid_alias privacy.
Spoiler: we don't! :(

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-09 16:31:36 -05:00
niftynei 3fcf60ab7c bkpr: track channel rebalances, display in listincome
Track rebalances, and report income events for them.

Previously `listincome` would report:
	- invoice event, debit, outgoing channel
	- invoice_fee event, debit, outgoing channel
	- invoice event, credit, inbound channel

Now reports:
	- rebalance_fee, debit, outgoing channel
	(same value as invoice_fee above)

Note: only applies on channel events; if a rebalance falls to chain
we'll use the older style of accounting.

Changelog-None
2022-08-09 11:57:18 +09:30
Rusty Russell 80a6d9b58e lightningd: set the channel_type feature.
AFAICT we should have been doing this since we started sending and
receiving it, but didn't.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now advertize the `option_channel_type` feature (which we actually supported since v0.10.2)
2022-08-08 11:49:56 -05:00
Rusty Russell 5260ea2911 pytest: make sure we never break channels in multhtlc test.
It's perfectlty safe to extend the timeout to 100 blocks, so let's do
it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-04 11:16:01 -05:00
Rusty Russell 93303ffdad pytest: change multihtlc topology for simpler testing.
We were seeing flakes due to channel closures on intermidiary nodes if
the blocks came too fast.

If we use a star topology it's actually what we want and it's much simpler to
figure out what's going on.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-04 11:16:01 -05:00
Rusty Russell b5ee5e7fb1 pytest: simplify test_onchain_multihtlc_our_unilateral/their_unilateral
We don't actually care how it does it, just that it ends the HTLCs,
so simplify this logic which tries to match it exactly.

This also fixes the flake where we would sometimes close the upstream
channels, simply because we now wait at least one second per block.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-04 11:16:01 -05:00
Rusty Russell 66d8ce7c8c pytest: clarify test_onchain_multihtlc_our_unilateral / test_onchain_multihtlc_their_unilateral
Use the names, not calculations on an array.  It's simply clearer,
especially when debugging.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-08-04 11:16:01 -05:00
niftynei 4e503f7d0a bkpr/listpeeers: add lease_fees back to funds; separate out in listpeers
First, how we record "our_funds" and then apply pushes vs lease_fees
(for liquidity ad buys/sales) was exactly opposite.

For pushes we were reporting the total funded into the channel, with the
push representing how much we'd later moved to the peer.

For lease_fees we were rerporting the total in the channel, with the
push representing how much was already moved to the peer.

We fix this (from a view perspective) by re-adding lease fees to what's
reported in the channel funding totals. Since this is now new behavior
(for leased channel values), we added new fields so we can take the old
field names thru a deprecation cycle.

We also make it possible to differentiate btw a push and a lease_fee
(before they were all the same), by adding to new fields to `listpeers`:
`fee_paid_msat` and `fee_rcvd_msat`.

This allows us to avoid math in the bookkeeper, instead we just pick
the numbers out directly and record them.

Fixes #5472

Changelog-Added: JSON-RPC: `listpeers` now has a few new fields for `funding` (`remote_funds_msat`, `local_funds_msat`, `fee_paid_msat`, `fee_rcvd_msat`).
Changelog-Deprecated: JSON-RPC: `listpeers`.`funded` fields `local_msat` and `remote_msat` are now deprecated.
2022-07-31 21:53:05 +09:30
Rusty Russell 8c9fa457ba pytest: fix flake in test_gossip_timestamp_filter
```
        # 0x0100 = channel_announcement
        # 0x0102 = channel_update
        # (Node announcement may have any timestamp)
        types = Counter([m[0:4] for m in msgs])
        assert types['0100'] == 1
>       assert types['0102'] == 2
E       assert 1 == 2

tests/test_gossip.py:324: AssertionError
```

Examining the logs shows that we ask l4 for timestamps
"first_timestamp=1658892115 timestamp_range=13", and the timestamp on
the missing update is exactly `1658892128`.

So round the end time up by 1 for filtering.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-07-28 15:08:44 +09:30
Rusty Russell 22ff007d64 connectd: control connect backoff from lightningd.
We used to tell connectd to remember our connect delay, and hand it
back (increased if necessary).

Instead, simply record when we last tried to connect.  If it was less
than 10 minutes ago, double delay (up to 5 minutes max), otherwise
reset delay to 1 second.

This covers all scenarios: whether we reconnect then immediately
disconnect, or never successfully connect, it doesn't matter.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #5453
2022-07-28 15:08:44 +09:30
Rusty Russell 9cad7d6a6a lightningd: don't consider AWAITING_UNILATERAL to be "active".
It's not active: we don't want to connect.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-07-28 15:08:44 +09:30
Rusty Russell a259698906 pytest: test that we don't try to reconnect in AWAITING_UNILATERAL.
We would just send an error, and it's annoying.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-07-28 15:08:44 +09:30
niftynei e048292fdf bkpr-zeroconf: Zeroconfs will emit 'channel_proposed' event
Keep the accounts as an 'append only' log, instead we move the marker
for the 'channel_open' forward when a 'channel_open' comes out.

We also neatly hide the 'channel_proposed' events in 'inspect' if
there's a 'channel_open' for that same event.

If you call inspect before the 'channel_open' is confirmed, you'll see
the tag as 'channel_proposed', afterwards it shows up as
'channel_open'. However the event log rolls forward -- listaccountevents
will show the correct history of the proposal then open confirming (plus
any routing that happened before the channel confirmed).
2022-07-28 12:08:18 +09:30
niftynei 30aa1d79fb bkpr: for zerconfs, we still wanna know you're opening a channel
We need a record of the channel account before you start sending
payments through it. Normally we don't start allowing payments to be
sent until after the channel has locked in but zeroconf does away with
this assumption.

Instead we push out a "channel_proposed" event, which should only show
up for zeroconfs.
2022-07-28 12:08:18 +09:30
niftynei 3c79a456c0 test-db-provider: if postgres in tests, startup a bookkeeper db
FXIME: Has a edge case where if you disable the bookkeeper, it'll
blowup because you've got an option that isn't present anywhere...
2022-07-28 12:08:18 +09:30
niftynei e5d3ce3b1f bkpr incomestmt: properly escape things for the CSVs
First off, when we pull data out of JSON, unescape it so we don't end up
with extraneous escapes in our bookkeeping data. I promise, it's worth
it.

Then, when we print descriptions out to the csvs, we gotta wrap
everything in quotes... but also we have to change all the double-quotes
to singles so that adding the quotes doesn't do anything untoward.

We also just pass it thru json_escape to get rid of linebreaks etc.

Note that in the tests we do a byte comparison instead of converting the
CSV dumps to strings because python will escape the strings on
conversion...
2022-07-28 12:08:18 +09:30
niftynei 5146baa00b bkpr csvs: koinly + cointracker only accept fees on the same line
So we print out invoice fees on the same line for those CSVs! This means
we have to do a little bit of gymnastics (but not too bad):
	- we save the fee amount onto the income event now so we can use
it later
	- we ignore every "invoice_fee" event for the koinly/cointracker

Note that since we're not skipping income events in the loops we also
move the newline character to the start of every `_entry` function so
skipped records dont incur empth lines.

Changelog-Added: bkpr: print out invoice fees on the same line for `koinly` and `cointracker` csv types
2022-07-28 12:08:18 +09:30
niftynei 352b419755 bkpr: save invoice description data to the database and display it
It'll be really nice to be able to read description data about an
invoice, if we've got it!
2022-07-28 12:08:18 +09:30
niftynei ab94c557c7 bkpr: add test for bookkeeper being added after channel has closed
We rescan and pick up the channel's close tx, but can't go back and
get the open info, since we've already deleted the channel from the
database.
2022-07-28 12:08:18 +09:30
niftynei 0617690981 coin_mvt/bkpr: add "stealable" tag to stealable outputs
If we expect further events for an onchain output (because we can steal
it away from the 'external'/rightful owner), we mark them.

This prevents us from marking a channel as 'onchain-resolved' before
all events that we're interested in have actually hit the chain.

Case that this matters:
Peer publishes a (cheating) unilateral close and a timeout htlc (which
we can steal).
We then steal the timeout htlc.

W/o the stealable flag, we'd have marked the channel as resolved when
the peer published the timeout htlc, which is incorrect as we're still
waiting for the resolution of that timeout htlc (b/c we *can* steal it).
2022-07-28 12:08:18 +09:30
niftynei c1cef773ca bkpr: make sure there's always at least on difference in blockheights
We can't divide by zero, so make sure it's always 1 (with a small loss
of precision for other cases, ce la vie)
2022-07-28 12:08:18 +09:30
niftynei d72033882f bkpr: check for channel resolution for any "originated" event
We were failing to mark channels as resolved b/c we weren't using later
events to external (but originated from this account) events as signals
to run the channel resolution check.

This fixes that, and adds a test.
2022-07-28 12:08:18 +09:30
niftynei 563910e667 bkpr: add docs, change names to 'bkpr-*'
Adds schema definitions and manpages for bkpr- commands; also renames
the commands to all start with 'bkpr-', so they're easier to identify/
make runes about.
2022-07-28 12:08:18 +09:30
niftynei e2ef44c043 bkpr: add 'msat' suffix to all msat denominated fields
Makes our json schema parsing work as expected.
2022-07-28 12:08:18 +09:30
niftynei a3d82d5a01 bkpr: exclude non-wallet events in the balance snapshot
Anchor outputs are ignored by the clightning wallet, but we keep track
of them in the bookkeeper. This causes problems when we do the balance
checks on restart w/ the balance_snapshot -- it results in us printing
out a journal_entry to 'get rid of' the anchors that the clightning node
doesnt know about.

Instead, we mark some outputs as 'ignored' and exclude these from our
account balance sums when we're comparing to the clightning snapshot.
2022-07-28 12:08:18 +09:30
niftynei eae1236db7 tests,bkpr: liquid fails all these for different reasons
- external wallet not supported yet for elements
- the close fails to propagate b/c the outputs are dusty (FIXME)
2022-07-28 12:08:18 +09:30
niftynei e7ed196f87 bkpr: separate the invoice_fees from the invoice paid
For income events, break out the amount paid in routing fees vs the
total amount of the *invoice* that is paid.

Also printout these fees, when available, on listaccountevents
2022-07-28 12:08:18 +09:30
niftynei 4326c08927 test nit: wait_for_mempool cleanup
Wait for mempool=1 before making a block
2022-07-28 12:08:18 +09:30
niftynei a7b7ea5d49 bkpr: add a 'consolidate-fees' flag to the income stmt
Defaults to true. This is actually what you want to see -- the fees for
an account's txid collapsed into a single entry.
2022-07-28 12:08:18 +09:30
niftynei 83c6cf25d2 bkpr: 'to_miner' spends are considered terminal
This is a rare case where we RBF the output of a penalty until it no
longer has an output value we can reclaim. We ignore the txid for these
events when closing a channel.
2022-07-28 12:08:18 +09:30
niftynei 1dd52ba003 bkpr: mark external deposits (withdraws) via blockheight when confirmed
We issue events for external deposits (withdrawals) before the tx is
confirmed in a block. To avoid double counting these, we don't count
them as confirmed/included until after they're confirmed. We do this
by keeping the blockheight as zero until the withdraw for the input for
them comes through.

Note that since we don't have any way to note when RBF'd withdraws
aren't eligible for block inclusion anymore, we don't really have a good
heuristic to trim them. Which is fine, they *will* show up in account
events however.
2022-07-28 12:08:18 +09:30
niftynei 25f0c76c9a tests: move 'bookkeeper' centric tests to their own file
Should we pull these out into a separate test suite?
2022-07-28 12:08:18 +09:30
niftynei 5f41d9247e bkpr: properly account for onchain fees for channel closes
onchain fees are weird at channel close because:

- you may be missing an trimmed htlc (which went to fees)
- the balance from close may have been rounded (msats cant land on
chain)
- the close might have been a past state and you've actually
  ended up with more money onchain than you had in the channel. wut

This commit accounts for all of this appropriately, with some tests.

channel_close.debit should equal onchain_fee.credit (for that txid)
plus sum(chain_event.credit [wallet/channel_acct]).

In the penalty case, channel_close.debit becomes channel_close.debit +
penalty_adj.debit, i.e.

	channel-close.debit + (penalty_adj.debit) =
		onchain_fee.credit
	      + sum(chain_event.credit [wallet/channel_acct])
2022-07-28 12:08:18 +09:30
Rusty Russell 1c26ebdb31 pytest: fix flake in test_wumbo_channels
We might only have seen one side of the channel, as shown below.  Wait
for both:

```
_____________________________ test_wumbo_channels ______________________________
[gw2] linux -- Python 3.7.13 /opt/hostedtoolcache/Python/3.7.13/x64/bin/python3

node_factory = <pyln.testing.utils.NodeFactory object at 0x7f5d51743b10>
bitcoind = <pyln.testing.utils.BitcoinD object at 0x7f5d51699d10>

    @pytest.mark.openchannel('v1')
    @pytest.mark.openchannel('v2')
    def test_wumbo_channels(node_factory, bitcoind):
        l1, l2, l3 = node_factory.get_nodes(3,
                                            opts=[{'large-channels': None},
                                                  {'large-channels': None},
                                                  {}])
        conn = l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
    
        expected_features = expected_peer_features(wumbo_channels=True)
        if l1.config('experimental-dual-fund'):
            expected_features = expected_peer_features(wumbo_channels=True,
                                                       extra=[21, 29])
    
        assert conn['features'] == expected_features
        assert only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['features'] == expected_features
    
        # Now, can we open a giant channel?
        l1.fundwallet(1 << 26)
        l1.rpc.fundchannel(l2.info['id'], 1 << 24)
    
        # Get that mined, and announced.
        bitcoind.generate_block(6, wait_for_mempool=1)
    
        # Connect l3, get gossip.
        l3.rpc.connect(l1.info['id'], 'localhost', port=l1.port)
        wait_for(lambda: len(l3.rpc.listnodes(l1.info['id'])['nodes']) == 1)
        wait_for(lambda: 'features' in only_one(l3.rpc.listnodes(l1.info['id'])['nodes']))
    
        # Make sure channel capacity is what we expected.
>       assert ([c['amount_msat'] for c in l3.rpc.listchannels()['channels']]
                == [Millisatoshi(str(1 << 24) + "sat")] * 2)
E       assert [16777216000msat] == [16777216000m...777216000msat]
E         Right contains one more item: 16777216000msat
E         Full diff:
E         - [16777216000msat, 16777216000msat]
E         + [16777216000msat]
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-07-27 19:31:04 +09:30
niftynei 2c2bcc8eb4 flake: permit test_v2_open_sigs_restart_while_dead to succeed/fail
There's a race btw disconnecting and returning a successful RPC call for
openchannel_signed; if we disconnect quickly, we get an RPC error back.

Too slow and it returns w/o an error.

This needs to be cleaned up on a whole, work that I'm planning to get
into as part of a funder re-write. For now, let's just let the test
continue whether this call succeeds or fails.
2022-07-27 19:31:04 +09:30
niftynei f4abc3a661 tests: local flake fix; l1 was waiting too long to reconnect
Impacts local tests, when TIMEOUT is set low...
2022-07-26 15:11:30 -07:00
Rusty Russell 17b9bd5ca3 pytest: fix test_commando_rune flake.
We reset counters every minute, so ratelimit tests can flake since we
might hit that boundary.

Instead, wait for the reset then test explicitly, assuming that takes
less than 60 seconds.

```
        for rune, cmd, params in failures:
            print("{} {}".format(cmd, params))
            with pytest.raises(RpcError, match='Not authorized:') as exc_info:
                l2.rpc.call(method='commando',
                            payload={'peer_id': l1.info['id'],
                                     'rune': rune['rune'],
                                     'method': cmd,
>                                    'params': params})
E               Failed: DID NOT RAISE <class 'pyln.client.lightning.RpcError'>
...
DEBUG:root:Calling commando with payload {'peer_id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'rune': 'O8Zr-ULTBKO3_pKYz0QKE9xYl1vQ4Xx9PtlHuist9Rk9NCZwbnVtPTAmcmF0ZT0zJnJhdGU9MQ==', 'method': 'getinfo', 'params': {}}
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-07-26 09:48:56 -07:00