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.

Sign up for GitHub

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

Jump to bottom

add hardhat_getForkedChainId JSON-RPC method #1592

Closed
wants to merge 15 commits into from
Closed

add hardhat_getForkedChainId JSON-RPC method #1592

wants to merge 15 commits into from

Conversation

aftermathdigital
Copy link

  • (If this PR includes a documentation change) This PR's branch was created from the website branch, and this PR uses the website branch as its base branch.
  • (If this PR includes a bug fix) Relevant tests have been included.
  • (If this PR includes a new feature) The change was previously discussed on an Issue or with someone from the team.
  • I didn't do anything of this.

Add the hardhat_getForkedChainId JSON-RPC method - which:

  • In the case this chain is forked, returns the chainId of the chain from which it was forked.
  • In the case this chain is not forked, returns null

Includes a documentation update describing the functionality.

@aftermathdigital aftermathdigital changed the title Hardhat get forked chain add hardhat_getForkedChainId JSON-RPC method Jun 26, 2021
@aftermathdigital aftermathdigital mentioned this pull request Jun 27, 2021
Open
@fvictorio
Copy link
Member

@alcuadrado this is ready (assuming tests will pass). The only thing I'm not sure about is the returned value of the new method. Right now it returns a number, while eth_chainId returns a hex string. IMO we should return that format too. What do you think?

fvictorio
fvictorio reviewed Aug 12, 2021
View reviewed changes
packages/hardhat-core/src/internal/core/providers/chainId.ts
})) as string;

// There's a node returning this as decimal instead of QUANTITY.
// TODO: Document here which node does that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the eth_chainId and net_version methods in geth, nethermind and OE, and they all return a quantity and a decimal, respectively.

I'm ok with erring on the side of safety here, but maybe we can update this comment to say something like:

// Most nodes seem to return a decimal instead of a QUANTITY for this method, but we accept QUANTITYs too just to be extra safe

@CLAassistant
Copy link

CLAassistant commented Nov 4, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@fvictorio
Copy link
Member

I think this is no longer necessary since we implemented #3382?

Sorry @aftermathdigital for not merging it 😞

@fvictorio fvictorio closed this Dec 13, 2022
@aftermathdigital
Copy link
Author

I think this is no longer necessary since we implemented #3382?

Sorry @aftermathdigital for not merging it 😞

No worries, was fun to work on it all the same.

Congratulations on the football!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alcuadrado alcuadrado alcuadrado left review comments

@fvictorio fvictorio fvictorio left review comments

Assignees

@alcuadrado alcuadrado

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aftermathdigital @fvictorio @CLAassistant @alcuadrado