Skip to content

feat(zcash): connect transparent watcher on Node 22 - #16

Open
a-shannon wants to merge 4 commits into
rosen-bridge:devfrom
a-shannon:feat/zcash-transparent-watcher
Open

a-shannon wants to merge 4 commits into
rosen-bridge:devfrom
a-shannon:feat/zcash-transparent-watcher

Conversation

@a-shannon

@a-shannon a-shannon commented Sep 26, 2026 •

Copy link
Copy Markdown

Add transparent Zcash watcher support

Register the Zcash scanner and observation extractor, configure the network and native inspector binding, and preserve fail-closed synchronization. The shared token map can contain Zcash without requiring a Zcash RPC configuration on an existing-chain watcher. Readiness distinguishes catch-up from a halt, preserves the first sanitized error and refuses progress after incomplete extraction.

The operator requirements cover diagnosis, controlled recovery, native builds, upgrade qualification, input references and the proposed runtime split.

Validation

  • The startup/readiness correction passed 42 focused tests, TypeScript and Rollup on Node 22.18.0 with the retained scanner-compatible graph.
  • The process test kills a real child after persisting an observation but before completing its block. Fresh SQLite processes refuse a persistent fault, recover without duplicates after repair, and roll back two blocks before replacements. It passes on the exact Linux Node 22/AOE 1.0.10 graph; independent Windows execution also passed the 228-test watcher suite.
  • Independent source review covers those corrections and the updated operator documentation. These fixtures qualify local scanner/database recovery, not live commitment reconciliation or an existing-chain runtime release campaign. Runtime source and dependency files are unchanged by the latest documentation commit.

Companion changes and release limits

utils #8 and scanner #12 add bounded native batches and protect rollback during inspection/persistence. The native pin recognizes NU7 with offline parser/v5-signature tests. guard-service #22 rechecks the branch after authorization and before sending; ui #31 renders input references without sender-address links. NU7 node admission and activation remain unqualified.

The operator follow-up accepts restart coverage and favors a shared runtime release qualified on existing chains before Zcash. The documentation maps the runtime/Zcash file split and the required synthetic existing-chain journey and packaging checks. The current committed lock still reflects the old manifest; a runtime-only lock is required first, followed by the Zcash consumer lock after producer releases.

The Node 22 work overlaps temporarily closed #12. Maintainers must choose a separate v1 runtime contribution or watcher-service-2 before extraction. Base #11 shares registration files but implements a different chain. The coordinating review owns producer releases, consumer locks, hosted CI and deployment qualification. No Mainnet readiness is claimed.

@odiseusme

Copy link
Copy Markdown

Reviewed at c46fc0d (scanner #12 at e25c2dd13d, health-check #4 at fc79eac192), from the watcher-operator side.

Startup failure can affect every chain: src/init.ts:52-65 requires zcash.network to be regtest/testnet/mainnet whenever the shared token map lists zcash, and config/default.yaml ships it as ''. So once the tokens file includes zcash, an upgraded Ergo/Cardano/Bitcoin/Binance watcher refuses to start unless the operator adds a zcash section. Could this get a working default, or at least a release note?

Sticky halt, cause discarded: FailClosedScannerLogger counts every error and never resets, and it is the logger for the scanner, observation extractor and rosen-extractor. A single error recorded through that logger makes assertZcashScannerHealthy() throw before every commitment and reveal until restart, and the log text is dropped, so the operator only sees a generic update error. Is a permanent halt the intended contract? Where does the operator read the cause?

Inspector per transaction: the native inspector is re-hashed and run with execFileSync for every transaction before any lock-script check. That blocks the event loop during each block and makes catch-up scale with total chain tx count. Combined with the latch above, one unparseable transaction wedges the watcher. A cheap prefilter on vout paying the lock script before spawning would avoid most of this.

Runtime changes for all watchers: Node 20.11 → 22.18, npm 11.6.2 pinned globally, pkg → @yao-pkg/pkg, rollup ignoreGlobal, a patch-package patch to extended-typeorm's SQLite driver, abstract-observation-extractor pinned to exact 1.0.10, and the shared job-order change in commitmentCreation.ts (redeem now starts before the commitment job). These affect every running watcher, not just Zcash ones. Could they go in a separate PR, qualified on existing chains first?

NU7: ZIP 259 invalidates v4, uses branch ID 0x77190AD9, and with ZIP 218 moves block spacing from 75 s to 25 s. With branches: [] shipped empty, every operator hand-types branch IDs; a 75 s scan interval spans ~3 target blocks; lag thresholds and tip gating need to be re-evaluated for NU7; the inspector must parse v6; and a job signed for the pre-NU7 branch cannot simply be submitted after activation, because v5/v6 signatures commit to the consensus branch ID. What is the plan, given mainnet is tentatively ~5 weeks out?

Catch-up gating: the tip in assertZcashScannerHealthy is set at the start of each update(), so commitments and reveals are blocked for the whole of any multi-block catch-up, logged as generic job failures. A distinct health status would help operators tell this apart from a real fault.

Smaller items: no tests in this PR (gating, config parser, init change); scanner/utils fixtures are regtest v5 only, with no v4/v6 or rollback-at-depth test. fromAddress becomes box:<txid>.<index>, so the watcher UI/API and third-party dashboards should be checked for whether they present it as a sender address. Operator docs should name a minimum Zebra version (6.4.0/6.4.1 were recalled; 6.4.2 fixes a remote crash), give sizing, and say where the inspector binary comes from and whether it is reproducible.

The loopback-only RPC restriction and fail-closed genesis/network checks are good choices — thanks for those.

@a-shannon

Copy link
Copy Markdown
Author

Thanks for the detailed review. The corrections are in 3b52cec, with operator requirements and upgrade boundaries.

  • Startup: address-codec initialization now handles a shared Zcash token entry on an existing-chain watcher without requiring Zcash scanner configuration. The active Zcash scanner still requires its explicit network configuration.
  • Sticky halt: the first sanitized cause is retained and exposed through zcash-scanner-readiness, alongside a separate catch-up state. The halt remains deliberate: the upstream scanner can log an extraction error and return, so a later successful RPC does not establish safe recovery. Failed extraction now also prevents the processed-block cursor from advancing. Recovery requirements are documented; restart alone is not recovery. The existing commitment/redeem ordering is restored.
  • NU7: this concern is valid. The current native library rejects the proposed 77190ad9 branch despite already parsing v6. The operator plan covers activation vectors, pending signed withdrawals, scan timing and reorgs. ZIP 259 still leaves activation heights unassigned. Native v4 tests already exist; v6 and activation coverage remain incomplete. Zebra 6.4.2 is the minimum patched node baseline, subject to integration qualification. This change does not add NU7 support.
  • Inspector cost: this remains open. An RPC vout skip would bypass the existing raw-byte integrity contract and can hide deposits when the redundant RPC projection is wrong. A bounded asynchronous batch preserving native validation is the useful next implementation. Catch-up now has a distinct health state and logging, so normal catch-up is distinguishable from a fault.
  • Shared runtime and operator scope: the Node/runtime changes still need Rosen's decision on a separate PR versus the watcher-service-2 route, plus qualification across existing chains. The operator document also covers native builds, node requirements and the output-descriptor display contract. It does not claim that external UI/dashboard consumers have been qualified.

Validation: 42 focused tests, TypeScript and Rollup pass on Node 22.18.0 with the retained scanner-compatible abstract-observation-extractor@1.0.10 graph. The tests include the real SQLite/scanner processing and restart boundary. Independent code and documentation reviews are complete. This correction has not received a fresh-install or complete chain-roundtrip run.

The most useful next reviews would be failure/restart with a retained scanner database, pending withdrawals across activation, and the descriptor display in the watcher API/UI. Throughput profiling should guide the bounded inspector batch.

@a-shannon

Copy link
Copy Markdown
Author

The follow-up implementation is now pushed across the existing PRs:

  • This watcher adds a real process-kill/SQLite restart regression, including persistent-fault refusal, duplicate-free replay and a two-block rollback. The operator guide records the tested recovery boundary.
  • utils #8 and scanner #12 replace per-transaction synchronous inspection with bounded native batches. Raw bytes remain authoritative; a late failure persists nothing, and rollback during native work or storage cannot leave orphaned observations or report successful processing.
  • The native pin now recognizes the actual NU7 branch. Offline v5/v6 inspection and v5 signature tests pass. guard-service #22 refuses a branch change during authorization before transport, while retaining observation of historical confirmed payments. This is still not NU7 node-admission qualification.
  • ui #31 renders box:<txid>.<index> as an input reference in the actual shared and desktop/mobile components, with no sender-address link.

The most useful next review is the shared-runtime/v1-versus-Service2 decision and the release/operator contract: exact producer versions, node/activation qualification, sustained backlog sizing and reconciliation of downstream liabilities after a deep reorg.

@odiseusme

Copy link
Copy Markdown

Thanks. Read both updates and checked f478f6c. With the cursor no longer advancing past a failed extraction, and the new process-kill/SQLite regression covering persistent-fault refusal and duplicate-free replay, that covers my concern about restart policies. From the operator side on the open runtime question: I'd prefer the Node 22 / npm / pkg / typeorm-patch changes shipped separately from Zcash and qualified on existing chains first, so an upgrade for everyone else doesn't depend on the Zcash review.

@a-shannon

Copy link
Copy Markdown
Author

Thanks for confirming that the restart concern is covered, and for the operator view on release scope.

I've made the proposed split concrete in 67ff561: Node/npm/pkg, Rollup, the SQLite import patch, scanner-v2 dependency pin and shared release tooling form the runtime contribution; Zcash dependencies, configuration and admission gates follow it. The plan requires a clean runtime-only lock and an existing-chain synthetic path through observation, commitment/reveal and permit return before broader chain-family and packaged-artifact qualification.

The committed lock in this PR still reflects the old manifest, so this is not an already-qualified standalone runtime release. Given the maintainer discussion on the temporarily closed #12, the remaining choice is a separate v1 runtime contribution or the watcher-service-2 route. The operator preference for separating the runtime is now recorded; branch extraction should follow that maintainer choice.

This branch has not been deployed

No deployments
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.

2 participants