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

devp2p TestLargeTxRequest hive test fails #1582

Closed
1 task done
Tracked by #1579
Rjected opened this issue Feb 28, 2023 · 4 comments
Closed
1 task done
Tracked by #1579

devp2p TestLargeTxRequest hive test fails #1582

Rjected opened this issue Feb 28, 2023 · 4 comments
Labels
A-devp2p Related to the Ethereum P2P protocol C-bug An unexpected or incorrect behavior

Comments

@Rjected
Copy link
Member

Rjected commented Feb 28, 2023

Describe the bug

Currently the devp2p/eth hive test TestLargeTxRequest fails with the following logs:

not ok 15 TestLargeTxRequest
# failed to send next block: failed to announce block: unexpected: (*ethtest.Error)(could not read from connection: read tcp 172.17.0.3:43840->172.17.0.4:30303: i/o timeout)
# 

The TestLargeTxRequest is implemented here:
https://github.com/ethereum/go-ethereum/blob/c155c8e179519888fce575b276091923af19ffbe/cmd/devp2p/internal/ethtest/suite.go#L454

This is ultimately due to the fact that we disconnect peers that send us block announcements, and do not process block announcements. However, that is not related to what is being tested.

Steps to reproduce

Run the hive tests using the instructions in #851, running the devp2p/eth simulator.

Node logs

No response

Platform(s)

No response

What version/commit are you on?

No response

Code of Conduct

  • I agree to follow the Code of Conduct
@Rjected Rjected added C-bug An unexpected or incorrect behavior A-devp2p Related to the Ethereum P2P protocol labels Feb 28, 2023
@mattsse
Copy link
Collaborator

mattsse commented Feb 28, 2023

failed to send next block: failed to announce block

this is again due to unsupported block announcements.
tbh unsure how to proceed here because I don't think we'll actually need block announcements anymore.

@Rjected
Copy link
Member Author

Rjected commented Mar 3, 2023

tbh unsure how to proceed here because I don't think we'll actually need block announcements anymore.

Yeah, I'm also not sure. One thing we could do is disable the test, and use our own integration tests to test this behavior.

Do we want to use hive to test this?
WDYT?

@mattsse
Copy link
Collaborator

mattsse commented Mar 3, 2023

I think we can skip this test, adding block propagation just for this is not worth it, at least now.

we should port the other test logic for the actual large tx test to a rust nativ test though

@Rjected
Copy link
Member Author

Rjected commented Mar 15, 2023

removed this test - creating an issue for the rust native test though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devp2p Related to the Ethereum P2P protocol C-bug An unexpected or incorrect behavior
Projects
Archived in project
Development

No branches or pull requests

2 participants