This repository has been archived by the owner on Apr 23, 2024. It is now read-only.
sdk/agent: make open/pay/close ops consistently async #241
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Split the
Close
function in two, so there isDeclareClose
andClose
, and make them asynchronous and responsible for a single task each, mostly. TheDeclareClose
is responsible for kicking off a close, that may or may not complete itself via negotiation in the background. TheClose
is responsible for submitting the final close tx after the observation period.Why
The
Open
andPayment
functions both have a single responsibility to open and pay respectfully. They perform these operations asynchronously by kicking off the process and then having corresponding handlers handle the response from the other participant should they agree. Their use is rather predictable.The
Close
function is more confusing about where its responsibility begins and ends. It kicks off a close, negotiates an early close, submits it if it can, otherwise blocks waiting for the observation period to submit the final close. This all seems very convenient except it ignores the fact that a real life payment channel could have an observation period of 24 hours. It is impractical for an in memory application to block for that long, or any observation period really no matter how short.The
Close
function is simply doing too much.It's clear that because of the observation period that sits between the declare and submit in a non-negotiated close that those two operations need to have their own functions. In the worst case where the other participant won't negotiate a quick close the calling code using the agent will need to declare and then close later. This makes the responsibility of the two functions rather simple and clear:
We are then left with where to place the negotiation. There could be a new function for that, such as
NegotiateClose
, but that is beginning to muddy the API and create more options than the caller really needs. Declaring the close, since it is a public announcement, signals much like the beginning of a negotiation. If the participant is going to publicly announce the close, they may as well also kick off a negotiation. In the worst case the negotiation goes ignored. In the best case agreement is confirmed. I would argue in this case that even though the declare does two things, they are one responsibility: declaring the close publicly and directly to the other participant.It's also worth noting the handling of the negotiation occurs in the background and both participants attempt to automatically close the channel once they have a complete close agreement. There's no reason for either not to at that point and because the agent doesn't yet have event handlers or callbacks there's no way for the code using the agent to find out the negotiation succeeded. We may want to revisit and remove the automatic close behavior once event handlers are added (#226) to give the application more control and make the negotiated and non-negotiated close processes more identical.
Close #224