merge queue: embarking main (92f4732) and [#7753 + #7773] together

.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.
+      #
+      # TODO: move this test command into entrypoint.sh
+      #       add a separate experimental workflow job if this job is slow
       - 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.
-  test-all-getblocktemplate-rpcs:
-    name: Test all with getblocktemplate-rpcs feature
-    runs-on: ubuntu-latest-xl
-    needs: build
-    steps:
-      - uses: r7kamura/[email protected]
-
-      - 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/[email protected]
@@ -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:
-    name: Build Release Testnet Mining Docker
+  build-experimental:
+    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"
-      rust_log: info
+      features: ${{ format('{0} {1}', vars.RUST_PROD_FEATURES, vars.RUST_EXPERIMENTAL_FEATURES) }}
+      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
-      # keep these in sync with:
-      # https://github.com/ZcashFoundation/zebra/blob/main/docker/Dockerfile#L83
+      # defaults to: vars.RUST_PROD_FEATURES
       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 }}
-            FEATURES=${{ inputs.features }}
-            TEST_FEATURES=${{ inputs.test_features }}
+            RUST_LOG=${{ env.RUST_LOG }}
+            FEATURES=${{ env.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

book/src/dev/continuous-integration.md

@@ -33,17 +33,34 @@ We try to use Mergify as much as we can, so all PRs get consistent checks.
 Some PRs don't use Mergify:
 - Mergify config updates
 - Admin merges, which happen when there are multiple failures on the `main` branch
-- Manual merges
+  (these are disabled by our branch protection rules, but admins can remove the "don't allow bypassing these rules" setting)
+- Manual merges (these are usually disabled by our branch protection rules)
 
 We use workflow conditions to skip some checks on PRs, Mergify, or the `main` branch.
-For example, some workflow changes skip Rust code checks.
+For example, some workflow changes skip Rust code checks. When a workflow can skip a check, we need to create [a patch workflow](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks)
+with an empty job with the same name. This is a [known Actions issue](https://github.com/orgs/community/discussions/13690#discussioncomment-6653382).
+This lets the branch protection rules pass when the job is skipped. In Zebra, we name these workflows with the extension `.patch.yml`.
+
+Branch protection rules should be added for every failure that should stop a PR merging, break a release, or cause problems for Zebra users.
+We also add branch protection rules for developer or devops features that we need to keep working, like coverage.
+
+But the following jobs don't need branch protection rules:
+* Testnet jobs: testnet is unreliable.
+* Optional linting jobs: some lint jobs are required, but some jobs like spelling and actions are optional.
+* Jobs that rarely run: for example, cached state rebuild jobs.
+* Setup jobs that will fail another later job which always runs, for example: Google Cloud setup jobs.
+  We have branch protection rules for build jobs, but we could remove them if we want.
+
+When a new job is added in a PR, use the `#devops` Slack channel to ask a GitHub admin to add a branch protection rule after it merges.
+Adding a new Zebra crate automatically adds a new job to build that crate by itself in [ci-build-crates.yml](https://github.com/ZcashFoundation/zebra/blob/main/.github/workflows/ci-build-crates.yml),
+so new crate PRs also need to add a branch protection rule.
 
 ### Pull Requests from Forked Repositories
 
 GitHub doesn't allow PRs from forked repositories to have access to our repository secret keys, even after we approve their CI.
 This means that Google Cloud CI fails on these PRs.
 
-Unril we [fix this CI bug](https://github.com/ZcashFoundation/zebra/issues/4529), we can merge external PRs by:
+Until we [fix this CI bug](https://github.com/ZcashFoundation/zebra/issues/4529), we can merge external PRs by:
 1. Reviewing the code to make sure it won't give our secret keys to anyone
 2. Pushing a copy of the branch to the Zebra repository
 3. Opening a PR using that branch