Specifically, there really should be no Errs, but in case there is
some case where duplicate HTLC removes are possible, return
IgnoreError and debug_assert to see if fuzzing can find them.
This is completely unsafe as we have provided the remote side with
a commitment_signed which they can broadcast, including the HTLC
transaction, and then could claim it on-chain after we've failed it
backwards!
This effecitlvey reverts the refactors in 383bd90a48,
however keeps the actully new test code.
It also writes documentation for the super confusing tx test func
and makes it a bit less permissive.
Instead of hopping a pointer, we're only ever going to return one
Transaction at max, so skip the Vec. Also avoid
re-pubkey-converting the revocation key.
Refactor check_spend_remote_transaction in part to check_spend_remote_htlc to
avoid lock mess in block_connected. We need
remote_commitment_txn_on_chain to match remote HTLC tx
I think this might have been my fault due to faulty review
feedback, but fuzzer caught trivial crash here where you try to
send a ping message larger than 16KB (but smaller than the
max-length 64KB) and you crash as message serialization is unwrap()
This is required by BOLT 2 to ensure that no attacker can simply
relay every public node a duplicate-payment_hash HTLC for each HTLC
it receives to deduce where an HTLC came from.
Note that this makes the claim logic much less incentive-compatible
as we will not claim all available HTLCs with the same payment_hash
even if we know the preimage! This is OK because, most likely, any
attackers trying to map the network will use small-value payments
and, hopefully, we will move away from constant hashes across an
entire payment at some point in the near future.
This further simplifies the payment transition state a bit, so
hopefully at least we got some readability out of all of this
Both get_update_fail_htlc and get_update_fulfill_htlc should never
be called before any HTLC could have been accepted (ie
pre-ChannelFunded) nor should they ever be called
post-ShutdownComplete as the Channel object should be destroyed at
that point. Previously get_update_fulfill_htlc would panic, but
get_update_fail_htlc would return an Err. For now make them both
panic but we can revisit this if we want to have fewer panics in
the future.
