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

Update geth version for integration tests #1301

Merged
merged 2 commits into from
Mar 29, 2019

Conversation

njgheorghita
Copy link
Contributor

@njgheorghita njgheorghita commented Mar 26, 2019

What was wrong?

  • update geth version for integration tests from 1.8.1 to 1.8.22 (inspired by convo here)

  • update go version used in integration tests

  • removed unused geth 1.6 fixture data

  • xfail test for eth_submitHashrate in 1.8.22 geth tests as it appears it has been dropped in favor of ethash_submitHashRate

Cute Animal Picture

image

@njgheorghita njgheorghita force-pushed the geth-integration branch 7 times, most recently from bb625ba to 72687ff Compare March 26, 2019 11:13
@njgheorghita njgheorghita changed the title dirty Update geth version for integration tests Mar 26, 2019
@@ -8,7 +8,8 @@
Web3ModuleTest,
)

GETH_SIGNED_TX = b'\xf8j\x80\x85\x040\xe24\x00\x82R\x08\x94\xdcTM\x1a\xa8\x8f\xf8\xbb\xd2\xf2\xae\xc7T\xb1\xf1\xe9\x9e\x18\x12\xfd\x01\x80\x86\xee\xca\xc4f\xe1\x16\xa0\xbb7^\x1f\xf0\x03(P\x07|\x053Q\xd3M\xf1\x83\xe9\xdcp\xdc\x02\xb4\xe7`\x85\xcd\x84\xdb\xb4\xd0\xaa\xa07\x8cl\xd7\xa6R\x01\xfaW\x0e\x0f\xc1_$\xdf`\x8dO\x18\x1dC\xbc\x87\x8fud\xd2R*W\xfd4' # noqa: E501
GETH_17_SIGNED_TX = b'\xf8j\x80\x85\x040\xe24\x00\x82R\x08\x94\xdcTM\x1a\xa8\x8f\xf8\xbb\xd2\xf2\xae\xc7T\xb1\xf1\xe9\x9e\x18\x12\xfd\x01\x80\x86\xee\xca\xc4f\xe1\x16\xa0\xbb7^\x1f\xf0\x03(P\x07|\x053Q\xd3M\xf1\x83\xe9\xdcp\xdc\x02\xb4\xe7`\x85\xcd\x84\xdb\xb4\xd0\xaa\xa07\x8cl\xd7\xa6R\x01\xfaW\x0e\x0f\xc1_$\xdf`\x8dO\x18\x1dC\xbc\x87\x8fud\xd2R*W\xfd4' # noqa: E501
GETH_18_SIGNED_TX = b'\xf8i\x80\x84;\x9a\xca\x00\x82R\x08\x94\xdcTM\x1a\xa8\x8f\xf8\xbb\xd2\xf2\xae\xc7T\xb1\xf1\xe9\x9e\x18\x12\xfd\x01\x80\x86\xee\xca\xc4f\xe1\x16\xa0W\x91\xe6a\xb7l\x93,\xd3\xdd?;\xa8\x0e\xd3\xbe\x04\x89(\xd49\x1e+\xd6\xee\x88k\xfb\xe7\x83\xeb(\xa0\x04\x9a%`\x91uc\x1a\xdd\xb8\x96\xbb\x10\xb5v|\xcf\x03\xb6\x90\xbb\x93x\xfef\xae\xe1L\xbc7\xafJ' # noqa: E501

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm having a hard time figuring out why these are returning different values for a signed tx. If the parameters are the same (i.e. gas price, to / from, tx...) shouldn't the signed transaction be the same? Why could different versions of geth be returning different values?

Copy link
Member

Choose a reason for hiding this comment

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

I'd decode it (rlp.decode(...)) and see what is different.

if 'v1.8.22' in web3.clientVersion:
# https://github.com/ethereum/go-ethereum/commit/51db5975cc5fb88db6a0dba1826b534fd4df29d7
pytest.xfail('eth_submitHashrate endpoint updated in 1.8.22 to ethash_submitHashRate')
super().test_eth_submitHashrate(web3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I can tell - geth no longer supports the eth_submitHashrate endpoint. What's the strategy for web3.eth.submitHashrate? Better to just leave it alone and let users running the latest geth run into "this method is not available" errors?

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

Will you update this to package the go-ethereum data directory in a zip-file and update tests to manage the unzipping of that fixture. I think it's a much cleaner approach than what I did historically and makes for more manageable diffs.

One thing that @kclowes and I discussed was ensuring that only trusted parties are allowed to update those zipped fixtures since they pose attack surface for someone to get malicious code of some form into our codebase.

@njgheorghita
Copy link
Contributor Author

@pipermerriam ping for final review

@njgheorghita njgheorghita merged commit 57f4a8d into ethereum:master Mar 29, 2019
@njgheorghita njgheorghita deleted the geth-integration branch March 29, 2019 15:39
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.

2 participants