Currently the underlying array backing the hop's TLVRecords is modified
when combining custom records with the primitive forwarding info. This
commit uses a fresh slice to prevent modifications from mutating the
hop itself.
Previously the invoice registry wasn't aware of replayed htlcs. This was
dealt with by keeping the invoice accept/settle logic idempotent, so
that a replay wouldn't have an effect.
This mechanism has two limitations:
1. No accurate tracking of the total amount paid to an invoice. The total
amount couldn't just be increased with every htlc received, because it
could be a replay which would lead to counting the htlc amount multiple
times. Therefore the total amount was set to the amount of the first
htlc that was received, even though there may have been multiple htlcs
paying to the invoice.
2. Impossible to check htlc expiry consistently for hodl invoices. When
an htlc is new, its expiry needs to be checked against the invoice cltv
delta. But for a replay, that check must be skipped. The htlc was
accepted in time, the invoice was moved to the accepted state and a
replay some blocks later shouldn't lead to that htlc being cancelled.
Because the invoice registry couldn't recognize replays, it stopped
checking htlc expiry heights when the invoice reached the accepted
state. This prevents hold htlcs from being cancelled after a restart.
But unfortunately this also caused additional htlcs to be accepted on an
already accepted invoice without their expiry being checked.
In this commit, the invoice registry starts to persistently track htlcs
so that replays can be recognized. For replays, an htlc resolution
action is returned early. This fixes both limitations mentioned above.
This commit refactors the invoice registry accept/settle logic so that
it doesn't rely anymore on a set of error values to indirectly
communicate from the update callback to the main function what action is
required on the htlc.
As the logic around invoice mutations gets more complex, the friction
caused by having this logic split between invoice registry and channeldb
becomes more apparent. This commit brings a clearer separation of
concerns by centralizing the accept/settle logic in the invoice
registry.
The original AcceptOrSettle method is renamed to UpdateInvoice because
the update to perform is controlled by the callback.
This commit adds a set of htlcs to the Invoice struct and
serializes/deserializes this set to/from disk. It is a preparation for
accurate invoice accounting across restarts of lnd.
A migration is added for the invoice htlcs.
In addition to these changes, separate final cltv delta and expiry
invoice fields are created and populated. Previously it was required
to decode this from the stored payment request. The reason to create
a combined commit is to prevent multiple migrations.
Currently the invoice registry cannot tell apart the htlcs that pay to
an invoice. Because htlcs may also be replayed on startup, it isn't
possible to determine the total amount paid to an invoice.
This commit is a first step towards fixing that. It reports the circuit
keys of htlcs to the invoice registry, which forms the basis for
accurate invoice accounting.
Previously a check was made for accepted and settled invoices against
the paid amount. This opens up a probe vector where an attacker can pay
to an invoice with an amt that is higher than the invoice amount and
find out if the invoice is already paid or not.
The test assumed that transactions would be broadcast and confirmed at
incorrect heights. Due to timing issues, it was possible for the test to
still succeed, resulting in a flake.
The test assumes that Bob will sweep a pending outgoing HTLC and commit
output back to their wallet. This commit ensures that these operations
are done when expected, i.e.:
1. Bob force closes the channel due to the HTLC timing out.
2. Once the channel is confirmed, Bob broadcasts their HTLC timeout
transaction.
3. Bob broadcasts their commit output sweep transaction once its CSV
expires.
4. Bob broadcasts their second layer sweep transaction for the timed out
HTLC once its CSV expires.
In this commit, we lower the mempool spend check timeout to be twice as
long as the trickle interval of the miner node, which will greatly
improve the execution time of this specific test. We're able to do this
now since we can specify custom trickle intervals for our test
harnesses.
Alice and Dave don't need to be connected in order to receive the node
announcement as we assume that she can receive it from Bob because they
are connected at the beginning of every test.
