Zcash PCZT clear-signing for 7.15.0#231
Draft
BitHighlander wants to merge 21 commits into
Draft
Conversation
Bug 1 (TRON display mismatch): fsm_msg_tron.h displayed deprecated to_address/amount fields that are NOT part of raw_data being signed. A malicious host could show one transfer and get a different tx signed. Replaced with a single blind-sign prompt showing only the raw_data size. Bug 5 (ETH RLP leading zeros): ethereum.c hashed nonce/gas/value via hash_rlp_field which preserves leading zero bytes. Per the Ethereum yellow paper, integer fields must have leading zeros stripped. Added hash_rlp_bytes_stripped() and applied it to nonce, gas_price, gas_limit, max_fee_per_gas, max_priority_fee_per_gas, and value. Bug 6 (EIP-712 cancel ignored): confirmName() and confirmValue() cast review() return value to void, allowing signing to continue after user cancels. Added USER_CANCELLED error code and propagate cancellation. Bug 7 (EIP-712 strtoll overflow): strtoll silently wraps values above int64_max. Added errno check, endptr validation, and rejection of negative values for uint types. Bug 8 (token chain_id truncation): TokenType.chain_id was uint8_t, silently truncating chainIds > 255 (Base=8453, Arbitrum=42161, etc.). Widened to uint32_t throughout: struct, tokenByChainAddress(), tokenByTicker().
Issue 1 (High) — wrong sighash: zcash_sign_transparent_inputs was signing
only the inner transparent_sig_digest. ZIP-244 §4.4 requires signing the
full signature_digest:
BLAKE2b("ZcashTxHash_"||branch_id, header||transparent_sig||sapling||orchard)
Fix: wrap transparent_sig_digest via zcash_compute_shielded_sighash before
calling hdnode_sign_digest.
Issue 2 (High) — early sig release: ZcashTransparentSigned was returned
immediately after transparent inputs streamed, before Orchard actions were
signed, before Orchard digest was verified, and before fee was confirmed.
Fix: buffer the ECDSA signatures in zcash_signing.pending_transparent;
send ZcashTransparentSigned at the same final approval gate as ZcashSignedPCZT
(after orchard digest verify + zcash_verify_and_confirm_fee). The transparent
input handler now sends ZcashPCZTActionAck(0) to begin Orchard streaming.
…mat violation make lint — dry-run check matching CI (clang-format --dry-run --Werror) make format — apply clang-format -i in-place Also formatted zxappliquid.c which lint caught locally.
…ts clang-format-20 clang-format-22 and clang-format-20 differ on pointer spacing (e.g. 'T* p' vs 'T *p'). CI uses clang-format-20.1.8; re-format with that exact version so local and CI agree. Makefile now auto-selects clang-format-20 if installed, falls back to clang-format.
eip712.c now includes <errno.h> for strtoll overflow detection (Bug 6 fix). On ARM bare-metal, arm-none-eabi errno.h declares '__errno' as a function prototype. The stray 'extern int errno' in tiny-json.h causes two errors: - function declaration isn't a prototype [-Werror=strict-prototypes] - redundant-decls (redeclaration of __errno) Standard errno.h is sufficient; the manual declaration is removed.
Bug 5 fix (security bugfixes commit) added calls to hash_rlp_bytes_stripped but never defined the function, causing ARM build failure: error: implicit declaration of function 'hash_rlp_bytes_stripped' Adds the implementation: strip leading zero bytes from big-endian integer fields before RLP-encoding, per Ethereum yellow paper §B minimal encoding.
Update to the matching python-keepkey that handles the deferred transparent sig protocol (ZcashPCZTActionAck after last transparent input, followed by ZcashTransparentSigned + ZcashSignedPCZT at the final approval gate).
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.
Summary
Scope
Sapling and memo binding remain outside this branch's scope. Sapling PCZT input is rejected in this path.
Validation
docs/coin-integration/zcash-clearsign-handoff.md.python-integration-tests: 439 tests, 0 failures/errors, 34 skippedZcash Orchard Clear-Signing [NEW] -- 17/17 passedmsg_zcash_sign_pcztscreenshot flows.Notes
No protoc version change or protobuf runtime pin is part of this PR. The Zcash Python protobuf remains in python-keepkey's legacy checked-in descriptor style for firmware CI compatibility.