delpay: delete the payment by status from the db

[vincenzopalazzo]

There are cases (difficult to reproduce with a test) where
a payment will fail one time and succeed later.

As far I understand in this case the groupid field of the payment
is the same, and the only thing that change is the status, so
our logic inside the delpay is ambiguous where it is not
possible to delete a payment as described in #6114

A sequence of commands that explain the problem is

$ lc -k listpays payment_hash=H
{
   "pays": [
      {
         "bolt11": "I",
         "destination": "redacted",
         "payment_hash": "H",
         "status": "complete",
         "created_at": redacted,
         "completed_at": redacted,
         "preimage": "P",
         "amount_msat": "redacted",
         "amount_sent_msat": "redacted"
      }
   ]
}
$ lc delpay H complete
{
   "code": 211,
   "message": "Payment with hash H has failed status but it should be complete"
}

In this case, the delpay is not able to delete a payment because the
listpays is returning only the succeeded one, so by running the
listsendpays we may see the following result where our delpay logic
will be stuck because it works to ensure that all the payments stored
in the database has the status specified by the user

➜  VincentSSD clightning --testnet listsendpays -k payment_hash=7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4
{
   "payments": [
      {
         "id": 322,
         "payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
         "groupid": 1,
         "partid": 1,
         "destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
         "amount_msat": 300,
         "amount_sent_msat": 1664,
         "created_at": 1679510203,
         "completed_at": 1679510205,
         "status": "failed",
         "bolt11": "lntb1pjpkj4xsp52trda39rfpe7qtqahx8jjplhnj3tatxy8rh6sc6afgvmdz7n0llspp50lr5hmdm0re0xvcp2hv3nf2wwvx0r8q3h3e7jmqz0awdfg6w206qdp0w3jhxarfdenjqargv5sxgetvwpshjgrzw4njqun9wphhyaqxqyjw5qcqp2rzjqtp28uqy77te96ylt7ek703h4ayldljsf8rnlztgf3p8mg7pd0qzwf8a3yqqpdqqqyqqqqt2qqqqqqgqqc9qxpqysgqgeya2lguaj6sflc4hx2d89jvah8mw9uax4j77d8rzkut3rkm0554x37fc7gy92ws9l76yprdva2lalrs7fqjp9lcx40zuty8gca0g5spme3dup"
      },
      {
         "id": 323,
         "payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
         "groupid": 1,
         "partid": 2,
         "destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
         "amount_msat": 300,
         "amount_sent_msat": 3663,
         "created_at": 1679510205,
         "completed_at": 1679510207,
         "status": "failed"
      },
      {
         "id": 324,
         "payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
         "groupid": 1,
         "partid": 3,
         "destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
         "amount_msat": 300,
         "amount_sent_msat": 3663,
         "created_at": 1679510207,
         "completed_at": 1679510209,
         "status": "failed"
      },
      {
         "id": 325,
         "payment_hash": "7fc74bedbb78f2f3330155d919a54e730cf19c11bc73e96c027f5cd4a34e53f4",
         "groupid": 1,
         "partid": 4,
         "destination": "030b686a163aa2bba03cebb8bab7778fac251536498141df0a436d688352d426f6",
         "amount_msat": 300,
         "amount_sent_msat": 4663,
         "created_at": 1679510209,
         "completed_at": 1679510221,
         "status": "complete",
         "payment_preimage": "43f746f2d28d4902489cbde9b3b8f3d04db5db7e973f8a55b7229ce774bf33a7"
      }
   ]
}

This commit solves the problem by forcing the delete query in the
database to specify status too, and work around this kind of
ambiguous case.

Fixes #6114
Introduced in vincenzopalazzo@f52ff07 (lightningd: allow delpay to delete a specific payment.)

[rustyrussell]

Ha! I forgot to push my version of this, since it was feature freeze! (Same branch where I found the issue where delpay got groupid/id around the wrong way).

Please compare, and combine as necessary?

https://github.com/rustyrussell/lightning/tree/guilt/autoclean-delpay-fix

[vincenzopalazzo]

Nice @rustyrussell that at least the solution is the same, but I like your message report more than mine so I cherry-pick the commit and adjust it on top of mine.

The only change that I check for is a good status and not the bad one this can save us some iteration in a lucky case!

Ah, I have stolen the test of course!

[rustyrussell]

Ack 4ea5515

[rustyrussell]

Ack abe46de