Skip to content

Commit 7869be1

Browse files
fix: resolve 8 EVM handler bugs (#35)
* fix: resolve 8 EVM handler bugs — chainId, precision, security, compliance - Bug 1: net_version no longer double-converts decimal chainId as hex - Bug 2: convertToHex uses parseEther instead of lossy float math - Bug 3: handleTransfer uses parseEther for amount conversion - Bug 4: eth_gasPrice handles null gasPrice on EIP-1559 chains - Bug 6: wallet_addEthereumChain requires user approval before adding - Bug 7: signTypedData handles both string and object typed data - Bug 8: eth_sign uses correct param order [address, message] - Bug 10: failedRpcs map cleaned up on each getProvider call Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review — personal_sign address param + addEvent type - personal_sign now prefers dApp-supplied params[1] address so multi-account wallets use the correct derivation path (EIP-191 params = [message, address]). Falls back to global ADDRESS when not provided. - Drop @ts-expect-error on addEthereumChain approval event by including the required unsignedTx field (null) and narrowing status literal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 54cb208 commit 7869be1

1 file changed

Lines changed: 42 additions & 18 deletions

File tree

chrome-extension/src/background/chains/ethereumHandler.ts

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Ethereum Provider Refactored
33
*/
44

5-
import { JsonRpcProvider } from 'ethers';
5+
import { JsonRpcProvider, parseEther } from 'ethers';
66
import { createProviderRpcError, ProviderRpcError } from '../utils';
77
import { requestStorage, web3ProviderStorage, assetContextStorage, blockchainDataStorage } from '@extension/storage';
88
import { EIP155_CHAINS } from '../chains';
@@ -129,6 +129,15 @@ const RPC_RETRY_DELAY = 60000; // Don't retry failed RPC for 1 minute
129129
// Helper function to get the provider with RPC failover
130130
const getProvider = async (): Promise<JsonRpcProvider> => {
131131
const tag = TAG + ' | getProvider | ';
132+
133+
// Clean up expired RPC failure entries
134+
const now = Date.now();
135+
for (const [url, failedAt] of failedRpcs) {
136+
if (now - failedAt >= RPC_RETRY_DELAY) {
137+
failedRpcs.delete(url);
138+
}
139+
}
140+
132141
const currentProvider = await web3ProviderStorage.getWeb3Provider();
133142
console.log(tag, 'currentProvider from storage:', currentProvider);
134143

@@ -148,7 +157,6 @@ const getProvider = async (): Promise<JsonRpcProvider> => {
148157
console.log(tag, 'Available RPCs:', rpcUrls.length);
149158

150159
// Filter out recently failed RPCs
151-
const now = Date.now();
152160
const availableRpcs = rpcUrls.filter(url => {
153161
const failedAt = failedRpcs.get(url);
154162
if (failedAt && now - failedAt < RPC_RETRY_DELAY) {
@@ -214,8 +222,7 @@ const handleEthChainId = async () => {
214222

215223
const handleNetVersion = async () => {
216224
const currentProvider = await web3ProviderStorage.getWeb3Provider();
217-
const netVersion = currentProvider.chainId.toString();
218-
return convertHexToDecimalChainId(netVersion).toString();
225+
return currentProvider.chainId.toString();
219226
};
220227

221228
const handleEthGetBlockByNumber = async params => {
@@ -293,7 +300,7 @@ const handleEthEstimateGas = async params => {
293300
const handleEthGasPrice = async () => {
294301
const provider = await getProvider();
295302
const feeData = await provider.getFeeData();
296-
return '0x' + feeData.gasPrice.toString(16);
303+
return feeData.gasPrice ? '0x' + feeData.gasPrice.toString(16) : '0x0';
297304
};
298305

299306
const handleEthGetCode = async params => {
@@ -440,7 +447,7 @@ const handleWalletSwitchEthereumChain = async (params, KEEPKEY_WALLET) => {
440447
};
441448

442449
// Handle wallet_addEthereumChain - add new chain with user approval
443-
const handleWalletAddEthereumChain = async (params, KEEPKEY_WALLET) => {
450+
const handleWalletAddEthereumChain = async (params, KEEPKEY_WALLET, requestInfo, requireApproval) => {
444451
const tag = TAG + ' | handleWalletAddEthereumChain | ';
445452
console.log(tag, 'Add Chain params: ', params);
446453
console.log(tag, 'KEEPKEY_WALLET exists:', !!KEEPKEY_WALLET);
@@ -503,9 +510,25 @@ const handleWalletAddEthereumChain = async (params, KEEPKEY_WALLET) => {
503510

504511
console.log(tag, 'Cleaned provider config:', newProvider);
505512

506-
// TODO: Show user approval dialog here
507-
// For now, auto-approve - but we should ask user permission
508-
console.log(tag, 'Auto-approving chain addition (TODO: add user prompt)');
513+
// Require user approval before adding chain
514+
const approvalEvent = {
515+
id: uuidv4(),
516+
networkId,
517+
chain: 'ethereum',
518+
type: 'wallet_addEthereumChain',
519+
request: params,
520+
status: 'request' as const,
521+
timestamp: new Date().toISOString(),
522+
unsignedTx: null,
523+
chainName: params[0].chainName,
524+
chainId: chainIdHex,
525+
requestInfo,
526+
};
527+
await requestStorage.addEvent(approvalEvent);
528+
const approval = await requireApproval(networkId, requestInfo, 'ethereum', 'wallet_addEthereumChain', params[0]);
529+
if (!approval?.success) {
530+
throw createProviderRpcError(4001, 'User rejected adding the chain');
531+
}
509532

510533
// Store the custom chain
511534
await blockchainStorage.addBlockchain(newProvider.networkId);
@@ -620,10 +643,7 @@ const handleSigningMethods = async (method, params, requestInfo, ADDRESS, KEEPKE
620643
};
621644

622645
const convertToHex = (amountInEther: string) => {
623-
const weiMultiplier = BigInt(1e18); // 1 Ether = 1e18 Wei
624-
const amountInWei = BigInt(parseFloat(amountInEther || '0') * 1e18); // Convert Ether to Wei
625-
626-
// Convert the amount in Wei to a hex string
646+
const amountInWei = parseEther(amountInEther || '0');
627647
return '0x' + amountInWei.toString(16);
628648
};
629649

@@ -642,8 +662,7 @@ const handleTransfer = async (params, requestInfo, ADDRESS, KEEPKEY_WALLET, requ
642662

643663
// Build EVM transfer locally
644664
const provider = await getProvider();
645-
const amountWei =
646-
'0x' + BigInt(Math.floor(parseFloat(params[0].amount?.amount || params[0].amount || '0') * 1e18)).toString(16);
665+
const amountWei = '0x' + parseEther(params[0].amount?.amount || params[0].amount || '0').toString(16);
647666
const chainId = currentProviderCtx?.chainId || '1';
648667

649668
requestInfo.id = uuidv4();
@@ -817,7 +836,7 @@ export const handleEthereumRequest = async (
817836
return await handleWalletSwitchEthereumChain(params, KEEPKEY_WALLET);
818837

819838
case 'wallet_addEthereumChain':
820-
return await handleWalletAddEthereumChain(params, KEEPKEY_WALLET);
839+
return await handleWalletAddEthereumChain(params, KEEPKEY_WALLET, requestInfo, requireApproval);
821840

822841
case 'wallet_getSnaps':
823842
return await handleWalletGetSnaps();
@@ -869,8 +888,12 @@ const processApprovedEvent = async (method: string, params: any, KEEPKEY_WALLET:
869888
let result;
870889
switch (method) {
871890
case 'personal_sign':
891+
// EIP-191 personal_sign: params = [message, address]. Prefer the dApp-supplied
892+
// address so multi-account wallets sign with the correct derivation path.
893+
result = await signMessage(params[0], KEEPKEY_WALLET, params[1] || ADDRESS);
894+
break;
872895
case 'eth_sign':
873-
result = await signMessage(params[0], KEEPKEY_WALLET, ADDRESS);
896+
result = await signMessage(params[1], KEEPKEY_WALLET, params[0]);
874897
break;
875898
case 'eth_sendTransaction':
876899
result = await sendTransaction(params, KEEPKEY_WALLET, ADDRESS, id);
@@ -1073,7 +1096,8 @@ const signTypedData = async (params: any, KEEPKEY_WALLET: any, ADDRESS: string)
10731096
try {
10741097
console.log(tag, '**** params: ', params);
10751098
const typedData = params[1];
1076-
const { domain, types, message, primaryType } = JSON.parse(typedData);
1099+
const parsed = typeof typedData === 'string' ? JSON.parse(typedData) : typedData;
1100+
const { domain, types, message, primaryType } = parsed;
10771101
const HDWalletPayload = {
10781102
address: ADDRESS,
10791103
addressNList: getAddressNListForAddress(ADDRESS),

0 commit comments

Comments
 (0)