fix(build): put gRPC tests behind an optional feature flag to fix production build issues (#4369)

* fix(ci): sentry is not longer being activated in test builds

This removes sentry from all the test execution, as some tests might fail as sentry wasn't initially built, or it might take more time to build as it will have to build with sentry.

* fix(build): workaround the failed to fetch oauth token error

* Drop sentry dependencies when enable-sentry feature is disabled

* Make lightwalletd gRPC tests depend on a new lightwalletd-grpc-tests feature

* fix(ci): remove enable-sentry feature from tests

* Add lightwalletd-grpc-tests feature for functionality or efficiency

And document where it is just used to stop re-compilations.

* Remove redundant `cmake` and `protobuf-compiler` dependencies

* Document Zebra's optional production and test feature flags

* Minimise dependencies in zcash-params/Dockerfile

* Minimise dependencies in docker/Dockerfile

* Add a workflow TODO

Co-authored-by: Gustavo Valverde <gustavo@iterativo.do>

.github/workflows/continous-integration-docker.yml

@@ -101,6 +101,8 @@ jobs:
 
   # Run all the zebra tests, including tests that are ignored by default.
   # Skips tests that need a cached state disk or a lightwalletd binary.
+  #
+  # (We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests.)
   test-all:
     name: Test all
     runs-on: ubuntu-latest
@@ -115,13 +117,17 @@ jobs:
       - name: Run all zebrad tests
         run: |
           docker pull ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }}
-          docker run --name zebrad-tests -t ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }} cargo test --locked --release --workspace -- --include-ignored
+          docker run --name zebrad-tests -t ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }} cargo test --locked --release --features lightwalletd-grpc-tests --workspace -- --include-ignored
 
+  # Run state tests with fake activation heights.
+  #
   # This test changes zebra-chain's activation heights,
   # which can recompile all the Zebra crates,
   # so we want its build products to be cached separately.
   #
   # Also, we don't want to accidentally use the fake heights in other tests.
+  #
+  # (The gRPC feature is a zebrad feature, so it isn't needed here.)
   test-fake-activation-heights:
     name: Test with fake activation heights
     runs-on: ubuntu-latest
@@ -140,7 +146,9 @@ jobs:
         env:
           TEST_FAKE_ACTIVATION_HEIGHTS: '1'
 
-  # Test that Zebra syncs and checkpoints a few thousand blocks from an empty state
+  # Test that Zebra syncs and checkpoints a few thousand blocks from an empty state.
+  #
+  # (We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests.)
   test-empty-sync:
     name: Test checkpoint sync from empty state
     runs-on: ubuntu-latest
@@ -155,9 +163,11 @@ jobs:
       - name: Run zebrad large sync tests
         run: |
           docker pull ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }}
-          docker run --name zebrad-tests -t ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }} cargo test --locked --release --test acceptance sync_large_checkpoints_ -- --ignored
+          docker run --name zebrad-tests -t ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }} cargo test --locked --release --features lightwalletd-grpc-tests --test acceptance sync_large_checkpoints_ -- --ignored
 
-  # Test launching lightwalletd with an empty lightwalletd and Zebra state
+  # Test launching lightwalletd with an empty lightwalletd and Zebra state.
+  #
+  # (We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests.)
   test-lightwalletd-integration:
     name: Test integration with lightwalletd
     runs-on: ubuntu-latest
@@ -172,7 +182,7 @@ jobs:
       - name: Run tests with empty lightwalletd launch
         run: |
           docker pull ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }}
-          docker run -e ZEBRA_TEST_LIGHTWALLETD --name lightwalletd-tests -t ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }} cargo test --locked --release --test acceptance -- lightwalletd_integration --nocapture
+          docker run -e ZEBRA_TEST_LIGHTWALLETD --name lightwalletd-tests -t ${{ env.GAR_BASE }}/${{ env.IMAGE_NAME }}:sha-${{ env.GITHUB_SHA_SHORT }} cargo test --locked --release --features lightwalletd-grpc-tests --test acceptance -- lightwalletd_integration --nocapture
         env:
           ZEBRA_TEST_LIGHTWALLETD: '1'
 

.github/workflows/continous-integration-os.yml

@@ -110,6 +110,10 @@ jobs:
 
       # Modified from:
       # https://github.com/zcash/librustzcash/blob/c48bb4def2e122289843ddb3cb2984c325c03ca0/.github/workflows/ci.yml#L20-L33
+      #
+      # TODO: split get-params-path and download-params examples into their own crate,
+      #       to speed up compilation
+      #       compile examples in release mode, to speed up downloads
       - name: Fetch path to Zcash parameters
         working-directory: ./zebra-consensus
         shell: bash

book/src/user/requirements.md

@@ -21,3 +21,22 @@ We usually run `zebrad` on systems with:
 
 `zebrad` might build and run fine on smaller and slower systems - we haven't
 tested its exact limits yet.
+
+# Additional Features
+
+## Sentry Production Monitoring
+
+Compile Zebra with `--features enable-sentry` to monitor it using Sentry in production.
+
+## Lightwalletd Test Requirements
+
+To test Zebra's `lightwalletd` RPC methods:
+- compile Zebra with the `--features lightwalletd-grpc-tests`
+- install a `lightwalletd` binary
+  - Zebra's tests currently target [adityapk00/lightwalletd](https://github.com/adityapk00/lightwalletd)
+  - some tests might fail on other lightwalletd versions, due to differences in the logs
+- install the `protoc` Protobuf compiler:
+  - the `protobuf-compiler` or `protobuf` package, or
+  - `cmake` to automatically compile `protoc` in the `zebrad` build script
+- set the required test environmental variables:
+  - TODO: list or link to test environmental variables - [see ticket #4363](https://github.com/ZcashFoundation/zebra/issues/4363)

docker/Dockerfile

@@ -11,7 +11,7 @@ FROM rust:bullseye as chef
 RUN cargo install cargo-chef --locked
 WORKDIR /app
 
-# Analyze the current project to determine the minimum subset of files 
+# Analyze the current project to determine the minimum subset of files
 # (Cargo.lock and Cargo.toml manifests) required to build it and cache dependencies
 #
 # The recipe.json is the equivalent of the Python requirements.txt file
@@ -34,7 +34,6 @@ RUN apt-get -qq update && \
     libclang-dev \
     clang \
     ca-certificates \
-    cmake \
     protobuf-compiler \
     ; \
     rm -rf /var/lib/apt/lists/* /tmp/*
@@ -96,12 +95,15 @@ COPY --from=us-docker.pkg.dev/zealous-zebra/zebra/zcash-params /root/.zcash-para
 COPY --from=us-docker.pkg.dev/zealous-zebra/zebra/lightwalletd /lightwalletd /usr/local/bin
 
 # Re-hydrate the minimum project skeleton identified by `cargo chef prepare` in the planner stage,
-# and build it to cache dependencies.
+# and build it to cache all possible sentry and test dependencies.
+#
 # This is the caching Docker layer for Rust!
-RUN cargo chef cook --release --workspace --recipe-path recipe.json
+#
+# TODO: is it faster to use --tests here?
+RUN cargo chef cook --release --features enable-sentry,lightwalletd-grpc-tests --workspace --recipe-path recipe.json
 
 COPY . .
-RUN cargo test --locked --release --workspace --no-run
+RUN cargo test --locked --release --features lightwalletd-grpc-tests --workspace --no-run
 
 COPY ./docker/entrypoint.sh /
 RUN chmod u+x /entrypoint.sh
@@ -111,7 +113,7 @@ CMD [ "cargo"]
 
 # In this stage we build a release (generate the zebrad binary)
 #
-# This step also adds `cargo chef` as this stage is completely independent from the 
+# This step also adds `cargo chef` as this stage is completely independent from the
 # `test` stage. This step is a dependency for the `runtime` stage, which uses the resulting
 # zebrad binary from this step.
 FROM deps AS release
@@ -119,7 +121,7 @@ RUN cargo chef cook --release --features enable-sentry --recipe-path recipe.json
 
 COPY . .
 # Build zebra
-RUN cargo build --locked --release --features enable-sentry --bin zebrad
+RUN cargo build --locked --release --features enable-sentry --package zebrad --bin zebrad
 
 # This stage is only used when deploying nodes or when only the resulting zebrad binary is needed
 #

docker/entrypoint.sh

@@ -4,18 +4,37 @@ set -x
 
 case "$1" in
     -- | cargo)
+        # For these tests, we activate the gRPC feature to avoid recompiling `zebrad`,
+        # but we might not actually run any gRPC tests.
         if [[ "$RUN_ALL_TESTS" -eq "1" ]]; then
-            exec cargo "test" "--locked" "--release" "--workspace" "--" "--include-ignored"
+            # Run all the available tests for the current environment.
+            # If the lightwalletd environmental variables are set, we will also run those tests.
+            exec cargo "test" "--locked" "--release" "--features" "lightwalletd-grpc-tests" "--workspace" "--" "--include-ignored"
+
+        # For these tests, we activate the gRPC feature to avoid recompiling `zebrad`,
+        # but we don't actually run any gRPC tests.
         elif [[ "$TEST_FULL_SYNC" -eq "1" ]]; then
-            exec cargo "test" "--locked" "--release" "--test" "acceptance" "--" "--nocapture" "--ignored" "full_sync_mainnet"
+            # Run a Zebra full sync test.
+            exec cargo "test" "--locked" "--release" "--features" "lightwalletd-grpc-tests" "--test" "acceptance" "--" "--nocapture" "--ignored" "full_sync_mainnet"
         elif [[ "$TEST_DISK_REBUILD" -eq "1" ]]; then
-            exec cargo "test" "--locked" "--release" "--features" "test_sync_to_mandatory_checkpoint_${NETWORK,,}" "--manifest-path" "zebrad/Cargo.toml" "sync_to_mandatory_checkpoint_${NETWORK,,}"
+            # Run a Zebra sync up to the mandatory checkpoint.
+            exec cargo "test" "--locked" "--release" "--features" "test_sync_to_mandatory_checkpoint_${NETWORK,,},lightwalletd-grpc-tests" "--manifest-path" "zebrad/Cargo.toml" "sync_to_mandatory_checkpoint_${NETWORK,,}"
         elif [[ "$TEST_CHECKPOINT_SYNC" -eq "1" ]]; then
-            exec cargo "test" "--locked" "--release" "--features" "test_sync_past_mandatory_checkpoint_${NETWORK,,}" "--manifest-path" "zebrad/Cargo.toml" "sync_past_mandatory_checkpoint_${NETWORK,,}"
+            # Run a Zebra sync starting at the cached mandatory checkpoint, and syncing past it.
+            exec cargo "test" "--locked" "--release" "--features" "test_sync_past_mandatory_checkpoint_${NETWORK,,},lightwalletd-grpc-tests" "--manifest-path" "zebrad/Cargo.toml" "sync_past_mandatory_checkpoint_${NETWORK,,}"
         elif [[ "$TEST_LWD_RPC_CALL" -eq "1" ]]; then
-            exec cargo "test" "--locked" "--release" "--test" "acceptance" "--" "--nocapture" "--ignored" "fully_synced_rpc_test"
+            # Starting at a cached tip, test a JSON-RPC call to Zebra.
+            exec cargo "test" "--locked" "--release" "--features" "lightwalletd-grpc-tests" "--test" "acceptance" "--" "--nocapture" "--ignored" "fully_synced_rpc_test"
+        elif [[ "$TEST_LWD_FULL_SYNC" -eq "1" ]]; then
+            # Starting at a cached Zebra tip, run a lightwalletd sync to tip.
+            exec cargo "test" "--locked" "--release" "--features" "lightwalletd-grpc-tests" "--test" "acceptance" "--" "--nocapture" "--ignored" "lightwalletd_full_sync"
+
+        # These tests actually use gRPC.
         elif [[ "$TEST_LWD_TRANSACTIONS" -eq "1" ]]; then
-            exec cargo "test" "--locked" "--release" "--test" "acceptance" "--" "--nocapture" "--ignored" "sending_transactions_using_lightwalletd"
+            # Starting at a cached tip, test a gRPC call to lightwalletd, which calls Zebra.
+            exec cargo "test" "--locked" "--release" "--features" "lightwalletd-grpc-tests" "--test" "acceptance" "--" "--nocapture" "--ignored" "sending_transactions_using_lightwalletd"
+
+        # These command-lines are provided by the caller.
         else
             exec "$@"
         fi
@@ -27,4 +46,4 @@ case "$1" in
         exec "$@"
 esac
 
-exit 1
+exit 1

docker/zcash-params/Dockerfile

@@ -18,14 +18,12 @@ RUN apt-get -qq update && \
     libclang-dev \
     clang \
     ca-certificates \
-    cmake \
-    protobuf-compiler \
     ; \
     rm -rf /var/lib/apt/lists/* /tmp/*
 
 ENV CARGO_HOME /app/.cargo/
 # Build dependencies - this is the caching Docker layer!
-RUN cargo chef cook --release --features enable-sentry --recipe-path recipe.json
+RUN cargo chef cook --release --features enable-sentry --package zebrad --recipe-path recipe.json
 
 ARG RUST_BACKTRACE=0
 ENV RUST_BACKTRACE ${RUST_BACKTRACE}
@@ -38,4 +36,4 @@ ENV COLORBT_SHOW_HIDDEN ${COLORBT_SHOW_HIDDEN}
 
 COPY . .
 # Pre-download Zcash Sprout and Sapling parameters
-RUN cargo run --locked --release --features enable-sentry --bin zebrad download
+RUN cargo run --locked --release --features enable-sentry --package zebrad --bin zebrad download

zebrad/Cargo.toml

@@ -18,6 +18,10 @@ enable-sentry = ["sentry", "sentry-tracing"]
 # Testing features that activate extra dependencies
 proptest-impl = ["proptest", "proptest-derive", "zebra-chain/proptest-impl", "zebra-state/proptest-impl", "zebra-consensus/proptest-impl", "zebra-network/proptest-impl"]
 
+# The gRPC tests also need an installed lightwalletd binary
+lightwalletd-grpc-tests = ["tonic-build"]
+
+# TODO: replace with environmental variables that skip the tests when not set (part of #2995)
 test_sync_to_mandatory_checkpoint_mainnet = []
 test_sync_to_mandatory_checkpoint_testnet = []
 test_sync_past_mandatory_checkpoint_mainnet = []
@@ -73,14 +77,15 @@ proptest = { version = "0.10.1", optional = true }
 proptest-derive = { version = "0.3.0", optional = true }
 
 [build-dependencies]
-tonic-build = "0.7.2"
 vergen = { version = "7.0.0", default-features = false, features = ["cargo", "git"] }
 
+# test feature lightwalletd-grpc-tests
+tonic-build = { version = "0.7.2", optional = true }
+
 [dev-dependencies]
 abscissa_core = { version = "0.5", features = ["testing"] }
 hex = "0.4.3"
 once_cell = "1.10.0"
-prost = "0.10.3"
 regex = "1.5.5"
 reqwest = "0.11"
 semver = "1.0.9"
@@ -88,6 +93,9 @@ semver = "1.0.9"
 serde_json = { version = "1.0.81", features = ["preserve_order"] }
 tempfile = "3.3.0"
 tokio = { version = "1.18.2", features = ["full", "test-util"] }
+
+# test feature lightwalletd-grpc-tests
+prost = "0.10.3"
 tonic = "0.7.2"
 
 proptest = "0.10.1"

zebrad/build.rs

@@ -56,6 +56,7 @@ fn main() {
         }
     }
 
+    #[cfg(feature = "lightwalletd-grpc-tests")]
     tonic_build::configure()
         .build_client(true)
         .build_server(false)

zebrad/tests/acceptance.rs

@@ -1525,6 +1525,7 @@ async fn fully_synced_rpc_test() -> Result<()> {
 /// Test sending transactions using a lightwalletd instance connected to a zebrad instance.
 ///
 /// See [`common::lightwalletd::send_transaction_test`] for more information.
+#[cfg(feature = "lightwalletd-grpc-tests")]
 #[tokio::test]
 #[ignore]
 async fn sending_transactions_using_lightwalletd() -> Result<()> {
@@ -1534,7 +1535,9 @@ async fn sending_transactions_using_lightwalletd() -> Result<()> {
 /// Test all the rpc methods a wallet connected to lightwalletd can call.
 ///
 /// See [`common::lightwalletd::wallet_grpc_test`] for more information.
+#[cfg(feature = "lightwalletd-grpc-tests")]
 #[tokio::test]
+#[ignore]
 async fn lightwalletd_wallet_grpc_tests() -> Result<()> {
     common::lightwalletd::wallet_grpc_test::run().await
 }

zebrad/tests/common/cached_state.rs

@@ -1,4 +1,9 @@
 //! Utility functions for tests that used cached Zebra state.
+//!
+//! Note: we allow dead code in this module, because it is mainly used by the gRPC tests,
+//! which are optional.
+
+#![allow(dead_code)]
 
 use std::path::{Path, PathBuf};
 

zebrad/tests/common/lightwalletd.rs

@@ -34,8 +34,11 @@ use super::{
 
 use LightwalletdTestType::*;
 
+#[cfg(feature = "lightwalletd-grpc-tests")]
 pub mod send_transaction_test;
+#[cfg(feature = "lightwalletd-grpc-tests")]
 pub mod wallet_grpc;
+#[cfg(feature = "lightwalletd-grpc-tests")]
 pub mod wallet_grpc_test;
 
 /// The name of the env var that enables Zebra lightwalletd integration tests.

zebrad/tests/common/sync.rs

@@ -60,6 +60,7 @@ pub const LARGE_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(180);
 /// The partially synchronized state is expected to be close to the tip, so this timeout can be
 /// lower than what's expected for a full synchronization. However, a value that's too short may
 /// cause the test to fail.
+#[allow(dead_code)]
 pub const FINISH_PARTIAL_SYNC_TIMEOUT: Duration = Duration::from_secs(60 * 60);
 
 /// The test sync height where we switch to using the default lookahead limit.
@@ -84,6 +85,7 @@ pub enum MempoolBehavior {
     ///
     /// [`sync_until`] will kill `zebrad` after it logs mempool activation,
     /// then the `stop_regex`.
+    #[allow(dead_code)]
     ShouldAutomaticallyActivate,
 
     /// The mempool should not become active during the test.
@@ -281,6 +283,7 @@ pub fn sync_until(
 ///
 /// The zebrad instance is executed on a copy of the partially synchronized chain state. This copy
 /// is returned afterwards, containing the fully synchronized chain state.
+#[allow(dead_code)]
 pub async fn perform_full_sync_starting_from(
     network: Network,
     partial_sync_path: &Path,