fix(phantom): normalize EIP-1474 hex quantities so yields approvals don't reject#12360
Conversation
…on't reject Phantom strictly enforces EIP-1474 hex QUANTITY encoding — no leading zeros except 0x0 itself — and rejects non-compact hex with a generic "Missing or invalid parameters" error. MetaMask silently normalizes, which is why yields approvals worked there but not on Phantom (yield.xyz returns padded hex like 0x054e0840 that the client passed through). - Normalize all hex fields in hdwallet-phantom ethSendTx via ethers hexValue. - Make toHexOrDefault always re-encode through BigInt; it was short- circuiting on isHex and preserving non-compact input verbatim. - Tighten the ETHSignTx EIP-1559 variant to require both maxFeePerGas and maxPriorityFeePerGas so discriminated-union narrowing actually works at callsites. - Add missing maxPriorityFeePerGas to coinbase/metamask-multichain/ vultisig/phantom adapter tests that the tightened type surfaced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR addresses Phantom wallet approval failures on EVM deposits by enforcing EIP-1559 fee parameter requirements in the core type contract, implementing proper hex encoding and fee logic in the Phantom wallet, and normalizing hex output to strip leading zeros that Phantom rejects. ChangesEIP-1559 Requirements and Phantom Wallet Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Description
Phantom strictly enforces EIP-1474 hex QUANTITY encoding — no leading zeros except
0x0itself — and rejects non-compact hex with a genericMissing or invalid parameterserror (code -32000). MetaMask silently normalizes, which is why yields approvals worked there but not on Phantom: yield.xyz returns padded hex like0x054e0840and the client was passing it through unchanged.Changes:
packages/hdwallet-phantom/src/ethereum.ts— normalize all hex QUANTITY fields (value,gasLimit,maxFeePerGas,maxPriorityFeePerGas,gasPrice) via ethers v5'shexValuebefore invokingphantom.request({ method: 'eth_sendTransaction' }).src/lib/yieldxyz/executeTransaction.ts—toHexOrDefaultwas short-circuiting onisHexand preserving non-compact input verbatim. Always re-encode throughBigInt/toHexso the output is EIP-1474 compact.packages/hdwallet-core/src/ethereum.ts— tighten the EIP-1559 variant ofETHSignTxto require bothmaxFeePerGasandmaxPriorityFeePerGas(matches the existingNetworkFeesdiscriminator). Enables proper discriminated-union narrowing at callsites.maxPriorityFeePerGasto the EIP-1559 ethSendTx tests incoinbase,metamask-multichain,vultisig, andphantomadapters — surfaced by the tighter type.executeTransaction.test.tsexpectations that codified the buggy non-compact output; added a regression test for the leading-zero case.Issue (if applicable)
closes #12350
SS-5675
Risk
High risk. Touches
hdwallet-core(theETHSignTxtype) and the EVM transaction send path inhdwallet-phantom. Type tightening surfaced three previously-incorrect adapter tests but does not change runtime behavior for any production construction site (all already pair both EIP-1559 fields). yield.xyz EVM execute path is also touched (toHexOrDefaultsemantics).hdwallet-phantom, all EVM transactions originating from yield.xyz flows.Testing
Engineering
Repro (without this fix): connect Phantom on Ethereum mainnet, attempt a yields deposit on USDC (Aave V3 or similar). Approval popup never appears, console shows
error signing & broadcasting txoriginating fromethereum.ts:50→ Phantom's bundle throwsMissing or invalid parametersonmaxPriorityFeePerGas.With this fix:
toHexOrDefaultchange didn't break paths that previously worked.pnpm --filter @shapeshiftoss/hdwallet-phantom test,pnpm --filter @shapeshiftoss/hdwallet-core test, and the yieldxyz tests in the web app.Build order if testing locally:
pnpm --filter @shapeshiftoss/hdwallet-core buildthenpnpm --filter @shapeshiftoss/hdwallet-phantom build(both packages resolve viadist/).Operations
QA on a preview build:
Screenshots (if applicable)
n/a
Summary by CodeRabbit