-
Notifications
You must be signed in to change notification settings - Fork 20
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
Feature/EVM txs logs #1257
base: develop
Are you sure you want to change the base?
Feature/EVM txs logs #1257
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
7 Skipped Deployments
|
WalkthroughThis pull request reorders entries in the changelog and introduces several UI and functional updates. Non-EVM components receive enhancements such as additional layout properties and responsive styling adjustments. In the EVM transaction details section, the code now fetches verification information and conditionally renders event logs. New components have been added to parse and display Ethereum event logs, including topics and decoded data, while type definitions and utility functions have been updated to support a more structured, type-safe approach. Changes
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-import". (The package "eslint-plugin-import" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-import" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (13)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx (2)
10-12
: Avoid usingany
type for better type safetyThe
decode
prop is typed asany
with an eslint disable comment. While this might be necessary for compatibility with external libraries or complex decoding scenarios, it weakens type safety.Consider creating a more specific type or interface for the decoded data if possible:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - decode: any; + decode: unknown; // as a minimum improvementOr even better, if the shape of decoded data is known:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - decode: any; + decode: { + toString: () => string; + [key: string]: unknown; + };
30-31
: Consider adding explicit error handling for invalid tuple dataWhen handling tuple data with
JSON.stringify
, there's a potential for errors if the data isn't serializable.- return <TextReadOnly text={jsonPrettify(JSON.stringify(decode))} />; + try { + return <TextReadOnly text={jsonPrettify(JSON.stringify(decode))} />; + } catch (error) { + return <Text color="error.main">Error: Unable to parse tuple data</Text>; + }src/lib/utils/evm.ts (1)
183-190
: Consider returning more detailed error informationWhile returning
undefined
on error is clean, it may hide important debugging information when trying to understand why a log couldn't be parsed.export const evmParseLog = (abi: JsonFragment[], log: TxReceiptJsonRpcLog) => { const iface = new Interface(abi); try { return iface.parseLog(log); - } catch { - return undefined; + } catch (error) { + console.debug(`Failed to parse log: ${error instanceof Error ? error.message : String(error)}`); + return undefined; } };src/lib/pages/evm-tx-details/components/EvmTxMsgDetails.tsx (1)
54-60
: Consider adding a fallback or loading stateWhen verification data is being loaded, or if it fails to load, there's no explicit handling of this state in the component.
Consider adding a loading indicator or fallback:
{evmTxData.txReceipt.logs.map((log) => ( + <> <EvmEventBox key={log.logIndex.toString()} log={log} evmVerifyInfo={data?.[log.address] ?? null} /> + {!data && <Text fontSize="sm" color="text.subtle">Loading verification data...</Text>} + </> ))}src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx (1)
52-52
: Improve readability by extracting complex logic to variablesThe code uses complex slicing and indexing logic inline, which can be hard to understand and maintain. Extracting this logic to a variable would make the code more readable.
- decode={parsedLog.args.slice(0, topics.length - 1)[index]} + decode={parsedLog.args[index]}Alternatively, consider extracting the sliced array to a variable before mapping:
+ {parsedLog?.fragment.inputs + .slice(0, topics.length - 1) + .map((input, index) => { + const decodedArgs = parsedLog.args.slice(0, topics.length - 1); + return ( + <EvmEventBoxDecoded + key={topics[index + 1]} + index={index} + input={input} + decode={decodedArgs[index]} + /> + ); + })}src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx (3)
19-20
: Improve address detection logicThe current approach for detecting Ethereum addresses by counting leading zeros might not be reliable. Ethereum addresses are 20 bytes (40 hex characters) plus the "0x" prefix.
- const count0x = text.match(/^0+/)?.[0].length || 0; - const isAddress = count0x >= 24; + // Ethereum addresses are 20 bytes (40 hex characters) + // Check if the string could be a valid Ethereum address after trimming leading zeros + const isAddress = text.length === 40 || (text.length > 40 && text.match(/^0+/)?.[0].length + 40 >= text.length);
126-128
: Add fallback for null match resultThe
.match()
function could returnnull
if there's no match, and even though optional chaining is used for the.map()
call, it would be better to provide a fallback to avoid rendering nothing.- ?.map((d, index) => ( - <EvmEventBoxDataBody text={d} key={`${d}-${index}`} /> - ))} + ?.map((d, index) => ( + <EvmEventBoxDataBody text={d} key={`${d}-${index}`} /> + )) || <Text variant="body2">No data chunks found</Text>}
152-152
: Improve readability by extracting complex slicing logicSimilar to the previous file, complex slicing logic is used inline which can be hard to understand and maintain. Extracting this logic to a variable would make the code more readable.
- decode={parsedLog.args.slice(topics.length - 1)[index]} + decode={parsedLog.args[topics.length - 1 + index]}Or better:
+ {parsedLog?.fragment.inputs + .slice(topics.length - 1) + .map((input, index) => { + const decodedArgs = parsedLog.args.slice(topics.length - 1); + return ( + <EvmEventBoxDecoded + key={input.name} + input={input} + decode={decodedArgs[index]} + /> + ); + })}src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx (5)
22-22
: Store parsed log in a useMemo for performance optimizationThe
evmParseLog
function is called on every render, which could be expensive if the ABI is large. Consider usinguseMemo
to only recompute when dependencies change.import { Flex, Stack, Text } from "@chakra-ui/react"; -import { Fragment, useState } from "react"; +import { Fragment, useState, useMemo } from "react"; import { ExplorerLink } from "lib/components/ExplorerLink"; // ... other imports export const EvmEventBox = ({ log, evmVerifyInfo }: EvmEventBoxProps) => { const [currentTab, setCurrentTab] = useState(EvmEventBoxTabs.Hex); - const parsedLog = evmParseLog(evmVerifyInfo?.abi ?? [], log); + const parsedLog = useMemo(() => evmParseLog(evmVerifyInfo?.abi ?? [], log), [evmVerifyInfo?.abi, log]);
24-47
: Simplify thehandleGenerateLogName
functionThe function creates complex JSX directly instead of building a simpler structure. Consider simplifying this by breaking it down or using a more structured approach.
const handleGenerateLogName = () => { if (!parsedLog) return ""; const { fragment } = parsedLog; - return ( - <> - {fragment.name}( - {fragment.inputs.map((input, index) => ( - <Fragment key={input.name}> - <Text as="span" color="success.main"> - {input.type} - </Text>{" "} - <Text as="span" color="text.dark"> - {input.indexed ? "indexed " : ""} - </Text> - <Text as="span" color="warning.main"> - {input.name} - </Text> - {index < fragment.inputs.length - 1 ? ", " : ""} - </Fragment> - ))} - ) - </> - ); + return ( + <> + {fragment.name}( + {fragment.inputs.map((input, index) => ( + <Fragment key={input.name || index}> + <Text as="span" color="success.main">{input.type}</Text> + {input.indexed && <Text as="span" color="text.dark"> indexed </Text>} + <Text as="span" color="warning.main">{input.name}</Text> + {index < fragment.inputs.length - 1 && ", "} + </Fragment> + ))} + ) + </> + ); };
95-101
: Consolidate verification icon rendering logicThe verification icon rendering logic is separate from the other verification-dependent elements. Consider consolidating the verification-dependent UI elements for better maintainability.
You can extract the verification icon to a constant and reuse it:
+ const verificationIcon = evmVerifyInfo?.isVerified && ( + <CustomIcon + name="verification-solid" + boxSize={4} + color="secondary.main" + /> + ); return ( // ... <ExplorerLink textFormat="normal" type="evm_contract_address" value={log.address} wordBreak="break-all" /> - {evmVerifyInfo?.isVerified && ( - <CustomIcon - name="verification-solid" - boxSize={4} - color="secondary.main" - /> - )} + {verificationIcon} </Flex>
103-113
: Improve conditional rendering logicThe component checks both
evmVerifyInfo?.isVerified
andparsedLog
before rendering the name section. This could be simplified and made more consistent with other parts of the component.- {evmVerifyInfo?.isVerified && parsedLog && ( + {evmVerifyInfo?.isVerified && parsedLog ? ( <LabelText label="Name" flexDirection={{ base: "column", md: "row" }} gap={{ base: 1, md: 4 }} minWidth="120px" alignItems="flex-start" > <Text variant="body2">{handleGenerateLogName()}</Text> </LabelText> - )} + ) : null}
151-155
: Add error boundary for data renderingThe
EvmEventBoxData
component processes and renders complex data. Consider adding error boundaries to gracefully handle any rendering errors.While React 18 doesn't require explicit error boundaries for each component, you could add error handling to ensure the UI doesn't break if there's an issue with the data:
import { Flex, Stack, Text } from "@chakra-ui/react"; -import { Fragment, useState } from "react"; +import { Fragment, useState, ErrorBoundary } from "react"; // ... other imports // Add this component somewhere in your file +const FallbackComponent = ({ error }) => ( + <Text color="error.main">Error displaying data: {error.message}</Text> +); // Then wrap the data rendering with error handling <LabelText label="Data" flexDirection={{ base: "column", md: "row" }} gap={{ base: 1, md: 4 }} minWidth="120px" alignItems="flex-start" > + <ErrorBoundary FallbackComponent={FallbackComponent}> <EvmEventBoxData data={log.data} topics={log.topics} parsedLog={parsedLog} tab={currentTab} /> + </ErrorBoundary> </LabelText>If you're not using a library with ErrorBoundary component, you could implement a simple try-catch wrapper or use a library like
react-error-boundary
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.md
(1 hunks)src/lib/components/LabelText.tsx
(2 hunks)src/lib/components/TypeSwitch.tsx
(1 hunks)src/lib/components/forms/SelectInput.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/EvmTxMsgDetails.tsx
(2 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx
(1 hunks)src/lib/pages/evm-tx-details/types.ts
(1 hunks)src/lib/services/types/tx.ts
(2 hunks)src/lib/utils/evm.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
[error] 44-54: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 37-37: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
[error] 145-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (13)
src/lib/pages/evm-tx-details/types.ts (1)
5-8
: Well-defined enum for tab selection.The
EvmEventBoxTabs
enum is properly defined with clear, descriptive members that will help maintain type safety when working with tab selection in the EVM event box components.src/lib/components/forms/SelectInput.tsx (1)
121-121
: Good improvement to font size responsiveness.Making the font size conditional based on the
size
prop enhances the component's flexibility and provides better visual consistency when the component is used in different contexts.src/lib/components/TypeSwitch.tsx (1)
40-40
: Good layout improvement with height="fit-content".This change allows the Flex container to adapt its height based on its content rather than using a fixed height, which improves layout flexibility and prevents potential overflow issues.
src/lib/components/LabelText.tsx (2)
15-15
: Good addition of minWidth property to interface.Adding the optional
minWidth
property to theLabelTextProps
interface enhances the component's flexibility for layout control.
27-27
: Properly implemented minWidth property.The
minWidth
property is correctly received as a parameter and applied to the Flex component, allowing consumers of this component to control minimum width constraints.Also applies to: 31-31
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx (2)
14-44
: Good component structure with conditional rendering based on data typeThe component efficiently handles different data types with appropriate rendering strategies, making it flexible and reusable across different event data scenarios.
46-67
: Well-structured component with good layout choicesThe grid layout and conditional rendering of the index work well for displaying hierarchical event data. The component is also properly typed with a clear interface.
src/lib/utils/evm.ts (1)
183-190
: Good error handling in the evmParseLog functionThe function cleanly handles parsing errors by returning undefined when the log can't be parsed, which prevents crashes in the UI layer.
src/lib/services/types/tx.ts (2)
466-477
: Well-structured Zod schema for EVM transaction logsThe schema is comprehensive and includes all necessary fields for EVM transaction logs. Good type exports make this reusable across the codebase.
488-488
: Proper integration of the new schema into the parent schemaUpdating the
logs
field to use the structured schema improves type safety throughout the application, especially when accessing log properties.src/lib/pages/evm-tx-details/components/EvmTxMsgDetails.tsx (3)
25-27
: Efficient data fetching with useEvmVerifyInfos hookThe hook correctly maps over log addresses to fetch verification information for all relevant contracts in a single request, which is efficient.
47-63
: Well-implemented conditional rendering of event logsThe component only renders the event logs section when logs exist, using proper React patterns with fragments and conditional rendering.
51-51
: Good UX with pluralized labelsUsing the
plur
library to correctly pluralize "Event Log" based on count is a nice attention to detail for user experience.
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
Show resolved
Hide resolved
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 (6)
src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx (3)
24-47
: Consider extracting the log name formatting to a utility function.The
handleGenerateLogName
function contains complex JSX formatting logic that might be reused elsewhere. Consider moving this to a separate utility function to improve reusability and make the component more focused on rendering.- const handleGenerateLogName = () => { - if (!parsedLog) return ""; - const { fragment } = parsedLog; - return ( - <> - {fragment.name}( - {fragment.inputs.map((input, index) => ( - <Fragment key={input.name}> - <Text as="span" color="success.main"> - {input.type} - </Text>{" "} - <Text as="span" color="text.dark"> - {input.indexed ? "indexed " : ""} - </Text> - <Text as="span" color="warning.main"> - {input.name} - </Text> - {index < fragment.inputs.length - 1 ? ", " : ""} - </Fragment> - ))} - ) - </> - ); - };This could be moved to a shared utility file like
src/lib/utils/evm-formatting.tsx
and imported where needed.
103-113
: Use optional chaining for safer property access.The code could benefit from optional chaining when accessing properties of parsedLog to prevent potential runtime errors.
- {evmVerifyInfo?.isVerified && parsedLog && ( + {evmVerifyInfo?.isVerified && parsedLog?.fragment && ( <LabelText label="Name" flexDirection={{ base: "column", md: "row" }} gap={{ base: 1, md: 4 }} minWidth="120px" alignItems="flex-start" > <Text variant="body2">{handleGenerateLogName()}</Text> </LabelText> )}
116-128
: Improve tooltip experience for non-verified contracts.The tooltip is correctly hidden for verified contracts, but consider adding more guidance in the tooltip to help users understand how to verify a contract or where to go for verification.
<Tooltip hidden={!!evmVerifyInfo?.isVerified} maxWidth="200px" - label="Verify the contract to enable decoded" + label="Verify the contract to enable decoded view. Visit the contract page for verification options." >src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx (3)
18-21
: Improve address detection logic for more reliability.The current method of detecting addresses by counting leading zeros may produce false positives or negatives. Consider using a more robust approach like checking if the string matches an Ethereum address pattern.
- const count0x = text.match(/^0+/)?.[0].length || 0; - const isAddress = count0x >= 24; + // More reliable Ethereum address detection - check if it's a valid hex string of correct length + const isAddress = text.length === 40 && /^[0-9a-fA-F]{40}$/.test(text);
126-128
: Handle potential undefined value from regex match.The
match
method could returnnull
if there are no matches, and although you're using the optional chaining operator, it's good to provide a fallback for the entire expression.- .match(/.{1,64}/g) - ?.map((d, index) => ( - <EvmEventBoxDataBody text={d} key={`${d}-${index}`} /> - ))} + .match(/.{1,64}/g) + ?.map((d, index) => ( + <EvmEventBoxDataBody text={d} key={`${d}-${index}`} /> + )) || <Text variant="body2">No data to display</Text>}
145-154
: Add bounds checking for slicing operation and handle edge cases.When slicing arrays based on dynamic values (
topics.length - 1
), ensure the resulting index is valid and handle edge cases where topics might be empty.- {parsedLog && - parsedLog.fragment.inputs - .slice(topics.length - 1) - .map((input, index) => ( - <EvmEventBoxDecoded - key={input.name} - input={input} - decode={parsedLog.args.slice(topics.length - 1)[index]} - /> - ))} + {parsedLog && topics.length > 0 && + parsedLog.fragment.inputs + .slice(Math.max(0, topics.length - 1)) + .map((input, index) => { + const argsIndex = Math.min(index, (parsedLog.args.slice(Math.max(0, topics.length - 1)).length - 1)); + return ( + <EvmEventBoxDecoded + key={input.name} + input={input} + decode={parsedLog.args.slice(Math.max(0, topics.length - 1))[argsIndex]} + /> + ); + })}🧰 Tools
🪛 Biome (1.9.4)
[error] 145-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/components/TypeSwitch.tsx
(2 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/components/TypeSwitch.tsx
- src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
[error] 145-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx (5)
1-14
: Imports and structure look well-organized.The component imports are logically grouped and include all necessary dependencies for the implementation.
15-18
: Props interface is properly typed.The props interface is clearly defined with appropriate types for both the log data and verification information.
20-23
: Good initialization of state and data parsing.The component correctly initializes state with a default tab and uses the utility function to parse the log data. The parsedLog variable will be null when evmVerifyInfo is null or has an empty ABI array.
49-158
: Good responsive design implementation with appropriate Chakra UI components.The component effectively uses Chakra UI's responsive props to create a layout that adapts well to different screen sizes. The use of stack and flex containers with responsive direction and gap values provides a clean and consistent layout.
137-143
:❓ Verification inconclusive
Ensure proper handling of missing or malformed topic data.
The component passes topics directly to EvmEventBoxTopics without validating that they exist and are in the expected format.
🏁 Script executed:
#!/bin/bash # Check for potential issues with topic data handling in related components # Look for null/undefined checks in EvmEventBoxTopics rg -A 5 "EvmEventBoxTopics" --type typescriptLength of output: 80
The previous command failed because the file type "typescript" wasn’t recognized. Please re-run the above script to inspect whether the EvmEventBoxTopics component is verifying that the passed
topics
data exists and conforms to the expected format before usage. This check is crucial to prevent potential runtime errors due to missing or malformed topic data.Once you confirm whether additional validations are needed, update the component accordingly. If the component already contains the necessary safeguards, kindly document this to clarify that the data is being handled appropriately.
Action: Verify handling of topic data in EvmEventBoxTopics and add validations if they’re missing.
🏁 Script executed:
#!/bin/bash # Re-run the search using the correct file type for TSX files rg -A 15 "EvmEventBoxTopics" --type tsxLength of output: 67
Action: Validate Topic Data Prop for Robustness
It appears that the current implementation passes the
topics
prop toEvmEventBoxTopics
without any inline validation, which could lead to issues if the data is missing or malformed. To ensure robust error handling and prevent potential runtime issues, please verify that:
- Input Check in Caller: In
src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx
, confirm that thetopics
variable (derived fromlog.topics
) is defined and adheres to the expected format before it is passed down.- Validation in
EvmEventBoxTopics
: Review the implementation ofEvmEventBoxTopics
(likely within a nearby TSX file) to see if it enforces any checks on thetopics
prop. If no validation exists, consider adding safeguards such as checking for null/undefined values or verifying the data type.If these validations are missing in one or both locations, please add proper defensive checks (or defaults) so that the component behaves gracefully in cases of missing or invalid topic data.
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx (5)
46-57
: Add error handling for text decoding.When decoding hex to text, if the input contains invalid hex characters or doesn't represent valid text, it could throw an exception or produce unexpected output. Add try-catch to handle potential errors.
- case "text": - return Buffer.from(text, "hex").toString("binary"); + case "text": + try { + return Buffer.from(text, "hex").toString("binary"); + } catch (error) { + console.error("Failed to decode hex to text:", error); + return `[Invalid text: ${text}]`; + }
97-104
: Good state management for formatting toggle.The component correctly uses useState to manage the formatting preference with a sensible default value of false.
105-142
: Good conditional rendering and formatting options for data display.The implementation provides a good user experience by offering both raw and formatted views of the data, with a clear toggle mechanism.
144-154
: Use optional chaining to prevent potential runtime errors.The code accesses
parsedLog.fragment.inputs
without properly verifying thatparsedLog
exists first. Using optional chaining would prevent potential runtime errors.- {parsedLog && - parsedLog.fragment.inputs - .slice(topics.length - 1) - .map((input, index) => ( - <EvmEventBoxDecoded - key={input.name} - input={input} - decode={parsedLog.args.slice(topics.length - 1)[index]} - /> - ))} + {parsedLog?.fragment.inputs + .slice(topics.length - 1) + .map((input, index) => ( + <EvmEventBoxDecoded + key={input.name} + input={input} + decode={parsedLog.args.slice(topics.length - 1)[index]} + /> + ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 145-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
81-88
:✅ Verification successful
Use the proper explorer link type for EVM addresses.
The current implementation uses
user_address
as the type for the ExplorerLink component. Verify if this is the correct type or ifevm_contract_address
should be used instead for consistency with other parts of the application.
🏁 Script executed:
#!/bin/bash # Check how address types are handled in ExplorerLink throughout the codebase # Find usage patterns of ExplorerLink with address types rg "ExplorerLink.*?type=\"(user_address|evm_contract_address)\"" --type typescriptLength of output: 117
I'll run a revised shell script using a file glob filter (i.e. searching only in TSX files) to verify how the
ExplorerLink
component is used with thetype
property throughout the codebase:Once we have the results from this updated search, we can determine whether the existing
user_address
type is consistent with other parts of the application or if we need to change it toevm_contract_address
.
🏁 Script executed:
#!/bin/bash # Search for ExplorerLink usages with address type in TSX files rg 'ExplorerLink.*?type="(user_address|evm_contract_address)"' -g "*.tsx"Length of output: 602
Address Linking Consistency Verified
After reviewing ExplorerLink usages across the repository, all instances (e.g., in SavedAccountModalHeader.tsx, ModulesTableRow.tsx, ModuleInfoBody.tsx, and AttributesModal.tsx) use the type
"user_address"
. This indicates that using"user_address"
in evm-event-box-data.tsx is consistent with the rest of the codebase. No change to"evm_contract_address"
is required.
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 (6)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx (3)
12-21
: Remove unnecessary key prop from component definitionThe
key={topic}
prop in theEvmEventBoxTopicHex
component definition is not needed. Thekey
prop is only required when creating elements in arrays/iterations, not in component definitions.const EvmEventBoxTopicHex = ({ topic, index }: EvmEventBoxTopicProps) => ( - <Flex gap={2} key={topic} alignItems="center"> + <Flex gap={2} alignItems="center"> <Text variant="body2" fontFamily="mono"> [{index}] </Text> <Text variant="body2" fontFamily="mono"> {topic} </Text> </Flex> );
35-38
: Using topic as key may lead to issues with duplicate valuesUsing
topic
as the key property might lead to issues if there are duplicate topics. React requires keys to be unique among siblings.{tab === EvmEventBoxTabs.Hex ? ( topics.map((topic, index) => ( - <EvmEventBoxTopicHex topic={topic} key={topic} index={index} /> + <EvmEventBoxTopicHex topic={topic} key={`topic-${index}-${topic.substring(0, 8)}`} index={index} /> ))
55-55
: Improve key uniqueness in EvmEventBoxDecoded listThe current key uses
topics[index]
which may not properly align with the input mapping and could lead to React reconciliation issues. Using a combination of index and input name would be more unique and reliable.- key={topics[index]} + key={`decoded-${index}-${input.name || index}`}src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx (3)
18-21
: Improve address detection logicThe current method for detecting if the text is an address (counting leading zeros) may not be reliable. Ethereum addresses are 20 bytes (40 hex characters) long and typically start with "0x". A more reliable approach would be to check the length after any potential "0x" prefix.
- const count0x = text.match(/^0+/)?.[0].length || 0; - const isAddress = count0x >= 24; + const isAddress = text.length === 40 || (text.startsWith("0x") && text.length === 42);
74-77
: Simplify SelectInput value handlingThe current implementation finds the option where
optionValue === value
, which works but could be simplified for better readability and performance.- value={options.find( - ({ value: optionValue }) => optionValue === value - )} + value={options.find(option => option.value === value)}
157-157
: Add fallback for potentially undefined input namesUsing
input.name
as a key could be problematic if the name is undefined. Add a fallback using the index to ensure key uniqueness.- key={input.name} + key={input.name || `input-${index}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
[error] 152-161: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
[error] 50-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx (1)
50-60
: Use optional chaining to prevent potential runtime errorsThe code accesses
parsedLog.fragment.inputs
without verifying thatparsedLog
is not null first. IfparsedLog
is null, this will cause a runtime error. Using optional chaining would prevent this issue.- {parsedLog && - parsedLog.fragment.inputs - .slice(0, topics.length - 1) - .map((input, index) => ( - <EvmEventBoxDecoded - key={topics[index]} - index={index} - input={input} - decode={parsedLog.args.slice(0, topics.length - 1)[index]} - /> - ))} + {parsedLog?.fragment.inputs + .slice(0, topics.length - 1) + .map((input, index) => ( + <EvmEventBoxDecoded + key={`decoded-${index}-${input.name || index}`} + index={index} + input={input} + decode={parsedLog.args.slice(0, topics.length - 1)[index]} + /> + ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 50-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx (2)
46-57
: Add error handling for text decodingWhen decoding hex to text, if the input contains invalid hex characters or doesn't represent valid text, it could throw an exception or produce unexpected output. Add try-catch to handle potential errors.
const handleDecodeText = () => { switch (value) { case "address": return "0x" + text.slice(-40); case "number": return parseInt(text, 16); case "text": - return Buffer.from(text, "hex").toString("binary"); + try { + return Buffer.from(text, "hex").toString("binary"); + } catch (error) { + console.error("Failed to decode hex to text:", error); + return `[Invalid text: ${text}]`; + } default: return text; } };
152-161
: Use optional chaining to prevent potential runtime errorsSimilar to the issue in the first file, the code accesses
parsedLog.fragment.inputs
without properly verifying thatparsedLog
is not null first. Using optional chaining would prevent potential runtime errors.- {parsedLog && - parsedLog.fragment.inputs - .slice(topics.length - 1) - .map((input, index) => ( - <EvmEventBoxDecoded - key={input.name} - input={input} - decode={parsedLog.args.slice(topics.length - 1)[index]} - /> - ))} + {parsedLog?.fragment.inputs + .slice(topics.length - 1) + .map((input, index) => ( + <EvmEventBoxDecoded + key={input.name || `input-${index}`} + input={input} + decode={parsedLog.args.slice(topics.length - 1)[index]} + /> + ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 152-161: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
Show resolved
Hide resolved
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 (1)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx (1)
37-37
: Consider ensuring uniqueness of thekey
prop.Using
key={topic}
may cause unexpected rendering behaviors if multiple items share the sametopic
. For stability, you might incorporate both the topic string and index, for example,key={\
topic-${index}`}`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/components/TypeSwitch.tsx
(2 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
(1 hunks)src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx
(1 hunks)src/lib/utils/evm.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/components/TypeSwitch.tsx
- src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-decoded.tsx
- src/lib/utils/evm.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx
[error] 150-159: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx
[error] 50-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-topics.tsx (1)
50-60
: Use optional chaining for safer access toparsedLog.fragment.inputs
.Switching to
parsedLog?.fragment?.inputs
will reduce potential runtime errors and is more succinct than combiningparsedLog && parsedLog.fragment
.🧰 Tools
🪛 Biome (1.9.4)
[error] 50-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/lib/pages/evm-tx-details/components/evm-event-box/index.tsx (1)
158-163
: Validate or documentlog.data
guarantees.If there's a possibility
log.data
could be invalid or non-hex, this might cause issues inEvmEventBoxData
. Consider adding explicit validation or clarifying thatlog.data
is always valid hex.src/lib/pages/evm-tx-details/components/evm-event-box/evm-event-box-data.tsx (2)
53-53
: Add try/catch for hex-to-text decoding.When decoding hex data to text, an invalid string may throw an error. Consider wrapping this operation in a try/catch to avoid unexpected runtime errors.
150-159
: Adopt optional chaining to avoid null checks with&&
.Using
parsedLog?.fragment?.inputs
is cleaner and removes the risk of undefined references insideparsedLog
.🧰 Tools
🪛 Biome (1.9.4)
[error] 150-159: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Summary by CodeRabbit
New Features
Style