-
Notifications
You must be signed in to change notification settings - Fork 107
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
feat(spaceward): fix sending cosmos not using the same rpc as in assets fetch data; fix handling collapsed transactions #892
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||||||
import clsx from "clsx"; | ||||||||||
import { useContext, useEffect } from "react"; | ||||||||||
import { useContext, useEffect, useRef, useState } from "react"; | ||||||||||
import { isDeliverTxSuccess } from "@cosmjs/stargate"; | ||||||||||
import { useChain, walletContext } from "@cosmos-kit/react-lite"; | ||||||||||
import { cosmos, warden } from "@wardenprotocol/wardenjs"; | ||||||||||
|
@@ -20,7 +20,9 @@ import { QueuedAction, QueuedActionStatus, useActionsState } from "./hooks"; | |||||||||
import { getActionHandler, GetStatus, handleCosmos, handleEth, handleEthRaw } from "./util"; | ||||||||||
import { TEMP_KEY, useKeySettingsState } from "../keys/state"; | ||||||||||
import Assets from "../keys/assets"; | ||||||||||
import { useQueryClient } from "@tanstack/react-query"; | ||||||||||
import { useQuery, useQueryClient } from "@tanstack/react-query"; | ||||||||||
import { capitalize } from "../modals/util"; | ||||||||||
import { queryCosmosClients } from "../assets/queries"; | ||||||||||
|
||||||||||
interface ItemProps extends QueuedAction { | ||||||||||
single?: boolean; | ||||||||||
|
@@ -46,6 +48,10 @@ const waitForVisibility = () => { | |||||||||
function ActionItem({ single, ...item }: ItemProps) { | ||||||||||
const queryClient = useQueryClient(); | ||||||||||
const { walletManager } = useContext(walletContext); | ||||||||||
const cosmosClients = useQuery(queryCosmosClients(walletManager)).data; | ||||||||||
const clientsRef = useRef(cosmosClients); | ||||||||||
clientsRef.current = cosmosClients; | ||||||||||
Comment on lines
+51
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Unnecessary use of The Suggested refactor: Remove the - const clientsRef = useRef(cosmosClients);
- clientsRef.current = cosmosClients; And update the usage accordingly: - const [, , rpcEndpoint] = clientsRef.current?.find((v) => v[1] === item?.chainName) ?? [];
+ const [, , rpcEndpoint] = cosmosClients?.find((v) => v[1] === item?.chainName) ?? []; This simplifies the code and avoids unnecessary complexity. Committable suggestion
Suggested change
|
||||||||||
|
||||||||||
const { data: ks, setData: setKeySettings } = useKeySettingsState(); | ||||||||||
const { toast } = useToast() | ||||||||||
const { w } = useWeb3Wallet("wss://relay.walletconnect.org"); | ||||||||||
|
@@ -311,7 +317,8 @@ function ActionItem({ single, ...item }: ItemProps) { | |||||||||
} else if (item.networkType === "eth") { | ||||||||||
res = await handleEth({ action: item, w, queryClient }); | ||||||||||
} else if (item.networkType === "cosmos") { | ||||||||||
res = await handleCosmos({ action: item, w, walletManager, queryClient }); | ||||||||||
const [, , rpcEndpoint] = clientsRef.current?.find((v) => v[1] === item?.chainName) ?? []; | ||||||||||
res = await handleCosmos({ action: item, w, queryClient, rpcEndpoint }); | ||||||||||
Comment on lines
+320
to
+321
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Fragile array destructuring when extracting Destructuring with positional indexes can lead to maintenance issues if the structure of the array changes. Relying on specific indices makes the code less readable and more error-prone. Suggested refactor: Consider restructuring your data to use objects instead of arrays, allowing for property-based access which is more robust. If modifying the data structure is not feasible, you can enhance readability by destructuring with meaningful variable names: - const [, , rpcEndpoint] = cosmosClients?.find((v) => v[1] === item?.chainName) ?? [];
+ const clientEntry = cosmosClients?.find((v) => v[1] === item?.chainName) ?? [];
+ const rpcEndpoint = clientEntry ? clientEntry[2] : undefined; Or, if possible, adjust the data to be an array of objects: // Example structure
[
{ key: ..., chainName: ..., rpcEndpoint: ... },
// ...
] And then use: const clientEntry = cosmosClients?.find((v) => v.chainName === item?.chainName);
const rpcEndpoint = clientEntry?.rpcEndpoint; This makes the code clearer and less dependent on the array structure. |
||||||||||
} | ||||||||||
} catch (e) { | ||||||||||
console.error("broadcast failed", e); | ||||||||||
|
@@ -407,7 +414,7 @@ function ActionItem({ single, ...item }: ItemProps) { | |||||||||
? "Action ready" | ||||||||||
: item.status === | ||||||||||
QueuedActionStatus.AwaitingBroadcast | ||||||||||
? "Awaiting broadcast" | ||||||||||
? `Awaiting broadcast on ${capitalize(item.chainName)}` | ||||||||||
: item.status === QueuedActionStatus.Success | ||||||||||
? "Success" | ||||||||||
: item.status === | ||||||||||
|
@@ -437,6 +444,7 @@ function ActionItem({ single, ...item }: ItemProps) { | |||||||||
} | ||||||||||
|
||||||||||
export default function StatusSidebar() { | ||||||||||
const [hide, setHide] = useState(true); | ||||||||||
const { data } = useActionsState(); | ||||||||||
const storeIds = Object.keys(data ?? {}); | ||||||||||
|
||||||||||
|
@@ -469,7 +477,9 @@ export default function StatusSidebar() { | |||||||||
<ActionItem single {...data?.[filtered[0]]!} /> | ||||||||||
) : null | ||||||||||
) : ( | ||||||||||
<Popover> | ||||||||||
<Popover onOpenChange={open => { | ||||||||||
setHide(!open); | ||||||||||
}}> | ||||||||||
Comment on lines
+480
to
+482
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent visibility logic for The logic controlling the visibility and mounting of
This could result in the Suggested fix: Simplify the state management by using the - const [hide, setHide] = useState(true);
+ const [open, setOpen] = useState(false);
...
<Popover
- onOpenChange={open => {
- setHide(!open);
- }}
+ open={open}
+ onOpenChange={setOpen}
> Update the <PopoverContent
side="left"
sideOffset={20}
- className={clsx("p-0", { hidden: hide })}
- forceMount={hide ? true : undefined}
+ className="p-0"
+ forceMount
> This ensures that:
Also applies to: 497-498 |
||||||||||
<PopoverTrigger asChild> | ||||||||||
<div className="flex flex-col relative cursor-pointer"> | ||||||||||
<p className="text-lg font-semibold"> | ||||||||||
|
@@ -484,9 +494,10 @@ export default function StatusSidebar() { | |||||||||
<PopoverContent | ||||||||||
side="left" | ||||||||||
sideOffset={20} | ||||||||||
className="p-0" | ||||||||||
className={clsx("p-0", { hidden: hide })} | ||||||||||
forceMount={hide ? true : undefined} | ||||||||||
> | ||||||||||
<div className="bg-fill-quaternary max-h-80 overflow-auto"> | ||||||||||
<div className={"bg-fill-quaternary max-h-80 overflow-auto"}> | ||||||||||
{filtered.map((id) => { | ||||||||||
const action = data?.[id]; | ||||||||||
|
||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -293,12 +293,12 @@ export const handleEth = async ({ | |
export const handleCosmos = async ({ | ||
action, | ||
w, | ||
walletManager, | ||
queryClient, | ||
rpcEndpoint, | ||
}: { | ||
action: QueuedAction; | ||
w: IWeb3Wallet | null; | ||
walletManager: WalletManager; | ||
rpcEndpoint?: string; | ||
Comment on lines
+297
to
+301
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make Since Apply this diff to make export const handleCosmos = async ({
action,
w,
queryClient,
rpcEndpoint,
}: {
action: QueuedAction;
w: IWeb3Wallet | null;
- rpcEndpoint?: string;
+ rpcEndpoint: string;
queryClient: QueryClient;
}) => {
|
||
queryClient: QueryClient; | ||
}) => { | ||
const { | ||
|
@@ -384,22 +384,11 @@ export const handleCosmos = async ({ | |
signatures: [sig], | ||
}); | ||
|
||
let rpc: string; | ||
if (chain.rpc) { | ||
[rpc] = chain.rpc; | ||
} else { | ||
const repo = walletManager.getWalletRepo(chainName); | ||
repo.activate(); | ||
const endpoint = await repo.getRpcEndpoint(); | ||
|
||
rpc = endpoint | ||
? typeof endpoint === "string" | ||
? endpoint | ||
: endpoint.url | ||
: "https://rpc.cosmos.directory/" + chainName; | ||
if (!rpcEndpoint) { | ||
throw new Error("missing rpcEndpoint"); | ||
} | ||
|
||
const client = await StargateClient.connect(rpc); | ||
const client = await StargateClient.connect(rpcEndpoint); | ||
|
||
const res = await client.broadcastTx( | ||
cosmos.tx.v1beta1.TxRaw.encode(txRaw).finish(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,7 +199,7 @@ const eip155NativeBalanceQuery = ({ | |
throw new Error("Address is required"); | ||
} | ||
|
||
const { provider, token } = getProvider(chainName); | ||
const { provider, tokenSymbol: token } = getProvider(chainName); | ||
const slinkyPrice = prices?.[token]; | ||
const priceFeed = EIP_155_NATIVE_PRICE_FEEDS[chainName]; | ||
|
||
|
@@ -427,7 +427,7 @@ const DEFAULT_BECH32_PREFIX = getCosmosChain("osmosis")!.bech32_prefix; | |
export const balancesQueryCosmos = ( | ||
enabled: boolean, | ||
keys?: QueryKeyResponse[], | ||
clients?: [CosmosQueryClient, string][], | ||
clients?: [CosmosQueryClient, string, string][], | ||
prices?: PriceMapSlinky, | ||
) => { | ||
const byAddress: Record<string, QueryKeyResponse> = {}; | ||
|
@@ -629,7 +629,10 @@ export const fiatPricesQuery = (enabled: boolean) => { | |
}; | ||
}; | ||
|
||
const rpcClients: Record<string, CosmosQueryClient | undefined> = {}; | ||
const rpcClients: Record< | ||
string, | ||
{ client: CosmosQueryClient; rpcEndpoint: string } | undefined | ||
> = {}; | ||
const rpcRetry: Record<string, number> = {}; | ||
|
||
const checkHealth = async ( | ||
|
@@ -666,18 +669,18 @@ export const queryCosmosClients = (walletManager: WalletManager) => { | |
return { | ||
queryKey: ["cosmos", "rpcClients"], | ||
queryFn: async () => { | ||
const clients: [CosmosQueryClient, string][] = []; | ||
const clients: [CosmosQueryClient, string, string][] = []; | ||
|
||
for (let i = 0; i < COSMOS_CHAINS.length; i++) { | ||
const { chainName, rpc } = COSMOS_CHAINS[i]; | ||
const retries = (rpc?.length ?? 0) + 1; | ||
|
||
for (let i = 0; i < retries; i++) { | ||
let client = rpcClients[chainName]; | ||
let { client, rpcEndpoint } = rpcClients[chainName] ?? {}; | ||
const retry = (rpcRetry[chainName] ?? 0) % (retries + 1); | ||
rpcRetry[chainName] = retry + 1; | ||
|
||
if (!client) { | ||
if (!client || !rpcEndpoint) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure proper handling when When checking Consider adjusting the condition to explicitly handle each case or adding further validation as needed. |
||
let endpoint: ExtendedHttpEndpoint | string; | ||
|
||
if (!rpc?.[retry]) { | ||
|
@@ -694,14 +697,16 @@ export const queryCosmosClients = (walletManager: WalletManager) => { | |
endpoint = rpc[retry]; | ||
} | ||
|
||
rpcEndpoint = | ||
typeof endpoint === "string" | ||
? endpoint | ||
: endpoint.url; | ||
|
||
try { | ||
client = | ||
await cosmos.ClientFactory.createRPCQueryClient( | ||
{ | ||
rpcEndpoint: | ||
typeof endpoint === "string" | ||
? endpoint | ||
: endpoint.url, | ||
rpcEndpoint, | ||
}, | ||
); | ||
} catch (e) { | ||
|
@@ -711,8 +716,8 @@ export const queryCosmosClients = (walletManager: WalletManager) => { | |
} | ||
|
||
if (await checkHealth(client, chainName)) { | ||
rpcClients[chainName] = client; | ||
clients.push([client!, chainName]); | ||
rpcClients[chainName] = { client, rpcEndpoint }; | ||
clients.push([client, chainName, rpcEndpoint]); | ||
break; | ||
} else if (rpcClients[chainName]) { | ||
delete rpcClients[chainName]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,10 @@ import { validateAddress } from "../intents/AddAddressModal"; | |
import { StargateClient } from "@cosmjs/stargate"; | ||
import { useModalState } from "./state"; | ||
import KeySelector from "./KeySelector"; | ||
import { COSMOS_CHAINS } from "@/config/tokens"; | ||
import { walletContext } from "@cosmos-kit/react-lite"; | ||
import { BalanceEntry } from "../assets/types"; | ||
import { useQuery } from "@tanstack/react-query"; | ||
import { queryCosmosClients } from "../assets/queries"; | ||
|
||
export default function SendAssetsModal({ | ||
// address, | ||
|
@@ -31,6 +32,7 @@ export default function SendAssetsModal({ | |
|
||
const { spaceId } = useSpaceId(); | ||
const { queryBalances } = useAssetQueries(spaceId); | ||
const cosmosClients = useQuery(queryCosmosClients(walletManager)).data; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue: Handle loading and error states for The Consider destructuring the result of - const cosmosClients = useQuery(queryCosmosClients(walletManager)).data;
+ const { data: cosmosClients, isLoading, error } = useQuery(queryCosmosClients(walletManager));
+ if (isLoading) {
+ // Optionally handle the loading state, e.g., disable submit button
+ }
+ if (error) {
+ // Handle the error state
+ console.error(error);
+ } Ensure that
|
||
|
||
const results: (BalanceEntry & { refetch: () => void })[] = queryBalances | ||
.filter((result) => result.data?.key?.key?.id === key?.key?.id) | ||
|
@@ -97,17 +99,12 @@ export default function SendAssetsModal({ | |
setModal({ type: undefined }); | ||
} | ||
} else if (txBuild.type === "cosmos") { | ||
const chain = COSMOS_CHAINS.find( | ||
(item) => item.chainName === chainName, | ||
); | ||
|
||
let rpc = chain?.rpc?.[0]; | ||
const [, , rpc] = cosmosClients?.find((item) => { | ||
return item[1] === chainName | ||
}) ?? []; | ||
Comment on lines
+102
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Refactor Suggestion: Improve clarity when extracting Destructuring array elements by index reduces code readability and can be error-prone. Consider refactoring to make the code clearer. Refactored code: - const [, , rpc] = cosmosClients?.find((item) => {
- return item[1] === chainName
- }) ?? [];
+ const client = cosmosClients?.find((item) => item[1] === chainName);
+ const rpc = client ? client[2] : undefined; Alternatively, if + const client = cosmosClients?.find((item) => item.chainName === chainName);
+ const rpc = client?.rpc; This refactor enhances readability by explicitly accessing the
|
||
|
||
if (!rpc) { | ||
const repo = walletManager.getWalletRepo(chainName); | ||
repo.activate(); | ||
const endpoint = await repo.getRpcEndpoint(); | ||
rpc = endpoint ? typeof endpoint === "string" ? endpoint : endpoint.url : `https://rpc.cosmos.directory/${chainName}`; | ||
throw new Error(`unable to find rpc for ${chainName}`); | ||
} | ||
|
||
const client = await StargateClient.connect(rpc); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,12 @@ | ||
// taken from https://chainlist.org | ||
export const ETH_CHAIN_CONFIG: Record< | ||
string, | ||
{ rpc: string[]; token?: string } | ||
{ rpc: string[]; token?: { symbol: string; name: string }; title?: string } | ||
> = { | ||
"1": { rpc: ["https://cloudflare-eth.com", "https://eth.llamarpc.com"] }, | ||
"1": { | ||
rpc: ["https://cloudflare-eth.com", "https://eth.llamarpc.com"], | ||
title: "Ethereum", | ||
}, | ||
Comment on lines
+6
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding the The addition of the For consistency, consider adding the "1": {
rpc: ["https://cloudflare-eth.com", "https://eth.llamarpc.com"],
title: "Ethereum",
token: { symbol: "ETH", name: "Ether" },
}, |
||
"5": { rpc: ["https://rpc.goerli.mudit.blog/"] }, | ||
"10": { | ||
rpc: ["https://mainnet.optimism.io/", "https://optimism.llamarpc.com"], | ||
|
@@ -13,30 +16,34 @@ export const ETH_CHAIN_CONFIG: Record< | |
"https://bsc-dataseed1.bnbchain.org", | ||
"https://binance.llamarpc.com", | ||
], | ||
token: "BNB", | ||
token: { symbol: "BNB", name: "Binance Coin" }, | ||
}, | ||
"61": { | ||
rpc: ["https://etc.etcdesktop.com", "https://geth-at.etc-network.info"], | ||
token: "ETC", | ||
token: { symbol: "ETC", name: "Ethereum Classic" }, | ||
title: "Ethereum Classic", | ||
}, | ||
"137": { | ||
rpc: ["https://polygon-rpc.com/", "https://polygon.llamarpc.com"], | ||
token: "MATIC", | ||
token: { symbol: "MATIC", name: "Polygon" }, | ||
}, | ||
"324": { rpc: ["https://mainnet.era.zksync.io/"] }, | ||
"420": { rpc: ["https://goerli.optimism.io"] }, | ||
"592": { | ||
rpc: ["https://astar-rpc.dwellir.com", "https://1rpc.io/astr"], | ||
token: "ASTR", | ||
token: { symbol: "ASTR", name: "Astar" }, | ||
}, | ||
"8453": { | ||
rpc: ["https://mainnet.base.org/", "https://base.llamarpc.com"], | ||
token: { symbol: "BASE", name: "Base" }, | ||
}, | ||
"8453": { rpc: ["https://mainnet.base.org/", "https://base.llamarpc.com"] }, | ||
"42161": { | ||
rpc: ["https://arb1.arbitrum.io/rpc", "https://arbitrum.llamarpc.com"], | ||
}, | ||
"42220": { rpc: ["https://forno.celo.org"] }, | ||
"43114": { | ||
rpc: ["https://api.avax.network/ext/bc/C/rpc"], | ||
token: "AVAX", | ||
token: { symbol: "AVAX", name: "Avalanche" }, | ||
}, | ||
"44787": { rpc: ["https://alfajores-forno.celo-testnet.org"] }, | ||
"80001": { rpc: ["https://rpc-mumbai.maticvigil.com"] }, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { ethers } from "ethers"; | ||
import { ETH_CHAINID_MAP, ETH_CHAIN_CONFIG } from "./constants"; | ||
import { capitalize } from "@/features/modals/util"; | ||
|
||
export const REVERSE_ETH_CHAINID_MAP = Object.fromEntries( | ||
Object.entries(ETH_CHAINID_MAP).map(([k, v]) => [v, k]), | ||
|
@@ -16,7 +17,9 @@ export const isSupportedNetwork = ( | |
export const getProvider = (type: SupportedNetwork) => { | ||
const chainId = ETH_CHAINID_MAP[type]; | ||
const config = ETH_CHAIN_CONFIG[chainId]; | ||
const token = config?.token ?? "ETH"; | ||
const tokenSymbol = config?.token?.symbol ?? "ETH"; | ||
const tokenName = config?.token?.name ?? "Ethereum"; | ||
const title = config?.title ?? capitalize(type); | ||
|
||
if (!providers[type]) { | ||
if (!chainId || !config) { | ||
|
@@ -53,7 +56,7 @@ export const getProvider = (type: SupportedNetwork) => { | |
} | ||
|
||
const provider = providers[type]; | ||
return { provider, token }; | ||
return { provider, tokenSymbol, tokenName, title }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Action Required: Update The Affected Files:
Analysis chainLGTM: Updated return value of getProvider function. The function now returns an object with additional properties ( Please ensure that all calls to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Find all usages of getProvider function
# Expected result: List of files and line numbers where getProvider is called
rg --type typescript -n 'getProvider\(' --glob '!spaceward/src/lib/eth/util.ts'
Length of output: 116 Script: #!/bin/bash
# Description: Find all usages of getProvider function
# Expected result: List of files and line numbers where getProvider is called
rg -g '*.ts' -g '*.tsx' -n 'getProvider\(' --glob '!spaceward/src/lib/eth/util.ts'
Length of output: 1014 |
||
}; | ||
|
||
export const getProviderByChainId = (chainId: string) => { | ||
|
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.
Handle loading and error states from
useQuery
The
cosmosClients
variable is directly accessed fromuseQuery(queryCosmosClients(walletManager)).data
without checking for loading or error states. This might lead to issues if the data is not immediately available.Suggested fix:
Destructure the result of
useQuery
to handle loading and error states:This ensures that your component handles all possible states and doesn't attempt to access data that may be undefined.