So far we didn't have a script hash for new scripts that were never included in a block. This diff adds a mempool index using a HashMap.
Details
Details
- Reviewers
tobias_ruck - Group Reviewers
Restricted Project - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- rABC695462edf1d1: [chronik] index scripts by scripthashes in the mempool
ninja check-functional
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- arcpatch-D17037_1
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Event Timeline
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.
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 failed with exit code 1
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.
Tail of the build log:
Checking serde_urlencoded v0.7.1
Checking futures-executor v0.3.30
Checking serde_path_to_error v0.1.16
Checking hyper-util v0.1.3
Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
Checking base64 v0.21.7
Checking matchit v0.7.3
Checking sync_wrapper v1.0.1
Checking unicode-segmentation v1.11.0
Checking unindent v0.2.3
Checking tower-http v0.5.2
Checking futures v0.3.30
Checking convert_case v0.6.0
Checking toml_edit v0.22.14
Compiling chronik-bridge v0.1.0 (/work/chronik/chronik-bridge)
Compiling librocksdb-sys v0.11.0+8.1.1
Checking axum v0.7.5
Compiling pyo3-macros v0.22.2
Checking toml v0.8.14
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)
error: unnecessary closure used to substitute value for `Option::None`
--> chronik/chronik-db/src/io/group_history.rs:225:30
|
225 | let cf_member_hash = self
| ______________________________^
226 | | .cf_member_hash
227 | | .ok_or_else(|| GroupDoesNotImplementMemberHash())?;
| |______________-----------------------------------------------^
| |
| help: use `ok_or(..)` instead: `ok_or(GroupDoesNotImplementMemberHash())`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
= note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_lazy_evaluations)]`
error: redundant closure
--> chronik/chronik-db/src/io/group_history.rs:227:25
|
227 | .ok_or_else(|| GroupDoesNotImplementMemberHash())?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `GroupDoesNotImplementMemberHash`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
= note: `-D clippy::redundant-closure` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::redundant_closure)]`
error: this boolean expression can be simplified
--> chronik/chronik-db/src/mem/group_history.rs:35:36
|
35 | let member_hash_index = if !conf.cf_member_hash_name.is_none() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `conf.cf_member_hash_name.is_some()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
error: could not compile `chronik-db` (lib) due to 3 previous errors
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1| chronik/chronik-db/src/mem/group_history.rs | ||
|---|---|---|
| 35–39 ↗ | (On Diff #50549) | let member_hash_index = conf.cf_member_hash_name.is_some().then(HashMap::new); might also work, not sure |
| 110 ↗ | (On Diff #50549) | we can point directly into the mempool and let the caller decide whether to allocate or not |
| 113 ↗ | (On Diff #50549) | no need to allocate here |
| 114 ↗ | (On Diff #50549) | I think this should work? |
| test/functional/chronik_scripthash.py | ||
| 167 ↗ | (On Diff #50549) | You should use the check_num_txs also for mempool txs, or change it upstream to not care about mempool txs Also it might be good to test txs being both in the mempool and in blocks for the same script, and handling that gracefully (esp. in history) |
| chronik/chronik-db/src/mem/group_history.rs | ||
|---|---|---|
| 110 ↗ | (On Diff #50549) | I wasn't able to make this work. If I just add .as_deref(): error[E0515]: cannot return value referencing temporary value
--> chronik/chronik-db/src/mem/group_history.rs:108:20
|
108 | return member_hash_index
| ____________________
| |
109 | || .get(member_hash.to_be_bytes().as_ref())
110 | || .cloned().as_deref();
| ||_________________________-__________^
| |_________________________|
| temporary value created hereIf I removed .cloned() before adding as_deref() error[E0308]: mismatched types
--> chronik/chronik-db/src/mem/group_history.rs:108:20
|
106 | ) -> Option<&[u8]> {
| ------------- expected `std::option::Option<&[u8]>` because of return type
107 | if let Some(member_hash_index) = &self.member_hash_index {
108 | return member_hash_index
|
109 | | .get(member_hash.to_be_bytes().as_ref())
110 | | .as_deref();
| |___________________________^
|
= note: expected enum `std::option::Option<&[u8]>`
found enum `std::option::Option<&Vec<u8>>`
help: try using `.map(|v| &**v)` to convert `std::option::Option<&Vec<u8>>` to `std::option::Option<&[u8]>`
|
110 | .as_deref().map(|v| &**v);
| ++++++++++++++ |
| test/functional/chronik_scripthash.py | ||
| 167 ↗ | (On Diff #50549) | I'm not sure what you mean. We currently do that already, don't we? We only check a single script (OP_TRUE) and we test both scenarios, transaction only in blocks (confirmed_txs) or txs both in block and mempool (via unconfirmed_txs) For history we check assert_equal(script_history.num_txs, num_block_txs + num_mempool_txs) |
| test/functional/chronik_scripthash.py | ||
|---|---|---|
| 206 ↗ | (On Diff #50589) | I know it’s annoying but can you also add a test where a block mines txs that conflict with mempool txs? Those have been a consistent source of bugs that are rarely found. There’s a few interesting scenarios:
|
| chronik/chronik-db/src/mem/group_history.rs | ||
|---|---|---|
| 111 ↗ | (On Diff #50589) | This should do it |
| chronik/chronik-db/src/mem/group_history.rs | ||
|---|---|---|
| 107 ↗ | (On Diff #50589) | It is causing an obscure error downstream error[E0521]: borrowed data escapes outside of method
--> chronik/chronik-indexer/src/query/group_history.rs:123:21
|
105 | impl<'a, G: Group> QueryGroupHistory<'a, G> {
| -- lifetime `'a` defined here
106 | fn member_ser_from_member(
107 | &self,
| ----- `self` is a reference that is only valid in the method body
...
123 | self.mempool_history.member_ser_by_member_hash(*memberhash)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `self` escapes the method body here
| argument requires that `'a` must outlive `'static`
For more information about this error, try `rustc --explain E0521`.I tried variations of .clone() in the callsite, but without success. self.mempool_history.member_ser_by_member_hash((*memberhash).clone()) |
| chronik/chronik-db/src/mem/group_history.rs | ||
|---|---|---|
| 107 ↗ | (On Diff #50589) | Yes, because now you have to turn the slice into a Bytes (see the other comment) |
| chronik/chronik-indexer/src/query/group_history.rs | ||
| 130 ↗ | (On Diff #50589) | this should fix the lifetime issue (and we can now build the Bytes directly from a slice). Not that that changes much in this case, but it just gives the callsite more flexibility. The reason is that Bytes::from requires the slice to be 'static (just because the writers of the library required this to be the case), or to be Vec<u8>. So this should also work, but not as idiomatic: Bytes::from(script_ser.to_vec) |
Comment Actions
Make member_ser_by_member_hash return a &[u8] and make it work in the callsite
Add an additional test that covers two of the requested scenarii:
- one where there’s still one left in the mempool
- one where there’s still one left in the db
The third one (block txs invalidating a mempool tx and clearing the history) is more involved, as I can't just use the miniwallet to create the utxos to spend. Still figuring it out.
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.
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 failed with exit code 1
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.
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.30s 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
Comment Actions
add test for scenario "a mined tx replaces a mempool tx leaving the history empty", remove unused import causing compile warning (seems unrelated to this diff??)
| chronik/chronik-db/src/test/value_group.rs | ||
|---|---|---|
| 5–6 | This change is needed because of a compiler telling me that the import is unused. Not sure what his has to do with this diff, the issue must have sneaked into the codebase in a different diff. | |
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.
Tail of the build log:
Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
Checking unicode-segmentation v1.11.0
Checking matchit v0.7.3
Checking sync_wrapper v1.0.1
Checking unindent v0.2.3
Checking tower-http v0.5.2
Checking futures v0.3.30
Checking convert_case v0.6.0
Checking toml_edit v0.22.14
Compiling chronik-bridge v0.1.0 (/work/chronik/chronik-bridge)
Compiling librocksdb-sys v0.11.0+8.1.1
Checking axum v0.7.5
Compiling pyo3-macros v0.22.2
Checking toml v0.8.14
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)
error: using `clone` on type `Option<&[u8]>` which implements the `Copy` trait
--> chronik/chronik-indexer/src/query/group_history.rs:122:43
|
122 | if let Some(member_ser) = self
| ___________________________________________^
123 | | .mempool_history
124 | | .member_ser_by_member_hash(*memberhash)
125 | | .clone()
| |____________________________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
= note: `-D clippy::clone-on-copy` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::clone_on_copy)]`
help: try removing the `clone` call
|
122 ~ if let Some(member_ser) = self
123 + .mempool_history
124 + .member_ser_by_member_hash(*memberhash)
|
error: could not compile `chronik-indexer` (lib) due to 1 previous error
[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.76.0-x86_64-unknown-linux-gnu/bin/rustc" CARGO_BUILD_RUSTDOC="/root/.cargo/bin/rustdoc" /root/.rustup/toolchains/1.76.0-x86_64-unknown-linux-gnu/bin/cargo --locked clippy --package 'abc-rust-*' -- -D warnings
warning: virtual workspace defaulting to `resolver = "1"` despite one or more workspace members being on edition 2021 which implies `resolver = "2"`
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
note: for more details see https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions
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 package cache
Blocking waiting for file lock on build directory
Checking memchr v2.7.2
Checking libc v0.2.153
Checking bytes v1.6.0
Checking http v1.1.0
Checking object v0.32.2
Checking backtrace v0.3.71
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 5m 00s
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1| chronik/chronik-db/src/test/value_group.rs | ||
|---|---|---|
| 5–6 | It comes from here: https://reviews.bitcoinabc.org/D16827#change-arWadc2ufxeT | |
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.
Tail of the build log:
Compiling bitcoinsuite-slp v0.1.0 (/work/chronik/bitcoinsuite-slp)
Compiling abc-rust-error v0.1.0 (/work/chronik/abc-rust-error)
Compiling chronik-plugin-common v0.1.0 (/work/chronik/chronik-plugin-common)
Compiling toml v0.8.14
Compiling chronik-plugin-impl v0.1.0 (/work/chronik/chronik-plugin-impl)
Compiling chronik-plugin v0.1.0 (/work/chronik/chronik-plugin)
Compiling rocksdb v0.21.0
Compiling chronik-db v0.1.0 (/work/chronik/chronik-db)
Finished test [unoptimized + debuginfo] target(s) in 7m 10s
Running unittests src/lib.rs (abc-ci-builds/build-chronik-plugins/cargo/build/debug/deps/chronik_db-a3c26c153718321c)
running 37 tests
test groups::script::tests::test_script_group ... ok
test index_tx::tests::test_tx_num_cache ... ok
test io::group_utxos::tests::test_value_group_utxos ... ok
test io::block_stats::tests::test_block_stats ... ok
test io::group_history::tests::test_value_group_history ... ok
test io::token::tests::test_batch_common::test_batch_cycle ... ok
test io::token::tests::test_batch_burn::test_batch_burn ... ok
test io::token::tests::test_batch_alp::test_batch_alp ... ok
test io::token::tests::test_batch_common::test_batch_topological_sort ... ok
test io::token::tests::test_batch_common::test_batch_skip_validation ... ok
test mem::group_history::tests::test_mempool_group_history ... ok
test mem::group_utxos::tests::test_mempool_group_utxos ... ok
test mem::spent_by::tests::test_mempool_spent_by ... ok
test io::merge::tests::test_catch_merge ... ok
test io::token::tests::test_batch_genesis::test_batch_genesis_alp ... ok
test io::token::tests::test_batch_disconnect_block::test_batch_disconnect ... ok
test io::token::tests::test_batch_genesis::test_batch_genesis_slp_fungible ... ok
test io::token::tests::test_batch_vault::test_batch_vault ... ok
test ser::tests::test_deserialize_err ... ok
test io::token::tests::test_batch_nft::test_batch_slp_nft1 ... ok
test ser::tests::test_deserialize_leftover_err ... ok
test ser::tests::test_err_display_deserialize ... ok
test ser::tests::test_err_display_deserialize_leftover ... ok
test ser::tests::test_err_display_serialize ... ok
test ser::tests::test_roundtrip ... ok
test ser::tests::test_roundtrip_vec ... ok
test ser::tests::test_serialize_err ... ok
test io::token::tests::test_batch_unknown::test_batch_unknown ... ok
test plugins::io::tests::test_plugin_metas ... ok
test io::spent_by::tests::test_spent_by ... ok
test mem::tokens::tests::test_mempool_tokens ... ok
test reverse_lookup::tests::test_reverse_lookup ... ok
test plugins::io::tests::test_plugin_writer ... ok
test index_tx::tests::test_prepare_indexed_txs ... ok
test io::blocks::tests::test_blocks ... ok
test io::txs::tests::test_insert_txs ... ok
test reverse_lookup::tests::test_reverse_lookup_rng ... ok
test result: ok. 37 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.29s
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
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1