Base AMP is centered around the concept of a 'payment_secret` - an
opaque 32-byte random string which is used to authenticate the
sender to the recipient as well as tie the various HTLCs which
make up one payment together. This new field gets exposed in a
number of places, though sadly only as an Option for backwards
compatibility when sending to a receiver/receiving from a sender
which does not support Base AMP.
Sadly a huge diff here, but almost all of it is changing the method
signatures for sending/receiving/failing HTLCs and the
PaymentReceived event, which all now need to expose an
Option<[u8; 32]> for the payment_secret.
It doesn't yet properly fail back pending HTLCs when the full AMP
payment is never received (which should result in accidental
channel force-closures). Further, as sending AMP payments is not
yet supported, the only test here is a simple single-path payment
with a payment_secret in it.
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 ability to merge ChannelMonitors in favor of
explicit ChannelMonitorUpdates. It further removes
ChannelManager::test_restore_channel_monitor in favor of the new
ChannelManager::channel_monitor_updated method, which explicitly
confirms a set of updates instead of providing the latest copy of
each ChannelMonitor to the user.
This removes almost all need for Channels to have the latest
channel_monitor, except for broadcasting the latest local state.
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.
The way PeerHandler was written, it was supposed to remove from
self.peers iff the API docs indicate that disconnect_event should
NOT be called (and otherwise rely on disconnect_event to do so).
Sadly, the implementation was way out of whack with reality - in
the implementation, essentially anywhere where PeerHandler
originated the disconnection, the peer was removed and no
disconnect_event was expected. The docs, however, indicated that
disconnect_event should nearly only be called, only not doing so
when the initial handshake message never completed.
We opt to change the docs, mostly, as well as clean up the
ping/pong handling somewhat and rename a few functions to clarify
what they actually do.
This is important for a number of reasons:
* Firstly, I hit this trying to implement rescan in the demo
bitcoinrpc client - if individual ChannelMonitors are out of
sync with each other, we cannot add them all into a
ManyChannelMonitor together and then rescan, but need to rescan
them individually without having to do a bunch of manual work.
Of the three return values in ChannelMonitor::block_connected,
only the HTLCsource stuff that is moved here makes no sense to
be exposed to the user.
* Secondly, the logic currently in ManyChannelMonitor cannot be
reproduced by the user! HTLCSource is deliberately an opaque
type but we use its data to decide which things to keep when
inserting into the HashMap. This would prevent a user from
properly implementing a replacement ManyChannelMonitor, which is
unacceptable.
* Finally, by moving the tracking into ChannelMonitor, we can
serialize them out, which prevents us from forgetting them when
loading from disk, though there are still other races which need
to be handled to make this fully safe (see TODOs in
ChannelManager).
This is safe as no two entries can have the same HTLCSource across
different channels (or, if they did, it would be a rather serious
bug), though note that, IIRC, when this code was added, the
HTLCSource field in the values was not present.
We also take this opportunity to rename the fetch function to match
our other event interfaces, makaing it clear that by calling the
function the set of HTLCUpdates will also be cleared.
Additional changes:
* Update fuzz crate to match ChannelManager's new API
* Update lightning-net-tokio library to match ChannelManager's new ChannelMonitor Deref API
* Update tests to match ChannelManager's new ChannelMonitor Deref API
This exposes the latest Init-context features in the ChannelDetails
passed to the Router during route calculation, which combines those
with the Node-context features tracked from node_announcements to
provide the latest Node-context features in RouteHop structs.
Fields are also added for Channel-context features, though those are
only partially used since no such features are defined today anyway.
These will be useful when determining whether to use new
TLV-formatted onion hop datas when generating onions for peers.
full_stack_target found a crash where we may overflow ruring fee
calculation if a transaction appears on-chain with massive value
available for us to claim. Since these transactions are clearly
bogus, we shouldn't allow full_stack_target to connect them, but
we also improve the error generated by explicitly panicing on them.
Previously, in each of our fuzz tests we had a dummy test which
had a hard-coded hex string which it passed into the fuzz target
so that when a failing test case was found, its hex could be
copied into the test and you could run cargo test to analyze the
failure. However, this was somewhat unwieldy as converting large
tests back and forth between hex and raw files is quite annoying.
Instead, we replace each of those tests with a test in each target
that looks for files in fuzz/test_cases and runs each file it finds.
Since we're editing every bin target anyway, we also automate adding
no_main to libfuzzer builds with #![cfg_attr].
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.
Simplify interfaces between ChannelMessageHandler and PeerManager,
by switching all ChannelMessageHandler errors to HandleError sent
internally instead of being return. With further refactors in Router
and PeerChannelEncryptor, errors management on the PeerManager-side
won't be splitted between try_potential_handleerror and HandleError
processing.
Inside ChannelManager, we now log MsgHandleErrInternal and send
ErrorAction to PeerManager.
On a high-level, it should allow client using API to be more flexible
by polling events instead of waiting function call returns.
We also update handle_error macro to take channel_state_lock from
caller which should avoid some deadlock potential for some edges
cases.
Filter out IgnoreError in handle_error macro, update test in
consequence.
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.
