* Add missing cases to PostRestart
When a channel is closed we want to remove its HTLCs from our
list of pending broken HTLCs (they are being resolved on-chain).
We should also ignore outgoing HTLCs that have already been
settled upstream (which can happen when downstream is closing).
* Watch for downstream HTLC resolved on-chain
When a downstream channel is closing, we can safely fail upstream the
HTLCs that were either timed out on-chain or not included in the
broadcast commit transaction.
Channels will not always raise events about those after a reboot, so we
need to inspect the channel state and detect such HTLCs.
* Add helper function to HTLC scripts
To extract the payment_hash or preimage from an HTLC script seen on-chain.
* Cleanup on-chain HTLC timeout handling for MPP
With MPP, it's possible that a channel contains multiple HTLCs for the
same payment hash, and potentially even for the same expiry and amount.
We add more fine-grained handling of HTLC timeouts that share the same
payment hash. This allows a cleaner handling after a restart, and makes
sure we correctly detect failure that should be propagated upstream.
Otherwise we wouldn't be losing any money, but some channels may be closed
that we can avoid.
* Handle out-of-order htlc-timeout txs
It may happen that a commit tx and some htlc-timeout txs end up in the
same block. In that case, there is no guarantee on the order we'll receive
the confirmation events.
If any tx in a local/remoteCommitPublished is confirmed, that implicitly
means that the commit tx is confirmed (because it spends from it).
So we can consider the closing type known and forward the failure upstream.
* removed the `Direction` class
* improved the non-reg test for htlcs
- check actual content instead of only success and roundtrip
- use randomized data for all fields instead of all-zero
- check the remaining data, not only the decoded value (codecs are
chained so a regression here will cause the next codec to fail)
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
* Sort commit transaction outputs using BIP69 + CLTV as tie-breaker for offered HTLCs
* Type DirectedHtlc:
We now use a small hierarchy of classes to represent HTLC directions.
There is also a type alias for a collection of commitment output links.
* front now handles ping/sync
Peer has been split in two and now handles only channel related stuff.
A new `PeerConnection` class is in charge of managing the BOLT 1 part
(init handshake, pings) and has the same lifetime as the underlying
connection.
Also, made `TransportHandler` be a child of `PeerConnection` by making
the `remoteNodeId` an attribute of the state of `PeerConnection` instead
of a constructor argument (since we cannot be sure of the remote nodeid
before the auth handshake is done). Now we don't need to worry about
cleaning up the underlying `TransportHandler` if the `PeerConnection`
dies.
* remove `Authenticator`
Instead of first authenticating a connection, then passing it to the
`PeerConnection` actor, we pass the connection directly to the
`PeerConnection` and let it handle the crypto handshake, before the LN
init. This removes a central point of management and makes things easier
to reason about. As a side effect, the `TransportHandler` actor is now a
child of `PeerConnection` which gives us a guarantee that it dies when
its parent dies.
* separated connection logic from `Peer`
The `ReconnectionTask` actor handles outgoing connections to a peer. The
goal is to free
the `Peer` actor from the reconnection logic and have it just react to
already established
connections, independently of whether those connections are incoming or
outgoing.
The base assumption is that the `Peer` will send its state transitions
to the `ReconnectionTask` actor.
This is more complicated than it seems and there are various corner
cases to consider:
- multiple available addresses
- concurrent outgoing connections and conflict between
automated/user-requested attempts
- concurrent incoming/outgoing connections and risk of reconnection
loops
- etc.
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
* Refactor timed out HTLC helpers: directly take a DATA_CLOSING
and extract the relevant parts.
* ClosingStateSpec: test dust HTLCs
* Improve ClosingStateSpec
* Clean up usage of AddHtlcFailed
We were abusing AddHtlcFailed in some cases where an outgoing HTLC
was correctly added, but was later settled on-chain (fulfilled, timed
out or overridden by a different commit transaction).
These cases are now specifically handled with new Relayer.ForwardMessage
dedicated to on-chain settling.
* Refactor Relayer's ForwardMessages
ForwardFail and ForwardFulfill are now traits.
Handle both on-chain and remote fail/fulfills.
We were allowing users to set htlc-minimum-msat to 0, which directly
contradicts the fact that we must never send an HTLC for 0 msat.
We now explicitly disallow that behavior: the minimum is 1 msat.
In case the remote side of a channel had set its htlc-minimum-msat to 0,
we would forward HTLC with a value of 0 msat if a sender crafted such a
payment. The spec disallows that, so we now explicitly check for that
lower bound.
Also removed string interpolation for some of the more expensive debug
lines. It's a trade-off performance vs readability and is probably not
worth changing for info level logs, which will be enabled anyway.
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
See https://github.com/lightningnetwork/lightning-rfc/issues/728
Add an additional reserve on the funder to prevent emptying and then
being stuck with an unusable channel.
As fundee we don't verify funders comply with that change.
We may enforce it in the future when we're confident the network as a
whole enforces that.
Previous implementation in #1317 wasn't working because in a bug in the
transition. Added a test and fixed it.
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
Refactor and improve payment metrics:
* Unify in a Monitoring object.
* Add helper functions and objects.
* Add more error metrics.
* Add more latency metrics.
* Add metrics to post-restart HTLC cleanup
* Add metrics to router path-finding
As long as we receive messages from our peer, we consider it is
online and don't send ping requests. If we don't hear from the
peer, we send pings and expect timely answers, otherwise we'll
close the connection.
This is implemented by scheduling a ping request every 30 seconds,
and pushing it back every time we receive a message from the peer.
* Support wumbo channels:
- use bits 18, 19
- compute the min depth for the funding transaction according to the channel size
- update routing heuristics for a wumbo world:
- the lower bound is the 25th percentile of current channel capacity on the network
- the higher bound is the most common capacity for wumbo channels
- add 'max-funding-satoshis' configuration key to allow to set the maximum channel size that will be accepted
Make DLP data mandatory in ChannelReestablish.
We make them mandatory to allow extending the message with TLVs.
Make upfront_shutdown_script a TLV record that we always include in
open_channel / accept_channel.
See https://github.com/lightningnetwork/lightning-rfc/pull/714.
Add an abstraction of a PaymentPart.
Remove unnecessary intermediary case classes.
This allows extending how payment parts can be received.
It's not limited to HTLCs (could be swaps, pay-to-opens, etc).
Currently, those methods throw exceptions, and we rely on `Channel` to
call them within a `Try(...)`. It puts more burden on `Channel` and
isn't very functional.
Some methods were returning an `Either[]`, which seem to play the role
of a `Try` but isn't used. It seems the idea was to not fail the channel
upon receiving a `fulfill`/`fail` for an unknown htlc, but it is not
fully wired, and isn't compliant (BOLT 2):
> A receiving node:
> if the id does not correspond to an HTLC in its current commitment
transaction:
> MUST fail the channel.
For signature-related methods, I went with the minimal change of
encapsulating portions of the code inside a `Try {...}` to minimize risk
of regression. We could also make `CommitmentSpec` methods return
`Try[]` but I suspect that would be more complicated with little
benefit.
Note that if some part isn't covered by the `Try` and ends up throwing
an exception, that will be covered by the `handleException` handler of
`Channel`.
Fixes#1305.
SendToRoute previously didn't accept invoice routing hints.
This was a limitation when paying a wallet invoice.
Invoice hints are now correctly taken into account.
Writing the PaymentRelayed event to the DB is not atomic.
Each part is written independently of the others (one row per part).
This is fine as nothing relies on this event being written atomically.
However tests were expecting that and we would observe rare
test failures on travis.
When coming back online, re-send private channels' `channel_update`.
This makes sure it gets rebroadcast regularly in case it was missed.
Since it's a private channel, it won't spam the network.
We use the socks5 proxy that is defined in the configuration and is typically used to connect to LN nodes running as TOR hidden services.
This should allow users to connect to Electrum servers that are running behind TOR.
* Refactor payment errors
When sending payments, it makes it easier for a wallet to display
the correct localized error message to users.
* Faster Trampoline payments fulfill
We were previously waiting for the whole downstream payment
to be settled (all individual HTLCs).
We can do better and fulfill upstream as soon as we get the preimage
(which only needs one downstream fulfill).
We currently rely on `require`, which is convenient, but doesn't allow
fine-grained exception control.
Also, in case of errors, logging is done at the supervisor level, where
we lose the remote `node_id`.
Instead, we type some crypto-related errors and log them in the
`TransportHandler`, which already has the correct MDC.
The DB ordering is not deterministic.
For multi-part payments, the first timestamp is taken.
This can vary depending on which record is listed first.
Using the same timestamp avoids a failed assertion.
Instead of emitting this event when we send a signature, we emit it when
our `availableBalanceForSend` actually changes. This happens:
- when we send a new `update_*`;
- when we receive a `commit_sig`, which may acknowledge one or several
`update_*` sent by our peer.
We choose to only emit this event in `NORMAL` state, because its goal is
to give information about what payments we can make, which can only
happen in that state.
NB: other events `ChannelSignatureSent` and `ChannelSignatureReceived` give
a different type of information, and are sent in all states where
signatures are exchanges, not only in `NORMAL`.
The field `localBalance` has been removed because it was ambiguous, and so is
the balance tracking in the database, which wasn't very useful.
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
* Electrum: allow watcher to watch for mempool transactions
Watcher now handles WatchConfirmed watches where min depth
is set to 0: the watch event will be sent when the tx enters the
mempool of the bitcoin node our Electrum server is connected to.
For 0-conf channel, use scids with a height of 0 and a tx index
generated from the first 16 bytes of the funding txid. This gives us
unique ids that can still be identified as 0-conf channel.
Let a sender manually split a payment and specify a trampoline route.
Fix two flaky tests where the order of payment parts could be
different, resulting in a failed equality test.
If we're relaying multiple HTLCs for the same payment_hash,
we need to list all of those.
The previous code only handled that when Trampoline was used.
Comparing with the router ActorRef simply didn't work.
The reason is probably because Peers receive the router's supervisor ref
which doesn't match what `self` is inside `Router`.
Checking that the origin was the router felt brittle anyway.
We're now correctly typing the gossip origin.
We don't implement the upfront_shutdown_script feature.
However we update our encoding to always specify it.
This allows extending OpenChannel/AcceptChannel with tlv streams.
There is one caveat: Phoenix shipped with a version that's incompatible.
So we use a workaround to identify unpatched Phoenix versions
and send them the old encoding.
With MPP and Trampoline (and particularly the combination of the two),
we need to keep track of multiple amounts, recipients and fees.
There's a trampoline fee and a fee to reach the first trampoline node.
The trampoline nodes must appear in the route, but not as payment recipients.
Adding new fields to payment events and DB structs lets us distinguish those.
We also relax the spec requirement about feature graph dependencies.
The requirement to include `var_onion_optin` in invoice feature bits
was added after the first Phoenix release.
Phoenix users will thus have non spec-compliant invoices in their
payment history.
We accept invoices that don't set this field; this is a harmless
spec violation (as long as we set it in new invoices).
There was a rounding issue with the availableForSend/Receive calculation.
Because CommitTx fee and Htlc fee were computed separately,
but each was individually rounded down to Satoshis, we could
end up with an off-by-one error.
This resulted in an incapacity to send/receive the maximum amount available.
We now allow computing fees in msat, which removes rounding issues.
c-lightning fails to decode empty arrays of scids or timestamps with an encoding type set to COMPRESSED_ZLIB.
The spec is not specific enough on whether this is valid or not, so we'll set the encoding type of empty arrays to UNCOMPRESSED.
