We used to always target `now() + 300`, which ends up never really
confirming, as the fee estimate bumps into the min-relay-fee
limit. With this commit we set an absolute target of 2 weeks, and a
linear fee rampup, until we are at T-2h, at which point we just stick
with the estimate, and try with this increased feerate to try and get
the sweep confirmed.
This ought to make RBF transactions much more efficient for closing
channels.
Changelog-Fixed onchaind: The sweep deadline for to_us outputs would be reset on each restart of the subdaemon. Now the deadline is absolute in terms of the close height.
The signer may not be present at this time. If we want to keep the
check to protect against bit flips we should move it into `onchaind`
where it doesn't matter as much that the signer may be slow to
respond.
We could get the current key from the reestablish message even if we'd
lost our db, but there are very few of these channels left: we upgraded to use them
in the 2019-01-09 release.
We will eventually remove support altogether, but this is a nice removal of
some ugly code for something which "never happens".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These were removed from the spec.
We still support existing ones, though we were the only implementation
which ever did, and only in experimental mode, so we should be able to
upgrade them and avoid a forced close, with a bit of engineering...
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We still support *existing* channels. Just not new ones (before they could,
in theory, explicitly ask for one).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a u64, we should pass by copy. This is a big sweeping change,
but mainly mechanical (change one, compile, fix breakage, repeat).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the convention everywhere else: allocation ctx comes first, any
other context comes second.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has the benefit of being shorter, as well as more reliable (you
will get a link error if we can't print it, not a runtime one!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
peer_htlcs has become a bit of a dumping ground: move listforwards
etc to its own file.
Also move `struct channel_info` from peer_htlcs.h to channel.h where
it more logically belongs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously, every broadcast was attached to a channel, but we can
make it explicit, so when the context is freed, the re-broadcast stops
(if rebroadcast is set).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I obviously like the word "capabilities" since I reused it to refer
to the HSM's overall features :(
Suggested-by: @ksedgwic
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
In most cases, it's the same as option_anchor_outputs, but for
fees it's different. This transformation is the simplest:
pass it as a pair, and test it explicitly.
In future we could rationalize some paths, but this was nice
and mechanical.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we've set everything up, the replacement code is quite simple.
Some tests now have to deal with RBF though, and our rbf tests need work
since they look for the old onchaind messages.
In particular, when we can't afford the fee we want, we back off to
the next blockcount estimate, rather than spending all on fees
(necessarily). So test_penalty_rbf_burn no longer applies.
Changelog-Changed: Protocol: spending unilateral close transactions now use dynamic fees based on deadlines (and RBF), instead of fixed fees.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Splitting into onchaind_tx() into onchaind_tx_unsigned() and
sign_and_get_witness() makes it easier to reuse for RBF.
Adding more information in onchain_signing_info is required too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have the FEERATE_FLOOR constant if you don't care, but usually you want
to use the current bitcoind lower limit, so call get_feerate_floor()
(which is currently the same, but coming!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is when they closed the channel, we can simply make our own tx to
expire the HTLC. (The other case is where we closed the channel, and
we have a special htlc_timeout tx which we have their signature for).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This breaks tests/test_closing.py::test_onchain_all_dust's accouting
checks.
That test doesn't really test what it claims to test; sure, onchaind
*says* it's going to ignore the output due to high fees, but the tx
still gets mined.
I cannot figure out what the test is supposed to look like, so I
simply disabled the accounting checks :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We add code for the case of spending a (timelocked) to-us output of an
HTLC output, so lightningd can do it (rather than onchaind doing all
the work itself).
onchaind still needs to know whether we bothered to create the tx
(fees might have caused it to evaporate, so it should consider it
immediately resolved rather than waiting for it), and what the
witnesses were, and which parts of the witnesses were signatures (as
these parts might change, with RBF or in future, combining other txs).
The inputs (known to onchaind) and the witnesses (told by lightningd)
uniquely identify the spend for the purposes of onchaind. In
particular, they definitely distinguish HTLC-timeout and HTLC-success
cases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fun story. We're changing onchaind to hand txs to us, and we will
construct them and do the broadcast for it. lightningd tells onchaind
the witness it used (with flags to indicate which fields were
signatures so should be ignored) so onchaind can recognize the tx
when/if it is mined.
And when onchaind was waiting for a CLTV delay, it wouldn't tell
lightningd yet, but wait until the parent was sufficiently deep
But this caused bugs!
In particular, on replay, onchaind would see transactions which it
hasn't sent yet. This was not a problem before, as onchaind had
created the tx, even if it hadn't told lightningd to broadcast it, so
recognized the variant when it came in. When we're relying on
lightningd to tell us what the tx will look like, this doesn't work
any more.
The cause of this is that we fire off txowatches ("this output was
spent!") while we process blocks, and only fire off txwatches ("this
tx increased depth") once all the current blocks are processed. Often
this didn't matter, since we replay messages to onchaind from the
database, *but* we trim the last few blocks on restart (or, if there's
a small reorg while we're stopped), and we can hit this misordering.
Changing our topology code to only ever process one block at a time
would be a solution, but slows down catchup (and tests, where we often
mine a run of blocks).
So, this seems like a premature optimization, but it's really
required! And in future, lightningd can use this knowledge of pending
transactions to combine them in more clever ways.
Note that if a tx is valid at block N, we broadcast it once we see
block N-1, to get it in the mempool for block N.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
At the moment only lightingd needs it, and this avoids missing any
places where we do bip32 derivation.
This uses a hsm capability to mean we're backwards compatible with older
hsmds.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now always double-check bitcoin addresses are correct (no memory errors!) before issuing them.
It's needed as the db and wallet is being set up (db migrations), so
it's simpler this way to always use ld->bip32_base for the next patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We didn't actually populate them properly, and the real annotations
are on inputs and outputs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: JSON-RPC: `listtransactions` `channel` and `type` field removed at top level.
On testnet I noticed if we can't reach bitcoind for some reason, we'll
keep RBFing our penalty tx ("it didn't go in, RBF harder!!"). Makes
no sense to grossly exceed the amount needed for next block, so simply
cap penalty at 2x "estimatesmartfee 2 CONSERVATIVE".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
First, merge the _ahf_ and non-ahf interfaces.
Second, remove the always-NULL txs->cmd field.
Then, add optional id_prefix for bitcoind_sendrawx, so if it's
triggered by a command (e.g. "withdraw") it's shown correctly in logs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
