Fix Trezor signing in hypecli and build it in CI - #82
Open
ifdario wants to merge 14 commits into
Open
Conversation
Replace the unused Error enum with a focused ApiError struct. Errors from reqwest, serde_json, and alloy propagate as-is through anyhow so callers can downcast to the real type. Add Response::into_default() helper to collapse repeated match blocks, and an error_handling example.
Add patched trezor-client and alloy-signer-trezor to enable: - EIP-712 typed data signing via Trezor streaming protocol - Passphrase handling fix for standard wallets - Hardware wallet test-signer command - signer-trezor feature for alloy
# Conflicts: # src/hypercore/http.rs
SendToken Display now emits name:token_id as the API expects. Added missing Debug derives on internal deserialization structs.
start_gas and end_gas are Decimal rather than strings, so the parse calls and as_deref no longer compile.
The workspace was pinned to alloy 1.x while hypecli was on 2.x, putting two alloy majors in one graph and breaking every signer type. iroh 1.0 moved mdns address lookup out into iroh-mdns-address-lookup, so that becomes its own dependency. The trezor patches now point at git rather than sibling checkouts, so the build no longer depends on the layout of the parent directory. alloy-signer-trezor 2.1.0 did not satisfy alloy 2.4's ^2.4.1 requirement and was silently dropped from the graph, so the upstream crate compiled instead and typed-data signing fell back to a sign_hash stub that always returns UnsupportedOperation. hypecli also gets a tracing subscriber, inactive unless RUST_LOG is set, so the Trezor message exchange can be observed.
Point the trezor-client patch at feat/eip712-empty-passphrase, which acks a PassphraseRequest with an empty passphrase unless the device sets passphrase_always_on_device. The previous branch keyed off passphrase_protection, which the device has already confirmed by the time it sends the request, so the standard wallet was prompted every session. The tracing filter defaulted to ERROR when RUST_LOG was unset, which printed one "Ledger device not found" line per derivation path while scanning. Default to off instead.
GossipPrioritySlot's doc example still parsed start_gas and end_gas as strings after they became Decimal, so it no longer described the API. list-hip3 called perp_dexs, which is deprecated in favour of perp_dexes.
The empty-passphrase fix now lives on feat/eip712-streaming-protocol, so the temporary branch is no longer needed.
main had already landed the iroh 1.0 migration, the Decimal fix in priority-fee-bid and the perp_dexes rename, so this takes main's version of those and keeps only the Trezor work from this branch. Kept the alloy 2.4 pin rather than main's "2": alloy-signer-trezor is patched from a fork that declares an exact version, and a floating major silently drops the patch when alloy moves ahead of it.
CI ran only `cargo test --lib` on the root crate, so hypecli was never built. An alloy major split, an iroh 1.0 break and a silently dropped [patch.crates-io] entry all reached a release without turning CI red. Adds `cargo check --all-targets` to the root job, which covers examples, and a second job that builds hypecli and fails if Cargo.lock contains a [[patch.unused]] entry. That entry is the only signal cargo gives when a patched crate stops satisfying the dependent's version requirement and the upstream crate is silently substituted.
Replaces arrows with plain ASCII `->` and em-dashes with a comma, or a colon where the text introduces a definition. Two of these were runtime output rather than comments, in positions.rs and list-outcomes.rs, and now print a plain hyphen.
Same treatment as the source comments. CHANGELOG.md is left alone: its entries are a record of past releases, not prose to restyle.
PR #7038 was squashed from six commits to three. The content is unchanged, but the old revision is no longer reachable from any branch.
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.
Makes Trezor signing work in hypecli, and closes the CI gap that let it break unnoticed.
Trezor
hypeclipatches in two forks to get EIP-712 typed data signing. One of them was not applying: the fork declaredalloy-signer-trezor 2.1.0while alloy 2.4 requires^2.4.1, so cargo silently substituted the upstream crate, whosesign_hashreturnsUnsupportedOperationand which has nosign_dynamic_typed_data. Typed data signing could not succeed, and nothing failed to compile. The fork is now2.4.1.Separately, both passphrase handlers in
trezor-clienttestedFeatures.passphrase_protectionbefore deciding how to answer aPassphraseRequest. The device only sends that message when passphrase protection is already on, so the test was always true and the empty-passphrase branch was unreachable. Every session deferred to on-device entry, prompting even for a standard wallet with no passphrase set. Fixed in the fork by testingFeatures.passphrase_always_on_deviceinstead, which is the only case the device enforces.Confirmed working against hardware.
CI
CI ran one command,
cargo test --libon the root crate. hypecli was never built. That is why an alloy major split, an iroh 1.0 break and the dropped patch all reached a release without turning CI red.cargo check --all-targetson the root job, which covers examplesCargo.lockcontains[[patch.unused]], the only signal cargo gives when a patch stops applyingAlso
2.4rather than2: a floating major silently drops the patch when alloy moves ahead of the forkprintln!output rather than commentsNote
hypecli depends on trezor/trezor-firmware#7038, which is still unmerged upstream. Until it lands, the
[patch.crates-io]entries are required to build.