Matt Corallo [Mon, 19 Nov 2018 03:01:32 +0000 (22:01 -0500)]
Close channels on Err returns inside the same channel_state lock
If we never accessed channels for a peer outside of a message
handler for that peer then this wouldn't be a problem since message
handlers are required to be serialized per-peer. However, that
isn't the world we live in - we may want to forward payments or we
may get a send_payment call.
Matt Corallo [Fri, 23 Nov 2018 01:50:13 +0000 (20:50 -0500)]
Remove remaining uses of HandleError in Channel Err return values
This converts block_connected failures to returning the
ErrorMessage that needs to be sent directly, since it always
results in channel closure and never results in needing to call
force_shutdown. It also converts update_add_htlc and closing_signed
handlers to ChannelError as the rest of the message handlers.
Matt Corallo [Fri, 23 Nov 2018 04:44:42 +0000 (23:44 -0500)]
Split channel_reserve_test so we don't rely on unfilled Err actions
Currently channel_reserve_test sends a garbage update_add_htlc
message and then relies on it being silently ignored to continue
using the channel. This shouldn't be the case, so take the easy
way out and split the test in two, at first not delivering the
bogus update_add_htlc and then delivering it, but not running the
rest of the test.
Matt Corallo [Tue, 20 Nov 2018 20:09:47 +0000 (15:09 -0500)]
Check P2WPKH script against expected before gen'ing an output event
This fixes a bug in 3518f1f85d8a3daff451b3fe56cc7854b833e2bd where
we may generate an output event for a P2WPKH output which is not
ours if the transaction has a sequence/lock_time combination which
false-positives our remote tx detection.
Also note that the TODO is removed as this should already be
covered without issue if the client properly replays the chain on
restart.
Matt Corallo [Mon, 19 Nov 2018 22:12:17 +0000 (17:12 -0500)]
Provide commitment point to monitor with the remote txn update
This extends 1b33064554ae48c9173acf8bf1e0052d33a855df by
re-simplifying the ChannelMonitor <-> Channel interface a bit as we
never have any use for the latest remote commitment point until we
have knowledge of a remote transaction generated using it.
Matt Corallo [Tue, 30 Oct 2018 00:38:29 +0000 (20:38 -0400)]
Move a ton of Channel functions to ChannelError from HandleError
This is a big patch, but its all very mechanical, everything here
should be pretty obvious, and it all has to happen at once due to a
few common utility functions all having the same return type.
Note that this exposes a race in channel closure where we may
access a channel via some non-peer-specific mechanism like
forwarding an HTLC or sending a payment during the time between
the channel gave us a Close error and expected us to never call it
again and the time we actually removed it from the channel_state
set outside of the internal_* handler.
Matt Corallo [Sun, 18 Nov 2018 21:15:08 +0000 (16:15 -0500)]
Don't unwrap() get_channel_update result in HTLC router
This fixes a bug in 78232f2aeded08b32fa4ebfeb0b77d80b337518d found
by fuzzer - if the channel isn't yet fully established we will call
get_channel_update(), get an Err result, and then unwrap() it. If
this actually happens it means someone on the network is making up
short_channel_ids and trying to route over them, but that shouldn't
result in us crashing
Matt Corallo [Thu, 8 Nov 2018 00:06:34 +0000 (10:36 +1030)]
Fix pre-noise peer disconnect panic on non-Err disconnect
366e79615b7251771465d6c69c2941ac233674da fixed the same crash for
Errs that come up during handshake, but was incomplete and should
have just dropped the node_id being different based on
inbound/outbound. This patch does so and actually fixes the issue.
Matt Corallo [Fri, 2 Nov 2018 14:50:32 +0000 (10:50 -0400)]
Fix pre-noise outbound peer disconnect panic found by fuzzer
If we make an outbound connection to a peer who we are already
connected to, and the outbound connection fails
pre-noise-completion, we will remove the original peer connection
from our node_id_to_descriptor map.
The fuzzer managed to find this by crashing in Channel's assertions
that we don't do a get_channel_reestablish() when the Channel isn't
already marked disconnected.
Matt Corallo [Tue, 30 Oct 2018 19:53:34 +0000 (15:53 -0400)]
Always send Shutdown resposnes to Shutdown messages
We always require that any changes to Channel state be committed
immediately (within the same lock) so we should never have
uncommitted changes which would prevent us from sending a Shutdown
response.
Matt Corallo [Wed, 31 Oct 2018 18:38:07 +0000 (14:38 -0400)]
Use non-funder's funding block wait instead of max with ours
This is both required by the protocol and also makes sense - if
we're the funder we don't mind accepting payment on the channel
after one confirmation because we assume we won't double-spend
ourselves.
Matt Corallo [Sat, 20 Oct 2018 22:17:19 +0000 (18:17 -0400)]
Avoid reentrancy of send_data from PeerHandler::read_bytes.
This greatly simplifies clients of PeerHandler, and because almost
all response messages have already been moved to process_events
this doesn't change much effeciency-wise.
Matt Corallo [Sat, 20 Oct 2018 22:46:03 +0000 (18:46 -0400)]
Add a BIG lock to ChannelManager
During normal operation we should never need to take this, so we
use a RwLock that allows normal parallelism until we want to
serialize out our ChannelManager, at which point we can take the
write-mode lock.