This diff adds the API to run the Chronik Electrum JSON RPC server over TLS.
Details
Details
- Reviewers
PiRK tobias_ruck - Group Reviewers
Restricted Project - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- rABC67384414e3f9: [chronik] Add support for TLS to the electrum server
# Check it starts with TCP and default port as before ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1 # Check it starts with TCP and port 50001 ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001 ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001:t # Check it returns a meaningful error ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001: ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001:q # unknown protocol ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001:s # missing cert and key
Generate a self signed certificate with:
openssl req -nodes -new -x509 -keyout server.key -out server.pem
Then
./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001:s -chronikelectrumcert=server.pem # require both cert and key ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001:s -chronikelectrumprivkey=server.key # require both cert and key # No error, tested ping via a custom ping.py script that uses electrum functions ./src/bitcoind -regtest -chronik -chronikelectrumbind=127.0.0.1:50001:s -chronikelectrumcert=server.pem -chronikelectrumprivkey=server.key
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- chronik_electrum_tls
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Event Timeline
| chronik/chronik-lib/src/bridge.rs | ||
|---|---|---|
| 51 ↗ | (On Diff #51518) | Those doc line seem very redundant when the error message and the error names are already very descriptive. I wonder if we could somehow disable the linter rule that forces us to add them. Here you used 3 times the same line with just a minor difference in how you capitalize "electrum". |
| chronik/chronik-lib/src/bridge.rs | ||
|---|---|---|
| 51 ↗ | (On Diff #51518) | Don't forget that the comment will appear in the doc but not the error string |
| chronik/chronik-http/src/electrum.rs | ||
|---|---|---|
| 57–58 ↗ | (On Diff #51525) | this is more a personal style, to make it clear it's a path to a file, not the content of the file (it wasn't immediately clear when I first read it for example) |
| 85 ↗ | (On Diff #51525) | |
| 99 ↗ | (On Diff #51525) | |
| 140 ↗ | (On Diff #51525) | then you don't have to use all the confusing .0 and .1 |
| 145 ↗ | (On Diff #51525) | |
| 172 ↗ | (On Diff #51525) | I don't think the clone is necessary (it expects impl AsRef<Path>) |
| 183 ↗ | (On Diff #51525) | dito |
| chronik/chronik-lib/src/bridge.rs | ||
| 57 ↗ | (On Diff #51525) | is it possible to use char here consistently? That way it's UTF-8 safe, but more importantly IMO it's more readable since u8 often is more for numbers not for chars |
| 239–242 ↗ | (On Diff #51525) | |
| chronik/chronik-http/src/electrum.rs | ||
|---|---|---|
| 57–58 ↗ | (On Diff #51525) | fair enough |
| 140 ↗ | (On Diff #51525) | TIL, great |
| 172 ↗ | (On Diff #51525) | Good catch |
| chronik/chronik-lib/src/bridge.rs | ||
| 57 ↗ | (On Diff #51525) | I considered this, but really what we want here is a single ascii char, and we need the type to be C++ compatible (because it's parsed from the node options). In the end I found this was simple enough to use u8 for this stuff and it avoids converting to/from char assuming the encoding. It also makes the error show the non printable chars in a readable way. |
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.