Skip to content

Conversation

@njgheorghita
Copy link
Contributor

What was wrong?

  • Moved eth_protocolVersion endpoint from web3.version.ethereum to web3.eth.protocolVersion
  • Moved web3_clientVersion endpoint from web3.version.node to web3.clientVersion

Upon approval of this pr, I'll create a pr to raise deprecation warnings in v4

Cute Animal Picture

image

@njgheorghita
Copy link
Contributor Author

njgheorghita commented Mar 12, 2019

Now that Version.api is the only method left in the Version module, any thoughts on fully deprecating the Version module and moving api to the web3 namespace (web3.apiVersion)

@njgheorghita njgheorghita force-pushed the relocate-eth_protocolVersion branch 4 times, most recently from a9a3176 to 6f03c1d Compare March 12, 2019 15:47
@njgheorghita njgheorghita force-pushed the relocate-eth_protocolVersion branch from 6f03c1d to e0ec4ac Compare March 12, 2019 16:02
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

Looks good to me @njgheorghita!

@@ -0,0 +1,2 @@
def test_eth_protocolVersion(web3):
assert web3.eth.protocolVersion == '99'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this 99?

Copy link
Member

Choose a reason for hiding this comment

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

I believe it's talking to eth-tester and should probably be updated internally to be 63

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, strange - according to this line it should be returning 63. Will dive deeper into this tomorrow

@njgheorghita njgheorghita merged commit 9bb63e7 into ethereum:master Mar 15, 2019
@njgheorghita njgheorghita deleted the relocate-eth_protocolVersion branch March 15, 2019 08:15
.. code-block:: python
>>> web3.version.node
>>> web3.clientVersion
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is merged already, but I noticed that there isn't documentation for web3.clientVersion. Do you mind adding it when you get a chance? 🙏

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.

3 participants