Skip to content

expired inbound payments stuck as Pending: test and fix#99

Open
bitwalt wants to merge 2 commits intoRGB-Tools:masterfrom
bitwalt:inbound-expired-payments-test
Open

expired inbound payments stuck as Pending: test and fix#99
bitwalt wants to merge 2 commits intoRGB-Tools:masterfrom
bitwalt:inbound-expired-payments-test

Conversation

@bitwalt
Copy link
Contributor

@bitwalt bitwalt commented Feb 25, 2026

this PR add a test to reproduce #98

Copy link
Member

@nicbus nicbus left a comment

Choose a reason for hiding this comment

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

Please look for a test where this can be integrated, in order to avoid the cost of setting up a new one.

Also, please add the fix (as you proposed in #98 (comment)) on top of this commit.

@bitwalt bitwalt force-pushed the inbound-expired-payments-test branch from 776d09c to d00bea4 Compare February 25, 2026 17:26
Copy link
Member

@nicbus nicbus left a comment

Choose a reason for hiding this comment

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

a few more changes

please also update the openapi spec

src/ldk.rs Outdated
}
}

pub(crate) fn fail_inbound_pending_payments(&self) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub(crate) fn fail_inbound_pending_payments(&self) {
pub(crate) fn list_updated_payments(&self) {

please return the list of updated payments so the calling methods don't need to get them a second time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can i call it list_updated_inbound_payments instead of list_updated_payments ?

Copy link
Member

Choose a reason for hiding this comment

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

yes, that was the intention, I mistyped

@bitwalt bitwalt force-pushed the inbound-expired-payments-test branch from d00bea4 to 97fa6d4 Compare February 26, 2026 11:15
@bitwalt
Copy link
Contributor Author

bitwalt commented Feb 26, 2026

suggestions should have been addressed, changed the openapi in the other PR, do we need to change something also here?

@zoedberg zoedberg changed the title test: add regression test for expired inbound payments stuck as Pending expired inbound payments stuck as Pending: test and fix Feb 26, 2026
Copy link
Member

@nicbus nicbus left a comment

Choose a reason for hiding this comment

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

thanks, a few more details to address

please also rebase on top of the updated master

// wait for the invoices to expire
tokio::time::sleep(std::time::Duration::from_secs(SHORT_EXPIRY_SEC as u64 + 1)).await;

let payment = get_payment(node2_addr, &decoded1.payment_hash).await;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let payment = get_payment(node2_addr, &decoded1.payment_hash).await;
// getting a payment should trigger expiration-based status transition
let payment = get_payment(node2_addr, &decoded1.payment_hash).await;

src/ldk.rs Outdated
Comment on lines 307 to 310
self.get_inbound_payments().payments.clone()
} else {
inbound.payments.clone()
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.get_inbound_payments().payments.clone()
} else {
inbound.payments.clone()
}
}
inbound.payments.clone()

src/ldk.rs Outdated
}
}

pub(crate) fn fail_inbound_pending_payments(&self) {
Copy link
Member

Choose a reason for hiding this comment

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

yes, that was the intention, I mistyped

@nicbus
Copy link
Member

nicbus commented Feb 26, 2026

changed the openapi in the other PR, do we need to change something also here?

yes, since this PR adds expires_at in PaymentInfo

@bitwalt bitwalt force-pushed the inbound-expired-payments-test branch from 97fa6d4 to 3aa646d Compare February 27, 2026 11:02
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 this pull request may close these issues.

2 participants