(draft) feat: add PaymentFailureReason to PaymentStatus

[reez]

Add reason to PaymentStatus failure #567

Probably some nuance I'm missing in my implementation, and a lot of spots I didn't use reason, but tried to use reason in the location where had the PaymentFailureReason to attach to the payment status.

Tried to build the swift bindings to test this in Monday app but was getting this error at the moment (aws-lc-sys is building Kyber code for iOS but that code isn’t compatible with iOS due to missing platform symbols? Not totally sure though):

Undefined symbols for architecture arm64:
  "___chkstk_darwin", referenced from:
      _aws_lc_0_25_0_pqcrystals_kyber512_ref_indcpa_keypair_derand in libaws_lc_sys-c89d8e1fcc00660b.rlib[102](kyber512r3_ref.c.o)
      ... (other Kyber-related symbols)
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

[ldk-reviews-bot]

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

Probably some nuance I'm missing in my implementation, and a lot of spots I didn't use reason, but tried to use reason in the location where had the PaymentFailureReason to attach to the payment status.

Hmm, so at least in this PR we won't have a reason to set, as we mark payments as failed. I honestly also like the simplicity of the PaymentStatus enum previously, but if we'd want to go this way, we should probably introduce our own reason object that is a superset of LDK's PaymentFailureReason and have it cover all cases to always describe why we failed the payment.

[tnull]

Tried to build the swift bindings to test this in Monday app but was getting this error at the moment (aws-lc-sys is building Kyber code for iOS but that code isn’t compatible with iOS due to missing platform symbols? Not totally sure though):

Yes, currently Swift builds are broken on main due to aws-lc-sys not building. Post LDK 0.2 we'll be able to use lightningdevkit/rust-lightning#3587 to switch on the ring feature for electrum-client for Swift builds, until then we use a patched backport of that PR for the Swift binary builds as a woraround. All other builds work fine with the otherwise-preferable aws-lc-rs.

[reez]

Yes, currently Swift builds are broken on main due to aws-lc-sys not building. Post LDK 0.2 we'll be able to use lightningdevkit/rust-lightning#3587 to switch on the ring feature for electrum-client for Swift builds, until then we use a patched backport of that PR for the Swift binary builds as a woraround. All other builds work fine with the otherwise-preferable aws-lc-rs.

Got it, thanks for this info about the patched backport

[reez]

Probably some nuance I'm missing in my implementation, and a lot of spots I didn't use reason, but tried to use reason in the location where had the PaymentFailureReason to attach to the payment status.

Hmm, so at least in this PR we won't have a reason to set, as we mark payments as failed. I honestly also like the simplicity of the PaymentStatus enum previously, but if we'd want to go this way, we should probably introduce our own reason object that is a superset of LDK's PaymentFailureReason and have it cover all cases to always describe why we failed the payment.

Ah yes, I didn't fully flesh it out once I got stuck at the swift bindings part, this makes sense.

I like the simplicity of PaymentStatus enum too, but also was running into spots where I wish I had some associated data/reason for when the case was a failure.

[reez]

Closing the PR part of this (keeping Issue open), thanks for the comments!