fix(zcash): smooth, always-moving progress bar during shielded signing#303
Merged
Conversation
Shielded Zcash (Orchard/PCZT) signing blocks the device on the host while it generates zk-proofs between action messages. The progress bar was drawn only on discrete milestones (layoutProgress), so it sat frozen at 0% through the first — longest — host proof, then jumped in large steps. Users read the frozen bar as the device having failed. Add a timer-driven 'trickle' progress animation: - layoutProgressTrickle(desc, base, target) eases the bar asymptotically from the last real milestone toward the next (add = span*t/(t+TAU)): fast at first, slowing as it nears the target, never actually reaching it — so it is always visibly moving but never falsely shows 100%. - layout_animate_poll(), called from usbPoll() (device usb.c + emulator udp.c), pumps the animation off the existing ANIMATION_PERIOD timer while the device blocks on host I/O. Gated on a dedicated trickle_active flag, so every other flow (confirm dialogs, PIN entry, home screen) is untouched. Wire it into the Zcash signing FSM: - zcash_send_action_ack() arms the trickle toward the milestone the requested action will reach — the single choke point covering the initial wait and every inter-action wait. - fsm_msgZcashPCZTAction() stops it on arrival so the exact per-action milestone (and the final fee confirm) draw cleanly; layout_clear_animations() clears it on every layoutHome/exit path. The animation is queued only during the Orchard action-wait windows, so it can never draw over the transparent-output or fee confirm dialogs.
Review catch (P1): trickle_progress_callback() drew via
animating_progress_handler(), which calls layout_clear() ->
layout_clear_animations() -- so every frame dequeued the animation and reset
trickle_active. The bar advanced at most once, then froze; and the initial
layoutProgress(..., 0) issued after arming wiped the trickle before its first
frame.
- Split the renderer: progress_render() clears only the framebuffer
(layout_clear_static) and redraws, leaving the animation queue intact.
animating_progress_handler() keeps its historical queue-clearing behaviour
for one-shot callers by wrapping progress_render(). The trickle callback and
layoutProgressTrickle() now render via progress_render(), and the arm draws
an immediate first frame.
- Ordering: draw the initial/transition static layoutProgress("Signing
Zcash", 0) BEFORE zcash_send_action_ack() at all three sites (SignPCZT and
the two transparent->action transitions), so arming the trickle is the last
display action and is never cleared by a following static draw.
Review round 2 (both P2): - Abort paths left the trickle running: fsm_msgCancel() and fsm_msgClearSession() call zcash_signing_abort() without a subsequent layoutHome(), so cancelling during an Orchard wait left the animation timer and OLED redraw loop active after signing ended. Centralize cleanup: zcash_signing_abort() now calls layoutProgressTrickleStop() (idempotent), so every abort path kills the animation regardless of caller. - Long waits eventually looked frozen anyway: the eased fill is floor()'d to 248 bar pixels, so with the asymptotic curve the final visible pixel transition lands after roughly 18-73s per segment and later frames were identical. Add a perpetual sweep marker: progress_render_ex() takes a 0..999 marker phase and draws a small dim block cycling across the unfilled region (1.6s period, phase = elapsed % period). The marker loops forever, so the display keeps visibly changing for arbitrarily slow proofs; static one-shot draws (progress_render / animating_progress_handler) pass -1 and are pixel-identical to before.
Review round 3 (P2): the sweep marker was confined to the unfilled region and gated on span_w > marker_w + 2, so for the final action of a 16-action tx (base 937 permil) the fill reached 952 permil within ~380ms, span_w hit 10, and the marker vanished -- after which the eased fill pixel-saturated and the display froze again. Sweep the marker across the FULL inner track instead: travel is constant (~238px) regardless of fill level, so it never disappears. Mid-gray (0x99) reads as a darker notch over the 0xff fill and a brighter block over the 0x00 remainder, visible at any progress value. The marker shrinks only if the whole track is narrower than 8px (never on this display). Also: clang-format pass over the three files CI flagged (comment re-wraps and pointer-spacing only; no functional change).
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
1 participant
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.
Problem
Shielded Zcash (Orchard/PCZT) signing feels stuck. The device signs each action's RedPallas signature (fast) but then blocks waiting for the host to generate the next zk-proof (slow). The progress bar was only redrawn on discrete milestones via
layoutProgress(), which draws once and returns — nothing animates between calls. Result:n_actions).Users read the frozen bar as the KeepKey having failed mid-sign.
Fix — timer-driven "trickle" progress
A progress animation that is always visibly moving but never falsely completes:
layoutProgressTrickle(desc, base, target)(layout.c) eases the bar asymptotically from the last real milestone toward the next:add = span * t / (t + TAU)— fast at first, slowing as it nears the target, never reaching it. So it moves instantly and continuously, but can't show 100% until signing actually completes.layout_animate_poll()— called fromusbPoll()(deviceusb.c+ emulatorudp.c) — pumps the animation off the existingANIMATION_PERIOD(20 ms) timer while the device blocks on host I/O. Gated on a dedicatedtrickle_activeflag, so every other flow is untouched (confirm dialogs, PIN entry, home screen).Wired into the signing FSM (
fsm_msg_zcash.h)zcash_send_action_ack()— the single choke point — arms the trickle toward the milestone the requested action will reach. Covers the initial wait and every inter-action wait uniformly.fsm_msgZcashPCZTAction()stops the trickle on arrival, so the exact per-action milestone (and the final fee confirm) draw cleanly.layout_clear_animations()clears it on everylayoutHome()/exit path.The animation is queued only during the Orchard action-wait windows, so it can never draw over the transparent-output review or fee-confirm dialogs.
Safety / scope
trickle_active, so non-Zcash flows are byte-for-byte unaffected.Verification
lib/board/layout.c,lib/board/udp.c,lib/firmware/fsm.c(which includesfsm_msg_zcash.h) all compile clean with the real emulator toolchain flags (-fsyntax-only -Wall).