[chronik] Add failover_proxy to bitcoinsuite-chronik-client
Changes PlannedPublic
Actions

Authored by hazzarust on Thu, Apr 3, 09:39.

Details

Reviewers

Fabien
tobias_ruck

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

This code implements a fault-tolerant proxy client for connecting to Chronik blockchain indexers. Key features include:

Automatic failover between multiple Chronik endpoints
Conversion between HTTP/HTTPS URLs and their WebSocket equivalents
HTTP request handling (GET/POST) with error management
WebSocket connection establishment and validation
Subscription management for scripts, Lokad IDs, tokens, and blocks
Automatic reconnection with retention of previous subscriptions
Connection timeouts and status tracking
Error handling for server indexing states and connection failures

The client maintains state about working endpoints and can cycle through multiple servers when the current one becomes unavailable.

Test Plan

Please set BUILD_DIR env to export BUILD_DIR="/path/to/build_dir
UNIX: ./contrib/teamcity/build-configurations.py build-bitcoinsuite-chronik-client

Diff Detail

Repository

rABC Bitcoin ABC

Branch

fallover

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32921
Build 65326: Build Diff	build-chronik-plugins · ecash-agora-integration-tests · chronik-client-integration-tests · ecash-lib-integration-tests · build-bitcoinsuite-chronik-client · build-explorer · build-chronik
Build 65325: arc lint + arc unit

Event Timeline

hazzarust requested review of this revision.Thu, Apr 3, 09:39

hazzarust created this revision.

lint

Harbormaster completed remote builds in B32880: Diff 53351.Thu, Apr 3, 10:01

Harbormaster completed remote builds in B32881: Diff 53352.Thu, Apr 3, 10:06

Build Bitcoin ABC Diffs / Diff Testing (build-bitcoinsuite-chronik-client) failed.

Tail of the build log:

running 1 test
test test_chronik_version ... ok

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

     Running tests/failover_proxy_test.rs (/work/target/debug/deps/failover_proxy_test-c2934d124792a348)

running 8 tests
test test_append_ws_urls_combines_mixed_urls ... ok
test test_append_ws_urls_empty_input ... ok
test test_append_ws_urls_combines_valid_urls ... ok
test test_append_ws_urls_invalid_url ... ok
test test_derive_endpoint_index_default_working_index ... ok
test test_derive_endpoint_index_working_index_3 ... ok
test test_invalid_script_subscriptions ... ok
test test_script_subscription_validation ... ok

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

     Running tests/test_chronik_client.rs (/work/target/debug/deps/test_chronik_client-11e41fdb35201110)

running 14 tests
test test_slpv1_token ... ok
test test_broadcast_txs ... ok
test test_chronik_info ... ok
test test_raw_tx_missing ... ok
test test_broadcast_tx ... ok
test test_blocks ... ok
test test_raw_tx ... ok
test test_tx ... ok
test test_tx_missing ... ok
test test_blockchain_info ... ok
test test_history ... ok
test test_block ... ok
test test_block_txs ... FAILED
test test_slpv2_token ... ok

failures:

---- test_block_txs stdout ----
Error: HTTP request error

Caused by:
   0: error sending request for url (https://chronik.e.cash/block-txs/00000000000053807791091d70e691abff37fc4f8196df306ade8fd8fc40b9e8?page=0): error trying to connect: dns error: failed to lookup address information: Name or service not known
   1: error trying to connect: dns error: failed to lookup address information: Name or service not known
   2: dns error: failed to lookup address information: Name or service not known
   3: failed to lookup address information: Name or service not known

Location:
    /work/modules/bitcoinsuite-chronik-client/src/lib.rs:308:14


failures:
    test_block_txs

test result: FAILED. 13 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 4.15s

error: test failed, to rerun pass `-p bitcoinsuite-chronik-client --test test_chronik_client`
Build build-bitcoinsuite-chronik-client failed with exit code 101

Please dont review yet,

If it's not ready to review please don't put it in the review queue

modules/bitcoinsuite-chronik-client/src/failover_proxy.rs
1 ↗	(On Diff #53352)
modules/bitcoinsuite-chronik-client/src/handler.rs
78 ↗	(On Diff #53352)	unrelated ?
modules/bitcoinsuite-chronik-client/src/hex.rs
1 ↗	(On Diff #53352)
9 ↗	(On Diff #53352)	what is the 4B in the name ? 4 bits ?
107 ↗	(On Diff #53352)	You can introduce (and make use of) this file in its own diff, it's unrelated to the failover proxy
modules/bitcoinsuite-chronik-client/src/lib.rs
6 ↗	(On Diff #53352)	no longer needed ?
320 ↗	(On Diff #53352)	can we just use crate::hex and avoid all the boilerplate ?

This revision now requires changes to proceed.Fri, Apr 4, 07:50

hazzarust added inline comments.Sat, Apr 5, 13:32

modules/bitcoinsuite-chronik-client/src/failover_proxy.rs
1 ↗	(On Diff #53352)	I'm so dumb its painful
modules/bitcoinsuite-chronik-client/src/hex.rs
9 ↗	(On Diff #53352)	Yes Ill make this explicit
107 ↗	(On Diff #53352)	This is used in lib.rs. Would you like me to make the lib.rs changes and the file above in a seperate diff?

Removed hex.rs (will be its own seperate diff in future), removed hex boilerplate, removed non diff specific changes

Harbormaster completed remote builds in B32921: Diff 53416.Tue, Apr 8, 12:05

tobias_ruck requested changes to this revision.Tue, Apr 8, 14:15

tobias_ruck added a subscriber: tobias_ruck.

tobias_ruck added inline comments.

modules/bitcoinsuite-chronik-client/Cargo.toml
22	undo this
modules/bitcoinsuite-chronik-client/src/failover_proxy.rs
26	add a Debug derive
27	I think this is more rusty
33	shadowing is more idiomatic here (TS cant do that hehehe)
modules/bitcoinsuite-chronik-client/src/lib.rs
6	I think it's clearer to just use `hex::decode` and `hex::encode`, that way we know it's hex
344	so undo this

This revision now requires changes to proceed.Tue, Apr 8, 14:15

Made url idiomatic hehe, removed hex import ::* to make more readable, made the other changes suggested

Harbormaster completed remote builds in B32928: Diff 53427.Wed, Apr 9, 12:00

Apart from a few nits and bad tests this code looks good.

Macro however:

This is not adding the failover proxy feature. This does:

Add support for ws subcription (with no test)
Add the failover proxy feature

As you can see there are 2 distinct things that this diff achieves and this should be split. First add ws support with an integration test (you can do one by one, 1 diff each e.g. 1 diff for block ws with test, then 1 diff for script ws with test, etc.
Once complete you can add the failover proxy on top of that.

modules/bitcoinsuite-chronik-client/src/failover_proxy.rs
153	I wonder if this is actually reachable
modules/bitcoinsuite-chronik-client/src/lib.rs
281 ↗	(On Diff #53427)	don't mix random changes like this in a diff. If it's worth it make it its own diff
modules/bitcoinsuite-chronik-client/tests/failover_proxy_test.rs
94 ↗	(On Diff #53427)	empty_input ?
153 ↗	(On Diff #53427)	This is not testing script subscription validation. Also there test for rejection so your code could as well always accept anything without your test catching it.
203 ↗	(On Diff #53427)	This is testing exactly nothing but the validation function you just created for this test. Nothing like script subscriptions.