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

♻️ Chore: Move proxy behavior to base tevm class #816

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

roninjin10
Copy link
Collaborator

@roninjin10 roninjin10 commented Jan 12, 2024

Description

We need this behavior in the base tevm class to implement ethers memory provider

Testing

Explain the quality checks that have been done on the code changes

Additional Information

Your ENS/address:

Summary by CodeRabbit

  • New Features

    • Introduced new error handling classes for improved user feedback on proxy configuration and internal server issues.
    • Enhanced HTTP handler creation for more robust server-client communication.
  • Documentation

    • Updated documentation to reflect new error handling classes and their usage.
    • Adjusted module documentation with updated line numbers and definitions.
  • Refactor

    • Streamlined the creation and configuration of remote clients.
    • Refined the server's request processing logic for better performance and clarity.
  • Bug Fixes

    • Fixed incorrect documentation references and updated class definitions.
  • Style

    • Improved code organization by consolidating error exports into a single module.

Copy link

vercel bot commented Jan 12, 2024

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

Name Status Preview Comments Updated (UTC)
evmts-docs 🛑 Canceled (Inspect) Jan 12, 2024 4:39am

Copy link

changeset-bot bot commented Jan 12, 2024

⚠️ No Changeset found

Latest commit: eee903c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Jan 12, 2024

Walkthrough

The changes across these files reflect a significant refactoring of the tevm and vm modules, focusing on the creation and handling of HTTP requests. The createHttpHandler function has been centralized and its usage standardized, while error handling has been improved with the introduction of new error classes. The documentation has been updated to reflect these changes, with adjustments to line references and removal of outdated properties.

Changes

File Path Change Summary
.../tevmViemExtension.spec.ts, .../createRemoteClient.spec.ts, .../createHttpHandler.spec.ts Updated import and usage of createHttpHandler from @tevm/server.
tevm/docs/..., vm/server/docs/..., vm/vm/docs/... Updated documentation, line references, and removed createHttpHandler property from type definitions.
.../createHttpHandler.js, .../createTevm.js Refactored createHttpHandler function and updated createTevm to use new processRequest logic.
.../DaiContract.sol.ts Added TypeScript mock for Dai smart contract.
vm/vm/docs/classes/...Error.md Introduced documentation for new error classes (NoProxyConfiguredError, ProxyFetchError, UnexpectedInternalServerError).
vm/vm/src/errors/index.js, .../processRequest.js, .../proxyRequest.js Centralized export of new error classes and updated import paths.
vm/vm/src/index.js, .../index.ts Exported new error classes and existing entities.
vm/vm/src/Tevm.ts Removed createHttpHandler import and property from Tevm type.

"In the realm of code, where the rabbits hop,
🐇 Refactoring's done, with a skip and a bop.
Errors now caught, with a gentle embrace,
Documentation aligns, in its proper place."

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>.
    • Generate unit-tests for this file.
  • 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 tests 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 generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • 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/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@roninjin10 roninjin10 mentioned this pull request Jan 12, 2024
1 task
@roninjin10 roninjin10 force-pushed the surface-revert-reason-when-contract-reverts branch from 6eb7d4b to 2722a06 Compare January 12, 2024 04:12
@roninjin10 roninjin10 force-pushed the move-proxy-behavior-to-base-tevm-class branch from b9ebab9 to cf5b390 Compare January 12, 2024 04:12
@roninjin10 roninjin10 force-pushed the surface-revert-reason-when-contract-reverts branch from 2722a06 to c02847d Compare January 12, 2024 04:16
@roninjin10 roninjin10 force-pushed the move-proxy-behavior-to-base-tevm-class branch from cf5b390 to 337e147 Compare January 12, 2024 04:16
@roninjin10 roninjin10 force-pushed the surface-revert-reason-when-contract-reverts branch from c02847d to 0b4869f Compare January 12, 2024 04:16
@roninjin10 roninjin10 force-pushed the move-proxy-behavior-to-base-tevm-class branch 2 times, most recently from 435e147 to dcdf271 Compare January 12, 2024 04:22
Base automatically changed from surface-revert-reason-when-contract-reverts to main January 12, 2024 04:30
@roninjin10 roninjin10 force-pushed the move-proxy-behavior-to-base-tevm-class branch from dcdf271 to eee903c Compare January 12, 2024 04:31
Copy link
Contributor

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between dc53a1b and eee903c.
Files ignored due to filter (6)
  • extensions/viem/package.json
  • nx.json
  • pnpm-lock.yaml
  • vm/client/package.json
  • vm/server/package.json
  • vm/vm/package.json
Files selected for processing (19)
  • extensions/viem/src/tevmViemExtension.spec.ts (2 hunks)
  • tevm/docs/modules/index.md (1 hunks)
  • tevm/docs/modules/vm.md (4 hunks)
  • vm/client/src/createRemoteClient.spec.ts (2 hunks)
  • vm/server/docs/modules.md (1 hunks)
  • vm/server/src/createHttpHandler.js (4 hunks)
  • vm/server/src/test/DaiContract.sol.ts (1 hunks)
  • vm/server/src/test/createHttpHandler.spec.ts (2 hunks)
  • vm/vm/docs/classes/NoProxyConfiguredError.md (1 hunks)
  • vm/vm/docs/classes/ProxyFetchError.md (1 hunks)
  • vm/vm/docs/classes/UnexpectedInternalServerError.md (1 hunks)
  • vm/vm/docs/modules.md (6 hunks)
  • vm/vm/src/Tevm.ts (2 hunks)
  • vm/vm/src/createTevm.js (2 hunks)
  • vm/vm/src/errors/index.js (1 hunks)
  • vm/vm/src/index.js (1 hunks)
  • vm/vm/src/index.ts (1 hunks)
  • vm/vm/src/processRequest.js (1 hunks)
  • vm/vm/src/proxyRequest.js (1 hunks)
Files skipped from review due to trivial changes (2)
  • tevm/docs/modules/index.md
  • vm/server/docs/modules.md
Additional comments: 23
vm/vm/src/index.js (1)
  • 1-2: The changes to the exports in index.js look good and are consistent with the refactoring effort to centralize error handling.
vm/vm/src/errors/index.js (1)
  • 1-3: The export of new error classes NoProxyConfiguredError, ProxyFetchError, and UnexpectedInternalServerError is correct and aligns with the refactoring to improve error handling.
vm/vm/src/index.ts (1)
  • 3-6: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-6]

The addition of the export for all entities from the 'errors' module in index.ts is consistent with the changes in the JavaScript index file and supports the centralized error handling strategy.

vm/vm/src/processRequest.js (1)
  • 1-1: The update to the import path for UnexpectedInternalServerError in processRequest.js is correct and reflects the new location of the error classes.
vm/vm/src/Tevm.ts (1)
  • 1-1: The removal of the createHttpHandler import and property from the Tevm type is consistent with the refactoring to move proxy behavior to the base tevm class.
vm/vm/src/proxyRequest.js (1)
  • 1-1: The consolidation of error imports in proxyRequest.js is correct and aligns with the centralized error handling approach.
vm/server/src/test/createHttpHandler.spec.ts (1)
  • 1-1: The renaming of the imported function to createHttpHandler and the update to the function call in createHttpHandler.spec.ts are correct and reflect the changes in the HTTP handler creation process.
extensions/viem/src/tevmViemExtension.spec.ts (1)
  • 4-4: The addition of the createHttpHandler import from @tevm/server and the modification to the createServer function call in tevmViemExtension.spec.ts are correct and reflect the new way of creating the HTTP handler.
tevm/docs/modules/vm.md (1)
  • 83-83: The documentation update in vm.md correctly removes the createHttpHandler property from the Tevm type, aligning with the code changes.
vm/vm/src/createTevm.js (2)
  • 2-2: The addition of the processRequest import in createTevm.js is correct and necessary for the refactored request handling.

  • 80-80: The update to the request property to include the processRequest function call with an additional argument is correct and reflects the new centralized request handling mechanism.

vm/client/src/createRemoteClient.spec.ts (2)
  • 5-5: The addition of the createHttpHandler import from @tevm/server in createRemoteClient.spec.ts is correct and reflects the changes in the HTTP handler creation process.

  • 18-20: The modification to the createServer call to use the newly imported createHttpHandler with a modified configuration object is correct and aligns with the refactoring efforts.

vm/vm/docs/modules.md (1)
  • 9-11: The addition of new classes NoProxyConfiguredError, ProxyFetchError, and UnexpectedInternalServerError in modules.md is correct and provides the necessary documentation for the new error handling classes.
vm/server/src/createHttpHandler.js (2)
  • 6-6: The update to the CreateHttpHandlerParameters type to include a request property of type TevmJsonRpcRequestHandler is correct and reflects the new function signature.

  • 37-37: The refactoring of the createHttpHandler function to accept a single parameters object and use the parameters.request function for processing requests is correct and aligns with the centralized request handling approach.

vm/vm/docs/classes/ProxyFetchError.md (1)
  • 1-1: The documentation for the ProxyFetchError class in ProxyFetchError.md is correct and provides the necessary information about the class hierarchy, constructors, properties, and methods.
vm/vm/docs/classes/NoProxyConfiguredError.md (1)
  • 1-1: The documentation for the NoProxyConfiguredError class in NoProxyConfiguredError.md is correct and provides the necessary information about the class hierarchy, constructors, properties, and methods.
vm/vm/docs/classes/UnexpectedInternalServerError.md (1)
  • 1-1: The documentation for the UnexpectedInternalServerError class in UnexpectedInternalServerError.md is correct and provides the necessary information about the class hierarchy, constructors, properties, and methods.
vm/server/src/test/DaiContract.sol.ts (4)
  • 1-1: The import statement for Address from 'viem' is correct and follows TypeScript conventions.

  • 3-4: The deployedBytecode variable is a string representing the bytecode of the contract. Ensure that this bytecode corresponds to the compiled result of the Solidity contract it represents.

  • 5-250: The ABI array is well-defined and matches the expected structure for an ERC20 token contract. Each function and event is correctly typed with appropriate inputs and outputs.

  • 252-281: The exported DaiContract object correctly structures the read and script methods to interact with the contract. The use of TypeScript's as const ensures that the object's structure is readonly and cannot be altered, which is a good practice for ABI definitions.

@roninjin10 roninjin10 merged commit 8651d27 into main Jan 12, 2024
36 checks passed
@roninjin10 roninjin10 deleted the move-proxy-behavior-to-base-tevm-class branch January 12, 2024 05:30
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.

1 participant