When we force-close a channel, for whatever reason, it is nice to
send an error message to our peer. This allows them to closes the
channel on their end instead of trying to send through it and
failing. Further, it may induce them to broadcast their commitment
transaction, possibly getting that confirmed and saving us on fees.
This commit adds a few more cases where we should have been sending
error messages but weren't. It also includes an almost-global
replace in tests of the second argument in
`check_closed_broadcast!()` from false to true (indicating an error
message is expected). There are only a few exceptions, notably
those where the closure is the result of our counterparty having
sent *us* an error message.
Electrum clients primarily operate in a world where they query (and
subscribe to notifications for) transactions by script_pubkeys.
They may never learn very much about the actual blockchain and
orient their events around individual transactions, not the
blockchain.
This makes our ChannelManager interface somewhat more amenable to
such a client by splitting `block_connected` into
`transactions_confirmed` and `update_best_block`. The first handles
checking the funding transaction and storing its height/confirmation
block, whereas the second handles funding_locked and reorg logic.
Sadly, this interface is somewhat easy to misuse - notifying the
channel of the funding transaction being reorganized out of the
chain is complicated when the only notification received is that
a new block is connected at a given height. This will be addressed
in a future commit.
Previously, we expected every block to be connected in-order,
allowing us to track confirmations by simply incrementing a counter
for each new block connected. In anticipation of moving to a
update-height model in the next commit, this moves to tracking
confirmations by simply storing the height at which the funding
transaction was confirmed.
This commit also corrects our "funding was reorganized out of the
best chain" heuristic, instead of a flat 6 blocks, it uses half the
confirmation count required as the point at which we force-close.
Even still, for low confirmation counts (eg 1 block), an ill-timed
reorg may still cause spurious force-closes, though that behavior
is not new in this commit.
This expands the assertions on block ordering to apply to
`#[cfg(test)]` builds in addition to normal builds, requiring that
unit and functional tests have syntactically-valid (ie the previous
block hash pointer and the heights match the blocks) blockchains.
This requires a reasonably nontrivial diff in the functional tests
however it is mostly straightforward changes.
Many functional tests rely on being able to call block_connected
arbitrarily, jumping back in time to confirm a transaction at a
specific height. Instead, this takes us one step towards having a
well-formed blockchain in the functional tests.
We also take this opportunity to reduce the number of blocks
connected during tests, requiring a number of constant tweaks in
various functional tests.
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
Co-authored-by: Matt Corallo <git@bluematt.me>
Sadly the connected-in-order tests have to be skipped in our normal
test suite as many tests violate it. Luckily we can still enforce
it in the tests which run in other crates.
Co-authored-by: Matt Corallo <git@bluematt.me>
Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>
We allow users to configure the to_self_delay, which is analogous to
the cltv_expiry_delta in terms of its security context, so we should
allow users to specify both.
We similarly bound it on the lower end, but reduce that bound
somewhat now that it is configurable.
Useful for constructing route hints for private channels in invoices.
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
Co-authored-by: Antoine Riard <ariard@student.42.fr>
This will be used to expose forwarding info for route hints in the next commit.
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
Co-authored-by: Antoine Riard <ariard@student.42.fr>
This will be filled in in upcoming commits, then exposed in ChannelDetails
to allow constructing route hints for invoices.
Also update the cltv_expiry_deta comment in msgs::ChannelUpdate
Co-authored-by: Valentine Wallace <vwallace@protonmail.com>
Co-authored-by: Antoine Riard <ariard@student.42.fr>
Modify NetGraphMsgHandler::handle_query_channel_range to always use
first_blocknum=0 in replies. This is spec compliant after changes to
make sequence completion explicity using sync_complete.
Modifies NetGraphMsgHandler::handle_query_channel_range to use a constant
max value in replies. Modifies tests to generate 8000 channels instead
of making this value configurable.
We currently only use it to override the graph-specific features
returned in the route, though we should also use it to enable or
disable MPP.
Note that tests which relied on MPP behavior have had all of their
get_route calls upgraded to provide the MPP flag.
In the past we skipped doing this since invoice parsing occurs in a
different crate. However, we need to accept InvoiceFeatures in routing
now that we support MPP route collection, to detect if we can select
multiple paths or not. Further, we should probably take
rust-lightning-invoice as either a module or a subcrate in this repo.
`get_outputs_to_watch` returned a reference to an existing
`HashMap` avoiding extra clones, but there isn't a huge reason to
do so now that we have to clone to copy it out of the
`ChannelMonitor` mutex. Instead, return a `Vec` since it may be
less memory and it allows us to have a bindings C mapping for the
function.
Co-authored-by: Jeffrey Czyz <jkczyz@gmail.com>
Co-authored-by: Matt Corallo <git@bluematt.me>
