Skip to content

Zcashd support#1252

Merged
AloeareV merged 6 commits into
devfrom
zcashd_support
Jun 18, 2026
Merged

Zcashd support#1252
AloeareV merged 6 commits into
devfrom
zcashd_support

Conversation

@zancas

@zancas zancas commented Jun 18, 2026

Copy link
Copy Markdown
Member

This PR changes the CONTRACT for:

makers container-test

and

makers integration-test they no longer run zcashd by default!

To run tests INCLUDING zcashd tests use:

makers zcashd_test

zancas added 3 commits June 18, 2026 10:02
  Introduce a default-on, additive Cargo feature `zcashd_support` as the
  deprecation gate for zcashd. Enabling it (the default) keeps today's
  behaviour; `--no-default-features` compiles zcashd out end-to-end. The
  name is `zcashd_support`, not `deprecate_zcashd` — a default-on feature
  whose name implies removal would invert Cargo's additive semantics.

  Gating surface (two layers; the third is deliberately left alone):

  - Production parsing (zaino-fetch): gate the GetPeerInfo::Zcashd variant
    and ZcashdPeerInfo / ServiceFlags / PeerStateStats. Self-contained —
    the deserializer falls through to the zebrad branch when off.
  - Test validator infra (zaino-testutils): gate ValidatorKind::Zcashd,
    the Zcashd/ZcashdConfig imports, the ZCASHD_BIN/ZCASH_CLI_BIN/
    ZCASHD_CHAIN_CACHE_DIR statics, impl ValidatorExt for Zcashd,
    ZcashdDualFetchServices + launch_zcashd_dual_fetch_services, the
    mod zcashd smoke tests, and the state-backend guard. Downstream
    zcashd tests gated in walletless-tests and wallet-tests.
  - Wire fields zcashd_build / zcashd_subversion (LightdInfo) are left
    unchanged: they are prost-generated lightwallet-protocol field names,
    not zcashd capability. Rename tracked upstream at lightwalletd#566.

  Propagation forwards `zcashd_support` through zaino-fetch -> state ->
  serve -> zainod and zaino-testutils -> walletless-tests / wallet-tests,
  each in its own `default`. Because a default-on feature only drops out
  when every consumer pulls the carrier crates with default-features =
  false, all three workspaces' [workspace.dependencies] set that on the
  zaino crates that carry the feature.

  DRY: the activation-height selection, previously matched inline and
  duplicated in zaino-testutils and wallet-tests, is now
  ValidatorKind::default_activation_heights — the Zcashd arm is gated in
  one place and the enum stays exhaustive when the variant compiles out.

  Enforce the off-state: `makers check-no-zcashd` runs
  `cargo check --no-default-features --all-targets` across all three
  workspaces, wired into a no-zcashd-build CI job, so the disabled build
  cannot bit-rot.

  Decision recorded in docs/adr/0001-zcashd-support-feature-gate.md.

  Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
  Introduce a default-on, additive Cargo feature `zcashd_support` as
  the
  deprecation gate for zcashd. Enabling it (the default) keeps today's
  behaviour; `--no-default-features` compiles zcashd out end-to-end.
  The
  name is `zcashd_support`, not `deprecate_zcashd` — a default-on
  feature
  whose name implies removal would invert Cargo's additive semantics.

  Gating surface (two layers; the third is deliberately left alone):

  - Production parsing (zaino-fetch): gate the GetPeerInfo::Zcashd
  variant
    and ZcashdPeerInfo / ServiceFlags / PeerStateStats. Self-contained
  —
    the deserializer falls through to the zebrad branch when off.
  - Test validator infra (zaino-testutils): gate ValidatorKind::Zcashd,
    the Zcashd/ZcashdConfig imports, the ZCASHD_BIN/ZCASH_CLI_BIN/
    ZCASHD_CHAIN_CACHE_DIR statics, impl ValidatorExt for Zcashd,
    ZcashdDualFetchServices + launch_zcashd_dual_fetch_services, the
    mod zcashd smoke tests, and the state-backend guard. Downstream
    zcashd tests gated in walletless-tests and wallet-tests.
  - Wire fields zcashd_build / zcashd_subversion (LightdInfo) are left
    unchanged: they are prost-generated lightwallet-protocol field
  names,
    not zcashd capability. Rename tracked upstream at lightwalletd#566.

  Propagation forwards `zcashd_support` through zaino-fetch -> state ->
  serve -> zainod and zaino-testutils -> walletless-tests /
  wallet-tests,
  each in its own `default`. Because a default-on feature only drops
  out
  when every consumer pulls the carrier crates with default-features =
  false, all three workspaces' [workspace.dependencies] set that on the
  zaino crates that carry the feature (never on zaino-proto, whose own
  default `heavy` must survive --no-default-features).

  DRY: the activation-height selection, previously matched inline and
  duplicated in zaino-testutils and wallet-tests, is now
  ValidatorKind::default_activation_heights — the Zcashd arm is gated
  in
  one place and the enum stays exhaustive when the variant compiles
  out.

  Tests default to the zebrad-only world: container-test.sh runs
  `cargo nextest run --no-default-features`, so `makers container-test`
  and `makers integration-test` (and the per-workspace tasks) exercise
  the no-zcashd build by default. New `makers
  container-test-with-zcashd`
  sets CONTAINER_TEST_WITH_ZCASHD=1 to run the zcashd-backed suite.

  Enforce the off-state so it cannot bit-rot: `makers check-no-zcashd`
  runs `cargo check --no-default-features --all-targets` across all
  three
  workspaces and guards, via tools/scripts/check-zaino-proto-heavy.rs,
  that zaino-proto's `heavy` feature stays enabled under
  --no-default-features (a stray `default-features = false` on a
  zaino-proto edge would otherwise silently drop
  zebra-state/zebra-chain/
  which from the test build). Wired into a no-zcashd-build CI job.

  Decision recorded in docs/adr/0001-zcashd-support-feature-gate.md.

  Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d_support

  The test tasks default to `--no-default-features` (zcashd compiled out)
  now that zcashd is being deprecated. Add `makers zcashd_test` to run the
  whole suite WITH the feature on: it exports CONTAINER_TEST_WITH_ZCASHD=1
  and runs the existing container-test (production crates) and
  integration-test (walletless + wallet workspaces) flows. container-test.sh
  honours that env var for every nested `cargo nextest run`, and the var is
  inherited by the makers subprocesses integration-test spawns, so the
  entire suite builds with default features on.

  Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nachog00

Copy link
Copy Markdown
Contributor

Why adding a new interface? couldn't we keep the "run integration tests" command unified and have it accept an optional flag like --with-zcashd or something like that?

@zancas

zancas commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Because developers hate adding a single flag, or changing a single invocation. This makes it just work without human intervention.. the way it always has.

And we're NOW in the situation where testing against zcashd is not the default flow.. it's the special case (driven in part by the 2x runtime overhead).

@nachog00

Copy link
Copy Markdown
Contributor

If the --with-zcashd flag defaults to false, existing calls to makers integration-tests would still work just like before. Moreover, future improvements/changes to the integration tests run logic wouldn't need to be propagated to the new integration tests running command.

@zancas

zancas commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Fine.. please push a commit with the proposed change. I like it. It's DRYer.

Unify the zcashd test interface: instead of a separate
container-test-with-zcashd task that duplicates container-test's config,
both container-test and integration-test now accept --with-zcashd as an
inline flag (the CONTAINER_TEST_WITH_ZCASHD env var is still honoured).
The zcashd_test convenience task is kept as a shorthand.
@nachog00

Copy link
Copy Markdown
Contributor

Done. Now we need a new reviewer.

@zancas zancas left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UtAck.. nice upgrade @nachog00 !

@AloeareV AloeareV merged commit 2aabc51 into dev Jun 18, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants