From 211dbb437b4a0967065fe370bca53ceefea87a54 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 3 Oct 2022 09:34:44 +1000 Subject: [PATCH 1/5] 1. fix(rpc): Fix slow getblock RPC (verbose=1) using transaction ID index (#5307) * Add RPC timing to zcash-rpc-diff * Use transaction hash index for verbose block requests, rather than block data --- zebra-rpc/src/methods.rs | 81 ++++++++++++------- zebra-state/src/request.rs | 13 +++ zebra-state/src/response.rs | 8 +- zebra-state/src/service.rs | 35 +++++++- .../service/finalized_state/zebra_db/block.rs | 33 ++++++++ .../src/service/non_finalized_state/chain.rs | 15 ++++ zebra-state/src/service/read.rs | 2 +- zebra-state/src/service/read/block.rs | 25 ++++++ zebra-utils/zcash-rpc-diff | 6 +- 9 files changed, 182 insertions(+), 36 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 4f9e4bfb40e..79f2d34f66e 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -540,46 +540,65 @@ where let mut state = self.state.clone(); async move { - let height = height.parse().map_err(|error: SerializationError| Error { + let height: Height = height.parse().map_err(|error: SerializationError| Error { code: ErrorCode::ServerError(0), message: error.to_string(), data: None, })?; - let request = - zebra_state::ReadRequest::Block(zebra_state::HashOrHeight::Height(height)); - let response = state - .ready() - .and_then(|service| service.call(request)) - .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; + if verbosity == 0 { + let request = zebra_state::ReadRequest::Block(height.into()); + let response = state + .ready() + .and_then(|service| service.call(request)) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })?; - match response { - zebra_state::ReadResponse::Block(Some(block)) => match verbosity { - 0 => Ok(GetBlock::Raw(block.into())), - 1 => Ok(GetBlock::Object { - tx: block - .transactions - .iter() - .map(|tx| tx.hash().encode_hex()) - .collect(), + match response { + zebra_state::ReadResponse::Block(Some(block)) => { + Ok(GetBlock::Raw(block.into())) + } + zebra_state::ReadResponse::Block(None) => Err(Error { + code: MISSING_BLOCK_ERROR_CODE, + message: "Block not found".to_string(), + data: None, }), - _ => Err(Error { - code: ErrorCode::InvalidParams, - message: "Invalid verbosity value".to_string(), + _ => unreachable!("unmatched response to a block request"), + } + } else if verbosity == 1 { + let request = zebra_state::ReadRequest::TransactionIdsForBlock(height.into()); + let response = state + .ready() + .and_then(|service| service.call(request)) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })?; + + match response { + zebra_state::ReadResponse::TransactionIdsForBlock(Some(tx_ids)) => { + let tx_ids = tx_ids.iter().map(|tx_id| tx_id.encode_hex()).collect(); + Ok(GetBlock::Object { tx: tx_ids }) + } + zebra_state::ReadResponse::TransactionIdsForBlock(None) => Err(Error { + code: MISSING_BLOCK_ERROR_CODE, + message: "Block not found".to_string(), data: None, }), - }, - zebra_state::ReadResponse::Block(None) => Err(Error { - code: MISSING_BLOCK_ERROR_CODE, - message: "Block not found".to_string(), + _ => unreachable!("unmatched response to a transaction_ids_for_block request"), + } + } else { + Err(Error { + code: ErrorCode::InvalidParams, + message: "Invalid verbosity value".to_string(), data: None, - }), - _ => unreachable!("unmatched response to a block request"), + }) } } .boxed() @@ -1111,7 +1130,7 @@ pub enum GetBlock { Raw(#[serde(with = "hex")] SerializedBlock), /// The block object. Object { - /// Vector of hex-encoded TXIDs of the transactions of the block + /// List of transaction IDs in block order, hex-encoded. tx: Vec, }, } diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 044c6a9a9c0..bca3f01409b 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -597,6 +597,18 @@ pub enum ReadRequest { /// * [`ReadResponse::Transaction(None)`](ReadResponse::Transaction) otherwise. Transaction(transaction::Hash), + /// Looks up the transaction IDs for a block, using a block hash or height. + /// + /// Returns + /// + /// * An ordered list of transaction hashes, or + /// * `None` if the block was not found. + /// + /// Note: Each block has at least one transaction: the coinbase transaction. + /// + /// Returned txids are in the order they appear in the block. + TransactionIdsForBlock(HashOrHeight), + /// Looks up a UTXO identified by the given [`OutPoint`](transparent::OutPoint), /// returning `None` immediately if it is unknown. /// @@ -728,6 +740,7 @@ impl ReadRequest { ReadRequest::Depth(_) => "depth", ReadRequest::Block(_) => "block", ReadRequest::Transaction(_) => "transaction", + ReadRequest::TransactionIdsForBlock(_) => "transaction_ids_for_block", ReadRequest::BestChainUtxo { .. } => "best_chain_utxo", ReadRequest::AnyChainUtxo { .. } => "any_chain_utxo", ReadRequest::BlockLocator => "block_locator", diff --git a/zebra-state/src/response.rs b/zebra-state/src/response.rs index ca66938b20f..7ca08b3fd60 100644 --- a/zebra-state/src/response.rs +++ b/zebra-state/src/response.rs @@ -67,6 +67,11 @@ pub enum ReadResponse { /// Response to [`ReadRequest::Transaction`] with the specified transaction. Transaction(Option<(Arc, block::Height)>), + /// Response to [`ReadRequest::TransactionIdsForBlock`], + /// with an list of transaction hashes in block order, + /// or `None` if the block was not found. + TransactionIdsForBlock(Option>), + /// Response to [`ReadRequest::BlockLocator`] with a block locator object. BlockLocator(Vec), @@ -130,7 +135,8 @@ impl TryFrom for Response { ReadResponse::BlockHashes(hashes) => Ok(Response::BlockHashes(hashes)), ReadResponse::BlockHeaders(headers) => Ok(Response::BlockHeaders(headers)), - ReadResponse::BestChainUtxo(_) + ReadResponse::TransactionIdsForBlock(_) + | ReadResponse::BestChainUtxo(_) | ReadResponse::SaplingTree(_) | ReadResponse::OrchardTree(_) | ReadResponse::AddressBalance(_) diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index de881879171..b3ee4d392dd 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -1173,7 +1173,7 @@ impl Service for ReadStateService { .boxed() } - // Used by get_block RPC and the StateService. + // Used by the get_block (raw) RPC and the StateService. ReadRequest::Block(hash_or_height) => { let timer = CodeTimer::start(); @@ -1227,6 +1227,39 @@ impl Service for ReadStateService { .boxed() } + // Used by the getblock (verbose) RPC. + ReadRequest::TransactionIdsForBlock(hash_or_height) => { + let timer = CodeTimer::start(); + + let state = self.clone(); + + let span = Span::current(); + tokio::task::spawn_blocking(move || { + span.in_scope(move || { + let transaction_ids = state.non_finalized_state_receiver.with_watch_data( + |non_finalized_state| { + read::transaction_hashes_for_block( + non_finalized_state.best_chain(), + &state.db, + hash_or_height, + ) + }, + ); + + // The work is done in the future. + timer.finish( + module_path!(), + line!(), + "ReadRequest::TransactionIdsForBlock", + ); + + Ok(ReadResponse::TransactionIdsForBlock(transaction_ids)) + }) + }) + .map(|join_result| join_result.expect("panic in ReadRequest::Block")) + .boxed() + } + // Currently unused. ReadRequest::BestChainUtxo(outpoint) => { let timer = CodeTimer::start(); diff --git a/zebra-state/src/service/finalized_state/zebra_db/block.rs b/zebra-state/src/service/finalized_state/zebra_db/block.rs index 6e092758ab5..cc707f51770 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block.rs @@ -226,6 +226,39 @@ impl ZebraDb { .map(|tx| (tx, transaction_location.height)) } + /// Returns the [`transaction::Hash`]es in the block with `hash_or_height`, + /// if it exists in this chain. + /// + /// Hashes are returned in block order. + /// + /// Returns `None` if the block is not found. + #[allow(clippy::unwrap_in_result)] + pub fn transaction_hashes_for_block( + &self, + hash_or_height: HashOrHeight, + ) -> Option> { + // Block + let height = hash_or_height.height_or_else(|hash| self.height(hash))?; + + // Transaction hashes + let hash_by_tx_loc = self.db.cf_handle("hash_by_tx_loc").unwrap(); + + // Manually fetch the entire block's transaction hashes + let mut transaction_hashes = Vec::new(); + + for tx_index in 0..=Transaction::max_allocation() { + let tx_loc = TransactionLocation::from_u64(height, tx_index); + + if let Some(tx_hash) = self.db.zs_get(&hash_by_tx_loc, &tx_loc) { + transaction_hashes.push(tx_hash); + } else { + break; + } + } + + Some(transaction_hashes.into()) + } + // Write block methods /// Write `finalized` to the finalized state. diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 0d707b197e0..1c91f9c42e3 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -471,6 +471,21 @@ impl Chain { .get(tx_loc.index.as_usize()) } + /// Returns the [`transaction::Hash`]es in the block with `hash_or_height`, + /// if it exists in this chain. + /// + /// Hashes are returned in block order. + /// + /// Returns `None` if the block is not found. + pub fn transaction_hashes_for_block( + &self, + hash_or_height: HashOrHeight, + ) -> Option> { + let transaction_hashes = self.block(hash_or_height)?.transaction_hashes.clone(); + + Some(transaction_hashes) + } + /// Returns the [`block::Hash`] for `height`, if it exists in this chain. pub fn hash_by_height(&self, height: Height) -> Option { let hash = self.blocks.get(&height)?.hash; diff --git a/zebra-state/src/service/read.rs b/zebra-state/src/service/read.rs index e2a0c01695d..d1f7f4c0a81 100644 --- a/zebra-state/src/service/read.rs +++ b/zebra-state/src/service/read.rs @@ -27,7 +27,7 @@ pub use address::{ tx_id::transparent_tx_ids, utxo::{address_utxos, AddressUtxos, ADDRESS_HEIGHTS_FULL_RANGE}, }; -pub use block::{any_utxo, block, block_header, transaction, utxo}; +pub use block::{any_utxo, block, block_header, transaction, transaction_hashes_for_block, utxo}; pub use find::{ block_locator, chain_contains_hash, depth, find_chain_hashes, find_chain_headers, hash_by_height, height_by_hash, tip, tip_height, diff --git a/zebra-state/src/service/read/block.rs b/zebra-state/src/service/read/block.rs index 985c44eaf94..54eb485c367 100644 --- a/zebra-state/src/service/read/block.rs +++ b/zebra-state/src/service/read/block.rs @@ -93,6 +93,31 @@ where .or_else(|| db.transaction(hash)) } +/// Returns the [`transaction::Hash`]es for the block with `hash_or_height`, +/// if it exists in the non-finalized `chain` or finalized `db`. +/// +/// The returned hashes are in block order. +/// +/// Returns `None` if the block is not found. +pub fn transaction_hashes_for_block( + chain: Option, + db: &ZebraDb, + hash_or_height: HashOrHeight, +) -> Option> +where + C: AsRef, +{ + // # Correctness + // + // Since blocks are the same in the finalized and non-finalized state, we + // check the most efficient alternative first. (`chain` is always in memory, + // but `db` stores blocks on disk, with a memory cache.) + chain + .as_ref() + .and_then(|chain| chain.as_ref().transaction_hashes_for_block(hash_or_height)) + .or_else(|| db.transaction_hashes_for_block(hash_or_height)) +} + /// Returns the [`Utxo`] for [`transparent::OutPoint`], if it exists in the /// non-finalized `chain` or finalized `db`. /// diff --git a/zebra-utils/zcash-rpc-diff b/zebra-utils/zcash-rpc-diff index 885c773ee69..f134f0a43e1 100755 --- a/zebra-utils/zcash-rpc-diff +++ b/zebra-utils/zcash-rpc-diff @@ -89,10 +89,12 @@ echo "$@" echo echo "Querying $ZEBRAD $ZEBRAD_NET chain at height >=$ZEBRAD_HEIGHT..." -$ZCASH_CLI -rpcport="$ZEBRAD_RPC_PORT" "$@" > "$ZEBRAD_RESPONSE" +time $ZCASH_CLI -rpcport="$ZEBRAD_RPC_PORT" "$@" > "$ZEBRAD_RESPONSE" +echo echo "Querying $ZCASHD $ZCASHD_NET chain at height >=$ZCASHD_HEIGHT..." -$ZCASH_CLI "$@" > "$ZCASHD_RESPONSE" +time $ZCASH_CLI "$@" > "$ZCASHD_RESPONSE" +echo echo From 1f4a1b4c4c7aaeee51a13ecfe88c5b07e91809ba Mon Sep 17 00:00:00 2001 From: Marek Date: Sun, 2 Oct 2022 18:45:23 -0700 Subject: [PATCH 2/5] fix(ci): Reduce false positives for the `C-trivial` label (#5309) * Reduce false positives for the `C-trivial` label * Delete `zebra-utils` entry Co-authored-by: teor Co-authored-by: teor --- .github/release-drafter.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml index 2ed2ab40b1b..65e55e76cf3 100644 --- a/.github/release-drafter.yml +++ b/.github/release-drafter.yml @@ -55,7 +55,6 @@ autolabeler: - 'CHANGELOG.md' - 'zebra-consensus/src/checkpoint/*-checkpoints.txt' # Developer-only changes - - 'zebra-utils' - '.gitignore' # Test-only changes - 'zebra-test' From d8f803a82611b5d9d78c59e692cea41f79528a8c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 3 Oct 2022 17:50:10 +0000 Subject: [PATCH 3/5] build(deps): bump actions/github-script from 6.3.0 to 6.3.1 (#5317) Bumps [actions/github-script](https://github.com/actions/github-script) from 6.3.0 to 6.3.1. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](https://github.com/actions/github-script/compare/v6.3.0...v6.3.1) --- updated-dependencies: - dependency-name: actions/github-script dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/continous-delivery.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/continous-delivery.yml b/.github/workflows/continous-delivery.yml index 754219c8f9e..7424b6ad058 100644 --- a/.github/workflows/continous-delivery.yml +++ b/.github/workflows/continous-delivery.yml @@ -51,7 +51,7 @@ jobs: steps: - name: Getting Zebrad Version id: get - uses: actions/github-script@v6.3.0 + uses: actions/github-script@v6.3.1 with: result-encoding: string script: | From 6767f31e483aa2756b08017ba2b03af4b52e4a9a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 3 Oct 2022 17:50:17 +0000 Subject: [PATCH 4/5] build(deps): bump reviewdog/action-actionlint from 1.31.0 to 1.32.0 (#5318) Bumps [reviewdog/action-actionlint](https://github.com/reviewdog/action-actionlint) from 1.31.0 to 1.32.0. - [Release notes](https://github.com/reviewdog/action-actionlint/releases) - [Commits](https://github.com/reviewdog/action-actionlint/compare/v1.31.0...v1.32.0) --- updated-dependencies: - dependency-name: reviewdog/action-actionlint dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index ea271ead82d..bab1a201629 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -175,7 +175,7 @@ jobs: if: ${{ needs.changed-files.outputs.workflows == 'true' }} steps: - uses: actions/checkout@v3.0.2 - - uses: reviewdog/action-actionlint@v1.31.0 + - uses: reviewdog/action-actionlint@v1.32.0 with: level: warning fail_on_error: false From f71bb74951ae11f10b68242a5e26da71076aed8c Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 4 Oct 2022 03:50:27 +1000 Subject: [PATCH 5/5] fix(docker): Make default command work in docker images, disable optional listener ports (#5313) * Disable optional listener ports so the default config is secure * Fix Zebra config file path in Dockerfile --- docker/Dockerfile | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 62f4d84006e..5fe669a1c48 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -109,8 +109,8 @@ COPY ./docker/entrypoint.sh / RUN chmod u+x /entrypoint.sh # By default, runs the entrypoint tests specified by the environmental variables (if any are set) -ENTRYPOINT ["/entrypoint.sh"] -CMD [ "cargo"] +ENTRYPOINT [ "/entrypoint.sh" ] +CMD [ "cargo" ] # In this stage we build a release (generate the zebrad binary) # @@ -148,27 +148,35 @@ ENV ZEBRA_CONF_FILE ${ZEBRA_CONF_FILE} # Build the `zebrad.toml` before starting the container, using the arguments from build # time, or using the default values set just above. And create the conf path and file if -# it does not exist +# it does not exist. # -# TODO: move this file creation to an entrypoint as we can use default values at runtime, -# and modify those as needed when starting the container (at runtime and not at build time) +# It is safe to use multiple RPC threads in Docker, because we know we are the only running +# `zebrad` or `zcashd` process in the container. +# +# TODO: +# - move this file creation to an entrypoint as we can use default values at runtime, +# and modify those as needed when starting the container (at runtime and not at build time) +# - make `cache_dir`, `rpc.listen_addr`, `metrics.endpoint_addr`, and `tracing.endpoint_addr` into Docker arguments RUN mkdir -p ${ZEBRA_CONF_PATH} \ && touch ${ZEBRA_CONF_PATH}/${ZEBRA_CONF_FILE} RUN set -ex; \ { \ - echo "[consensus]"; \ - echo "checkpoint_sync = ${CHECKPOINT_SYNC}"; \ - echo "[metrics]"; \ - echo "endpoint_addr = '0.0.0.0:9999'"; \ echo "[network]"; \ echo "network = '${NETWORK}'"; \ + echo "[consensus]"; \ + echo "checkpoint_sync = ${CHECKPOINT_SYNC}"; \ echo "[state]"; \ echo "cache_dir = '/zebrad-cache'"; \ + echo "[rpc]"; \ + echo "listen_addr = None"; \ + echo "parallel_cpu_threads = 0"; \ + echo "[metrics]"; \ + echo "endpoint_addr = None"; \ echo "[tracing]"; \ - echo "endpoint_addr = '0.0.0.0:3000'"; \ + echo "endpoint_addr = None"; \ } > "${ZEBRA_CONF_PATH}/${ZEBRA_CONF_FILE}" -EXPOSE 3000 8233 18233 +EXPOSE 8233 18233 ARG SHORT_SHA ENV SHORT_SHA $SHORT_SHA @@ -177,4 +185,4 @@ ARG SENTRY_DSN ENV SENTRY_DSN ${SENTRY_DSN} # TODO: remove the specified config file location and use the default expected by zebrad -CMD [ "zebrad", "-c", "${ZEBRA_CONF_PATH}/${ZEBRA_CONF_FILE}", "start" ] +CMD zebrad -c "${ZEBRA_CONF_PATH}/${ZEBRA_CONF_FILE}" start