Peerd connect treatmeant #787
Conversation
Peer messages sent to a remote peer are now cached until the remote peer has replied with a corresponding receipt. If the connection fails in a listener spawned peerd, the cache is emptied before peerd shuts down and the messages are sent back to their respective origin (swapd). Swapd then processes them into its own cache. If the connection fails in a connecting peerd, the cache is only emptied once the reconnect has been a success. Messages are not sent back to their origin.
This allows us to consistently inform the user on the connection status of the swap. Peerd now emits Disconnected and Reconnected requests to farcasterd, which in turn can route them on to swapd.
The connect command now allows a user to manually trigger a connect for a restored swap that was previously unable to connect.
was calling colorize func, not swap_id itself
…aster.dev-testing
| // is only possible to a listening peer, its address thus has to | ||
| // match the address of the offer. | ||
| if peerd.is_none() { | ||
| Some(ServiceId::Peer(node_addr_from_public_offer(public_offer))) |
There was a problem hiding this comment.
with a taker that successively takes offers from the maker once the last completes: if the last successful run was Bob maker, then Alice taker will most likely terminate connection before Bob has sweeped his xmr. Then Alice launches another swap.
In my understanding, a new peer connection will not be registered because awaiting_connect_from(..) will still match against the tsm of the last successful run here, since Bob's peerd is down, yet node_addr_from_public_offer(..) is the same for the old and the new swap?
There was a problem hiding this comment.
I still don't quite understand how this triggers a state where the peerd.is_none(), but this has to be the case, so I added an additional check to only trigger this when taking / connecting a swap.
sedited
force-pushed
the
peerdConnectTreatmeant
branch
from
b022b00 to
cc47368
Compare
| ); | ||
| } | ||
| } | ||
| let peerd_up = runtime.registered_services.contains(&peerd); |
There was a problem hiding this comment.
with a unique ServiceId for every peerd instantiation, if we create a new peerd instance for a remote and kill the old one, this will return false since the old peerd's ServiceId is cached in the tsm but won't be in the runtime.registered_services anymore, so swap will never launch. So we can instead here check only against the node address of the peerd key in runtime.registered_services, but if there are multiple concurrent peerds living for said remote, then still should ensure that we launch with the latest one.
There was a problem hiding this comment.
I've pushed a commit that does an ugly impl of this. TODO if we go with this change:
- it currently just uses the first peerd it finds in registered services - imo we'd need to encode creation time in the peerd
ServiceIdto do that correctly - the
SwapdLaunchedstruct should not keep the full peerdServiceIdsince only theNodeAddris read with this change
Lederstrumpf
added
the
farcaster.dev testing
sedited
force-pushed
the
peerdConnectTreatmeant
branch
from
56fb536 to
b8484ac
Compare
Handle Connect msg properly for running and restoring swapd Peerd: Add a Pong to our own hacky handshake Peerd: Only insert protocol messages to unchecked_msg_cache
Farcaster: Correct handling of restoring tsm connection
Ensure ConnectSuccess message is only expected for Taker swaps Handle new and failed connections in a coherent manner Trigger peerd termination externally, not internally
log swapids if awaiting connection
Peerd: Emit Reconnected only after checking cached messages Do not reconnect as a listener-spawned peerd
The peerd service Id is now randomized per instance
sedited
force-pushed
the
peerdConnectTreatmeant
branch
from
b8484ac to
7745f55
Compare
|
Closing this PR in favor of #792 |
3 participants
Based on @Lederstrumpf farcasterd.dev-testing branch.