feat(rn_cli_wallet): migrate ethers v5 → v6

[ignaciosantise]

Context

wallets/rn_cli_wallet was pinned to ethers@5.8.0. A prior attempt (0c2a8a9b) upgraded to v6 but bundled it with secure-enclave key storage and was reverted (6945952e). This PR re-does only the ethers v5 → v6 half, keeping the existing MMKV plaintext storage untouched.

Scope

• ethers v6 only — no expo-secure-store / secure-enclave; storage unchanged.
• Included drive-bys that shipped in the original commit: reject eth_sign for security, and handleRedirect in the Canton/Tron sign modals.

Changes

• Deps: ethers 5.8.0 → ^6.13.5 (resolves 6.17.0); dropped the @ethersproject/pbkdf2 yarn patch + resolutions (v6 has no @ethersproject); kept the bip39 patch for non-EVM seed derivation.
• EIP155Lib: HDNodeWallet.fromPhrase, signTypedData, JsonRpcProvider, Wallet | HDNodeWallet typing. Public _signTypedData wrapper name kept so callers don't change.
• PaymentTransactionUtil: BigNumber → native bigint; StaticJsonRpcProvider → JsonRpcProvider + staticNetwork; getBlock() null-guards.
• SettingsStore: wrap wallet setters in valtio ref() — load-bearing: v6's private #signingKey throws under valtio's proxy and would corrupt the shared eip155Wallets instance.
• Named v6 imports across EIP155WalletUtil (Mnemonic.isValidMnemonic), HelperUtil, ERC20BalanceService, EIP155RequestHandlerUtil, PaymentStore; updated the quick-crypto.web.ts shim comment.
• New __tests__/SettingsStore.test.ts regression test for the v6 + ref() interaction.

Verification

• yarn install — lockfile synced, @ethersproject/pbkdf2 fully removed.
• yarn tsc --noEmit — clean.
• yarn test — 36/36 pass, incl. the new SettingsStore.test.ts. (App-test.tsx fails on RNBootSplash — pre-existing native-mock issue, unrelated.)
• ESLint on touched files — clean.

Still needs a device (not yet run)

Native/web smoke tests. Clear caches before running (this bit the first revert): expo start --clear; on Android delete the baked bundle before rebuilding. Then confirm balances load, personal_sign / eth_signTypedData_v4 succeed, eth_sign is rejected, and a Pay tx goes through.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
deposit-app-demo	Error		Jul 3, 2026 9:18pm
pos-demo	Ready	Preview, Comment	Jul 3, 2026 9:18pm

[claude]

Claude finished @ignaciosantise's task in 3m 40s —— View job

---

Code Review

Tasks:

• Gather context
• Read changed files
• Run license/breaking changes/data classification subagents
• Compile findings and post review

---

License compliance: ✅ No issues — ethers v5→v6 stays MIT; no net-new packages introduced.

Data classification: Pre-existing finding noted below (storage unchanged by this PR per stated scope).

---

Found 2 issue(s)

Issue 1: || null coerces 0n to null for EIP-1559 fee fields — behavioral regression from v5

ID: paymenttxutil-bigint-falsy-fee-coercion-b7e2
File: wallets/rn_cli_wallet/src/utils/PaymentTransactionUtil.ts:279,287
Severity: MEDIUM
Category: code_quality / correctness

Context:

• Pattern: buildFreshTxRequest passes fee fields as feeData.maxPriorityFeePerGas || null and feeData.maxFeePerGas || null
• Risk: In ethers v5 feeData.* fields were BigNumber objects — truthy even when representing zero. In ethers v6 they are native bigint, and 0n is falsy in JavaScript. So 0n || null evaluates to null, silently dropping a valid zero value.
• Impact: On L2s that legitimately return maxPriorityFeePerGas = 0n (e.g. Arbitrum chains without miner tips), priorityFee becomes null, maxFee becomes null, and the code falls through to the legacy gasPrice path. On chains that have disabled type-0 (legacy) transactions this can cause transaction submission to fail entirely.
• Trigger: Any chain where getFeeData() returns maxPriorityFeePerGas = 0n or maxFeePerGas = 0n.

Recommendation: Replace || with ?? (nullish coalescing) — which only falls through on null/undefined, not on 0n:

// Line 278–280
const priorityFee = getHighestBigInt([
  chainFloor,
  feeData.maxPriorityFeePerGas ?? null,  // was || null
]);

// Line 285–290
const maxFee = priorityFee
  ? getHighestBigInt([
      latestBlock.baseFeePerGas ? latestBlock.baseFeePerGas * 2n + priorityFee : null,
      feeData.maxFeePerGas ?? null,       // was || null
      priorityFee,
    ])
  : null;

Fix this →

---

Issue 2: Mnemonic / private key stored plaintext in unencrypted MMKV (pre-existing, storage unchanged by this PR)

ID: dcl-storage-plaintext-key-material-a3f1
File: wallets/rn_cli_wallet/src/utils/EIP155WalletUtil.ts:27,95,98
Severity: CRITICAL (pre-existing)
Category: security / data_classification

Context:

• Pattern: storage.setItem('EIP155_MNEMONIC_1', ...) and storage.setItem('EIP155_PRIVATE_KEY_1', ...) write to an MMKV instance constructed without an encryptionKey.
• Risk: MMKV stores data in plaintext on the device filesystem. Mnemonic phrases and private keys are Critical-tier key material.
• Impact: Full wallet compromise on rooted/jailbroken devices, unencrypted backups (ADB, iTunes), or forensic extraction.
• Trigger: Any rooted Android, jailbroken iOS, or unencrypted device backup.

Note: This PR explicitly keeps storage unchanged (no secure-enclave path). Flagging for awareness — the PR description acknowledges this is out of scope. Tracking it here so it is not lost.

Recommendation: Encrypt the MMKV instance with a device-hardware-bound key (Android Keystore / iOS Secure Enclave via react-native-keychain):

const encryptionKey = await getOrCreateDeviceBoundKey(); // keychain-backed
const mmkv = new MMKV({ encryptionKey });

---

Other observations (no action required):

• ref() applied consistently to all wallet setters (EIP155, Sui, Ton, Tron, Canton, Solana) — correct fix for valtio v6 proxy incompatibility.
• eth_sign rejection in EIP155RequestHandlerUtil.ts is a security improvement.
• getHighestBigInt correctly uses v == null guard internally; the issue is only at call sites where || coerces before the function is invoked.
• AGENTS.md still lists ethers (5.8.0) in the Tech Stack — minor doc drift worth updating.
• SettingsStore.test.ts regression test is well-targeted and directly guards the valtio ref() invariant.

[ignaciosantise]

Thanks for the review — actions taken (commit 9fcfaf2):

Issue 1 — || null coerces 0n → null on EIP-1559 fee fields (MEDIUM): ✅ Fixed.
Confirmed the regression — v5 BigNumber(0) was truthy, v6 0n is falsy, so 0n || null dropped a valid zero and fell through to the legacy gasPrice path (breaking L2s that return maxPriorityFeePerGas = 0n and reject type-0 txs). Swapped || → ?? at both call sites in buildFreshTxRequest (PaymentTransactionUtil.ts:279,287).

Issue 2 — plaintext MMKV key material (CRITICAL, pre-existing): ⏭️ Out of scope (deferred).
Intentional for this PR — scoped to ethers v5→v6 only, keeping MMKV storage unchanged (the secure-enclave work was the previously-reverted bundle). Not introduced here; tracking separately for when secure-enclave is revisited.

Doc drift — AGENTS.md listed ethers (5.8.0): ✅ Fixed. Updated to 6.13.5.

**Other observations (ref() consistency, eth_sign rejection, getHighestBigInt guard, regression test): ** noted, no action needed — thanks for confirming.

[Copilot]

Pull request overview

Migrates the wallets/rn_cli_wallet example wallet from ethers@5 to ethers@6, updating EVM signing/provider usage while keeping the existing (non-secure-enclave) storage approach intact. The PR also adds a regression test for the valtio + ethers v6 private fields interaction, rejects eth_sign requests, and triggers redirect handling after signing in Canton/Tron modals.

Changes:

• Upgrade EVM stack to ethers v6 (imports, wallet derivation/signing, providers, fee math).
• Harden request handling (reject eth_sign) and add post-approve redirect handling in sign modals.
• Add regression test ensuring SettingsStore.setWallet() uses valtio.ref() to avoid proxying ethers v6 internals.

Reviewed changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
wallets/rn_cli_wallet/package.json	Pins ethers to v6 and removes @ethersproject/pbkdf2 patch mappings.
wallets/rn_cli_wallet/yarn.lock	Lockfile updates reflecting ethers v6 and removed v5-era transitive deps.
wallets/rn_cli_wallet/src/lib/EIP155Lib.ts	Updates wallet init/typing and typed-data signing to ethers v6 APIs.
wallets/rn_cli_wallet/src/utils/PaymentTransactionUtil.ts	Migrates provider/fee types to v6 and switches fee math from BigNumber to bigint.
wallets/rn_cli_wallet/src/utils/HelperUtil.ts	Replaces ethers.utils usage with v6 top-level helpers.
wallets/rn_cli_wallet/src/utils/EIP155WalletUtil.ts	Updates mnemonic validation to ethers v6 Mnemonic API.
wallets/rn_cli_wallet/src/utils/EIP155RequestHandlerUtil.ts	Rejects eth_sign and migrates provider creation to v6 JsonRpcProvider.
wallets/rn_cli_wallet/src/store/SettingsStore.ts	Wraps wallet setters in valtio.ref() to avoid proxying ethers v6 instances.
wallets/rn_cli_wallet/src/store/PaymentStore.ts	Updates tx request typing to v6 TransactionRequest.
wallets/rn_cli_wallet/src/shims/quick-crypto.web.ts	Updates shim comment to reflect bip39 seed-derivation patch behavior.
wallets/rn_cli_wallet/src/services/ERC20BalanceService.ts	Migrates formatting/provider usage to ethers v6 APIs.
wallets/rn_cli_wallet/src/modals/SessionSignTronModal.tsx	Adds handleRedirect after approval and updates hook deps.
wallets/rn_cli_wallet/src/modals/SessionSignCantonModal.tsx	Adds handleRedirect after approval and updates hook deps.
wallets/rn_cli_wallet/AGENTS.md	Updates documented ethers version for the sample wallet.
wallets/rn_cli_wallet/tests/SettingsStore.test.ts	Adds regression test for ethers v6 + valtio proxy interaction.

Comments suppressed due to low confidence (1)

wallets/rn_cli_wallet/src/utils/PaymentTransactionUtil.ts:298

• More bigint truthiness: if (priorityFee) / if (maxFee) will skip valid 0n values and incorrectly delete fee fields.

Switch these to explicit null checks so 0n is treated as a real value.

  if (priorityFee) {
    request.maxPriorityFeePerGas = priorityFee;
  } else {
    delete request.maxPriorityFeePerGas;
  }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.