Skip to content
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

Merged
merged 9 commits into from
Jan 29, 2025
Merged

feat: interaction section #1208

merged 9 commits into from
Jan 29, 2025

Conversation

songwongtp
Copy link
Collaborator

@songwongtp songwongtp commented Jan 24, 2025

Summary by CodeRabbit

Based on the comprehensive summary of changes, here are the updated release notes:

  • New Features

    • Added EVM contract interaction capabilities.
    • Introduced new components for reading (ReadBox, AbiRead) and writing (WriteBox, AbiWrite) to EVM contracts.
    • Enhanced contract details page with interactive contract interfaces (InteractEvmContract).
    • Introduced a new component for displaying EVM code snippets (EvmCodeSnippet).
    • Added new utility functions for enhanced Ethereum proxy interactions.
  • Improvements

    • Updated ABI handling for EVM contracts to use structured formats.
    • Improved transaction simulation and fee estimation for EVM interactions.
    • Enhanced error handling and control flow in contract interaction components.
    • Introduced new types and enums for better type management in EVM interactions.
    • Enhanced loading state management for EVM contract details.
  • Bug Fixes

    • Updated type handling for contract addresses and function parameters.
    • Improved error handling in contract interaction components.
  • Chores

    • Refactored utility functions for EVM contract interactions.
    • Updated naming conventions across multiple components.
    • Enhanced documentation and changelog entries for better clarity.

Copy link

vercel bot commented Jan 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
celatone-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 29, 2025 6:13am
6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
celatone-frontend-main ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 6:13am
initia-celatone-frontend ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 6:13am
neutron-celatone-frontend ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 6:13am
osmosis-celatone-frontend ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 6:13am
sei-celatone-frontend ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 6:13am
terra-celatone-frontend ⬜️ Ignored (Inspect) Visit Preview Jan 29, 2025 6:13am

Copy link

coderabbitai bot commented Jan 24, 2025

Walkthrough

This 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 CHANGELOG.md has been updated to reflect these new features, ensuring comprehensive documentation of the changes made.

Changes

File Path Change Summary
CHANGELOG.md Added new feature entries for "evm contract details interaction" and "evm interaction section".
src/lib/amplitude/types.ts Added three new Amplitude event types for EVM interactions: ACTION_EVM_WRITE, ACTION_EVM_READ, ACTION_EVM_READ_AGAIN.
src/lib/app-fns/tx/evm/requestEvm.tsx Updated RequestEvmTxParams interface to include a new value parameter.
src/lib/app-provider/tx/evm/requestEvm.ts Updated RequestEvmStreamParams interface to include a new required value property.
src/lib/components/ContractInputSection.tsx Renamed destructured variable from wasmContract to contract.
src/lib/components/EstimatedFeeEvmRender.tsx Updated gasPrice and gasUsed parameter types from Big to Option<Big>.
src/lib/components/evm-abi/EvmAbiForm.tsx Made isPayable optional and added a new callback for handling value changes.
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx Introduced new ReadBox component for reading data from EVM contracts.
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx Introduced new WriteBox component for writing data to EVM contracts.
src/lib/services/evm/index.ts Enhanced useEthCall function with additional query options and introduced useGetEvmProxyTarget.
src/lib/services/tx/simulateFeeEvm.ts Updated useSimulateFeeEvmQuery to handle a new value parameter.
src/lib/types/hex.ts Added a new type alias Hex for hexadecimal strings.

Sequence Diagram

sequenceDiagram
    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
Loading

Possibly related PRs

Suggested Reviewers

  • Poafs1

Poem

🐰 Hop into the EVM dance,
Where contracts sing and transactions prance!
With read and write, we'll interact,
Our blockchain rabbit's got the knack!
Smart contracts bloom like springtime's art! 🌱

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 maintainability

Defining 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 statements

While 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 existing TabIndex 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 to isDisabled.

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 using ref.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 enum InteractType 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 to TabIndex and TxsTabIndex. Make sure these enum-like constructs are all necessary; trivial differences can lead to confusion.


47-48: Update doc references for selectedType and selectedFn.

The new props in EvmContractDetailsBodyProps are beneficial for controlling interaction, but consider adding doc comments or dedicated JSDoc to clarify required usage.


139-145: Move InteractEvmContract 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 in useEthCall. 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

📥 Commits

Reviewing files that changed from the base of the PR and between b61fd52 and d394f60.

📒 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 to contract 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 to contract 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 to contract is correct as the code properly handles Bech32-formatted addresses for contract instantiation. The implementation includes proper validation through validateContractAddress 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 5

Length 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.ts

Length of output: 1716

src/lib/pages/evm-contract-details/types.ts (1)

35-42: Double-check the fallback behavior for selectedType.

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 of value.

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 for Option is appropriate.

Using Option<Big> communicates optionality clearly, matching the subsequent usage for gasPrice/gasUsed.


14-15: Ensure optional gas fields are properly handled downstream.

Switching gasPrice and gasUsed to Option<Big> is beneficial, but confirm that any further fee calculations or checks gracefully handle null or undefined values.

src/lib/components/evm-abi/EvmAbiForm.tsx (8)

11-11: Marking isPayable 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.
Introducing propsOnChangeInputs and propsOnChangeValue clarifies how each type of change is handled, improving maintainability.


20-20: Consistent default props usage.
Destructuring isPayable 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: Adding value to the form schema.
The value: 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 for value.
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.
Destructuring inputs and value together keeps the form updates in sync and is a good pattern for maintainability.


47-49: Use of cloneDeep ensures immutability.
Passing a deep clone to propsOnChangeInputs helps avoid accidental mutations in the parent. This is a good defensive coding practice.


51-53: New effect for notifying value changes.
The effect notifying propsOnChangeValue is consistent with the approach for inputs. This improves clarity and modularizes state updates.

src/lib/services/tx/simulateFeeEvm.ts (5)

4-4: Importing toBeHex from ethers.
This import paves the way for hex-encoding transaction values.


8-9: Typed imports from lib/types.
These type imports improve clarity in the codebase. No issues here.


17-17: Added value to SimulateQueryEvmParams.
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 with value.
Including value in the query key properly differentiates distinct calls, preventing cache conflicts.

Also applies to: 47-47


60-60: Hex-encoded value in getSimulateFeeEvm.
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: Importing toBeHex.
Import aligns with your plan to hex-encode numeric transaction values, preserving consistency across the codebase.


18-18: Adding value to RequestEvmTxParams.
This ensures transaction requests support monetary amounts in addition to data payloads.


28-28: Destructuring value in requestEvmTx.
Good step to unify the additional parameter with the rest of the function's logic.


37-37: Hex-encoding transaction value.
Converting value to hex within signAndBroadcastEvm is consistent and avoids potential type errors during the JSON-RPC call.

src/lib/services/evm/index.ts (4)

1-1: Importing UseQueryOptions.
Allows users of this module to pass customizable query behaviors to your hooks. Well done.


74-78: Enhanced signature for useEthCall.
Accepting an options: UseQueryOptions<string> parameter adds flexibility for consumers to control query retry, caching, and more. Excellent improvement.


85-85: Using useQuery<string>.
Explicitly defining the return type as string 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 on contractAddress and isAsProxy 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 that Text 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.

Copy link

@coderabbitai coderabbitai bot left a 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 to mainRef, it would be helpful to add JSDoc comments explaining the purpose of mainRef and how it differs from inputRef.

+  /** 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 for EIP-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 from Promise.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.
Renaming immutable to isImmutable (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 named DEFAULT_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

📥 Commits

Reviewing files that changed from the base of the PR and between d394f60 and 53530ed.

📒 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 to mainRef 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 to mainRef 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 to mainRef 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:

  1. Updating the title to something like "refactor: rename FilterInput ref prop to mainRef"
  2. 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 tsx

Length of output: 65


Script:

#!/bin/bash
# Search for FilterInput usage without type restrictions
rg "FilterInput" -A 3

Length 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.tsx

Length of output: 248

src/lib/services/evm/json-rpc/proxy/index.ts (2)

52-52: Confirm Node.js compatibility for Promise.any usage.
Ensure your Node.js environment supports Promise.any (added in Node 15.0). If you need broader compatibility, consider using a polyfill or an alternative like Promise.race with custom logic.


114-129: Consider validating the target for EIP-897 proxies.
After you fetch the target 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 to JsonFragment[] helps ensure type safety, making it clearer how contract interfaces are structured. This is a solid improvement.


14-14: Validate ABI content before serialization.
While JSON.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 the Hex 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 the zHex 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 available
src/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 with Option<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 to SimulateQueryEvmParams 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 to RequestEvmTxParams 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.tsx

Length of output: 7714

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 53530ed and 2e8c0ba.

📒 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 optional selectedFn 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.tsx

Length of output: 5759

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2e8c0ba and 3e7696d.

📒 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.

@evilpeach
Copy link
Collaborator

evilpeach commented Jan 28, 2025

Got an error when sending the transaction to this contract

could not coalesce error (error={ "code": -32603, "data": { "originalError": { "message": "Cannot read properties of 
undefined (reading 'request')", "stack": "TypeError: Cannot read properties of undefined (reading 'request')\n at L.ethRpc 
(chrome-extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:10066900)\n at chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:10086922\n at r (chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:9504215)\n at chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:9504273\n at T (chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:10090944)\n at u (chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:10084869)\n at o.listenCallback (chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:10168473)\n at o.onRequest (chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:11709228)\n at chrome-
extension://acmacodkjbdgmoleebolmdjonilkdbch/background.js:2:11710074" } }, "message": "Cannot read properties of 
undefined (reading 'request')" }, payload={ "id": 4, "jsonrpc": "2.0", "method": "eth_blockNumber", "params": [ ] }, 
code=UNKNOWN_ERROR, version=6.13.5)

Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Simplify 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 and gas 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 as selectedFunctionName or activeFunction.


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

📥 Commits

Reviewing files that changed from the base of the PR and between 3e7696d and 34c6177.

📒 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.

Copy link

@coderabbitai coderabbitai bot left a 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.

  1. 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]);
};
  1. 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.

  1. Add specific error types:
type EthCallError = {
  code?: number;
  message: string;
  details?: unknown;
};
  1. 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("");
     },
  1. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 34c6177 and 6090a78.

📒 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:

  1. Duplicated Read button logic between input and non-input sections
  2. Incorrect amptrackSection value for CopyButton in the non-input section
  3. 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.

Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Simplify 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6090a78 and 6257a7a.

📒 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:

  1. Testing browser navigation behavior
  2. 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
+  }

@Poafs1 Poafs1 merged commit 944d128 into develop Jan 29, 2025
13 checks passed
@Poafs1 Poafs1 deleted the feat/interaction-section branch January 29, 2025 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants