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

RPC: add client side ids to notification listeners. #2085

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from

Conversation

D-Stacks
Copy link
Contributor

@D-Stacks D-Stacks commented Jun 16, 2022

this has been requested by devs in the discord development channel, specfically for kdx, and perhaps for usage in the vitex gateway?

it allows, for example, kaspad clients which handle multiple users, to differentiate the kaspad stream responses specific for each user.

To Do:

  • add to and from kaspadmessage Id conversion
  • add Register...WithId to kaspad rpcclient, for id usage in the native client.
  • add tests.

on a further note, it can be used to perhaps create a new listener with each id requested, but may be for further development based on this pr, but beyond the scope of this pr itself.

I also hope some of the code can be used for future rpc reworks.

Note: currently I believe this only works for situations where clients intiate a seperate rpc connection, for each notification stream.

@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #2085 (41cb5f5) into dev (b9093d5) will increase coverage by 0.34%.
The diff coverage is 67.33%.

❗ Current head 41cb5f5 differs from pull request most recent head 93d9a75. Consider uploading reports for the commit 93d9a75 to get more accurate results

@@            Coverage Diff             @@
##              dev    #2085      +/-   ##
==========================================
+ Coverage   59.19%   59.54%   +0.34%     
==========================================
  Files         678      678              
  Lines       32831    33071     +240     
==========================================
+ Hits        19434    19691     +257     
+ Misses      10596    10527      -69     
- Partials     2801     2853      +52     
Impacted Files Coverage Δ
app/appmessage/rpc_stop_notifying_utxos_changed.go 0.00% <0.00%> (ø)
...stop_notifying_pruning_point_utxo_set_overrides.go 0.00% <0.00%> (ø)
...pp/rpc/rpchandlers/stop_notifying_utxos_changed.go 0.00% <0.00%> (ø)
...rver/protowire/rpc_stop_notifying_utxos_changed.go 0.00% <0.00%> (ø)
...pcclient/rpc_on_pruning_point_utxo_set_override.go 38.70% <41.66%> (-3.23%) ⬇️
...wire/rpc_notify_pruning_point_utxo_set_override.go 36.36% <43.47%> (+0.29%) ⬆️
app/appmessage/rpc_notify_finality_conflicts.go 40.00% <50.00%> (+40.00%) ⬆️
...cserver/protowire/rpc_notify_finality_conflicts.go 34.32% <52.94%> (+34.32%) ⬆️
...ure/network/rpcclient/rpc_on_finality_conflicts.go 29.31% <56.66%> (+29.31%) ⬆️
...sage/rpc_notify_pruning_point_utxo_set_override.go 60.00% <60.00%> (ø)
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9093d5...93d9a75. Read the comment docs.

@D-Stacks D-Stacks marked this pull request as ready for review June 19, 2022 11:16
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.

1 participant