Sync zsa1 with upstream Zebra v5.2.0 - #155
Open
dmidem wants to merge 165 commits into
Open
Conversation
…0398) * Try unsetting `github_token` for lychee * fix(ci): reduce lychee concurrency to avoid GitHub rate limits
fix the sync for regtest
Dispatches a `zebra-interop-request` event to zcash/integration-tests on code changes, using the enterprise-owned Z3 Integration Testing GitHub App for cross-org authentication. Integration test results are reported back as commit statuses on the triggering SHA.
…n#10357) * Update non-compiling nu7/tx_v6 tests * ZIP-235 Implementation * ZIP-235 Tests * Remove zip234 configuration flag, as this code only handles zip235
* Add changelog entry for Zebra v4.3 * Update ECC dependency guide to reference ZODL instead * Update estimated release height to 3286000 * Add conditional zip235 handling and InvalidZip233Amount error variant * Fix miner subsidy computation in changelog for version 6.0.1 * Add ZIP-235 support under zcash_unstable flag in changelog * Mention fix miner reward computation in `zebra-rpc` changelog * Bump zebra-chain zebra-consensus and zebra-rpc crate versions
Cache parsed checkpoint lists
…hFoundation#10411) Add profiling build profile and update profiling instructions
…shFoundation#10380) fix(ci): migrate lints from .cargo/config.toml to [workspace.lints] The setup-rust-toolchain action sets RUSTFLAGS="-D warnings" by default, which overrides .cargo/config.toml rustflags entirely. This silently dropped all custom lints (clippy::print_stdout, unsafe_code, missing_docs, etc.) since ZcashFoundation#9883. Move lint configuration to [workspace.lints] in Cargo.toml, which is immune to RUSTFLAGS overrides. Closes ZcashFoundation#10378
Update Mainnet checkpoints
Update Testnet checkpoints
…ed on only mined ids (ZcashFoundation#10425) fix proof skip bug Co-authored-by: Conrado Gouvea <conradoplg@gmail.com>
…cashFoundation#10429) Add NU5 branch ID strategy and update V5 transaction generator
bumps zebrad version
…Foundation#10432) Update README to reference zebrad installation tag v4.3.0
…Foundation#10431) Adjust arbitrary input data length range to start from 1 instead of 0
…tion#10407) Bumps [rustls-webpki](https://github.com/rustls/webpki) from 0.103.9 to 0.103.10. - [Release notes](https://github.com/rustls/webpki/releases) - [Commits](rustls/webpki@v/0.103.9...v/0.103.10) --- updated-dependencies: - dependency-name: rustls-webpki dependency-version: 0.103.10 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add Docker Compose setup for Zebra and S-NOMP mining pool * Extract s-nomp entrypoint script into dedicated shell file * Add healthcheck for zebra and update service dependencies in compose * Remove extra blank line before version 6.0.0 section in changelog * Add blank lines in mining README documentation
…tion#10456) * ci(release): enforce conventional commit format on PR titles Add a PR title validation workflow using action-semantic-pull-request. Since PRs are squash-merged, the PR title becomes the commit message on main — this enforces consistent commit history for automated changelog generation and version detection. Allowed types: feat, fix, perf, refactor, build, chore, docs, test, ci, style, revert, release. Scopes are optional and match crate names. The error message includes format examples and links to CONTRIBUTING.md. * fix(ci): use pull_request trigger instead of pull_request_target pull_request_target runs with base branch context and elevated permissions — unnecessary for title validation and a known attack vector for fork PRs. Matches the trigger pattern used by all other PR-triggered workflows in this repo. Drop statuses:write in favor of contents:read (minimum permissions). * fix(ci): add sticky comment feedback for invalid PR titles Post a helpful comment when the title is invalid, auto-delete it when fixed. Matches the pattern used by better-auth. Requires pull_request_target with pull-requests:write (job-level only) — no code checkout, no secrets access, only reads the title via API. * fix(ci): pin sticky-comment to v3.0.3, add zizmor annotation Fix zizmor findings: - SHA now matches exact tag v3.0.3 (was pointing to v3.0.2 labeled as v3) - Add ignore annotation for pull_request_target — this workflow does not checkout code or access secrets, only reads the PR title and writes a comment * docs(ci): clarify null comparison in PR title check The action only sets error_message on validation failure. When the title is valid, the output key is absent (null in GHA expressions). Add comments explaining this per the action's README, preempting false positive reviews.
…on#10455) * fix(ci): add dependency advisory scanning, license checking, and cargo-vet enforcement Enable three previously missing security checks in CI: 1. cargo-deny advisories: scans dependencies against the RustSec advisory database. Yanked crates are denied; 5 unmaintained advisories are ignored with rationale (all transitive via abscissa_core/structopt or direct). 2. cargo-deny licenses: enforces a 15-entry allowlist of permissive licenses. Verified against all 32 unique license expressions in the dependency tree. 3. cargo-vet: enforces supply chain audits on dependency changes. Upgraded from v0.9 to v0.10, added 4 import sources matching librustzcash (bytecodealliance, embark-studios, fermyon, isrg), and cleaned up 18 stale policy entries for crates no longer in the dependency tree. The lint.yml `continue-on-error` for advisories was already wired up but never activated — this commit adds `advisories` and `licenses` to the cargo-deny matrix and adds a new `vet` job. * fix(ci): add CDDL-1.0 to license allowlist for inferno crate The `inferno` crate (flamegraph generation, behind `--all-features`) uses CDDL-1.0. This license only appears in CI's `--all-features` matrix variant, which is why it was missed in local testing. CDDL-1.0 (Common Development and Distribution License) is a permissive open-source license originally from Sun Microsystems.
…10460) * Update SECURITY.md with public encryption key * Update email format in SECURITY.md to pass markdown-lint
…hFoundation#10459) * feat(ci): automate checkpoint updates and end-of-support height Add artifact upload to the integration test workflow so checkpoint output is captured as a downloadable file instead of only appearing in step logs. Add a checkpoint-update workflow triggered by weekly integration test completion that downloads the artifacts, appends new checkpoint entries, updates ESTIMATED_RELEASE_HEIGHT from the latest mainnet checkpoint, validates the files, and opens a PR for human review. Checkpoints are consensus-critical — the automated PR still requires maintainer review before merge. * fix(ci): add zizmor ignore for workflow_run trigger * fix(ci): address Codex review findings for checkpoint automation - Fix workflow name: "CI integration tests on GCP" → "Integration Tests on GCP" - Add `actions: read` permission for downloading cross-workflow artifacts - Add `branches: [main]` filter to prevent PR runs from triggering checkpoint updates - Add `upload-checkpoint-artifact` to delete-instance needs to prevent race condition where the VM is deleted before checkpoint data is extracted * docs(ci): clarify checkpoint artifact naming and skip conditions * fix(ci): rename checkpoint artifacts to match repo file names * fix(ci): address review findings in checkpoint automation - Fix artifact upload glob: checkpoints-*.txt did not match the renamed main-checkpoints.txt / test-checkpoints.txt files (empty artifact) - Combine two SSH sessions into one for checkpoint extraction - Rewrite validate-checkpoints.sh as a single-pass awk script - Replace em dashes with semicolons/commas per style guide * fix(ci): address code review findings for checkpoint automation - Add missing SSH key setup (shimataro/ssh-key-action + keygen) - Update GCP auth/setup-gcloud from v2 to v3 to match other jobs - Add upload-checkpoint-artifact to deployment-success allowed-skips to prevent alls-green failures on non-checkpoint tests - Replace GNU-specific sed with portable awk for number formatting - Fix indentation inconsistency in check-artifacts step * feat(ci): add workflow_dispatch trigger for checkpoint testing * fix(ci): move template expansions to env vars for zizmor compliance
Add RocksDB dependency and dynamic linking Include librocksdb-dev in the setup-zebra-build action and in the Dockerfile, and define ROCKSDB_LIB_DIR to enable dynamic linking, reducing build time.
ZcashFoundation#10474) * fix(ci): rewrite checkpoint artifact upload to scp a pre-captured file PR ZcashFoundation#10459 introduced an upload-checkpoint-artifact job that rediscovers the test container with `docker ps -a --filter name=${TEST_ID}` and greps HEIGHT HASH lines out of `docker logs`. Every run since the merge has reported "Extracted 0 checkpoint lines" because the filter does not match the `klt-` prefixed container name injected by Container-Optimized OS, and the anchored regex is brittle against nextest output wrapping. Capture the lines during the test-result job while the correct container ID is still in `steps.find-container.outputs.CONTAINER_ID`, save them to `/tmp/checkpoints.txt` on the instance, and retrieve them with a single `gcloud compute scp` in the upload job. The container-lookup step is gone and the grep runs with `-oE` so it tolerates nextest prefixes. * fix(ci): drop --ssh-flag from gcloud compute scp invocation `gcloud compute scp` does not accept `--ssh-flag` (that flag is for `gcloud compute ssh`). Each `--ssh-flag=...` is being parsed as a positional source path, producing 'Source(s) must be remote when destination is local'. The capture step verified our architecture works: 'Captured 61 checkpoint lines'. The transfer is tiny so no keep-alive is needed; just drop the flags.
…oundation#10475) The `run-stateful-tests` label had to be removed and re-added after every push to re-trigger the integration tests. Two coordinated fixes: - Expand the `pull_request` trigger to `[labeled, synchronize, reopened]` so the workflow fires on push, not only when a label is toggled. - Replace `github.event.label.name == 'run-stateful-tests'` with `contains(github.event.pull_request.labels.*.name, 'run-stateful-tests')` at all four gate sites. The old expression is null outside label events; the new expression reads the PR's current label set on any `pull_request` activity, matching the pattern used for the `A-release` label in `tests-unit.yml`.
…shFoundation#10472) * fix(ci): remove dead gcr-cleaner job from GCP cleanup workflow The `clean-registries` job pinned a Docker image from a deleted GCP project (`us-docker.pkg.dev/gcr-cleaner/gcr-cleaner/gcr-cleaner-cli`), so every daily run failed with "Project #78287296261 has been deleted" for ~12 days. Artifact Registry image cleanup is now handled server-side by a native cleanup policy on the `zebra` repository: keep the 2 newest versions, delete anything older than 504h / 21 days. Policy is defined in the cloud-foundation-fabric Terraform stage. Semantically identical to the retired gcr-cleaner args. Drop the job, its Docker login, the `DELETE_IMAGE_HOURS` env var, the `clean-registries` entry on `delete-resources-success.needs`, and the related documentation. The disk/instance/template/image cleanup jobs that use gcloud CLI are unaffected. * fix(ci): drop transitive comment about registry cleanup
…ion#10471) * fix(ci): grant issues: write to failure-issue jobs The failure-issue jobs in zfnd-ci-integration-tests-gcp.yml and zfnd-deploy-nodes-gcp.yml inherit only contents: read from the workflow-level permissions, so jayqi/failed-build-issue-action failed with "Resource not accessible by integration" whenever a real failure triggered the job. Add a job-scoped permissions block (contents: read, issues: write) so auto-issue creation works again, without broadening the token at workflow scope. * fix(ci): restore contents: read on book.yml build job Job-level permissions replace the workflow-level block entirely, so the Build Docs job was running with only id-token: write and statuses: write. actions/checkout currently works because the public repo allows anonymous HTTPS clone, but the job should declare the contents: read it actually needs.
…o we don't forget to remove it
The previous version of `v6_coinbase_transaction_with_enable_zsa_flag_fails_validation` used `Transaction::new_v6_coinbase`, which was based on `Transaction::new_coinbase` from `zebra-chain/src/transaction/builder.rs`. That builder module was removed completely upstream. Coinbase construction now lives in `zebra-rpc` as `TransactionTemplate::new_coinbase`, which is not accessible from `zebra-consensus`. Keep the test in `zebra-consensus`, where the consensus rule is implemented, and reuse a valid V6 coinbase from the OrchardZSA workflow blocks instead. The test replaces its Orchard shielded data, sets `ENABLE_ZSA`, and verifies that it is rejected with `CoinbaseHasEnableZSA`.
The check was commented out during the upstream merge with a FIXME because V5 and V6 use different generic Orchard shielded-data types. This resolves that FIXME. Add `Transaction::orchard_proof_size_is_canonical()` to hide that difference and return: * `None` for transactions without Orchard shielded data; * `Some(true)` for a canonical Orchard proof; * `Some(false)` for a non-canonical Orchard proof. Use this version-independent accessor in `zebra-consensus` so the check works for both V5 `OrchardVanilla` and V6 `OrchardZSA` transactions.
Replace the existing OrchardZSA workflow vector blocks with the versions generated by QED-it/zcash_tx_tool#100. The new blocks were downloaded from the PR’s CI artifacts and are compatible with Zebra v5.2.0. They were regenerated using the updated Orchard, Halo2, and librustzcash dependencies and include newly generated proofs and possibly other updated transaction data.
The test was ignored during the upstream merge because it failed with the old OrchardZSA workflow blocks. Regenerate the blocks with the updated `zcash_tx_tool` setup and remove the ignore. The test passes now.
`zebra-chain` test code uses Orchard's `bundle::testing` and `issuance::testing` modules, which are now gated behind the `test-dependencies` feature. Enable that feature so plain `cargo check --tests` and related test builds can compile the OrchardZSA arbitrary helpers.
…lementation is V5-only
Select the all-tests nextest profile in ci-basic. The workflow ran profile.default, which also runs the stateful and network-dependent tests that belong to the integration workflows, including get_peer_info. Scope retries to the acceptance binary. They were attached to three test names, which did not cover the tests that actually failed, and a profile-wide setting would hide real bugs in unit tests. Run trusted_chain_sync_handles_forks_correctly on its own. It restarts a node against live Mainnet and expects a chain tip within 200s; it passes in isolation and fails at ~250s when other zebrad processes compete for CPU and network. Cap acceptance tests at 10 minutes each. A test that hung rather than failed ran until the workflow timeout and took the whole job with it. Fix the Windows skip: it used `filter`, which only scopes an override's settings and deselects nothing, so the listed tests still ran.
Move the ZSA-specific ZIP-317 terms into a new `zsa` submodule gated behind the ZSA feature flag, so that `conventional_actions` stays close to upstream. Replace the `nAssetCreations` stub, fixed at zero because ZIP-227 defines it in terms of the Global Issuance State, with `nReferenceNotes`: the number of Issue Actions whose first note is a reference note. A reference note is required on the first issuance of an Asset and optional afterwards, so this count is never lower than the true number of Asset creations and the fee cannot be underpaid. Deriving it from the transaction alone keeps the conventional fee computable from public transaction data, as ZIP-317 requires, and avoids querying `zebra-state` from the verifier.
Merged
* fix(deps): allow-list every patched git source for cargo-deny `deny.toml` has `unknown-git = "deny"` and `lint.yml` runs `cargo deny check sources`, so every git source in `Cargo.lock` must appear in `allow-git`. Three did not: - `QED-it/halo2`, newly introduced by the v5.2.0 merge (halo2_gadgets, halo2_proofs, halo2_poseidon), replacing the `zcash/halo2` entry; - `zcash/sinsemilla` and `zcash/zcash_note_encryption`, both still in the patch table but dropped from the list while resolving the merge conflict. Also drop the now-unused `zcash/librustzcash.git` entry: the patch table points every librustzcash crate at `QED-it/librustzcash`. * build(deps): restore upstream's incrementalmerkletree 0.8.2 requirement The v5.2.0 merge kept the fork's older `0.8.1`. Both resolve to 0.8.2 in `Cargo.lock`, so this is drift, not a behaviour change. * ci: restore retries for sync_large_checkpoints_mempool_mainnet The nextest rework dropped this test when moving the retry overrides from `profile.default` to `profile.all-tests`. It is `#[ignore]`d, so only the `--run-ignored=all` run is affected. Also grouped with the other live-Mainnet sync tests so they don't compete for peers. * fix(consensus): verify NU7 vanilla Orchard bundles with the vanilla key The v5.2.0 merge collapsed two independent axes of verifier selection into one: upstream's circuit era (pre/post-NU6.2, GHSA-jfw5-j458-pfv6), keyed on the network upgrade, and the fork's flavor, keyed on the bundle. Only the era survived, as `Nu7 => &VERIFIER_ZSA`. V5 transactions stay valid at NU7 and carry vanilla bundles, so they were verified against the OrchardZSA key, which cannot verify a vanilla proof: any NU7 block with a V5 Orchard transaction would be rejected. Both cfg flags are on by default, so the default build was affected. Route on the bundle at the call site, as before the merge, and restore `verifier_for` to upstream's era-only mapping. The routing test asserted the NU7-to-ZSA mapping rather than catching it, so it is fixed too. * test(consensus): guard vanilla Orchard routing at NU7 Add a regression test for the previous commit. Both the correct and the incorrect key reject the only vanilla fixture available — a pre-NU6.2 proof from the mainnet test blocks — so the outcome cannot distinguish them and the test asserts the verifier choice instead. To make that choice observable, move the flavor match out of the call site into `verifier_for_bundle`, which composes flavor over `verifier_for`. This also leaves `verifier_for` identical to upstream, so it merges cleanly next time. Verified by reintroducing `Nu7 => &VERIFIER_ZSA`: the new test and the existing routing test both fail. The ZSA direction stays covered end to end by `check_orchard_zsa_workflow`. A true acceptance test for a V5 Orchard transaction at NU7 still needs a post-NU6.2 vanilla fixture, which the test vectors do not have. * fix(chain): count distinct Assets in the ZIP-317 issuance fee `nReferenceNotes` stands in for `nAssetCreations`, which cannot be computed from the transaction alone. Counting Issue Actions overcounts whenever two actions in one transaction carry a reference note for the same Asset: an Asset is created at most once, so those cannot each imply a creation. Count distinct Asset bases instead. Still a floor on `nAssetCreations`, so the fee cannot be underpaid, but a tighter one. * fix(chain): require an NU7 branch ID in V6 transactions The V6 arm reused the V5 guard verbatim during the v5.2.0 merge, so it rejected only pre-NU5 branch IDs while claiming to reject pre-NU5 ones for a version that is only valid from NU7. `verify_v6_transaction_network_upgrade` already rejects everything before NU7, so nothing invalid was accepted, but the parse-time floor did not match the rule it was copied to enforce. `NetworkUpgrade` derives `Ord` over variants declared in activation-height order, so `< Nu7` admits exactly NU7 and later.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates
zsa1with upstream Zebra v5.2.0, the last NU6.2-only release before Ironwood/NU6.3.