Failing this requirement at sending means a strict receiver would
fail our channel while processing a HTLC routed from a third-party.
Fix by enforcing check on both sender and receiver side.
Fixes issue #493 and should resolve some issues where other nodes
(incorrectly) reject channel_update/node_announcement messages
which have a serial number that is not a relatively recent
timestamp.
This is a cheap way to fix an error in Router serialization
roundtrip due to us calling read_to_end during the read of
channel/node announcement/updates. During normal message reading,
we only have limited bytes to read (specifically the message buffer)
so this is fine, however when we read them inside Router, we have
more data from other fields of the Router available as well. Thus,
we end up reading the entire rest of the Router into one message
field, and failing to deserialize.
Because such fields are always stored in Option<>s, we can simply
use a LengthLimitingStream in the Option<> serialization format and
make only the correct number of bytes available.
By using a variable-length integer for the new field, we avoid
wasting space compared to the existing serialization format.
This makes Readable symmetric with Writeable and makes sense -
something which is Readable should be Readable for any stream which
implements std::io::Read, not only for a stream type it decides on.
This solves some lifetime-compatibility issues in trying to read()
from a LengthLimitingReader in arbitrary Readable impls.
This removes the somewhat-easy-to-misuse Clone from ChannelMonitors,
opening us up to being able to track Events in ChannelMonitors with
less risk of misuse.
Sadly it doesn't remove the Clone requirement for ChannelKeys,
though gets us much closer - we now just need to request a second
copy once when we go to create the ChannelMonitors.
This is a rather huge diff, almost entirely due to removing the
type parameter from ChannelError which was added in
c20e930b31 due to holding the
ChannelKeys in ChannelMonitors.
This prepares for only creating the ChannelMonitor on funding by
removing any channel_monitor calls from Channel open/accept-time to
funding-signed time.
This is a rather big step towards using the new ChannelMonitorUpdate
flow, using it in the various commitment signing and commitment
update message processing functions in Channel. Becase they all
often call each other, they all have to be updated as a group,
resulting in the somewhat large diff in this commit.
In order to keep the update_ids strictly increasing by one for
ease of use on the user end, we have to play some games with the
latest_monitor_update_id field, though its generally still pretty
readable, and the pattern of "get an update_id at the start, and
use the one we got at the start when returning, irrespective of
what other calls into the Channel during that time did" is
relatively straightforward.
This is the first of several steps to update ChannelMonitor updates
to use the new ChannelMonitorUpdate objects, demonstrating how the
new flow works in Channel.
This is the first step in migrating ChannelMonitor updating logic
to use incremental Update objects instead of copying the
ChannelMonitors themselves and insert_combine()ing them.
This adds most of the scaffolding and updates relevant comments to
refer to the new architecture, without changing how any actual
updates occur.
Currently Channel relies on its own internal channel_monitor copy
to keep track of funding_txo information, which is both a bit
awkward and not ideal if we want to get rid of the ChannelMonitor
copy in Channel.
Instead, just duplicate it (its small) and keep it directly in
Channel, allowing us to remove the (super awkward)
ChannelMonitor::unset_funding_txo().
In the process of removing a local ChannelMonitor in each Channel,
we need to track our counterpartys' commitment secrets so that we
can check them locally instead of calling our channel monitor to
do that work for us.
Previously, when attempting to write out a channel with some
RemoteAnnounced pending inbound HTLCs, we'd write out the count
without them, but write out some of their fields. We should drop
them as intended as they will need to be reannounced upon
reconnection.
This was found while attempting to simply reproduce a different
bug by adding tests for ChannelManager serialization rount-trip at
the end of each functional_test (in Node::drop). That test is
included here to prevent some classes of similar bugs in the future.
We previously tracked funding transaction confirmation by marking
funding_tx_confirmations to 1 when we see it in a block and
incrementing each block thereafter if its non-0. To avoid
double-incrementing the first confirmation, we did the increment
(and funding_locked check) after doing the first-confirmation
checks. Thus, we'd never hit the funding_locked case during the
first confirmation.
To address this, we simply swap the order of the checks, though
bumping the funding_tx_confirmations increment up to the top.
Reported-by: Igor Cota <igor@codexapertus.com>
If our counterparty burns their funds by revoking their current
commitment transaction before we've sent them a new one, we'll step
forward the remote commitment number. This would be otherwise fine
(and may even encourage them to broadcast their revoked state(s) on
chain), except that our new EnforcingChannelKeys expects us to not
jump forward in time. Since it isn't too important that we punish
our counterparty in such a corner-case, we opt to just close the
channel in such a case and move on.
Fix a crash where previously we weren't able to detect any accepted
HTLC if its witness-encoded cltv expiry was different from expected
ACCEPTED_HTLC_SCRIPT_WEIGHT. This should work for any cltv expiry
included between 0 and 16777216 on mainnet, testnet and regtest.
The Features::new() method is nonsense and doesn't describe what
features were being set - we introduce an empty() and supported()
constructors instead.
This merges local and global features into one struct, which is
parameterized by where it appers. The parameterization restricts
which queries can be made and which features can be set, in line
with the latest BOLT 9.
Closes#427.
We now have current-local-tx broadcast ability in channel monitors
directly (for ChannelManager deserialization), so we can just use
that instead of always having the Channel store signed ready-to-go
copies of the latest local commitment transaction.
This is further kinda nice since ChannelMonitor is live and can, eg
broadcast HTLC-Success transactions immediately as they will be
generated at broadcast time instead of in advance.
Finally, this lets us clean up a tiny bit in Channel.
This adds a new fn to ChannelKeys which is called when we generte
a new remote commitment transaction for signing. While it may be
theoretically possible to unwind state updates by disconnecting and
reconnecting as well as making appropriate state machine changes,
the effort required to get it correct likely outweighs the UX cost
of "preflighting" the requests to hardwre wallets.
Instead of having in-memory access to the list of private keys
associated with a channel, we should have a generic API which
allows us to request signing, allowing the user to store private
keys any way they like.
The first step is the (rather mechanical) process of templating
the entire tree of ChannelManager -> Channel impls by the
key-providing type. In a later commit we should expose only public
keys where possible.
