-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
Enforce chainId #324
Enforce chainId #324
Conversation
This reverts commit 20b0adb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! Just a few questions and requests.
mainnet = '1', | ||
kovan = '42', | ||
rinkeby = '4', | ||
goerli = '5', | ||
ropsten = '3', | ||
localhost = '', | ||
rpc = '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we store these in hexadecimal instead?
Possibly related: #326
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wait, this would need a migration as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for transactions, I'd put that together with the work on the tx controller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided to do this in a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* chainif * txcontroller * not default * mutex * test * Revert "mutex" This reverts commit 20b0adb. * tests * fulltest * format * comments
* chainif * txcontroller * not default * mutex * test * Revert "mutex" This reverts commit 20b0adb. * tests * fulltest * format * comments
This PR makes sure that all custom rpc networks have a valid chainId, and disregard the usage of networkId in favor of chainId