devp2p rlpx eth-test TestBlobTxWithoutSidecar gets stuck

[flcl42]

System information

Geth version: master
CL client & version: no CL
OS & Version: Windows/Linux
Commit hash : 701df4b

Expected behaviour

https://github.com/ethereum/go-ethereum/blob/master/cmd/devp2p/internal/ethtest/suite.go#L1136 fails on timeout

Actual behaviour

In case of not getting expected message it's blocked forever. It timeouts when replaced with

	done := make(chan struct{})
	go func() {
		req, err = readUntil[eth.GetPooledTransactionsPacket](context.Background(), conn)
		close(done)
	}()

	select {
	case <-done:
	case <-time.After(5 * time.Second):
		errc <- fmt.Errorf("timeout waiting for pooled tx request")
		return
	}

but I'm not sure how to set this timeout properly. It's not triggered when the test passes

Steps to reproduce the behaviour

Run .\devp2p.exe rlpx eth-test -node ... -engineapi http://127.0.0.1:8551 -chain ... --run TestBlobTxWithoutSidecar against Nethermind or any node that does not send GetPooledTransactionsPacket in response to a NewPooledTransactionHashesPacket

Backtrace

No logs

[fjl]

Why is Nethermind not fetching the transaction?

[flcl42]

Why is Nethermind not fetching the transaction?

Right now we cache requested hashes immediately on request receival, which leads to no reaction on the second request. It seems not OK, considering the test case, so we are fixing it.

The current issue is not critical for sure, because we will fix the client to make it pass this test. So made it just because it may improve testing experience and there is a small chance that this broken timeout may cause an issue somewhere in more important code

[fjl]

We can also adapt the test to use different tx hashes!

[cskiraly]

But changing the hash would be a different test. Here badTx is not just a generic bad TX, but a one that eventually suppresses (censors) a good TX.