-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: viem #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughUpdated dependencies and wallet packages; added Viem-based contract modules and multicall balance/allowance fetching; introduced Phantom support and connectPhantom; propagated an onBalanceFetched callback; removed legacy Web3/web3composer/estimator modules; multiple UI, config, and style tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bridge as Bridge UI
participant PeggyProv as PeggyProvider
participant Erc20 as Erc20WethContract
participant Peggy as PeggyContract
participant Broadcaster as Web3Broadcaster
participant AccountProv as AccountProvider
participant Viem as Viem (Multicall)
participant EventProv as EventProvider
User->>Bridge: Submit deposit (token, amount)
Bridge->>PeggyProv: requestDeposit(token, amount, ethDestination)
alt Allowance insufficient
PeggyProv->>Erc20: setTokenAllowance(...) -- build & estimate tx
Erc20-->>Broadcaster: approval tx (sent)
Broadcaster-->>PeggyProv: approval confirmed
end
PeggyProv->>Peggy: sendToInjective(...) -- build & estimate tx
Peggy-->>Broadcaster: bridge tx (sent)
Broadcaster-->>PeggyProv: tx hash
PeggyProv->>Bridge: return tx hash
Note over Bridge,AccountProv: after tx or on-demand balance refresh
Bridge->>AccountProv: fetch balances & allowances
AccountProv->>Viem: multicall(balanceOf+allowance for tokens)
Viem-->>AccountProv: multicall results
AccountProv->>EventProv: onBalanceFetched(results)
EventProv-->>Bridge: callback triggers UI refresh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/bridge/BridgeForm.tsx (2)
103-105: Use dynamic token symbol in validation message.Hardcoding “USDT” is wrong on testnet.
- return `You can only transfer up to ${availableBalance} USDT`; + return `You can only transfer up to ${availableBalance} ${token.symbol}`;
107-109: Disallow zero amounts per message.Check should be lte(0) if zero is invalid.
- if (parsedValue.lt(0)) { + if (parsedValue.lte(0)) { return "Amount must be greater than 0"; }src/main.tsx (1)
21-22: Fix createWidget return type to expose unmount handle.Function returns an unmount callback but the type is void, hiding it from consumers.
-type CreateWidgetFn = (selector: string, props: WidgetProps) => void; +type CreateWidgetFn = (selector: string, props: WidgetProps) => () => void; @@ -const createWidget: CreateWidgetFn = ( +const createWidget: CreateWidgetFn = ( selector: string, { onInit, onSuccess, onError, onBalanceFetched, wallet, mock }: WidgetProps ) => { @@ - return root.unmount.bind(root); + return root.unmount.bind(root);Also applies to: 29-31, 49-50
🧹 Nitpick comments (19)
src/components/common/CurrencyInput.tsx (1)
34-83: Styling updates improve visual hierarchy.The presentational changes enhance the UI with better spacing, font weights, and color consistency. The changes include improved label/input padding, font sizing, and token display styling.
src/components/bridge/Selector.tsx (1)
1-28: Consider making this component reusable.The component is currently hardcoded to display USDT on Ethereum. If token selection will be dynamic in the future, consider accepting token data as props to improve reusability.
src/context/eventContext.ts (1)
3-9: Unify event callback signatures and return types (return void, consistent params).Current types differ from Providers’ props and return
unknown. Recommend a single alias and no-arg onBalanceFetched.+type EventCallback = (...args: unknown[]) => void; interface EventContextType { - onInit: (...args: unknown[]) => unknown; - onError: (...args: unknown[]) => unknown; - onSuccess: (...args: unknown[]) => unknown; - onBalanceFetched: (...args: unknown[]) => unknown; + onInit: EventCallback; + onError: EventCallback; + onSuccess: EventCallback; + // currently used without payload; widen later if needed + onBalanceFetched: () => void; mock: boolean; } const EventContext = createContext<EventState>({ onInit: () => {}, onError: () => {}, - onSuccess: () => {}, - onBalanceFetched: () => {}, + onSuccess: () => {}, + onBalanceFetched: () => {}, mock: false, });Also applies to: 13-19
src/context/WalletProvider.tsx (1)
54-67: DRY the connect flows (single helper to set state from addresses).Both connect methods duplicate steps. Extract a helper and reuse.
+ async function connectAndSetState(target: Wallet) { + await connectWallet(target); + const addresses = await getAddresses(); + const [addr] = addresses; + setWallet(target); + setAddress(addr); + setAddresses(addresses); + setInjectiveAddress(getInjectiveAddress(addr)); + setAddressConfirmation(await walletStrategy.getSessionOrConfirm(addr)); + // on connect + } async function connectMetamask() { - await connectWallet(Wallet.Metamask); - - const addresses = await getAddresses(); - const [address] = addresses; - - setWallet(Wallet.Metamask); - setAddress(address); - setAddresses(addresses); - setInjectiveAddress(getInjectiveAddress(address)); - setAddressConfirmation(await walletStrategy.getSessionOrConfirm(address)); - - // on connect + await connectAndSetState(Wallet.Metamask); } async function connectPhantom() { - await connectWallet(Wallet.Phantom); - - const addresses = await getAddresses(); - const [address] = addresses; - - setWallet(Wallet.Phantom); - setAddress(address); - setAddresses(addresses); - setInjectiveAddress(getInjectiveAddress(address)); - setAddressConfirmation(await walletStrategy.getSessionOrConfirm(address)); - - // on connect + await connectAndSetState(Wallet.Phantom); }Also applies to: 69-82
src/components/assets/wallet/Phantom.tsx (1)
1-21: LGTM; consider minor a11y tweak if decorative.If decorative, mark as hidden from AT.
- <svg + <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 128 128" fill="none" + aria-hidden="true" + focusable="false" >src/Providers.tsx (1)
18-19: Make onBalanceFetched optional with a default no-op and align type.Improves backwards compatibility and matches current no-arg usage.
- onBalanceFetched: (args: unknown) => void; + onBalanceFetched?: () => void; @@ - onBalanceFetched, + onBalanceFetched = () => {}, mock, }: ProvidersProps) => { @@ - onBalanceFetched={onBalanceFetched} + onBalanceFetched={onBalanceFetched}Additionally, consider updating onInit/onSuccess/onError to use a shared
EventCallback = (...args: unknown[]) => voidfor consistency with EventContext.Also applies to: 28-30, 36-37
src/app/wallet/metamask.ts (1)
77-84: Harden network switch: guard provider and handle unknown chain (4902).Add a provider null check and optionally fall back to wallet_addEthereumChain when chain isn’t added.
-async function updateMetamaskNetwork() { - return walletStrategy.getEip1193Provider().then((provider) => { - return provider.request({ - method: "wallet_switchEthereumChain", - params: [{ chainId: `0x${ETHEREUM_CHAIN_ID.toString(16)}` }], - }); - }); -} +async function updateMetamaskNetwork() { + const provider = await walletStrategy.getEip1193Provider(); + if (!provider) { + throw new GeneralException(new Error("Metamask provider not found"), { + code: UnspecifiedErrorCode, + type: ErrorType.WalletError, + }); + } + try { + return await provider.request({ + method: "wallet_switchEthereumChain", + params: [{ chainId: `0x${ETHEREUM_CHAIN_ID.toString(16)}` }], + }); + } catch (err: any) { + // Optional: handle unrecognized chain by adding it (requires chain params) + if (err?.code === 4902) { + // await provider.request({ method: "wallet_addEthereumChain", params: [ { chainId: `0x${...}`, rpcUrls: [/* rpc */], chainName: /* name */, nativeCurrency: { name: "ETH", symbol: "ETH", decimals: 18 } } ] }); + } + throw err; + } +}src/app/data/web3.ts (1)
4-21: Type the address map for safety and completeness.Add a compile-time contract that all Network keys map to hex addresses and catch omissions early.
export const injectivePeggyAddressMap = { [Network.Mainnet]: "0xF955C57f9EA9Dc8781965FEaE0b6A2acE2BAD6f3", ... [Network.Local]: "0x3c92F7779A7845d5eEf307aEF39066Ddba04A54b", -}; +} as const satisfies Record<Network, `0x${string}`>;src/components/bridge/BridgeForm.tsx (3)
39-41: Handle clipboard promise and failures.Await and catch to avoid silent failures (e.g., non-HTTPS, denied permission).
-const handleCopy = () => { - navigator.clipboard.writeText(address); -}; +const handleCopy = async () => { + try { + await navigator.clipboard.writeText(address); + // optional: toast success + } catch { + // optional: fallback or toast error + console.error("Copy failed"); + } +};
70-74: Avoid duplicate immediate balance fetches.Consolidate into a single delayed refresh (or rely on the new onBalanceFetched flow) to reduce RPC load.
- fetchBalanceAndAllowance(); - - setTimeout(() => { - fetchBalanceAndAllowance(); - }, 3000); + setTimeout(() => fetchBalanceAndAllowance(), 2500);Confirm if EventProvider already triggers onBalanceFetched after fetch; if yes, consider removing this fetch entirely to prevent double work.
129-134: Make copy control accessible.Use a button with aria-label and prevent form submit.
-<span - onClick={handleCopy} - className="cursor-pointer hover:text-[#4390FA] *:size-4" -> - <Copy /> -</span> +<button + type="button" + aria-label="Copy address" + onClick={handleCopy} + className="cursor-pointer hover:text-[#4390FA] *:size-4" +> + <Copy /> +</button>Selector currently renders static “USDT on Ethereum”. If testnet selects injErc20Token, consider passing token to Selector to keep UI consistent.
src/context/PeggyProvider.tsx (1)
19-39: Optional: avoid discarding tx hashes from approvals.You return txHash from setAllowance but ignore it at call sites; either log/emit for observability, or remove the return to reduce confusion.
Do you want these hashes surfaced via onBalanceFetched/onSuccess for telemetry?
Also applies to: 82-85
src/app/contracts/PeggyContract.ts (1)
70-79: Consider includingchainIdand explicit EIP‑1559 type for wallet interoperability.Some wallets/signers behave better with
chainIdandtype: "0x2"present; harmless to include.const tx = { from: fromAddress as Address, to: this.peggyAddress as Address, data: calldata, value: toHex(0n), gas: toHex(gas), maxFeePerGas: toHex(fees.maxFeePerGas), maxPriorityFeePerGas: toHex(fees.maxPriorityFeePerGas), nonce: toHex(nonce), + chainId: this.chain.id, + type: "0x2" as const, };src/app/contracts/fetchErc20BalanceAndAllowance.ts (1)
17-20: Unify chain selection via shared helper; also validate RPC URL.Prefer
getInjNetworkToChain(NETWORK)and guard empty RPC as elsewhere for consistency.- const client = createPublicClient({ - chain: IS_MAINNET ? mainnet : sepolia, - transport: http(getAlchemyUrl(NETWORK)), - }); + const chain = IS_MAINNET ? mainnet : sepolia; // or getInjNetworkToChain(NETWORK) + const rpcUrl = getAlchemyUrl(NETWORK); + if (!rpcUrl) throw new Error(`Alchemy URL not configured for ${NETWORK}`); + const client = createPublicClient({ chain, transport: http(rpcUrl) });src/app/contracts/utils.ts (2)
15-19: Alchemy URL map supports only Mainnet/Testnet.Extend map or document fallbacks. Otherwise
getAlchemyUrlmay return"".export const alchemyUrlNetworkMap: Partial<Record<Network, string>> = { [Network.Mainnet]: "https://eth-mainnet.g.alchemy.com/v2/" + ALCHEMY_KEY, [Network.Testnet]: "https://eth-sepolia.g.alchemy.com/v2/" + ALCHEMY_SEPOLIA_KEY, + // Add other networks or use public RPCs as fallback if needed. };
27-51: Add gas price fallback for non‑EIP‑1559 RPCs.
estimateFeesPerGasmay not return EIP‑1559 fields on all RPCs. Provide agetGasPrice()fallback.const [gas, fees, nonce] = await Promise.all([ publicClient.estimateGas({ account: from, to, data: calldata, value, }), - publicClient.estimateFeesPerGas(), + publicClient.estimateFeesPerGas().catch(async () => { + const gp = await publicClient.getGasPrice(); + return { + maxPriorityFeePerGas: gp, + maxFeePerGas: gp, + }; + }), publicClient.getTransactionCount({ address: from }), ]);src/app/contracts/Erc20WethContract.ts (3)
53-55: Redundant nullish coalescing.
getInjNetworkToChainalready returns aChain;?? mainnetis unnecessary.- this.chain = getInjNetworkToChain(params.network) ?? mainnet; + this.chain = getInjNetworkToChain(params.network);
126-134: IncludechainIdandtypefor signer compatibility (optional).const tx = { data: calldata, gas: toHex(gas), from: fromAddress, nonce: toHex(nonce), to: tokenAddress, maxFeePerGas: toHex(fees.maxFeePerGas), maxPriorityFeePerGas: toHex(fees.maxPriorityFeePerGas), + chainId: this.chain.id, + type: "0x2" as const, };
156-165: Same optional tx shape improvement for deposit.Add
chainIdandtype: "0x2"as above for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
index.html(3 hunks)package.json(2 hunks)src/App.tsx(1 hunks)src/Providers.tsx(3 hunks)src/app/constants/index.ts(1 hunks)src/app/constants/setup.ts(2 hunks)src/app/contracts/Erc20WethContract.ts(1 hunks)src/app/contracts/PeggyContract.ts(1 hunks)src/app/contracts/fetchErc20BalanceAndAllowance.ts(1 hunks)src/app/contracts/utils.ts(1 hunks)src/app/data/web3.ts(2 hunks)src/app/wallet/metamask.ts(3 hunks)src/app/wallet/walletService.ts(2 hunks)src/app/wallet/walletStrategy.ts(2 hunks)src/components/assets/ChevronDown.tsx(1 hunks)src/components/assets/Copy.tsx(1 hunks)src/components/assets/wallet/Phantom.tsx(1 hunks)src/components/bridge/Bridge.tsx(2 hunks)src/components/bridge/BridgeForm.tsx(5 hunks)src/components/bridge/Selector.tsx(1 hunks)src/components/common/CurrencyInput.tsx(2 hunks)src/components/ui/Button.tsx(1 hunks)src/components/wallet/ConnectWallet.tsx(1 hunks)src/components/wallet/WalletItem.tsx(1 hunks)src/context/AccountProvider.tsx(1 hunks)src/context/EventProvider.tsx(1 hunks)src/context/PeggyProvider.tsx(2 hunks)src/context/WalletProvider.tsx(3 hunks)src/context/eventContext.ts(1 hunks)src/context/walletContext.ts(2 hunks)src/demo.tsx(2 hunks)src/main.tsx(4 hunks)src/types/index.ts(1 hunks)tailwind.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
src/components/bridge/Bridge.tsx (1)
src/context/eventContext.ts (1)
useEvent(21-27)
src/app/contracts/utils.ts (1)
src/app/constants/setup.ts (2)
ALCHEMY_KEY(29-29)ALCHEMY_SEPOLIA_KEY(36-37)
src/Providers.tsx (1)
src/context/EventProvider.tsx (1)
EventProvider(13-34)
src/components/bridge/Selector.tsx (2)
src/app/data/tokens.ts (2)
usdtToken(63-78)wethToken(98-117)src/components/assets/ChevronDown.tsx (1)
ChevronDown(1-18)
src/components/wallet/ConnectWallet.tsx (3)
src/context/walletContext.ts (1)
useWallet(25-31)src/components/wallet/WalletItem.tsx (1)
WalletItem(7-17)src/components/assets/wallet/Phantom.tsx (1)
Phantom(1-19)
src/app/wallet/walletService.ts (1)
src/app/constants/setup.ts (1)
ETHEREUM_CHAIN_ID(13-15)
src/context/AccountProvider.tsx (4)
src/context/walletContext.ts (1)
useWallet(25-31)src/app/data/tokens.ts (2)
usdtToken(63-78)injErc20Token(80-96)src/app/contracts/fetchErc20BalanceAndAllowance.ts (1)
getErc20BalancesAndAllowances(8-67)src/app/data/web3.ts (1)
injectivePeggyBridgeAddress(23-23)
src/context/PeggyProvider.tsx (6)
src/context/accountContext.ts (1)
useAccount(15-21)src/context/walletContext.ts (1)
useWallet(25-31)src/app/contracts/Erc20WethContract.ts (1)
erc20WethContract(171-173)src/app/data/web3.ts (1)
injectivePeggyBridgeAddress(23-23)src/app/wallet/walletService.ts (1)
web3Broadcaster(21-25)src/app/contracts/PeggyContract.ts (1)
peggyContract(85-87)
src/app/wallet/metamask.ts (2)
src/app/wallet/walletStrategy.ts (1)
walletStrategy(22-30)src/app/constants/setup.ts (1)
ETHEREUM_CHAIN_ID(13-15)
src/components/bridge/BridgeForm.tsx (4)
src/context/walletContext.ts (1)
useWallet(25-31)src/context/accountContext.ts (1)
useAccount(15-21)src/components/bridge/Selector.tsx (1)
Selector(4-28)src/components/assets/Copy.tsx (1)
Copy(1-18)
src/app/contracts/fetchErc20BalanceAndAllowance.ts (3)
src/app/constants/setup.ts (2)
IS_MAINNET(18-18)NETWORK(10-11)src/app/contracts/utils.ts (1)
getAlchemyUrl(21-25)src/types/index.ts (1)
ERC20BalanceAndAllowance(54-60)
src/app/contracts/Erc20WethContract.ts (3)
src/app/contracts/utils.ts (3)
getInjNetworkToChain(7-13)getAlchemyUrl(21-25)estimateGasAndNonce(27-51)src/app/data/tokens.ts (1)
wethToken(98-117)src/app/constants/setup.ts (1)
NETWORK(10-11)
src/app/data/web3.ts (1)
src/app/constants/setup.ts (1)
NETWORK(10-11)
src/context/WalletProvider.tsx (1)
src/app/wallet/walletStrategy.ts (2)
getAddresses(36-60)walletStrategy(22-30)
src/app/wallet/walletStrategy.ts (1)
src/app/constants/setup.ts (3)
CHAIN_ID(16-16)ETHEREUM_CHAIN_ID(13-15)alchemyRpcEndpoint(59-62)
src/app/contracts/PeggyContract.ts (3)
src/app/contracts/utils.ts (3)
getInjNetworkToChain(7-13)getAlchemyUrl(21-25)estimateGasAndNonce(27-51)src/app/data/web3.ts (1)
injectivePeggyBridgeAddress(23-23)src/app/constants/setup.ts (1)
NETWORK(10-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (23)
package.json (2)
3-3: Version bump aligns with significant refactoring scope.Bumping from 0.0.8 to 0.0.12 is appropriate for this PR's scope (dependency upgrades, wallet ecosystem migration, new contract utilities).
19-26: Dependency migration executed cleanly.The transition from monolithic
wallet-tsto modular packages (wallet-core,wallet-base,wallet-strategy,wallet-evm) is well-coordinated. All @InjectiveLabs packages are pinned to exact version 1.16.20, which ensures internal ecosystem coherence. This fixed-version approach is appropriate for tightly integrated dependencies while allowing flexibility in other packages (react, react-dom, etc., which use caret ranges).tailwind.config.ts (1)
18-22: LGTM!The fontFamily configuration correctly sets Inter as the default sans-serif font, aligning with the Google Fonts stylesheet added in index.html.
index.html (2)
5-13: Good: Added essential meta tags and optimized font loading.The addition of favicon, viewport meta tags, and Google Fonts preconnect/stylesheet improves page loading performance and SEO.
50-50: Minor visual update to dark mode card background.Changed from
hsl(240, 3.7%, 9.88%)to#232323- the colors are nearly identical (both are very dark gray). This simplifies the color value.src/types/index.ts (1)
54-60: LGTM!The new
ERC20BalanceAndAllowancetype is well-structured for tracking token balance and allowance data. Using strings for balance and allowance values is appropriate for preserving precision with large numbers.src/context/EventProvider.tsx (1)
10-10: LGTM!The
onBalanceFetchedcallback is correctly added to the Props interface and properly threaded through the provider. The explicit value object ensures all handlers are passed to context consumers, and themock ?? falsedefaulting is good defensive coding.Also applies to: 18-18, 22-30
src/App.tsx (1)
16-17: LGTM!The styling update removes the padding utility (
p-5) from the inner container. This is a minor presentational adjustment with no functional impact.src/components/wallet/WalletItem.tsx (1)
11-13: Code change is correct and safe.Both Phantom and Metamask components work without the size prop:
- Phantom accepts no props at all (line 1 of Phantom.tsx)
- Metamask accepts optional SVGProps (line 5 of Metamask.tsx)
The CSS approach via
[&_svg]:size-10on the parent button correctly handles SVG sizing.src/app/constants/setup.ts (1)
8-8: Import migration verified; external consumer verification required.The import change on line 8 (EvmChainId) and function signature on lines 43-56 are correct. However, the function
getRpcUrlsForChainIdshas no internal consumers in the codebase—no call sites were found. This means undefined value handling cannot be verified within this repository. If external code consumes this exported function, verify that callers handle thePartial<Record<EvmChainId, string>>return type appropriately.src/components/assets/Copy.tsx (1)
1-18: LGTM!Clean SVG icon component with proper structure and attributes.
src/components/wallet/ConnectWallet.tsx (1)
4-4: LGTM!Phantom wallet support is properly integrated. The import, context hook usage, and WalletItem rendering are all correctly wired.
Also applies to: 8-8, 20-20
src/app/constants/index.ts (1)
11-12: LGTM!USDT logo URL updated to a new CDN path.
src/components/assets/ChevronDown.tsx (1)
1-18: LGTM!Clean SVG icon component with proper structure and attributes.
src/components/ui/Button.tsx (1)
8-8: LGTM!Button styling updated with new color scheme and typography.
src/context/walletContext.ts (1)
1-1: LGTM!Wallet context properly updated with the new
connectPhantommethod and migrated import fromwallet-tstowallet-base.Also applies to: 18-18
src/app/wallet/walletService.ts (1)
1-2: Verify typing of ETHEREUM_CHAIN_ID at source in setup.ts to eliminate double casts.The double cast
ETHEREUM_CHAIN_ID as unknown as EvmChainIdat lines 24 (walletService.ts) and 25 (walletStrategy.ts) correctly identifies a type mismatch: ETHEREUM_CHAIN_ID is defined as parseInt(...) returning number, while EvmChainId is a numeric enum. The workaround bypasses TypeScript's type system unnecessarily.Since EvmChainId uses standard numeric chain IDs (Mainnet=1, Goerli=5, Sepolia=11155111) and ETHEREUM_CHAIN_ID defaults to 1 when unset, type it directly as
EvmChainIdat its source in setup.ts (lines 13–15). This eliminates the need for casts throughout the codebase and makes the intent explicit.src/demo.tsx (1)
10-14: LGTM for demo wiring.Handlers look good; expect duplicate logs in dev due to React StrictMode.
Also applies to: 24-26
src/app/wallet/metamask.ts (1)
9-14: ****The review concern is based on a misunderstanding of
parseInt()behavior. The code correctly handles the fallback: whenimport.meta.env.VITE_ETHEREUM_CHAIN_IDis missing, it falls back to the number1, andparseInt(1)returns1—notNaN. TheparseInt()function converts its argument to a string first, soparseInt(1)is equivalent toparseInt("1"), which safely yields1. The existing code is correct and requires no changes.Likely an incorrect or invalid review comment.
src/context/AccountProvider.tsx (1)
16-18: ****The breaking change concern is unfounded. The
denomBalanceMapis consistently keyed by address throughout the codebase:
- Population: AccountProvider.tsx lines 33-38 populates the map with
getAddress(token.address)keys.- Consumption: All accesses (PeggyProvider.tsx:65, BridgeForm.tsx:36) use
getAddress(token.address)to read from the map.- No denom keys: No code accesses the map using
token.denom.All consumer code already expects and uses address-based keys consistently. The only issue is a naming mismatch—
denomBalanceMapsuggests denom keys when it actually uses address keys—but this poses no breaking change.Likely an incorrect or invalid review comment.
src/app/wallet/walletStrategy.ts (1)
24-30: The concern aboutstrategies: {}is incorrect; passing an empty object does not disable built-ins.Omitting the
strategiesoption uses built-in defaults (MetaMask for EVM, Keplr for Cosmos), and passing an emptystrategies: {}object does not disable those defaults—it only overrides if you add entries. The current code is correct in this regard.The typing issue with the double cast (
as unknown as EvmChainId) is valid and worth improving, but the strategies concern should be removed from the review comment.Likely an incorrect or invalid review comment.
src/app/contracts/fetchErc20BalanceAndAllowance.ts (1)
22-35: Optional suggestion is architecturally sound but not required for current usage.The function is currently called with a fixed 2-token list, generating 4 multicall contracts—well within typical RPC limits. The batching suggestion is valid preventive guidance for future scaling, but no action is needed at this time since the token list is hardcoded and not dynamically sized.
src/app/contracts/PeggyContract.ts (1)
51-60: The destination encoding is already correct—review comment is based on incorrect assumptions.The Peggy contract expects the destination as a 32-byte hex with the last 20 bytes being the Ethereum-format address and the first 12 bytes as zeros. The current implementation correctly converts the 20-byte EVM address to this format by left-padding it to 32 bytes using
padHex(), which matches what both call sites already do. The app accepts and processes EVM addresses throughout (as seen inPeggyProvider.tsxline 56 whereethDestinationAddress = addressis the connected wallet). No bech32 decoding is needed, and funds are not at risk.Likely an incorrect or invalid review comment.
| async setTokenAllowance({ | ||
| amount = maxUint256, | ||
| fromAddress, | ||
| tokenAddress, | ||
| spenderAddress, | ||
| }: { | ||
| amount?: bigint; | ||
| fromAddress: Address; | ||
| tokenAddress: Address; | ||
| spenderAddress: Address; | ||
| }) { | ||
| const calldata = encodeFunctionData({ | ||
| abi: erc20WethAbi, | ||
| functionName: "approve", | ||
| args: [spenderAddress, amount], | ||
| }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Infinite approval as default is risky.
Defaulting amount = maxUint256 increases blast radius if the spender is compromised. Prefer explicit infinite approvals via a flag, otherwise approve exact amount.
- async setTokenAllowance({
- amount = maxUint256,
+ async setTokenAllowance({
+ amount,
fromAddress,
tokenAddress,
spenderAddress,
}: {
- amount?: bigint;
+ amount: bigint;
@@
- args: [spenderAddress, amount],
+ args: [spenderAddress, amount],Or add infinite?: boolean and branch to maxUint256 only when requested.
🤖 Prompt for AI Agents
In src/app/contracts/Erc20WethContract.ts around lines 63 to 79, the function
currently defaults amount = maxUint256 which grants infinite approval by
default; change the API to avoid implicit infinite approvals by either (A)
remove the default and require callers to pass an explicit amount, or (B) add an
optional boolean infinite?: boolean and only set amount = maxUint256 when
infinite is true (otherwise require or default to the explicit amount). Update
the function signature, validation logic to reject undefined amount when
infinite is false, and the approve calldata construction to use the resolved
amount; update any call sites/tests to pass the new parameters accordingly.
| const results = await client.multicall({ contracts }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multicall will throw on any revert; handle partial failures safely.
Use allowFailure: true and check per‑call status to avoid the whole batch failing due to one token.
- const results = await client.multicall({ contracts });
+ const results = await client.multicall({ contracts, allowFailure: true });Then when reading results:
- const balanceRes = results[i * 2 + 0];
- const allowanceRes = results[i * 2 + 1];
+ const balanceRes = results[i * 2 + 0];
+ const allowanceRes = results[i * 2 + 1];
@@
- if (!balanceRes) {
+ if (!balanceRes || balanceRes.status === "failure") {
data.push({
address: token.address as string,
balance: "0",
- allowance: "0",
+ allowance:
+ !allowanceRes || allowanceRes.status === "failure"
+ ? "0"
+ : String(allowanceRes.result),
decimals: token.decimals || 18,
symbol: token.symbol,
});
continue;
}
@@
- balance: String(balanceRes.result),
- allowance: String(allowanceRes.result),
+ balance: String(balanceRes.result),
+ allowance:
+ !allowanceRes || allowanceRes.status === "failure"
+ ? "0"
+ : String(allowanceRes.result),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/contracts/fetchErc20BalanceAndAllowance.ts around lines 37-38, the
multicall invocation will throw if any underlying call reverts; change the
multicall to include allowFailure: true and then iterate the returned results
checking each call's success/status before accessing `.result` — for failed
calls set balance/allowance to a safe default (e.g., zero or null) and only
parse values from successful call results, so a single token revert doesn't fail
the whole batch.
| constructor(params: { network: Network }) { | ||
| this.chain = getInjNetworkToChain(params.network ?? Network.Mainnet)!; | ||
| this.peggyAddress = injectivePeggyBridgeAddress; | ||
| this.publicClient = createPublicClient({ | ||
| chain: this.chain, | ||
| transport: http(getAlchemyUrl(params.network)), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Peggy address bound to global NETWORK, not constructor network.
this.peggyAddress uses injectivePeggyBridgeAddress computed from the global env. If you ever instantiate with a different network, address/chain may diverge.
Use the per‑network map directly and type as Address:
-import { injectivePeggyBridgeAddress } from "../data/web3";
+import { injectivePeggyAddressMap } from "../data/web3";
@@
- private peggyAddress: string;
+ private peggyAddress: Address;
@@
- this.chain = getInjNetworkToChain(params.network ?? Network.Mainnet)!;
- this.peggyAddress = injectivePeggyBridgeAddress;
+ this.chain = getInjNetworkToChain(params.network ?? Network.Mainnet);
+ this.peggyAddress = injectivePeggyAddressMap[params.network] as Address;🏁 Script executed:
#!/bin/bash
rg -nP 'injectivePeggy(Address|Bridge)Address(Map)?' -C2 src/app/dataLength of output: 153
---
🏁 Script executed:
```shell
#!/bin/bash
# Check definition of NETWORK and injectivePeggyAddressMap
rg -nP '(export\s+const\s+NETWORK|export\s+const\s+injectivePeggyAddressMap)' -A3 src/app/data
Length of output: 339
🏁 Script executed:
#!/bin/bash
# Check all usages of peggyAddress in PeggyContract.ts
rg -nP 'peggyAddress' -C3 src/app/contracts/PeggyContract.tsLength of output: 879
🏁 Script executed:
#!/bin/bash
# Check if Address type is imported/available in the data file
rg -nP '(import.*Address|type Address)' -B2 -A2 src/app/data/web3.tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Show the full constructor and class signature of PeggyContract
rg -nP 'class PeggyContract|constructor|private peggy' -B2 -A5 src/app/contracts/PeggyContract.ts | head -30Length of output: 465
---
🏁 Script executed:
```shell
#!/bin/bash
# Check imports in PeggyContract.ts, especially Address type
head -25 src/app/contracts/PeggyContract.ts
Length of output: 631
🏁 Script executed:
#!/bin/bash
# Search for other uses of injectivePeggyBridgeAddress in codebase
rg -nP 'injectivePeggyBridgeAddress' src/appLength of output: 289
🏁 Script executed:
#!/bin/bash
# Check the full context of web3.ts to see what's exported
head -30 src/app/data/web3.tsLength of output: 1311
Peggy address bound to global NETWORK, not constructor network.
this.peggyAddress uses the global constant injectivePeggyBridgeAddress computed from global NETWORK. If instantiated with a different network parameter, the address/chain will diverge. Use the per-network map directly and type as Address:
-import { injectivePeggyBridgeAddress } from "../data/web3";
+import { injectivePeggyAddressMap } from "../data/web3";
@@
- private peggyAddress: string;
+ private peggyAddress: Address;
@@
- this.chain = getInjNetworkToChain(params.network ?? Network.Mainnet)!;
- this.peggyAddress = injectivePeggyBridgeAddress;
+ this.chain = getInjNetworkToChain(params.network ?? Network.Mainnet);
+ this.peggyAddress = injectivePeggyAddressMap[params.network ?? Network.Mainnet] as Address;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/contracts/PeggyContract.ts around lines 29 to 36, the constructor
sets this.peggyAddress from the global injectivePeggyBridgeAddress (tied to the
global NETWORK) which can diverge from the provided params.network; change it to
look up the per-network peggy address map using params.network (e.g.
getInjectivePeggyBridgeAddress(params.network)) and assign the result typed as
Address, preserving a sensible fallback or throwing if missing, so the
peggyAddress always matches the chain/publicClient created from the constructor
network.
| this.publicClient = createPublicClient({ | ||
| chain: this.chain, | ||
| transport: http(getAlchemyUrl(params.network)), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Guard against missing/empty RPC URL.
getAlchemyUrl can return ""; constructing http("") will fail later. Fail fast with a clear message.
- this.publicClient = createPublicClient({
- chain: this.chain,
- transport: http(getAlchemyUrl(params.network)),
- });
+ const rpcUrl = getAlchemyUrl(params.network);
+ if (!rpcUrl) throw new Error(`Alchemy URL not configured for ${params.network}`);
+ this.publicClient = createPublicClient({ chain: this.chain, transport: http(rpcUrl) });🏁 Script executed:
#!/bin/bash
rg -n 'ALCHEMY_(SEPOLIA_)?KEY' src/app/constantsLength of output: 739
I need to examine the getAlchemyUrl function to verify the review comment's concern and the appropriateness of the suggested fix.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find getAlchemyUrl function definition
rg -n 'getAlchemyUrl' src/app --type=ts -B2 -A5
Length of output: 3855
🏁 Script executed:
#!/bin/bash
# Check the complete setup.ts file to understand URL construction
cat src/app/constants/setup.ts | head -70Length of output: 2269
🏁 Script executed:
#!/bin/bash
# Find all usages of getAlchemyUrl
rg -n 'getAlchemyUrl' src/app --type=ts -C2Length of output: 2517
Need to see the alchemyUrlNetworkMap construction to understand the full picture.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full utils.ts file to see alchemyUrlNetworkMap
cat src/app/contracts/utils.ts
Length of output: 1282
🏁 Script executed:
#!/bin/bash
# Check what viem's http() does with empty strings by searching for related code
rg -n 'transport.*http' src/app/contracts --type=ts -B1 -A1Length of output: 675
Add guard against empty RPC URL in all affected files, or validate in getAlchemyUrl.
Valid concern: getAlchemyUrl can return empty or malformed URLs when environment variables are missing. However, the same risk exists in three files:
src/app/contracts/PeggyContract.ts(line 34)src/app/contracts/Erc20WethContract.ts(line 59)src/app/contracts/fetchErc20BalanceAndAllowance.ts(line 19)
Either validate at the source in getAlchemyUrl (recommended), or apply the guard consistently to all three call sites.
+ const rpcUrl = getAlchemyUrl(params.network);
+ if (!rpcUrl) throw new Error(`Alchemy URL not configured for ${params.network}`);
- this.publicClient = createPublicClient({
- chain: this.chain,
- transport: http(getAlchemyUrl(params.network)),
- });
+ this.publicClient = createPublicClient({ chain: this.chain, transport: http(rpcUrl) });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.publicClient = createPublicClient({ | |
| chain: this.chain, | |
| transport: http(getAlchemyUrl(params.network)), | |
| }); | |
| const rpcUrl = getAlchemyUrl(params.network); | |
| if (!rpcUrl) throw new Error(`Alchemy URL not configured for ${params.network}`); | |
| this.publicClient = createPublicClient({ chain: this.chain, transport: http(rpcUrl) }); |
🤖 Prompt for AI Agents
In src/app/contracts/PeggyContract.ts around lines 32–35 (and likewise in
src/app/contracts/Erc20WethContract.ts line ~59 and
src/app/contracts/fetchErc20BalanceAndAllowance.ts line ~19), the call to
http(getAlchemyUrl(params.network)) can receive an empty or malformed URL;
validate getAlchemyUrl to ensure it returns a non-empty, well-formed URL (or
throw a clear error) and update callers to either guard before calling
createPublicClient/createClient or rely on the validated getAlchemyUrl;
specifically add a check in getAlchemyUrl that environment variables exist and
the constructed URL is non-empty (and optionally matches a URL pattern), throw a
descriptive error if invalid, or return a safe default, and remove/replace any
direct calls that assume a valid URL with a guarded call that surfaces the
error.
|
|
||
| <div className='mt-2'> | ||
| <p className='text-sm text-gray-500'> | ||
| <div className="mt-2 flex items-center justify-Start space-x-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Tailwind class casing.
“justify-Start” should be “justify-start”.
-<div className="mt-2 flex items-center justify-Start space-x-2">
+<div className="mt-2 flex items-center justify-start space-x-2">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="mt-2 flex items-center justify-Start space-x-2"> | |
| <div className="mt-2 flex items-center justify-start space-x-2"> |
🤖 Prompt for AI Agents
In src/components/bridge/BridgeForm.tsx around line 124, the Tailwind class name
"justify-Start" is incorrectly cased; replace it with "justify-start" to match
Tailwind's lowercase utility naming so the layout utility is applied correctly.
|
|
||
| export default function Selector() { | ||
| return ( | ||
| <div className="border rounded-md px-4 py-3 border-gray-300 dark:border-[#42474E] block flex items-center cursor-not-allowed"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove conflicting block class.
The className includes both block and flex, which conflict. The flex display will take precedence, making block redundant and potentially confusing.
Apply this diff:
- <div className="border rounded-md px-4 py-3 border-gray-300 dark:border-[#42474E] block flex items-center cursor-not-allowed">
+ <div className="border rounded-md px-4 py-3 border-gray-300 dark:border-[#42474E] flex items-center cursor-not-allowed">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="border rounded-md px-4 py-3 border-gray-300 dark:border-[#42474E] block flex items-center cursor-not-allowed"> | |
| <div className="border rounded-md px-4 py-3 border-gray-300 dark:border-[#42474E] flex items-center cursor-not-allowed"> |
🤖 Prompt for AI Agents
In src/components/bridge/Selector.tsx around line 6, the element's className
contains both "block" and "flex" which conflict; remove the redundant "block"
class so only "flex" remains (or remove "flex" if you intend it to be a block
element); update the className to eliminate the conflicting class and keep the
desired display utility.
| <label | ||
| htmlFor='id' | ||
| className='border rounded-md p-2 border-gray-300 dark:border-gray-700 block' | ||
| htmlFor="id" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix: Use variable reference instead of string literal for htmlFor.
The htmlFor attribute is set to the string literal "id" instead of the variable {id}. This breaks the label-input association, preventing click-to-focus behavior and breaking accessibility.
Apply this diff:
- htmlFor="id"
+ htmlFor={id}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| htmlFor="id" | |
| htmlFor={id} |
🤖 Prompt for AI Agents
In src/components/common/CurrencyInput.tsx around line 33, the label's htmlFor
is using the string literal "id" instead of the id variable; change htmlFor="id"
to htmlFor={id} so the label targets the actual input id (ensure the component
has an id prop or local id variable defined and passed to the input element).
src/context/AccountProvider.tsx
Outdated
| const fetchBalanceAndAllowance = useCallback(async () => { | ||
| { | ||
| for await (const token of tokens) { | ||
| const { balance, allowance } = | ||
| await web3Client.fetchTokenBalanceAndAllowance({ | ||
| address: address, | ||
| contractAddress: token.denom.replace("peggy", ""), | ||
| }); | ||
|
|
||
| setDenomBalanceMap((p) => ({ | ||
| ...p, | ||
| [token.denom]: { balance, allowance }, | ||
| })); | ||
| } | ||
| } | ||
| const balancesAndAllowances = await getErc20BalancesAndAllowances({ | ||
| address: address, | ||
| tokenList: tokens, | ||
| spender: injectivePeggyBridgeAddress, | ||
| }); | ||
|
|
||
| console.log({ balancesAndAllowances }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add guard for empty address and remove noisy/PII logs.
Avoid unnecessary network calls and leaking balances/allowances to console.
const fetchBalanceAndAllowance = useCallback(async () => {
- const balancesAndAllowances = await getErc20BalancesAndAllowances({
+ if (!address) return;
+ const balancesAndAllowances = await getErc20BalancesAndAllowances({
address: address,
tokenList: tokens,
spender: injectivePeggyBridgeAddress,
});
-
- console.log({ balancesAndAllowances });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fetchBalanceAndAllowance = useCallback(async () => { | |
| { | |
| for await (const token of tokens) { | |
| const { balance, allowance } = | |
| await web3Client.fetchTokenBalanceAndAllowance({ | |
| address: address, | |
| contractAddress: token.denom.replace("peggy", ""), | |
| }); | |
| setDenomBalanceMap((p) => ({ | |
| ...p, | |
| [token.denom]: { balance, allowance }, | |
| })); | |
| } | |
| } | |
| const balancesAndAllowances = await getErc20BalancesAndAllowances({ | |
| address: address, | |
| tokenList: tokens, | |
| spender: injectivePeggyBridgeAddress, | |
| }); | |
| console.log({ balancesAndAllowances }); | |
| const fetchBalanceAndAllowance = useCallback(async () => { | |
| if (!address) return; | |
| const balancesAndAllowances = await getErc20BalancesAndAllowances({ | |
| address: address, | |
| tokenList: tokens, | |
| spender: injectivePeggyBridgeAddress, | |
| }); | |
| }, [address, tokens, injectivePeggyBridgeAddress]); |
🤖 Prompt for AI Agents
In src/context/AccountProvider.tsx around lines 22 to 30, the
fetchBalanceAndAllowance callback calls getErc20BalancesAndAllowances even when
address may be empty and logs the returned balances to console; add a guard that
returns early if address is falsy (or not a valid checksum address) or if tokens
list is empty to avoid unnecessary network calls, and remove the console.log({
balancesAndAllowances }) line so balances/allowances are not written to stdout
(instead handle results silently or update state as intended).
| const allowanceByDenom = | ||
| denomBalanceMap[getAddress(token.address)].allowance; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against missing allowance entry.
denomBalanceMap[getAddress(token.address)] can be undefined; current code will throw.
-const allowanceByDenom =
- denomBalanceMap[getAddress(token.address)].allowance;
+const allowanceByDenom =
+ denomBalanceMap[getAddress(token.address)]?.allowance ?? "0";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const allowanceByDenom = | |
| denomBalanceMap[getAddress(token.address)].allowance; | |
| const allowanceByDenom = | |
| denomBalanceMap[getAddress(token.address)]?.allowance ?? "0"; |
🤖 Prompt for AI Agents
In src/context/PeggyProvider.tsx around lines 64 to 66, the access
denomBalanceMap[getAddress(token.address)].allowance can throw when the map
entry is undefined; change it to first resolve the address into a variable, then
safely look up the map entry and read allowance with a guard/optional chaining
and a sensible default (e.g. 0 or an empty allowance object) so allowanceByDenom
never reads a property of undefined; update subsequent logic to handle the
default allowance value consistently.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
19-26: Good: exact version pinning for @InjectiveLabs packages ensures deterministic builds.Switching from caret ranges (^) to exact versions (1.16.22) for @InjectiveLabs dependencies is a sound decision that aligns with the modularization strategy and enables reproducible builds. This is well-suited for a widget/library package.
Consider extending this pinning discipline to @bangjelkoski/vite-plugin-node-polyfills (currently
^0.0.2at line 18) for full consistency, though this is optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json(2 hunks)
🔇 Additional comments (1)
package.json (1)
3-3: Version mismatch: PR objectives state 0.0.12, but file shows 0.0.13.The PR summary explicitly states the version should be bumped to 0.0.12, but the annotated code shows 0.0.13. Verify the intended version number and update accordingly.
| "@injectivelabs/exceptions": "1.16.22", | ||
| "@injectivelabs/networks": "1.16.22", | ||
| "@injectivelabs/sdk-ts": "1.16.22", | ||
| "@injectivelabs/utils": "1.16.22", | ||
| "@injectivelabs/wallet-core": "1.16.22", | ||
| "@injectivelabs/wallet-base": "1.16.22", | ||
| "@injectivelabs/wallet-strategy": "1.16.22", | ||
| "@injectivelabs/wallet-evm": "1.16.22", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify all wallet package imports have been refactored across the codebase.
The removal of @injectivelabs/wallet-ts and introduction of modular wallet packages (wallet-core, wallet-base, wallet-strategy, wallet-evm) represents a breaking change. Ensure that all code files importing from the old wallet-ts have been updated to use the new modular packages.
The PR objectives mention updating imports across the codebase (e.g., src/Providers.tsx). Please run the following script to confirm no stale wallet-ts imports remain:
🏁 Script executed:
#!/bin/bash
# Search for any remaining imports from @injectivelabs/wallet-ts
rg -n "@injectivelabs/wallet-ts" --type=ts --type=tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for any remaining imports from @injectivelabs/wallet-ts
# Using file glob pattern to capture both .ts and .tsx files
rg -n "@injectivelabs/wallet-ts" --type=ts -g "*.tsx"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Alternative: search without type specification, using simple pattern
rg -n "@injectivelabs/wallet-ts" -g "*.ts" -g "*.tsx"Length of output: 211
🏁 Script executed:
#!/bin/bash
# Verify the new wallet packages are properly imported
rg -n "@injectivelabs/wallet-(core|base|strategy|evm)" --type=ts -g "*.tsx" | head -20Length of output: 226
🏁 Script executed:
#!/bin/bash
# Check package.json to confirm wallet-ts is removed
rg -n "wallet-ts" package.jsonLength of output: 0
Stale wallet-ts imports remain in the codebase and must be refactored.
The migration from @injectivelabs/wallet-ts to modular wallet packages is incomplete. Two stale imports remain in src/app/wallet/cosmos.ts:
- Line 7:
import { CosmosWalletStrategy, Wallet } from "@injectivelabs/wallet-ts" - Line 8:
import { KeplrWallet } from "@injectivelabs/wallet-ts/dist/esm/utils/wallets/keplr"
These must be updated to use the new modular packages (wallet-core, wallet-base, wallet-strategy, wallet-evm) to complete the breaking change.
🤖 Prompt for AI Agents
In src/app/wallet/cosmos.ts around lines 7-8, the file still imports from the
deprecated "@injectivelabs/wallet-ts"; replace those two stale imports by
importing the Cosmos strategy and Wallet type from the modular packages: import
CosmosWalletStrategy from "@injectivelabs/wallet-strategy" (or the named export
if used in code), import the Wallet type/abstraction from
"@injectivelabs/wallet-core", and import KeplrWallet from the new package where
Keplr lives (e.g. "@injectivelabs/wallet-base" or its esm utils path) so the
code uses @injectivelabs/wallet-core, @injectivelabs/wallet-strategy and
@injectivelabs/wallet-base instead of "@injectivelabs/wallet-ts"; preserve
existing identifiers and adjust named/default import syntax to match the new
packages' exports.
There was a problem hiding this 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
♻️ Duplicate comments (5)
src/context/AccountProvider.tsx (1)
22-27: Guard against empty/invalid address before fetching.Avoid unnecessary calls and possible errors when address is falsy.
const fetchBalanceAndAllowance = useCallback(async () => { - const balancesAndAllowances = await getErc20BalancesAndAllowances({ + if (!address) return; + const balancesAndAllowances = await getErc20BalancesAndAllowances({ address: address, tokenList: tokens, spender: injectivePeggyBridgeAddress, });src/app/contracts/PeggyContract.ts (1)
20-32: Critical: peggyAddress tied to global NETWORK, not constructor network.Use the per‑network resolver to avoid mismatched address/chain.
-import { injectivePeggyBridgeAddress } from "../data/web3"; +import { getInjectivePeggyBridgeAddress } from "../data/web3"; @@ - private peggyAddress: string; + private peggyAddress: Address; @@ - constructor(params: { network: Network }) { - this.chain = getInjNetworkToChain(params.network ?? Network.Mainnet)!; - this.peggyAddress = injectivePeggyBridgeAddress; - this.publicClient = createPublicClient({ - chain: this.chain, - transport: http(getAlchemyUrl(params.network)), - }); - } + constructor(params: { network: Network }) { + this.chain = getInjNetworkToChain(params.network); + const rpcUrl = getAlchemyUrl(params.network); + if (!rpcUrl) throw new Error(`Alchemy URL not configured for ${params.network}`); + this.peggyAddress = getInjectivePeggyBridgeAddress(params.network); + this.publicClient = createPublicClient({ chain: this.chain, transport: http(rpcUrl) }); + }src/app/contracts/fetchErc20BalanceAndAllowance.ts (1)
37-64: Handle per-call failures and avoid String(undefined).multicall returns per-call statuses; you must guard on status before reading
.resultto prevent "undefined" strings and partial-batch failures bleeding through. Explicitly setallowFailure: true(default) and branch onstatus === 'success'. (viem.sh)- const results = await client.multicall({ contracts }); + const results = await client.multicall({ contracts, allowFailure: true }); - const data: ERC20BalanceAndAllowance[] = []; + const data: ERC20BalanceAndAllowance[] = []; for (let i = 0; i < tokenList.length; i++) { const token = tokenList[i]; const balanceRes = results[i * 2 + 0]; const allowanceRes = results[i * 2 + 1]; - if (!balanceRes) { - data.push({ - address: token.address as string, - balance: "0", - allowance: "0", - decimals: token.decimals || 18, - symbol: token.symbol, - }); - continue; - } - - data.push({ - address: token.address as string, - balance: String(balanceRes.result), - allowance: String(allowanceRes.result), - decimals: token.decimals || 18, - symbol: token.symbol, - }); + const balanceOk = balanceRes && balanceRes.status === "success"; + const allowanceOk = allowanceRes && allowanceRes.status === "success"; + + data.push({ + address: token.address as string, + balance: balanceOk ? String(balanceRes.result) : "0", + allowance: allowanceOk ? String(allowanceRes.result) : "0", + decimals: token.decimals ?? 18, + symbol: token.symbol, + }); }src/app/contracts/Erc20WethContract.ts (2)
53-57: WETH address is hardcoded to mainnet; wrong on Sepolia/testnets.Resolve WETH per network (e.g., Sepolia: 0xfff9…6b14) to avoid failed deposits on testnets. (docs.uniswap.org)
-import { wethToken } from "../data/tokens"; +import { wethToken } from "../data/tokens"; +// Consider moving this map to a shared constants module. +const WETH_ADDRESS_BY_NETWORK: Record<Network, Address> = { + // Ethereum mainnet + Mainnet: "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2" as Address, + // Sepolia testnet + Testnet: "0xfff9976782d46cc05630d1f6ebab18b2324d6b14" as Address, + // Fallbacks for other non-mainnet envs to Sepolia (adjust if you support more) + Devnet: "0xfff9976782d46cc05630d1f6ebab18b2324d6b14" as Address, +} as const; @@ - this.wethAddress = wethToken.address as Address; + this.wethAddress = + WETH_ADDRESS_BY_NETWORK[params.network] ?? + (wethToken.address as Address);
60-70: Avoid implicit infinite approvals.Defaulting
amount = maxUint256expands risk if the spender is compromised. Require explicitamountor gate infinite approvals behind a flag.- async setTokenAllowance({ - amount = maxUint256, + async setTokenAllowance({ + amount, fromAddress, tokenAddress, spenderAddress, }: { - amount?: bigint; + amount: bigint;If you need “approve max” behavior, introduce
infinite?: booleanand setamount = maxUint256only when explicitly requested.
🧹 Nitpick comments (8)
src/app/constants/setup.ts (1)
32-35: Remove legacy Kovan/Goerli keys or mark deprecated.These envs aren’t used; simplify config to avoid confusion.
src/context/AccountProvider.tsx (3)
3-3: Also validate the address format.Use isAddress to fail fast on malformed inputs.
-import { getAddress } from "viem"; +import { getAddress, isAddress } from "viem";And add before the fetch:
- const fetchBalanceAndAllowance = useCallback(async () => { + const fetchBalanceAndAllowance = useCallback(async () => { + if (!address || !isAddress(address)) return;
29-38: Normalize possibly undefined multicall results.Protect against
"undefined"strings if a call errored.- const balanceAndAllowanceMap = balancesAndAllowances.reduce( - (acc, token) => { - acc[getAddress(token.address)] = { - balance: token.balance, - allowance: token.allowance, - }; - return acc; - }, - {} as BalanceAndAllowance - ); + const balanceAndAllowanceMap = balancesAndAllowances.reduce((acc, t) => { + const key = getAddress(t.address); + const balance = t.balance && t.balance !== "undefined" ? t.balance : "0"; + const allowance = t.allowance && t.allowance !== "undefined" ? t.allowance : "0"; + acc[key] = { balance, allowance }; + return acc; + }, {} as BalanceAndAllowance);
41-41: Include stable deps or suppress intentionally.If lint warns, add injectivePeggyBridgeAddress to deps or document why it’s excluded.
src/app/contracts/utils.ts (1)
27-37: Optional: add fee estimation fallback for non‑EIP‑1559 chains.Defensive path avoids failures if estimateFeesPerGas is unavailable.
- const [gas, fees, nonce] = await Promise.all([ + const [gas, fees, nonce] = await Promise.all([ publicClient.estimateGas({ account: from, to, data: calldata, value, }), - publicClient.estimateFeesPerGas(), + publicClient + .estimateFeesPerGas() + .catch(async () => { + const gp = await publicClient.getGasPrice(); + return { maxFeePerGas: gp, maxPriorityFeePerGas: gp }; + }), publicClient.getTransactionCount({ address: from }), ]);src/app/contracts/PeggyContract.ts (1)
50-56: Normalize token and destination addresses.Ensure checksum/format before encoding.
- args: [ - tokenAddress as Address, - padHex(destination, { size: 32 }), + args: [ + (tokenAddress as Address), + padHex(destination, { size: 32 }), BigInt(amount), "", ],If
destinationAddressis not a hex string (e.g., a bech32 Injective address), convert to the expected 32‑byte format beforepadHex.src/app/contracts/fetchErc20BalanceAndAllowance.ts (1)
8-21: Short-circuit on empty token list.
Avoid a network roundtrip and constructing an empty multicall.export async function getErc20BalancesAndAllowances({ address, spender, tokenList, }: { address: string; spender: string; tokenList: TokenStatic[]; }) { + if (!tokenList?.length) return [];src/app/wallet/walletStrategy.ts (1)
25-26: Confirm strategies are configured.An empty
strategies: {}may leave no EVM connectors;enableAndGetAddresses()could fail at runtime. Verify upstream initialization or wire in at least MetaMask/WalletConnect strategies here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/app/constants/setup.ts(2 hunks)src/app/contracts/Erc20Contract.ts(0 hunks)src/app/contracts/Erc20WethContract.ts(1 hunks)src/app/contracts/Peggy.ts(0 hunks)src/app/contracts/PeggyContract.ts(1 hunks)src/app/contracts/fetchErc20BalanceAndAllowance.ts(1 hunks)src/app/contracts/utils.ts(1 hunks)src/app/data/web3.ts(1 hunks)src/app/estimator.ts(0 hunks)src/app/ethGasPrice.ts(0 hunks)src/app/wallet/cosmos.ts(0 hunks)src/app/wallet/walletStrategy.ts(2 hunks)src/app/web3Client.ts(0 hunks)src/app/web3composer.ts(0 hunks)src/components/bridge/Bridge.tsx(1 hunks)src/context/AccountProvider.tsx(1 hunks)
💤 Files with no reviewable changes (7)
- src/app/estimator.ts
- src/app/contracts/Peggy.ts
- src/app/web3Client.ts
- src/app/ethGasPrice.ts
- src/app/web3composer.ts
- src/app/wallet/cosmos.ts
- src/app/contracts/Erc20Contract.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/bridge/Bridge.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
src/context/AccountProvider.tsx (4)
src/context/walletContext.ts (1)
useWallet(25-31)src/app/data/tokens.ts (2)
usdtToken(63-78)injErc20Token(80-96)src/app/contracts/fetchErc20BalanceAndAllowance.ts (1)
getErc20BalancesAndAllowances(8-67)src/app/data/web3.ts (1)
injectivePeggyBridgeAddress(16-17)
src/app/data/web3.ts (1)
src/app/constants/setup.ts (1)
NETWORK(10-11)
src/app/contracts/Erc20WethContract.ts (3)
src/app/contracts/utils.ts (2)
getInjNetworkToChain(6-12)estimateGasAndNonce(14-38)src/app/data/tokens.ts (1)
wethToken(98-117)src/app/constants/setup.ts (2)
getAlchemyUrl(55-65)NETWORK(10-11)
src/app/contracts/PeggyContract.ts (3)
src/app/contracts/utils.ts (2)
getInjNetworkToChain(6-12)estimateGasAndNonce(14-38)src/app/data/web3.ts (1)
injectivePeggyBridgeAddress(16-17)src/app/constants/setup.ts (2)
getAlchemyUrl(55-65)NETWORK(10-11)
src/app/contracts/fetchErc20BalanceAndAllowance.ts (2)
src/app/constants/setup.ts (3)
IS_MAINNET(18-18)getAlchemyUrl(55-65)NETWORK(10-11)src/types/index.ts (1)
ERC20BalanceAndAllowance(54-60)
src/app/wallet/walletStrategy.ts (1)
src/app/constants/setup.ts (4)
CHAIN_ID(16-16)ETHEREUM_CHAIN_ID(13-15)getAlchemyUrl(55-65)NETWORK(10-11)
🔇 Additional comments (4)
src/app/constants/setup.ts (3)
55-65: Fail fast if Alchemy keys are missing.Guard against empty/misconfigured envs so errors surface early.
[ suggest_essential_refactor ]export const getAlchemyUrl = (network: Network): string => { - if (isMainnet(network)) { - return `https://eth-mainnet.alchemyapi.io/v2/${ALCHEMY_KEY}`; - } - - if (isTestnet(network)) { - return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; - } - - return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; + if (isMainnet(network)) { + if (!ALCHEMY_KEY) throw new Error("Missing ALCHEMY_KEY for Mainnet"); + return `https://eth-mainnet.alchemyapi.io/v2/${ALCHEMY_KEY}`; + } + // Treat all non‑mainnet as Sepolia for now + if (!ALCHEMY_SEPOLIA_KEY) { + throw new Error(`Missing ALCHEMY_SEPOLIA_KEY for ${String(network)}`); + } + return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; };
43-53: Add the same guard for the chainId-based resolver.Mirror the fail-fast behavior to avoid silent bad endpoints.
[ suggest_essential_refactor ]export const getAlchemyRpcEndpointForChainId = (chainId: EvmChainId) => { if (chainId === EvmChainId.Mainnet) { - return `https://eth-mainnet.alchemyapi.io/v2/${ALCHEMY_KEY}`; + if (!ALCHEMY_KEY) throw new Error("Missing ALCHEMY_KEY for Mainnet"); + return `https://eth-mainnet.alchemyapi.io/v2/${ALCHEMY_KEY}`; } if (chainId === EvmChainId.Sepolia) { - return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; + if (!ALCHEMY_SEPOLIA_KEY) throw new Error("Missing ALCHEMY_SEPOLIA_KEY for Sepolia"); + return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; } - return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; + if (!ALCHEMY_SEPOLIA_KEY) throw new Error("Missing ALCHEMY_SEPOLIA_KEY"); + return `https://eth-sepolia.alchemyapi.io/v2/${ALCHEMY_SEPOLIA_KEY}`; };
13-15: The search confirmsETHEREUM_CHAIN_IDis actively referenced and used across the codebase: imported and utilized inwalletService.ts,metamask.ts, andwalletStrategy.ts. The constant is not unused.
Likely an incorrect or invalid review comment.
src/app/contracts/utils.ts (1)
6-12: Network→chain mapping looks consistent with RPC routing.Mainnet→mainnet, others→sepolia matches getAlchemyUrl behavior.
If other networks (Devnet3, Local) are expected to use non‑Sepolia RPC, call out and extend this mapping accordingly.
| getNetworkEndpoints, | ||
| } from "@injectivelabs/networks"; | ||
| import { EthereumChainId } from "@injectivelabs/ts-types"; | ||
| import { EvmChainId } from "@injectivelabs/ts-types"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify @injectivelabs/ts-types is declared and version-aligned.
Importing EvmChainId requires the package; ensure it’s in dependencies and consistent with @injectivelabs/networks.
🏁 Script executed:
#!/bin/bash
# Check dependency presence and versions
jq -r '.dependencies, .devDependencies' package.json | rg -n '"@injectivelabs/(ts-types|networks)"' -nLength of output: 41
---
🏁 Script executed:
```shell
#!/bin/bash
# Check full package.json for @injectivelabs packages
jq -r '.dependencies, .devDependencies' package.json | grep -i "@injectivelabs"
Length of output: 331
🏁 Script executed:
#!/bin/bash
# Check for all imports from @injectivelabs/ts-types in the codebase
rg -n "from\s+['\"]@injectivelabs/ts-types" --type ts --type tsxLength of output: 27
🌐 Web query:
@injectivelabs/ts-types package dependency
💡 Result:
Summary — @injectivelabs/ts-types
- What it is: a small npm package from Injective that exports shared TypeScript enums, interfaces and types used across Injective packages (SDK, networks, UI, etc.). [1][2]
- Latest published version (npm): 1.16.10 (recently published). [1]
- Install: npm i @injectivelabs/ts-types (or yarn add / pnpm add). [1][2]
- Usage: import types/ enums like ChainId, EthereumChainId, etc., in your TypeScript code. Documentation and API reference are in the injective-ts monorepo docs. [2][3]
Sources:
[1] npm package page for @injectivelabs/ts-types.
[2] InjectiveLabs GitHub (injective-ts monorepo, packages/ts-types).
[3] Injective TypeScript API docs / injective-ts documentation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for all imports from @injectivelabs/ts-types
rg -n "from\s+['\"]@injectivelabs/ts-types" --type ts
Length of output: 435
Add @injectivelabs/ts-types to dependencies at version 1.16.22.
@injectivelabs/ts-types is a package that exports shared TypeScript enums, interfaces and types used across Injective packages, but it is missing from package.json. The import statement in src/app/constants/setup.ts (and 4 other files) will fail without it. Align the version with other @InjectiveLabs packages in the codebase (1.16.22).
🤖 Prompt for AI Agents
In src/app/constants/setup.ts around line 8, the import of EvmChainId from
@injectivelabs/ts-types will fail because that package is missing from
package.json; add "@injectivelabs/ts-types": "1.16.22" to your dependencies (not
devDependencies) in package.json, run your package manager install
(npm/yarn/pnpm) to update node_modules and the lockfile, and then rebuild/test
to ensure the import resolves.
| export const getInjectivePeggyBridgeAddress = (network: Network) => { | ||
| if (isMainnet(network)) { | ||
| return "0xF955C57f9EA9Dc8781965FEaE0b6A2acE2BAD6f3"; | ||
| } | ||
|
|
||
| if (isTestnet(network)) { | ||
| return "0x12e1181a741b70BE6A9D81f85af3E92B6ba41897"; | ||
| } | ||
|
|
||
| return "0x430544ca09F7914077a0E8F405Da62292428F49D"; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Checksum and type the derived Peggy address.
Return a validated Address and normalize casing.
-import { isMainnet, isTestnet, Network } from "@injectivelabs/networks";
-import { NETWORK } from "../constants";
+import { isMainnet, isTestnet, Network } from "@injectivelabs/networks";
+import { NETWORK } from "../constants";
+import { getAddress } from "viem";
+import type { Address } from "viem";
@@
-export const getInjectivePeggyBridgeAddress = (network: Network) => {
- if (isMainnet(network)) {
- return "0xF955C57f9EA9Dc8781965FEaE0b6A2acE2BAD6f3";
- }
+export const getInjectivePeggyBridgeAddress = (network: Network): Address => {
+ if (isMainnet(network)) {
+ return getAddress("0xF955C57f9EA9Dc8781965FEaE0b6A2acE2BAD6f3");
+ }
@@
- if (isTestnet(network)) {
- return "0x12e1181a741b70BE6A9D81f85af3E92B6ba41897";
- }
+ if (isTestnet(network)) {
+ return getAddress("0x12e1181a741b70BE6A9D81f85af3E92B6ba41897");
+ }
@@
- return "0x430544ca09F7914077a0E8F405Da62292428F49D";
+ return getAddress("0x430544ca09F7914077a0E8F405Da62292428F49D");
};
@@
-export const injectivePeggyBridgeAddress =
- getInjectivePeggyBridgeAddress(NETWORK);
+export const injectivePeggyBridgeAddress: Address =
+ getInjectivePeggyBridgeAddress(NETWORK);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getInjectivePeggyBridgeAddress = (network: Network) => { | |
| if (isMainnet(network)) { | |
| return "0xF955C57f9EA9Dc8781965FEaE0b6A2acE2BAD6f3"; | |
| } | |
| if (isTestnet(network)) { | |
| return "0x12e1181a741b70BE6A9D81f85af3E92B6ba41897"; | |
| } | |
| return "0x430544ca09F7914077a0E8F405Da62292428F49D"; | |
| }; | |
| import { isMainnet, isTestnet, Network } from "@injectivelabs/networks"; | |
| import { NETWORK } from "../constants"; | |
| import { getAddress } from "viem"; | |
| import type { Address } from "viem"; | |
| export const getInjectivePeggyBridgeAddress = (network: Network): Address => { | |
| if (isMainnet(network)) { | |
| return getAddress("0xF955C57f9EA9Dc8781965FEaE0b6A2acE2BAD6f3"); | |
| } | |
| if (isTestnet(network)) { | |
| return getAddress("0x12e1181a741b70BE6A9D81f85af3E92B6ba41897"); | |
| } | |
| return getAddress("0x430544ca09F7914077a0E8F405Da62292428F49D"); | |
| }; | |
| export const injectivePeggyBridgeAddress: Address = | |
| getInjectivePeggyBridgeAddress(NETWORK); |
🤖 Prompt for AI Agents
In src/app/data/web3.ts around lines 4 to 14, the Peggy bridge addresses are
returned as plain strings without validation or checksum normalization; change
the function to return a typed Address and normalize/validate each constant
using a checksum routine (e.g., ethers.utils.getAddress) before returning.
Update the function signature to return Address (import the Address type), run
the raw hex literals through the checksum normalizer and return those validated
values so callers always get a properly checksummed Address.
| evmOptions: { | ||
| evmChainId: ETHEREUM_CHAIN_ID as unknown as EvmChainId, | ||
| rpcUrl: getAlchemyUrl(NETWORK), | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent chainId/RPC mismatches (mainnet vs Sepolia).
If VITE_ETHEREUM_CHAIN_ID isn’t set for testnet, this will pass 1 with a Sepolia RPC URL, causing provider errors. Derive evmChainId from NETWORK.
- evmOptions: {
- evmChainId: ETHEREUM_CHAIN_ID as unknown as EvmChainId,
- rpcUrl: getAlchemyUrl(NETWORK),
- },
+ evmOptions: {
+ evmChainId: IS_MAINNET ? EvmChainId.Mainnet : EvmChainId.Sepolia,
+ rpcUrl: getAlchemyUrl(NETWORK),
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| evmOptions: { | |
| evmChainId: ETHEREUM_CHAIN_ID as unknown as EvmChainId, | |
| rpcUrl: getAlchemyUrl(NETWORK), | |
| }, | |
| evmOptions: { | |
| evmChainId: IS_MAINNET ? EvmChainId.Mainnet : EvmChainId.Sepolia, | |
| rpcUrl: getAlchemyUrl(NETWORK), | |
| }, |
🤖 Prompt for AI Agents
In src/app/wallet/walletStrategy.ts around lines 20 to 23, the evmChainId is
currently taken from VITE_ETHEREUM_CHAIN_ID which can default to mainnet while
rpcUrl is derived from NETWORK (e.g., Sepolia), causing chainId/RPC mismatches;
change to derive evmChainId from NETWORK instead (create or call a helper that
maps NETWORK -> correct numeric chainId and cast to EvmChainId), then use that
value for evmOptions.evmChainId so the chainId always matches the chosen RPC
URL; ensure proper typing and fallback behavior for unknown NETWORK values.
There was a problem hiding this 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
🧹 Nitpick comments (2)
src/context/WalletProvider.tsx (2)
75-88: Consolidate duplicate wallet connection logic.
connectPhantomis nearly identical toconnectMetamask(lines 60-73), differing only in the wallet type. This violates the DRY principle and creates a maintenance burden.Refactor into a single parameterized function:
- async function connectMetamask() { - await connectWallet(Wallet.Metamask); - - const addresses = await getAddresses(); - const [address] = addresses; - - setWallet(Wallet.Metamask); - setAddress(address); - setAddresses(addresses); - setInjectiveAddress(getInjectiveAddress(address)); - setAddressConfirmation(await walletStrategy.getSessionOrConfirm(address)); - - // on connect - } - - async function connectPhantom() { - await connectWallet(Wallet.Phantom); + async function connectWalletAndSetState(walletType: Wallet) { + await connectWallet(walletType); const addresses = await getAddresses(); const [address] = addresses; - setWallet(Wallet.Phantom); + setWallet(walletType); setAddress(address); setAddresses(addresses); setInjectiveAddress(getInjectiveAddress(address)); setAddressConfirmation(await walletStrategy.getSessionOrConfirm(address)); // on connect } + + async function connectMetamask() { + await connectWalletAndSetState(Wallet.Metamask); + } + + async function connectPhantom() { + await connectWalletAndSetState(Wallet.Phantom); + }
72-72: Remove or complete placeholder comments.The
// on connectcomments appear to be placeholders without meaningful content. Either remove them or document the intended post-connection logic.Also applies to: 87-87
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(2 hunks)src/context/WalletProvider.tsx(4 hunks)src/demo.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/demo.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/context/WalletProvider.tsx (2)
src/app/wallet/walletStrategy.ts (2)
walletStrategy(18-26)getAddresses(32-56)src/app/wallet/metamask.ts (1)
validateMetamask(16-71)
🔇 Additional comments (7)
package.json (2)
19-26: Version discrepancy between PR objectives and actual code—verify intentional.The PR objectives state that all @InjectiveLabs packages were upgraded to v1.16.20, but package.json specifies 1.16.22. Confirm whether 1.16.22 is the intended final version (e.g., a point release bump post-planning) or if this needs adjustment.
19-26: Wallet package migration and exact version pinning look good.The migration from @injectivelabs/wallet-ts to modular wallet packages (wallet-core, wallet-base, wallet-strategy, wallet-evm) is properly reflected in package.json. Pinning all @InjectiveLabs dependencies to exact versions (1.16.22) ensures reproducibility and consistency across environments.
However, as flagged in the previous review, ensure all codebase imports (e.g., in src/app/wallet/cosmos.ts) have been updated to use the new modular packages rather than the deprecated wallet-ts.
src/context/WalletProvider.tsx (5)
1-2: LGTM!The updated imports correctly reflect the migration from deprecated
wallet-tsto the modularwallet-basepackage, and the addition ofuseEffectanduseCallbacksupports the refactored initialization flow.
51-53: LGTM!The
initfunction is correctly wrapped inuseCallbackwith the appropriate[wallet]dependency, ensuring it's memoized and only recreates when the wallet changes.
90-92: LGTM!The
useEffecthook correctly invokesinit()with the appropriate dependency on[init], ensuring initialization runs on mount and when the wallet changes.
94-107: LGTM!The context value correctly exposes
connectPhantomalongside the existing wallet management methods, maintaining a consistent API shape.
40-48: Verify redundancy of account request calls for Metamask.The code calls
eth_requestAccountsfor all EVM wallets (line 40-44), then additionally callsvalidateMetamaskfor Metamask (line 46-48), which internally callsenableAndGetAddresses. The overlap between these calls and whether they're redundant for Metamask depends on whatenableAndGetAddresses(from@injectivelabs/wallet-strategy) does internally—this cannot be confirmed from the codebase alone.Clarify the intent: is the
eth_requestAccountscall intentional for all EVM wallets including Metamask, or should it only apply to non-Metamask wallets wherevalidateMetamaskis not called?
This pull request introduces significant updates to the project's dependencies and EVM contract interaction logic, as well as some minor UI and codebase improvements. The main focus is on migrating to the latest Injective Labs wallet packages, refactoring EVM-related utilities and contracts for better maintainability, and improving token and bridge contract handling. There are also minor updates to UI styling and project metadata.
Major dependency and EVM contract refactoring:
@injectivelabsdependencies to version1.16.20and replaced deprecated packages (wallet-ts) with the new modular packages (wallet-core,wallet-base,wallet-strategy,wallet-evm). Updated all relevant imports throughout the codebase to use the new packages. [1] [2] [3]Erc20WethContract.ts,PeggyContract.ts,fetchErc20BalanceAndAllowance.ts, andutils.tsto encapsulate contract ABI definitions, transaction creation, multicall for balances/allowances, and network/chain utilities. [1] [2] [3] [4]EvmChainIdinstead ofEthereumChainId, and refactored related constants and functions for chain/network mapping and Alchemy URLs. [1] [2]Wallet integration and event handling improvements:
UtilsWalletsusage and improving error handling. [1] [2] [3]onBalanceFetchedevent prop to theProviderscomponent and passed it through the provider hierarchy for improved event handling. [1] [2]Bridge and token data enhancements:
injectivePeggyBridgeAddressfor the current network. [1] [2]UI and metadata updates:
index.htmlfile: improved meta tags, font loading, and minor CSS tweaks for dark mode card backgrounds. [1] [2] [3]App.tsxfor more consistent styling.0.0.8to0.0.12inpackage.json.These changes modernize the codebase, improve maintainability, and lay the groundwork for more robust EVM and wallet interactions.
Summary by CodeRabbit
New Features
UI/Style Updates
Chores