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

Support eth_chainId #1293

Closed
carver opened this issue Mar 20, 2019 · 7 comments

Comments

@carver
Copy link
Collaborator

@carver carver commented Mar 20, 2019

geth and parity both support the endpoint now (admittedly, this is hearsay), so we should to.

Also, we could update the local signing middleware and transaction builder to include the connected chain id when signing.

Huge bonus points for opening a PR to add it to https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1474.md

@kclowes

This comment has been minimized.

Copy link
Contributor

@kclowes kclowes commented Apr 10, 2019

  • eth_chainId implemented - #1295
  • Update local signing middleware and transaction builder to include the connected chainId when signing
  • 🏅Bonus: Add PR to EIP-1474
@cygnusv

This comment has been minimized.

Copy link

@cygnusv cygnusv commented Aug 13, 2019

There's an inconsistency in the docs wrt w3.eth.chainId. They say it returns an integer value, but in fact it just returns the original hex string defined by EIP-695

@carver

This comment has been minimized.

Copy link
Collaborator Author

@carver carver commented Aug 13, 2019

it just returns the original hex string

Ah, sounds like a bug.

@cygnusv

This comment has been minimized.

Copy link

@cygnusv cygnusv commented Aug 13, 2019

My bad! It seems you guys already fixed this in the last release.
#1387

@carver

This comment has been minimized.

Copy link
Collaborator Author

@carver carver commented Aug 13, 2019

Great! I'll close this issue, then.

  • Update local signing middleware and transaction builder to include the connected chainId when signing

@kclowes IIRC, this already happened, right? ^

@carver carver closed this Aug 13, 2019
@kclowes

This comment has been minimized.

Copy link
Contributor

@kclowes kclowes commented Aug 14, 2019

@carver Unless I'm missing something (very possible), I don't think we've added chainId to the middleware and transaction builder yet. I'll open a new issue with a better title though so it's less confusing.

@carver

This comment has been minimized.

Copy link
Collaborator Author

@carver carver commented Aug 14, 2019

Ok sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.