A proposed solution to fix json null value for bolt11 key when is used the pay plugin

[vincenzopalazzo]

This PR proposed a solution to fix the problem mentioned at the following PR #3876

I can have some idea bug in my mind, so if this PR is wrong, I'm so sorry.

The json below is an example of use cases when I used the command keysend and pay (json_paymod).

{
   "pays": [
      {
         "bolt11": "lnbcrt100n1p03chsnpp5n72j2e0ykwz7ue37e60qn623z9elq4fguv52ny7jjqmv990zajeqdql2pshjmt9de6zqamfw35zqurp09kk2eqxqyjw5qcqp2sp5h0renl745kd7c86z2g49lkuhua3j66an80szd58qdpeyzhdkrl3srzjqfnup6dhq3xls5v5pw0ksgrx0gcerqpr2pd6y6ckhstql6qfncta6qq24qqqqqgqqqqqqqqpqqqqqzsqqc9qy9qsqfy8e5x6aya75n3ayd5e8rsvkmepgq9gwf8v9ram560y5vlrapk9pwjxcfewfpvvnw97v49xzlce6zqmc5znv0ke6e3un5lm4hk8uewqq5dug0s",
         "status": "complete",
         "preimage": "c0c09fb65814b71f56c0a0faa9bf223b1d9667df90b55648a7c023e200c66b70",
         "amount_sent_msat": "10000msat"
      },
      {
         "bolt11": "(null)",
         "status": "failed",
         "label": "Pay with keysend",
         "amount_sent_msat": "0msat"
      },
      {
         "bolt11": "lnbcrt50n1p0s5zvvpp5ylm3y040va7wu08tjw7kegzq0ku2zqav99u6tzfdd7mh6w8elecsdpsg3jhvgz5v4ehggrwv4mjqunsvvsxxmmdd4skuepqw3jhxapnxqyjw5qcqp2sp5jxrwha4g0rjdxf3cs8gc9rqs5zunu3670v5mpppw0zajw7f4yrsq9qy9qsqf043ey32l952hq6rry34r9vugh2kdydr7hnuujpfadt5gxykguf5lsj8rczr7c3kpc6e0ldjw3r3nwehetwfd2f4djwtnjztjwzrnjqqaj8fg2",
         "status": "complete",
         "preimage": "391f288bc4425328742aafe26a01d02a07ce8b7420003aacd3ef4b2741284523",
         "amount_sent_msat": "5000msat"
      },
      {
         "bolt11": "(null)",
         "status": "complete",
         "label": "Pay with keysend",
         "preimage": "459906aba846e9f3ba6b0e67313bf2210c09e06319fe0bc991428310d1bb2eca",
         "amount_sent_msat": "10000msat"
      }
   ]
}

[vincenzopalazzo]

Mh, travis CI discovered a fault inside my idea, one of the error is reported here https://travis-ci.org/github/ElementsProject/lightning/jobs/711777073#L7957

Mh, I'm losing sometings

[rustyrussell]

This is perfect, I was just writing the exact same thing!

Your only mistake was that new arguments need to go at the end, and the documentation needs updating.

I've split combined our work and split your patches into three...

[ZmnSCPxj]

Would have ACK, but we need better docs.

[ZmnSCPxj]

Also, check-source: https://travis-ci.org/github/ElementsProject/lightning/jobs/711844157#L879-L880

tests/test_pay.py:3219:1: W391 blank line at end of file
tests/test_pay.py:3219:1: W293 blank line contains whitespace

[ZmnSCPxj]

Fixed the CI error, also added necessary documentation by copying from sendpay docs.

[vincenzopalazzo]

Hi all, thanks for fixing my mistakes and sorry if I lost to update the documentation, the next time I will pay more attention to it.

In addition, I'm looking inside the travis log and I still see these errors inside the test unit. Any idea about the problem that caused it?

[cdecker]

Are you referring to the valgrind errors here:

Valgrind error file: valgrind-errors.8822
==8822== Conditional jump or move depends on uninitialised value(s)
==8822==    at 0x11B3CC: payment_createonion_success (libplugin-pay.c:1043)
==8822==    by 0x114A0C: handle_rpc_reply (libplugin.c:551)
==8822==    by 0x11507C: rpc_read_response_one (libplugin.c:666)
==8822==    by 0x11516D: rpc_conn_read_response (libplugin.c:685)
==8822==    by 0x146407: next_plan (io.c:59)
==8822==    by 0x146F84: do_plan (io.c:407)
==8822==    by 0x146FC2: io_ready (io.c:417)
==8822==    by 0x149188: io_loop (poll.c:445)
==8822==    by 0x11732F: plugin_main (libplugin.c:1303)
==8822==    by 0x1135BD: main (pay.c:2010)
==8822==

This means that on line libplugin-pay.c:1043 there was an access to a variable that wasn't initialized. This happens because unless we set the bolt11 in the plugin that calls the libplugin library that field is never written to, which is an issue if we then rely on it being initialized correctly.

I added a small fixup that sets it to NULL when the instance is created, This means the caller can write to it before starting the payment, and we can use p->bolt11 to differentiate between having an invoice string or not.

[vincenzopalazzo]

Ops! Thanks for fix my mistake @cdecker the next time I will try to use more attention before to open a PR, sorry

[cdecker]

Ops! Thanks for fix my mistake @cdecker the next time I will try to use more attention before to open a PR, sorry

No problem at all, these things need time to get a hang of, so I thought I'd give you pointers on how to identify them in the future :-)

[cdecker]

Rebased, and will merge as soon as CI gives the all-clear.

ACK 8163cea