ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.84k stars 901 forks source link

Can't return `incorrect_or_unknown_payment_details` from `htlc_accepted` #4070

Closed fiatjaf closed 4 years ago

fiatjaf commented 4 years ago

Issue and Steps to Reproduce

I'm returning failure code 16399 from htlc_accepted (which corresponds to INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS), but this message is showing up in the logs:

**BROKEN** lightningd: htlc_accepted_hook plugin returned failure_code 16399, turning to WIRE_TEMPORARY_NODE_FAILURE

I must return that error specifically because it is expected by a bunch of softly-malicious wallets that perform pre-payment probes automatically on every payment using unknown hashes and expect that exact failure code in order to continue with the payment proper.

getinfo output

{
   "id": "02c16cca44562b590dd279c942200bdccfd4f990c3a69fad620c10ef2f8228eaff",
   "alias": "@lntxbot",
   "color": "296683",
   "num_peers": 22,
   "num_pending_channels": 1,
   "num_active_channels": 16,
   "num_inactive_channels": 1,
   "address": [
      {
         "type": "ipv6",
         "address": "2a01:4f8:c0c:7b31::1",
         "port": 9735
      }
   ],
   "binding": [
      {
         "type": "ipv6",
         "address": "::",
         "port": 9735
      },
      {
         "type": "ipv4",
         "address": "0.0.0.0",
         "port": 3597
      }
   ],
   "version": "v0.8.0-1168-g95294e2-modded",
   "blockheight": 649312,
   "network": "bitcoin",
   "msatoshi_fees_collected": 31725172,
   "fees_collected_msat": "31725172msat",
   "lightning-dir": "/home/fiatjaf/.lightning/bitcoin"
}
cdecker commented 4 years ago

I'm assuming that you are returning a {"result": "fail", "failure_code": "400F"}, which uses the deprecated failure_code key instead of the newer failure_message. If this is the case then this is working as expected:

lightningd cannot build a value error from just the failcode, since some errors require additional information (in the case of PERM|15 the payload consists of a htlc_msat and a height which is not specified by just returning the 2 byte failcode 400F). Since the error message is not specification compliant without the payload we opted to map errors with incomplete payloads to a generic NODE|2, which does not require a payload to be valid:

https://github.com/ElementsProject/lightning/blob/7db8680530c34385898829d16163f388e96a3190/lightningd/peer_htlcs.c#L807-L835

If you want to return PERM|15 = 16399 you have to return {"result": "fail", "failure_code": "400F010203040506070809000102"} which is a fully valid error message with htlc_msat=0102030405060708 and height=09000102.

I added #4084 to replicate this behavior and make sure it's correct.

rustyrussell commented 4 years ago

Note that you should be doing development with 'enable-deprecated-apis=false' so you'd find such depreciation before it bites you...

fiatjaf commented 4 years ago

Thank you very much for clarifying!