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

Engine now gets chain id from chain service #1014

Merged
merged 2 commits into from
Dec 7, 2022
Merged

Engine now gets chain id from chain service #1014

merged 2 commits into from
Dec 7, 2022

Conversation

lalexgap
Copy link
Contributor

@lalexgap lalexgap commented Dec 7, 2022

Fixes #601

This PR now uses a real chain id instead of hardcoded id of 1337 or 9001 when constructing states. This means states should have the correct chain id and the correctChainId check on the contract should pass. Previously we were using a hardcoded 1337, which would pass on a simulated or hardhat network, but not other chains.

A new function has been added to the ChainService interface, GetChainId. This called by the engine to determine the correct chain id, which is then passed into objectives. I've added a new function to ethChain, `ChainID(ctx context.Context) (*big.Int, error). An actual eth client implements this and for a simulated or mock chain we just return 1337.

I've also tried to clean up uses of a chainId of 9001 and switch them over to a const TEST_CHAIN_ID=1337. This means I touched a lot of tiles but the changes were minor.

@netlify
Copy link

netlify bot commented Dec 7, 2022

Deploy Preview for nitrodocs canceled.

Name Link
🔨 Latest commit 937ae7d
🔍 Latest deploy log https://app.netlify.com/sites/nitrodocs/deploys/6390155e5534ad00085e9295

@lalexgap lalexgap force-pushed the proper-chain-id branch 3 times, most recently from b2a04ef to 758b6d2 Compare December 7, 2022 02:18
@lalexgap lalexgap marked this pull request as ready for review December 7, 2022 02:25
@lalexgap lalexgap merged commit eafac67 into main Dec 7, 2022
@lalexgap lalexgap deleted the proper-chain-id branch December 7, 2022 16:03
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.

ChainId is hardcoded in production code
2 participants