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

Formalize the Ping Protocol #575

Closed
wants to merge 20 commits into from
Closed

Formalize the Ping Protocol #575

wants to merge 20 commits into from

Conversation

sappenin
Copy link
Collaborator

@sappenin sappenin commented May 2, 2020

This PR has been dormant for a very long time, but I'm reintroducing it for a few reasons:

  1. We have a new proposals process, and formalizing "ping" still seems like a good proposal.
  2. This has been on my TODO list forever
  3. Uni-directional mode seems useful in "send-only" modes (see discussion here)
  4. Bi-directional mode is deployed in the JS connector and also seems useful in certain production networks.

Links for Context

  • Interledger forum discussion (here).
  • Previous PR from long ago (here).

sappenin and others added 18 commits February 28, 2019 22:45
Signed-off-by: sappenin <[email protected]>
Signed-off-by: sappenin <[email protected]>
Remove errant article and change spelling.
Signed-off-by: sappenin <[email protected]>
Add images for the unidirectional-flow and bidirectional-flow.
* Reword various paragraphs with new suggested terminology.
* Move mode-specific portions to the appropriate sections.
* Improve Implementation Considerations.
* Add link to OER encoding notes.
* Fix ordered-list numbering

Signed-off-by: sappenin <[email protected]>
* Update ping protocol verbiage
* Update svg flow for bidirectional mode.

Signed-off-by: sappenin <[email protected]>
@sappenin sappenin self-assigned this May 2, 2020
@sappenin sappenin changed the title Formal the Ping Protocol Formalize the Ping Protocol May 2, 2020
Copy link

@matdehaast matdehaast left a comment

Choose a reason for hiding this comment

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

Thanks for this @sappenin . Mostly looks good to me. The only question/comment I have is should the unidirectional flow also have a predefined data payload. The reason I ask is because I am unsure what mechanism the receiver would know that this is specifically a PING packet. Rather than a random packet addressed to it.

@sappenin
Copy link
Collaborator Author

sappenin commented May 5, 2020

I am unsure what mechanism the receiver would know that this is specifically a PING packet

In unidirectional mode, the receiver knows this is a ping packet based upon two things: 1) The packet is destined to the Connector's operating address; 2) The packet uses the well-known condition.

In my mind, there's no data payload needed in unidirectional flow. In bi-directional flow, the data payload is needed to signal receiver where to send the pong packets to.

Copy link

@sharafian sharafian left a comment

Choose a reason for hiding this comment

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

Glad to have this protocol written up!

One suggestion is that the diagram might be more effective as a message sequence, particularly for the bidirectional flow.

<g id="Graphic_8">
<title>Text</title>
<text transform="translate(112.65458 326.66667)" fill="black">
<tspan font-family="Courier" font-size="12" font-weight="400" fill="black" x="0" y="11">(4)Fulfill</tspan>

Choose a reason for hiding this comment

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

I think this should be 6

The recipient sends the packet to the sender (4) and then the sender would fulfill it (5) and then the recipient would use that fulfillment to fulfill the initial packet (6)

Although it might need to be a higher number if it's meant to match the steps in the bidirectional flow section of the doc above

Copy link
Collaborator Author

@sappenin sappenin May 8, 2020

Choose a reason for hiding this comment

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

I updated the images to make them more flow-based (27b710b) - take another look?


This protocol is not designed to debug routing issues and does not provide additional diagnostic information about the state of routing. That use case would be better served by a separate `traceroute` mechanism.

All Interledger implementations _SHOULD_ respond to ping protocol requests, unless a node has a specific reason not to, such as security or privacy concerns.

Choose a reason for hiding this comment

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

can we clarify implementation here? Is a STREAM server expected to respond to pings, or just connectors?

Copy link
Collaborator Author

@sappenin sappenin May 8, 2020

Choose a reason for hiding this comment

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

hmmm...great question. I'm not sure I have a strong opinion. In Java-land, we've moved most of our receiver logic into the Connector runtime, so this deployment style would sort of get ping functionality for free because it's all running in the connector. Though you sort of have to know the connector operator address to make it work.

I suppose after re-reading line 15 above, it's sort of optional for a STREAM receiver to support this functionality -- maybe that's enough?

Signed-off-by: sappenin <[email protected]>
@matdehaast
Copy link

I am unsure what mechanism the receiver would know that this is specifically a PING packet

In unidirectional mode, the receiver knows this is a ping packet based upon two things: 1) The packet is destined to the Connector's operating address; 2) The packet uses the well-known condition.

In my mind, there's no data payload needed in unidirectional flow. In bi-directional flow, the data payload is needed to signal receiver where to send the pong packets to.

Thanks that makes sense.

@stale
Copy link

stale bot commented Jun 14, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is important, please feel free to bring it up on the next Interledger Community Group Call or in the Gitter chat.

@stale stale bot added the wontfix label Jun 14, 2020
@stale stale bot closed this Jun 21, 2020
@huijing huijing deleted the df-ping-protocol branch November 1, 2023 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants