Commit Graph

107 Commits

Author SHA1 Message Date
Rusty Russell acc30c0b3f lightningd: split DUALOPEND_OPEN_INIT into DUALOPEND_OPEN_INIT and DUALOPEND_OPEN_COMMITTED.
The latter is used when we're put in the db, the former is the uncommitted state.
Currently dbid == 0 is used in addition to the state, which is unwieldy.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Experimental: JSON-RPC: added new dual-funding state `DUALOPEND_OPEN_COMMITTED`
2023-10-02 11:41:19 +10:30
Rusty Russell 6c15ea44dd pytest: use --developer instead of environment variable.
And we always enable it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-09-21 20:08:24 +09:30
Tony Giorgio a5367773d5 wallet: Exclude uneconomical UTXOs from fundchannel
If a node has an onchain balance with at least one uneconomical UTXO, the fundchannel RPC call will lock up the node and will eventually crash it with OOM issues if the economical UTXO(s) do not add up to the fundchannel amount. This is because the while loop never exits because it keeps pulling in the same uneconomical UTXOs forever.

Changelog-Fixed: wallet: fundchannel no longer loops forever if the wallet contains insufficient funds, but an uneconomical UTXO.
2023-09-13 14:34:59 +09:30
Rusty Russell 47f4e11c87 tests: test for fundchannel fail with tiny utxo
Tony Giorgio <tonygiorgio@protonmail.com> says:

Reproduce:

1.  Add 1 600 sat UTXO to a fresh node

2.  Verify the fundchannel command fails with a low fee rate:

```
./lightning-cli fundchannel 0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a 100000 1000
{
   "code": 301,
   "message": "Could not afford 100000sat using all 1 available UTXOs: 99522sat short"
}
```

3.  Now do the command again, but with a higher fee rate, making the 600 sat UTXO uneconomical:

```
./lightning-cli fundchannel 0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a 100000 10000
```

4. Observe the RPC call and the logs. The RPC call will never return, and the logs will stop after this:

```
2023-04-16T10:58:45.839Z DEBUG   plugin-spenderp: mfc 34: multiconnect done.
2023-04-16T10:58:45.839Z DEBUG   plugin-spenderp: mfc 34: 'parsefeerate' done
2023-04-16T10:58:45.839Z DEBUG   plugin-spenderp: mfc 34: fundpsbt.
```

5. Keep CLN running long enough and you'll eventually run OOM.
2023-09-13 14:34:59 +09:30
niftynei 9b8909e507 dual-fund: keep track of aborted requests, seamlessly restart daemon
Clean restart of daemon after a tx-abort is a nice way to work around
the 'persistent' disconnect that we t-bast noticed.

Changelog-Fixed: `dualopend`: Fix behavior for tx-aborts. No longer hangs, appropriately continues re-init of RBF requests without reconnction msg exchange.
2023-07-30 15:20:04 +09:30
niftynei c807db4b7f dual-fund tests: add tests for reported incompat with Eclair
Bug Report:
- initiate a channel open eclair -> cln
- wait for the transaction to be published
- eclair initiates rbf, and cancels it by sending tx_abort before exchanging commit_sig
- at that point everything looks good, cln echoes the tx_abort and stays connected
- eclair initiates another RBF attempt and sends tx_init_rbf: for some unknown reason,
  cln answers with channel_reestablish (??) followed by an error saying
  "Bad reestablish message: WIRE_TX_INIT_RBF"

Diagnosis:
  CLN is doing a reconnect after a tx-abort is sent.

Extra Test:
  Realized that if we abort, we won't correctly advanced to NORMAL if
  blocks are mined while we're in hanging state. CLN should advance
  after block containing channel open is mined.

Reported-By: @t-bast
2023-07-30 15:20:04 +09:30
Rusty Russell e91f8ddb51 pytest: fix test_anchor_min_emergency "bad gossip" messages.
l1 shuts down too fast, channeld doesn't get to tell gossipd about the channel,
and l2 sends (private) update_channel and we complain:

```
lightningd-2 2023-07-20T03:42:37.744Z DEBUG   gossipd: received private channel announcement from channeld for 103x1x0
lightningd-2 2023-07-20T03:42:37.791Z DEBUG   022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-hsmd: Got WIRE_HSMD_CUPDATE_SIG_REQ
lightningd-2 2023-07-20T03:42:37.796Z DEBUG   hsmd: Client: Received message 3 from client
lightningd-1 2023-07-20T03:42:37.857Z DEBUG   022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 103x1x0/0
lightningd-1 2023-07-20T03:42:37.864Z DEBUG   022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 103x1x0/0
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-07-22 08:52:55 +09:30
Rusty Russell e6d23b5677 pytest: test for emergency minimum fund violations.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-29 11:28:47 -04:00
Rusty Russell 7894d7136f pytest: adapt all the anchor-iff-EXPERIMENTAL tests to --experimental-anchors.
We use parameterization here.  The old `anchor_expected()` was for
non-zero-fee anchors, and have bitrotted so there are some other
changes as well.

Unfortunately, all the anchor accounting seems to be broken, but I
cannot understand these tests at all.  I had to simply disable them
for now.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-29 11:28:47 -04:00
Rusty Russell f64188f925 pytest: make dual-funding tests using option_anchors_zero_fee_htlc_tx.
They used to force option_anchor_outputs, so switch them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-29 11:28:47 -04:00
Rusty Russell 96e198f751 openingd: use a lower feerate for first commitment tx for anchors.
We need to know both, because in theory we could negotiate a
non-anchor channel even if they support it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-29 11:28:47 -04:00
Rusty Russell d895ef4fee pytest: test for commitment feerate.
Interesting we didn't actually test that the feerate we use is
actually delivered.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-29 11:28:47 -04:00
Rusty Russell ea928bfca1 pytest: use modern listconfigs.
Use the configs object, as the others are about to be deprecated.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-03 10:50:29 +09:30
Rusty Russell ec8bba6b8a dev-allowdustreserve: make this a DEVELOPER option.
Otherwise, we should rename it?

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-06-03 10:50:29 +09:30
Rusty Russell 548b54e971 pytest: update test_v2_fail_second for listpeers changes.
Now should use listpeerchannels, since listpeers['channels'] is
deprecated.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-05-29 13:46:21 +09:30
Rusty Russell c11ae1aa34 pytest: remove EXPERIMENTAL_FEATURES as a consideration.
This currently means anchors tests are disabled, awaiting the
PR which implements zero-fee-htlc anchors to reenable them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-05-23 09:34:08 +09:30
niftynei 3586559fac test (df): check 'require-confirmed-inputs' for v2 opens 2023-02-07 21:03:36 -06:00
niftynei 195a2cf44b dual-open: use tx-abort instead of warning/errors
When a channel open fails, we use tx-abort instead of warning/error.

This means that the peer won't disconnect! And instead when a new
message arrives, we'll need to rebuild the dualopend subd (if missing).

Makes opens a bit easer to retry (no reconnect needed), as well as keeps
the connection alive for other channels we may have with that peer.

Changelog-Changed: Experimental-Dual-Fund: open failures don't disconnect, but instead fail the opening process
2023-02-05 10:02:46 +01:00
niftynei b375a35fa0 v2 open tests: don't drop connection when an openchannel fails
Dropping the connection is bad behavior on an openchannel failure,
especially given that there might be other channels currently connected.

We should maintain the connection but close out the dualopend
daemon for that attempt.

This test partially documents the behaivor, but fails

Changelog-None
2023-02-05 10:02:46 +01:00
niftynei 4c46750001 dual-open-rbf: remember the requested lease amount btw restarts
Don't forget the requested lease across restarts.
2023-02-04 15:31:16 +10:30
niftynei 314c021e2c test: restart node during rbf
state isn't kept around for lease amount, so it should fail
2023-02-04 15:31:16 +10:30
niftynei 4fe8e1eccf tests: check that non-anchor opens can't use liquidity ads
liquidity ads (as proposed) rely on the CSV addition to the to_remote output
that anchors introduced.
2023-02-04 15:31:16 +10:30
niftynei c9c367d770 dual-fund: remove anchor assumption for all dual-funded channels
Only add the anchor channel_type if it's negotiated separately!
2023-02-04 15:31:16 +10:30
niftynei 976f6ef51a df tests: use the amount from the logs to check for! 2023-02-04 15:31:16 +10:30
Michael Schmoock ad249607d6 dual-fund: update extracted CSVs to latest bolt draft
Changelog-None
2023-02-04 15:31:16 +10:30
Rusty Russell 85fc46f76c pyln-testing: don't default openchannel and fundwallet to p2sh-segwit, use bech32.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-01-14 21:17:45 +10:30
Vincenzo Palazzo a2347c7452 tests: use listpeerchannels. 2023-01-13 10:42:42 +10:30
niftynei 26f5dcd2a5 wallet: mark coinbase outputs as 'immature' until spendable
Changelog-Changed: JSON-RPC: `listfunds` now lists coinbase outputs as 'immature' until they're spendable
Changelog-Changed: JSON-RPC: UTXOs aren't spendable while immature
2022-11-09 11:55:25 +01:00
niftynei d60dbba43b tests: test for coinbase wallet spend.
Attempt to spend a coinbase transaction, expected to fail.
2022-11-09 11:55:25 +01:00
niftynei e00857827f funder: cleanup datastore on state-change/channel failure
Let's not leave old state hanging around! Note that this fires
for pretty much every/any channel (even if we're not the opener).
2022-10-20 13:42:41 +02:00
niftynei 38e2428f12 funder: use utxopsbt to build psbt for RBFs
We use the saved previous outputs (plus maybe some new ones?) to build a
psbt for an RBF request.

RBFs utxo reuse is now working so we can unfail the test (and update
it to reflect that the lease sticks around through an RBF cycle).

Changelog-Fixed: Plugins: `funder` now honors lease requests across RBFs
2022-10-20 13:42:41 +02:00
niftynei 00d3e3e492 df: for rbfs, since we know what they asked for, we can abort
if they request less than we wanted/accepted

FIXME: add a test for this?
2022-10-20 13:42:41 +02:00
Rusty Russell 695f001161 pytest: fix flake in test_zeroconf_forward
Second pay can fail if first is not completely settled:

```
    def test_zeroconf_forward(node_factory, bitcoind):
        """Ensure that we can use zeroconf channels in forwards.

...
        # Make sure (esp in non-dev-mode) blockheights agree so we don't WIRE_EXPIRY_TOO_SOON...
        sync_blockheight(bitcoind, [l1, l2, l3])
        inv = l3.rpc.invoice(42 * 10**6, 'inv1', 'desc')['bolt11']
        l1.rpc.pay(inv)

        # And now try the other way around: zeroconf channel first
        # followed by a public one.
        wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 4)
        inv = l1.rpc.invoice(42, 'back1', 'desc')['bolt11']
>       l3.rpc.pay(inv)

...
>           raise RpcError(method, payload, resp['error'])
E           pyln.client.lightning.RpcError: RPC call failed: method: pay, payload: {'bolt11': 'lnbcrt420p1p3junrssp588gtjzmlrr4pfj7ssmdlulzhhushrpq3rdqxjuz2m33scsvzdjlspp5fk5yhu6netc0d0sgp8es52vjk6akhd3uayr08u8max4d8rwzpjuqdq8v3jhxccxqyjw5qcqp99qyysgqcpyyugejv4nya97v6gw8fhtr0ru3vq87jjlltav99wlat436a95n0z8yzdp699p9md0zz9tmnsjpvfj622n9g9fh7r6ldhpgh9wmr4qpcru3rk'}, error: {'code': 210, 'message': 'Destination 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518 is not reachable directly and all routehints were unusable.', 'attempts': [{'status': 'failed', 'failreason': 'Destination 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518 is not reachable directly and all routehints were unusable.', 'partid': 0, 'amount_msat': 42msat}]}
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-27 13:14:20 +02:00
Rusty Russell 6e86fa9220 lightningd: figure out optimal channel *before* forward_htlc hook.
Otherwise what the hook sees is actually a lie, and if it sets it
we might override it.

The side effect is that we add an explicit "forward_to" field, and
allow hooks to override it.  This lets a *hook* control channel
choice explicitly.

Changelod-Added: Plugins: `htlc_accepted_hook` return can specify what channel to forward htlc to.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2022-09-26 13:52:04 +02:00
Christian Decker 774d16a72e openingd: Fail if dust and max_htlcs result in 0output commitment tx
Prior to this we might end up with a commitment transaction without
any outputs, if combined with `--dev-allowdustreserve`. Otherwise the
reserve being larger than dust means the funder could not drop its
direct output to be below dust.

Reported-by: Rusty Russell <@rustyrussell>
2022-09-21 11:25:47 +02:00
Christian Decker 493a0dfcd4 pytest: Exercise all dust zeroreserve case
This just shows we need to add that constraint.

Reported-by: Rusty Russell <@rustyrussell>
2022-09-21 11:25:47 +02:00
Christian Decker bdda62e1a4 pytest: Add test for mixed zeroreserve funding
Tests that we can have zeroreserve nodes accept and open new channels
with non-zeroreserve nodes, without throwing errors
2022-09-21 11:25:47 +02:00
Christian Decker 54b4baabb0 opening: Add `dev-allowdustreserve` option to opt into dust reserves
Technically this is a non-conformance with the spec, hence the `dev`
flag to opt-in, however I'm being told that it is also implemented in
other implementations. I'll follow this up with a proposal to the spec
to remove the checks we now bypass.
2022-09-21 11:25:47 +02:00
Christian Decker c5b2aee5c6 pytest: Add a zeroreserve test 2022-09-21 11:25:47 +02:00
niftynei efad09f966 bkpr: confirm that replaying the open+lock-in txs at start is ok
Make sure that we're not issuing duplicate lease_fee events!
2022-09-14 15:59:37 +09:30
niftynei 1980ba420b notif: dont send balance snapshot for not yet opened channel
We were double counting channel lease fees because we were double firing
the channel open event sequence (so to speak). If we don't report
balances for unopened channels, we don't have this problem?

Changelog-Changed: Plugins: `balance_snapshot` notification does not send balances for channels that aren't locked-in/opened yet
2022-09-14 15:59:37 +09:30
niftynei 8452d903b4 bkpr: failing test for bookkeeper crash
Reproduce crash for #5557!

If we record the channel open because bookkeeper was added after the
channel open request started but the channel confirms later, we end up
with re-recording any associated push or leased fees (paid or rcvd).

In the case where you've paid for these fees, your channel balance goes
negative and the node crashes the next time you call `listbalances`.

Reported-by: @chrisguida
2022-09-14 15:59:37 +09:30
Rusty Russell 5b7f14a7cb channeld/dualopend/lightningd: use channel_ready everywhere.
This alters the billboard, but that's a human-readable thing so not
noted in CHANGELOG.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `listpeers` `status` now refers to "channel ready" rather than "funding locked" (BOLT language change for zeroconf channels)
Changelog-Added: JSON-RPC: `channel_opened` notification `channel_ready` flag.
Changelog-Deprecated: JSON-RPC: `channel_opened` notification `funding_locked` flag (use `channel_ready`: BOLTs namechange).
2022-09-12 09:34:52 +09:30
Rusty Russell 1b30ea4b82 doc: update BOLTs to bc86304b4b0af5fd5ce9d24f74e2ebbceb7e2730
This contains the zeroconf stuff, with funding_locked renamed to
channel_ready.  I change that everywhere, and try to fix up the
comments.

Also the `alias` field is called `short_channel_id`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: `funding_locked` is now called `channel_ready` as per latest BOLTs.
2022-09-12 09:34:52 +09:30
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
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
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 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 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