feat: add accessor methods for common PaymentKind fields

[jgmcalpine]

Description

Addresses issue #730.

Added payment_hash() as requested, but also identified that preimage and secret are common fields shared across most PaymentKind variants. Added accessor methods for all three to unify the API.

Changes

• Added payment_hash(), preimage(), and secret() to PaymentKind.
• Refactored PaymentDetailsUpdate::from to use these new methods, removing boilerplate matching logic.
• Added unit test payment_kind_accessor_methods to verify behavior across all variants (Bolt11, Bolt12, Spontaneous, Onchain).

PR Tasks

• Added tests
• Ran cargo fmt
• Ran cargo test

[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.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[ldk-reviews-bot]

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[ldk-reviews-bot]

🔔 3rd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[tnull]

Bit on the fence regarding adding Lightning-specific accessor APIs for PaymentKind which is explicitly an umbrella type also covering other payment methods that don't feature payment hashes/secrets/pre-images.

However, if we want to add these convenience methods, they should at the very least also be also exposed in bindings, i.e., via the UDL file.

[jgmcalpine]

Bit on the fence regarding adding Lightning-specific accessor APIs for PaymentKind which is explicitly an umbrella type also covering other payment methods that don't feature payment hashes/secrets/pre-images.

However, if we want to add these convenience methods, they should at the very least also be also exposed in bindings, i.e., via the UDL file.

I understand your point regarding the API design. It is definitely a trade-off to add specific helpers to a general enum.

That said, this change reduces boilerplate code for the most common use cases, which was the motivation behind the original issue.

I agree that if we add these, they must be available in the bindings to be useful. I will update the UDL file to include them and push a new commit shortly.

[jgmcalpine]

Bit on the fence regarding adding Lightning-specific accessor APIs for PaymentKind which is explicitly an umbrella type also covering other payment methods that don't feature payment hashes/secrets/pre-images.
However, if we want to add these convenience methods, they should at the very least also be also exposed in bindings, i.e., via the UDL file.

I understand your point regarding the API design. It is definitely a trade-off to add specific helpers to a general enum.

That said, this change reduces boilerplate code for the most common use cases, which was the motivation behind the original issue.

I agree that if we add these, they must be available in the bindings to be useful. I will update the UDL file to include them and push a new commit shortly.

Bindings updated.

I implemented the accessors as top-level helpers in the UDL, but I gated the Rust logic behind #[cfg(feature = "uniffi")] and #[doc(hidden)] to make sure they are only compiled for the bindings and don't clutter the standard Rust API or docs.

Ready for a look!

[tnull]

I implemented the accessors as top-level helpers in the UDL, but I gated the Rust logic behind #[cfg(feature = "uniffi")] and #[doc(hidden)] to make sure they are only compiled for the bindings and don't clutter the standard Rust API or docs.

Ah, right, we can't even expose the actual methods given that it's an enum interface. Well, tbh, I don't find exposing this at the top-level acceptable, and also dislike if we'd resorted to just do it for the Rust API.

Given that I wasn't convinced we should do this from an API-design perspective in the to begin with, I now more and more think it's not worth the interface complication/incongruence across languages.

I now closed #730. Thank you for looking into it though!

[jgmcalpine]

I implemented the accessors as top-level helpers in the UDL, but I gated the Rust logic behind #[cfg(feature = "uniffi")] and #[doc(hidden)] to make sure they are only compiled for the bindings and don't clutter the standard Rust API or docs.

Ah, right, we can't even expose the actual methods given that it's an enum interface. Well, tbh, I don't find exposing this at the top-level acceptable, and also dislike if we'd resorted to just do it for the Rust API.

Given that I wasn't convinced we should do this from an API-design perspective in the to begin with, I now more and more think it's not worth the interface complication/incongruence across languages.

I now closed #730. Thank you for looking into it though!

Understood. The UniFFI limitations on Enums definitely make the trade-off steep for the bindings. Thanks for taking a look.

[tnull]

Understood. The UniFFI limitations on Enums definitely make the trade-off steep for the bindings. Thanks for taking a look.

Thanks again, closing this for now!