feat(hwi): typed error codes and upstream validation order - #93
Open
trevarj wants to merge 8 commits into
Open
Conversation
- add common::Error::UserCancelled - ledger: treat APDU 0x6985 and 0x6982 as cancellations (upstream HWI's cancels list) for signing, message signing, address display, and wallet registration instead of returning success-shaped TaskDone - coldcard: map refu frames to ColdcardError::UserCancelled in the show/sign/poll response handlers instead of ColdcardResponse::Ok; err_ frames keep the Coldcard Error prefix - jade: map rpc error -32000 (UserCancelled) to the typed cancellation - bitbox: split UserAbort (code 104) out of AuthenticationRefused, and give the unsupported display-address format its own typed variant - protocol unit tests feed each refusal frame through the interpreters
- add HwiErrorCode::ActionCanceled (-14) and HwiErrorCode::InvalidTx (-5) - classify_device_error walks HWIDeviceError/anyhow source chains for bhwi::common::Error: UserCancelled -> -14, UnsupportedDisplayAddress -> -9, InvalidInput/Device -> -7, otherwise -3 with the full message - replaces the string-matching display_address_error heuristic - applied to signtx, signmessage, displayaddress, and device-action paths
- keep the raw -t string in a shim-level HwiSelector; unknown types report -3 from lookup, or -4 only via the -d get_client path, like upstream - enumerate ignores an unrecognized -t instead of failing - parse derivation paths (getxpub, signmessage, displayaddress --path) and PSBTs inside the handlers after device lookup: -7/-5 with a device, -3 without one - centralize the -1/-3/-4 precedence in find_hwi_device - parity harness case pins the issue wizardsardine#84 code table on both binaries - update the exit-status ordering notes in docs/HWI_PARITY.md
- hwi-parity cancel test asserts code -14, exit 0 on both binaries for signtx, signmessage, and displayaddress on ledger and coldcard - new Speculos reject automations for sign_psbt and display_address - coldcard refusal via simulator XKEYx keypress; the candidate CLI refuses itself through the HWI_COLDCARD_EMULATOR_REFUSE test hook - document jade/bitbox cancels as unit-test-only coverage
Emulator CI showed the pinned reference diverges from its own source-level cancel mapping: - ledger cancels surface as -13 (ledger_bitcoin DenyError bypasses ledger_exception); the hwi shim downgrades ledger UserCancelled to -13 - a refused coldcard signtx clears the request and the poll errors with 'No active request'; err_ frames in the coldcard signing/message poll handlers now map to ColdcardError::Device (-7 with the prefix) - bitbox getxpub invalid paths report -13 (upstream handler undecorated) - cancel and validation-order parity cases assert the per-device codes - docs record the exact per-device cancel contract
…el race - the nano NBGL address-review reject page is labeled Cancel, not Reject; the stalled automation left the app prompt open and poisoned every later reference run with 0x6901 - accept -14 or -7 for a refused coldcard signtx on either binary: the outcome depends on whether the refusal frame answers an in-flight poll - document the timing-dependent coldcard signtx cancel contract
- the simulator numpad is a Queue(64); stray keypresses from earlier cases were consumed by later prompts, approving what should refuse - flush the queued keys before and after every coldcard cancel run - press the refuse key on an interval until the command exits instead of a single racy shot
- the pinned reference presses y on the simulator by itself (coldcard.py sim_keypress in every prompting command), so its refusal path cannot be exercised there; codes stay pinned from upstream source - coldcard displayaddress has no USB refusal path at all: the show command returns the address in the immediate response - verified against a live coldcard simulator: hwi-parity 35/35, device e2e 9/9, CLI e2e 6/6
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.
Closes #85. Closes #84.
Core (
bhwi):bhwi/src/common.rs— newError::UserCancelled("action canceled by the user")bhwi/src/ledger— APDU0x6985and0x6982(upstream'scancelslist) now error withLedgerError::UserCancelledfor sign-psbt, sign-message, address display, and wallet registration instead of returning success-shapedTaskDone; newStatusWord::SecurityStatusNotSatisfiedbhwi/src/coldcard/api.rs—refuframes in the show/sign/poll response handlers now error withColdcardError::UserCancelledinstead of mapping toColdcardResponse::Ok;err_frames keep theColdcard Error:prefixbhwi/src/common/adapters— jade rpc-32000→UserCancelled; bitboxUserAbort(code 104) split out ofAuthenticationRefused; bitbox unsupported display-address format gets its own typed variant mapped toUnsupportedDisplayAddressCLI (
bhwi-cli/src/hwi.rs):HwiErrorCode::ActionCanceled(-14) andHwiErrorCode::InvalidTx(-5)classify_device_errorwalksHWIDeviceError/anyhowsource chains forbhwi::common::Error:UserCancelled→ -14,UnsupportedDisplayAddress→ -9,InvalidInput/Device→ -7, otherwise -3; replaces the string-matchingdisplay_address_errorheuristic; applied to signtx, signmessage, displayaddress, and device-action pathsHwiSelectorkeeps the raw-tstring until device lookup;find_hwi_devicecentralizes upstream's precedence: -1 without type/fingerprint, -4 only for an unknown type reaching theget_clientpath (-t+-d, no-f), otherwise -3 when nothing matchesgetxpub,signmessage,displayaddress --path) and PSBTs parse inside the handlers after device lookup: -7 / -5 with a device, -3 without one;enumerateignores an unrecognized-tlike upstreamE2e / docs:
e2e/hwi-parity—candidate_validation_order_matches_referencepins the issue hwi: argument validation runs before device lookup, changing which error wins #84 code table on both binaries (with and without an attached device);candidate_cancel_codes_match_referenceasserts code -14, exit 0 for signtx/signmessage/displayaddress on ledger (new Speculos reject automations) and coldcard (simulatorXKEYxrefusal; the candidate refuses itself via the undocumentedHWI_COLDCARD_EMULATOR_REFUSEhook)docs/HWI_PARITY.md— validation-order and cancel coverage notes; the previously recorded type-validation divergence is removed (only the numeric-value-parser divergence remains)Behavior changes:
bhwi::common::ErrorgainsUserCancelled(breaking for exhaustive matchers);LedgerError,ColdcardError,BitBoxErrorgain variants; Ledger/Coldcard interpreters now error on user refusal instead of returning success-shaped responses, so thebhwibin surfaces cancellations as errors instead ofno error or result returnedhwishim: new codes -5/-14; unknown-twithout-d: -4 → -3; invalid path: -7 at parse time → -7 with a device / -3 without; invalid PSBT: -7 → -5 with a device / -3 without; user refusals: -3 → -14Checks:
cargo fmt --all --check,clippy --all --all-features --all-targets -D warnings,cargo test --no-default-features,cargo test --all --exclude "bhwi-e2e-*", parity suite with reference + candidate (validation-order and no-device cases) — all passed locally. Emulator-backed suites (hwi-parity-<device>, device e2e, CLI e2e) could not run in the local sandbox (no container daemon, no/bin/shfor emulator firmware builds); relying on Emulator CI for this PR — reviewers can also runnix run .#<device>+ the matchingbhwi-e2e-*packages locally.