Surface outbound payment events via next_event()#34
Conversation
Previously, PaymentSuccessful events from LDK were silently ACKed and discarded by next_event()'s catch-all arm. This meant callers using the event-polling model (as opposed to the blocking wait_for_payment_outcome path) could never learn when an outbound payment completed or obtain its preimage. This is a prerequisite for making agent-wallet payments non-blocking: instead of calling payWhileRunning with a 30-second timeout that blocks the Node.js thread and risks losing the preimage on timeout, the agent-wallet will fire-and-forget the payment and pick up the result through the event loop. Add a Sent variant (discriminant 3) to PaymentEventType and map Event::PaymentSuccessful to it in next_event(), carrying the payment_id and preimage. Also extend PaymentEvent with payment_id and preimage fields, and expose the previously-discarded payment_id on PaymentFailed events so callers can correlate failures to specific outbound payments. All new fields are Option and set to None on Claimable/Received events, so this is backwards-compatible for existing consumers that only match on the first three variants.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75338a275f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Claimable, | ||
| Received, | ||
| Failed, | ||
| Sent, |
There was a problem hiding this comment.
This change extends the N-API surface (PaymentEvent now has payment_id/preimage, and PaymentEventType now includes Sent), but the checked-in generated typings still expose only the old shape (index.d.ts lines 50-61 still lack those fields and enum member). Because publish uses the repository’s index.d.ts, TypeScript consumers cannot type-check handlers for outbound success events and will see an API/type mismatch at compile time; please regenerate and commit the updated bindings with this Rust API change.
Useful? React with 👍 / 👎.
The previous commit added Sent to PaymentEventType and payment_id/preimage to PaymentEvent in Rust, but the checked-in index.d.ts was not regenerated. TypeScript consumers would see a stale API surface and could not type-check handlers for outbound payment events.
1 participant
Previously, PaymentSuccessful events from LDK were silently ACKed and discarded by next_event()'s catch-all arm. This meant callers using the event-polling model (as opposed to the blocking wait_for_payment_outcome path) could never learn when an outbound payment completed or obtain its preimage.
This is a prerequisite for making agent-wallet payments non-blocking: instead of calling payWhileRunning with a 30-second timeout that blocks the Node.js thread and risks losing the preimage on timeout, the agent-wallet will fire-and-forget the payment and pick up the result through the event loop.
Add a Sent variant (discriminant 3) to PaymentEventType and map Event::PaymentSuccessful to it in next_event(), carrying the payment_id and preimage. Also extend PaymentEvent with payment_id and preimage fields, and expose the previously-discarded payment_id on PaymentFailed events so callers can correlate failures to specific outbound payments.
All new fields are Option and set to None on Claimable/Received events, so this is backwards-compatible for existing consumers that only match on the first three variants.