fix: bug in wallet base node peer switching #3217
Conversation
sdbondi
force-pushed
the
wallet-monitoring-fix
branch
from
0e0fc0e to
6da18e5
Compare
- Connectivity retries connecting indefinitely, however previously if this continuously fails and the user updates their peer, the new peer will not be read until the previous peer was connected to. This PR fixes this. - Add protocl information to comms RPC logs - Cleanup peer state, making the wallet connectivity service the source of truth for the base node peer - Adds an `Ack` flag to RPC protocol, this is not currently used but could be implemented in the client side in future if required without breaking the network (server supports it, client support may or may not be needed).
sdbondi
force-pushed
the
wallet-monitoring-fix
branch
from
6da18e5 to
17e44aa
Compare
| self.wallet_connectivity.set_base_node(peer.node_id.clone()).await?; | ||
|
|
||
| self.publish_event(BaseNodeEvent::BaseNodeStateChanged(new_state)); | ||
| self.wallet_connectivity.set_base_node(peer.clone()).await?; |
There was a problem hiding this comment.
This feels like it should happen in the wallet container set_base_node_peer function where all the other wallet services have their set_base_node method called.
There was a problem hiding this comment.
Yeah agree, will require a bit more of a refactor (BaseNodeEvent::BaseNodePeerSet event needs to be moved around / base node service needs to monitor the change and emit the event), wanted to get this in quicker.
|
PR queued successfully. Your position in queue is: 2 |
|
PR is on top of the queue now |
|
PR failed to merge with reason: Failed due to unknown reason |
|
PR queued successfully. Your position in queue is: 5 |
|
PR is on top of the queue now |
|
PR queued successfully. Your position in queue is: 1 |
Description
this continuously fails and the user updates their peer, the new peer will not be
read until the previous peer was connected to. This PR fixes this.
of truth for the base node peer
Ackflag to RPC protocol, this is not currently used butcould be implemented in the client side in future if required without
breaking the network (server supports it, we may choose in future to implement a client keep alive should it be needed,
current server side impl easy and zero cost).
Motivation and Context
Critical bug fix
How Has This Been Tested?
Switching the console wallet base node peer a number of times
Checklist:
developmentbranch.