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.
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>
It's weird to have connectd ask gossipd, when lightningd can just do it
and hand all the addresses together.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
1. tal_strndup(.., str, strlen(str)) == tal_strdup()
2. tal_strdup also takes(), so document that.
3. Avoid passing 'struct sha256' on the stack: use ptr.
4. Generally, structures shouldn't keep pointers to things they don't own.
In this case, mvt->node_id.
5. Make payment_hash a pointer, since NULL is more natural than an all-zero
hash.
And add NON_NULL_ARGS() to the functions; it's cumbersome, but make it
fairly clear what params are optional.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Freeing an unconfirmed channel already releases the subd, so don't
do that explicitly.
2. Use channel->owner to transfer ownership where possible, using
channel_set_owner() which handles all the cases.
This simplifies the code and makes it more readable, IMHO.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
we used this originally to suppress duplicate issuance of coin-move
events; we're assuming that any plugin expects duplicate events though
(and knows how to de-dupe them), so we no longer need this logic.
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.
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>
Various unit tests were creating temporary files unconditionally in /tmp
and were not cleaning up after themselves. Introduce a new variant of
mkstemp(3p) that respects the TMPDIR environment variable, and use it in
the offending unit tests. This allows each test run to use a dedicated
TMPDIR that can be cleaned up after the run.
Changelog-None
Signed-off-by: Matt Whitlock <c-lightning@mattwhitlock.name>
Before:
Ten builds, laptop -j5, no ccache:
```
real 0m36.686000-38.956000(38.608+/-0.65)s
user 2m32.864000-42.253000(40.7545+/-2.7)s
sys 0m16.618000-18.316000(17.8531+/-0.48)s
```
Ten builds, laptop -j5, ccache (warm):
```
real 0m8.212000-8.577000(8.39989+/-0.13)s
user 0m12.731000-13.212000(12.9751+/-0.17)s
sys 0m3.697000-3.902000(3.83722+/-0.064)s
```
After:
Ten builds, laptop -j5, no ccache: 8% faster
```
real 0m33.802000-35.773000(35.468+/-0.54)s
user 2m19.073000-27.754000(26.2542+/-2.3)s
sys 0m15.784000-17.173000(16.7165+/-0.37)s
```
Ten builds, laptop -j5, ccache (warm): 1% faster
```
real 0m8.200000-8.485000(8.30138+/-0.097)s
user 0m12.485000-13.100000(12.7344+/-0.19)s
sys 0m3.702000-3.889000(3.78787+/-0.056)s
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This affects the range we offer even without quick-close, but it's
more critical for quick-close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSONRPC: `close` now takes a `feerange` parameter to set min/max fee rates for mutual close.
The variable `block` (instace of `struct block`) is
allocated on the stack without being initialized, i.e. its
member `prev` points to nowhere. This causes a segmentation
fault on my machine on the binding of "prev_hash" on running
`wallet_block_add`, as the following core-dump analysis
shows:
$ egdb ./wallet/test/run-wallet ./run-wallet.core
[...]
Core was generated by `run-wallet'.
Program terminated with signal SIGSEGV, Segmentation fault.
---Type <return> to continue, or q <return> to quit---
#0 0x000008f67a04b660 in memcpy (dst0=<optimized out>, src0=0x100007f8c, length=32) at /usr/src/lib/libc/string/memcpy.c:97
97 TLOOP1(*dst++ = *src++);
(gdb) bt
#0 0x000008f67a04b660 in memcpy (dst0=<optimized out>, src0=0x100007f8c, length=32) at /usr/src/lib/libc/string/memcpy.c:97
#1 0x000008f73e838f60 in sqlite3VdbeMemSetStr () from /usr/local/lib/libsqlite3.so.37.12
#2 0x000008f73e83cb11 in bindText () from /usr/local/lib/libsqlite3.so.37.12
#3 0x000008f44bc91345 in db_sqlite3_query (stmt=0x8f6845bf028) at wallet/db_sqlite3.c:77
#4 0x000008f44bc91122 in db_sqlite3_exec (stmt=0x8f6845bf028) at wallet/db_sqlite3.c:110
#5 0x000008f44bcbb3b2 in db_exec_prepared_v2 (stmt=0x8f6845bf028) at ./wallet/db.c:2055
#6 0x000008f44bcc6890 in wallet_block_add (w=0x8f688b5bba8, b=0x7f7ffffca788) at ./wallet/wallet.c:3556
#7 0x000008f44bce2607 in test_wallet_outputs (ld=0x8f6a35a7828, ctx=0x8f6a35c0268) at wallet/test/run-wallet.c:1104
#8 0x000008f44bcddec0 in main (argc=1, argv=0x7f7ffffcaaf8) at wallet/test/run-wallet.c:1930
Fix by explicitely setting the whole structure to zero.
[ Rebuilt generated files, too --RR ]
This supports reestablish on a closed channel: we tell channeld to
respond to the reestablish message appropriately, then close the
channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It handles all the cases of retransmission, and in the normal case
retransmits shutdown and immediately returns for us to run closingd.
This is actually far simpler and reduces code duplication.
[ Includes fixup to stop warn_unused_result from Christian ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We could get stuck on signature exchange if we needed to retransmit the final revoke_and_ack.
Prior to this, sending a v1 address (or, in fact, any random crap!)
would cause the unsupporting node to unilaterally close.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tor v2 hidden services have been deprecated for a while:
https://blog.torproject.org/v2-deprecation-timeline .
This prevents user from being able to set them in the configuration
and to connect to them while still letting us be able to parse them
for gossip.
Changelog-Deprecated: lightningd: v2 Tor addresses. Use v3. See https://blog.torproject.org/v2-deprecation-timeline.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
If the peer is offline when we see the funding txid, we don't actually
update the channel's info. Here, we move it up to where the scid is set,
so that we always update the channel's funding_txid to the correct
(mined) information.
Trying to put all the disconnect logic into the same path was a dumb
idea. If you asked to reconnect but passed in an 'unsaved' channel, we
would not call the 'reconnect' code.
Instead, we make a differentiation between "unsaved" channels
(ones that we haven't received commitment tx for) and handle the
disconnect for these separate from where we want to do a reconnect.
This means remembering the connection direction. We also use the address to try
to reconnect, which we shouldn't bother with if they connect to us.
For peers from the database, we currently always save the addr: we shouldn't really
do this if they connected to us, since it's not useful for reconnecting (we don't
show the addr in JSON reply to listpeers unless we're connected, so it's only an
internal issue). This is left for future work.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This matters: if we connected, the address is probably usable for future connections.
But if they connected, the port is probably not (but the IP address may be).
Changelog-Added: JSON-RPC: `connect` returns "direction" ("in": they iniatated, or "out": we initiated)
Changelog-Added: plugins: `peer_connected` hook and `connect` notifications have "direction" field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Otherwise, we might find an address other than the one given and
the user might think that address worked.
Fixes: #4185
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `connect` returns `address` it actually connected to
Changelog-Added: lightningd: experimental-shutdown-wrong-funding to allow remote nodes to close incorrectly opened channels.
Changelog-Added: JSON-RPC: close has a new `wrong_funding` option to try to close out unused channels where we messed up the funding tx.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Allows us to clean up an in-progress open that we won't be completing
Changelog-Added: EXPERIMENTAL JSON-RPC: Permit user-initiated aborting of in-progress opens. Only valid for not-yet-committed opens and RBF-attempts
We move over to the new "warning" paradigm, instead of using
an "rbf_fail" message.
Every failure is either a warning or an error; on warnings we
hang up and reconnect later, effectively resetting the state.
Users have no idea what they would pay for unilateral closes.
At least this gives them a clue!
Reported-by: @az0re on IRC.
Changelog-Added: JSON-RPC: `listpeers` now shows latest feerate and unilaral close fee.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were always prefixing the `message` field with the internal type
prefix 0x0407, followed by the length prefix. Neither is needed since
the type being constant is of no interest to the plugin and the length
being implicit due to the JSON-encoding.
Reported-by: Ilya Evdokimov
Changelog-Fixed: plugin: The `custommsg` hook no longer includes the internal type prefix and length prefix in its `payload`
Changelog-Deprecated: plugin: The `message` field on the `custommsg` hook is deprecated in favor of the `payload` field, which skips the internal prefix.
No more sending "all-channel" errors; in particular, gossipd now only
sends warnings (which make us hang up), not errors, and peer_connected
rejections are warnings (and disconnect), not errors.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `peer_connected` rejections now send a warning, not an error, to the peer.
Prior to this, all reconnect logic lived in channeld. If you
disconnected before we finished building a funding transaction, that was
no big deal. Now, however, we're waiting for the funding to lock in in
dualopend, instead of handing straight to channeld to wait.
So we need a way to restart dualopend.
This reverts commit c239a7161b.
The goal of c239a716 was to reduce the memory footprint of our
internal UTXO set tracking, and testing against the sqlite3 backend
showed no performance impact. We have since found that the added
roundtrips to the DB server with postgres was having a considerable
performance impact, and backups would also bloat due to the increased
number of queries.
Undoing this change skips the noop updates that were causing this
regression.
Changelog-Fixed: db: Fixed a performance regression during block sync, resulting in many more queries against the DB than necessary.
This allows us to mark an offer used when an invoice derived from it
is paid, and importantly, avoid any other invoices for the offer being
paid.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Still asserts that it's the standard size, but makes it a dynamic
member. For simpliciy, changes the parse_onionpacket API (it must be
a tal object now, so we might as well allocate it here to catch all
the callers).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's actually a (very unlikely) race here: we would previously have
crashed with an assertion in invoices_resolve.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Avoids much cut & paste. Some tests don't need any of it, but most
want at least some of this infrastructure.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This adds a `state_change` 'cause' to a channel.
A 'cause' is some initial 'reason' a channel was created or closed by:
/* Anything other than the reasons below. Should not happen. */
REASON_UNKNOWN,
/* Unconscious internal reasons, e.g. dev fail of a channel. */
REASON_LOCAL,
/* The operator or a plugin opened or closed a channel by intention. */
REASON_USER,
/* The remote closed or funded a channel with us by intention. */
REASON_REMOTE,
/* E.g. We need to close a channel because of bad signatures and such. */
REASON_PROTOCOL,
/* A channel was closed onchain, while we were offline. */
/* Note: This is very likely a conscious remote decision. */
REASON_ONCHAIN
If a 'cause' is known and a subsequent state change is made with
`REASON_UNKNOWN` the preceding cause will be used as reason, since a lot
(all `REASON_UNKNOWN`) state changes are a subsequent consequences of a prior
cause: local, user, remote, protocol or onchain.
Changelog-Added: Plugins: Channel closure resaon/cause to channel_state_changed notification
For compatibility, we only do this if `allow-deprecated-apis` is false
for now. Otherwise scripts parsing should use `grep -v '^# '` or
start using `-N none`.
Changelog-Added: JSON-RPC: `close` now sends notifications for slow closes (if `allow-deprecated-apis`=false)
Changelog-Deprecated: cli: scripts should filter out '^# ' or use `-N none`, as commands will start returning notifications soon
Fixes: #3925
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need the PSBT to create the finalized tx from once the peer's
tx_signatures are received. Since we're passing the PSBT, we no longer
need the secondary message to be passed, as it was derived from the
PSBT.
Also removes now unused witness serialization code
This notification will be raised whenever a channel state changes.
The payload includes the channel and peer identifiers and the
old and the new state.
Example payload:
```
{
"channel_state_changed": {
"peer_id": "03bc9337c7a28bb784d67742ebedd30a93bacdf7e4ca16436ef3798000242b2251",
"channel_id": "a2d0851832f0e30a0cf778a826d72f077ca86b69f72677e0267f23f63a0599b4",
"short_channel_id" : "561820x1020x1",
"old_state": "CHANNELD_NORMAL",
"new_state": "AWAITING_UNILATERAL"
}
}
```
Changelog-Added: Plugins: channel_state_changed notification
Greatly simplify the changeset API. Instead of 'diff' we simply generate
the changes.
Also pulls up the 'next message' method, as at some point the
interactive tx protocol will be used for other things as well
(splices/closes etc)
Suggested-By: @rustyrussell
v2 of channel establishment, in the accpeter case, now sends 2 messages
to our peer after saving the information to disk (our commitment
signatures and our funding transaction signatures)
v2 channel open uses a different method to derive the channel_id, so now
we save it to the database so that we dont have to remember how to
derive it for each.
includes a migration for existing channels
We're going to want this for bolt13 formation as well.
As a result of reworking the logic into "candidate selection" then
"route hint selection", we need to change the way round-robin works.
We use a simple incrementing index now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This removes the reservation cleanup at startup, too, now they're all
using 'reserved_til'.
This changes test_withdraw, since it asserted that outputs were marked
spent as soon as we broadcast a transaction: now they're reserved until
it's mined. Similarly, test_addfunds_from_block assumed we'd see funds
as soon as we broadcast the tx.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `withdraw` now randomizes input and output order, not BIP69.
We create ALL_PROGRAMS, ALL_TEST_PROGRAMS, ALL_C_SOURCES and
ALL_C_HEADERS. Then the toplevel Makefile knows which are
autogenerated (by wildcard), so it can have all the rules to clean
them or check the source as necessary.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that other directories were explicitly depending on the generated
file, instead of relying on their (already existing) dependency on
$(LIGHTNINGD_HSM_CLIENT_OBJS), so we remove that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means some files get renamed, and I took the opportunity to clarify
our naming (the *d* is important!)
1. channeld/channel_wire.csv -> channeld/channeld_wire.csv
2. channeld/gen_channel_wire.h -> channeld/channeld_wiregen.h
3. enum channel_wire_type -> enum channeld_wire
4. WIRE_CHANNEL_FUNDING_DEPTH -> WIRE_CHANNELD_FUNDING_DEPTH.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to remember this in the db (it's a P2WSH for option_anchor_outputs),
and we need to set nSequence to 1 to spend it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is best done by passing `struct bitcoin_signature` around instead
of raw signatures. We still save raw sigs to the db, and of course the
wire protocol uses them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
HTLC fees increase (larger weight), and the fee paid by the opener
has to include the anchor outputs (i.e. 660 sats).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
the way we use PSBTs to sign things requires that we have the
scriptpubkey available on the utxo so we can populate the witness-utxo
field with it.
this causes problems if we don't already have the scriptpubkey cached in
the database, as in *some* cases we require a round trip to the HSM to
populate them
to get over this hump, we backfill any and all missing scriptpubkey
information for the utxo's that we hold in our wallet.
this will allow us to clean up the NULL handling of missing
scriptpubkeys.
Rusty Russell
niftynei
Ken Sedgwick
Matt Whitlock
Christian Decker
Sebastian Falbesoner
Antoine Poinsot
Michael Schmoock
ZmnSCPxj jxPCSnmZ