Page MenuHomePhabricator

[chronik-client] Support websocket subscriptions to plugins
ClosedPublic

Authored by bytesofman on Wed, Sep 18, 19:00.

Details

Summary

Support the chronik functionality introduced in D16696 in chronik-client

Test Plan

CI for chronik-client integration tests

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Correct diff link for change log

correct test script to match resolved mempool bugs, add validation for plugin subs, test plugin subs and unsubs, clean up logs

tobias_ruck added a subscriber: tobias_ruck.

otherwise lgtm

modules/chronik-client/src/ChronikClient.ts
765 ↗(On Diff #49725)

eventually we should abstract this for all subscription types but not in this diff

modules/chronik-client/src/validation.ts
75 ↗(On Diff #49725)

is there a reason for the "first parameter"? since it's an object and not a list

83 ↗(On Diff #49725)

same here

89 ↗(On Diff #49725)

and here

This revision is now accepted and ready to land.Thu, Sep 19, 09:05
bytesofman added inline comments.
modules/chronik-client/src/validation.ts
75 ↗(On Diff #49725)

In this case, the user only sees this error msg by calling subscribeToPlugin(pluginName, group)

case to be made that all subscription functions should be more like subscribeTo<Thing>(objectOfExpectedShape), e.g. in this case subscribeToPlugin({pluginName: <name>, group: <group>}) -- that is not currently the API tho (subscribeToScript is like this as well).

That said ... yeah I don't think "first parameter" is really helping. If we do truly need it, good sign the API needs to be changed.

This revision was automatically updated to reflect the committed changes.
bytesofman marked an inline comment as done.