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.
While its not necessarily a common operation on a running node,
`get_our_node_id()` is used incredibly heavily in tests, and there
is no reason to not eat the extra ~64 bytes to just cache it.
chain::Filter::register_output may return an in-block dependent
transaction that spends the output. Test the scenario where the txdata
given to ChainMonitor::block_connected includes a commitment transaction
whose HTLC output is spent in the same block but not included in txdata.
Instead, it is returned by chain::Filter::register_output when given the
commitment transaction's HTLC output. This is a common scenario for
Electrum clients, which provided filtered txdata.
Add a method to TestChainSource to test chain::Filter expectations. This
is limited to register_output, allowing tests to assert that the method
was called with a specific output and dictate what the return value is.
Multiple expectations are checked in the order in which they were added.
Failure occurs if a call doesn't match the next expectation or if there
are unsatisfied expectations. If not expectations are added, then no
calls are checked.
Electrum clients will only provide transaction data for outputs that
have been explicitly registered. Hence, upon registering new outputs,
recursively register any outputs to watch contained within dependent
transactions from the same block.
When registering a watched transaction output, any in-block descendant
transactions spending the output must be supplied. Give the block hash
when registering such outputs such that this is possible. Otherwise,
spends from other blocks may be returned inadvertently.
Electrum clients primarily operate by subscribing to notifications of
transactions by script pubkeys. Therefore, they will send filtered
transaction data without including dependent transactions. Outputs for
such transactions must be explicitly registered with these clients.
Therefore, upon block_connected, provide a mechanism for an Electrum-
backed chain::Filter to return new transaction data to scan.
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>
In some PRs, codecov gets mad that the coverage of the patch itself
is lower than the base. In most cases, we largely don't want a Big
Red X, at least as long as the total coverage has not gone down
substantially.
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.
Relative HTML doc paths in doc links works locally, but breaks on
crates.io. Luckily, we can now use explicit full paths and rustdoc
will resolve them for us.
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>
