Skip to content

Surface consensus-branch mismatch diagnostics in the Sync Error sheet - #2372

Draft
pacu wants to merge 2 commits into
mainfrom
fix/sync-error-consensus-branch-diagnostics
Draft

Surface consensus-branch mismatch diagnostics in the Sync Error sheet#2372
pacu wants to merge 2 commits into
mainfrom
fix/sync-error-consensus-branch-diagnostics

Conversation

@pacu

@pacu pacu commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Android counterpart of zodl-ios#1948 / zodl-ios#1947.

The scenario

A lightwalletd server publishes a consensus branch ID the app doesn't expect — in practice a server on NU6.3/Ironwood (0x37a5165b) while the SDK expects NU6.2 (0x5437f330). Sync can never progress.

Branch IDs are unordered constants, so the error itself cannot say which side is stale. In the observed case the app was behind, not the server, so neither the copy nor the code blames the server.

The gap this fixes

Android's gap was the inverse of iOS's. The SDK's message was already fine — already hex, already naming both remedies — but the user was shown none of it.

Two problems, and the second was the bigger one:

  1. The sheet showed nothing specific. SyncErrorView rendered a generic sync_error_message ("check your connection, restart the app") plus four buttons. That copy is actively wrong here: restarting cannot help.
  2. These failures never reached that sheet at all. NavigateToErrorUseCase.isSyncError() matched only ResponseException 5xx, code 3200 transport errors, and UninitializedTorClientException. MismatchedConsensusBranch is a CompactBlockProcessorException, so it fell through to the generic error bottom sheet — a truncated stack trace with no Switch server button, i.e. no route to the one remedy that works.

Support also received only getStackTrace(null), with the useful sentence buried in a Java trace.

What changed

  • Routing — the server-compatibility family (MismatchedConsensusBranch, MismatchedNetwork, MismatchedSaplingActivationHeight) now routes to the Sync Error sheet, where Switch server already exists. No button was added.
  • Sheet — for this family only, the generic message is replaced by a written explanation, followed by one labelled fact per line: the selected server, both sides of the disagreement, and the error type. Generic and transient sync errors are untouched and gain no detail, since retrying is the correct remedy there.
  • Support report — leads with the same facts in readable form, above the stack trace (which is kept). Unlocalized, matching AppInfo/EnvironmentInfo in the existing report, since support staff read it.
  • New ServerCompatibilityError is the single source of truth shared by the routing gate, the sheet and the email.

Values are read off the SDK exception's properties, not parsed out of its message — that wording is not a contract.

Design

ZashiSimpleListItem (the existing label/value row, already used for fact lists in Swap and Review Transaction) inside the existing sheet structure. It wraps values in a SelectionContainer, so users can copy a branch ID straight into a support message. Only ZashiColors/ZashiTypography/Spacer(dp) tokens; no new components, no hardcoded colors. Strings in values/ and values-es/. A second @PreviewScreens preview covers the diagnostics variant.

Not built on

Synchronizer.validateConsensusBranch() (broken, SDK#1405) and ext/ConsensusBranchId.kt (hardcoded enum stopping at CANOPY — fromHex("37a5165b") returns null and both sides read as "unknown").

Verification

Run against the SDK change composite-built in (-PSDK_INCLUDED_BUILD_PATH), on JDK 21.

./gradlew :ui-lib:testZcashmainnetStoreDebugUnitTest — passing, exit code 0.

471 tests, 0 failed, 0 skipped

NavigateToErrorUseCaseTest:    5 tests, 0 failed
ServerCompatibilityErrorTest:  7 tests, 0 failed
SyncErrorVMTest:               6 tests, 0 failed

./gradlew ktlint detektAll — passing, exit code 0.

18 new tests cover: reading the mismatch off each of the three exception types, finding it through a wrapped cause chain, ignoring unrelated errors, the routing decision for each type (and that unrelated failures are still left alone), the rendered fact list including the server line, omission of the server line when no endpoint is selected, omission of a side the SDK could not name, and that Switch server still routes to ChooseServerArgs with no button added.

./gradlew :ui-lib:pixel2TargetZcashmainnetStoreDebugAndroidTest — passing, exit code 0.

Starting 4 tests on pixel2Target
Finished 4 tests on pixel2Target
BUILD SUCCESSFUL

SyncErrorViewTest renders SyncErrorContent directly on a Gradle managed device (Pixel 2, API 36, Google APIs ARM64) and asserts the generic sheet keeps its default message with no detail rows, the explanation replaces that message for an incompatible server, the server and both branch IDs render, and Switch server is present with no extra button.

BEFORE/AFTER screenshots are attached in a comment below.

⛔ Still blocked on the SDK

Depends on zcash/zcash-android-wallet-sdk#2053, which promotes those constructor params to vals. That PR targets maint/v2.5.x and merges forward; since this app tracks ZCASH_SDK_VERSION=2.6.5-SNAPSHOT, it will pick the change up with no version bump.

The passing run above used a local composite build of that SDK branch. CI here will fail until #2053 merges forward and a snapshot publishes — hence still draft.

UI changes

Before

sync-error-BEFORE-generic

After

sync-error-AFTER-incompatible-server

🤖 Generated with Claude Code

pacu and others added 2 commits July 25, 2026 16:07
When a lightwalletd server publishes a consensus branch ID the app does
not expect, sync can never progress. The SDK's message was already
well-written and already hex, but none of it reached anyone: the sheet
rendered a generic "check your connection" message and the support email
carried only a raw Java stack trace.

Worse, these failures never reached that sheet at all.
NavigateToErrorUseCase.isSyncError() matched only ResponseException 5xx,
3200 transport errors and UninitializedTorClientException, so a
CompactBlockProcessorException fell through to the generic error bottom
sheet, which shows a truncated stack trace and offers no way to change
server. Route the server-compatibility family to the Sync Error sheet
instead, where Switch server already exists. No button is added.

Read the mismatched values off the SDK exception's new properties rather
than parsing them out of its message, whose wording is not a contract.
The sheet shows a localized explanation, the selected server, both sides
of the disagreement and the error type; support reports lead with the
same facts in English, matching the rest of the report. Generic and
transient sync errors are unchanged and gain no detail, since retrying
is the remedy there.

Branch IDs are unordered constants, so neither the copy nor the code
claims which side is stale. In the case that prompted this the app was
behind, not the server.

Requires the SDK-side change promoting those constructor params to vals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tests launched their state collector on backgroundScope without a
dispatcher, so it never ran before the assertion read state.value. The
assertions saw the StateFlow's initial value, which is built with a null
endpoint, and the two tests covering the server line failed on a
list that was correct except for the missing "Server" row.

Use UnconfinedTestDispatcher(testScheduler), matching SwapSlippageVMTest.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pacu
pacu requested review from nesence-m, noop-sk and zcash-harry and removed request for nesence-m and noop-sk July 25, 2026 22:01
@pacu
pacu marked this pull request as ready for review July 25, 2026 22:01
@pacu
pacu marked this pull request as draft July 25, 2026 22:04
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.

1 participant