Page MenuHomePhabricator
Differential D16605

[chronik-client] Support plugins
ClosedPublic
Actions

Authored by bytesofman on Aug 7 2024, 21:36.

Details

Reviewers
tobias_ruck
Group Reviewers
Restricted Project
Commits
rABC179560629131: [chronik-client] Support plugins
Summary

Add support and tests for new plugins endpoint in chronik-client

Test Plan

CI tests

Diff Detail

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

Event Timeline

bytesofman created this revision.Aug 7 2024, 21:36
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 7 2024, 21:36
Harbormaster completed remote builds in B29895: Diff 49092.Aug 7 2024, 22:34
teamcity added a comment.Aug 7 2024, 22:34
Build Bitcoin ABC Diffs / Diff Testing (chronik-client-integration-tests) failed.

Tail of the build log:

Running Unit Tests for Test Framework Modules
setup_scripts/chronik-client_mempool_conflicts.py started
setup_scripts/chronik-client_mempool_conflicts.py passed, Duration: 1 s

TEST                                              | STATUS    | DURATION

setup_scripts/chronik-client_mempool_conflicts.py | ✓ Passed  | 1 s

ALL                                               | ✓ Passed  | 1 s (accumulated) 
Runtime: 1 s

testRunner complete in mempool_conflicts.ts
Starting test_runner for chronik-client_plugin_groups
Test runner for chronik-client_plugin_groups started
2024-08-07T22:33:04.123000Z TestFramework (WARNING): Test Skipped: Chronik indexer plugins have not been compiled.
2024-08-07T22:33:04.173000Z TestFramework (INFO): Stopping nodes
2024-08-07T22:33:04.173000Z TestFramework (INFO): Cleaning up /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20240807_223303/setup_scripts/chronik-client_plugin_groups_0 on exit
2024-08-07T22:33:04.173000Z TestFramework (INFO): Test skipped
Running Unit Tests for Test Framework Modules
setup_scripts/chronik-client_plugin_groups.py started
setup_scripts/chronik-client_plugin_groups.py skipped

TEST                                          | STATUS    | DURATION

setup_scripts/chronik-client_plugin_groups.py | ○ Skipped | 0 s

ALL                                           | ✓ Passed  | 0 s (accumulated) 
Runtime: 0 s

plugin_groups.ts tests complete, shutting down child process
Test runner error for chronik-client_plugin_groups, aborting: Error [ERR_IPC_CHANNEL_CLOSED]: Channel closed
-----------------------|---------|----------|---------|---------|-----------------------------------
File                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                 
-----------------------|---------|----------|---------|---------|-----------------------------------
All files              |   19.01 |     5.03 |   15.48 |   18.94 |                                   
 chronik-client        |     100 |      100 |     100 |     100 |                                   
  index.ts             |     100 |      100 |     100 |     100 |                                   
 chronik-client/proto  |   14.58 |      3.8 |    9.14 |   14.68 |                                   
  chronik.ts           |     6.1 |     0.83 |    2.54 |    6.09 | ...,3978-3985,3990-4027,4031-4036 
  chronikNode.ts       |   19.81 |     5.97 |   13.52 |   19.99 | ...,5990-6029,6037-6110,6145-6154 
 chronik-client/src    |   44.54 |    22.18 |   41.17 |   43.75 |                                   
  ChronikClient.ts     |    4.24 |        0 |       0 |    4.29 | 33-163,178-222,290-692            
  ChronikClientNode.ts |   47.32 |    33.08 |    52.8 |   47.32 | ...,1182,1192,1213-1221,1229-1294 
  failoverProxy.ts     |   75.22 |    51.61 |   62.06 |   74.52 | ...67,275-285,294,301,305,310,314 
  hex.ts               |   89.47 |       50 |      75 |   87.87 | 58,66-68                          
  validation.ts        |   66.66 |    18.18 |   66.66 |   62.96 | 17,21,33,38-49,62-63              
-----------------------|---------|----------|---------|---------|-----------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='880']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='4629']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='214']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='4249']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='133']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='859']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='868']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='4582']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/chronik-client-integration-tests-junit.xml': No such file or directory
Build chronik-client-integration-tests failed with exit code 1
bytesofman updated this revision to Diff 49093.Aug 7 2024, 22:48

compile with chronik plugins for chronik-client integration tests

Harbormaster completed remote builds in B29896: Diff 49093.Aug 7 2024, 23:17
bytesofman published this revision for review.Aug 7 2024, 23:18
bytesofman added inline comments.
modules/chronik-client/proto/chronikNode.ts
308 ↗(On Diff #49092)

This file is generated

will need to add support for the new blockheader endpoints in a separate diff. at the moment I'm not aware of a chronik-client use case for this but doesn't hurt to have it available.

modules/chronik-client/src/ChronikClientNode.ts
1011 ↗(On Diff #49092)

In this way, we get groups as an array of hex strings, ['aaa', 'aab', 'aba'], instead of the object array format in proto, [{group: 'aaa'}, {group: 'aab'}, {group: 'aba'}]

Is there some reason the object array format should be preserved here in chronik-client? Mb more info will be added later?

tobias_ruck requested changes to this revision.Aug 8 2024, 15:58
tobias_ruck added a subscriber: tobias_ruck.
tobias_ruck added inline comments.
modules/chronik-client/src/ChronikClientNode.ts
1011 ↗(On Diff #49092)

I made it its own independent message in case we wanna add more fields in the future, e.g. the number of UTXOs or txs of the group. I think it would be better to mimick the proto so we have the extensibility if needed.

This revision now requires changes to proceed.Aug 8 2024, 15:58
bytesofman updated this revision to Diff 49100.Aug 8 2024, 20:01

preserve group object for extensibility, remove unused constants from plugin_groups.ts test file, rebase

bytesofman mentioned this in D16543: [chronik-client] Add `plugins` field to outputs/inputs, add `PluginEndpoint`.Aug 8 2024, 20:14
tobias_ruck added inline comments.Aug 8 2024, 20:28
modules/chronik-client/test/integration/plugin_groups.ts
113 ↗(On Diff #49100)

It should return 404: Plugin "<plugin_name>" not loaded — maybe the client needs to have some special handling?

tobias_ruck mentioned this in D16609: [Chronik] Add tests for 404 errors on plugin endpoints.Aug 8 2024, 21:00
bytesofman updated this revision to Diff 49105.Aug 8 2024, 21:40

Correct implementation bug in plugin not existing test case

Harbormaster completed remote builds in B29902: Diff 49100.Aug 8 2024, 21:41
bytesofman marked an inline comment as done.Aug 8 2024, 21:50
bytesofman added inline comments.
modules/chronik-client/test/integration/plugin_groups.ts
113 ↗(On Diff #49100)

typo in test implementation, corrected

Harbormaster completed remote builds in B29906: Diff 49105.Aug 8 2024, 22:09
tobias_ruck requested changes to this revision.Aug 9 2024, 01:22

some ideas for better code style

modules/chronik-client/src/ChronikClientNode.ts
983 ↗(On Diff #49105)

wouldn't it make sense to use Object.entries here?

984 ↗(On Diff #49105)

I think it would be better style to use a continue here to save the indent

987 ↗(On Diff #49105)

if you use the .map as below, you don't need an extra variable

988 ↗(On Diff #49105)

wouldn't this be cleaner? dito for data

1015 ↗(On Diff #49105)

less convinced of this one, but could be cleaner; up to you

This revision now requires changes to proceed.Aug 9 2024, 01:22
tobias_ruck mentioned this in rABC193c2dc00373: [Chronik] Add tests for 404 errors on plugin endpoints.Aug 9 2024, 01:29
bytesofman marked 6 inline comments as done.Aug 9 2024, 03:29
bytesofman added inline comments.
modules/chronik-client/src/ChronikClientNode.ts
983 ↗(On Diff #49105)

not sure what we need entries for vs plugins. we don't need the values; if there are no keys then there are no plugins. is there some advantage i'm overlooking?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries

988 ↗(On Diff #49105)

yes this is much better

bytesofman updated this revision to Diff 49114.Aug 9 2024, 03:30
bytesofman marked 2 inline comments as done.

improving methods using .map, style improvement for for loop

Harbormaster completed remote builds in B29914: Diff 49114.Aug 9 2024, 03:49
tobias_ruck added inline comments.Aug 9 2024, 08:41
modules/chronik-client/src/ChronikClientNode.ts
983 ↗(On Diff #49105)

well you can do this right away:

for (const [pluginName, plugin] of Object.entries(plugins) {

and then you don't have to do the plugins[plugin] later because you already have the plugin

tobias_ruck accepted this revision.Aug 9 2024, 09:04

if you don't like the Object.entries change, fine to ignore

modules/chronik-client/test/integration/plugins.ts
377–378 ↗(On Diff #49114)

every other place is ordered groups, data

This revision is now accepted and ready to land.Aug 9 2024, 09:04
bytesofman marked an inline comment as done.Aug 9 2024, 13:31
bytesofman added inline comments.
modules/chronik-client/src/ChronikClientNode.ts
983 ↗(On Diff #49105)

ohhh I was misreading this, for not-paying-attention reasons I thought the comment was on

if (Object.keys(utxo.plugins).length > 0) {
        // We only return a plugins key if we have plugins
        utxoInNode.plugins = convertToPluginEntries(utxo.plugins);
    }

yeah, Object.entries is better here, replaced

bytesofman updated this revision to Diff 49115.Aug 9 2024, 13:31

use object.entries, patch order

Harbormaster completed remote builds in B29915: Diff 49115.Aug 9 2024, 13:56
Closed by commit rABC179560629131: [chronik-client] Support plugins (authored by bytesofman). · Explain WhyAug 9 2024, 16:14
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC179560629131: [chronik-client] Support plugins.

Revision Contents
Changeset List

PathSize
contrib/
teamcity/
1 line
modules/
chronik-client/
1 line
4 lines
2 lines
proto/
1033 lines
src/
144 lines
test/
integration/
464 lines
450 lines
test/
functional/
setup_scripts/
404 lines
262 lines

Diff 49116

View Options

contrib/teamcity/build-configurations.yml

Loading...
View Options

modules/chronik-client/README.md

Loading...
View Options

modules/chronik-client/package-lock.json

Loading...
View Options

modules/chronik-client/package.json

Loading...
View Options

modules/chronik-client/proto/chronikNode.ts

Loading...
View Options

modules/chronik-client/src/ChronikClientNode.ts

Loading...
View Options

modules/chronik-client/test/integration/plugin_groups.ts

Loading...
View Options

modules/chronik-client/test/integration/plugins.ts

Loading...
View Options

test/functional/setup_scripts/chronik-client_plugin_groups.py

Loading...
View Options

test/functional/setup_scripts/chronik-client_plugins.py

Loading...