MetaMask / MetaMask/core

eth-json-rpc-middleware: `TransactionParamsStruct` rejects numeric `chainId`, breaking dapp transactions

Open Beginner friendly
#9,964 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
413
Forks
308
Avg merge
1d 4h
Merged PRs (30d)
253

Description

`TransactionParamsStruct` types `chainId` as `optional(string())` while every sibling quantity field uses `QuantityStruct = union([string(), number()])`. Dapps that send `chainId` as a JSON number in `eth_sendTransaction` therefore get `-32602`, where they previously succeeded.

Introduced in `24.0.0` by #9482.

### Reproduction

Against `@metamask/eth-json-rpc-middleware@24.0.0`:

```js
const { validateTransactionParams } = require('@metamask/eth-json-rpc-middleware');

const base = {
from: '0x1234567890123456789012345678901234567890',
to: '0x0987654321098765432109876543210987654321',
value: '0x0',
};

validateTransactionParams({ ...base, chainId: '0x1237' }); // ok
validateTransactionParams({ ...base, gas: 21000 }); // ok — number accepted
validateTransactionParams({ ...base, chainId: 4663 }); // throws
```

```
OK chainId as hex string
OK gas as number
REJECT chainId as number
Invalid params | chainId - Expected a string, but received: 4663
```

`gas` as a number is accepted; `chainId` as a number is not. Both are quantities.

### Why this looks unintended

`QuantityStruct` is introduced at `src/utils/validation.ts:248` with:

> `// Numerical fields accept both hex strings and numbers, as some dapps send numbers and `TransactionController` normalizes them downstream.`

That rationale applies to `chainId` as much as to `gas` or `value`, and `chainId` is the only quantity in the struct that did not get the union. In the #9482 review thread the union was added in response to a comment naming `value`, `gasPrice` and `gasLimit` specifically; `chainId` was not in that list. There is also no test in `validation.test.ts` covering the number branch of `QuantityStruct` for any field — every fixture is hex-only — so nothing flagged the gap.

### Why relaxing it is safe

The value is discarded downstream regardless. `chainId` is not present in the `NORMALIZERS` map used by `normalizeTransactionParams` in `@metamask/transaction-controller`, which rebuilds the params object from a fixed allow-list. Consumers derive the chain from their own network state — the extension does so from `networkClientId` — and never read the dapp-supplied value.

### Impact

- **metamask-extension 13.45.0+** — MetaMask/metamask-extension#45763, labeled `Sev1-high` / `regression-prod-13.45.0`. Uniswap's swap and approval paths always send `chainId` as a number (`ValidatedTransactionRequest` requires `chainId: number`), so swaps fail with no confirmation window ever opening. 13.44.0 is unaffected.
- **metamask-mobile 8.8.0+** — same bump in MetaMask/metamask-mobile#34392, which additionally validates on the WalletConnect path.

`24.0.0` is currently `latest` on npm, so both are broken in production with no released fix.

### Proposed fix

`src/utils/validation.ts`:

```diff
- chainId: optional(string()),
+ chainId: optional(QuantityStruct),
```

Suitable for a `24.0.1` patch — it restores prior behavior and changes no API surface. I'll open a PR with this plus the missing numeric-branch tests.

I'd suggest leaving `authorizationList[].chainId`/`nonce`/`yParity` alone for now: viem and ethers v6 both hex-encode those, so there's no evidence anything breaks there.

### Possibly worth separate discussion

Not proposed here, but the same struct newly rejects shapes that `normalizeTransactionParams` has always stripped harmlessly — `null` for optional fields, `to: null` on contract deployments, and unknown top-level keys such as web3.js's `input`/`networkId`/`common`, viem's `blobs`/`maxFeePerBlobGas`, and ethers v6's `blobVersionedHashes`. Happy to open a separate issue if that's of interest.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/utils/validation.ts at TransactionParamsStruct and QuantityStruct, then inspect validation.test.ts and the validateTransactionParams reproduction. Update the chainId validation and add coverage for numeric quantities; done means numeric chainId values are accepted while the existing hex-string behavior remains covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
86/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.