Page MenuHomePhabricator

[Chronik] Plugins: Load plugins based on plugins.toml in datadir
ClosedPublic

Authored by tobias_ruck on May 24 2024, 18:32.

Details

Summary

Allow the user to specify a plugins.toml file in the datadir, which will load Python plugins at runtime.

Plugins are supposed to be placed in a plugins directory and then loaded if specified in the plugins.toml, however, any module that can be found on the PYTHONPATH can be loaded this way. This allows users to both install plugins globally (e.g. using pip install), but also have datadir-specific plugins.

This mean normal python import rules apply, i.e. if the plugin is just one file, it's fine to have a myplugin.py in the plugins folder, or if more complex handling is required, a directory with a __init__.py works too, as well as .egg files, and others.

The approach using the plugins.toml file has been picked for these reasons:

  • The alternative would be automatic discovery, e.g. using the approaches listed here. However, it would mean that plugins cannot be disabled easily, other than deleting/moving the plugin; also, if users install plugins globally (e.g. using pip install), they will be shared with all instances, which might not be desirable.
  • It allows us to configure plugins individually (using TOML), e.g. to enable/disable a certain feature of a plugin, or to specify a token ID of a plugin, etc.
  • It allows having some plugins enabled for testnet/regtest but not for mainnet.

A plugin has to define a plugin class, the naming convention is to PascalCase the module name and append Plugin (e.g. my_demo -> MyDemoPlugin), and this can be overridden by setting the "class" config of the plugin. Currently, the init function of a plugin has to take a config object; for now this will always be the empty dict but in the future we can provide config params from the plugins.toml.

The specification for plugins can be found here.

Test Plan

./test/functional/test_runner.py chronik_plugins_setup

Event Timeline

Tail of the build log:

  File "/work/abc-ci-builds/ecash-lib-integration-tests/test/functional/test_runner.py", line 361, in main
    os.makedirs(tmpdir)
  File "/usr/lib/python3.9/os.py", line 225, in makedirs
    mkdir(name, mode)
FileExistsError: [Errno 17] File exists: '/work/abc-ci-builds/ecash-lib-integration-tests/test/tmp/test_runner_₿₵_🏃_20240524_185356'
Test runner completed with code 1
----------------------------|---------|----------|---------|---------|------------------------------
File                        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
----------------------------|---------|----------|---------|---------|------------------------------
All files                   |   63.22 |    52.85 |   62.85 |   63.27 |                              
 ecash-lib                  |       0 |        0 |       0 |       0 |                              
  eslint.config.js          |       0 |        0 |       0 |       0 |                              
 ecash-lib/src              |   74.36 |    53.57 |   74.02 |   74.14 |                              
  ecc.ts                    |   57.14 |       80 |      40 |   57.14 | 23-31                        
  hash.ts                   |   88.88 |       80 |      80 |   88.88 | 14                           
  index.ts                  |       0 |        0 |       0 |       0 |                              
  indexBrowser.ts           |       0 |        0 |       0 |       0 |                              
  indexNodeJs.ts            |       0 |        0 |       0 |       0 |                              
  initBrowser.ts            |       0 |      100 |       0 |       0 | 11-13                        
  initNodeJs.ts             |     100 |      100 |     100 |     100 |                              
  op.ts                     |   49.15 |    55.55 |      80 |   49.15 | ...89,92,104,107,109,117-122 
  opcode.ts                 |     100 |       80 |     100 |     100 | 1                            
  script.ts                 |    61.7 |    36.36 |   64.28 |      60 | ...88-99,110,120,130,152-163 
  sigHashType.ts            |   77.77 |    37.03 |   85.71 |   77.77 | 26-38                        
  tx.ts                     |   93.47 |    78.26 |    90.9 |   93.18 | 123-125                      
  txBuilder.ts              |   52.74 |    37.83 |   66.66 |   51.72 | ...3-107,124-179,214,244-248 
  unsignedTx.ts             |    73.8 |    56.25 |   78.57 |   74.07 | ...3,151,159,184,192,198-201 
 ecash-lib/src/ffi          |   28.26 |    15.94 |   16.98 |   28.98 |                              
  ecash_lib_wasm_browser.js |       0 |        0 |       0 |       0 | 3-336                        
  ecash_lib_wasm_nodejs.js  |    61.9 |       55 |   39.13 |   62.75 | ...1,197-215,237,250-251,255 
 ecash-lib/src/io           |   59.55 |       60 |   70.58 |   58.77 |                              
  bytes.ts                  |     7.4 |    57.14 |    12.5 |     7.4 | 13-64                        
  hex.ts                    |   82.05 |    66.66 |      80 |   82.35 | 41-45,50,58                  
  int.ts                    |       0 |        0 |       0 |       0 |                              
  str.ts                    |   85.71 |       80 |   66.66 |   85.71 | 15                           
  varsize.ts                |      32 |    33.33 |   66.66 |      32 | 14-24,40-47                  
  writer.ts                 |       0 |        0 |       0 |       0 |                              
  writerbytes.ts            |   83.33 |    73.91 |     100 |   83.33 | 33,43,53,63,79               
  writerlength.ts           |     100 |       80 |     100 |     100 | 1                            
 ecash-lib/src/test         |   87.67 |       58 |    87.5 |   88.23 |                              
  testRunner.ts             |   87.67 |       58 |    87.5 |   88.23 | 69-71,84-85,108,119,162      
 ecash-lib/src/token        |   87.15 |    74.02 |   93.33 |   87.07 |                              
  alp.ts                    |   82.92 |    89.47 |   83.33 |   82.92 | 110-123,142                  
  common.ts                 |     100 |       80 |     100 |     100 | 1                            
  empp.ts                   |    92.3 |    77.77 |     100 |   91.66 | 12                           
  slp.ts                    |   89.74 |     65.9 |     100 |   89.74 | ...9,161,167,175,178,197,202 
----------------------------|---------|----------|---------|---------|------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='772']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='1221']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='259']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='490']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='132']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='210']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='753']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='1190']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/ecash-lib-integration-tests-junit.xml': No such file or directory
Build ecash-lib-integration-tests failed with exit code 1

Tail of the build log:

  File "/work/test/functional/setup_scripts/../test_framework/util.py", line 297, in wait_until_helper
    raise AssertionError(
AssertionError: Predicate ''''
            self.wait_until(lambda: is_finalblock(next_blockhash))
''' not true after 60.0 seconds
2024-05-24T18:59:20.375000Z TestFramework (INFO): Stopping nodes
2024-05-24T18:59:20.931000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20240524_185808/setup_scripts/chronik-client_websocket_0
2024-05-24T18:59:20.931000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20240524_185808/setup_scripts/chronik-client_websocket_0/test_framework.log
2024-05-24T18:59:20.931000Z TestFramework (ERROR): 
2024-05-24T18:59:20.932000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20240524_185808/setup_scripts/chronik-client_websocket_0' to consolidate all logs
2024-05-24T18:59:20.932000Z TestFramework (ERROR): 
2024-05-24T18:59:20.932000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-05-24T18:59:20.933000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2024-05-24T18:59:20.933000Z TestFramework (ERROR): 
Running Unit Tests for Test Framework Modules
setup_scripts/chronik-client_websocket.py started
setup_scripts/chronik-client_websocket.py failed, Duration: 72 s

stdout:

stderr:


TEST                                      | STATUS    | DURATION

setup_scripts/chronik-client_websocket.py | ✖ Failed  | 72 s

ALL                                       | ✖ Failed  | 72 s (accumulated) 
Runtime: 72 s

Test runner for chronik-client_websocket completed with code 1
-----------------------|---------|----------|---------|---------|-----------------------------------
File                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                 
-----------------------|---------|----------|---------|---------|-----------------------------------
All files              |   28.35 |     11.1 |   24.57 |   28.35 |                                   
 chronik-client        |     100 |      100 |     100 |     100 |                                   
  index.ts             |     100 |      100 |     100 |     100 |                                   
 chronik-client/proto  |    21.6 |     8.03 |    13.9 |   21.74 |                                   
  chronik.ts           |     6.1 |        1 |    2.54 |    6.09 | ...,3978-3985,3990-4027,4031-4036 
  chronikNode.ts       |   33.03 |    13.33 |    23.6 |   33.28 | ...,4991-5030,5038-5111,5146-5151 
 chronik-client/src    |   65.77 |    46.64 |   63.41 |   65.38 |                                   
  ChronikClient.ts     |    4.24 |        0 |       0 |    4.29 | 33-163,178-222,290-692            
  ChronikClientNode.ts |   90.84 |    71.51 |   96.38 |   90.84 | ...,1068,1078,1103,1115,1121,1127 
  failoverProxy.ts     |   75.22 |    59.09 |   62.06 |   74.52 | ...67,275-285,294,301,305,310,314 
  hex.ts               |   89.47 |       75 |      75 |   87.87 | 58,66-68                          
  validation.ts        |   93.33 |    88.23 |     100 |   92.59 | 33,39                             
-----------------------|---------|----------|---------|---------|-----------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='1188']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='4189']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='500']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='4503']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='187']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='761']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='1176']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='4148']
##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
Fabien requested changes to this revision.May 27 2024, 08:10
Fabien added inline comments.
chronik/chronik-cpp/chronik.cpp
47 ↗(On Diff #47959)

You should pass the ArgsManager directly to the Start function, as this is using the global all over the place.
Another better solution is to decouple entirely from ArgsManager by passing an option object, and create a parsing function that will use to ArgsManager and return a ChronikOption object (see how it's done for other classes). The callsite will the look like this:

ArgsManager args;
ChronikOptions = ParseChronikOptions(args);
Start(ChronikOptions, ...);
chronik/chronik-lib/src/bridge.rs
74 ↗(On Diff #47959)

This is like so to make the plugins chain independent ? I'm not sure if this is the best option:

+ You don't duplicate the plugin for each chain
- You risk using WIP/debug plugins on mainnet, which is error prone
- You can't easily have different options for the different chains
- The index files will be chain dependent anyway, so it's not obvious if the index will contain the plugin data or not

Side note: if you move the config to plugins/plugins.toml, you make sure the plugins/ dir exists which could simplify the loading logic.

test/functional/chronik_plugins_setup.py
34 ↗(On Diff #47959)

Using the log is fine for now, but you should add a plugins/ endpoint that returns a list of plugins in a future diff and use that in tests

This revision now requires changes to proceed.May 27 2024, 08:10

redesign plugin system, use Plugin class, and add [main.plugin] [test.plugin] and [regtest.plugin] sections

fix some comments, remove unused/unneeded error variants

Fabien requested changes to this revision.Jul 6 2024, 08:35

Please split out the renaming chronik-plugin-impl -> chronik-plugin-common, it just makes it harder to review due to how phab shows the diff

This revision now requires changes to proceed.Jul 6 2024, 08:35
This revision is now accepted and ready to land.Jul 9 2024, 21:04