change(ci): Remove duplicate CI jobs for getblocktemplate-rpcs (#7753)

* Remove duplicate CI docker job and make features consistent

* Remove duplicate OS job

* Rename experimental release build

* Make Rust features into GitHub repository variables

* Remove redundant features in entrypoint.sh

* Remove a dependency on a deleted job

* Fix syntax of array

* Another fix attempt

* Undo some accidental merge overwrites

* Add missing space

* Explain how default is implemented

* Fix missing --features and quoting

* We can fix this later

* Use vars directly in with: blocks

* Use correct features for fake activation heights test

.github/workflows/ci-unit-tests-docker.patch.yml

@@ -45,12 +45,6 @@ jobs:
     steps:
       - run: 'echo "No build required"'
 
-  test-all-getblocktemplate-rpcs:
-    name: Test all with getblocktemplate-rpcs feature
-    runs-on: ubuntu-latest
-    steps:
-      - run: 'echo "No build required"'
-
   test-fake-activation-heights:
     name: Test with fake activation heights
     runs-on: ubuntu-latest

.github/workflows/ci-unit-tests-docker.yml

@@ -65,6 +65,16 @@ on:
       - '.github/workflows/sub-find-cached-disks.yml'
       - '.github/workflows/sub-build-docker-image.yml'
 
+env:
+  # We need to combine the features manually because some tests don't use the Docker entrypoint
+  TEST_FEATURES: ${{ format('{0} {1}', vars.RUST_PROD_FEATURES, vars.RUST_TEST_FEATURES) }}
+  EXPERIMENTAL_FEATURES: ${{ format('{0} {1} {2}', vars.RUST_PROD_FEATURES, vars.RUST_TEST_FEATURES, vars.RUST_EXPERIMENTAL_FEATURES) }}
+  RUST_LOG: ${{ vars.RUST_LOG }}
+  RUST_BACKTRACE: ${{ vars.RUST_BACKTRACE }}
+  RUST_LIB_BACKTRACE: ${{ vars.RUST_LIB_BACKTRACE }}
+  COLORBT_SHOW_HIDDEN: ${{ vars.COLORBT_SHOW_HIDDEN }}
+  CARGO_INCREMENTAL: ${{ vars.CARGO_INCREMENTAL }}
+
 jobs:
   # Build the docker image used by the tests.
   #
@@ -85,10 +95,7 @@ jobs:
 
   # Run all the zebra tests, including tests that are ignored by default.
   #
-  # - We run all the tests behind the `getblocktemplate-rpcs` feature as a separated step.
   # - We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests.
-  #
-  # TODO: turn this test and the getblocktemplate test into a matrix, so the jobs use exactly the same diagnostics settings
   test-all:
     name: Test all
     timeout-minutes: 180
@@ -105,32 +112,17 @@ jobs:
       # Run unit, basic acceptance tests, and ignored tests, only showing command output if the test fails.
       #
       # If some tests hang, add "-- --nocapture" for just that test, or for all the tests.
-      - name: Run zebrad tests
-        run: |
-          docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }}
-          docker run -e NETWORK --name zebrad-tests --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "lightwalletd-grpc-tests" --workspace -- --include-ignored
-        env:
-          NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
-
-  # zebrad tests without cached state with `getblocktemplate-rpcs` feature
       #
-  # Same as above but we run all the tests behind the `getblocktemplate-rpcs` feature.
+      # TODO: move this test command into entrypoint.sh
-  test-all-getblocktemplate-rpcs:
+      #       add a separate experimental workflow job if this job is slow
-    name: Test all with getblocktemplate-rpcs feature
-    runs-on: ubuntu-latest-xl
-    needs: build
-    steps:
-      - uses: r7kamura/rust-problem-matchers@v1.4.0
-
-      - name: Inject slug/short variables
-        uses: rlespinasse/github-slug-action@v4
-        with:
-          short-length: 7
-
       - name: Run zebrad tests
         run: |
           docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }}
-          docker run -e NETWORK --name zebrad-tests --tty -e ${{ inputs.network || vars.ZCASH_NETWORK }} ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "lightwalletd-grpc-tests getblocktemplate-rpcs" --workspace -- --include-ignored
+          docker run -e NETWORK --name zebrad-tests --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --workspace -- --include-ignored
+          # Currently GitHub doesn't allow empty variables
+          if [[ -n "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" && "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" != " " ]]; then
+            docker run -e NETWORK --name zebrad-tests-experimental --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.EXPERIMENTAL_FEATURES }} " --workspace -- --include-ignored
+          fi
         env:
           NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
 
@@ -142,7 +134,7 @@ jobs:
   #
   # 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.)
+  # (We activate the test features to avoid recompiling dependencies, but we don't actually run any gRPC tests.)
   test-fake-activation-heights:
     name: Test with fake activation heights
     timeout-minutes: 60
@@ -156,17 +148,17 @@ jobs:
         with:
           short-length: 7
 
+      # TODO: move this test command into entrypoint.sh
+      #       make sure that at least one test runs, and that it doesn't skip itself due to the environmental variable
       - name: Run tests with fake activation heights
         run: |
           docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }}
-          docker run -e NETWORK -e TEST_FAKE_ACTIVATION_HEIGHTS --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --package zebra-state --lib -- --nocapture --include-ignored with_fake_activation_heights
+          docker run -e NETWORK -e TEST_FAKE_ACTIVATION_HEIGHTS --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "zebra-test" --package zebra-state --lib -- --nocapture --include-ignored with_fake_activation_heights
         env:
           TEST_FAKE_ACTIVATION_HEIGHTS: '1'
           NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
 
   # 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
     timeout-minutes: 60
@@ -180,16 +172,15 @@ jobs:
         with:
           short-length: 7
 
+      # TODO: move this test command into entrypoint.sh
       - name: Run zebrad large sync tests
         run: |
           docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }}
-          docker run -e NETWORK --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features lightwalletd-grpc-tests --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_
+          docker run -e NETWORK --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_
         env:
           NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
 
   # 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
     timeout-minutes: 60
@@ -203,10 +194,11 @@ jobs:
         with:
           short-length: 7
 
+      # TODO: move this test command into entrypoint.sh
       - name: Run tests with empty lightwalletd launch
         run: |
           docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }}
-          docker run -e NETWORK -e ZEBRA_TEST_LIGHTWALLETD --name lightwalletd-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features lightwalletd-grpc-tests --package zebrad --test acceptance -- --nocapture --include-ignored lightwalletd_integration
+          docker run -e NETWORK -e ZEBRA_TEST_LIGHTWALLETD --name lightwalletd-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --package zebrad --test acceptance -- --nocapture --include-ignored lightwalletd_integration
         env:
           ZEBRA_TEST_LIGHTWALLETD: '1'
           NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
@@ -255,7 +247,7 @@ jobs:
     #
     # This list is for reliable tests that are run on the `main` branch.
     # Testnet jobs are not in this list, because we expect testnet to fail occasionally.
-    needs: [ test-all, test-all-getblocktemplate-rpcs, test-fake-activation-heights, test-empty-sync, test-lightwalletd-integration, test-configuration-file, test-zebra-conf-path ]
+    needs: [ test-all, test-fake-activation-heights, test-empty-sync, test-lightwalletd-integration, test-configuration-file, test-zebra-conf-path ]
     # Only open tickets for failed scheduled jobs, manual workflow runs, or `main` branch merges.
     # (PR statuses are already reported in the PR jobs list, and checked by Mergify.)
     # TODO: if a job times out, we want to create a ticket. Does failure() do that? Or do we need cancelled()?

.github/workflows/ci-unit-tests-os.patch.yml

@@ -21,19 +21,12 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        # TODO: Windows was removed for now, see https://github.com/ZcashFoundation/zebra/issues/3801
-        # TODO: macOS tests were removed for now, see https://github.com/ZcashFoundation/zebra/issues/6824
         os: [ubuntu-latest]
         rust: [stable, beta]
-        features: ["", " --features getblocktemplate-rpcs"]
+        features: [""]
         exclude:
           - os: macos-latest
             rust: beta
-          - os: macos-latest
-            features: " --features getblocktemplate-rpcs"
-          - os: ubuntu-latest
-            rust: beta
-            features: " --features getblocktemplate-rpcs"
 
     steps:
       - run: 'echo "No build required"'

.github/workflows/ci-unit-tests-os.yml

@@ -78,21 +78,18 @@ jobs:
         # TODO: macOS tests were removed for now, see https://github.com/ZcashFoundation/zebra/issues/6824
         os: [ubuntu-latest]
         rust: [stable, beta]
-        features: ["", " --features getblocktemplate-rpcs"]
+        # TODO: When vars.EXPERIMENTAL_FEATURES has features in it, add it here.
+        #       Or work out a way to trim the space from the variable: GitHub doesn't allow empty variables.
+        #       Or use `default` for the empty feature set and EXPERIMENTAL_FEATURES, and update the branch protection rules.
+        #features: ${{ fromJSON(format('["", "{0}"]', vars.EXPERIMENTAL_FEATURES)) }}
+        features: [""]
         exclude:
-        # We're excluding macOS for the following reasons:
+        # We're excluding macOS beta for the following reasons:
         # - the concurrent macOS runner limit is much lower than the Linux limit
         # - macOS is slower than Linux, and shouldn't have a build or test difference with Linux
         # - macOS is a second-tier Zebra support platform
           - os: macos-latest
             rust: beta
-          - os: macos-latest
-            features: " --features getblocktemplate-rpcs"
-        # getblocktemplate-rpcs is an experimental feature, so we just need to test it on stable Rust
-        # beta is unlikely to fail just for this feature, and if it does, we can fix it when it reaches stable.
-          - os: ubuntu-latest
-            rust: beta
-            features: " --features getblocktemplate-rpcs"
 
     steps:
       - uses: actions/checkout@v4.1.1
@@ -119,7 +116,7 @@ jobs:
         #with:
         #  workspaces: ". -> C:\\zebra-target"
         with:
-          # Split the getblocktemplate-rpcs cache from the regular cache, to avoid linker errors.
+          # Split the experimental features cache from the regular cache, to avoid linker errors.
           # (These might be "disk full" errors, or they might be dependency resolution issues.)
           key: ${{ matrix.features }}
 
@@ -190,17 +187,15 @@ jobs:
       # If some tests hang, add "-- --nocapture" for just that test, or for all the tests.
       - name: Run tests${{ matrix.features }}
         run: |
-          cargo test ${{ matrix.features }} --release --verbose --workspace
+          cargo test --features "${{ matrix.features }}" --release --verbose --workspace
 
       # Explicitly run any tests that are usually #[ignored]
 
       - name: Run zebrad large sync tests${{ matrix.features }}
         # Skip the entire step on Ubuntu and Windows, because the test would be skipped anyway due to ZEBRA_SKIP_NETWORK_TESTS
-        # Currently, this also skips large sync with `getblocktemplate-rpcs`,
-        # but that is already covered by the Docker tests.
         if: matrix.os == 'macos-latest'
         run: |
-          cargo test ${{ matrix.features }} --release --verbose --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_
+          cargo test --features "${{ matrix.features }}" --release --verbose --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_
 
   # Install Zebra with lockfile dependencies, with no caching and default features
   install-from-lockfile-no-cache:

.github/workflows/release-binaries.yml

@@ -26,28 +26,29 @@ jobs:
       dockerfile_path: ./docker/Dockerfile
       dockerfile_target: runtime
       image_name: zebra
-      rust_log: info
+      features: ${{ vars.RUST_PROD_FEATURES }}
+      rust_log: ${{ vars.RUST_LOG }}
     # This step needs access to Docker Hub secrets to run successfully
     secrets: inherit
 
   # The image will be named `zebra:<semver>.experimental`
-  build-mining-testnet:
+  build-experimental:
-    name: Build Release Testnet Mining Docker
+    name: Build Experimental Features Release Docker
     uses: ./.github/workflows/sub-build-docker-image.yml
     with:
       dockerfile_path: ./docker/Dockerfile
       dockerfile_target: runtime
       image_name: zebra
       tag_suffix: .experimental
-      features: "default-release-binaries getblocktemplate-rpcs"
+      features: ${{ format('{0} {1}', vars.RUST_PROD_FEATURES, vars.RUST_EXPERIMENTAL_FEATURES) }}
-      rust_log: info
+      rust_log: ${{ vars.RUST_LOG }}
     # This step needs access to Docker Hub secrets to run successfully
     secrets: inherit
 
   failure-issue:
     name: Open or update issues for release binaries failures
     # When a new job is added to this workflow, add it to this list.
-    needs: [ build, build-mining-testnet ]
+    needs: [ build, build-experimental ]
     # Open tickets for any failed build in this workflow.
     if: failure() || cancelled()
     runs-on: ubuntu-latest

.github/workflows/sub-build-docker-image.yml

@@ -21,19 +21,17 @@ on:
       rust_lib_backtrace:
         required: false
         type: string
+      # defaults to: vars.RUST_LOG
       rust_log:
         required: false
         type: string
-        default: info
+      # defaults to: vars.RUST_PROD_FEATURES
-      # keep these in sync with:
-      # https://github.com/ZcashFoundation/zebra/blob/main/docker/Dockerfile#L83
       features:
         required: false
-        default: "default-release-binaries"
         type: string
+      # defaults to: vars.RUST_TEST_FEATURES (and entrypoint.sh adds vars.RUST_PROD_FEATURES)
       test_features:
         required: false
-        default: "lightwalletd-grpc-tests zebra-checkpoints"
         type: string
       tag_suffix:
         required: false
@@ -49,6 +47,13 @@ on:
         description: 'The image digest to be used on a caller workflow'
         value: ${{ jobs.build.outputs.image_digest }}
 
+
+env:
+  FEATURES: ${{ inputs.features || vars.RUST_PROD_FEATURES }}
+  TEST_FEATURES: ${{ inputs.test_features || vars.RUST_TEST_FEATURES }}
+  RUST_LOG: ${{ inputs.rust_log || vars.RUST_LOG }}
+  CARGO_INCREMENTAL: ${{ vars.CARGO_INCREMENTAL }}
+
 jobs:
   build:
     name: Build images
@@ -143,9 +148,9 @@ jobs:
           labels: ${{ steps.meta.outputs.labels }}
           build-args: |
             SHORT_SHA=${{ env.GITHUB_SHA_SHORT }}
-            RUST_LOG=${{ inputs.rust_log }}
+            RUST_LOG=${{ env.RUST_LOG }}
-            FEATURES=${{ inputs.features }}
+            FEATURES=${{ env.FEATURES }}
-            TEST_FEATURES=${{ inputs.test_features }}
+            TEST_FEATURES=${{ env.TEST_FEATURES }}
           push: true
           # Don't read from the cache if the caller disabled it.
           # https://docs.docker.com/engine/reference/commandline/buildx_build/#options

docker/entrypoint.sh

@@ -43,17 +43,14 @@ set -o pipefail
 : "${TRACING_ENDPOINT_PORT:=3000}"
 # [rpc]
 : "${RPC_LISTEN_ADDR:=0.0.0.0}"
-# if ${RPC_PORT} is not set and ${FEATURES} contains getblocktemplate-rpcs,
+# if ${RPC_PORT} is not set, use the default value for the current network
-# set ${RPC_PORT} to the default value for the current network
 if [[ -z "${RPC_PORT}" ]]; then
-  if [[ " ${FEATURES} " =~ " getblocktemplate-rpcs " ]]; then
   if [[ "${NETWORK}" = "Mainnet" ]]; then
     : "${RPC_PORT:=8232}"
   elif [[ "${NETWORK}" = "Testnet" ]]; then
     : "${RPC_PORT:=18232}"
   fi
 fi
-fi
 
 ####
 # Test Variables
@@ -319,16 +316,15 @@ case "$1" in
         run_cargo_test "${ENTRYPOINT_FEATURES}" "sending_transactions_using_lightwalletd"
 
       # These tests use mining code, but don't use gRPC.
-      # We add the mining feature here because our other code needs to pass tests without it.
       elif [[ "${TEST_GET_BLOCK_TEMPLATE}" -eq "1" ]]; then
         # Starting with a cached Zebra tip, test getting a block template from Zebra's RPC server.
         check_directory_files "${ZEBRA_CACHED_STATE_DIR}"
-        run_cargo_test "getblocktemplate-rpcs,${ENTRYPOINT_FEATURES}" "get_block_template"
+        run_cargo_test "${ENTRYPOINT_FEATURES}" "get_block_template"
 
       elif [[ "${TEST_SUBMIT_BLOCK}" -eq "1" ]]; then
         # Starting with a cached Zebra tip, test sending a block to Zebra's RPC port.
         check_directory_files "${ZEBRA_CACHED_STATE_DIR}"
-        run_cargo_test "getblocktemplate-rpcs,${ENTRYPOINT_FEATURES}" "submit_block"
+        run_cargo_test "${ENTRYPOINT_FEATURES}" "submit_block"
 
       else
           exec "$@"