Page MenuHomePhabricator

[chronik] Create a new optional JSON RPC interface
ClosedPublic

Authored by Fabien on Nov 22 2024, 16:24.

Details

Reviewers
tobias_ruck
PiRK
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC857f5c91159a: [chronik] Create a new optional JSON RPC interface
Summary

This will be used to serve the result in an Electrum compliant way if possible.

Note that the Electrum protocol is not 100% compliant with the JSON RPC standard as it enforces the use of a newline as a delimiter.
This diff also updates ElectrumABC so it is compatible with both versions.

Test Plan
./src/bitcoind -regtest -chronik -chronikelectrumbind="[::1]:50001" -chronikelectrumbind="127.0.0.1:50001"

then:

echo '{"jsonrpc": "2.0", "method": "server.ping", "id": "test"}' | nc 127.0.0.1 50001

ninja all check-all

Event Timeline

Fabien requested review of this revision.Nov 22 2024, 16:24

Tail of the build log:

    Checking tokio-util v0.7.12
    Checking tower v0.4.13
    Checking jsonrpsee-core v0.24.7
    Checking serde_spanned v0.6.6
    Checking toml_datetime v0.6.6
   Compiling pyo3 v0.22.2
    Checking topo_sort v0.4.0
    Checking h2 v0.4.7
    Checking tokio-stream v0.1.16
    Checking sync_wrapper v0.1.2
    Checking seahash v4.1.0
    Checking winnow v0.6.13
    Checking mime v0.3.17
    Checking route-recognizer v0.3.1
    Checking axum-core v0.4.3
    Checking tokio-tungstenite v0.21.0
    Checking memoffset v0.9.1
   Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
    Checking serde_urlencoded v0.7.1
    Checking serde_path_to_error v0.1.16
    Checking unicode-segmentation v1.11.0
    Checking base64 v0.21.7
    Checking unindent v0.2.3
    Checking sync_wrapper v1.0.1
    Checking matchit v0.7.3
    Checking tower-http v0.5.2
    Checking convert_case v0.6.0
   Compiling chronik-bridge v0.1.0 (/work/chronik/chronik-bridge)
    Checking toml_edit v0.22.14
   Compiling librocksdb-sys v0.11.0+8.1.1
   Compiling pyo3-macros v0.22.2
    Checking toml v0.8.14
    Checking hyper v1.5.1
    Checking hyper-util v0.1.3
    Checking jsonrpsee-server v0.24.7
    Checking axum v0.7.5
    Checking chronik-plugin-impl v0.1.0 (/work/chronik/chronik-plugin-impl)
    Checking jsonrpsee v0.24.7
    Checking rocksdb v0.21.0
    Checking chronik-db v0.1.0 (/work/chronik/chronik-db)
    Checking chronik-indexer v0.1.0 (/work/chronik/chronik-indexer)
    Checking chronik-http v0.1.0 (/work/chronik/chronik-http)
error: unused variable: `indexer`
  --> chronik/chronik-http/src/electrum.rs:87:9
   |
87 |         indexer: ChronikIndexerRef,
   |         ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_indexer`
   |
   = note: `-D unused-variables` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(unused_variables)]`

error: unused variable: `node`
  --> chronik/chronik-http/src/electrum.rs:88:9
   |
88 |         node: NodeRef,
   |         ^^^^ help: if this is intentional, prefix it with an underscore: `_node`

error: could not compile `chronik-http` (lib) due to 2 previous errors
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Tail of the build log:

   Doc-tests chronik-bridge

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-db

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-http

running 1 test
test chronik/chronik-http/src/protobuf.rs - protobuf::Protobuf (line 29) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.31s

   Doc-tests chronik-indexer

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-impl

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-proto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-util

running 2 tests
test chronik/chronik-util/src/log.rs - log::log (line 65) ... ignored
test chronik/chronik-util/src/log.rs - log::log_chronik (line 87) ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1

I have a test script in D17187 to check if the Electrum ABC code can query this. So far I haven't been successful.

Tail of the build log:

  File "/work/abc-ci-builds/ecash-lib-integration-tests/test/functional/test_runner.py", line 362, in main
    os.makedirs(tmpdir)
  File "<frozen os>", line 225, in makedirs
FileExistsError: [Errno 17] File exists: '/work/abc-ci-builds/ecash-lib-integration-tests/test/tmp/test_runner_₿₵_🏃_20241123_124724'
Test runner completed with code 1
----------------------------|---------|----------|---------|---------|------------------------------
File                        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
----------------------------|---------|----------|---------|---------|------------------------------
All files                   |   60.35 |    49.03 |   58.33 |   60.24 |                              
 ecash-lib                  |       0 |        0 |       0 |       0 |                              
  eslint.config.js          |       0 |        0 |       0 |       0 |                              
 ecash-lib/src              |   75.79 |    56.13 |   76.25 |   75.47 |                              
  consts.ts                 |       0 |      100 |     100 |       0 | 6-8                          
  ecc.ts                    |   57.14 |    83.33 |      40 |   57.14 | 23-31                        
  hash.ts                   |   88.88 |    83.33 |      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                     |      40 |    44.44 |   66.66 |      40 | ...4,107,109,117-122,133-161 
  opcode.ts                 |     100 |    83.33 |     100 |     100 | 1                            
  script.ts                 |   52.63 |    38.09 |      60 |    50.9 | ...4-135,146,156,166,188-199 
  sigHashType.ts            |   77.77 |       44 |   85.71 |   77.77 | 26-38                        
  tx.ts                     |   93.47 |    79.16 |    90.9 |   93.18 | 123-125                      
  txBuilder.ts              |   80.89 |    59.25 |   84.61 |   80.23 | ...1,154,173-178,183,236-240 
  unsignedTx.ts             |      75 |    57.14 |   85.71 |    75.3 | ...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.29 |   70.58 |   58.77 |                              
  bytes.ts                  |     7.4 |    71.42 |    12.5 |     7.4 | 13-64                        
  hex.ts                    |   82.05 |     62.5 |      80 |   82.35 | 41-45,50,58                  
  int.ts                    |       0 |        0 |       0 |       0 |                              
  str.ts                    |   85.71 |    83.33 |   66.66 |   85.71 | 15                           
  varsize.ts                |      32 |    36.36 |   66.66 |      32 | 14-24,40-47                  
  writer.ts                 |       0 |        0 |       0 |       0 |                              
  writerbytes.ts            |   83.33 |    68.42 |     100 |   83.33 | 33,43,53,63,79               
  writerlength.ts           |     100 |    83.33 |     100 |     100 | 1                            
 ecash-lib/src/test         |   86.58 |    52.08 |   84.21 |   86.84 |                              
  testRunner.ts             |   86.58 |    52.08 |   84.21 |   86.84 | 73-75,87-89,112,123,166,203  
 ecash-lib/src/token        |   60.33 |    47.14 |   53.33 |   60.11 |                              
  alp.ts                    |   82.92 |    89.47 |   83.33 |   82.92 | 110-123,142                  
  common.ts                 |     100 |    83.33 |     100 |     100 | 1                            
  empp.ts                   |    92.3 |       75 |     100 |   91.66 | 12                           
  slp.ts                    |    28.2 |    13.51 |    7.69 |    28.2 | ...7,174-178,185-197,201-211 
----------------------------|---------|----------|---------|---------|------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='758']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='1256']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='229']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='467']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='126']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='216']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='738']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='1225']
##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
Fabien planned changes to this revision.Nov 25 2024, 10:27

It happens that electrum is over tcp, not http

Fabien edited the test plan for this revision. (Show Details)

Use JSON RPC over TCP

Tail of the build log:

  File "/work/abc-ci-builds/ecash-lib-integration-tests/test/functional/test_runner.py", line 362, in main
    os.makedirs(tmpdir)
  File "<frozen os>", line 225, in makedirs
FileExistsError: [Errno 17] File exists: '/work/abc-ci-builds/ecash-lib-integration-tests/test/tmp/test_runner_₿₵_🏃_20241125_213630'
Test runner completed with code 1
----------------------------|---------|----------|---------|---------|------------------------------
File                        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
----------------------------|---------|----------|---------|---------|------------------------------
All files                   |   60.35 |    49.03 |   58.33 |   60.24 |                              
 ecash-lib                  |       0 |        0 |       0 |       0 |                              
  eslint.config.js          |       0 |        0 |       0 |       0 |                              
 ecash-lib/src              |   75.79 |    56.13 |   76.25 |   75.47 |                              
  consts.ts                 |       0 |      100 |     100 |       0 | 6-8                          
  ecc.ts                    |   57.14 |    83.33 |      40 |   57.14 | 23-31                        
  hash.ts                   |   88.88 |    83.33 |      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                     |      40 |    44.44 |   66.66 |      40 | ...4,107,109,117-122,133-161 
  opcode.ts                 |     100 |    83.33 |     100 |     100 | 1                            
  script.ts                 |   52.63 |    38.09 |      60 |    50.9 | ...4-135,146,156,166,188-199 
  sigHashType.ts            |   77.77 |       44 |   85.71 |   77.77 | 26-38                        
  tx.ts                     |   93.47 |    79.16 |    90.9 |   93.18 | 123-125                      
  txBuilder.ts              |   80.89 |    59.25 |   84.61 |   80.23 | ...1,154,173-178,183,236-240 
  unsignedTx.ts             |      75 |    57.14 |   85.71 |    75.3 | ...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.29 |   70.58 |   58.77 |                              
  bytes.ts                  |     7.4 |    71.42 |    12.5 |     7.4 | 13-64                        
  hex.ts                    |   82.05 |     62.5 |      80 |   82.35 | 41-45,50,58                  
  int.ts                    |       0 |        0 |       0 |       0 |                              
  str.ts                    |   85.71 |    83.33 |   66.66 |   85.71 | 15                           
  varsize.ts                |      32 |    36.36 |   66.66 |      32 | 14-24,40-47                  
  writer.ts                 |       0 |        0 |       0 |       0 |                              
  writerbytes.ts            |   83.33 |    68.42 |     100 |   83.33 | 33,43,53,63,79               
  writerlength.ts           |     100 |    83.33 |     100 |     100 | 1                            
 ecash-lib/src/test         |   86.58 |    52.08 |   84.21 |   86.84 |                              
  testRunner.ts             |   86.58 |    52.08 |   84.21 |   86.84 | 73-75,87-89,112,123,166,203  
 ecash-lib/src/token        |   60.33 |    47.14 |   53.33 |   60.11 |                              
  alp.ts                    |   82.92 |    89.47 |   83.33 |   82.92 | 110-123,142                  
  common.ts                 |     100 |    83.33 |     100 |     100 | 1                            
  empp.ts                   |    92.3 |       75 |     100 |   91.66 | 12                           
  slp.ts                    |    28.2 |    13.51 |    7.69 |    28.2 | ...7,174-178,185-197,201-211 
----------------------------|---------|----------|---------|---------|------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='758']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='1256']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='229']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='467']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='126']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='216']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='738']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='1225']
##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
Fabien planned changes to this revision.Nov 26 2024, 13:36

This is a bit clumsy but seems to work;

diff --git a/electrum/electrumabc/interface.py b/electrum/electrumabc/interface.py
index b1198bb889..42e6e23193 100644
--- a/electrum/electrumabc/interface.py
+++ b/electrum/electrumabc/interface.py
@@ -341,17 +341,40 @@ class TcpConnection(threading.Thread, PrintError):


 def parse_json(message):
-    # Try to decode after a closing bracket is found
-    n = message.find(b"}")
-    if n == -1:
+    if not message:
         return None, message
+
+    # Fulcrum uses newlines to delimit JSON objects
+    n = message.find(b"\n")
+    if n == -1:
+        # Try counting open braces as an alternative method (chronik sends a single
+        # json object per reply)
+        n_open_braces = 0
+        found_first = False
+        for n, c in enumerate(message):
+            c = bytes([c])
+            if c == b"{":
+                found_first = True
+                n_open_braces += 1
+            if c == b"}":
+                n_open_braces -= 1
+            if found_first and n_open_braces == 0:
+                # we closed the last brace
+                break
+        # There should be at least one loop iteration because we checked that message
+        # is not empty.
+        assert n != -1
+
+        if n == len(message) - 1 and (c != b"}" or n_open_braces !=0) :
+            return None, message
+
     try:
         # Loads ignore any whitespace char including newlines. This is important
         # because Fulcrum uses a newline as a delimiter
         j = json.loads(message[0 : n + 1].decode("utf8"))
     except Exception:
-        # We didn't find the matching closing bracket yet, continue
-        return None, message
+        # just consume the line and ignore error.
+        j = None
     return j, message[n + 1 :]

Tested via

echo '{"jsonrpc": "2.0", "method": "server.ping", "id": "test"}' | nc 127.0.0.1 50001

and the script from D17187

pierre@simak:~/dev/bitcoin-abc/electrum/scripts$ python ping.py --server 127.0.0.1:50001:t -v
|  0.009| |00| [127.0.0.1] connected
Request: ('server.ping', [], 0)
Response: {'id': 0, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 1)
Response: {'id': 1, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 2)
Response: {'id': 2, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 3)
Response: {'id': 3, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 4)
Response: {'id': 4, 'jsonrpc': '2.0', 'result': None}

pierre@simak:~/dev/bitcoin-abc/electrum/scripts$ python ping.py  -v
|  1.560| |00| [electrum.bitcoinabc.org] connected
Request: ('server.ping', [], 0)
Response: {'id': 0, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 1)
Response: {'id': 1, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 2)
Response: {'id': 2, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 3)
Response: {'id': 3, 'jsonrpc': '2.0', 'result': None}

Request: ('server.ping', [], 4)
Response: {'id': 4, 'jsonrpc': '2.0', 'result': None}

An optional codec for the server has been upstreamed (https://github.com/karyontech/karyon/pull/10) and we can leverage this to add the weird \n that fulcrum expects.
This makes it compatible with electrum with no parsing change required (but we still need to send the jsonrpc field which is required by the standard).

Fabien planned changes to this revision.Wed, Dec 4, 20:12
Fabien requested review of this revision.
chronik/chronik-http/src/electrum_codec.rs
27 ↗(On Diff #51391)

I would add a comment here mentioning that this newline adheres to Electrum's jsonrpc behavior and is used in the client for payload delineation.

tobias_ruck added a subscriber: tobias_ruck.
tobias_ruck added inline comments.
chronik/chronik-http/src/electrum.rs
18 ↗(On Diff #51391)

I converged on doing it like this because VSCode get's really confused (move the use from below into here)

82 ↗(On Diff #51391)
84 ↗(On Diff #51391)

built-in in karyon? or std?

86 ↗(On Diff #51391)

what's the plan for supporting ws here? should there be another config option?

103 ↗(On Diff #51391)

Use https://docs.rs/futures/latest/futures/future/fn.pending.html or similar.

Not that I'm worried the timeout might be reached.

105 ↗(On Diff #51391)
122 ↗(On Diff #51391)

pretty sure a normal function would work too; might be difficult with methods (might be an idea to again have fn handle_... instead—but maybe it works well with self.... too

chronik/chronik-http/src/electrum_codec.rs
11 ↗(On Diff #51391)

idk why karyon adds the {} for JsonCodec, but we don't do that here

21 ↗(On Diff #51391)

or so

27 ↗(On Diff #51391)

I think this is much clearer

41 ↗(On Diff #51391)

is the ref really needed? my internal brain compiler says no but it's not very reliable

chronik/chronik-lib/src/bridge.rs
164–165 ↗(On Diff #51391)

maybe this works also

This revision now requires changes to proceed.Wed, Dec 4, 21:45
chronik/chronik-http/src/electrum.rs
122 ↗(On Diff #51391)

(can be left as todo for now)

Fabien marked 13 inline comments as done.Thu, Dec 5, 10:36
Fabien added inline comments.
chronik/chronik-http/src/electrum.rs
82 ↗(On Diff #51391)

This returns a Box not a Result

86 ↗(On Diff #51391)

For tls we could reuse the electrum scheme: <host>:<port>:<t|s> where t stands for tcp and s for tls. For ws we can either have another option or extend this scheme with some more letters for ws and wss. Since electrum abc doesn't use ws it's a lower priority (tls first).

105 ↗(On Diff #51391)

No idea how to specify the return type of an async move block (if possible at all) so it has to be explicit here.

122 ↗(On Diff #51391)

Keeping the todo for now, there are a few options to explore and this doesn't really impact this diff which is mostly a PoC

chronik/chronik-http/src/electrum_codec.rs
27 ↗(On Diff #51391)

I tryed to avoid searching for the correct syntax here :) thanks

Fabien marked 5 inline comments as done.

Rebase, feedback

tobias_ruck added inline comments.
chronik/chronik-http/src/electrum.rs
23 ↗(On Diff #51404)

You don't need to redefine ChronikIndexerRef, you can just import it from server.rs, or move it to a common file.

25 ↗(On Diff #51404)

dito

103 ↗(On Diff #51404)

I assume the let () = is necessary, but not sure why?

86 ↗(On Diff #51391)

cool

This revision now requires changes to proceed.Thu, Dec 5, 18:05
chronik/chronik-http/src/electrum.rs
103 ↗(On Diff #51404)

It's needed to infer the type

Fabien marked 2 inline comments as done.

Sanitize the imports

It works with only a minor change in Electrum ABC code

chronik/chronik-http/src/electrum_codec.rs
25

Electrum expects or Fulcrum provides

This revision is now accepted and ready to land.Thu, Dec 5, 23:59
PiRK added a task: Restricted Maniphest Task.Fri, Dec 13, 13:34