Details

Reviewers

tobias_ruck
Fabien

Group Reviewers

Restricted Project

Summary

Add endpoint to in-node chronik server to deliver version

Notes:

Necessary to publish a beta-rc version of chronik-client for this diff in order for the apps/examples unit tests to pass. When the diff is approved, an updated version of chronik-client will be published as the latest
This new endpoint is not added to the chronik-client docs as it is only supported by in-node chronik, which chronik-client does not yet really support. This endpoint is being added as a pre-req to supporting in-node chronik with the chronik-client lib.

Test Plan

cd /bitcoin-abc/
mkdir build && cd build/
cmake .. -GNinja -DBUILD_BITCOIN_CHRONIK=on
ninja

Modify bitcoin.conf to include

chronik=1
regtest=1

Run regtest instance with
./src/bitcoind

Note your chronik URL in the logs, default should be http://127.0.0.1:18442

cd /bitcoin-abc/apps/examples
npm run getChronikServerInfo

output should be

chronikServerInfo { version: '0.1.0' }
Connected to chronik instance running version 0.1.0

In apps/examples, run npm test

Diff Detail

Repository

rABC Bitcoin ABC

Branch

chronik-add-version-endpoint

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25439
Build 50459: Build Diff	build-chronik
Build 50458: arc lint + arc unit

Event Timeline

bytesofman created this revision.Oct 13 2023, 18:22

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 13 2023, 18:22

bytesofman requested review of this revision.Oct 13 2023, 18:22

Harbormaster completed remote builds in B25350: Diff 42672.Oct 13 2023, 18:22

Endpoint should be quite simple after learning rust. However there are no other "just give them hardcoded text" endpoints so I'm not sure I can successfully do this just by copy pasting.

lint

diff is a stub

chronik/chronik-http/src/server.rs
175 ↗	(On Diff #42673)	not sure we need to involve `indexer` here as it is not being queried. also not sure how to refactor without it
179 ↗	(On Diff #42673)	this is a complete guess

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

Tail of the build log:

   Compiling num_cpus v1.16.0
   Compiling prost-build v0.11.9
   Compiling socket2 v0.5.3
   Compiling getrandom v0.2.10
   Compiling rand_core v0.6.4
   Compiling cxx-build v1.0.106
   Compiling tokio v1.32.0
   Compiling cexpr v0.6.0
   Compiling rand_chacha v0.3.1
   Compiling idna v0.4.0
   Compiling rand v0.8.5
   Compiling url v2.4.0
   Compiling ryu v1.0.15
   Compiling link-cplusplus v1.0.9
   Compiling backtrace v0.3.68
   Compiling cxx v1.0.106
   Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
   Compiling heapless v0.7.16
   Compiling socket2 v0.4.9
   Compiling bindgen v0.65.1
   Compiling tungstenite v0.20.0
   Compiling want v0.3.1
   Compiling pin-project v1.1.3
   Compiling mime v0.3.17
   Compiling postcard v1.0.6
   Compiling httpdate v1.0.3
   Compiling stable-eyre v0.2.2
   Compiling axum-core v0.3.4
   Compiling serde_json v1.0.105
   Compiling abc-rust-error v0.1.0 (/work/chronik/abc-rust-error)
   Compiling serde_urlencoded v0.7.1
   Compiling futures-executor v0.3.28
   Compiling serde_path_to_error v0.1.14
   Compiling rand v0.4.6
   Compiling sync_wrapper v0.1.2
error: failed to run custom build command for `chronik-proto v0.1.0 (/work/chronik/chronik-proto)`

Caused by:
  process didn't exit successfully: `/work/abc-ci-builds/build-chronik/cargo/build/debug/build/chronik-proto-3219e0c5b9f908f8/build-script-build` (exit status: 1)
  --- stderr
  Error: Custom { kind: Other, error: "protoc failed: chronik.proto:32:13: Expected field name.\n" }
warning: build failed, waiting for other jobs to finish...
[6/6] cd /work && /usr/bin/cmake -E env CARGO_TARGET_DIR="/work/abc-ci-builds/build-chronik/cargo/build" CARGO_BUILD_RUSTC="/root/.rustup/toolchains/1.72.0-x86_64-unknown-linux-gnu/bin/rustc" CARGO_BUILD_RUSTDOC="/root/.rustup/toolchains/1.72.0-x86_64-unknown-linux-gnu/bin/rustdoc" /root/.rustup/toolchains/1.72.0-x86_64-unknown-linux-gnu/bin/cargo --locked clippy --package abc-rust-* -- -D warnings
warning: some crates are on edition 2021 which defaults to `resolver = "2"`, but virtual workspaces default to `resolver = "1"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on build directory
   Compiling libc v0.2.147
    Checking memchr v2.5.0
    Checking object v0.31.1
   Compiling cc v1.0.83
   Compiling backtrace v0.3.68
    Checking stable-eyre v0.2.2
    Checking abc-rust-error v0.1.0 (/work/chronik/abc-rust-error)
    Finished dev [unoptimized + debuginfo] target(s) in 2m 05s
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Harbormaster completed remote builds in B25351: Diff 42673.Oct 13 2023, 18:28

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

Tail of the build log:

    Checking mio v0.8.8
    Checking num_cpus v1.16.0
   Compiling jobserver v0.1.26
   Compiling clang-sys v1.6.1
    Checking getrandom v0.2.10
   Compiling which v4.4.0
   Compiling cc v1.0.83
    Checking tokio v1.32.0
    Checking rand_core v0.6.4
    Checking idna v0.4.0
    Checking rand_chacha v0.3.1
    Checking ryu v1.0.15
    Checking tower-layer v0.3.2
    Checking try-lock v0.2.4
    Checking rand v0.8.5
    Checking data-encoding v2.4.0
    Checking want v0.3.1
    Checking socket2 v0.4.9
   Compiling cxx-build v1.0.106
   Compiling prost-build v0.11.9
    Checking url v2.4.0
   Compiling cexpr v0.6.0
    Checking pin-project v1.1.3
    Checking chronik-util v0.1.0 (/work/chronik/chronik-util)
    Checking tungstenite v0.20.0
    Checking mime v0.3.17
    Checking heapless v0.7.16
    Checking httpdate v1.0.3
    Checking seahash v4.1.0
    Checking serde_path_to_error v0.1.14
    Checking axum-core v0.3.4
    Checking futures-executor v0.3.28
    Checking serde_urlencoded v0.7.1
    Checking serde_json v1.0.105
   Compiling link-cplusplus v1.0.9
   Compiling backtrace v0.3.68
   Compiling cxx v1.0.106
    Checking matchit v0.7.2
    Checking postcard v1.0.6
    Checking base64 v0.21.2
    Checking sync_wrapper v0.1.2
   Compiling bindgen v0.65.1
    Checking futures v0.3.28
   Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
    Checking stable-eyre v0.2.2
    Checking abc-rust-error v0.1.0 (/work/chronik/abc-rust-error)
   Compiling chronik-bridge v0.1.0 (/work/chronik/chronik-bridge)
   Compiling chronik-lib v0.1.0 (/work/chronik/chronik-lib)
    Checking tokio-tungstenite v0.20.0
    Checking tower v0.4.13
    Checking hyper v0.14.27
error: failed to run custom build command for `chronik-proto v0.1.0 (/work/chronik/chronik-proto)`

Caused by:
  process didn't exit successfully: `/work/abc-ci-builds/build-chronik/cargo/build/debug/build/chronik-proto-169ebda0c6afb607/build-script-build` (exit status: 1)
  --- stderr
  Error: Custom { kind: Other, error: "protoc failed: chronik.proto:32:13: Expected field name.\n" }
warning: build failed, waiting for other jobs to finish...
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

tobias_ruck requested changes to this revision.Oct 24 2023, 13:21

tobias_ruck added a subscriber: tobias_ruck.

tobias_ruck added inline comments.

chronik/chronik-http/src/server.rs
175 ↗	(On Diff #42673)	You should be able to just delete the line if its not needed, see the examples here: https://docs.rs/axum/latest/axum/#responses
179 ↗	(On Diff #42673)	`?` is to unwrap a Result, you don't have to use it here
chronik/chronik-proto/proto/chronik.proto
32 ↗	(On Diff #42673)	add a type

Add type, remove unused params for handle_chronik_info

Harbormaster completed remote builds in B25423: Diff 42763.Oct 24 2023, 20:08

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

Tail of the build log:

    |
178 |     Ok(Protobuf({ version }))
    |                 ^^       ^^
    |
note: the lint level is defined here
   --> chronik/chronik-http/src/lib.rs:7:1
    |
7   | / abc_rust_lint::lint! {
8   | |     pub mod error;
9   | |     pub mod handlers;
10  | |     pub mod parse;
...   |
14  | |     pub mod ws;
15  | | }
    | |_^
    = note: `#[warn(unused_braces)]` implied by `#[warn(unused)]`
    = note: this warning originates in the macro `abc_rust_lint::lint` (in Nightly builds, run with -Z macro-backtrace for more info)
help: remove these braces
    |
178 -     Ok(Protobuf({ version }))
178 +     Ok(Protobuf(version))
    |

error[E0308]: mismatched types
   --> chronik/chronik-http/src/server.rs:178:19
    |
175 | ) -> Result<Protobuf<proto::ChronikInfo>, ReportError> {
    |      ------------------------------------------------- expected `ChronikInfo` because of return type
...
178 |     Ok(Protobuf({ version }))
    |                   ^^^^^^^ expected `ChronikInfo`, found `&str`

error[E0277]: the trait bound `fn() -> impl futures::Future<Output = Result<Protobuf<ChronikInfo>, ReportError>> {handle_chronik_info}: Handler<_, _, _>` is not satisfied
   --> chronik/chronik-http/src/server.rs:134:50
    |
134 |             .route("/chronik-info", routing::get(handle_chronik_info))
    |                                     ------------ ^^^^^^^^^^^^^^^^^^^ the trait `Handler<_, _, _>` is not implemented for fn item `fn() -> impl futures::Future<Output = Result<Protobuf<ChronikInfo>, ReportError>> {handle_chronik_info}`
    |                                     |
    |                                     required by a bound introduced by this call
    |
    = help: the following other types implement trait `Handler<T, S, B>`:
              <Layered<L, H, T, S, B, B2> as Handler<T, S, B2>>
              <MethodRouter<S, B> as Handler<(), S, B>>
note: required by a bound in `axum::routing::get`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-0.6.20/src/routing/method_routing.rs:403:1
    |
403 | top_level_handler_fn!(get, GET);
    | ^^^^^^^^^^^^^^^^^^^^^^---^^^^^^
    | |                     |
    | |                     required by a bound in this function
    | required by this bound in `get`
    = note: this error originates in the macro `top_level_handler_fn` (in Nightly builds, run with -Z macro-backtrace for more info)

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
warning: `chronik-http` (lib) generated 1 warning
error: could not compile `chronik-http` (lib) due to 2 previous errors; 1 warning emitted
warning: build failed, waiting for other jobs to finish...
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Getting cargo build to work

Harbormaster completed remote builds in B25424: Diff 42764.Oct 24 2023, 22:15

Fabien requested changes to this revision.Oct 25 2023, 08:26

Fabien added a subscriber: Fabien.

Fabien added inline comments.

chronik/chronik-http/src/server.rs
176 ↗	(On Diff #42764)	This is worth 3 seconds of googling: https://stackoverflow.com/questions/27840394/how-can-a-rust-program-access-metadata-from-its-cargo-package
chronik/chronik-proto/proto/chronik.proto
32 ↗	(On Diff #42764)	We can use semantic versioning and have the version be an integer, saving some precious bytes e.g. version 1.2.3 could be 0x01020300

This revision now requires changes to proceed.Oct 25 2023, 08:26

Get version from env

chronik/chronik-proto/proto/chronik.proto
32 ↗	(On Diff #42764)	I think keeping it a string is worth the byte tradeoff. It's conceivable we will have live servers, or other users may deploy live servers, where the version does not necessarily follow semantic versioning convention (e.g. maybe someone has `1.2.3my-chronik-custom`). In this case, we get a lot of value from the endpoint in knowing that this is a non-standard version. It's a string in `cargo.toml` and I think it's best to keep this typing.

Harbormaster completed remote builds in B25439: Diff 42793.Oct 25 2023, 17:29

while this might work -- need to update chronik-client to parse this, and need to be do this successfully to really test this feature.

so -- will add this to chronik-client and apps/examples.

Modify chronik-client and examples repo to support new API endpoint

bytesofman edited the summary of this revision. (Show Details)Oct 25 2023, 20:45

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

bytesofman added inline comments.Oct 25 2023, 20:53

modules/chronik-client/chronik.ts
444 ↗	(On Diff #42796)	@tobias_ruck use of `10` here is arbitrary. Does a fixed value need to be used? Following convention of other methods in this file.

Add unit test for this endpoint in chronik-client (for failing as expected with nng version)

Harbormaster completed remote builds in B25440: Diff 42796.Oct 25 2023, 20:58

Harbormaster completed remote builds in B25441: Diff 42797.Oct 25 2023, 21:16

In D14640#329887, @bytesofman wrote:

while this might work -- need to update chronik-client to parse this, and need to be do this successfully to really test this feature.

so -- will add this to chronik-client and apps/examples.

You don't need chronik-client to test this, and in fact you'd better do it in another diff, this is large enough already.
We have functional tests and a ChronikClient python class in the test framework to help with that, look at what the other tests do.

This revision now requires changes to proceed.Oct 26 2023, 08:57

You don't need chronik-client to test this, and in fact you'd better do it in another diff, this is large enough already.

Agree with @Fabien; my other points are still valid for that other diff

modules/chronik-client/chronik.ts
444 ↗	(On Diff #42796)	This must be auto generated, please use the `build-proto` script
modules/chronik-client/index.ts
78 ↗	(On Diff #42796)	You should name it the same way as the HTTP endpoint: `chronikInfo`

Will use this as a template to rebuild in a series of diffs

implement endpoint in chronik with py test
implement in chronik-client
implement in examples

[chronik] Add endpoint to query chronik version
AbandonedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 42793

chronik/chronik-http/src/server.rs

chronik/chronik-proto/proto/chronik.proto

[chronik] Add endpoint to query chronik versionAbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 42793

chronik/chronik-http/src/server.rs

chronik/chronik-proto/proto/chronik.proto

[chronik] Add endpoint to query chronik version
AbandonedPublic
Actions

Revision Contents
Changeset List