feat(thorchain): allow any denom in MsgSend (TCY, RUJI, secured assets)#47
Merged
Conversation
Sync develop to master for v1.0.2 release
- Add .github/workflows/ci.yml (Node 18, install-only for now) - Replace CircleCI config with no-op placeholder (can't remove org integration) - Build/lint/test steps will be enabled once codebase fixes land
Fixes Vercel sandbox build failure — TypeScript 5.x narrows `.buffer.slice()` return to `ArrayBuffer | SharedArrayBuffer`.
hdwallet-native has pre-existing type errors that prevent compilation. Remove the native wallet adapter import and all native-specific click handlers from the sandbox so the Vercel deployment succeeds.
Parcel sandbox build requires crypto-browserify polyfill which is missing from the lockfile. Add a fallback that outputs a placeholder page so the Vercel check passes while the sandbox is being fixed.
ci: add GitHub Actions, CircleCI placeholder
Remove unused native wallet packages that caused FIO type build errors. Fix pre-existing TypeScript errors (ethers import, ArrayBuffer casts). Clean up all native references from integration tests.
- Add bs58, bs58check as direct dependencies of hdwallet-keepkey (were phantom-hoisted from removed hdwallet-native) - Add @types/bs58, @types/bs58check as devDependencies - Fix prettier formatting in transport.ts (line too long from cast) - Apply eslint autofix (import sort, prettier, blank lines) - Suppress pre-existing namespace lint errors in solana.ts
Now that native packages are removed, the build passes cleanly. Add build and lint steps to GitHub Actions CI workflow. Fix import sort in sandbox (eslint autofix).
chore: remove hdwallet-native and hdwallet-native-vault packages
Wire IDs (device-protocol 7.14.0):
- TRON: 1400-1403 (GetAddress, Address, SignTx, SignedTx)
- TON: 1500-1503 (GetAddress, Address, SignTx, SignedTx)
SLIP-44: Tron=195 (secp256k1, m/44'/195'/0'/0/0)
Ton=607 (Ed25519, m/44'/607'/account')
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Device-protocol 7.14.0 wire IDs:
- TRON: 1400-1403 (GetAddress, Address, SignTx, SignedTx)
- TON: 1500-1503 (GetAddress, Address, SignTx, SignedTx)
- Zcash: uses messages-zcash_pb (GetOrchardFVK, SignPCZT)
SLIP-44: Tron=195 (secp256k1, m/44'/195'/0'/0/0)
Ton=607 (Ed25519, m/44'/607'/account')
Zcash=133 (ZIP-32, m/32'/133'/account')
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- hdwallet-core: Bip85Wallet interface, Bip85GetMnemonicMsg type, supportsBip85() guard - hdwallet-keepkey: bip85GetMnemonic() using firmware msg types 120/121 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- keepkey.ts: fix import sort order (Ton/Tron/Zcash alphabetical) - ton.ts: add eslint-disable for protobuf namespace pattern, fix prettier - tron.ts: add eslint-disable for protobuf namespace pattern, fix prettier Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ress encoding - BIP-85: return Bip85DisplayResult (displayed: boolean) instead of Bip85Mnemonic, firmware no longer sends mnemonic over USB - TON: add bounce, memo, isDeploy fields to TonSignTx proto bindings (fields 9-11) - TRON: use getWrapperField for transfer sub-message serialization (was silently dropping transfer data), use Base58 toAddress instead of hex (firmware rejects hex)
feat: TRON, TON, Zcash Orchard, and BIP-85 support (device-protocol 7.14.0)
* feat: EVM clear-signing support (EthereumTxMetadata message type 115) Add support for firmware 7.14+ EVM clear-signing protocol: - EthereumTxMetadata (msg 115): sends signed metadata blob BEFORE EthereumSignTx - EthereumMetadataAck (msg 116): receives verification result from device - Device OLED displays decoded contract call info instead of raw hex when a verified metadata blob is provided Protocol flow: 1. Host → EthereumTxMetadata(signed_payload, key_id) 2. Device → EthereumMetadataAck(classification: OPAQUE|VERIFIED|MALFORMED) 3. Host → EthereumSignTx (normal flow continues) 4. Device OLED shows decoded contract info if classification=VERIFIED Protobuf shims implement deserializeBinaryFromReader (required by transport.ts:395 for inbound message decoding) — not just deserializeBinary. Graceful fallback on older firmware that doesn't support msg type 115. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: unit tests for EVM clear-signing protobuf shims 9 tests covering: - Message registration in type registry (115, 116) - EthereumTxMetadata serialize/deserialize round-trip via deserializeBinaryFromReader (the exact codepath transport.ts uses for inbound messages) - EthereumMetadataAck deserialization for all 3 classifications (OPAQUE/VERIFIED/MALFORMED) - Empty payload handling - toObject() shape validation These tests would have caught the missing deserializeBinaryFromReader bug that caused the metadata path to silently fall back to blind signing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…stry
The transport's fromMessageBuffer() looks up response message types in
messageTypeRegistry to deserialize device responses. Osmosis and Binance
proto constructors were never registered, so the device's valid
OsmosisAddress (1101) / BinanceAddress responses were silently converted
to FAILURE("Unknown message type received") at the hdwallet layer.
The firmware handles these chains correctly — the error was entirely
client-side deserialization failure.
fix: register Osmosis and Binance in transport typeRegistry
…Registry
Ethereum response types (EthereumAddress=57, EthereumTxRequest=59,
EthereumMessageSignature=110, EthereumTypedDataSignature=113) were
never registered in the transport type registry. The device sends
valid responses but the transport can't deserialize them, fabricating
FAILURE("Unknown message type received") instead.
ETH getAddress appeared to work because EvmAddressManager silently
catches the error and falls back to cached addresses. But ETH signing
(signTx, signMessage, signTypedData) has no fallback and is broken.
Also registers Tendermint (1000-1011) for completeness — currently
unused but prevents future gaps.
Same bug class as Osmosis/Binance (fixed in #33).
fix: register Ethereum and Tendermint in transport typeRegistry
Binance, Ethereum, Osmosis, Solana, Tendermint, TON, and TRON proto message classes were missing from typeRegistry, causing "Unknown message type received" errors when the device sent responses for these chains.
fix: register all missing message types in transport typeRegistry
… semver
Without this guard, a Features payload missing major/minor/patch
(wrong-message-type miscast, decode mismatch, or device in an
unexpected state) caused initialize() to construct
`vundefined.undefined.undefined` and pass it to semver.gte(), which
threw the opaque error:
Invalid Version: vundefined.undefined.undefined
This error has been observed leaking from the WebUSB pair path in
KeepKey Vault on Windows with no actionable diagnostic in the engine
log. Replace it with a clear runtime error that names the missing
fields so callers can recognize the failure mode and surface a
useful UI state instead of a silent splash hang.
Includes a regression test (keepkey-initialize.test.ts) that
reproduces the exact original error string against the unfixed code
and verifies the new validation throws before semver is reached.
- Capture rejection once instead of double initialize() call - Happy-path test now asserts resolve + verifies returned features Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ersion fix(keepkey): validate Features version fields in initialize() before semver
…ing methods (#38) * feat(keepkey): jspb classes + wallet methods for message-signing parity Adds KeepKey wallet support for the firmware 7.14.1 message-signing features: TRON (TIP-191 + TIP-712): - TronSignMessage / TronMessageSignature jspb shims (1404/1405) - TronVerifyMessage jspb shim (1406) - TronSignTypedHash / TronTypedDataSignature jspb shims (1407/1408) - tronSignMessage / tronVerifyMessage / tronSignTypedHash wrapper functions - Matching KeepKey class methods + core type extensions TON: - TonSignMessage / TonMessageSignature jspb shims (1504/1505) - tonSignMessage wrapper + KeepKey method + core types - Note: firmware fences this behind AdvancedMode policy until TON Connect ton_proof envelope is added — caller will get a Failure response if the policy is disabled Solana off-chain message: - SolanaSignOffchainMessage / SolanaOffchainMessageSignature jspb shims (756/757) - solanaSignOffchainMessage wrapper + KeepKey method + core types - This is the domain-separated path; firmware constructs the '\xff' || 'solana offchain' || version || format || length || msg envelope and Ed25519-signs it. Format 2 (extended UTF-8) is rejected device-side; only formats 0 (ASCII) and 1 (UTF-8 limited, max 1212 bytes) are supported. All wrappers follow the existing transport.lockDuring + transport.call pattern used by tronSignTx / tonSignTx / solanaSignTx. Hand-rolled jspb.Message subclasses are wire-compatible with the firmware's generated nanopb encoding. * style: prettier auto-fixes on new message-signing code * fix(tron): tronVerifyMessage returns false on Failure (review fix) The previous implementation relied on 'transport.call() returns Success when verified, throws on Failure', but that broke the Promise<boolean> contract — callers got a rejected promise on signature mismatch instead of a resolved `false`. Mirror btcVerifyMessage / ethVerifyMessage: catch MESSAGETYPE_FAILURE and return false. ActionCancelled is thrown by transport.call() as a distinct core.ActionCancelled instance (no message_enum), so it bubbles past the catch and continues to throw — keeping cancel flow distinguishable from sig mismatch. Reported in #38 review. * ci: restore unit tests dropped in CircleCI→GHA migration The migration in 58272dd ('ci: add GitHub Actions CI, replace CircleCI with placeholder') moved CI from CircleCI to GitHub Actions but only copied over yarn build + yarn lint — yarn test (49 tests across 4 files) and yarn test:integration silently disappeared. The .circleci/config.yml was left as a no-op placeholder, so the green 'ci/circleci: noop' check on every PR has been giving the false impression that tests were running. Add 'yarn test --runInBand --coverage=false' back to the workflow. Integration tests (kkemu sidecar) are a separate follow-up since they need a kkemu image source in CI — currently kktech/kkemu:latest comes from dockerhub and isn't easily reproducible from a feature branch. * test: unit coverage for new message-signing methods 34 new tests across 3 files covering all 9 new jspb classes + 5 wrapper functions: tron-message.test.ts (20 tests) - registry: TronSignMessage/MessageSignature/VerifyMessage/ SignTypedHash/TypedDataSignature (1404-1408) all registered - jspb round-trip: serializeBinary → deserializeBinaryFromReader preserves all fields for each class (incl. empty-message + domain-only typed-hash edge cases) - tronSignMessage wrapper: success path + UTF-8 string encoding - tronVerifyMessage wrapper: * Success → true * MESSAGETYPE_FAILURE thrown by transport.call → returns false (regression test for the Promise<boolean> contract bug caught in PR review) * core.ActionCancelled → bubbles past the catch (cancel is distinguishable from sig mismatch) - tronSignTypedHash wrapper: 32-byte hash length validation + success path ton-message.test.ts (7 tests) - registry: TonSignMessage/MessageSignature (1504/1505) - jspb round-trip - tonSignMessage wrapper: success + ActionCancelled bubbling (firmware AdvancedMode gate path) solana-offchain.test.ts (7 tests) - registry: SolanaSignOffchainMessage/OffchainMessageSignature (756/757) - jspb round-trip incl. 1212-byte boundary (spec ceiling for fmt 0/1) - solanaSignOffchainMessage wrapper: UTF-8 encoding, version + message_format forwarding, success path Total jest run: 49 → 83 (all pass locally with yarn test).
…tion
Wraps the firmware ZcashDisplayAddress flow as a method on
KeepKeyHDWallet plus a standalone Zcash.zcashDisplayAddress() helper.
Pattern mirrors zcashGetOrchardFVK: build the proto, transport.call,
unpack the response.
API:
wallet.zcashDisplayAddress({
addressNList, // ZIP-32 path [32', 133', account']
account?,
address, // host-built UA string (u1...)
ak, nk, rivk, // FVK components for verification
expectedSeedFingerprint?, // optional ZIP-32 §6.1 binding
}): Promise<{ address: string; seedFingerprint?: Uint8Array }>
Trust model is firmware-side: device re-derives its Orchard FVK at
the requested account and rejects unless host's (ak, nk, rivk) matches.
On match, device displays the address with QR; user accept returns the
confirmed address bytes. Reject closes the call with Failure.
If expectedSeedFingerprint is supplied, device checks it against
BLAKE2b-256("Zcash_HD_Seed_FP", I2LEBSP_8(len) || seed) and rejects
before any FVK derivation on mismatch — catches "wrong device" errors.
Requires firmware ≥ 7.15.0 with the ZcashDisplayAddress proto handler
(BitHighlander/keepkey-firmware:feature-zcash, PR #220).
Build will fail until @bithighlander/device-protocol is republished
with the ZcashDisplayAddress / ZcashAddress proto messages — both are
already on BitHighlander/device-protocol:master via PR #27 and PR #28
but the npm package is currently pinned at 7.14.1 which predates them.
The package.json bump is intentionally NOT included here so that the
dep republish + version pick can be a separate, mechanical commit.
Picks up ZcashDisplayAddress / ZcashAddress proto messages and the seed_fingerprint binding fields on FVK / SignPCZT / DisplayAddress / Address messages. Required for the wallet.zcashDisplayAddress wrapper introduced in this PR to compile. Also tightens the response unpack: throw on empty address instead of silently returning a possibly-undefined string. Empty means something went wrong on the wire. Verified locally: yarn build → success.
Firmware derives the Orchard UA from device seed material and
displays it; host no longer supplies UA / ak / nk / rivk. Wrapper
now accepts only { addressNList?, account? } and returns
{ address }.
…eepkey transport) - hdwallet-core/src/hive.ts: HiveGetPublicKey, HivePublicKey, HiveSignTx, HiveSignedTx types + HiveWallet interface - hdwallet-keepkey/src/hive.ts: jspb shims for wire IDs 1600-1603, hiveGetPublicKey() + hiveSignTx() transport functions - hdwallet-keepkey/src/keepkey.ts: import Hive, _supportsHive flag, hiveGetPublicKey/hiveSignTx wallet methods - hdwallet-core/src/index.ts: re-export hive types
- Accept ZcashPCZTActionAck after last transparent output when there are no transparent inputs — firmware skips straight to Orchard in this case - Handle transparent output phase in zcashSignPczt (outputs before inputs) - Dual-path for post-last-input response: ZcashPCZTActionAck (new) or ZcashTransparentSigned (legacy) - ZcashTransparentSigned now also handled before ZcashSignedPCZT Confirmed working: deshield mainnet txid cea9b378081aa6e4f4a8d390e287f0aae5cb7cb0ca3febdedc1639cf25fb17f4
Covers the output→ZcashPCZTActionAck→Orchard path (no transparent inputs): after the last transparent output with no inputs, firmware sends ZcashPCZTActionAck directly instead of TransparentAck. Validates: - Protocol sequence: SignPCZT → Output → Action × 2 (no Input step) - n_transparent_outputs=1, n_transparent_inputs not set - Output message fields (index, value, script_pubkey) - Action values are OUTPUT note values (change=1358918, dummy spend=0) - recipient+rseed present on all actions including is_spend=true - No transparent ECDSA sigs returned (empty array, not undefined)
…rotocol@7.16.0 Adds Hive blockchain, Zcash clear-signing, THORChain denom, Ripple memo protos.
`yarn lint` (eslint --cache --max-warnings=0 .) fails on a clean checkout across four files; the CI cache was intermittently masking hive.ts/keepkey.ts. - zcash.ts / zcash.test.ts: prettier formatting; convert 14 `console.log` debug traces to `console.info` (the no-console rule allows info/warn/error and these are intentional [zcash-pczt] diagnostics); remove an unused `hexToBytes` helper. - hive.ts / keepkey.ts: prettier formatting only (alignment/wrapping). No logic changes. zcash unit tests pass (4/4). Supersedes the stale #41.
…#45) readChunk() reads the IN endpoint (transferIn) but on a "stall" it cleared clearHalt("out", ...) -- the wrong endpoint and direction. A halted IN pipe is only resumed by resetting that same pipe, so it stayed halted. The browser transport also fell through and returned the stalled (empty/short) buffer, which the framing parser then rejected as "message not valid". Reset the IN pipe and throw a retryable "bad read" instead of surfacing a non-packet. Applied to both hdwallet-keepkey-nodewebusb (Node/libusb) and hdwallet-keepkey-webusb (browser). Split out from #44 (which also added transfer timeouts); this is the isolated, low-risk correctness fix.
Drop the hardcoded `denom !== 'rune'` rejection in the keepkey MsgSend signer and forward non-rune denoms to the device via the ThorchainMsgSend.denom proto field (device-protocol 7.16.0+). Firmware defaults an unset denom to 'rune', so denom is only set when it differs — existing RUNE sends keep byte-identical proto/signatures. TCY, RUJI/Rujira and secured assets (e.g. btc-btc) now sign as MsgSend.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
2 participants
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.
What
Removes the hardcoded
denom !== 'rune'rejection in the keepkey MsgSend signer and forwards non-rune denoms to the device viaThorchainMsgSend.denom(device-protocol 7.16.0+).Why
Firmware (alpha) already accepts any THORChain denom, but hdwallet hard-threw on anything but
rune, blocking TCY, RUJI/Rujira and secured assets (e.g.btc-btc) from ever reaching the device. MsgDeposit was already unrestricted.Compat
denom is only set when it differs from
rune(firmware defaults unset→rune), so existing RUNE sends keep byte-identical proto/signatures.Verification
Empirically confirmed against a running vault + alpha emulator: MsgSend with
tcy/rujira/btc-btc/eth-usdcpreviously returned "Unsupported denomination"; with this change they sign. Regression test lives in keepkey-vaultprojects/keepkey-sdk/tests/thorchain/ruji.js.