Page MenuHomePhabricator
Differential D16508

[chronik] add a block-header endpoint
ClosedPublic
Actions

Authored by PiRK on Jul 19 2024, 15:59.

Details

Reviewers
Fabien
tobias_ruck
Group Reviewers
Restricted Project
Commits
rABC32e93a4e2fb3: [chronik] add a block-header endpoint
Summary

Fetch the header directly from the in-memory block index.

For now only the raw header data is returned. In a future diff I plan to add a cp_height query parameter that causes the endpoint to also return the checkpoint's merkle tree data (similar to how electrumx does it).

Test Plan

With chronik
ninja all check-all

Diff Detail

Event Timeline

PiRK created this revision.Jul 19 2024, 15:59
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 19 2024, 15:59
Harbormaster completed remote builds in B29638: Diff 48747.Jul 19 2024, 16:03
teamcity added a comment.Jul 19 2024, 16:03
Build Bitcoin ABC Diffs / Diff Testing (chronik-client-integration-tests) failed.

Tail of the build log:

[239/581] Building CXX object src/CMakeFiles/script.dir/script/standard.cpp.o
[240/581] Building CXX object src/CMakeFiles/common.dir/consensus/merkle.cpp.o
[241/581] Building CXX object src/CMakeFiles/common.dir/common/system.cpp.o
[242/581] Building CXX object src/CMakeFiles/common.dir/common/bloom.cpp.o
[243/581] Building CXX object src/CMakeFiles/common.dir/config.cpp.o
[244/581] Building CXX object src/CMakeFiles/common.dir/chainparams.cpp.o
[245/581] Building CXX object src/CMakeFiles/common.dir/cashaddrenc.cpp.o
[246/581] Building CXX object src/CMakeFiles/common.dir/eventloop.cpp.o
[247/581] Building CXX object src/CMakeFiles/script.dir/script/sign.cpp.o
[248/581] Building CXX object src/CMakeFiles/script.dir/script/interpreter.cpp.o
[249/581] Building CXX object src/CMakeFiles/common.dir/feerate.cpp.o
[250/581] Building CXX object src/CMakeFiles/common.dir/compressor.cpp.o
[251/581] Building CXX object src/CMakeFiles/script.dir/script/signingprovider.cpp.o
[252/581] Building CXX object src/CMakeFiles/common.dir/common/configfile.cpp.o
[253/581] Building CXX object src/CMakeFiles/common.dir/key.cpp.o
[254/581] Building CXX object src/CMakeFiles/common.dir/coins.cpp.o
[255/581] Building CXX object src/CMakeFiles/common.dir/key_io.cpp.o
[256/581] Building CXX object src/CMakeFiles/common.dir/merkleblock.cpp.o
[257/581] Building CXX object src/CMakeFiles/common.dir/common/args.cpp.o
[258/581] Building CXX object src/CMakeFiles/common.dir/net_permissions.cpp.o
[259/581] Building CXX object src/CMakeFiles/common.dir/kernel/chainparams.cpp.o
[260/581] Building CXX object src/CMakeFiles/common.dir/outputtype.cpp.o
[261/581] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[262/581] Building CXX object src/CMakeFiles/common.dir/policy/policy.cpp.o
[263/581] Building C object src/secp256k1/CMakeFiles/recover-bench.dir/src/bench_recover.c.o
[264/581] Building CXX object src/CMakeFiles/common.dir/primitives/block.cpp.o
[265/581] Building CXX object src/CMakeFiles/common.dir/networks/abc/chainparamsconstants.cpp.o
[266/581] Building CXX object src/CMakeFiles/common.dir/netaddress.cpp.o
[267/581] Building CXX object src/CMakeFiles/common.dir/scheduler.cpp.o
[268/581] Building C object src/secp256k1/CMakeFiles/verify-bench.dir/src/bench_verify.c.o
[269/581] Building CXX object src/CMakeFiles/common.dir/warnings.cpp.o
[270/581] Building C object src/secp256k1/CMakeFiles/sign-bench.dir/src/bench_sign.c.o
[271/581] Building CXX object src/CMakeFiles/common.dir/core_read.cpp.o
[272/581] Building CXX object src/CMakeFiles/common.dir/protocol.cpp.o
[273/581] Building CXX object src/CMakeFiles/common.dir/networks/abc/checkpoints.cpp.o
[274/581] Building CXX object src/CMakeFiles/common.dir/netbase.cpp.o
[275/581] Building CXX object src/CMakeFiles/common.dir/core_write.cpp.o
[276/581] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[277/581] Linking C static library src/secp256k1/libsecp256k1.a
[278/581] Linking C executable src/secp256k1/recover-bench
[279/581] Linking C executable src/secp256k1/verify-bench
[280/581] Linking C executable src/secp256k1/sign-bench
[281/581] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[282/581] Linking C executable src/secp256k1/ecmult-bench
[283/581] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[284/581] Linking C executable src/secp256k1/internal-bench
[285/581] Building CXX object src/CMakeFiles/common.dir/rpc/rawtransaction_util.cpp.o
[286/581] Building CXX object src/CMakeFiles/common.dir/psbt.cpp.o
[287/581] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[288/581] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[289/581] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[290/581] Linking CXX static library src/libcommon.a
[291/581] Linking CXX static library src/libscript.a
[292/581] Linking CXX static library src/libbitcoinconsensus.a
[293/581] Linking CXX shared library src/libbitcoinconsensus.so.0.29.9
[294/581] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[295/581] Linking CXX executable src/bitcoin-cli
[296/581] Linking CXX executable src/bitcoin-tx
ninja: build stopped: cannot make progress due to previous errors.
Build chronik-client-integration-tests failed with exit code 1
teamcity added a comment.Jul 19 2024, 16:04
Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Tail of the build log:

[239/581] Building CXX object src/CMakeFiles/script.dir/script/standard.cpp.o
[240/581] Building CXX object src/CMakeFiles/common.dir/common/system.cpp.o
[241/581] Building CXX object src/CMakeFiles/common.dir/config.cpp.o
[242/581] Building CXX object src/CMakeFiles/common.dir/common/bloom.cpp.o
[243/581] Building CXX object src/CMakeFiles/common.dir/consensus/merkle.cpp.o
[244/581] Building CXX object src/CMakeFiles/common.dir/cashaddrenc.cpp.o
[245/581] Building CXX object src/CMakeFiles/common.dir/chainparams.cpp.o
[246/581] Building CXX object src/CMakeFiles/common.dir/eventloop.cpp.o
[247/581] Building CXX object src/CMakeFiles/script.dir/script/sign.cpp.o
[248/581] Building CXX object src/CMakeFiles/script.dir/script/signingprovider.cpp.o
[249/581] Building CXX object src/CMakeFiles/script.dir/script/interpreter.cpp.o
[250/581] Building CXX object src/CMakeFiles/common.dir/feerate.cpp.o
[251/581] Building CXX object src/CMakeFiles/common.dir/common/configfile.cpp.o
[252/581] Building CXX object src/CMakeFiles/common.dir/compressor.cpp.o
[253/581] Building CXX object src/CMakeFiles/common.dir/coins.cpp.o
[254/581] Building CXX object src/CMakeFiles/common.dir/key.cpp.o
[255/581] Building CXX object src/CMakeFiles/common.dir/merkleblock.cpp.o
[256/581] Building CXX object src/CMakeFiles/common.dir/key_io.cpp.o
[257/581] Building CXX object src/CMakeFiles/common.dir/kernel/chainparams.cpp.o
[258/581] Building CXX object src/CMakeFiles/common.dir/net_permissions.cpp.o
[259/581] Building CXX object src/CMakeFiles/common.dir/common/args.cpp.o
[260/581] Building CXX object src/CMakeFiles/common.dir/outputtype.cpp.o
[261/581] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[262/581] Building CXX object src/CMakeFiles/common.dir/primitives/block.cpp.o
[263/581] Building CXX object src/CMakeFiles/common.dir/policy/policy.cpp.o
[264/581] Building C object src/secp256k1/CMakeFiles/recover-bench.dir/src/bench_recover.c.o
[265/581] Building CXX object src/CMakeFiles/common.dir/networks/abc/chainparamsconstants.cpp.o
[266/581] Building CXX object src/CMakeFiles/common.dir/netaddress.cpp.o
[267/581] Building CXX object src/CMakeFiles/common.dir/scheduler.cpp.o
[268/581] Building CXX object src/CMakeFiles/common.dir/warnings.cpp.o
[269/581] Building C object src/secp256k1/CMakeFiles/verify-bench.dir/src/bench_verify.c.o
[270/581] Building C object src/secp256k1/CMakeFiles/sign-bench.dir/src/bench_sign.c.o
[271/581] Building CXX object src/CMakeFiles/common.dir/protocol.cpp.o
[272/581] Building CXX object src/CMakeFiles/common.dir/netbase.cpp.o
[273/581] Building CXX object src/CMakeFiles/common.dir/networks/abc/checkpoints.cpp.o
[274/581] Building CXX object src/CMakeFiles/common.dir/core_read.cpp.o
[275/581] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[276/581] Linking C static library src/secp256k1/libsecp256k1.a
[277/581] Building CXX object src/CMakeFiles/common.dir/core_write.cpp.o
[278/581] Linking C executable src/secp256k1/recover-bench
[279/581] Linking C executable src/secp256k1/verify-bench
[280/581] Linking C executable src/secp256k1/sign-bench
[281/581] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[282/581] Linking C executable src/secp256k1/ecmult-bench
[283/581] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[284/581] Linking C executable src/secp256k1/internal-bench
[285/581] Building CXX object src/CMakeFiles/common.dir/rpc/rawtransaction_util.cpp.o
[286/581] Building CXX object src/CMakeFiles/common.dir/psbt.cpp.o
[287/581] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[288/581] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[289/581] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[290/581] Linking CXX static library src/libcommon.a
[291/581] Linking CXX static library src/libscript.a
[292/581] Linking CXX static library src/libbitcoinconsensus.a
[293/581] Linking CXX shared library src/libbitcoinconsensus.so.0.29.9
[294/581] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[295/581] Linking CXX executable src/bitcoin-cli
[296/581] Linking CXX executable src/bitcoin-tx
ninja: build stopped: cannot make progress due to previous errors.
Build ecash-agora-integration-tests failed with exit code 1
teamcity added a comment.Jul 19 2024, 16:04
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-integration-tests) failed.

Tail of the build log:

[239/581] Building CXX object src/CMakeFiles/script.dir/script/standard.cpp.o
[240/581] Building CXX object src/CMakeFiles/common.dir/common/system.cpp.o
[241/581] Building CXX object src/CMakeFiles/common.dir/common/bloom.cpp.o
[242/581] Building CXX object src/CMakeFiles/common.dir/consensus/merkle.cpp.o
[243/581] Building CXX object src/CMakeFiles/common.dir/config.cpp.o
[244/581] Building CXX object src/CMakeFiles/common.dir/chainparams.cpp.o
[245/581] Building CXX object src/CMakeFiles/common.dir/eventloop.cpp.o
[246/581] Building CXX object src/CMakeFiles/common.dir/cashaddrenc.cpp.o
[247/581] Building CXX object src/CMakeFiles/script.dir/script/sign.cpp.o
[248/581] Building CXX object src/CMakeFiles/common.dir/feerate.cpp.o
[249/581] Building CXX object src/CMakeFiles/script.dir/script/signingprovider.cpp.o
[250/581] Building CXX object src/CMakeFiles/common.dir/compressor.cpp.o
[251/581] Building CXX object src/CMakeFiles/script.dir/script/interpreter.cpp.o
[252/581] Building CXX object src/CMakeFiles/common.dir/common/configfile.cpp.o
[253/581] Building CXX object src/CMakeFiles/common.dir/coins.cpp.o
[254/581] Building CXX object src/CMakeFiles/common.dir/merkleblock.cpp.o
[255/581] Building CXX object src/CMakeFiles/common.dir/key.cpp.o
[256/581] Building CXX object src/CMakeFiles/common.dir/key_io.cpp.o
[257/581] Building CXX object src/CMakeFiles/common.dir/common/args.cpp.o
[258/581] Building CXX object src/CMakeFiles/common.dir/outputtype.cpp.o
[259/581] Building CXX object src/CMakeFiles/script.dir/script/descriptor.cpp.o
[260/581] Building CXX object src/CMakeFiles/common.dir/kernel/chainparams.cpp.o
[261/581] Building CXX object src/CMakeFiles/common.dir/net_permissions.cpp.o
[262/581] Building CXX object src/CMakeFiles/common.dir/policy/policy.cpp.o
[263/581] Building CXX object src/CMakeFiles/common.dir/primitives/block.cpp.o
[264/581] Building C object src/secp256k1/CMakeFiles/recover-bench.dir/src/bench_recover.c.o
[265/581] Building CXX object src/CMakeFiles/common.dir/networks/abc/chainparamsconstants.cpp.o
[266/581] Building CXX object src/CMakeFiles/common.dir/netaddress.cpp.o
[267/581] Building CXX object src/CMakeFiles/common.dir/scheduler.cpp.o
[268/581] Building CXX object src/CMakeFiles/common.dir/warnings.cpp.o
[269/581] Building C object src/secp256k1/CMakeFiles/verify-bench.dir/src/bench_verify.c.o
[270/581] Building CXX object src/CMakeFiles/common.dir/protocol.cpp.o
[271/581] Building C object src/secp256k1/CMakeFiles/sign-bench.dir/src/bench_sign.c.o
[272/581] Building CXX object src/CMakeFiles/common.dir/netbase.cpp.o
[273/581] Building CXX object src/CMakeFiles/common.dir/networks/abc/checkpoints.cpp.o
[274/581] Building C object src/secp256k1/CMakeFiles/secp256k1.dir/src/secp256k1.c.o
[275/581] Linking C static library src/secp256k1/libsecp256k1.a
[276/581] Linking C executable src/secp256k1/verify-bench
[277/581] Linking C executable src/secp256k1/recover-bench
[278/581] Linking C executable src/secp256k1/sign-bench
[279/581] Building CXX object src/CMakeFiles/common.dir/core_read.cpp.o
[280/581] Building CXX object src/CMakeFiles/common.dir/core_write.cpp.o
[281/581] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[282/581] Linking C executable src/secp256k1/ecmult-bench
[283/581] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[284/581] Linking C executable src/secp256k1/internal-bench
[285/581] Building CXX object src/CMakeFiles/common.dir/psbt.cpp.o
[286/581] Building CXX object src/CMakeFiles/common.dir/rpc/rawtransaction_util.cpp.o
[287/581] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[288/581] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[289/581] Building CXX object src/CMakeFiles/common.dir/rpc/util.cpp.o
[290/581] Linking CXX static library src/libcommon.a
[291/581] Linking CXX static library src/libscript.a
[292/581] Linking CXX static library src/libbitcoinconsensus.a
[293/581] Linking CXX shared library src/libbitcoinconsensus.so.0.29.9
[294/581] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[295/581] Linking CXX executable src/bitcoin-cli
[296/581] Linking CXX executable src/bitcoin-tx
ninja: build stopped: cannot make progress due to previous errors.
Build ecash-lib-integration-tests failed with exit code 1
teamcity added a comment.Jul 19 2024, 16:06
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Tail of the build log:

   Compiling pyo3-macros-backend v0.20.3
    Checking sync_wrapper v0.1.2
    Checking seahash v4.1.0
    Checking winnow v0.6.13
    Checking topo_sort v0.4.0
    Checking mime v0.3.17
    Checking bimap v0.6.3
    Checking axum-core v0.4.3
    Checking parking_lot v0.12.1
   Compiling prost-build v0.11.9
    Checking memoffset v0.9.1
    Checking serde_json v1.0.115
    Checking portable-atomic v1.6.0
    Checking hyper v1.2.0
    Checking tower v0.4.13
    Checking hyper-util v0.1.3
    Checking tokio-tungstenite v0.21.0
   Compiling chronik-bridge v0.1.0 (/work/chronik/chronik-bridge)
    Checking toml_edit v0.22.14
    Checking serde_urlencoded v0.7.1
    Checking chronik-plugin-common v0.1.0 (/work/chronik/chronik-plugin-common)
    Checking futures-executor v0.3.30
    Checking serde_path_to_error v0.1.16
    Checking itertools v0.13.0
    Checking matchit v0.7.3
    Checking unicode-segmentation v1.11.0
    Checking base64 v0.21.7
   Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
    Checking sync_wrapper v1.0.1
    Checking unindent v0.2.3
    Checking futures v0.3.30
    Checking chronik-plugin v0.1.0 (/work/chronik/chronik-plugin)
    Checking tower-http v0.5.2
    Checking convert_case v0.6.0
    Checking axum v0.7.5
   Compiling pyo3-macros v0.20.3
    Checking toml v0.8.14
    Checking versions v6.3.0
   Compiling librocksdb-sys v0.11.0+8.1.1
    Checking chronik-plugin-impl v0.1.0 (/work/chronik/chronik-plugin-impl)
    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[E0277]: `?` couldn't convert the error to `error::ReportError`
   --> chronik/chronik-http/src/server.rs:298:67
    |
298 |         raw_hdr: chronik_bridge::ffi::get_block_header(block_index?).to_vec(),
    |                                                                   ^ the trait `std::convert::From<cxx::exception::Exception>` is not implemented for `error::ReportError`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following other types implement trait `std::convert::From<T>`:
              <error::ReportError as std::convert::From<abc_rust_error::Report>>
              <error::ReportError as std::convert::From<server::ChronikServerError>>
    = note: required for `std::result::Result<protobuf::Protobuf<chronik_proto::proto::Header>, error::ReportError>` to implement `std::ops::FromResidual<std::result::Result<std::convert::Infallible, cxx::exception::Exception>>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `chronik-http` (lib) due to 1 previous error
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1
teamcity added a comment.Jul 19 2024, 16:06
Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.

Tail of the build log:

   Compiling portable-atomic v1.6.0
   Compiling itertools v0.13.0
   Compiling sha1 v0.10.6
   Compiling unicode-segmentation v1.11.0
   Compiling httpdate v1.0.3
   Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
   Compiling data-encoding v2.5.0
   Compiling ryu v1.0.17
   Compiling utf-8 v0.7.6
   Compiling unindent v0.2.3
   Compiling convert_case v0.6.0
   Compiling hyper v1.2.0
   Compiling tungstenite v0.21.0
   Compiling pin-project v1.1.5
   Compiling rand v0.4.6
   Compiling mime v0.3.17
   Compiling toml v0.8.14
   Compiling versions v6.3.0
   Compiling remove_dir_all v0.5.3
   Compiling sync_wrapper v0.1.2
   Compiling axum-core v0.4.3
   Compiling tempdir v0.3.7
   Compiling hyper-util v0.1.3
   Compiling pyo3-macros v0.20.3
   Compiling tokio-tungstenite v0.21.0
   Compiling tower v0.4.13
   Compiling serde_json v1.0.115
   Compiling serde_urlencoded v0.7.1
   Compiling futures-executor v0.3.30
   Compiling serde_path_to_error v0.1.16
   Compiling matchit v0.7.3
   Compiling base64 v0.21.7
   Compiling sync_wrapper v1.0.1
   Compiling futures v0.3.30
   Compiling tower-http v0.5.2
   Compiling chronik-plugin v0.1.0 (/work/chronik/chronik-plugin)
   Compiling librocksdb-sys v0.11.0+8.1.1
   Compiling axum v0.7.5
   Compiling chronik-plugin-impl v0.1.0 (/work/chronik/chronik-plugin-impl)
   Compiling rocksdb v0.21.0
   Compiling chronik-db v0.1.0 (/work/chronik/chronik-db)
   Compiling chronik-indexer v0.1.0 (/work/chronik/chronik-indexer)
   Compiling chronik-http v0.1.0 (/work/chronik/chronik-http)
error[E0277]: `?` couldn't convert the error to `ReportError`
   --> chronik/chronik-http/src/server.rs:298:67
    |
298 |         raw_hdr: chronik_bridge::ffi::get_block_header(block_index?).to_vec(),
    |                                                                   ^ the trait `From<cxx::exception::Exception>` is not implemented for `ReportError`
    |
    = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
    = help: the following other types implement trait `From<T>`:
              <ReportError as From<abc_rust_error::Report>>
              <ReportError as From<ChronikServerError>>
    = note: required for `Result<Protobuf<Header>, ReportError>` to implement `FromResidual<Result<Infallible, cxx::exception::Exception>>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `chronik-http` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1
PiRK updated this revision to Diff 48770.Jul 22 2024, 13:47

fix compilation errors

Harbormaster completed remote builds in B29656: Diff 48770.Jul 22 2024, 14:10
PiRK updated this revision to Diff 48772.Jul 22 2024, 16:32

add a functional test
TODO: figure out how to get error "404: Block not found: {i}" instead of error status 500, error: msg: "Unknown error, contact admins"

Harbormaster completed remote builds in B29658: Diff 48772.Jul 22 2024, 17:04
PiRK updated this revision to Diff 48780.Jul 23 2024, 06:39
PiRK edited the summary of this revision. (Show Details)
PiRK edited the test plan for this revision. (Show Details)

return a 404 error when querying an invalid height or hash

PiRK updated this revision to Diff 48781.Jul 23 2024, 06:44

no need to make enum HashOrHeight now that the code using it is moved to the same crate

PiRK published this revision for review.Jul 23 2024, 06:56
Harbormaster completed remote builds in B29665: Diff 48780.Jul 23 2024, 07:28
Fabien requested changes to this revision.Jul 23 2024, 08:01
Fabien added a subscriber: Fabien.
Fabien added inline comments.
chronik/chronik-bridge/src/ffi.rs
202 ↗(On Diff #48781)
chronik/chronik-cpp/chronik_bridge.cpp
205 ↗(On Diff #48781)

Despite non-intuitive this is redundant with the below operator that also check the height is within bounds. You can simplify with a comment explaining that the boundary check is performed in the operator[](int nHeight) method.
Also you can slightly reduce the scope of the cs_main lock.

chronik/chronik-proto/proto/chronik.proto
24 ↗(On Diff #48781)

Please use a more clear (sorry, longer) name: raw_header.

Also is the raw header really the data we want ? What is the use case for this (vs decoded header) ?

test/functional/chronik_header.py
65 ↗(On Diff #48781)

Nit: while COINBASE_MATURITY is indeed value 100, you're not maturing anything here, you could as well use any other number. So it's inappropriate to use the constant here.

This revision now requires changes to proceed.Jul 23 2024, 08:01
Harbormaster completed remote builds in B29666: Diff 48781.Jul 23 2024, 08:48
PiRK added inline comments.Jul 23 2024, 09:39
chronik/chronik-proto/proto/chronik.proto
24 ↗(On Diff #48781)

In Electrum the raw header is stored locally and hashed to check it connects to the next block, but we also deserialize it to get the data in a dict. And when it needs to be hashed, we actually reserialize it first from the dict instead of directly using the raw header for some stupid reason.

So I guess either way works for me, I will have to either serialize or deserialize it anyway.

PiRK updated this revision to Diff 48790.Jul 23 2024, 13:09

review

chronik/chronik-proto/proto/chronik.proto
24 ↗(On Diff #48781)

If we want to do something similar to the getblockheader RPC, we can later add a verbose parameter that returns decoded data if true, and the raw header if false.

PiRK updated this revision to Diff 48791.Jul 23 2024, 13:14

add a comment about boundary check

Harbormaster completed remote builds in B29673: Diff 48790.Jul 23 2024, 13:47
Harbormaster completed remote builds in B29675: Diff 48791.Jul 23 2024, 14:10
PiRK updated this revision to Diff 48793.Jul 23 2024, 15:15

nit: use another constant for the number of blocks, make it smaller because it does not change the validity of the test

Harbormaster completed remote builds in B29676: Diff 48793.Jul 23 2024, 15:46
PiRK added a child revision: D16526: [chronik] add a block-headers endpoint.Jul 23 2024, 15:49
Fabien requested changes to this revision.Jul 23 2024, 19:16

Otherwise LGTM

chronik/chronik-indexer/src/query/blocks.rs
283–299 ↗(On Diff #48793)

This is technically correct but a bit weird to keep the Result up to the get_block_header call, to only notice there that there might be an error. The suggestion seems more idiomatic to me.

This revision now requires changes to proceed.Jul 23 2024, 19:16
PiRK updated this revision to Diff 48817.Jul 24 2024, 08:27

review

Harbormaster completed remote builds in B29697: Diff 48817.Jul 24 2024, 09:02
tobias_ruck requested changes to this revision.Jul 24 2024, 09:28
tobias_ruck added a subscriber: tobias_ruck.

just some fanatic suggestions

chronik/chronik-indexer/src/query/blocks.rs
284 ↗(On Diff #48817)

you might want to do this and then use it below to avoid all the newlines, but no blocker

289 ↗(On Diff #48817)

Actually it seems like you could factor this map_err out to avoid the redundancy. Something like

let block_index = match {
    HashOrHeight::Hash(hash) => bridge.lookup_block_index(...),
    ....
};
let block_index = block_index.map_err(|_| BlockNotFound(hash_or_height))?;

would be the most idiomatic (uses shadowing nicely)

297 ↗(On Diff #48817)

Usually I import use chronik_bridge::ffi and then call ffi::get_..., which should avoid the newline

This revision now requires changes to proceed.Jul 24 2024, 09:28
PiRK updated this revision to Diff 48843.Jul 25 2024, 11:50

idiomaticity

Harbormaster completed remote builds in B29715: Diff 48843.Jul 25 2024, 12:16
Fabien accepted this revision.Jul 25 2024, 14:02
PiRK updated this revision to Diff 48953.Jul 31 2024, 13:55

use /block-header for the path, BlockHeader for the protobuf message and chronik_block_header.py for the functional test, for consistency with other endpoints and in case we need another type of header in the future
Discussed in private with Tobias.

PiRK updated this revision to Diff 48954.Jul 31 2024, 14:04

fix client.py

Harbormaster completed remote builds in B29803: Diff 48953.Jul 31 2024, 14:19
teamcity added a comment.Jul 31 2024, 14:19
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: chronik_block_header.py ======

------- Stdout: -------
2024-07-31T14:17:55.758000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20240731_141012/chronik_block_header_268
2024-07-31T14:17:57.923000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 147, in main
    self._run_test_internal()
  File "/work/test/functional/test_framework/test_framework.py", line 137, in _run_test_internal
    self.run_test()
  File "/work/test/functional/chronik_block_header.py", line 41, in run_test
    chronik.block_header("1234f").err(400).msg,
  File "/work/test/functional/test_framework/chronik/client.py", line 42, in err
    raise AssertionError(
AssertionError: Expected error response status 400, but got different error status 404, error: msg: "404: Not found: /header/1234f"

2024-07-31T14:17:57.974000Z TestFramework (INFO): Stopping nodes
2024-07-31T14:17:58.276000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20240731_141012/chronik_block_header_268
2024-07-31T14:17:58.276000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20240731_141012/chronik_block_header_268/test_framework.log
2024-07-31T14:17:58.276000Z TestFramework (ERROR): 
2024-07-31T14:17:58.276000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20240731_141012/chronik_block_header_268' to consolidate all logs
2024-07-31T14:17:58.276000Z TestFramework (ERROR): 
2024-07-31T14:17:58.276000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-07-31T14:17:58.276000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2024-07-31T14:17:58.277000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: chronik_block_header.py

tobias_ruck accepted this revision.Jul 31 2024, 14:20
tobias_ruck added inline comments.
chronik/chronik-http/src/server.rs
283 ↗(On Diff #48954)
This revision is now accepted and ready to land.Jul 31 2024, 14:20
teamcity added a comment.Jul 31 2024, 14:24
Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: chronik_block_header.py ======

------- Stdout: -------
2024-07-31T14:21:32.799000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-chronik-plugins/test/tmp/test_runner_₿₵_🏃_20240731_141649/chronik_block_header_268
2024-07-31T14:21:33.343000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 147, in main
    self._run_test_internal()
  File "/work/test/functional/test_framework/test_framework.py", line 137, in _run_test_internal
    self.run_test()
  File "/work/test/functional/chronik_block_header.py", line 41, in run_test
    chronik.block_header("1234f").err(400).msg,
  File "/work/test/functional/test_framework/chronik/client.py", line 42, in err
    raise AssertionError(
AssertionError: Expected error response status 400, but got different error status 404, error: msg: "404: Not found: /header/1234f"

2024-07-31T14:21:33.394000Z TestFramework (INFO): Stopping nodes
2024-07-31T14:21:33.495000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-chronik-plugins/test/tmp/test_runner_₿₵_🏃_20240731_141649/chronik_block_header_268
2024-07-31T14:21:33.495000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-chronik-plugins/test/tmp/test_runner_₿₵_🏃_20240731_141649/chronik_block_header_268/test_framework.log
2024-07-31T14:21:33.495000Z TestFramework (ERROR): 
2024-07-31T14:21:33.496000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-chronik-plugins/test/tmp/test_runner_₿₵_🏃_20240731_141649/chronik_block_header_268' to consolidate all logs
2024-07-31T14:21:33.496000Z TestFramework (ERROR): 
2024-07-31T14:21:33.496000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-07-31T14:21:33.496000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2024-07-31T14:21:33.496000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: chronik_block_header.py

Harbormaster completed remote builds in B29804: Diff 48954.Jul 31 2024, 15:41
PiRK updated this revision to Diff 48975.Aug 1 2024, 08:46

handle_header -> handle_block_header

PiRK retitled this revision from [chronik] add a header endpoint to [chronik] add a block-header endpoint.Aug 1 2024, 09:12
Harbormaster completed remote builds in B29820: Diff 48975.Aug 1 2024, 09:14
teamcity added a comment.Aug 1 2024, 09:14
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-integration-tests) failed.

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_₿₵_🏃_20240801_091408'
Test runner completed with code 1
----------------------------|---------|----------|---------|---------|------------------------------
File                        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
----------------------------|---------|----------|---------|---------|------------------------------
All files                   |   62.79 |    53.17 |   62.55 |   62.83 |                              
 ecash-lib                  |       0 |        0 |       0 |       0 |                              
  eslint.config.js          |       0 |        0 |       0 |       0 |                              
 ecash-lib/src              |   73.12 |    56.58 |   73.07 |   72.88 |                              
  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                     |   49.15 |     50.9 |      80 |   49.15 | ...89,92,104,107,109,117-122 
  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              |   52.74 |    46.42 |   66.66 |   51.72 | ...3-107,124-179,214,244-248 
  unsignedTx.ts             |    73.8 |    57.14 |   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.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         |   87.67 |    53.33 |    87.5 |   88.23 |                              
  testRunner.ts             |   87.67 |    53.33 |    87.5 |   88.23 | 69-71,84-85,108,119,162      
 ecash-lib/src/token        |   87.15 |    72.85 |   93.33 |   87.07 |                              
  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                    |   89.74 |    62.16 |     100 |   89.74 | ...9,161,167,175,178,197,202 
----------------------------|---------|----------|---------|---------|------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='773']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='1231']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='243']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='457']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='132']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='211']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='754']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='1200']
##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
PiRK updated this revision to Diff 48994.Aug 1 2024, 16:11

rebase

Harbormaster completed remote builds in B29833: Diff 48994.Aug 1 2024, 16:45
Closed by commit rABC32e93a4e2fb3: [chronik] add a block-header endpoint (authored by PiRK). · Explain WhyAug 1 2024, 19:13
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC32e93a4e2fb3: [chronik] add a block-header endpoint.

Revision Contents
Changeset List

PathSize
chronik/
chronik-bridge/
src/
7 lines
chronik-cpp/
2 lines
12 lines
chronik-http/
src/
11 lines
chronik-indexer/
src/
query/
19 lines
chronik-proto/
proto/
6 lines
test/
functional/
115 lines
test_framework/
chronik/
3 lines

Diff 48953

View Options

chronik/chronik-bridge/src/ffi.rs

Loading...
View Options

chronik/chronik-cpp/chronik_bridge.h

Loading...
View Options

chronik/chronik-cpp/chronik_bridge.cpp

Loading...
View Options

chronik/chronik-http/src/server.rs

Loading...
View Options

chronik/chronik-indexer/src/query/blocks.rs

Loading...
View Options

chronik/chronik-proto/proto/chronik.proto

Loading...
View Options

test/functional/chronik_block_header.py

Loading...
View Options

test/functional/test_framework/chronik/client.py

Loading...