Skip to content

Fix (or doc, for 0.2) missing Event::PaymentSent::bolt12_invoice on payments loaded from monitors #3714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TheBlueMatt opened this issue Apr 8, 2025 · 1 comment · Fixed by #3719
Milestone

Comments

@TheBlueMatt
Copy link
Collaborator

See #3593 (comment)

@TheBlueMatt TheBlueMatt added this to the 0.2 milestone Apr 8, 2025
@vincenzopalazzo
Copy link
Contributor

Good catch, going to look into this and pause the bolt12 contacts implementation for a bit, thanks!

vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 8, 2025
Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 8, 2025
Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 22, 2025
Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 22, 2025
Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 22, 2025
Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 26, 2025
Matt noted during the last round of review the following:

>Oof. Sorry I missed this until now. This is not, in fact, "only used for retries", we use it on claims only, in fact. If a user is relying on the event field for PoP, what this can mean is that we can initiate a send, restart with a stale ChannelManager, notice the payment is pending, then when it claims fail to provide the invoice (only the preimage) to the payer.
>In practice, to fix this, we'll need to include the PaidBolt12Invoice in the HTLCSource::OutboundRoute, I believe.

This commit is trying to store the PaidBolt12Invoice inside the HTLCSource::OutboundRoute, but
this is not enough because we have to store the invoice also inside the PendingOutboundPayment.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 29, 2025
It moves the PaidBolt12Invoice (BOLT 12 invoice)
into HTLCSource::OutboundRoute to ensure the invoice
is available for proof-of-payment and event emission,
as discussed in issue lightningdevkit#3714. The commit also updates
hashing implementations and derives to ensure correct
behavior when the invoice is present, and propagates
the invoice through relevant payment and event structures.

This fixes a potential issue where the invoice could
be lost on restart, affecting PoP reliability.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 30, 2025
It moves the PaidBolt12Invoice (BOLT 12 invoice)
into HTLCSource::OutboundRoute to ensure the invoice
is available for proof-of-payment and event emission,
as discussed in issue lightningdevkit#3714. The commit also updates
hashing implementations and derives to ensure correct
behavior when the invoice is present, and propagates
the invoice through relevant payment and event structures.

This fixes a potential issue where the invoice could
be lost on restart, affecting PoP reliability.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 30, 2025
It moves the PaidBolt12Invoice (BOLT 12 invoice)
into HTLCSource::OutboundRoute to ensure the invoice
is available for proof-of-payment and event emission,
as discussed in issue lightningdevkit#3714. The commit also updates
hashing implementations and derives to ensure correct
behavior when the invoice is present, and propagates
the invoice through relevant payment and event structures.

This fixes a potential issue where the invoice could
be lost on restart, affecting PoP reliability.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo added a commit to vincenzopalazzo/rust-lightning that referenced this issue Apr 30, 2025
It moves the PaidBolt12Invoice (BOLT 12 invoice)
into HTLCSource::OutboundRoute to ensure the invoice
is available for proof-of-payment and event emission,
as discussed in issue lightningdevkit#3714. The commit also updates
hashing implementations and derives to ensure correct
behavior when the invoice is present, and propagates
the invoice through relevant payment and event structures.

This fixes a potential issue where the invoice could
be lost on restart, affecting PoP reliability.

Link: lightningdevkit#3714
Signed-off-by: Vincenzo Palazzo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants