Skip to content
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

lnpeer: async maybe_send_commitment #7836

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SomberNight
Copy link
Member

this builds on top of #7835 (first commit is from there)

This is re cc1b4a5#commitcomment-74922012

  • Await pong before sending commitment_signed (per BOLT-2)

We await self.ping_if_required() inside maybe_send_commitment().
This needed making maybe_send_commitment async, which really quite snowballed o.O

Anyway, this is a separate PR from #7835, as that PR is useful in itself.

Note: `process_message` is only called from `_message_loop`.
There are(would be) basically three types of message handlers:
1. "traditional blocking msg handlers". non-async ones. When these handlers are called, `process_message` naturally blocks until the handler returns, which means `_message_loop` also blocks until the message is fully processed before starting the next iteration.
2. "async blocking msg handlers". async ones where we want the previous property, i.e. we want the `_message_loop` to wait until the handler finishes. We await the handler inside `process_message`, and `_message_loop` awaits `process_message`.
3. "async non-blocking msg handlers". async message handlers that can be spawned e.g. onto `Peer.taskgroup` and the loop is free to start processing subsequent messages. e.g. msg handlers that start a negotiation, such as `on_shutdown` and `on_open_channel`.

Any non-async message handler (`def on_...`) automatically goes into category 1.
An async message handler, by default, goes into category 2, "blocking";
to go into category 3 ("non-blocking"), we use the `runs_in_taskgroup` function decorator.
@SomberNight SomberNight marked this pull request as draft May 30, 2022 16:19
@SomberNight
Copy link
Member Author

I've changed this to draft as it needs some discussion before a potential merge.

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.

None yet

1 participant