Let the callers do that (only channeld needs to do this).
We temporarily send an error on unknown reestablish in openingd, as
this mimic previous behavior and avoids breaking tests (it does leave
a BROKEN message in the logs though, so
test_funding_external_wallet_corners needs to ignore that for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Peer sends funding locked, we tell lightningd who saves it to disk.
Then we restart/reconnect and they retransmit funding_locked. We were
re-notifying lightningd about their lock-in, which was crashing/breaking
things. Instead, we ignore duplicate lock-in messages from the peer.
lightningd-1: 2021-05-11T18:00:12.844Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: Internal error DUALOPEND_AWAITING_LOCKIN: channel_got_funding_locked twice
This is set by the peer and is non-negotiable. We're not even going to
check if you got it right. You were told about it via `openchannel2`.
It is what it is.
The other side doesn't know it until *after* it parses this msg. We
add a quick hack to still allow old nodes to work (for now!).
This also fixes a bug (spotted by @niftynei) where any errors we sent
before accepter_start2 would have the new (unknowable!) channel_id
rather than the temp one.
Authored-by: Rusty Russell <rusty@rustcorp.com.au>
You can now activate dual-funded channels using the
`--experimental-dual-fund` flag
Changelog-Changed: Config: `--experimental-dual-fund` runtime flag will enable dual-funded protocol on this node
In the case of failures/disconnects between dualopend receiving the sigs
and the main man disconnecting (as can happen using the testing
disconnects), failing to forward their sigs over to lightningd can
result in the channel never progressing from DUALOPEND_OPEN_INIT
to DUALOPEND_AWAITING_LOCKIN.
Always re-relay.
> If the peer's revocation basepoint is unknown (e.g. `open_channel2`),
> a temporary `channel_id` should be found by using a zeroed out basepoint
> for the unknown peer.
We consolidate to the latest/singular RFC patch for dual-funding, so
there's just a single patchfile for the change. Plus we move back to the
opener setting the desired feerate, the accepter merely declines to
participate if they disagree with the set rate.
Looks like #4394 treated a symptom but not the root cause. We were
actually sending the message framed with the WIRE_CUSTOMMSG_OUT and
the length prefix over the encrypted connection to the peer. It just
happened to be a valid custommsg...
This fixes the issue, and this time I made sure we actually send the
raw message over the wire. However for backward compatibility we
needed to imitate the faulty behavior which is 90% of this patch :-)
Changelog-Fixed: plugin: `dev-sendcustommsg` included the type and length prefix when sending a message.
If we're doing an RBF, it's possible that the peer will send us a
funding_locked, shutdown, or tx_signatures message. (We get tx_sigs out
of order on a reconnect)
This lets us gracefully handle a shutdown or funding_locked
sent at any time (after first funding tx) as well.
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.
We're *mostly* set up for both sides doing RBF, except that it reverses
the callback flow (using the plugin vs RPC calls) and we're not
currently smart enough to flip between them gracefully
We need to make sure that there's at least one input that's represented
in every single RBF-attempt for this channel, to prevent "parallel"
subsequent RBFs from succeeding/opening (the multi-channel backdoor?!)
The spec doesn't say to do this, but it makes sense, otherwise
they'll never be able to mutually close the channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
And make all the callers choose which one. In general, I prefer warn,
which lets them reconnect and try again, however some places are either
stated that they must be errors in the spec itself, or in openingd
where we abandon the channel when we close the connection anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: we now send warning messages and close the connection, except on unrecoverable errors.
This is in line with the warnings draft, where all-zeroes in a
channel_id is no longer special (i.e. it will be ignored).
But gossipd would send these if it got upset with us, so it's best
practice to ignore them for now anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we treat error messages from peer which refer to "all channels" as warnings, not errors.
This takes from the draft spec at https://github.com/lightningnetwork/lightning-rfc/pull/834
Note that if this draft does not get included, the peer will simply
ignore the warning message (we always close the connection afterwards
anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now report the new (draft) warning message.
Let plugins know when a channel open has failed.
We need to notify accepters now too, so we remove the check on who's
funding the channel before sending the 'failed' message from
dualopend->master.
We already know what the channel id is, we should go ahead and pass it
on to any listening plugins -- this makes clean up easier/possible
if a open fails early on and we've got reserved utxos.
There's a case where a dropped funding_locked will result in the peer
moving onto channeld, while we stay in dualopend. As we haven't
received their funding_locked, we retransmit tx_sigs, which channeld
will need to handle.
With the patch the peer drops it on the floor; the peer will resend
funding_locked on reconnect, which will correctly advance us to
channeld and CHANNELD_NORMAL
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.
We were incorrectly saving the *remote's* commitment transaction as the
'last_tx' for a peer, not our own local one.
When we applied the 'remote_commit_sig' to it, it would fail since the
remote's signature doesn't validate for their commitment transaction.
This will make it possible to do RBF, since we can re-start the opening
process in dualopend while waiting for lock-in.
Note the new channel states are being used, DUALOPEND_INIT and
DUALOPEND_AWAITING_LOCKIN, to differentiate from openingd/channeld opens
This is vital for calculating merkle trees; I previously used
towire+fromwire to get this!
Requires generation change so we can magic the ARRAY_SIZE var (the C
pre-processor can't uppercase things).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We let the plugin decide what feerate to accept/whether or not to add
funds to the open. To aid this decision, we also send the plugin what we
(c-lightning) currently have as our max and min acceptable feerates.
We also now use these as our default for max/min acceptable feerate
range when sending an openchannel offer to a peer.
In the future, it might be a good idea to make these more easily
changeable, either via a config setting (?) or a command param.
Note that check-whitespace and check-bolt already do this, so we
can eliminate redundant lines in common/Makefile and bitcoin/Makefile.
We also include the plugin headers in ALL_C_HEADERS so they get
checked.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes#4140
Reported-By: @PsySc0rpi0n
Changelog-Fixed: openingd now uses the correct dust limit for determining the allowable floor for a channel open (affects fundee only)
This is handy/necessary for getting multifundchannel to work, as we need
to know what output to tell all the other peers about.
Changelog-Added: Experimental!! JSON-RPC: openchannel_init returns a field `funding_serial` that indicates the serial_id of the funding output in the provided PSBT
`check_balances` had a weird interface because it was meant to be able
to be used at any 'intermediate' point to verify that a single side had
a valid inputs/output balance.
This was worse than useless. Now it just straight checks for both sides'
balances are correct and that both sides pay their fees. Called after
transaction is constructed.
There's a few structs/wire calls that only exist under experimental features.
These were in a common file that was shared/used a bunch of places but
this causes problems. Here we move one of the problematic methods back
into `openingd`, as it's only used locally and then isolate the
references to the `witness_stack` in a new `common/psbt_internal` file.
This lets us remove the iff EXP_FEATURES inclusion switches in most of
the Makefiles.
Rusty pointed out that having an empty channel_id is suboptimal; adding
another call is probably the right idea rather than re-using an existing
one.
Suggested-By: @rustyrussell
There are 3 commands for opening a channel with dualfunding.
`openchannel_init` is the first of these.
It initializes the open-channel dialog, and stops once we've run out of
updates (input/outputs) to send to the peer.
`test_opening_tiny_channel` fails if EXPERIMENTAL_FEATURES is on because
we don't include the anchor in our reserve if we're the channel opener.
Seems fine to include in all cases?
1. Rename memleak_enter_allocations to memleak_find_allocations.
2. Unify scanning for pointers into memleak_remove_region / memleak_remove_pointer.
3. Document the functions.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
clang did the hard work here:
openingd/dualopend.c:958:42: error: result of comparison of constant 'WIRE_DUAL_OPEN_FAIL' (7003) with expression of type 'u8' (aka 'unsigned char') is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if ((msg_type = fromwire_peektype(msg)) == WIRE_DUAL_OPEN_FAIL) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
Co-Authored-by: the clang compiler
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
We can use a fixed value and close the channel if they don't cover their
amount; this wasn't really helping with anything other than setting a
floor for an expected feerate
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 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
There's a lot of it, and it means we can't `make check-source` on
these files.
Also bring bolt quotes up-to-date.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's now only needed by devtools/mkfunding, so include a reduced one
there, and this also means we remove tx_spending_utxos().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids overwriting the ones in git, and generally makes things neater.
We have convenience headers wire/peer_wire.h and wire/onion_wire.h to
avoid most #ifdefs: simply include those.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to make experimental versions of these completely separate files.
Also remove the dependency on the Makefile itself: it simply causes
unnecessary churn. We can always force-rebuild when we change a rule.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And document exactly what it does: insist that an HTLC can pass of
this value (module assumptions of feerate).
Note that we remove the "is_opener" test from the capacity calculation
for anchor fees: it doesn't matter which side it is, someone has to pay
for anchor fees to it deducts from capacity.
This change breaks the test, which we rewrite.
Changelog-Changed: config: `min-capacity-sat` is now stricter about checking usable capacity of channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Includes:
psbt: Use renamed functions for new wally version
psbt: Set the transaction directly to avoid script workarounds
psbt: Use low-S grinding when computing signatures
tx: Use wally_tx_clone from libwally now that its exported
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
It's not all that rare to do these operations, and requiring annotations
for it is a little painful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #3815
Changelog-Fixed: Fixed a deviation from BOLT#2: if both nodes advertised `option_upfront_shutdown_script` feature: MUST include ... a zero-length `shutdown_scriptpubkey`.
The main change here is that the previously-optional open/accept
fields and reestablish fields are now compulsory (everyone was
including them anyway). In fact, the open/accept is a TLV
because it was actually the same format.
For more details, see lightning-rfc/f068dd0d8dfa5ae75feedd99f269e23be4777381
Changelog-Removed: protocol: support for optioned form of reestablish messages now compulsory.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
now that witness script data is saved into the tx/psbt which is
serialized across the wire, there's no reason to use witscript to do
this. good bye witscript!
Since we now over-write the wally malloc/free functions, we need to do
so for tests as well. Here we pull up all of the common setup/teardown
logic into a separate place, and update the tests that use libwally to
use the new common_setup core
Changelog-None
Previously we've used the term 'funder' to refer to the peer
paying the fees for a transaction; v2 of openchannel will make
this no longer true. Instead we rename this to 'opener', or the
peer sending the 'open_channel' message, since this will be universally
true in a dual-funding world.
It's almost always "their_features" and "our_features" respectively, so
make those names clear.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that now we check capacity once we've figured out which peer, which
broke a test (we returned "unknown peer" instead of "capacity exceeded"),
so we rework that too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Turns out that unnecessary: all callers can access the feature_set,
so make it much more like a normal primitive.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't compile with NDEBUG defined, but if we did, this code would
vanish. I did a quick audit, inspired by @ZmnSCPxj.
I actually hacked up something to compile with NDEBUG (many unused vars
resulted, and of course unit tests are allowed to rely on assert()), and
after this the testsuite still passes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These messages may be exchanged between the master and any daemon. For now
these are just the daemons that a peer may be attached to at any time since
the first example of this is the custommsg infrastructure.
This makes it clear we're dealing with a message which is a wrapped error
reply (needing unwrap_onionreply), not an already-wrapped one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We still close the channel if we *send* an error, but we seem to have hit
another case where LND sends an error which seems transient, so this will
make a best-effort attempt to preserve our channel in that case.
Some test have to be modified, since they don't terminate as they did
previously :(
Changelog-Changed: quirks: We'll now reconnect and retry if we get an error on an established channel. This works around lnd sending error messages that may be non-fatal.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is an intermediary step: we still don't save it to the database,
but we do use the fee_states struct to track it internally.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This uses the same state machine as HTLCs, but they're only
ever added, not removed. Since we can only have one in each
state, we use a simple array; mostly NULL.
We could make this more space-efficient by folding everything into the
first 5 states, but that would be more complex than just using the
identical state machine.
One subtlety: we don't send uncommitted fee_states over the wire.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
test_openchannel_hook_1:
MEMLEAK: 0x557593c164e8'
label=wire/fromwire.c:320:char[]'
backtrace:'
ccan/ccan/tal/tal.c:437 (tal_alloc_)'
ccan/ccan/tal/tal.c:466 (tal_alloc_arr_)'
wire/fromwire.c:320 (fromwire_wirestring)'
openingd/gen_opening_wire.c:205 (fromwire_opening_got_offer_reply)'
openingd/openingd.c:1067 (fundee_channel)'
openingd/openingd.c:1279 (handle_peer_in)'
openingd/openingd.c:1535 (main)'
parents:
fromwire_opening_got_offer_reply() allocates two fields off NULL:
err_reason and our_upfront_shutdown_script. err_reason is used
immediately afterwards (and was the leak detected here), so fixing
that is easy.
To fix the leak of our_upfront_shutdown_script, it makes sense to simply
make it a member of 'state'.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rounds out the application of `upfront_shutdown_script`, allowing
an accepting node to specify a close_to address.
Prior to this, only the opening node could specify one.
Changelog-Added: Plugins: Allow the 'accepter' to specify an upfront_shutdown_script for a channel via a `close_to` field in the openchannel hook result
--dev-force-tmp-channel-id flag takes a 64-character hex string
to use as the temporary channel id. Useful for spec tests
[ Fixed crash in non-DEVELOPER mode --RR ]
Changelog-None
Takes advantage of upfront-shutdown-script to permit users to
specify the close-to address for a channel at open, by adding
a `close_to` field to `fundchannel_start`.
Note that this only is in effect if `fundchannel_start` returns
with `close_to` set -- otherwise, peer doesn't
support `option_upfront_shutdown_script`.
This is mainly an internal-only change, especially since we don't
offer any globalfeatures.
However, LND (as of next release) will offer global features, and also
expect option_static_remotekey to be a *global* feature. So we send
our (merged) feature bitset as both global and local in init, and fold
those bitsets together when we get an init msg.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now have a pointer to chainparams, that fails valgrind if we do anything
chain-specific before setting it.
Suggested-by: Rusty Russell <@rustyrussell>
Valgrind error file: valgrind-errors.112365
==112365== Conditional jump or move depends on uninitialised value(s)
==112365== at 0x1105E0: main (openingd.c:1504)
==112365==
==112365== Conditional jump or move depends on uninitialised value(s)
==112365== at 0x110604: main (openingd.c:1507)
==112365==
==112365== Conditional jump or move depends on uninitialised value(s)
==112365== at 0x110628: main (openingd.c:1510)
==112365==
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's generally clearer to have simple hardcoded numbers with an
#if DEVELOPER around it, than apparent variables which aren't, really.
Interestingly, our pruning test was always kinda broken: we have to pass
two cycles, since l2 will refresh the channel once to avoid pruning.
Do the more obvious thing, and cut the network in half and check that
l1 and l3 time out.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When the peer sends back an error, we return null, but sending
a NULL message back to lightningd causes a parsing error on their
side. negotiation_failed already calls back the peer, all we need
to do here is exit.
The way we build transactions, serialize them, and compute fees depends on the
chain we are working on, so let's add some context to the transactions.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
It probably doesn't matter to "fundchannel_cancel" exactly why the
fundchannel didn't work (though it can read the error msg), and we
should always fail any pending fundchannel_complete command.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Big wiring re-org for funding-continue
In openingd, we move the 'persistent' state (their basepoints,
pubkey, and the minimum_depth requirement for the opening tx) into
the state object. We also look to keep code-reuse between
'continue' and normal 'fundchannel' as high as possible. Both
of these call the same 'fundchannel_reply' at the end.
In opening_control.c, we remap fundchannel_reply such that it is
now aware of the difference between an external/internally funded
channel open. It's the same return path, with the difference that
one finishes making and broadcasting the funding transaction; the
other is skips this.
Add an RPC method (not working at the moment) called
`fundchannel_continue` that takes as its parameters a
node_id and a txid for a transaction (that ostensibly has an output
for a channel)
