Skip to content

feat/PRO-3790/7702-implementation#192

Merged
RanaBug merged 8 commits into
masterfrom
feat/PRO-3790/7702-implementation
Oct 20, 2025
Merged

feat/PRO-3790/7702-implementation#192
RanaBug merged 8 commits into
masterfrom
feat/PRO-3790/7702-implementation

Conversation

@RanaBug
Copy link
Copy Markdown
Collaborator

@RanaBug RanaBug commented Oct 16, 2025

Description

Added Changes

  • EIP-7702 Support: Added delegated EOA (Externally Owned Account) functionality with isDelegateSmartAccountToEoa(), delegateSmartAccountToEoa() and undelegateSmartAccountToEoa() methods for EIP-7702 transactions
  • New Wallet Mode: Introduced walletMode configuration with support for both modular and delegatedEoa modes
  • Enhanced Security: Added secure configuration management with separate PrivateConfig and PublicConfig interfaces for sensitive data handling
  • Bundler Configuration: Added BundlerConfig class for flexible bundler URL management with API key support and custom formatting options
  • Client Management: Added per-chain client management with getPublicClient(), getBundlerClient(), and getWalletClient() methods for efficient multi-chain operations
  • Account Management: Added getDelegatedEoaAccount() and getOwnerAccount() methods for EIP-7702 account operations and owner account access
  • Network Constants and Support: Added comprehensive network constants and supported networks configuration
  • Batch Operations: Enhanced batch processing with new estimateBatches() and sendBatches() methods for improved batch transaction handling on multi-chains
  • Batch State Management: Implemented intelligent batch state cleanup that removes successful chain groups and transactions from internal state after sending
  • Multi-Chain Grouping: Added chain-based transaction grouping within batches for efficient multi-chain processing and cost tracking
  • ZeroDev Integration: Added @zerodev/sdk dependency for enhanced account abstraction capabilities
  • Viem Update: Updated viem library to version ^2.38.0 for improved compatibility and features

Breaking Changes

  • Configuration Structure: Updated configuration interfaces with new security-focused structure separating public and private configurations
  • Dependencies: Added new @zerodev/sdk dependency requirement

How Has This Been Tested?

  • Unit tests and manual testing

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

  • New Features

    • Added EIP-7702 delegated EOA support and switchable wallet modes (modular ↔ delegatedEoa).
    • Multi-chain batching with per-chain grouping, estimation, and cost aggregation.
    • Flexible bundler configuration and expanded network constants for many chains.
  • Breaking Changes

    • Configuration restructured to separate public and private settings.
  • Documentation

    • README updated with mode-specific quick starts, delegation flows, and multi-chain examples.
  • Chores

    • Released v2.1.0 and added updated dependencies.

@RanaBug RanaBug requested a review from IAmKio October 16, 2025 16:20
@RanaBug RanaBug self-assigned this Oct 16, 2025
@linear
Copy link
Copy Markdown

linear Bot commented Oct 16, 2025

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 16, 2025

Walkthrough

Adds dual wallet modes (modular and EIP-7702 delegatedEoa) with mode-specific public/private config separation, per-chain network constants, a BundlerConfig helper, expanded provider APIs and caches for delegatedEoa, utilities for chain/network resolution and sanitization, updated tests/examples/docs, and a package bump to 2.1.0.

Changes

Cohort / File(s) Summary
Network Constants & Bundler
lib/constants/index.ts, lib/BundlerConfig.ts
New centralized network configuration (SupportedNetworks, NetworkConfig, NETWORK_NAME_TO_CHAIN_ID, Networks) and a BundlerConfig class to construct bundler URLs with optional API key formatting.
Provider Core & Public API
lib/EtherspotProvider.ts
Refactors provider for dual wallet modes with private/public config separation, per-chain caches, delegatedEoa APIs (getPublicClient, getDelegatedEoaAccount, getOwnerAccount, getWalletClient, getBundlerClient), mode guards, cache lifecycle methods (clearSdkCache, clearDelegatedEoaCache, clearAllCaches), and updated config/destroy/update flows.
Types & Interfaces
lib/interfaces/index.ts
Adds WalletMode, PrivateConfig, PublicConfig, ModularModeConfig, DelegatedEoaModeConfig, EtherspotTransactionKitConfig (union), BundlerClientExtended, new delegation methods on IInitial, and expands BatchSendResult / BatchEstimateResult to include per-chain grouping and aggregate fields.
Utilities
lib/utils/index.ts
Adds sanitizeObject (redacts sensitive keys), CHAIN_ID_TO_NETWORK_NAME map, getNetworkConfig, and getChainFromId (resolves viem Chain or throws on unsupported).
Public Exports
lib/index.ts
Re-exports new modules: export * from './BundlerConfig' and export * from './constants'.
Examples & Docs
example/src/App.tsx, README.md, CHANGELOG.md
Example app updated to show wallet-mode switching, delegatedEoa flows, and expanded UI for batching/transactions; README and CHANGELOG updated for 2.1.0 features, breaking changes, and usage examples.
Tests & Build
__tests__/EtherspotProvider.test.ts, package.json, rollup.config.js
Tests extended for multi-mode lifecycles and edge cases; package bumped to 2.1.0, adds @zerodev/sdk and viem upgraded; rollup externals include viem/account-abstraction and @zerodev/sdk.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant App
    participant Provider as EtherspotProvider
    participant Bundler
    participant SDK as Modular SDK / DelegatedEoa Stack

    User->>App: Select wallet mode (modular | delegatedEoa)
    App->>Provider: updateConfig({ walletMode, ... })
    Provider->>Provider: validate config, partition public/private
    Provider->>Provider: clear relevant caches
    Provider->>SDK: initialize mode-specific clients

    alt modular
        Provider->>SDK: getSdk() -> standard provider/sdk
        SDK-->>Provider: sdk instance
    else delegatedEoa
        Provider->>SDK: getPublicClient(chain)
        Provider->>SDK: getOwnerAccount(privateKey)
        Provider->>SDK: getDelegatedEoaAccount()
        Provider->>SDK: getWalletClient() / getBundlerClient()
    end

    User->>App: Create batch across chains
    App->>Provider: batchEstimate()/batchSend()
    Provider->>Bundler: per-chain requests via BundlerConfig.url
    Bundler-->>Provider: per-chain results (estimates / userOpHash)
    Provider-->>App: BatchResult (chainGroups, totalCost, statuses)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • feat/transaction-2 #184 — overlapping changes to provider, interfaces, BundlerConfig and delegatedEoa APIs (strong code-level relation).

Suggested reviewers

  • IAmKio

Poem

🐰
I hopped through code with tiny paws,
Two wallet modes, and many laws,
Bundlers stitch the chain-lit night,
Batches bloom and hops take flight,
A carrot for the merge—what a sight! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The PR title "feat/PRO-3790/7702-implementation" uses a technical branch-naming format rather than a clear, descriptive sentence. While the title does reference EIP-7702, which is a real and significant part of the changeset, the PR actually encompasses much broader changes including new wallet modes (modular and delegatedEoa), configuration restructuring with separate PrivateConfig/PublicConfig, enhanced batch processing with multi-chain support, new client management methods, network constants, and BundlerConfig class. The title focuses on one technical aspect without capturing the full scope of these interconnected features, making it partially related to the main body of work rather than a clear summary of the primary changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed The PR description covers all required sections from the template: it includes a comprehensive "Description" section with detailed "Added Changes" covering twelve feature areas, provides a "How Has This Been Tested?" section indicating both unit tests and manual testing were performed, and appropriately checks both "New feature" and "Breaking change" in the Types of changes checklist. The description also includes an additional "Breaking Changes" section beyond the template that clearly documents the configuration structure changes and new dependency requirements, providing readers with essential context about backward compatibility implications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/PRO-3790/7702-implementation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (17)
example/src/App.tsx (5)

34-39: Avoid module‑level mutable kit; use a React ref.

Global let kit and reassignments make lifecycle/error handling brittle (hot reloads, multiple mounts). Store the instance in a ref and always access via kitRef.current.

Sketch:

// Inside component:
const kitRef = useRef(TransactionKit(initialConfig));
const kit = () => kitRef.current;

// On mode switch: kit().reset(); kitRef.current = TransactionKit(newConfig);

Then replace uses of kit with kit() (or kitRef.current). This keeps instance ownership within the component and avoids hidden shared state.

Also applies to: 295-323


109-116: Prefer Number(...) or radix with parseInt for chain IDs.

Use Number(...) for clarity (chain IDs are decimals), or add radix 10. Apply across usages.

-const chainId = parseInt(txChainId);
+const chainId = Number(txChainId);
-const chainId = parseInt(txHashChainId);
+const chainId = Number(txHashChainId);

Also applies to: 132-134, 168-175, 547-555, 282-283


52-53: Default chain ID mismatches kit config (10 vs 137).

Kit is initialized for Optimism (10), but the UI defaults to 137, which can confuse users. Align the default or derive from kit state.

-const [txChainId, setTxChainId] = useState<string>('137');
+const [txChainId, setTxChainId] = useState<string>('10');

Alternatively, initialize from kit.getState().chainId.


2309-2317: “Reset All & Clear Logs” re-adds a log after clearing.

You clear logs then immediately push a new log entry, so logs aren’t cleared.

 onClick={() => {
   kit.reset();
   setLogs([]);
   setCurrentState(kit.getState());
-  logAndUpdateState('🔄 All state reset and logs cleared');
 }}

1176-1186: “Toggle Debug Mode” only enables; implement a true toggle.

Maintain a local debug boolean and flip it, or expose a getter if available.

Sketch:

const [debug, setDebug] = useState(false);
kit.setDebugMode(!debug);
setDebug(!debug);
package.json (1)

29-33: Dependency alignment and semver strategy

  • Adding @zerodev/sdk and bumping viem looks fine. Consider pinning viem to a fixed minor (e.g., 2.38.x) or moving viem and @zerodev/sdk to peerDependencies to reduce duplicate installs and unexpected breakage for consumers.

Please confirm no API surface you use changed between 2.21.x → 2.38.x (esp. account-abstraction helpers).

rollup.config.js (1)

18-20: Also externalize 'viem/accounts'

You import types from 'viem/accounts' elsewhere; if any runtime import appears later, externalizing now avoids bundle surprises.

   'viem/chains',
   'viem/account-abstraction',
+  'viem/accounts',
   '@zerodev/sdk',
lib/utils/index.ts (1)

74-116: Sanitizer: make redaction extensible and include dataApiKey

Great addition. Recommend allowing custom key lists and redacting dataApiKey by default.

-const SENSITIVE_KEYS = ['privateKey', 'bundlerApiKey', 'bundlerApiKeyFormat'];
+const SENSITIVE_KEYS = [
+  'privateKey',
+  'bundlerApiKey',
+  'bundlerApiKeyFormat',
+  'dataApiKey',
+];

-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const sanitizeObject = (obj: any): any => {
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export const sanitizeObject = (obj: any, redactedKeys: string[] = SENSITIVE_KEYS): any => {
   if (!obj || typeof obj !== 'object') {
     return obj;
   }

   // Handle arrays
   if (Array.isArray(obj)) {
-    return obj.map((item) => sanitizeObject(item));
+    return obj.map((item) => sanitizeObject(item, redactedKeys));
   }

   // Handle objects
   const sanitized = { ...obj };

   for (const key in sanitized) {
     if (Object.prototype.hasOwnProperty.call(sanitized, key)) {
       const value = sanitized[key];

       // Check if this key should be redacted
       if (
-        SENSITIVE_KEYS.includes(key) &&
+        redactedKeys.includes(key) &&
         value !== undefined &&
         value !== null
       ) {
         sanitized[key] = '[REDACTED]';
       } else if (typeof value === 'object' && value !== null) {
         // Recursively sanitize nested objects
-        sanitized[key] = sanitizeObject(value);
+        sanitized[key] = sanitizeObject(value, redactedKeys);
       }
     }
   }

   return sanitized;
 };
CHANGELOG.md (1)

3-24: Release date order is inconsistent

2.1.0 (2025-01-27) is dated before 2.0.3 (2025-08-22). Adjust the 2.1.0 date to be later than 2.0.3.

-## [2.1.0] - 2025-01-27
+## [2.1.0] - 2025-10-16
lib/network/index.ts (1)

35-41: Single source of truth for supported chain IDs

You compute supported IDs from Networks here while constants.ts declares a SupportedNetworks array. Prefer one source to avoid drift; reuse SupportedNetworks.

-    const supportedChainIds = Object.keys(Networks)
-      .map(Number)
-      .sort((a, b) => a - b);
+    const supportedChainIds = [...SupportedNetworks].sort((a, b) => a - b);

Additionally, add this import at the top:

import { SupportedNetworks } from './constants';
lib/network/constants.ts (2)

1-4: Import cleanup: avoid duplicate chain imports

Use the existing Chain.* namespace consistently; remove separate named imports for bsc and gnosis.

-import { Chain as ChainType, defineChain } from 'viem';
-import * as Chain from 'viem/chains';
-import { bsc, gnosis } from 'viem/chains';
+import { Chain as ChainType, defineChain } from 'viem';
+import * as Chain from 'viem/chains';
   [100]: {
     chainId: 100,
-    chain: gnosis,
+    chain: Chain.gnosis,
   [56]: {
     chainId: 56,
-    chain: bsc,
+    chain: Chain.bsc,

Also applies to: 230-236, 334-338


5-9: Prevent drift between SupportedNetworks and Networks

SupportedNetworks is hard-coded while Networks is the authoritative registry. Consider deriving SupportedNetworks from Networks (or remove the array and centralize the helper in network/index.ts).

Also please verify all Chain.* names exist in viem@^2.38.0 (e.g., mantleSepoliaTestnet, ancient8/ancient8Sepolia, xdc/xdcTestnet). A missing export will break the build.

Also applies to: 110-594

lib/interfaces/index.ts (2)

12-13: Avoid deep import from modular-sdk internals

@etherspot/modular-sdk/dist/... is brittle. Re-export the type from the SDK root or this package, or import from a stable public path.


374-377: Bundler client type composition

BundlerClient & PublicActions & WalletActions is sensible. Ensure the generics align where used (Transport/Chain/Account) to preserve type inference.

lib/BundlerConfig.ts (1)

12-13: Prefer numeric chainId over string for clarity and type safety.

Storing chainId as a string is surprising; most consumers expect a number.

-  readonly chainId: string;
+  readonly chainId: number;
@@
-    this.chainId = chainId.toString();
+    this.chainId = chainId;

Also applies to: 37-38

lib/EtherspotProvider.ts (2)

300-305: Use referential equality for provider change detection.

Deep equality on providers is unnecessary and costly; identity check is sufficient.

-    const providerChanged =
-      !this.prevProvider ||
-      !isEqual(this.prevProvider, this.#publicConfig.provider);
+    const providerChanged = this.prevProvider !== this.#publicConfig.provider;

521-523: Optionally normalize privateKey to Hex.

If callers pass a non-0x-prefixed key, privateKeyToAccount will fail. Consider normalizing.

-    const owner = privateKeyToAccount(this.#privateConfig.privateKey as Hex);
+    const pk = this.#privateConfig.privateKey!.startsWith('0x')
+      ? this.#privateConfig.privateKey!
+      : (`0x${this.#privateConfig.privateKey!}` as const);
+    const owner = privateKeyToAccount(pk as Hex);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb48f08 and caa6bcf.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (11)
  • CHANGELOG.md (1 hunks)
  • example/src/App.tsx (14 hunks)
  • lib/BundlerConfig.ts (1 hunks)
  • lib/EtherspotProvider.ts (3 hunks)
  • lib/index.ts (1 hunks)
  • lib/interfaces/index.ts (6 hunks)
  • lib/network/constants.ts (1 hunks)
  • lib/network/index.ts (1 hunks)
  • lib/utils/index.ts (1 hunks)
  • package.json (2 hunks)
  • rollup.config.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
lib/network/index.ts (1)
lib/network/constants.ts (3)
  • NETWORK_NAME_TO_CHAIN_ID (69-108)
  • NetworkConfig (55-67)
  • Networks (110-594)
lib/EtherspotProvider.ts (4)
lib/interfaces/index.ts (6)
  • PrivateConfig (25-29)
  • PublicConfig (32-38)
  • DelegatedEoaModeConfig (50-58)
  • BundlerClientExtended (374-376)
  • EtherspotTransactionKitConfig (61-63)
  • WalletMode (22-22)
lib/BundlerConfig.ts (1)
  • BundlerConfig (11-66)
lib/utils/index.ts (1)
  • log (63-68)
lib/network/index.ts (1)
  • getNetworkConfig (20-22)
lib/BundlerConfig.ts (1)
lib/network/index.ts (1)
  • getNetworkConfig (20-22)
example/src/App.tsx (2)
lib/TransactionKit.ts (4)
  • TransactionKit (4140-4144)
  • batch (747-774)
  • name (697-732)
  • isDelegateSmartAccountToEoa (220-291)
lib/interfaces/index.ts (3)
  • WalletMode (22-22)
  • INamedTransaction (125-145)
  • IBatch (168-174)
🪛 Gitleaks (8.28.0)
lib/network/constants.ts

[high] 135-135: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 499-499: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 LanguageTool
CHANGELOG.md

[grammar] ~29-~29: There might be a mistake here.
Context: ...mandatory in the transaction() method. - For send and estimate, the `etherspo...

(QB_NEW_EN)

🔇 Additional comments (6)
example/src/App.tsx (2)

1285-1317: Good: EIP‑7702 actions gated by wallet mode.

Actions are only rendered when walletMode === 'delegatedEoa', aligning with the kit’s guard on isDelegateSmartAccountToEoa() and related methods.

As per interfaces and method guard in lib/TransactionKit.ts shown in snippets.


23-27: Ensure provider chain alignment for modular mode.

client is created for Optimism (10), and kit is also initialized with chainId: 10. If users input transactions for other chains (e.g., 137), confirm the kit/provider handles per‑tx multi‑chain correctly or guide the UI to switch networks accordingly.

Also applies to: 36-39, 309-315

lib/interfaces/index.ts (1)

74-103: Delegated EOA API surface looks good; confirm provider semantics

The interface still exposes getProvider(): WalletProviderLike. In delegatedEoa mode, ensure this returns a meaningful provider (or throws with a clear error).

lib/index.ts (1)

6-12: Re-exports look good; confirm intended public surface.

Making BundlerConfig and network modules public from the main entry is fine. Please confirm these are stable APIs you intend to support long‑term to avoid accidental surface expansion.

lib/EtherspotProvider.ts (2)

646-652: The code uses the correct API signature; no changes needed.

The viem documentation explicitly supports { client, transport } as a valid argument shape for createBundlerClient. The implementation in lib/EtherspotProvider.ts (lines 646–652) matches the documented API.

Likely an incorrect or invalid review comment.


471-475: Supported: createKernelAccount EIP-7702 options
createKernelAccount (v5.5.x) accepts eip7702Account; valid kernelVersion values are KERNEL_V3_1, KERNEL_V3_3, KERNEL_V3_3_BETA (or "0.3.1", "0.3.3"), and entryPoint should be obtained via getEntryPoint("0.7") (or "0.8" as needed).

Comment thread example/src/App.tsx
Comment thread example/src/App.tsx
Comment thread lib/BundlerConfig.ts
Comment thread lib/EtherspotProvider.ts
Comment thread lib/EtherspotProvider.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

56-67: Remove viem WalletClient; pass EIP-1193 provider directly.

The EtherspotProvider's WalletProviderLike type accepts string | WalletLike | WalletProvider | EthereumProvider, where EthereumProvider is an EIP-1193-compatible provider. The README example at lines 56–74 incorrectly passes a viem WalletClient, which is not in this union. Use window.ethereum directly instead.

-import { createWalletClient, custom } from 'viem';
-import { privateKeyToAccount } from 'viem/accounts';
 import { polygon } from 'viem/chains';
 
-// Set up your wallet provider (this is just an example)
-const account = privateKeyToAccount('0x...your-private-key...');
-const client = createWalletClient({
-  account,
-  chain: polygon,
-  transport: custom(window.ethereum!),
-});
+// Set up your EIP-1193 wallet provider (e.g., MetaMask)
+const provider = (window as any).ethereum;
 
 // Initialize TransactionKit
 const kit = TransactionKit({
-  provider: client,
+  provider,
   chainId: 137, // Polygon mainnet
   bundlerApiKey: 'your-bundler-api-key', // Optional but recommended
   walletMode: 'modular', // Optional: this is the default
 });
🧹 Nitpick comments (5)
__tests__/EtherspotProvider.test.ts (3)

565-659: Speed up and de-flake retry tests using fake timers.

Relying on real 1s sleeps slows CI. Use Jest fake timers and advance time instead.

-        it('should wait 1 second between retries', async () => {
-          mockModularSdk.getCounterFactualAddress
-            .mockRejectedValueOnce(new Error('Network error'))
-            .mockResolvedValueOnce('0x123');
-
-          const setTimeoutSpy = jest.spyOn(global, 'setTimeout');
-
-          await etherspotProvider.getSdk();
-
-          expect(setTimeoutSpy).toHaveBeenCalledWith(
-            expect.any(Function),
-            1000
-          );
-
-          setTimeoutSpy.mockRestore();
-        });
+        it('should wait 1 second between retries', async () => {
+          jest.useFakeTimers();
+          mockModularSdk.getCounterFactualAddress
+            .mockRejectedValueOnce(new Error('Network error'))
+            .mockResolvedValueOnce('0x123');
+
+          const sdkPromise = etherspotProvider.getSdk();
+          await jest.advanceTimersByTimeAsync(1000);
+          await sdkPromise;
+
+          expect(setTimeout).toHaveBeenCalledWith(expect.any(Function), 1000);
+          jest.useRealTimers();
+        });

Run tests locally to confirm the suite time improves.


501-517: Tighten assertion: concurrent calls to same chain should coalesce to one constructor.

Caching stores a single in-flight Promise per chain. Expect exactly one constructor call.

-        expect(MockedModularSdk.mock.calls.length).toBeGreaterThanOrEqual(1);
+        expect(MockedModularSdk).toHaveBeenCalledTimes(1);

1151-1175: Concurrent same-chain calls should still dedupe to one constructor.

Expect a single constructor call while returning the same instance to all callers.

-        // Each call creates a new SDK instance - this is the actual behavior
-        expect(MockedModularSdk).toHaveBeenCalledTimes(10);
+        expect(MockedModularSdk).toHaveBeenCalledTimes(1);
README.md (2)

232-236: Hyphenation nit.

Use “EIP‑7702‑specific” for correct hyphenation.

-For EIP-7702 specific functionalities:
+For EIP-7702‑specific functionalities:

504-515: Suggested change is valid but represents a workaround; proper fix should be gitleaks configuration.

The README.md file content is confirmed at lines 504-515. The current hardcoded example '0x1234567890abcdef...' does resemble an Ethereum private key and can trigger gitleaks pattern matching. Replacing it with '0xYOUR_PRIVATE_KEY_GOES_HERE' would reduce false positives.

However, the proper solution is to configure gitleaks using allowlists, entropy thresholds, or inline ignores (like //gitleaks:allow comments) rather than obfuscating documentation examples. The suggested change is a workaround rather than addressing the root cause in gitleaks configuration.

The change itself is harmless and reasonable for a BAD example, but should be considered optional rather than critical.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between caa6bcf and 423e153.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • README.md (11 hunks)
  • __tests__/EtherspotProvider.test.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
__tests__/EtherspotProvider.test.ts (2)
lib/interfaces/index.ts (2)
  • ModularModeConfig (41-47)
  • EtherspotTransactionKitConfig (61-63)
lib/EtherspotProvider.ts (1)
  • EtherspotProvider (53-760)
🪛 Gitleaks (8.28.0)
README.md

[high] 506-506: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 LanguageTool
README.md

[style] ~109-~109: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ated EOA Mode (EIP-7702) For users who want to use EIP-7702 delegated EOAs: ```typesc...

(REP_WANT_TO_VB)


[grammar] ~234-~234: Use a hyphen to join words.
Context: ...702 Delegated EOA Examples For EIP-7702 specific functionalities: ```typescript...

(QB_NEW_EN_HYPHEN)


[grammar] ~471-~471: Ensure spelling is correct
Context: ...egation ### Client Management Methods (delegatedEoa mode only) - getPublicClient() - Get...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~477-~477: Ensure spelling is correct
Context: ... chain ### Account Management Methods (delegatedEoa mode only) - `getDelegatedEoaAccount()...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

Comment thread __tests__/EtherspotProvider.test.ts Outdated
Comment thread __tests__/EtherspotProvider.test.ts
Comment thread __tests__/EtherspotProvider.test.ts
Comment thread __tests__/EtherspotProvider.test.ts
Comment thread __tests__/EtherspotProvider.test.ts
Comment thread __tests__/EtherspotProvider.test.ts
Comment thread README.md
Comment on lines +416 to +422
const queryFormat = new BundlerConfig(
137,
'your-api-key',
'https://bundler.example.com',
'&key=' // Results in: https://bundler.example.com&key=your-api-key
);
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Invalid URL in BundlerConfig query example.

Using “&key=” on a base URL without a query string yields an invalid URL. Prefer “?key=” or document that bundlerUrl must already contain a query.

-  'https://bundler.example.com',
-  '&key=' // Results in: https://bundler.example.com&key=your-api-key
+  'https://bundler.example.com',
+  '?key=' // Results in: https://bundler.example.com?key=your-api-key
🤖 Prompt for AI Agents
In README.md around lines 416 to 422, the BundlerConfig example uses '&key='
which produces an invalid URL when the base URL has no query string; update the
example to use '?key=' (so the resulting URL becomes
https://bundler.example.com?key=your-api-key) or alternatively add a short note
stating that bundlerUrl must already include a query string if using '&key=';
modify the example and/or add the clarifying sentence accordingly.

Comment thread README.md
Copy link
Copy Markdown
Collaborator

@IAmKio IAmKio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor changes and checks / questions but other than that it's a massive and substantial change for the better!

Comment thread lib/EtherspotProvider.ts
Comment thread lib/EtherspotProvider.ts Outdated
Comment thread lib/network/index.ts Outdated
Comment thread package.json Outdated
Comment thread lib/TransactionKit.ts Outdated
Comment thread lib/TransactionKit.ts
Comment thread lib/TransactionKit.ts Outdated
Comment thread lib/TransactionKit.ts
Comment thread lib/TransactionKit.ts
Comment thread lib/TransactionKit.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/utils/index.ts (2)

39-57: Logic bug: function always returns an error even after successful chain switch.

switchWalletProviderToChain returns { errorMessage: 'Failed to switch chain!' } unconditionally. Return success on happy path and only return error on failure.

 export const switchWalletProviderToChain = async (
   chainId: number
 ): Promise<{ errorMessage?: string }> => {
   if (!window?.ethereum) {
     console.warn('Unsupported browser!');
     return { errorMessage: 'Unsupported browser!' };
   }

   try {
     await window.ethereum.request({
       method: 'wallet_switchEthereumChain',
       params: [{ chainId: toHex(chainId) }], // chainId must be in hex
     });
-  } catch (e) {
-    console.warn('Failed to switch chain', e);
-  }
-
-  return { errorMessage: 'Failed to switch chain!' };
+    return {};
+  } catch (e) {
+    console.warn('Failed to switch chain', e);
+    return { errorMessage: 'Failed to switch chain!' };
+  }
 };

66-75: Prevent secret leakage in debug logs: sanitize before logging.

log() writes raw data; sanitizeObject() exists but isn’t used. Redact privateKey, bundlerApiKey, etc., in all debug logs.

 export const log = (message: string, data?: any, debugMode?: boolean): void => {
   if (debugMode) {
     // eslint-disable-next-line no-console
-    console.log(`[EtherspotTransactionKit] ${message}`, data || '');
+    console.log(
+      `[EtherspotTransactionKit] ${message}`,
+      data ? sanitizeObject(data) : ''
+    );
   }
 };

Also applies to: 77-123

♻️ Duplicate comments (5)
README.md (2)

419-425: Invalid URL example for BundlerConfig query format.

Using &key= on a base URL without an existing query yields an invalid URL. Use ?key= or document the preexisting query requirement. This was flagged earlier and still persists.

-  '&key=' // Results in: https://bundler.example.com&key=your-api-key
+  '?key=' // Results in: https://bundler.example.com?key=your-api-key

444-449: Align getTransactionHash example parameter comments with implementation.

Change inline comments to txChainId and retryInterval (camelCase) to match the code. Previously flagged, still present.

-  137, // chainId
+  137, // txChainId
   30000, // timeout (optional)
-  1000 // retry interval (optional)
+  1000 // retryInterval (optional)
lib/BundlerConfig.ts (1)

41-47: Robust bundler URL handling and presence check.

Still brittle: only checks empty string; may create malformed URLs (double '?', ignore fragments, etc.). Reuse prior suggestion to parse with URL, handle existing params, and treat falsy networkConfig.bundler as missing.

-    if (!bundlerUrl) {
-      const networkConfig = getNetworkConfig(chainId);
-      if (!networkConfig || networkConfig.bundler === '') {
-        throw new Error(`No bundler url provided for chain ID ${chainId}`);
-      }
-      bundlerUrl = networkConfig.bundler;
-    }
+    if (!bundlerUrl) {
+      const networkConfig = getNetworkConfig(chainId);
+      if (!networkConfig?.bundler) {
+        throw new Error(`No bundler URL provided for chain ID ${chainId}`);
+      }
+      bundlerUrl = networkConfig.bundler;
+    }
@@
-    // Append API key if provided
+    // Append API key if provided
     if (this._apiKey) {
       if (apiKeyFormat) {
         // Use custom format - just concatenate bundlerUrl + apiKeyFormat + apiKey
         // This gives maximum flexibility for any format (/, ?, &, etc.)
         this.url = bundlerUrl + apiKeyFormat + this._apiKey;
       } else {
-        if (bundlerUrl.includes('?api-key=')) {
-          this.url = bundlerUrl + this._apiKey;
-        } else {
-          this.url = `${bundlerUrl}?api-key=${this._apiKey}`;
-        }
+        try {
+          const u = new URL(bundlerUrl);
+          u.searchParams.set('api-key', this._apiKey);
+          this.url = u.toString();
+        } catch {
+          const hasQuery = bundlerUrl.includes('?');
+          const hasApiKeyParam = /[?&]api-key=/.test(bundlerUrl);
+          this.url = hasApiKeyParam
+            ? bundlerUrl.replace(/([?&]api-key=)[^&]*/, `$1${this._apiKey}`)
+            : `${bundlerUrl}${hasQuery ? '&' : '?'}api-key=${this._apiKey}`;
+        }
       }
     } else {
       this.url = bundlerUrl;
     }

Also applies to: 49-64

lib/EtherspotProvider.ts (2)

733-739: Return the public config type (not the union).

Method returns only public fields. Expose PublicConfig and drop the cast.

-  getConfig(): EtherspotTransactionKitConfig {
-    return { ...this.#publicConfig } as EtherspotTransactionKitConfig;
-  }
+  getConfig(): PublicConfig {
+    return { ...this.#publicConfig };
+  }

230-246: First delegatedEoa config change may skip cache invalidation.

Guarding on prevDelegatedEoaConfig prevents clearing on the first change. Compare unconditionally.

-      if (
-        this.prevDelegatedEoaConfig &&
-        !isEqual(this.prevDelegatedEoaConfig, newConfigObject)
-      ) {
-        this.clearDelegatedEoaCache();
-      }
-      this.prevDelegatedEoaConfig = newConfigObject;
+      if (!isEqual(this.prevDelegatedEoaConfig, newConfigObject)) {
+        this.clearDelegatedEoaCache();
+      }
+      this.prevDelegatedEoaConfig = newConfigObject;
🧹 Nitpick comments (14)
lib/utils/index.ts (2)

1-3: Avoid pulling lodash for simple numeric key sort.

Use native sort to reduce bundle size and a dependency.

-/* eslint-disable quotes */
-import { sortBy } from 'lodash';
+/* eslint-disable quotes */
@@
-export const getObjectSortedByKeys = (object: TypePerId<any>) =>
-  sortBy(Object.keys(object).map((key) => +key)).map((key) => object[key]);
+export const getObjectSortedByKeys = (object: TypePerId<any>) => {
+  return Object.keys(object)
+    .map((key) => Number(key))
+    .sort((a, b) => a - b)
+    .map((key) => object[key]);
+};

Also applies to: 16-19


81-81: Redaction scope nit: consider removing bundlerApiKeyFormat from SENSITIVE_KEYS.

Formatting hints aren’t secrets; redacting them can hinder debugging without improving security.

lib/BundlerConfig.ts (1)

12-14: Keep chainId as a number to avoid downstream type friction.

The class stores chainId as string; consumers typically expect a number.

 export class BundlerConfig {
   readonly url: string;
-  readonly chainId: string;
+  readonly chainId: number;
   private readonly _apiKey: string | undefined;
@@
   ) {
-    this.chainId = chainId.toString();
+    this.chainId = chainId;
     this._apiKey = apiKey;

Also applies to: 31-39

lib/constants/index.ts (5)

5-9: Tighten types for IDs/maps to prevent drift.

Use literal typing for SupportedNetworks and precise Records for the maps to catch mismatches at compile time.

-export const SupportedNetworks = [
+export const SupportedNetworks = [
   1, 10, 14, 30, 31, 50, 51, 56, 97, 100, 114, 122, 123, 137, 2357, 5000, 5003,
   8453, 10200, 42161, 42220, 43113, 43114, 44787, 59140, 59144, 80002, 84532,
   421614, 534351, 534352, 11155111, 11155420, 28122024, 79479957, 888888888,
-];
+] as const;
+
+export type SupportedNetworkId = typeof SupportedNetworks[number];
-export const NETWORK_NAME_TO_CHAIN_ID: {
-  [key: string]: number;
-} = {
+export const NETWORK_NAME_TO_CHAIN_ID: Record<NetworkNames, number> = {
-export const Networks: {
-  [key: string]: NetworkConfig;
-} = {
+export const Networks: Record<number, NetworkConfig> = {

Also applies to: 69-71, 110-112


1-3: Unify viem chain imports for consistency.

Mixing * as Chain with named bsc, gnosis is noisy. Prefer a single style.

-import * as Chain from 'viem/chains';
-import { bsc, gnosis } from 'viem/chains';
+import * as Chain from 'viem/chains';
-    chain: gnosis,
+    chain: Chain.gnosis,
-    chain: bsc,
+    chain: Chain.bsc,

Also applies to: 232-232, 336-336


135-136: Gitleaks “generic-api-key” is a false positive on 0x addresses.

The flagged strings are contract addresses, not credentials. Add an allowlist to stop noisy failures.

Example .gitleaks.toml entries:

[[rules.allowlist.regexes]]
description = "Ethereum addresses (40 hex chars with 0x)"
regex = '''0x[a-fA-F0-9]{40}'''

[[rules.allowlist.paths]]
description = "Network constants"
path = '''lib/constants/index\.ts'''

Please confirm CI quiets after this change.

Also applies to: 499-500


361-364: Empty bundler URLs will make delegatedEoa clients fail.

Chains with bundler: '' will throw in BundlerConfig and break 7702 flows. Either:

  • remove such chains from SupportedNetworks for delegatedEoa, or
  • document and guard with a clearer error, or
  • provide a fallback RPC/bundler per chain.

Also applies to: 389-390, 243-247


27-29: Normalize enum value casing.

'Mantle' | 'MantleSepolia' values deviate from the lower‑camel pattern used elsewhere (e.g., 'baseSepolia'). Consider harmonizing to avoid subtle key mismatches in external code.

lib/EtherspotProvider.ts (4)

298-307: Use reference equality for provider change detection.

isEqual on providers is slow and unreliable. Identity is enough here.

-    const providerChanged =
-      !this.prevProvider ||
-      !isEqual(this.prevProvider, this.#publicConfig.provider);
+    const providerChanged =
+      this.prevProvider !== this.#publicConfig.provider;

127-138: Validate privateKey format early.

Add a simple hex check to fail fast on malformed keys.

   } else if (config.walletMode === 'delegatedEoa') {
@@
-      if (!delegatedEoaConfig.privateKey) {
+      if (!delegatedEoaConfig.privateKey) {
         throw new Error(
           'privateKey is required when walletMode is "delegatedEoa". Please provide a private key in the configuration.'
         );
       }
+      if (!/^0x[0-9a-fA-F]{64}$/.test(delegatedEoaConfig.privateKey)) {
+        throw new Error('Invalid privateKey format. Expected 0x-prefixed 32-byte hex string.');
+      }

381-424: Make cache writes idempotent to avoid duplicate in-flight creations.

Two concurrent callers can each create a new Promise before the map is set. Use ??= to coalesce.

-    if (chainId in this.publicClientPerChain) {
-      return this.publicClientPerChain[chainId];
-    }
-    this.publicClientPerChain[chainId] = (async () => {
+    this.publicClientPerChain[chainId] ??= (async () => {
       // ...
     })();
     return this.publicClientPerChain[chainId];

Repeat the same pattern for walletClientPerChain, bundlerClientPerChain, and sdkPerChain.

Also applies to: 559-597, 628-662, 311-356


318-321: Avoid sending a placeholder API key to the bundler.

'__ETHERSPOT_BUNDLER_API_KEY__' may hit the network and confuse analytics. Prefer omitting the key when absent (if constructor allows).

Can new EtherspotBundler(chainId, apiKey) accept undefined? If yes, change:

bundlerProvider: new EtherspotBundler(+sdkChainId, this.#privateConfig.bundlerApiKey)

Otherwise consider an empty string and handle it internally without appending.

lib/interfaces/index.ts (2)

92-103: Optional undelegateSmartAccountToEoa impacts DX.

Making the method optional forces callers to null‑check. If the feature is gated at runtime, prefer a required method that rejects when unsupported, or document the optionality clearly in release notes.


12-12: Avoid deep import for BigNumberish.

Deep paths are brittle. If available, import the type from the package’s public exports.

-import { BigNumberish } from '@etherspot/modular-sdk/dist/types/sdk/types/bignumber';
+import type { BigNumberish } from '@etherspot/modular-sdk';

Please confirm the public export exists; if not, consider re‑exporting it in your SDK.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 423e153 and cf9d989.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • README.md (11 hunks)
  • example/src/App.tsx (14 hunks)
  • lib/BundlerConfig.ts (1 hunks)
  • lib/EtherspotProvider.ts (3 hunks)
  • lib/constants/index.ts (1 hunks)
  • lib/index.ts (1 hunks)
  • lib/interfaces/index.ts (6 hunks)
  • lib/utils/index.ts (2 hunks)
  • package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json
🧰 Additional context used
🧬 Code graph analysis (4)
lib/utils/index.ts (1)
lib/constants/index.ts (3)
  • NETWORK_NAME_TO_CHAIN_ID (69-108)
  • NetworkConfig (55-67)
  • Networks (110-594)
lib/EtherspotProvider.ts (3)
lib/interfaces/index.ts (6)
  • PrivateConfig (25-29)
  • PublicConfig (32-38)
  • DelegatedEoaModeConfig (50-58)
  • BundlerClientExtended (374-376)
  • EtherspotTransactionKitConfig (61-63)
  • WalletMode (22-22)
lib/BundlerConfig.ts (1)
  • BundlerConfig (11-66)
lib/utils/index.ts (2)
  • log (70-75)
  • getNetworkConfig (134-136)
example/src/App.tsx (2)
lib/TransactionKit.ts (4)
  • TransactionKit (4135-4139)
  • batch (742-769)
  • name (692-727)
  • isDelegateSmartAccountToEoa (217-288)
lib/interfaces/index.ts (3)
  • WalletMode (22-22)
  • INamedTransaction (125-145)
  • IBatch (168-174)
lib/BundlerConfig.ts (1)
lib/utils/index.ts (1)
  • getNetworkConfig (134-136)
🪛 Gitleaks (8.28.0)
lib/constants/index.ts

[high] 135-135: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 499-499: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 LanguageTool
README.md

[style] ~109-~109: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ated EOA Mode (EIP-7702) For users who want to use EIP-7702 delegated EOAs: ```typesc...

(REP_WANT_TO_VB)


[grammar] ~237-~237: Use a hyphen to join words.
Context: ...702 Delegated EOA Examples For EIP-7702 specific functionalities: ```typescript...

(QB_NEW_EN_HYPHEN)


[grammar] ~474-~474: Ensure spelling is correct
Context: ...egation ### Client Management Methods (delegatedEoa mode only) - getPublicClient() - Get...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~480-~480: Ensure spelling is correct
Context: ... chain ### Account Management Methods (delegatedEoa mode only) - `getDelegatedEoaAccount()...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments (3)
lib/index.ts (1)

6-10: Public re-exports look good.

Minimal surface-area change; no runtime risk detected.

example/src/App.tsx (1)

1211-1219: Unnecessary refactoring—getState() does not expose secrets.

Based on verification of the codebase:

  • The IInstance interface returned by getState() contains only: selectedTransactionName, selectedBatchName, workingTransaction, namedTransactions, batches, isEstimating, isSending, containsSendingError, containsEstimatingError, and walletAddresses.
  • TransactionBuilder contains only: chainId, to, value, data, transactionName, and batchName—no sensitive fields like privateKey or bundlerApiKey.
  • No secrets are exposed via logging kit.getState(). The review comment's concern was based on a hypothetical assumption that was not validated against the actual interface definition.

The logging code is safe as-is. No refactoring is required.

Likely an incorrect or invalid review comment.

lib/EtherspotProvider.ts (1)

406-411: Confirm bundler endpoints act as full JSON-RPC for viem clients.

http(bundlerConfig.url) is used for Public/Wallet/Bundler clients. Ensure these endpoints proxy standard eth_* methods; otherwise separate RPC and bundler transports are needed.

Also applies to: 578-584, 642-649

Comment thread example/src/App.tsx
Comment thread lib/EtherspotProvider.ts
Comment on lines +215 to +229
this.#publicConfig = {
...this.#publicConfig,
chainId: newConfig.chainId ?? this.#publicConfig.chainId,
walletMode: newConfig.walletMode ?? this.#publicConfig.walletMode,
debugMode: newConfig.debugMode ?? this.#publicConfig.debugMode,
bundlerUrl:
'bundlerUrl' in newConfig
? newConfig.bundlerUrl
: this.#publicConfig.bundlerUrl,
provider:
'provider' in newConfig
? newConfig.provider
: this.#publicConfig.provider,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate modular mode still has a provider after updates.

updateConfig() can unset provider while staying in 'modular'. Add a guard.

     this.#publicConfig = {
       ...this.#publicConfig,
       chainId: newConfig.chainId ?? this.#publicConfig.chainId,
       walletMode: newConfig.walletMode ?? this.#publicConfig.walletMode,
       debugMode: newConfig.debugMode ?? this.#publicConfig.debugMode,
       bundlerUrl:
         'bundlerUrl' in newConfig
           ? newConfig.bundlerUrl
           : this.#publicConfig.bundlerUrl,
       provider:
         'provider' in newConfig
           ? newConfig.provider
           : this.#publicConfig.provider,
     };
 
     // Clear all caches if wallet mode changed
+    if (this.getWalletMode() === 'modular' && !this.#publicConfig.provider) {
+      throw new Error('Provider is required when walletMode is "modular".');
+    }

Also applies to: 230-233

🤖 Prompt for AI Agents
In lib/EtherspotProvider.ts around lines 215 to 229 (and likewise for lines
230-233), updateConfig currently allows clearing provider while walletMode
remains 'modular'; after merging the newConfig ensure that if the resulting
walletMode is 'modular' there is a valid provider — either preserve the existing
provider when newConfig explicitly omits or sets provider to undefined, or
throw/return an error if no prior provider exists; implement the guard
immediately after the merge so the public config never ends up in modular mode
without a provider.

Comment thread README.md
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (7)
__tests__/EtherspotProvider.test.ts (5)

1211-1222: Exercise string chainId conversion by passing a string to the constructor.

Current test never passes a string; it asserts the numeric path only.

-      it('should handle chainId as string (converted to number)', async () => {
-        mockModularSdk.getCounterFactualAddress.mockResolvedValue('0x123');
-
-        await etherspotProvider.getSdk();
-
-        expect(MockedModularSdk).toHaveBeenCalledWith(
-          mockProvider,
-          expect.objectContaining({
-            chainId: 1, // Should be converted to number
-          })
-        );
-      });
+      it('should handle chainId as string (converted to number)', async () => {
+        mockModularSdk.getCounterFactualAddress.mockResolvedValue('0x123');
+        const providerWithStringId = new EtherspotProvider({
+          provider: mockProvider,
+          chainId: '1' as any,
+        });
+        await providerWithStringId.getSdk();
+        expect(MockedModularSdk).toHaveBeenCalledWith(
+          mockProvider,
+          expect.objectContaining({ chainId: 1 })
+        );
+      });

887-895: Make “invalid private key” test actually fail by throwing once from the mock.

Currently privateKeyToAccountMock always returns an account; the test cannot assert a failure.

       it('should handle invalid private key format', async () => {
+        // Make viem/accounts reject this input once
+        privateKeyToAccountMock.mockImplementationOnce(() => {
+          throw new Error('Invalid private key');
+        });
         const delegated = new EtherspotProvider({
           chainId: 1,
           walletMode: 'delegatedEoa',
           privateKey: 'invalid-private-key',
         } as EtherspotTransactionKitConfig);

         await expect(delegated.getOwnerAccount()).rejects.toThrow();
       });

936-947: Override getNetworkConfig to return undefined for this test.

A global mock returns a config; the “network config not found” path is never reached.

       it('should handle network config not found', async () => {
+        const { getNetworkConfig } = require(networkModulePath) as {
+          getNetworkConfig: jest.Mock;
+        };
+        getNetworkConfig.mockReturnValueOnce(undefined);
         const delegated = new EtherspotProvider({
           chainId: 999999, // Non-existent chain
           walletMode: 'delegatedEoa',
           privateKey:
             '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
         } as EtherspotTransactionKitConfig);

         await expect(delegated.getPublicClient(999999)).rejects.toThrow(
           'Network configuration not found for chain ID 999999'
         );
       });

975-988: Return a config with no bundler URL to exercise the error path.

This test assumes missing bundler but the mock always provides one.

       it('should handle network config missing for getWalletClient', async () => {
-        // Create a new provider with a non-existent chain ID to test network config missing
+        const { getNetworkConfig } = require(networkModulePath) as {
+          getNetworkConfig: jest.Mock;
+        };
+        getNetworkConfig.mockReturnValueOnce({
+          chain: { id: 999999, name: 'NoBundlerChain' },
+          bundler: undefined,
+        });
         const delegated = new EtherspotProvider({
           chainId: 1,
           walletMode: 'delegatedEoa',
           privateKey:
             '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
         } as EtherspotTransactionKitConfig);

         // Test with a non-existent chain ID
         await expect(delegated.getWalletClient(999999)).rejects.toThrow(
           'No bundler url provided for chain ID 999999'
         );
       });

1121-1141: Fix SDK caching expectation: same chain returns the same instance.

Provider caches per chain; expect one constructor call and identical instances.

-      it('should not create duplicate SDK instances for same chain', async () => {
+      it('should not create duplicate SDK instances for same chain', async () => {
         // Create a fresh provider for this test
         const freshProvider = new EtherspotProvider({
           chainId: 1,
           provider: mockProvider,
           walletMode: 'modular',
         });

         // Reset mock call count
         MockedModularSdk.mockClear();

         // Multiple calls to same chain
-        await freshProvider.getSdk(1);
-        await freshProvider.getSdk(1);
-        await freshProvider.getSdk(1);
+        const s1 = await freshProvider.getSdk(1);
+        const s2 = await freshProvider.getSdk(1);
+        const s3 = await freshProvider.getSdk(1);
+
+        expect(s1).toBe(s2);
+        expect(s2).toBe(s3);

-        // The SDK should be called multiple times because each call creates a new instance
-        // This is the actual behavior - caching happens at the provider level, not SDK level
-        expect(MockedModularSdk).toHaveBeenCalledTimes(3);
+        expect(MockedModularSdk).toHaveBeenCalledTimes(1);
       });
README.md (1)

419-425: Fix invalid URL example: use “?key=” when base URL has no query.

Using “&key=” on a URL without “?” yields an invalid URL.

 const queryFormat = new BundlerConfig(
   137,
   'your-api-key',
   'https://bundler.example.com',
-  '&key=' // Results in: https://bundler.example.com&key=your-api-key
+  '?key=' // Results in: https://bundler.example.com?key=your-api-key
 );
lib/EtherspotProvider.ts (1)

215-233: Post-merge validation missing: modular mode can end up without a provider; delegatedEoa can lack privateKey.

updateConfig merges values but never re-validates the resulting state. Switching to 'modular' without a provider or to 'delegatedEoa' without a privateKey leaves the instance invalid and causes later crashes.

Apply after the public/private merge:

     this.#publicConfig = {
       ...this.#publicConfig,
       chainId: newConfig.chainId ?? this.#publicConfig.chainId,
       walletMode: newConfig.walletMode ?? this.#publicConfig.walletMode,
       debugMode: newConfig.debugMode ?? this.#publicConfig.debugMode,
       bundlerUrl:
         'bundlerUrl' in newConfig
           ? newConfig.bundlerUrl
           : this.#publicConfig.bundlerUrl,
       provider:
         'provider' in newConfig
           ? newConfig.provider
           : this.#publicConfig.provider,
     };

-    // Clear all caches if wallet mode changed
+    // Validate resulting mode requirements before proceeding
+    if (this.getWalletMode() === 'modular' && !this.#publicConfig.provider) {
+      throw new Error('Provider is required when walletMode is "modular".');
+    }
+    if (this.getWalletMode() === 'delegatedEoa' && !this.#privateConfig.privateKey) {
+      throw new Error('privateKey is required when walletMode is "delegatedEoa".');
+    }
+
+    // Clear all caches if wallet mode changed
     if (walletModeChanged) {
       this.clearAllCaches();
     } else if (this.getWalletMode() === 'delegatedEoa') {
🧹 Nitpick comments (7)
lib/EtherspotProvider.ts (2)

295-304: Provider change detection: use reference equality instead of deep isEqual.

Deep compare on providers (functions, event emitters) is slow and brittle. Reference check is sufficient.

-    const providerChanged =
-      !this.prevProvider ||
-      !isEqual(this.prevProvider, this.#publicConfig.provider);
+    const providerChanged = this.prevProvider !== this.#publicConfig.provider;

508-517: Normalize and validate private key before privateKeyToAccount.

Some callers may pass a non-0x string; viem expects 0x-prefixed hex and 32-byte length.

-    const owner = privateKeyToAccount(this.#privateConfig.privateKey as Hex);
+    const raw = this.#privateConfig.privateKey!;
+    const hex = raw.startsWith('0x') ? raw : (`0x${raw}` as string);
+    if (hex.length !== 66) {
+      throw new Error('getOwnerAccount(): privateKey must be 32 bytes (0x-prefixed).');
+    }
+    const owner = privateKeyToAccount(hex as Hex);
__tests__/EtherspotProvider.test.ts (1)

1254-1264: Await getSdk() in destroy test to avoid unhandled promise work.

Make test async and await the call before destroy.

-      it('should handle destroy method cleanup', () => {
+      it('should handle destroy method cleanup', async () => {
         // Create some state
         mockModularSdk.getCounterFactualAddress.mockResolvedValue('0x123');
-        etherspotProvider.getSdk();
+        await etherspotProvider.getSdk();

         // Destroy should not throw
         expect(() => etherspotProvider.destroy()).not.toThrow();

         // After destroy, should be able to create new instances
-        expect(() => etherspotProvider.getSdk()).not.toThrow();
+        await expect(etherspotProvider.getSdk()).resolves.toBeDefined();
       });
README.md (1)

444-449: Keep parameter comment consistent: retryInterval (camelCase).

 const txHash = await kit.getTransactionHash(
   '0x123...userOpHash',
   137, // txChainId
   30000, // timeout (optional)
-  1000 // retry interval (optional)
+  1000 // retryInterval (optional)
 );
example/src/App.tsx (1)

295-316: Minor: avoid reassigning module-scoped “kit” in React; prefer state.

Reassigning a module variable can confuse React lifecycles. Consider storing the kit in a ref/state and updating via setter.

lib/interfaces/index.ts (2)

49-58: Type the delegatedEoa privateKey as Hex for stronger guarantees.

Catching non-0x keys at compile time prevents runtime errors.

-import { SignAuthorizationReturnType } from 'viem/accounts';
+import { SignAuthorizationReturnType } from 'viem/accounts';
+import type { Hex } from 'viem';
@@
 export interface DelegatedEoaModeConfig {
   chainId: number;
@@
   walletMode: 'delegatedEoa';
-  privateKey: string;
+  privateKey: Hex;
 }

103-115: Document mode restriction on getProvider in the interface.

At runtime this throws in delegatedEoa mode; make it explicit in JSDoc.

   getProvider(): WalletProviderLike;
+  /**
+   * Only valid in 'modular' wallet mode; will throw in 'delegatedEoa'.
+   */
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf9d989 and 64647dd.

📒 Files selected for processing (5)
  • README.md (11 hunks)
  • __tests__/EtherspotProvider.test.ts (3 hunks)
  • example/src/App.tsx (14 hunks)
  • lib/EtherspotProvider.ts (3 hunks)
  • lib/interfaces/index.ts (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
lib/EtherspotProvider.ts (3)
lib/interfaces/index.ts (6)
  • PrivateConfig (25-29)
  • PublicConfig (32-38)
  • DelegatedEoaModeConfig (50-58)
  • BundlerClientExtended (374-376)
  • EtherspotTransactionKitConfig (61-63)
  • WalletMode (22-22)
lib/BundlerConfig.ts (1)
  • BundlerConfig (11-66)
lib/utils/index.ts (2)
  • log (70-75)
  • getNetworkConfig (134-136)
__tests__/EtherspotProvider.test.ts (2)
lib/interfaces/index.ts (2)
  • ModularModeConfig (41-47)
  • EtherspotTransactionKitConfig (61-63)
lib/EtherspotProvider.ts (1)
  • EtherspotProvider (50-754)
example/src/App.tsx (2)
lib/TransactionKit.ts (4)
  • TransactionKit (4135-4139)
  • batch (742-769)
  • name (692-727)
  • isDelegateSmartAccountToEoa (217-288)
lib/interfaces/index.ts (3)
  • WalletMode (22-22)
  • INamedTransaction (125-145)
  • IBatch (168-174)
🪛 LanguageTool
README.md

[style] ~109-~109: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ated EOA Mode (EIP-7702) For users who want to use EIP-7702 delegated EOAs: ```typesc...

(REP_WANT_TO_VB)


[grammar] ~237-~237: Use a hyphen to join words.
Context: ...702 Delegated EOA Examples For EIP-7702 specific functionalities: ```typescript...

(QB_NEW_EN_HYPHEN)


[grammar] ~474-~474: Ensure spelling is correct
Context: ...egation ### Client Management Methods (delegatedEoa mode only) - getPublicClient() - Get...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~480-~480: Ensure spelling is correct
Context: ... chain ### Account Management Methods (delegatedEoa mode only) - `getDelegatedEoaAccount()...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

Copy link
Copy Markdown
Collaborator

@IAmKio IAmKio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rocking and rolling 🚀

@RanaBug RanaBug merged commit 0c9476a into master Oct 20, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants