fix(thorchain): clear-sign deposits on every EVM chain, not just mainnet#309
Open
BitHighlander wants to merge 1 commit into
Open
fix(thorchain): clear-sign deposits on every EVM chain, not just mainnet#309BitHighlander wants to merge 1 commit into
BitHighlander wants to merge 1 commit into
Conversation
thor_isThorchainTx pinned the router to the Ethereum-mainnet address with no chain awareness, so a THORChain depositWithExpiry on Avalanche (or any non-mainnet chain) never matched and fell into the AdvancedMode blind-sign gate — the device returned a bare ActionCancelled instead of clear-signing. Scope the pin to (address, chain_id): add the live-verified Avalanche router and look it up by chain. EthereumSignTx already carries chain_id, so no plumbing is needed. Also fix a latent memo bug on the same path: thor_confirm_deposit_tx read a hardcoded 64 memo bytes instead of the ABI length word. That truncated a longer memo (dropping trailing fields such as the affiliate fee bps) and — with a crafted length word — let the router execute a longer memo than the device displayed. Read the real length word with canonical high-byte and presence checks, and relax the min-chunk floor (which was derived from the same 64) to the true structural minimum. BSC and Base routers exist on-chain but are omitted until verified against a live node: the shipped host catalog lists a stale AVAX router, and those pairs route via Relay today, so no such tx reaches the device. Tests: 7 new (chain-scoped match/reject, unpinned chain/address reject, 67-byte memo decodes fully, over-declared memo length rejected). Full firmware unit suite 254/254.
BitHighlander
force-pushed
the
fix/thorchain-clearsign-per-chain-router
branch
from
86fa168 to
715c173
Compare
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.
What & why
A THORChain AVAX→ETH swap on the emulator hit the AdvancedMode blind-sign gate and returned a bare
ActionCancelledinstead of clear-signing. Root cause:thor_isThorchainTxpins the router to the Ethereum-mainnet address (d37bbe57..) with no chain awareness, so adepositWithExpiryon Avalanche — whose router is00dc6100..— can never match.ethereum_contractHandledreturns false, the signed-metadata escape isn't wired, and the tx falls into the hard blind-sign block (ethereum.c:850-859). THORChain EVM deposits were therefore clear-signable on Ethereum and structurally impossible on every other chain.Changes
Chain-scoped router pin.
thor_router_for_chain(msg)resolves the THORChain router bychain_id(Ethereum + Avalanche today);thor_isThorchainTxpins on(address, chain_id)together.EthereumSignTxalready carrieschain_id, so no plumbing was needed. The confirm screen labels the AVAX router as "Thorchain router" too.Memo-length read (latent security fix on the same path).
thor_confirm_deposit_txread a hardcoded 64 memo bytes instead of the ABI length word. That truncated a longer memo (the live 67-byte memo drops its trailing affiliate-fee field) and, with a crafted length word, let the router execute a longer memo than the device displayed (display-vs-execute divergence). Now reads the real length word with canonical high-byte + full-presence checks, and relaxes themin_chunkfloor (which was itself derived from the same 64) to the true structural minimum.Scope note
BSC (
56) and Base (8453) THORChain routers exist on-chain but are intentionally omitted until verified against a live node: the shipped host catalog lists a stale AVAX router (8f66c4ae..vs the live00dc6100..), so its BSC/Base entries can't be trusted, and Pioneer routes those pairs via Relay today — no THORChain deposit reaches the device. Each drops in as a one-line addition tothor_router_for_chainonce verified. The Avalanche address here was verified live via a Pioneer quote (/inbound_addresses).Verification
firmware-unit, emulator cache)..c/.h.ConfirmThorTxLongMemoDecodesFullytest drives the exact live AVAX calldata (67-char memo, AVAX router, chain 43114) through the realthor_confirmThorTxconfirm path.Remaining
On-device / emulator Gate-3 OLED capture of the AVAX deposit clear-sign screens is the human-gated approval step (emulator dylib rebuilt from this branch + restart).
The vault-side clear-sign predictor mirror (
calldata-decoder.ts firmwareClearSigns) is updated in lockstep in a separate vault change so the REST/WalletConnect overlay predicts AVAX clear-signability correctly.