Dxp 108 create genlayer client like viem examples #106
Conversation
…ypes; add comment on viem conflict
WalkthroughRefactors action factories and client assembly to use a new generic BaseActionsClient, widens public signatures (accounts/chains/wallet/transactions/receipts/contracts), introduces ContractCapabilities and BaseActionsClient types, changes client assembly to a mergeActions pipeline, and adjusts connect to accept a minimal {chain?} object. No runtime behavior changes intended. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Build as createPublicClient
participant Base as baseClient
participant Pub as publicActions
participant Wal as walletActions
participant Acc as accountActions
participant GWal as genlayerWalletActions
participant Cha as chainActions
participant Con as contractActions
participant Tx as transactionActions
participant Rc as receiptActions
Note over Build: Client assembly via mergeActions (sequential merges)
Build->>Base: start with baseClient
Build->>Base: merge(Public + Wallet) -> baseWithViem
Build->>Base: merge(accountActions)
Build->>Base: merge(genlayerWalletActions)
Build->>Base: merge(chainActions)
Build->>Base: merge(contractActions)
Build->>Base: merge(transactionActions)
Build->>Base: merge(receiptActions)
Note over Build,Base: finalClientTyped used for init and return
sequenceDiagram
autonumber
participant UI as Caller
participant TxA as transactionActions
participant Cl as BaseActionsClient<TChain>
participant Pub as PublicClient
alt Localnet branch
UI->>TxA: getTransaction(hash)
TxA->>Cl: request("eth_getTransactionByHash", [hash])
Cl-->>TxA: GenLayerTransaction
TxA->>TxA: decodeLocalnetTransaction(tx)
TxA-->>UI: Decoded tx
else Remote branch
UI->>TxA: getTransaction(hash)
TxA->>Pub: readContract(...)
Pub-->>TxA: Encoded tx
TxA->>TxA: decodeTransaction(...)
TxA-->>UI: Decoded tx
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (4)
src/contracts/actions.ts (2)
194-219: Guard against missing consensus ABI before encoding
_encodeAddTransactionDatacallsencodeFunctionDatawithclient.chain.consensusMainContract?.abi as any. If the contract wasn’t initialized, this will throw at runtime. Fail fast with a clear error.Outside-the-hunk code change:
const _encodeAddTransactionData = ({ client, senderAccount, recipient, data, consensusMaxRotations = client.chain.defaultConsensusMaxRotations, }: { client: ContractCapabilities; senderAccount?: Account; recipient?: `0x${string}`; data?: `0x${string}`; consensusMaxRotations?: number; }): `0x${string}` => { const validatedSenderAccount = validateAccount(senderAccount); if (!client.chain.consensusMainContract?.abi) { throw new Error("Consensus main contract not initialized. Call initializeConsensusSmartContract() first."); } return encodeFunctionData({ abi: client.chain.consensusMainContract.abi as any, functionName: "addTransaction", args: [ validatedSenderAccount.address, recipient, client.chain.defaultNumberOfInitialValidators, consensusMaxRotations, data, ], }); };
235-305: Unify return semantics between remote and local signing; also stop hardcoding gas
- Remote path currently returns the Ethereum tx hash from
eth_sendTransaction, while the local path waits for a receipt and returns the consensustxIdparsed fromNewTransaction. This asymmetry is surprising and likely breaks callers expecting a consensustxId.- Hardcoded
gas: 21000nand'0x5208'are too low for contract calls and can cause OOG. Let viem/provider estimate or compute it explicitly.Apply this diff to (a) remove hardcoded gas, and (b) also wait and parse the event in the remote path so both branches return the consensus
txId:const transactionRequest = await client.prepareTransactionRequest({ account: validatedSenderAccount, to: client.chain.consensusMainContract?.address as Address, data: encodedData, type: "legacy", nonce: Number(nonce), value: value, - gas: 21000n, }); @@ - const formattedRequest = { + const formattedRequest = { from: transactionRequest.from, to: transactionRequest.to, data: encodedData, value: transactionRequest.value ? `0x${transactionRequest.value.toString(16)}` : "0x0", - gas: transactionRequest.gas ? `0x${transactionRequest.gas.toString(16)}` : "0x5208", }; - - return await client.request({ - method: "eth_sendTransaction", - params: [formattedRequest as any], - }); + const txHash = (await client.request({ + method: "eth_sendTransaction", + params: [formattedRequest as any], + })) as `0x${string}`; + + const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash }); + if (receipt.status === "reverted") throw new Error("Transaction reverted"); + + const newTxEvents = parseEventLogs({ + abi: client.chain.consensusMainContract?.abi as any, + eventName: "NewTransaction", + logs: receipt.logs, + }) as unknown as { args: { txId: `0x${string}` } }[]; + if (newTxEvents.length === 0) throw new Error("Transaction not processed by consensus"); + return newTxEvents[0].args["txId"];src/client/client.ts (2)
46-55: Guard window.ethereum for non-browser environments.Accessing window throws in Node/SSR. Use a typeof check.
- const provider = config.provider || window.ethereum; + const provider = + config.provider ?? + (typeof window !== "undefined" ? (window as any).ethereum : undefined);
56-58: Avoid “Chain is not set” when callers pass an empty config.If createClient({}) is used, config.chain is undefined here, even though you computed chainConfig. Pass the resolved chain into the transport config.
- const customTransport = custom(getCustomTransportConfig(config)); + const customTransport = custom(getCustomTransportConfig({ ...config, chain: chainConfig }));Also applies to: 96-96
♻️ Duplicate comments (1)
src/wallet/actions.ts (1)
5-8: Type mismatch withconnectafter its signature change
connectexpects an object with a requiredchainproperty, but you pass the fullBaseActionsClient<TChain>wherechainis optional per viem. This will fail type-checking. After adjustingconnect’s param to{ chain?: GenLayerChain }(see connect.ts review), this call becomes valid and preserves the desired side-effect of mutating the original client’schain.
🧹 Nitpick comments (16)
src/wallet/connect.ts (3)
33-39: GuardblockExplorerUrlsto avoid passing[undefined]to MetaMaskIf a chain doesn’t define a block explorer,
blockExplorerUrls: [undefined]can cause the RPC to fail. Only include the property when present.Apply this diff:
- const chainParams = { + const chainParams: any = { chainId: chainIdHex, chainName: selectedNetwork.name, rpcUrls: selectedNetwork.rpcUrls.default.http, nativeCurrency: selectedNetwork.nativeCurrency, - blockExplorerUrls: [selectedNetwork.blockExplorers?.default.url], }; + if (selectedNetwork.blockExplorers?.default?.url) { + chainParams.blockExplorerUrls = [selectedNetwork.blockExplorers.default.url]; + }
41-51: Prefer “switch then add-if-4902” flow for better UX and fewer promptsBest practice is to try
wallet_switchEthereumChainfirst and only fall back towallet_addEthereumChainwhen the chain is unknown (error code 4902). Your current flow always calls add, which can prompt the user unnecessarily.Apply this diff:
- if (currentChainId !== chainIdHex) { - await window.ethereum.request({ - method: "wallet_addEthereumChain", - params: [chainParams], - }); - await window.ethereum.request({ - method: "wallet_switchEthereumChain", - params: [{chainId: chainIdHex}], - }); - } + if (currentChainId !== chainIdHex) { + try { + await window.ethereum.request({ + method: "wallet_switchEthereumChain", + params: [{chainId: chainIdHex}], + }); + } catch (err: any) { + if (err?.code === 4902) { + await window.ethereum.request({ + method: "wallet_addEthereumChain", + params: [chainParams], + }); + await window.ethereum.request({ + method: "wallet_switchEthereumChain", + params: [{chainId: chainIdHex}], + }); + } else { + throw err; + } + } + }
53-64: Tighten types and error handling for Snaps discovery/installation
installedSnaps: anyandsnap: anylose the benefit of the PR’s type-safety work.- Consider catching and surfacing errors from
wallet_requestSnaps(users often reject the prompt).Example improvement (outside current hunk):
type InstalledSnap = { id: string; version: string; permissionName?: string }; const installedSnaps = (await window.ethereum.request({ method: "wallet_getSnaps" })) as Record<string, InstalledSnap>; const isGenLayerSnapInstalled = Object.values(installedSnaps).some((snap) => snap.id === id); if (!isGenLayerSnapInstalled) { try { await window.ethereum.request({ method: "wallet_requestSnaps", params: { [id]: {} } }); } catch (e) { throw new Error(`Failed to install GenLayer Snap (${id}): ${(e as Error).message}`); } }src/types/clients.ts (1)
95-99: Giverequesta typed return to eliminateunknownat call sitesThe overload constrains params, but it still returns
Promise<unknown>. That forces downstream casts in contract actions. Add a method-to-result mapping for the subset you use (e.g.,gen_call,gen_getContractSchema*,eth_sendRawTransaction) and fall back tounknownfor the rest.Apply this diff to refine the return type:
- request: Client<Transport, TGenLayerChain>["request"] & { - <TMethod extends GenLayerMethod>( - args: Extract<GenLayerMethod, {method: TMethod["method"]}>, - ): Promise<unknown>; - }; + request: Client<Transport, TGenLayerChain>["request"] & { + <TMethod extends GenLayerMethod>( + args: Extract<GenLayerMethod, { method: TMethod["method"] }>, + ): Promise<GenLayerMethodResult<TMethod>>; + };And add this helper type (outside the hunk):
type GenLayerMethodResult<M extends GenLayerMethod> = M["method"] extends "gen_call" ? string : M["method"] extends "gen_getContractSchema" ? string : M["method"] extends "gen_getContractSchemaForCode" ? string : M["method"] extends "eth_sendRawTransaction" ? `0x${string}` : unknown;src/contracts/actions.ts (2)
185-192: Nit: parameter name casing invalidateAccount
validateAccount(Account?: Account)uses a capitalized variable name, which is easy to confuse with the type. Rename toaccountfor readability.Apply this diff (outside current hunk):
-const validateAccount = (Account?: Account): Account => { - if (!Account) { +const validateAccount = (account?: Account): Account => { + if (!account) { throw new Error( "No account set. Configure the client with an account or pass an account to this function.", ); } - return Account; + return account; };
49-92: Optional: tighten result typing inreadContractto drop castsOnce
BaseActionsClient.requestis typed (see clients.ts comment), you can avoid(await client.request(...)) as stringandas anycasts here.src/chains/actions.ts (1)
19-38: Add basic response shape validation and better error messagingThe RPC call assumes a JSON-RPC success envelope and
resultshape. Guard for malformed responses and surface the RPC error message if present.Example (outside current hunk):
const json = await contractsResponse.json(); if ("error" in json) { throw new Error(`sim_getConsensusContract failed: ${json.error?.message ?? "Unknown error"}`); } if (!json?.result?.abi || !json?.result?.address) { throw new Error("Unexpected ConsensusMain contract payload"); } client.chain.consensusMainContract = json.result;src/accounts/actions.ts (2)
11-15: Consider stronger typing for RPC method/return.You cast the result to Promise. Prefer tightening the request signature so this cast becomes unnecessary (e.g., add "sim_fundAccount" to your GenLayerMethod union with a specific return type).
23-27: Remove unreachable address fallback in getCurrentNonceThe wrapper for
getCurrentNoncecurrently falls back toclient.account?.address, but its TypeScript signature requires anaddressargument and all call sites (e.g. insrc/contracts/actions.ts:254) always pass one. The fallback and associated error check are never used and can be removed to keep the API consistent.• File:
src/accounts/actions.ts(lines 23–27)
• Call sites verified: onlyclient.getCurrentNonce({ address: … })is ever invoked, soaddressis never falsy.Minimal diff to simplify:
- const addressToUse = address || client.account?.address; - - if (!addressToUse) { - throw new Error("No address provided and no account is connected"); - } + const addressToUse = address;src/client/client.ts (5)
40-41: Fix account type detection for provider routing.The current check treats “no account provided” the same as “address string,” which is brittle.
- const isAddress = typeof config.account !== "object"; + const isAddress = typeof config.account === "string";
61-85: Harden fetch branch with HTTP error handling.response.json() will throw on non-JSON or mask HTTP errors. Check response.ok first and include status info in errors.
const response = await fetch(config.chain.rpcUrls.default.http[0], { @@ - const data = await response.json(); + if (!response.ok) { + const body = await response.text().catch(() => ""); + throw new Error(`GenLayer RPC ${method} failed: ${response.status} ${response.statusText} ${body}`); + } + const data = await response.json();
118-121: Background init may race with actions that depend on consensusDataContract.If a consumer calls a contract/tx action immediately after createClient, consensus contracts may be unset. Consider:
- Awaiting initialization when not testnet, or
- Lazily initializing inside the first call that needs it, or
- Exposing an option { initialize: "eager" | "lazy" } with default eager.
1-12: Prefer viem’sEIP1193Providerover the customEthereumProviderWhile
EthereumProvideris declared insrc/global.d.ts, relying on a bespoke global interface can introduce subtle mismatches with viem’s APIs. To align with viem’s types and avoid duplication, update your client config as follows:• In
src/client/client.ts, import the standard provider type:-import { +import { Account, createClient as createViemClient, createPublicClient as createPublicViemClient, publicActions, custom, Address, walletActions, Transport, PublicClient, Chain, + type EIP1193Provider, } from "viem";• And update the
ClientConfiginterface:interface ClientConfig { endpoint?: string; // Custom RPC endpoint account?: Account | Address; - provider?: EthereumProvider; // Custom provider for wallet framework integration + provider?: EIP1193Provider; // Standard EIP-1193 provider }• (Optional) Once you’ve migrated all usages, consider deprecating or removing the
EthereumProviderdefinition insrc/global.d.tsto keep your type definitions DRY.
21-23: Useclient.extendinstead ofmergeActionsto preserve hidden viem propertiesThe current pipeline uses
mergeActions, which internally callsObject.assign({}, base, ext). That drops non-enumerable and symbol properties (including any future prototype hooks or hidden markers viem may add). To guard against accidentally stripping those, prefer chaining.extend(...)calls, which preserves the client’s prototype and any non-enumerable/symbol fields.Locations to update:
src/client/client.ts:108–114
// current const baseWithViem = baseClient.extend(publicActions).extend(walletActions) as BaseActionsClient<GenLayerChain>; const withAccounts = mergeActions(baseWithViem, accountActions(baseWithViem)); const withWallet = mergeActions(withAccounts, genlayerWalletActions(withAccounts)); const withChain = mergeActions(withWallet, chainActions(withWallet)); const withContracts = mergeActions(withChain, contractActions(withChain, publicClient)); const withTx = mergeActions(withContracts, transactionActions(withContracts, publicClient)); const finalClient = mergeActions(withTx, receiptActions(withTx, publicClient));Proposed refactor:
- const baseWithViem = baseClient.extend(publicActions).extend(walletActions) as BaseActionsClient<GenLayerChain>; - const withAccounts = mergeActions(baseWithViem, accountActions(baseWithViem)); - const withWallet = mergeActions(withAccounts, genlayerWalletActions(withAccounts)); - const withChain = mergeActions(withWallet, chainActions(withWallet)); - const withContracts = mergeActions(withChain, contractActions(withChain, publicClient)); - const withTx = mergeActions(withContracts, transactionActions(withContracts, publicClient)); - const finalClient = mergeActions(withTx, receiptActions(withTx, publicClient)); + const finalClient = baseClient + .extend(publicActions) + .extend(walletActions) + .extend(accountActions) + .extend(genlayerWalletActions) + .extend(chainActions) + .extend((chain) => contractActions(chain, publicClient)) + .extend((chain) => transactionActions(chain, publicClient)) + .extend((chain) => receiptActions(chain, publicClient)) as GenLayerClient<GenLayerChain>;This ensures all non-enumerable/symbol properties and the client’s prototype remain intact while composing your custom actions.
src/transactions/actions.ts (2)
48-50: Avoid double-decoding localnet transactions.transactionActions already returns a decoded localnet tx. Decoding again here is redundant.
- if (client.chain.id === localnet.id) { - finalTransaction = decodeLocalnetTransaction(transaction as unknown as GenLayerTransaction); - }
96-106: Remote branch depends on consensusDataContract; ensure it’s initialized.If consensusDataContract is null, readContract will fail at runtime. Either guard and surface a better error or lazily initialize before the call.
Would you like me to add a small helper ensureConsensusContracts(client, publicClient) that initializes contracts when missing and call it here?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/accounts/actions.ts(1 hunks)src/chains/actions.ts(1 hunks)src/client/client.ts(3 hunks)src/contracts/actions.ts(4 hunks)src/transactions/actions.ts(3 hunks)src/types/clients.ts(1 hunks)src/wallet/actions.ts(1 hunks)src/wallet/connect.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/wallet/actions.ts (2)
src/types/chains.ts (1)
GenLayerChain(4-17)src/types/clients.ts (1)
BaseActionsClient(89-100)
src/wallet/connect.ts (1)
src/types/chains.ts (1)
GenLayerChain(4-17)
src/client/client.ts (7)
src/chains/localnet.ts (1)
localnet(3990-4014)src/wallet/actions.ts (1)
walletActions(5-10)src/types/clients.ts (2)
BaseActionsClient(89-100)GenLayerClient(28-86)src/accounts/actions.ts (1)
accountActions(4-34)src/chains/actions.ts (1)
chainActions(4-40)src/contracts/actions.ts (1)
contractActions(24-183)src/transactions/actions.ts (2)
transactionActions(76-107)receiptActions(16-70)
src/accounts/actions.ts (2)
src/types/chains.ts (1)
GenLayerChain(4-17)src/types/clients.ts (1)
BaseActionsClient(89-100)
src/chains/actions.ts (2)
src/types/chains.ts (1)
GenLayerChain(4-17)src/types/clients.ts (1)
BaseActionsClient(89-100)
src/contracts/actions.ts (2)
src/types/chains.ts (1)
GenLayerChain(4-17)src/types/clients.ts (1)
GenLayerClient(28-86)
src/transactions/actions.ts (5)
src/types/chains.ts (1)
GenLayerChain(4-17)src/types/clients.ts (1)
BaseActionsClient(89-100)src/types/transactions.ts (3)
TransactionHash(5-5)GenLayerTransaction(142-256)transactionsStatusNameToNumber(57-72)src/chains/localnet.ts (1)
localnet(3990-4014)src/transactions/decoders.ts (1)
decodeLocalnetTransaction(232-276)
src/types/clients.ts (1)
src/types/chains.ts (1)
GenLayerChain(4-17)
🔇 Additional comments (10)
src/types/clients.ts (1)
88-100: Good abstraction: BaseActionsClient aligns with viem’s extension modelThe capability surface and removal of protected actions look solid.
src/contracts/actions.ts (1)
15-22: ContractCapabilities type is a clean, minimal surfaceNice decoupling from the full client; narrows only what contract actions need.
src/chains/actions.ts (2)
4-4: LGTM on generic client adoptionUsing
BaseActionsClient<TChain>cleanly decouples this action factory from the full GenLayer client.
19-19: Validate RPC URL presence
client.chain.rpcUrls.default.http[0]may be undefined for misconfigured chains. Add a guard to fail early with a helpful message.Example (outside current hunk):
const url = client.chain.rpcUrls?.default?.http?.[0]; if (!url) throw new Error(`No RPC URL configured for chain ${client.chain.id}`); const contractsResponse = await fetch(url, { ... });src/accounts/actions.ts (2)
1-1: Good move: adopt BaseActionsClient and generic chain.This aligns the action factory with the new capability-based client and viem-style extension.
4-4: Generic factory signature is consistent with multi-chain design.No runtime behavior change; types become more precise.
src/client/client.ts (2)
97-100: LGTM: public client creation and scoping.Keeping a separate publicClient for remote reads is clear and avoids signature conflicts.
107-116: Merge ordering looks correct to override protected viem actions with GenLayer variants.transactionActions and receiptActions applied last will supersede viem’s getTransaction/waitForTransactionReceipt.
src/transactions/actions.ts (2)
12-14: Type alias for ClientWithGetTransaction is clear and scoped.This keeps receiptActions narrowly typed yet composable.
16-19: Receipt actions generic signature looks good.Accepting ClientWithGetTransaction avoids over-constraining on the full client.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/transactions/actions.ts (1)
85-95: Guard against null eth_getTransactionByHash responses and normalize unknown statuses.Localnet can return null for unknown hashes. The current code will throw at (transaction.status as string) when null. Also, normalize unexpected status strings (e.g., "ACTIVATED") defensively.
Apply this diff:
const transaction = (await client.request({ method: "eth_getTransactionByHash", params: [hash], - })) as GenLayerTransaction; - const localnetStatus = - (transaction.status as string) === "ACTIVATED" ? TransactionStatus.PENDING : transaction.status; - - transaction.status = Number(transactionsStatusNameToNumber[localnetStatus as TransactionStatus]); - transaction.statusName = localnetStatus as TransactionStatus; - return decodeLocalnetTransaction(transaction); + })) as GenLayerTransaction | null; + if (!transaction) { + throw new Error(`Transaction not found for hash ${hash}`); + } + const rawStatus = transaction.status as string | number | undefined; + const normalizedStatusName: TransactionStatus = + rawStatus === "ACTIVATED" + ? TransactionStatus.PENDING + : typeof rawStatus === "string" && rawStatus in transactionsStatusNameToNumber + ? (rawStatus as TransactionStatus) + : TransactionStatus.UNDETERMINED; + transaction.status = Number(transactionsStatusNameToNumber[normalizedStatusName]); + transaction.statusName = normalizedStatusName; + return decodeLocalnetTransaction(transaction);
🧹 Nitpick comments (6)
src/transactions/actions.ts (6)
48-50: Remove unnecessary double cast.transaction is already GenLayerTransaction-typed. The extra as unknown as GenLayerTransaction is noise.
- finalTransaction = decodeLocalnetTransaction(transaction as unknown as GenLayerTransaction); + finalTransaction = decodeLocalnetTransaction(transaction);
33-39: Drop redundant null-check here; centralize “not found” handling in getTransaction.Given getTransaction now throws when the RPC returns null (localnet path), this check is unreachable. Prefer a single, consistent error source.
- if (!transaction) { - throw new Error("Transaction not found"); - }
57-59: Enrich the timeout error with context.Include the hash and last-known status to aid debugging.
- throw new Error("Transaction status is not " + status); + throw new Error( + `Transaction ${hash} did not reach status ${status}. ` + + `Last known: ${transaction.statusName ?? transaction.status}.` + );
16-19: publicClient is unused in receiptActions; mark intentionally unused to satisfy linters.Renaming avoids “unused parameter” warnings while preserving the signature.
export const receiptActions = <TChain extends GenLayerChain>( client: ClientWithGetTransaction<TChain>, - publicClient: PublicClient, + _publicClient: PublicClient, ) => ({ ... - return receiptActions(client, publicClient).waitForTransactionReceipt({ + return receiptActions(client, _publicClient).waitForTransactionReceipt({Also applies to: 62-62
20-69: Optional: switch recursive polling to iterative loop to avoid re-instantiation.Current recursion is safe (promise chaining), but an iterative loop is a bit clearer and avoids re-creating the actions object.
Example refactor:
waitForTransactionReceipt: async ({ hash, status = TransactionStatus.ACCEPTED, interval = transactionsConfig.waitInterval, retries = transactionsConfig.retries, fullTransaction = false, }: { @@ - if (retries === 0) { - throw new Error("Transaction status is not " + status); - } - - await sleep(interval); - return receiptActions(client, _publicClient).waitForTransactionReceipt({ - hash, - status, - interval, - retries: retries - 1, - fullTransaction, - }); + let remaining = retries; + let last: GenLayerTransaction = transaction; + while (remaining > 0) { + await sleep(interval); + last = await client.getTransaction({hash}); + const s = String(last.status); + const finalized = transactionsStatusNameToNumber[TransactionStatus.FINALIZED]; + const requested = transactionsStatusNameToNumber[status]; + if (s === requested || (status === TransactionStatus.ACCEPTED && s === finalized)) { + const tx = client.chain.id === localnet.id ? decodeLocalnetTransaction(last) : last; + return fullTransaction ? tx : simplifyTransactionReceipt(tx); + } + remaining--; + } + throw new Error(`Transaction ${hash} did not reach status ${status}. Last known: ${last.statusName ?? last.status}.`);
80-107: Unit tests to add (happy/unhappy paths).To lock in behavior and avoid regressions:
- Localnet: getTransaction returns null → expect Error("Transaction not found for hash ...").
- Localnet: status "ACTIVATED" → normalize to PENDING; waitForTransactionReceipt should resolve immediately when requesting ACCEPTED and the tx is already FINALIZED.
- Remote: readContract getTransactionData returns a valid GenLayerRawTransaction → decodeTransaction produces expected GenLayerTransaction.
- waitForTransactionReceipt: retries exhausted → enriched error message includes hash and last-known status.
If you want, I can scaffold these tests with viem-mocked clients and a fake transport.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/chains/actions.ts(1 hunks)src/transactions/actions.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/chains/actions.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/transactions/actions.ts (4)
src/types/clients.ts (1)
BaseActionsClient(89-100)src/types/transactions.ts (3)
TransactionHash(5-5)GenLayerTransaction(142-256)transactionsStatusNameToNumber(57-72)src/chains/localnet.ts (1)
localnet(3990-4014)src/transactions/decoders.ts (1)
decodeLocalnetTransaction(232-276)
🔇 Additional comments (4)
src/transactions/actions.ts (4)
12-14: Type alias for ClientWithGetTransaction looks good.Captures capability-based typing cleanly and keeps receiptActions narrow. No changes needed.
72-79: TransactionCapabilities scaffolding is fine.The capability boundary is clear: localnet uses client.request; remote uses viem publicClient. No changes needed.
1-2: Imports look consistent with the new capability-based design.Centralized types from "@/types" and BaseActionsClient from "../types/clients" are the right dependencies.
96-104: All chain definitions include consensusDataContract; no guard neededI’ve verified that every
defineChaincall insrc/chains(localnet.ts, studionet.ts, testnetAsimov.ts) declares aconsensusDataContract, so there is no runtime risk ofreadContractbeing called with an undefined address or ABI.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/wallet/connect.ts (1)
33-39: Bug:blockExplorerUrlscan become[undefined]and breakwallet_addEthereumChain.When
selectedNetwork.blockExplorers?.defaultis absent (likely on localnet), this produces[undefined], which MetaMask rejects. Include the key only when a URL exists.Apply this diff:
- const chainParams = { - chainId: chainIdHex, - chainName: selectedNetwork.name, - rpcUrls: selectedNetwork.rpcUrls.default.http, - nativeCurrency: selectedNetwork.nativeCurrency, - blockExplorerUrls: [selectedNetwork.blockExplorers?.default.url], - }; + const chainParams = { + chainId: chainIdHex, + chainName: selectedNetwork.name, + rpcUrls: selectedNetwork.rpcUrls.default.http, + nativeCurrency: selectedNetwork.nativeCurrency, + ...(selectedNetwork.blockExplorers?.default?.url + ? { blockExplorerUrls: [selectedNetwork.blockExplorers.default.url] } + : {}), + };
♻️ Duplicate comments (1)
src/wallet/connect.ts (1)
16-19: Thanks for relaxing theclientparam to accept an optionalchain.This aligns with
BaseActionsClientwherechainis optional and unblocks callers passing a viem-style client directly. Good change.
🧹 Nitpick comments (4)
src/wallet/connect.ts (4)
20-22: SSR/Non-browser safety: guardwindowusage.Directly referencing
windowthrows in SSR/Node contexts. Add a typeof guard.- if (!window.ethereum) { + if (typeof window === "undefined" || !window.ethereum) { throw new Error("MetaMask is not installed."); }
41-51: More robust chain switching flow: switch first, add on 4902.Current flow always calls
wallet_addEthereumChainwhich can cause redundant prompts. Prefer switching first and add only if the chain is unknown (error code 4902).- const currentChainId = await window.ethereum.request({method: "eth_chainId"}); - if (currentChainId !== chainIdHex) { - await window.ethereum.request({ - method: "wallet_addEthereumChain", - params: [chainParams], - }); - await window.ethereum.request({ - method: "wallet_switchEthereumChain", - params: [{chainId: chainIdHex}], - }); - } + const currentChainId = await window.ethereum.request({ method: "eth_chainId" }); + if (currentChainId !== chainIdHex) { + try { + await window.ethereum.request({ + method: "wallet_switchEthereumChain", + params: [{ chainId: chainIdHex }], + }); + } catch (err: any) { + if (err?.code === 4902) { + await window.ethereum.request({ + method: "wallet_addEthereumChain", + params: [chainParams], + }); + await window.ethereum.request({ + method: "wallet_switchEthereumChain", + params: [{ chainId: chainIdHex }], + }); + } else { + throw err; + } + } + }
54-56: Minor: simpler and better-typed snap detection.Use the record key instead of scanning values, and give
installedSnapsa minimal shape.- const installedSnaps: any = await window.ethereum.request({method: "wallet_getSnaps"}); - const isGenLayerSnapInstalled = Object.values(installedSnaps).some((snap: any) => snap.id === id); + const installedSnaps: Record<string, { id: string }> = + await window.ethereum.request({ method: "wallet_getSnaps" }); + const isGenLayerSnapInstalled = Boolean(installedSnaps?.[id]);
24-25: Nit: clarify the mainnet error message.Message suggests “use localnet” even though studionet/testnetAsimov are supported. Make it explicit.
- throw new Error(`${network} is not available yet. Please use localnet.`); + throw new Error(`${network} is not available yet. Please use one of: localnet, studionet, or testnetAsimov.`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/wallet/connect.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/wallet/connect.ts (1)
src/types/chains.ts (1)
GenLayerChain(4-17)
🔇 Additional comments (1)
src/wallet/connect.ts (1)
4-4: No changes needed forGenLayerChainimport
Thesrc/types/index.tsbarrel file includes the lineexport * from "./chains";, which re-exportsGenLayerChainfromsrc/types/chains.ts. Therefore, the importimport { GenLayerChain } from "@/types";is valid and does not need to be adjusted.
What
unknowncasts with precise types via newBaseActionsClientand capability-based typings.createClientto use viem-style.extend(...)only for non-conflicting actions and typed object merges for conflicting ones.accounts,chains,wallet) to acceptBaseActionsClient.ContractCapabilitiesto typecontracts/actions.tshelpers (_encodeAddTransactionData,_sendTransaction, etc.).receiptActionstyped with a client that includes our owngetTransaction.eth_getTransactionByHashdirectly and adapt toGenLayerTransactionbeforedecodeLocalnetTransaction(with a short comment explaining viem signature/return-type conflict).wallet/connectclient param to{ chain: GenLayerChain }.GenLayerRawTransactionfrom@/typesfor testnet path.Why
unknown/anyand align with viem’s extension model while preserving our API surface.getTransaction,readContract,waitForTransactionReceipt).waitForTransactionReceipton localnet caused by decoding testnet-shaped data.Testing done
waitForTransactionReceipton localnet.getTransactionon localnet and studionet networks.getContractSchemaForCode,deployContract, andwriteContractflows.Decisions made
.extendwhen our signatures differ; merge custom actions instead.getTransaction, do not use viem’sgetTransactiondue to signature and return-type differences; use direct RPC and normalize.wallet/connectinput type to avoid over-constraining callers.Checks
Reviewing tips
src/types/clients.tsfor the newBaseActionsClient.src/client/client.tsto see the mixed extend/merge composition.accounts,chains,wallet,contracts,transactions) focusing on their input types and removal of casts.src/transactions/actions.tsexplaining why viem’sgetTransactionisn’t used on localnet.User facing release notes
unknowncasts.Summary by CodeRabbit
New Features
Refactor
Chores