Handle events asynchronously in the BackgroundProcessor's async variant #1787

wpaulino · 2022-10-20T23:17:33Z

Fixes #1674.

TheBlueMatt · 2022-10-24T04:23:43Z

Looks like doc examples now fail to build.

lightning/src/util/events.rs

TheBlueMatt · 2022-10-24T19:41:40Z

Slipping to 0.0.113.

lightning-invoice/src/payment.rs

lightning-background-processor/src/lib.rs

lightning/src/ln/channelmanager.rs

lightning-background-processor/src/lib.rs

lightning/src/routing/gossip.rs

lightning/src/util/events.rs

lightning-invoice/src/payment.rs

TheBlueMatt

Basically LGTM.

lightning-background-processor/src/lib.rs

lightning-invoice/src/payment.rs

lightning/src/util/events.rs

TheBlueMatt · 2022-11-03T16:33:38Z

Needs (hopefully trivial) rebase after #1743

lightning-invoice/src/payment.rs

wpaulino · 2022-11-04T18:20:43Z

Had to rebase on latest to address a small conflict.

codecov-commenter · 2022-11-08T01:27:28Z

Codecov Report

Base: 90.82% // Head: 90.82% // No change to project coverage 👍

Coverage data is based on head (15b79f8) compared to base (15b79f8).
Patch has no changes to coverable lines.

❗ Current head 15b79f8 differs from pull request most recent head d3f1e25. Consider uploading reports for the commit d3f1e25 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1787   +/-   ##
=======================================
  Coverage   90.82%   90.82%           
=======================================
  Files          89       89           
  Lines       48011    48011           
  Branches    48011    48011           
=======================================
  Hits        43604    43604           
  Misses       4407     4407

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

wpaulino · 2022-11-08T01:27:36Z

Ended up going with the struct wrappers approach since all of the alternatives we discussed offline didn't work out.

lightning-background-processor/src/lib.rs

lightning/src/chain/chainmonitor.rs

lightning/src/ln/channelmanager.rs

lightning/src/routing/gossip.rs

lightning-invoice/src/payment.rs

lightning/src/util/events.rs

We introduce a new sealed trait BaseEventHandler that has a blanket implementation for any T. Since the trait cannot be implemented outside of the crate, this allow us to expose specific implementations of InvoicePayer that allow for synchronous and asynchronous event handling.

wpaulino · 2022-11-10T19:02:14Z

Linked to the async implementations and rebased on latest to address a small conflict.

TheBlueMatt · 2022-11-10T20:13:09Z

CI fail is #1847, should pass after a re-run.

tnull self-requested a review October 21, 2022 17:37

wpaulino force-pushed the async-event-handler branch 2 times, most recently from 9579b76 to 91e53c1 Compare October 21, 2022 21:50

TheBlueMatt reviewed Oct 24, 2022

View reviewed changes

lightning/src/util/events.rs Outdated Show resolved Hide resolved

TheBlueMatt added this to the 0.0.113 milestone Oct 24, 2022

wpaulino force-pushed the async-event-handler branch from 91e53c1 to d28ee40 Compare October 31, 2022 17:48

wpaulino commented Oct 31, 2022

View reviewed changes

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

TheBlueMatt reviewed Oct 31, 2022

View reviewed changes

lightning-background-processor/src/lib.rs Outdated Show resolved Hide resolved

TheBlueMatt reviewed Oct 31, 2022

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning-background-processor/src/lib.rs Outdated Show resolved Hide resolved

wpaulino force-pushed the async-event-handler branch from d28ee40 to d110099 Compare October 31, 2022 23:21

jkczyz self-requested a review November 1, 2022 21:33

TheBlueMatt reviewed Nov 1, 2022

View reviewed changes

lightning/src/routing/gossip.rs Outdated Show resolved Hide resolved

TheBlueMatt reviewed Nov 1, 2022

View reviewed changes

lightning/src/util/events.rs Outdated Show resolved Hide resolved

jkczyz reviewed Nov 1, 2022

View reviewed changes

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

wpaulino force-pushed the async-event-handler branch 2 times, most recently from 448300f to e752325 Compare November 2, 2022 21:53

TheBlueMatt reviewed Nov 2, 2022

View reviewed changes

lightning-background-processor/src/lib.rs Outdated Show resolved Hide resolved

lightning-background-processor/src/lib.rs Outdated Show resolved Hide resolved

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

TheBlueMatt assigned TheBlueMatt and jkczyz Nov 2, 2022

tnull reviewed Nov 3, 2022

View reviewed changes

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

lightning/src/util/events.rs Outdated Show resolved Hide resolved

wpaulino force-pushed the async-event-handler branch from 8790e45 to 8d837f0 Compare November 3, 2022 20:15

jkczyz reviewed Nov 3, 2022

View reviewed changes

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

wpaulino force-pushed the async-event-handler branch from 8d837f0 to d046480 Compare November 4, 2022 18:18

wpaulino force-pushed the async-event-handler branch from d046480 to c5c196b Compare November 8, 2022 01:26

wpaulino force-pushed the async-event-handler branch from c5c196b to d8df37d Compare November 8, 2022 02:12

jkczyz reviewed Nov 8, 2022

View reviewed changes

wpaulino force-pushed the async-event-handler branch 2 times, most recently from 55b0bc1 to 23cca38 Compare November 8, 2022 19:28

TheBlueMatt previously approved these changes Nov 9, 2022

View reviewed changes

jkczyz reviewed Nov 9, 2022

View reviewed changes

lightning/src/util/events.rs Outdated Show resolved Hide resolved

wpaulino added 7 commits November 10, 2022 10:56

Apply network graph updates through NetworkUpdate's instead of Event's

b0d0f3d

Consume events by value in EventHandler's handle_event

05cb467

Implement async versions of process_pending_events

55b714c

Handle events asynchronously in the BackgroundProcessor's async variant

8d20ebc

Drop DecoratingEventHandler in favor of inline closure

9787b28

Note async versions of event handling within EventsProvider

d3f1e25

wpaulino dismissed TheBlueMatt’s stale review via d3f1e25 November 10, 2022 19:01

wpaulino force-pushed the async-event-handler branch from 23cca38 to d3f1e25 Compare November 10, 2022 19:01

TheBlueMatt approved these changes Nov 10, 2022

View reviewed changes

jkczyz approved these changes Nov 10, 2022

View reviewed changes

TheBlueMatt merged commit 2a3c99c into lightningdevkit:main Nov 10, 2022

wpaulino deleted the async-event-handler branch November 10, 2022 21:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle events asynchronously in the BackgroundProcessor's async variant #1787

Handle events asynchronously in the BackgroundProcessor's async variant #1787

wpaulino commented Oct 20, 2022 •

edited

Loading

TheBlueMatt commented Oct 24, 2022 •

edited

Loading

TheBlueMatt commented Oct 24, 2022

TheBlueMatt left a comment

TheBlueMatt commented Nov 3, 2022

wpaulino commented Nov 4, 2022

codecov-commenter commented Nov 8, 2022 •

edited

Loading

wpaulino commented Nov 8, 2022

wpaulino commented Nov 10, 2022

TheBlueMatt commented Nov 10, 2022

Handle events asynchronously in the BackgroundProcessor's async variant #1787

Handle events asynchronously in the BackgroundProcessor's async variant #1787

Conversation

wpaulino commented Oct 20, 2022 • edited Loading

TheBlueMatt commented Oct 24, 2022 • edited Loading

TheBlueMatt commented Oct 24, 2022

TheBlueMatt left a comment

Choose a reason for hiding this comment

TheBlueMatt commented Nov 3, 2022

wpaulino commented Nov 4, 2022

codecov-commenter commented Nov 8, 2022 • edited Loading

Codecov Report

wpaulino commented Nov 8, 2022

wpaulino commented Nov 10, 2022

TheBlueMatt commented Nov 10, 2022

wpaulino commented Oct 20, 2022 •

edited

Loading

TheBlueMatt commented Oct 24, 2022 •

edited

Loading

codecov-commenter commented Nov 8, 2022 •

edited

Loading