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

[TT-1546] add back chain id #1133

Merged
merged 3 commits into from
Sep 16, 2024
Merged

[TT-1546] add back chain id #1133

merged 3 commits into from
Sep 16, 2024

Conversation

Tofel
Copy link
Contributor

@Tofel Tofel commented Sep 12, 2024

What?

  1. add back chainId to Network (if it's not set we will still get it from the RPC node as we do now)
  2. allow to initialise ClientBuilder with existing config
  3. allow to select active network when building Seth with ClientBuilder

Why?

  1. Network names can vary a lot among different applications even for the same network (e.g. Sepolia, Sepolia Testnet, eth-sepolia), which makes it difficult to match Seth network by name. ChainID doesn't have any ambivalence.
  2. Some users want to initialise Seth from TOML config and apply some slight modifications to it (e.g. select active network)
  3. If you have a Seth config with multiple networks defined, but you don't want to hardcode active network in that config currently you have no easy way of selecting. Thus I've added options to the builder that will allow you to select active networks from all that are present in the config.

Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.

Why

The changes improve the flexibility and configurability of the SETH client, notably by introducing a way to specify a configuration path for testing and enhancing the client builder to support modifying an existing configuration or setting a network by chain ID. Additionally, adjustments to gas estimation, error handling, and contract deployment demonstrate a broader effort to refine the tool's usability and reliability for various blockchain development scenarios.

What

  • seth/Makefile

    • Added SETH_CONFIG_PATH environment variable to specify a configuration path for testing, enhancing flexibility in test environments.
  • seth/README.md

    • Updated documentation to include new client builder methods NewClientBuilderWithConfig and UseNetworkWithChainId, and provided examples of their usage. This improves the documentation's comprehensiveness regarding client configuration.
  • seth/client.go

    • Modified to use uint64 for ChainID instead of converting to and from strings, streamlining chain ID handling.
    • Adjusted NewClientRaw to conditionally fetch ChainID if not provided, ensuring the client has the necessary network information.
  • seth/client_builder.go

    • Introduced new methods (NewClientBuilderWithConfig, UseNetworkWithName, UseNetworkWithChainId, WithNetworkChainId) for enhanced client configuration capabilities, including support for modifying existing configurations and setting the network by chain ID.
    • Added error accumulation during the build process to provide comprehensive feedback on configuration errors.
  • seth/cmd/seth.go

    • Corrected the handling of ChainID to use uint64 directly, aligning with changes in chain ID data types.
  • seth/config.go

    • Added ChainID as a uint64 in the Network struct, directly storing the chain ID in its native format for easier access and manipulation.
  • seth/config_test.go

    • Added tests for new client builder functionalities, ensuring the behavior aligns with expectations, especially when modifying existing configurations or using new methods to set the network.

These changes collectively enhance the SETH tool's configurability, usability, and reliability, catering to a wider range of blockchain development scenarios.

@Tofel Tofel marked this pull request as ready for review September 12, 2024 09:17
@Tofel Tofel requested review from sebawo and a team as code owners September 12, 2024 09:17
@cl-sonarqube-production
Copy link

cl-sonarqube-production bot commented Sep 12, 2024

@Tofel Tofel changed the title [TT-1546] add back chain [TT-1546] add back chain id Sep 16, 2024
@Tofel Tofel merged commit ff808eb into main Sep 16, 2024
40 of 41 checks passed
@Tofel Tofel deleted the tt_1546_add_back_chain_id branch September 16, 2024 08:39
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.

2 participants