fix(keepkey): support ruji deposits and flag unsupported sends - #12604
Conversation
KeepKey firmware reconstructs the THORChain MsgSend it signs and hardcodes the `rune` denom, so it cannot sign transfers of native THORChain assets that aren't RUNE. Deposits are unaffected, as those carry the asset through verbatim - allow RUJI there alongside RUNE and TCY. Sends remain firmware-blocked, so surface that in the send modal upfront instead of failing at signing time with "failed to sign transaction". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 53 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe change adds RUJI to KeepKey THORChain deposit assets. It adds wallet asset support detection to the send form, displays a localized warning for unsupported assets, and disables the Next button when sending is unavailable. ChangesWallet asset support
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The PR enables RUJI deposits and prevents unsupported KeepKey sends from reaching a signing failure; it is mergeable with owner awareness that the warning should always include a wallet name. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1.62.41 is already on npm, so the ruji deposit change needs a bump to ship. publish-packages.yml publishes any packages/* whose version is not yet on the registry, but the bump itself is manual. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Description
KeepKey firmware reconstructs the THORChain
MsgSendit signs and hardcodes therunedenom, so it cannot sign transfers of native THORChain assets that aren't RUNE. That is why sending RUJI fails with "failed to sign transaction".Deposits are unaffected, as those carry the asset through verbatim. So this PR:
hdwallet-keepkey, alongside the RUNE and TCY already allowed (mirrors the earlier TCY fix in 6f08e44).Sending RUJI/TCY on KeepKey is not fixed here, and can't be from the client - it needs a firmware change so
MsgSendcarries the asset's denom rather than hardcodingrune.Issue (if applicable)
closes #12463
Linear: https://linear.app/shapeshift-dao/issue/SS-5707/unable-to-send-ruji-on-keepkey
Note the original acceptance criteria was "should be able to send Ruji on KK". That isn't achievable from the client -
MsgSendhardcodes therunedenom in firmware - so there is nothing further for us to do here. Closing on the basis that deposits now work and the send limitation is communicated clearly rather than surfacing as a signing failure. If firmware later carries the denom through, lifting the guard is a one-line change inwalletSupportsSendingAsset.Risk
Low, but it does touch signing.
hdwallet-keepkeychange widens an allowlist in theMsgDepositpath fromTHOR.RUNE/THOR.TCYto also acceptTHOR.RUJI. No signing logic changes; a previously rejected asset is now passed through the same code path TCY already used.MsgSendis untouched and still throws for any non-runedenom.walletSupportsSendingAssetreturnstruefor every other wallet and asset, so the send modal is unchanged outside KeepKey + RUJI/TCY.Testing
Engineering
pnpm run hdwallet:buildor a running dev server so thehdwallet-keepkeychange is compiled intodist).THORChain: Unsupported coin asset: THOR.RUJI.Operations
On a KeepKey, choosing to send RUJI or TCY now shows an explanatory message and a disabled button instead of letting you proceed to a "failed to sign transaction" error. RUJI deposits work. No other wallet or asset should behave differently.
Summary by CodeRabbit
New Features
Bug Fixes