fake_peer_set::mempool_requests_for_transactions failed with Nil response

[teor2345]

Motivation

In PR #5383, fake_peer_set::mempool_requests_for_transactions failed with:

internal error: entered unreachable code: MempoolTransactionIds requests should always respond Ok(Vec<UnminedTxId>), got Ok(Nil)

Originally posted by @teor2345 in #5383 (comment)

Solution

Just allow a Nil response, the other tests cover this case as well.

[teor2345]

Failed again in PR #5561

Message: response to MempoolTransactionIds request should match added_transaction_ids [Legacy(transaction::Hash("c4eaa58879081de3c24a7b117ed2b28300e7ec4c4c1dff1d3f1268b7857a4ddb"))]

https://github.com/ZcashFoundation/zebra/actions/runs/3413929832/jobs/5681223874#step:14:8817

[teor2345]

Failed again locally:

Message: response to MempoolTransactionIds request should match added_transaction_ids [Legacy(transaction::Hash("c4eaa58879081de3c24a7b117ed2b28300e7ec4c4c1dff1d3f1268b7857a4ddb"))]
Location: zebrad/src/components/inbound/tests/fake_peer_set.rs:81
failures:
components::inbound::tests::fake_peer_set::mempool_requests_for_transactions

[teor2345]

Failed in CI:
https://github.com/ZcashFoundation/zebra/actions/runs/3423847000/jobs/5703529589#step:3:9323

@mpguerra this test seems to be failing a lot, maybe we should just fix it quickly by accepting a Nil response as well?

[mpguerra]

How do we know that it failed with Nil response, as opposed to a different unexpected response? It's not clear to me from the logs...

[teor2345]

How do we know that it failed with Nil response, as opposed to a different unexpected response? It's not clear to me from the logs...

That specific log message is only used when Nil is returned:

zebra/zebrad/src/components/inbound/tests/fake_peer_set.rs

Lines 81 to 85 in 71f5e63

	Ok(Response::Nil) => assert!(
	added_transaction_ids.is_empty(),
	"response to `MempoolTransactionIds` request should match added_transaction_ids {:?}",
	added_transaction_ids
	),

The log should probably also say Nil, or log the actual response.

[mpguerra]

Seen in https://github.com/ZcashFoundation/zebra/actions/runs/3483993359/jobs/5828069602#step:14:5275 also

[mpguerra]

Hey team! Please add your planning poker estimate with Zenhub @arya2 @conradoplg @dconnolly @oxarbitrage @teor2345

[teor2345]

I think we should just allow a Nil response here.

We could also retry the request until it is not Nil, with a sleep in between, and a limit on the number of retries. But that will only work if the transactions are pending, rather than ignored or deleted.

[teor2345]

Failed on PR #5696 in the mergify queue:
https://github.com/ZcashFoundation/zebra/actions/runs/3528199882/jobs/5918491910#step:3:9480

@mpguerra can we schedule this fix in the next sprint? It is a very quick fix.

[teor2345]

Failed in PR #5693:
https://github.com/ZcashFoundation/zebra/actions/runs/3535813495/jobs/5934245122#step:3:9511

[teor2345]

Failed in PR #5710:
https://github.com/ZcashFoundation/zebra/actions/runs/3536581445/jobs/5935745732#step:3:9455

[mpguerra]

Failed on PR #5696 in the mergify queue: https://github.com/ZcashFoundation/zebra/actions/runs/3528199882/jobs/5918491910#step:3:9480

@mpguerra can we schedule this fix in the next sprint? It is a very quick fix.

Added to current sprint (sprint 24)

[teor2345]

Failed again in PR #5710:
https://github.com/ZcashFoundation/zebra/actions/runs/3543565196/jobs/5950131733#step:3:9419

[teor2345]

This has failed at least 5 times today, so I think it's a high priority.

@oxarbitrage can you fix this tomorrow, or would you like me to?

[arya2]

I think we should just allow a Nil response here.

We could also retry the request until it is not Nil, with a sleep in between, and a limit on the number of retries. But that will only work if the transactions are pending, rather than ignored or deleted.

I think the dummy_call() happening when the mempool is enabled for tests calls reject_and_remove_same_effects and the transaction ids are rejected/ignored.