-
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
feat: interaction section #1208
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis pull request introduces enhancements to the application's Ethereum Virtual Machine (EVM) contract interaction capabilities. Changes include the addition of new components for reading and writing to EVM contracts, updates to type definitions, utility functions, and modifications to interfaces to accommodate new parameters. The Changes
Sequence DiagramsequenceDiagram
participant User
participant InteractEvmContract
participant AbiRead
participant AbiWrite
participant EVMContract
User->>InteractEvmContract: Select Contract
InteractEvmContract->>AbiRead: Load Read Functions
AbiRead->>EVMContract: Query Contract Data
EVMContract-->>AbiRead: Return Contract Data
AbiRead-->>User: Display Read Results
User->>InteractEvmContract: Switch to Write
InteractEvmContract->>AbiWrite: Load Write Functions
User->>AbiWrite: Input Transaction Parameters
AbiWrite->>EVMContract: Submit Transaction
EVMContract-->>AbiWrite: Transaction Response
AbiWrite-->>User: Show Transaction Result
Possibly related PRs
Suggested Reviewers
Poem
✨ Finishing Touches
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
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: 5
🧹 Nitpick comments (19)
src/lib/pages/evm-contract-details/dummy.ts (1)
5-7
: Consider externalizing the ABI for maintainabilityDefining the entire ABI in-line can make maintenance unwieldy and inflate file size. Storing the ABI in a separate JSON file or using a reference from a configuration can improve clarity and manageability.
src/lib/pages/evm-contract-details/utils.ts (1)
3-7
: Use braces for clarity in conditional statementsWhile a single-line
if
is syntactically valid, wrapping the conditions in braces can improve clarity and reduce the risk of accidental bugs when future lines are added.export const getInteractTabsIndex = (isRead: boolean, isAsProxy: boolean) => { - if (isRead) - return isAsProxy ? InteractTabsIndex.ReadProxy : InteractTabsIndex.Read; - return isAsProxy ? InteractTabsIndex.WriteProxy : InteractTabsIndex.Write; + if (isRead) { + return isAsProxy ? InteractTabsIndex.ReadProxy : InteractTabsIndex.Read; + } else { + return isAsProxy ? InteractTabsIndex.WriteProxy : InteractTabsIndex.Write; + } };src/lib/pages/evm-contract-details/types.ts (1)
12-17
: Consider aligning the new enum with existing naming patterns.The new
InteractTabsIndex
enum appears functionally valid, but you might want to maintain consistent naming across enums (e.g., reuse or extend the existingTabIndex
or unify their naming style) for improved code cohesion.src/lib/app-provider/tx/evm/requestEvm.ts (1)
Line range hint
25-35
: Set sensible defaults for optional transaction values.If certain transactions do not require a non-zero
value
, consider initializing it to “0” or “0x0” by default. This avoids potential runtime errors when callers forget to supply a value and clarifies intent.src/lib/components/EstimatedFeeEvmRender.tsx (1)
33-33
: Improve the placeholder for missing fee data.Returning
--
is functional yet cryptic. Providing a more descriptive placeholder or tooltip could enhance the user’s understanding of the missing fee data.src/lib/components/evm-abi/fields/TupleField.tsx (1)
49-49
: Reconsider linking required status toisDisabled
.Making a field not required when it is disabled can be sensible, but confirm that disabling a field always implies it should no longer be required in form validation. This can sometimes obscure underlying validation logic.
src/lib/components/modal/EvmCodeSnippet.tsx (3)
15-16
: Optional event payload suggestion.
You're tracking the event with empty payload:track(AmpEvent.USE_CONTRACT_SNIPPET, {})
. Consider passing additional properties (e.g., snippet usage context) to improve analytics.
18-25
: Potential dynamic import for Ace libraries.
If bundle size becomes large, consider lazy-loading the Ace editor libraries.
30-77
: Replace placeholder text with actual snippet or contextual info.
Currently, the modal’s body shows lorem ipsum. Consider including real snippet code or removing the placeholder text if this is intended for production.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx (1)
35-56
: Avoid setTimeout for scrolling.
Using a setTimeout to scroll can introduce race conditions. A more robust approach could be usingref.current?.scrollIntoView({ behavior: "smooth" });
within a stable effect or callback.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/index.tsx (1)
35-56
: Scrolling approach.
Similar to the read component, consider a direct ref-based scroll rather than a setTimeout for improved reliability.src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (2)
15-18
: Rename or relocate the enumInteractType
for clarity.While the isolated enum definition is simple enough, consider placing it within a dedicated types file or naming it in alignment with the existing
InteractTabsIndex
. This will reduce future confusion and promote consistency with the rest of the codebase.
110-189
: Consider performance when using style objects in JSX.You’re dynamically applying many style rules in the
<Box sx={{ ... }}>
. For performance and readability, you might consider extracting these style rules into a memoized object, styled component, or separate CSS file.src/lib/pages/evm-contract-details/index.tsx (3)
31-36
: Coordinate with existing types to avoid confusion.You introduced
InteractTabsIndex
, which is loosely similar toTabIndex
andTxsTabIndex
. Make sure these enum-like constructs are all necessary; trivial differences can lead to confusion.
47-48
: Update doc references forselectedType
andselectedFn
.The new props in
EvmContractDetailsBodyProps
are beneficial for controlling interaction, but consider adding doc comments or dedicated JSDoc to clarify required usage.
139-145
: MoveInteractEvmContract
call to a more context-appropriate location.The comment suggests moving
InteractEvmContract
to a different section or component. If you have a plan to relocate it, avoid deferring too long, as future merges can complicate refactoring.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx (1)
136-199
: Avoid large visible class names for dynamic items.Appending
className={
abi_write_${abiSection.name}}
can produce unwieldy class names. Consider sanitizing or hashing the method name if there's a risk of special characters causing CSS or security issues.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx (2)
84-99
: Caching strategy for read calls.You disabled caching via
cacheTime: 0
inuseEthCall
. If you intend for repeated queries to benefit from minimal caching (e.g., ephemeral calls), consider setting a small non-zero cache time to avoid re-downloading or re-querying identical reads in a brief time window.
174-209
: Check for uniform user experience on repeated reads.When the user triggers "Read Again" with no input changes, the result might remain the same. Consider auto-refresh intervals or indicating that the output is unchanged to enhance clarity for repeated calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
CHANGELOG.md
(1 hunks)src/lib/amplitude/types.ts
(1 hunks)src/lib/app-fns/tx/evm/requestEvm.tsx
(3 hunks)src/lib/app-provider/hooks/useAddress.ts
(1 hunks)src/lib/app-provider/hooks/useExampleAddresses.ts
(2 hunks)src/lib/app-provider/tx/evm/requestEvm.ts
(3 hunks)src/lib/components/ContractInputSection.tsx
(1 hunks)src/lib/components/EstimatedFeeEvmRender.tsx
(2 hunks)src/lib/components/evm-abi/EvmAbiForm.tsx
(2 hunks)src/lib/components/evm-abi/fields/TupleField.tsx
(1 hunks)src/lib/components/evm-abi/fields/index.tsx
(2 hunks)src/lib/components/modal/EvmCodeSnippet.tsx
(1 hunks)src/lib/components/modal/contract/SaveNewContract.tsx
(1 hunks)src/lib/components/select-contract/SelectContractInstantiator.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/utils.ts
(1 hunks)src/lib/pages/evm-contract-details/dummy.ts
(1 hunks)src/lib/pages/evm-contract-details/index.tsx
(3 hunks)src/lib/pages/evm-contract-details/types.ts
(2 hunks)src/lib/pages/evm-contract-details/utils.ts
(1 hunks)src/lib/services/evm/index.ts
(3 hunks)src/lib/services/tx/simulateFeeEvm.ts
(4 hunks)src/lib/utils/evm.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/lib/app-provider/hooks/useExampleAddresses.ts
- src/lib/app-provider/hooks/useAddress.ts
- src/lib/components/ContractInputSection.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
[error] 79-79: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (45)
src/lib/components/modal/contract/SaveNewContract.tsx (1)
42-42
: Renaming increases consistency.Renaming
wasmContract
tocontract
aligns with the rest of the codebase and maintains consistent naming. This change looks good and improves clarity.src/lib/components/select-contract/SelectContractInstantiator.tsx (1)
46-46
: Confirm address usage correctness.Renaming
wasmContract
tocontract
is consistent with other files, but ensure it correctly references an EVM-compatible or Bech32-compatible address as intended.Below is a script to locate occurrences of
exampleContractAddress
across the codebase, confirming that all references match the intended address format:✅ Verification successful
Address usage is correctly implemented
The renaming from
wasmContract
tocontract
is correct as the code properly handles Bech32-formatted addresses for contract instantiation. The implementation includes proper validation throughvalidateContractAddress
and supports multiple address formats (Bech32, EVM) based on the chain configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searches usage of `exampleContractAddress` to ensure consistent usage rg -A 3 "exampleContractAddress"Length of output: 3239
Script:
#!/bin/bash # Find implementation of useExampleAddresses and useValidateAddress hooks ast-grep --pattern 'export function useExampleAddresses() { $$$ }' ast-grep --pattern 'export function useValidateAddress() { $$$ }'Length of output: 140
Script:
#!/bin/bash # Search for files containing these hooks with broader patterns echo "=== Searching for useExampleAddresses ===" rg -l "useExampleAddresses" echo -e "\n=== Searching for implementation of useExampleAddresses ===" rg "export.*useExampleAddresses" -A 5 echo -e "\n=== Searching for useValidateAddress ===" rg -l "useValidateAddress" echo -e "\n=== Searching for implementation of useValidateAddress ===" rg "export.*useValidateAddress" -A 5Length of output: 3608
Script:
#!/bin/bash # Get the complete implementation of both hooks echo "=== useExampleAddresses implementation ===" rg -A 20 "export const useExampleAddresses" src/lib/app-provider/hooks/useExampleAddresses.ts echo -e "\n=== useValidateAddress implementation ===" rg -A 20 "export const useValidateAddress" src/lib/app-provider/hooks/useAddress.tsLength of output: 1716
src/lib/pages/evm-contract-details/types.ts (1)
35-42
: Double-check the fallback behavior forselectedType
.By transforming all unrecognized or omitted inputs to
InteractTabsIndex.Read
, users might lose the opportunity to provide a custom string. Verify that this override is intentional and does not interfere with valid user input or future extension of interaction types.src/lib/app-provider/tx/evm/requestEvm.ts (1)
12-12
: Validate and document the format ofvalue
.Ensure that you explicitly define whether
value
should be a decimal string, a hex string (with “0x” prefix), or another format. Invalid formats could lead to transaction failures.src/lib/components/EstimatedFeeEvmRender.tsx (2)
6-6
: Import forOption
is appropriate.Using
Option<Big>
communicates optionality clearly, matching the subsequent usage forgasPrice
/gasUsed
.
14-15
: Ensure optional gas fields are properly handled downstream.Switching
gasPrice
andgasUsed
toOption<Big>
is beneficial, but confirm that any further fee calculations or checks gracefully handlenull
orundefined
values.src/lib/components/evm-abi/EvmAbiForm.tsx (8)
11-11
: MarkingisPayable
as optional is a good improvement.
Ensures the property can be omitted where not relevant and defaults to false, which maintains backward compatibility.
13-14
: Clear separation of callback responsibilities.
IntroducingpropsOnChangeInputs
andpropsOnChangeValue
clarifies how each type of change is handled, improving maintainability.
20-20
: Consistent default props usage.
DestructuringisPayable
and the new callback props with defaults ensures the component works seamlessly even when props are not provided.Also applies to: 22-23
34-34
: Addingvalue
to the form schema.
Thevalue: string;
field aligns the form state with the new payable logic. Consider validating numeric strings if this field must represent numerical amounts.
36-36
: Default to an empty string forvalue
.
If the transaction logic expects a numeric string, you might want to ensure "0" is used as the initial value rather than "". Otherwise, confirm empty string is acceptable.
39-39
: Synchronized form state.
Destructuringinputs
andvalue
together keeps the form updates in sync and is a good pattern for maintainability.
47-49
: Use ofcloneDeep
ensures immutability.
Passing a deep clone topropsOnChangeInputs
helps avoid accidental mutations in the parent. This is a good defensive coding practice.
51-53
: New effect for notifyingvalue
changes.
The effect notifyingpropsOnChangeValue
is consistent with the approach forinputs
. This improves clarity and modularizes state updates.src/lib/services/tx/simulateFeeEvm.ts (5)
4-4
: ImportingtoBeHex
from ethers.
This import paves the way for hex-encoding transaction values.
8-9
: Typed imports fromlib/types
.
These type imports improve clarity in the codebase. No issues here.
17-17
: Addedvalue
toSimulateQueryEvmParams
.
Ensures the query can simulate fees with a specified amount. This helps unify logic with the rest of the EVM transaction flow.
28-28
: Expanded query key withvalue
.
Includingvalue
in the query key properly differentiates distinct calls, preventing cache conflicts.Also applies to: 47-47
60-60
: Hex-encodedvalue
ingetSimulateFeeEvm
.
Converting the string to hex ensures the underlying JSON-RPC call is consistent with EVM expectations. This is a best practice for numeric amounts.src/lib/app-fns/tx/evm/requestEvm.tsx (4)
13-13
: ImportingtoBeHex
.
Import aligns with your plan to hex-encode numeric transaction values, preserving consistency across the codebase.
18-18
: Addingvalue
toRequestEvmTxParams
.
This ensures transaction requests support monetary amounts in addition to data payloads.
28-28
: Destructuringvalue
inrequestEvmTx
.
Good step to unify the additional parameter with the rest of the function's logic.
37-37
: Hex-encoding transactionvalue
.
Convertingvalue
to hex withinsignAndBroadcastEvm
is consistent and avoids potential type errors during the JSON-RPC call.src/lib/services/evm/index.ts (4)
1-1
: ImportingUseQueryOptions
.
Allows users of this module to pass customizable query behaviors to your hooks. Well done.
74-78
: Enhanced signature foruseEthCall
.
Accepting anoptions: UseQueryOptions<string>
parameter adds flexibility for consumers to control query retry, caching, and more. Excellent improvement.
85-85
: UsinguseQuery<string>
.
Explicitly defining the return type asstring
strengthens type safety and clarifies the expectations for consumers.
102-102
: Spreading additional query options.
Merging user-supplied options with defaults is a robust approach, letting advanced users fine-tune their queries as needed.src/lib/components/modal/EvmCodeSnippet.tsx (3)
1-14
: Solid import organization.
All imported dependencies (Chakra UI, etc.) are well-structured.
26-28
: Interface naming and usage look good.
Prop definitions are consistent with Chakra’s ButtonProps.
80-80
: Default export is clear.
Exporting the component by default is appropriate here.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx (2)
1-10
: Imports are well-managed.
No issues observed with third-party or internal imports.
11-15
: Prop definitions are concise.
Interfaces match usage in the component.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/index.tsx (3)
1-10
: Imports look good.
No issues with the combination of external or local dependencies.
11-15
: Well-defined props.
Naming is clear, and optional props are properly typed.
58-85
: EmptyState usage is consistent.
“No write function found.” message aligns with this file’s focus.src/lib/utils/evm.ts (1)
123-125
: Confirm that all result types serialize correctly.
.map((v) => v.toString())
enforces a string type. If certain values (e.g., nested arrays, structs) appear, you risk losing type precision. Validate that your usage only handles primitive or BigNumber returns.src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (3)
44-46
: Confirm reversed ABI approach when proxying.Reversing the ABI (
contractAbi.toReversed()
) is not a common practice unless you have specific logic that requires it. Double-check that reversing the array is the intended behavior, as it may unexpectedly alter the order in which functions are displayed or processed.
48-55
: Ensure consistent handling of payable functions.You classify functions with
abi.stateMutability?.endsWith("payable")
as write. Confirm that the logic aligns with how you want to handle functions marked as "nonpayable", since these can also change state and typically should be categorized alongside payable.
70-85
: Use memoized callbacks to avoid unnecessary re-renders.
handleSetInteractType
relies oncontractAddress
andisAsProxy
in its array dependencies. Confirm that this callback won’t be recreated more often than intended if these props or states update frequently.src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx (1)
78-92
: Handle potential rate-limiting or repeated fee simulation errors.
useSimulateFeeEvmQuery
might be triggered repeatedly if inputs change or if the user toggles repeatedly. Confirm you have no rate-limiting concerns or plan to handle potential repeated error states.src/lib/amplitude/types.ts (1)
94-96
: Nice addition to the EVM action taxonomy.
These new enum values align well with EVM read/write operations.src/lib/components/evm-abi/fields/index.tsx (3)
9-9
: Remove unused imports if unneeded.
Please confirm thatText
from@chakra-ui/react
is indeed necessary elsewhere in this file. If not, removing it will keep dependencies minimal.
36-57
: Conditional rendering correctly handles cases without components.
Maintaining user clarity by providing a distinct message for empty inputs/outputs is a good approach.
65-65
: Ensure consistent use of the renamed field.
Confirm that the rest of your codebase references"value"
rather than any legacy"payableAmount"
usage.CHANGELOG.md (1)
42-42
: Good practice adding a changelog entry.
Documenting PR #1208 ensures future maintainers can track EVM interaction updates.
src/lib/pages/evm-contract-details/components/interact-evm-contract/utils.ts
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx
Show resolved
Hide resolved
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx
Show resolved
Hide resolved
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
Outdated
Show resolved
Hide resolved
feat: contract details interaction
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 (8)
src/lib/components/filter/FilterInput.tsx (1)
13-13
: Consider documenting the purpose of mainRef.Since this is a breaking change from
ref
tomainRef
, it would be helpful to add JSDoc comments explaining the purpose ofmainRef
and how it differs frominputRef
.+ /** Reference forwarded from parent components for external control */ mainRef: ForwardedRef<HTMLInputElement>;
src/lib/services/evm/json-rpc/proxy/index.ts (2)
72-91
: Confirm fallback logic forEIP-1967
beacon proxy.
In case both beacon interface calls fail (.catch(() => ...)
), you proceed without logging or tracking the error. This might hamper debugging or make it unclear why the detection failed. Consider adding some form of logging or debug output to maintain traceability.
48-148
: Avoid potential collisions in multi-proxy detection.
Since you rely on the first successful resolution fromPromise.any
, a contract that matches multiple patterns might inadvertently be classified incorrectly. If multi-proxy detection is possible, consider refactoring to evaluate which pattern is most accurate rather than returning the first success.src/lib/services/evm/json-rpc/proxy/types.ts (1)
14-18
: Name boolean flags in an expressive manner.
Renamingimmutable
toisImmutable
(or similar) can improve clarity.src/lib/pages/evm-contract-details/components/evm-contract-details-contract/index.tsx (1)
23-26
: Consider adding a constant for the default tab.The tab selection logic is correct, but consider extracting
EvmContractDetailsContractTabs.ByteCode
as a constant namedDEFAULT_TAB
to improve maintainability.+const DEFAULT_TAB = EvmContractDetailsContractTabs.ByteCode; + const [currentTab, setCurrentTab] = useState( - evmVerifyInfo - ? EvmContractDetailsContractTabs.Code - : EvmContractDetailsContractTabs.ByteCode + evmVerifyInfo ? EvmContractDetailsContractTabs.Code : DEFAULT_TAB );src/lib/services/evm/json-rpc/proxy/utils.ts (1)
5-20
: Consider adding checksum validation.The address validation looks good but could be enhanced by validating the checksum for mixed-case addresses.
+import { getAddress } from "ethers"; + export const readAddress = (value: unknown) => { if (typeof value !== "string" || value === "0x") { throw new Error(`Invalid address value: ${value}`); } let address = value; if (address.length === 66) { address = "0x" + address.slice(-40); } if (address === ZERO_ADDRESS) { throw new Error("Empty address"); } + try { + address = getAddress(address); // Validates and converts to checksum address + } catch (error) { + throw new Error(`Invalid address format: ${error.message}`); + } + return zHexAddr20.parse(address); };src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts (1)
28-31
: Consider adding retry limit for polling.While the polling logic works, it could benefit from a maximum retry limit to prevent infinite polling in case of network issues.
+const MAX_RETRIES = 10; +let retryCount = 0; + const pollForEvmTx = async (_txHash: string): Promise<TxReceiptJsonRpc> => { if (timedOut) { throw new Error( `Transaction with hash ${_txHash} was submitted but was not yet found on the chain. You might want to check later. There was a wait of ${ TIME_OUT_MS / 1000 } seconds.` ); } + if (retryCount >= MAX_RETRIES) { + throw new Error( + `Transaction polling exceeded maximum retries (${MAX_RETRIES})` + ); + } await sleep(POLL_INTERVAL_MS); + retryCount++; const result = await getEthGetTransactionReceipt( jsonRpcEndpoint, txHash ).catch(() => undefined); return result ? result : pollForEvmTx(_txHash); };src/lib/pages/evm-contract-details/index.tsx (1)
109-115
: Simplify loading state handling.The loading check combines multiple independent loading states, which could delay rendering even when the required data is ready.
Consider this more granular approach:
-if ( - isEvmCodesByAddressLoading || - isEvmVerifyInfoLoading || - isProxyTargetLoading || - isProxyTargetEvmVerifyInfoLoading -) - return <Loading />; +const isLoadingRequired = isEvmCodesByAddressLoading; +const isLoadingOptional = + isEvmVerifyInfoLoading || + isProxyTargetLoading || + isProxyTargetEvmVerifyInfoLoading; + +if (isLoadingRequired) return <Loading />;This change allows the component to render as soon as the essential data is available, while optional data can be loaded progressively.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
CHANGELOG.md
(1 hunks)src/lib/app-fns/tx/evm/requestEvm.tsx
(3 hunks)src/lib/app-provider/env.ts
(1 hunks)src/lib/app-provider/hooks/useSignAndBroadcastEvm.ts
(2 hunks)src/lib/components/evm-abi/EvmAbiForm.tsx
(2 hunks)src/lib/components/filter/FilterInput.tsx
(3 hunks)src/lib/pages/evm-contract-details/components/evm-contract-details-contract/ContractAbi.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/evm-contract-details-contract/index.tsx
(2 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/dummy.ts
(0 hunks)src/lib/pages/evm-contract-details/index.tsx
(6 hunks)src/lib/pages/my-module-verifications/components/MoveVerifyTaskStatusFilter.tsx
(1 hunks)src/lib/pages/proposals/components/ProposalStatusFilter.tsx
(1 hunks)src/lib/pages/proposals/components/ProposalTypeFilter.tsx
(1 hunks)src/lib/services/block/jsonRpc.ts
(1 hunks)src/lib/services/evm/index.ts
(3 hunks)src/lib/services/evm/json-rpc/base.ts
(2 hunks)src/lib/services/evm/json-rpc/eth.ts
(1 hunks)src/lib/services/evm/json-rpc/index.ts
(1 hunks)src/lib/services/evm/json-rpc/proxy/index.ts
(1 hunks)src/lib/services/evm/json-rpc/proxy/types.ts
(1 hunks)src/lib/services/evm/json-rpc/proxy/utils.ts
(1 hunks)src/lib/services/tx/jsonRpc.ts
(1 hunks)src/lib/services/tx/simulateFeeEvm.ts
(4 hunks)src/lib/services/types/verification/evm.ts
(2 hunks)src/lib/services/verification/evm/index.ts
(2 hunks)src/lib/types/hex.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/lib/pages/evm-contract-details/dummy.ts
✅ Files skipped from review due to trivial changes (3)
- src/lib/services/block/jsonRpc.ts
- src/lib/services/evm/json-rpc/index.ts
- src/lib/services/tx/jsonRpc.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- src/lib/components/evm-abi/EvmAbiForm.tsx
🔇 Additional comments (26)
src/lib/components/filter/FilterInput.tsx (1)
50-50
: LGTM! Correct usage of mergeRefs utility.The mergeRefs utility is properly used to combine both the internal inputRef and the forwarded mainRef.
src/lib/pages/proposals/components/ProposalStatusFilter.tsx (1)
101-101
: LGTM! Consistent with FilterInput changes.The prop update from
ref
tomainRef
aligns with the FilterInput component changes.src/lib/pages/my-module-verifications/components/MoveVerifyTaskStatusFilter.tsx (1)
92-92
: LGTM! Consistent with FilterInput changes.The prop update from
ref
tomainRef
aligns with the FilterInput component changes.src/lib/pages/proposals/components/ProposalTypeFilter.tsx (2)
103-103
: LGTM! Consistent with FilterInput changes.The prop update from
ref
tomainRef
aligns with the FilterInput component changes.
Line range hint
1-1
: Update PR title and add description.The current PR title "feat: interaction section" doesn't clearly reflect the changes made. Consider:
- Updating the title to something like "refactor: rename FilterInput ref prop to mainRef"
- Adding a PR description explaining:
- The motivation for this change
- That this is a breaking change
- Any other components that might need updates
Let's check if there are any other components using FilterInput that need updates:
✅ Verification successful
No breaking changes impact found in dependent components
The FilterInput component is used in three places:
- ProposalStatusFilter
- ProposalTypeFilter
- MoveVerifyTaskStatusFilter
None of these components use the ref prop being renamed to mainRef, so no additional updates are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for FilterInput usage in the codebase rg "FilterInput" --type ts --type tsxLength of output: 65
Script:
#!/bin/bash # Search for FilterInput usage without type restrictions rg "FilterInput" -A 3Length of output: 3261
Script:
#!/bin/bash # Search for ref prop usage in files using FilterInput rg "ref.*FilterInput|FilterInput.*ref" -A 2 -B 2 src/lib/pages/proposals/components/ProposalStatusFilter.tsx src/lib/pages/proposals/components/ProposalTypeFilter.tsx src/lib/pages/my-module-verifications/components/MoveVerifyTaskStatusFilter.tsxLength of output: 248
src/lib/services/evm/json-rpc/proxy/index.ts (2)
52-52
: Confirm Node.js compatibility forPromise.any
usage.
Ensure your Node.js environment supportsPromise.any
(added in Node 15.0). If you need broader compatibility, consider using a polyfill or an alternative likePromise.race
with custom logic.
114-129
: Consider validating thetarget
forEIP-897
proxies.
After you fetch thetarget
address, no extra checks confirm if the returned value is a valid proxy implementation. Consider verifying that the address is nonzero or has valid code, especially if subsequent logic relies on a functional implementation.src/lib/pages/evm-contract-details/components/evm-contract-details-contract/ContractAbi.tsx (2)
6-6
: Good transition to typed ABI arrays.
Switching from a string-based ABI toJsonFragment[]
helps ensure type safety, making it clearer how contract interfaces are structured. This is a solid improvement.
14-14
: Validate ABI content before serialization.
WhileJSON.stringify(abi, null, 2)
works for typical scenarios, consider gracefully handling malformed or partially loaded ABI data to prevent potential runtime errors.src/lib/types/hex.ts (1)
9-9
: LGTM! Good addition of theHex
type.The new type alias provides strong type safety for hex strings by leveraging the existing zod schema validation.
src/lib/services/evm/json-rpc/eth.ts (2)
6-14
: LGTM! Well-implemented eth_call method.The implementation correctly handles the optional
from
address and validates the response using thezHex
schema.
16-19
: LGTM! Well-implemented eth_getTransactionReceipt method.The implementation properly validates the response using the
zTxReceiptJsonRpc
schema.src/lib/services/verification/evm/index.ts (1)
17-27
: LGTM! Great improvements to useEvmVerifyInfo.The changes enhance type safety and error handling:
- Using
Option<HexAddr20>
correctly represents optional contract address- Early error handling prevents undefined contract address
- The
enabled
flag prevents unnecessary API calls when contract address is not availablesrc/lib/services/evm/json-rpc/base.ts (1)
68-68
: LGTM! Good addition of error handling.The new error handling for batch results maintains consistency with single request error handling.
src/lib/pages/evm-contract-details/components/evm-contract-details-contract/index.tsx (2)
13-14
: LGTM! Props interface update looks good.The addition of
evmVerifyInfo
prop withOption<EvmVerifyInfo>
type is well-defined and properly typed.
Line range hint
32-48
: LGTM! Conditional rendering looks good.The conditional rendering based on
evmVerifyInfo
is well-structured and properly handles the component visibility.src/lib/services/evm/json-rpc/proxy/utils.ts (1)
27-67
: LGTM! EIP-1167 parsing implementation looks robust.The implementation correctly handles:
- Bytecode prefix/suffix validation
- Variable-length address detection
- Proper padding for vanity addresses
src/lib/app-provider/env.ts (1)
148-148
: LGTM! The new query key follows the established pattern.The addition of
EVM_PROXY_TARGET
is consistent with the existing naming convention and properly supports the new proxy target functionality.src/lib/services/tx/simulateFeeEvm.ts (2)
16-17
: LGTM! The interface update is well-typed.The addition of the
value
parameter toSimulateQueryEvmParams
is properly typed as string.
27-28
: LGTM! The value parameter is properly handled.The implementation:
- Correctly includes
value
in the query key for proper cache invalidation- Properly converts the value to hex format using
toBeHex
- Handles null case when value is not provided
Also applies to: 46-47, 59-60
src/lib/app-fns/tx/evm/requestEvm.tsx (2)
17-18
: LGTM! The interface update is well-typed.The addition of the
value
parameter toRequestEvmTxParams
is properly typed as string.
27-28
: LGTM! The transaction request properly handles the value parameter.The implementation:
- Correctly passes the value to
signAndBroadcastEvm
- Properly converts the value to hex format using
toBeHex
- Handles null case when value is not provided
Also applies to: 36-42
src/lib/services/evm/index.ts (2)
Line range hint
75-105
: LGTM! The useEthCall update properly handles query options.The changes:
- Add proper typing for the options parameter
- Correctly spread options into the query configuration
- Maintain existing error handling and enabled conditions
108-134
: LGTM! The useGetEvmProxyTarget hook follows established patterns.The implementation:
- Uses proper typing for parameters and return value
- Implements appropriate error handling
- Follows the same pattern as other query hooks in the file
- Uses the newly added EVM_PROXY_TARGET query key
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (1)
66-100
: Refactor navigation callbacks to reduce duplication and improve maintainability.The navigation callbacks share similar logic and could be consolidated.
Consider this improved implementation:
+const navigateToInteract = ( + navigate: ReturnType<typeof useInternalNavigate>, + params: { + contractAddress: HexAddr20; + isRead: boolean; + isAsProxy: boolean; + } +) => { + const { contractAddress, isRead, isAsProxy } = params; + navigate({ + pathname: EVM_CONTRACT_INTERACT_PATH_NAME, + query: { + contractAddress, + selectedType: getInteractTabsIndex(isRead, isAsProxy), + }, + options: { shallow: true }, + }); +}; + const handleSetInteractType = useCallback( (newType: InteractType) => - navigate({ - pathname: EVM_CONTRACT_INTERACT_PATH_NAME, - query: { - contractAddress, - selectedType: getInteractTabsIndex( - newType === InteractType.Read, - isAsProxy - ), - }, - options: { - shallow: true, - }, - }), + navigateToInteract(navigate, { + contractAddress, + isRead: newType === InteractType.Read, + isAsProxy, + }), [contractAddress, isAsProxy, navigate] ); const handleSetIsAsProxy = useCallback( (e: ChangeEvent<HTMLInputElement>) => - navigate({ - pathname: EVM_CONTRACT_INTERACT_PATH_NAME, - query: { - contractAddress, - selectedType: getInteractTabsIndex( - interactType === InteractType.Read, - e.target.checked - ), - }, - options: { - shallow: true, - }, - }), + navigateToInteract(navigate, { + contractAddress, + isRead: interactType === InteractType.Read, + isAsProxy: e.target.checked, + }), [contractAddress, interactType, navigate] );Also, verify if shallow routing is appropriate here:
✅ Verification successful
Refactoring suggestion is valid and shallow routing is appropriate
The use of shallow routing is correct in this context as it's used for tab switching within the same view, and the parent component properly handles query parameter validation and router readiness.
The original refactoring suggestion to consolidate navigation logic remains beneficial for:
- Reducing code duplication
- Centralizing navigation logic
- Improving maintainability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any dependencies on URL parameters in parent components rg -A 5 "useRouter|useNavigate" src/lib/pages/evm-contract-details/Length of output: 973
src/lib/pages/evm-contract-details/index.tsx (1)
155-162
: Address TODO comment and improve component placement.The TODO comment suggests that the component's placement needs to be reconsidered.
Let's analyze the component hierarchy to determine the best location:
Would you like me to help create a new issue to track the proper placement of the InteractEvmContract component?
✅ Verification successful
Move InteractEvmContract component to Contract tab panel
The InteractEvmContract component should be moved from the Overview tab to the Contract tab panel, placing it before the EvmContractDetailsContract component. This would:
- Group contract-specific functionality together
- Keep the Overview tab focused on general information
- Improve the logical organization of the UI
- Create a better user experience by having all contract interaction features in one place
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the component structure in the codebase fd -e tsx -e ts . src/lib/pages/evm-contract-details/components/ -x echo "=== {} ===" \; -x cat {}Length of output: 42260
Script:
#!/bin/bash # Check the main page component and structure cat src/lib/pages/evm-contract-details/index.tsxLength of output: 7714
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
Outdated
Show resolved
Hide resolved
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.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-contract-details/index.tsx (1)
66-73
: Consider optimizing loading states handling.While the loading states are correctly implemented, having multiple independent loading checks could lead to unnecessary re-renders. Consider combining these into a single loading state.
+ const isLoading = + isEvmCodesByAddressLoading || + isEvmVerifyInfoLoading || + isProxyTargetLoading || + isProxyTargetEvmVerifyInfoFetching; - if ( - isEvmCodesByAddressLoading || - isEvmVerifyInfoLoading || - isProxyTargetLoading || - isProxyTargetEvmVerifyInfoFetching - ) + if (isLoading) return <Loading />;Also applies to: 109-115
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/pages/evm-contract-details/index.tsx
(6 hunks)
🔇 Additional comments (4)
src/lib/pages/evm-contract-details/index.tsx (4)
22-22
: LGTM! Well-organized imports.The new imports are properly organized and necessary for implementing the contract interaction functionality.
Also applies to: 32-39
48-49
: LGTM! Props are well-typed and properly used.The new props
selectedType
and optionalselectedFn
are appropriately typed and correctly destructured in the component parameters.Also applies to: 55-56
155-155
: Address the TODO comment about component location.The TODO comment suggests that the current placement of
InteractEvmContract
might not be optimal. Consider creating a dedicated tab for contract interactions instead of placing it in the Overview tab.
156-162
: Verify ABI fallback behavior.The component uses optional chaining and nullish coalescing for ABI props. Ensure proper error handling when ABIs are unavailable.
✅ Verification successful
ABI handling is properly implemented ✅
The component has appropriate fallback behavior:
- Parent provides empty array fallback via
evmVerifyInfo?.abi ?? []
- TypeScript enforces ABI prop requirements
- Component safely handles empty ABIs by showing no functions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for ABI error handling in InteractEvmContract ast-grep --pattern 'function InteractEvmContract({ contractAbi, proxyTargetAbi }) { $$$ if (!contractAbi $$$) { $$$ } $$$ }'Length of output: 138
Script:
#!/bin/bash # Find the InteractEvmContract component file fd InteractEvmContract # Search for component definition with ripgrep for backup rg "InteractEvmContract.*=.*" -A 10 # Try alternative ast-grep pattern for arrow function ast-grep --pattern 'const InteractEvmContract = ({ $$ }) => { $$$ }'Length of output: 1933
Script:
#!/bin/bash # Get the full component implementation cat src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx # Search for error handling patterns rg "(!contractAbi|contractAbi\s+==|contractAbi\s+===|isEmpty.*contractAbi)" src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx # Look for error messages or validation rg "(error|warning|alert|message).*abi" -i src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsxLength of output: 5759
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/pages/evm-contract-details/types.ts (2)
12-17
: Standardize enum value naming convention.The enum values mix naming conventions between simple lowercase and kebab-case. Consider standardizing all values to use kebab-case for consistency:
export enum InteractTabsIndex { - Read = "read", - Write = "write", + Read = "read-direct", + Write = "write-direct", ReadProxy = "read-proxy", WriteProxy = "write-proxy", }
35-42
: Add JSDoc comments for new query parameters.The new query parameters are well-typed, but their purpose isn't immediately clear. Consider adding JSDoc comments to document:
- What
selectedType
represents and why it defaults to Read- The purpose and expected format of
selectedFn
+ /** Type of interaction (direct read/write or proxy read/write). Defaults to direct read. */ selectedType: z.union([ z.nativeEnum(InteractTabsIndex), z .string() .optional() .transform(() => InteractTabsIndex.Read), ]), + /** Selected function identifier for pre-selecting a specific contract function */ selectedFn: z.string().optional(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/pages/evm-contract-details/types.ts
(2 hunks)
🔇 Additional comments (1)
src/lib/pages/evm-contract-details/types.ts (1)
20-23
: LGTM! Improved acronym casing.The change from "Abi" to "ABI" better follows standard acronym casing conventions while maintaining PascalCase for other values.
Got an error when sending the transaction to this contract
|
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: 2
♻️ Duplicate comments (1)
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (1)
111-172
: 🛠️ Refactor suggestionSimplify component rendering using React's conditional rendering.
The current implementation uses CSS display properties for conditional rendering, which is less maintainable than React's built-in conditional rendering.
This reinforces the previous review comment. Using React's conditional rendering would make the code more maintainable and easier to understand.
🧹 Nitpick comments (5)
src/lib/services/evm/json-rpc/eth.ts (1)
6-14
: Consider adding optional parameters for complete eth_call support.The implementation looks good, but consider adding optional
value
andgas
parameters to fully support the eth_call JSON-RPC method specification.export const getEthCall = ( endpoint: string, from: Nullable<HexAddr20>, to: HexAddr20, - data: string + data: string, + value?: string, + gas?: string ) => requestJsonRpc(endpoint, "eth_call", [ - { from, to, data }, + { from, to, data, value, gas }, "latest" ]).then((result) => parseWithError(zHex, result));src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (2)
45-45
: Consider a more descriptive state variable name.The name
initialSelectedFn
could be more descriptive to better reflect its purpose, such asselectedFunctionName
oractiveFunction
.
55-89
: Extract common navigation logic to reduce duplication.Both callback handlers share similar navigation logic. Consider extracting the common parts into a reusable function.
+const navigateWithParams = ( + navigate: (params: any) => void, + contractAddress: string, + selectedType: string, + options = { shallow: true } +) => { + navigate({ + pathname: EVM_CONTRACT_INTERACT_PATH_NAME, + query: { + contractAddress, + selectedType, + }, + options, + }); +}; const handleSetInteractType = useCallback( (newType: InteractType) => - navigate({ - pathname: EVM_CONTRACT_INTERACT_PATH_NAME, - query: { - contractAddress, - selectedType: getInteractTabsIndex( - newType === InteractType.Read, - isAsProxy - ), - }, - options: { - shallow: true, - }, - }), + navigateWithParams( + navigate, + contractAddress, + getInteractTabsIndex(newType === InteractType.Read, isAsProxy) + ), [contractAddress, isAsProxy, navigate] ); const handleSetIsAsProxy = useCallback( (e: ChangeEvent<HTMLInputElement>) => - navigate({ - pathname: EVM_CONTRACT_INTERACT_PATH_NAME, - query: { - contractAddress, - selectedType: getInteractTabsIndex( - interactType === InteractType.Read, - e.target.checked - ), - }, - options: { - shallow: true, - }, - }), + navigateWithParams( + navigate, + contractAddress, + getInteractTabsIndex(interactType === InteractType.Read, e.target.checked) + ), [contractAddress, interactType, navigate] );src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx (2)
39-54
: Add display name to the memoized component.While the implementation is correct, add a display name to the memoized component for better debugging experience.
-const TimestampText = memo(({ timestamp }: { timestamp: Option<Date> }) => { +const TimestampText = memo(({ timestamp }: { timestamp: Option<Date> }) => { // ... component implementation -}); +}, { displayName: 'TimestampText' });
131-151
: Improve button layout consistency.The button layout and spacing is inconsistent between input and non-input sections. Consider using the same layout pattern for both sections.
- <Flex gap={2} justify="flex-start" mt={3}> + <Flex gap={2} mt={3}> <CopyButton ... /> <EvmCodeSnippet /> - <Button ... ml="auto" /> + <Flex gap={2} ml="auto"> + <Button ... /> + </Flex> </Flex>Also applies to: 174-206
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/utils.ts
(1 hunks)src/lib/services/evm/json-rpc/eth.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/pages/evm-contract-details/components/interact-evm-contract/utils.ts
- src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx
🔇 Additional comments (8)
src/lib/services/evm/json-rpc/eth.ts (4)
1-4
: LGTM! Well-organized imports.The imports are properly structured and all imported entities are utilized in the code.
16-19
: LGTM! Proper implementation with response validation.The function correctly implements the eth_getTransactionReceipt method with appropriate response validation.
21-24
: LGTM! Response validation implemented as suggested.The function now includes proper response validation using the zHex schema, addressing the previous review comment.
26-35
: Consider validating the position parameter.While the response validation has been properly implemented as suggested in the previous review, consider validating that the position parameter is a valid hex string before making the request.
export const getEthGetStorageAt = ( endpoint: string, address: HexAddr20, - position: string + position: `0x${string}` ) => { + if (!position.startsWith('0x')) { + throw new Error('Position must be a hex string starting with 0x'); + } return requestJsonRpc(endpoint, "eth_getStorageAt", [ address, position, "latest", ]).then((result) => parseWithError(zHex, result)); }src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (2)
1-28
: LGTM! Well-structured imports and type definitions.The imports are comprehensive, and the type definitions follow TypeScript best practices. The path constant format aligns with Next.js conventions.
91-93
: LGTM! Clean and focused effect hook.The effect hook is well-implemented with the correct dependency array.
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx (2)
1-37
: LGTM! Well-organized imports and optimized loading.The imports are properly organized and the dynamic import of
EvmCodeSnippet
with SSR disabled is a good optimization.
50-50
: Verify color contrast for timestamp text.The combination of dark color and opacity might make the timestamp text difficult to read. Consider using a more accessible color combination.
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
Show resolved
Hide resolved
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
Outdated
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 (3)
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx (3)
39-54
: Consider using a custom useInterval hook and add displayName.While the component logic is correct, it could be improved for better maintainability and debugging.
- Consider extracting the interval logic to a custom hook:
const useInterval = (callback: () => void, delay: number) => { useEffect(() => { const interval = setInterval(callback, delay); return () => clearInterval(interval); }, [callback, delay]); };
- Add displayName for better debugging:
const TimestampText = memo(({ timestamp }: { timestamp: Option<Date> }) => { // ... component logic ... }); +TimestampText.displayName = 'TimestampText';
56-66
: Add JSDoc documentation and improve prop naming.The interface is well-typed but could benefit from better documentation and naming.
+/** + * Component for reading data from EVM contracts. + * @param contractAddress - The address of the EVM contract + * @param abiSection - The ABI fragment containing the function definition + * @param isExpanded - Whether the accordion section is expanded + */ interface ReadBoxProps { contractAddress: HexAddr20; abiSection: JsonFragment; - opened: boolean; + isExpanded: boolean; }
70-108
: Improve error handling and simplify callback logic.The error handling could be more specific, and the callback logic could be simplified.
- Add specific error types:
type EthCallError = { code?: number; message: string; details?: unknown; };
- Improve error handling:
- onError: (err) => { + onError: (err: Error | EthCallError) => { + const errorMessage = 'code' in err + ? `Error ${err.code}: ${err.message}` + : err.message; - setQueryError((err as Error).message); + setQueryError(errorMessage); setTimestamp(undefined); setRes(""); },
- Simplify handleRead callback:
- const handleRead = useCallback(() => { + const handleRead = useCallback((isAgain = false) => { track(AmpEvent.ACTION_EVM_READ, { inputRequired, + isAgain, }); refetch(); - }, [inputRequired, refetch]); + }, [inputRequired, refetch]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
(1 hunks)
🔇 Additional comments (3)
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx (3)
1-37
: LGTM! Well-organized imports and correct use of dynamic imports.The imports are logically grouped, and the dynamic import for EvmCodeSnippet with SSR disabled is appropriate for modal components.
173-207
: Extract duplicated Read button logic and fix tracking.The Read button implementation and tracking logic need improvement.
This is a duplicate of previous review comments. The issues include:
- Duplicated Read button logic between input and non-input sections
- Incorrect amptrackSection value for CopyButton in the non-input section
- Complex conditional rendering that could be simplified
Please refer to the previous review comments for the suggested fixes.
83-97
: Verify EVM provider initialization to address reported errors.A user reported errors with eth_blockNumber calls, which might be related to the EVM provider initialization in useEthCall.
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
♻️ Duplicate comments (1)
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (1)
111-173
: 🛠️ Refactor suggestionSimplify display logic using React's conditional rendering.
The current implementation using CSS display properties is less maintainable. A previous review suggested using React's conditional rendering instead.
This approach would be cleaner and more maintainable. Additionally, it would help prevent potential race conditions during state updates.
🧹 Nitpick comments (1)
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (1)
14-15
: Consider using Next.js dynamic route pattern for the path constant.The current path pattern might not work correctly with Next.js dynamic routes. Consider using the Next.js pattern:
-export const EVM_CONTRACT_INTERACT_PATH_NAME = - "/evm-contracts/[contractAddress]"; +export const EVM_CONTRACT_INTERACT_PATH_NAME = + "/evm-contracts/:contractAddress";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
(1 hunks)src/lib/services/proposal/api.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
src/lib/services/proposal/api.ts (1)
158-158
: LGTM! Improved rank calculation for pagination.The modification to include
offset
in the rank calculation ensures consistent ranking across paginated results. For example, if the page size is 5, the first page (offset 0) will show ranks 1-5, and the second page (offset 5) will correctly show ranks 6-10.src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (3)
141-154
: Verify contract interaction implementation to address reported error.The PR objectives mention a transaction error: "Cannot read properties of undefined (reading 'request')". This suggests potential issues with contract instance creation or management.
Run this script to analyze the contract interaction implementation:
55-89
: Verify navigation state persistence with shallow routing.The use of shallow routing might cause state persistence issues when users navigate back/forward. Consider:
- Testing browser navigation behavior
- Adding state cleanup on unmount if needed
Run this script to check for potential navigation issues:
47-50
: Add error handling for ABI processing.Given the reported transaction error in the PR objectives, consider adding error handling for invalid ABI cases:
- const { read: abiRead, write: abiWrite } = categorizeAbi(contractAbi); - const { read: abiReadProxy, write: abiWriteProxy } = categorizeAbi( - proxyTargetAbi ?? [] - ); + const { read: abiRead, write: abiWrite } = categorizeAbi(contractAbi ?? []); + try { + const { read: abiReadProxy, write: abiWriteProxy } = categorizeAbi( + proxyTargetAbi ?? [] + ); + } catch (error) { + console.error('Error processing proxy ABI:', error); + // Handle the error appropriately + }
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the updated release notes:
New Features
ReadBox
,AbiRead
) and writing (WriteBox
,AbiWrite
) to EVM contracts.InteractEvmContract
).EvmCodeSnippet
).Improvements
Bug Fixes
Chores