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

feat: wallet connectivity service #3159

Merged
merged 2 commits into from
Aug 10, 2021

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Aug 4, 2021

Description

Adds a service responsible for wallet connectivity. This service
is responsible for and abstracts any complexity in the management of
the base node connections and RPC session management.

This PR makes use of this service in the base node montoring service but
does not "plumb" the WalletConenctivityService into the protocols. This
is left as a TODO, but we should expect this to remove many lines of
code and greaty simplify these protocols by removing the budren of
connection management in the various wallet components.

A number of simplifications on the peer connection and substream code,
debatably reducing places that bugs could hide.

Motivation and Context

Follow on from #3152, making use of the pool in the wallet base node monitoring. The rest of the protocols should follow.

How Has This Been Tested?

Extensive service unit tests
Existing tests pass

Washing machine test on local wallet, with the broadcast protocol modified to use the wallet connectivity
Base node RPC sessions max out at 10 each (2 wallets connected to same base node) even when pushing through 426 transactions.
image

Manually tested wallet connectivity states (Connecting -> Online | Connecting -> Offline | Connecting -> Online -> Offline -> Connecting -> Online)

Checklist:

  • I'm merging against the development branch.
  • I have squashed my commits into a single commit.

@sdbondi sdbondi force-pushed the wallet-use-rpc-pool branch from 72a05fd to 5449c4c Compare August 4, 2021 11:45
@sdbondi sdbondi changed the title feat: wallet connectivity service BLOCKED: feat: wallet connectivity service Aug 4, 2021
@sdbondi sdbondi force-pushed the wallet-use-rpc-pool branch 6 times, most recently from 7d12fa3 to 1930a16 Compare August 4, 2021 17:55
@sdbondi sdbondi changed the title BLOCKED: feat: wallet connectivity service feat: wallet connectivity service Aug 7, 2021
@sdbondi sdbondi force-pushed the wallet-use-rpc-pool branch 4 times, most recently from 483c545 to 4049968 Compare August 9, 2021 13:04
Adds a service responsible for wallet connectivity. This service
is responsible for and abstracts any complexity in the management of
the base node connections and RPC session management.

This PR makes use of this service in the base node montoring service but
does not "plumb" the WalletConenctivityService into the protocols. This
is left as a TODO, but we should expect this to remove many lines of
code and greaty simplify these protocols by removing the budren of
connection management in the various wallet components.

A number of simplifications on the peer connection and substream code,
debatably reducing places that bugs could hide.
@sdbondi sdbondi force-pushed the wallet-use-rpc-pool branch from 4049968 to e1e0f0c Compare August 9, 2021 13:22
Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

Nice. I prefer 'create_base_node_...toobtain_base_node..` but otherwise is good

@aviator-app
Copy link
Contributor

aviator-app bot commented Aug 10, 2021

PR queued successfully. Your position in queue is: 1

@sdbondi
Copy link
Member Author

sdbondi commented Aug 10, 2021

Yeah I named it that after initially naming it create_... just because the semantics of this call is not to necessarily create one but to somehow obtain one ("I don't care how/when just give me an RPC connection") - from a pool of existing connections.

@aviator-app aviator-app bot merged commit 54e8c8e into tari-project:development Aug 10, 2021
@sdbondi sdbondi deleted the wallet-use-rpc-pool branch August 10, 2021 12:05
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.

2 participants