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>
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>
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>
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.
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
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
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).
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.
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>
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>
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.
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.
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).
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.
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.
The only places which should call try_reconnect now are the "connect"
command, and the disconnect path when it decides there's still an
active channel.
This introduces one subtlety: if we disconnect when there's no active
channel, but then the subd makes one, we have to catch that case!
This temporarily reverts "slow" reconnections to fast ones: see next
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In various places, we assumed that when `connected` is false,
everything is finished. This is not true: we should wait for the
state we expect.
In addition, various places allows reconnections, which interfered
with the logic; suppress them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to avoid lost messages in the common cases.
This generalizes our drain code, by giving the subds each 5 seconds to
close themselves, but continue to allow them to send us traffic (if
peer is still connected) and continue to send them traffic.
We continue to send traffic *out* to the peer (if it's still
connected), until all subds are gone. We still have a 5 second timer
to close the connection to peer.
On reconnects, we don't do this "drain period" on reconnects: we kill
immediately.
We fix up one test which was looking for the "disconnect" message
explicitly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes#5271
In-Collaboration-With: Base58 'n Coding Seminar Participants
Changelog-Changed: `fundchannel` now errors if you try to buy a liquidity ad but dont' have `experimental-dual-fund` enabled
With zeroconf we have to duplicate the `local_channel_announcement`
since we locally announce the aliased version, and then on the first
confirmation we also add the funding scid version.
Changelog-Experimental: option `--lease-fee-base-msat` renamed to `--lease-fee-base-sat`
Changelog-Experimental: option `--lease-fee-base-msat` deprecated and will be removed next release
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>
Documentation for the funderupdate command on the funder plugin.
Realized we were missing the "leases_only" in the output; also adds
'_msat' to msat denominated outputs so they're parsed correctly by our
python bindings (also matches our naming conventions)
Changelog-EXPERIMENTAL: funder: `funderupdate` command to view and update params for contributing our wallet funds to v2 channel openings. Provides params for enabling `option_will_fund`.
Since we now use 'compact_lease' to gate an open (if the rates have
changed, we fail), we no longer need to rely on query rates for figuring
things out, so we make it dev-only.
Changelog-Changed: JSON-API: queryrates is now developer only
If there's no plugin currently in place, we simply won't return any
funding at all, in which case we'd expect them to handle however
they want. (our implementation would fail the open, as we only accept
opens that have at least as much as we've requested provided)
Using a 'feestep' is more restrictive than you'd want, instead we
enforce that the next feerate must be at least 1/64th more than the
last, but put no upper limit on it
Includes update to lnprototest changes
Contributed-By: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-EXPERIMENTAL: Protocol: Replaces init_rbf's `fee_step` for RBF of v2 opens with `funding_feerate_perkw`, breaking change
This test takes 695 seconds, because fundwallet waits for the wallet to
notice the tx, which takes 60 seconds if not DEVELOPER. Do all the waiting
at once, and this speeds the test up to 153 seconds.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
Christian Decker
niftynei
Simon Vrouwe
Dustin Dettmer