Skip to content

Add reason to Event::PaymentFailed and Event::ChannelClosed - #260

Merged
tnull merged 5 commits into
lightningdevkit:mainfrom
tnull:2024-02-add-payment-failed-reason
Feb 23, 2024
Merged

Add reason to Event::PaymentFailed and Event::ChannelClosed#260
tnull merged 5 commits into
lightningdevkit:mainfrom
tnull:2024-02-add-payment-failed-reason

Conversation

@tnull

@tnulltnull commented Feb 22, 2024

Copy link
Copy Markdown
Collaborator

Fixes#259.
Fixes#247.

To provide some more insight why a payment failed, we expose the PaymentFailureReason in our event. We also expose the reason in ChannelClosed.

@tnull
tnullforce-pushed the 2024-02-add-payment-failed-reason branch 2 times, most recently from 573e0dc to 311c500CompareFebruary 22, 2024 10:20
@tnulltnull changed the title Add reason to Event::PaymentFailedAdd reason to Event::PaymentFailed and Event::ChannelClosedFeb 22, 2024
@tnull
tnull requested a review from jkczyzFebruary 22, 2024 12:57
@tnull
tnullforce-pushed the 2024-02-add-payment-failed-reason branch from 3358062 to c4b53d5CompareFebruary 22, 2024 12:58
Comment threadbindings/ldk_node.udl Outdated
Comment threadsrc/event.rs
@tnull
tnullforce-pushed the 2024-02-add-payment-failed-reason branch from c4b53d5 to 1aab36aCompareFebruary 22, 2024 15:34
@tnull

Copy link
Copy Markdown
CollaboratorAuthor

FWIW, flaky Python test should be unrelated.

@tnull
tnullforce-pushed the 2024-02-add-payment-failed-reason branch 2 times, most recently from 6704dad to f697dc7CompareFebruary 22, 2024 15:59

@jkczyzjkczyz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please squash.

@tnull
tnullforce-pushed the 2024-02-add-payment-failed-reason branch from f697dc7 to a062cb2CompareFebruary 22, 2024 17:28
@tnull

Copy link
Copy Markdown
CollaboratorAuthor

LGTM. Please squash.

Squashed the fixup without further changes.

@jkczyzjkczyz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts on including the reason in PaymentStatus?

@tnull
tnull merged commit 299620e into lightningdevkit:mainFeb 23, 2024
@tnull

Copy link
Copy Markdown
CollaboratorAuthor

Any thoughts on including the reason in PaymentStatus?

Yeah, maybe we should, although I kind of like the simplicity of PaymentStatus currently. In any case, I'd be hesitant to touch anything payment info related until we're clear where the 'bigger picture' is going as part the BOLT12 upgrade.

@tnulltnull mentioned this pull request Mar 8, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose PaymentFailureReason in PaymentFailed Channel Closed "message"

2 participants

@tnull@jkczyz