Skip to content

feat(gre): desambiguate chainIds using secondary network name #714

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

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

tmigone
Copy link
Member

@tmigone tmigone commented Sep 22, 2022

Motivation

GRE uses chainId to get details about networks defined in hardhat.config.ts. It's possible that there are multiple networks using the same chainId so previously we used hardhat's main network name to disambiguate (i.e: hardhat, mainnet, localhost).

This works as long as the chain id being disambiguated is the main one (specified by the --network flag), however it won't work if the secondary network is the one affected.

For example, for this hardhat.config.ts:

...
  networks: {
    hardhat: {
      chainId: 1337,
    },
    localhost: {
      chainId: 1337,
      url: `http://localhost:8545`,
    },
    localnitrol1: {
      chainId: 1337,
      url: `http://localhost:8545`,
    },
    localnitrol2: {
      chainId: 412346,
      url: 'http://localhost:8547',
    },
}
...

Running a hardhat command using --network localnitrol1 will work as it will use localnitrol1 network name to disambiguate, selecting localnitrol1 as the L1 and localnitrol2 as the L2.

However running a hardhat command using --network localnitrol2 won't work as it doesn't know which L1 network to choose. It will select localnitrol2 as the L2 but the L1 will be uninitialized.

Changes

This PR changes the GRE network selection so that it obtains the secondary network name from the main network name, and uses that to help disambiguating the networks.

So in the example above it will get the L1 network name localnitrol2 --> localnitrol1, and will then use localnitrol1 to find the correct L1 network.

Signed-off-by: Tomás Migone [email protected]

@codecov
Copy link

codecov bot commented Sep 22, 2022

Codecov Report

Base: 90.57% // Head: 90.57% // No change to project coverage 👍

Coverage data is based on head (ea617f1) compared to base (af17480).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #714   +/-   ##
=======================================
  Coverage   90.57%   90.57%           
=======================================
  Files          35       35           
  Lines        1762     1762           
  Branches      296      296           
=======================================
  Hits         1596     1596           
  Misses        166      166           
Flag Coverage Δ
unittests 90.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tmigone tmigone requested a review from abarmat September 22, 2022 15:57
@tmigone tmigone merged commit 5ab5aa2 into dev Sep 22, 2022
@tmigone tmigone deleted the tmigone/gre-desambiguate branch September 22, 2022 20:36
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