Retry adding send_raw_transaction to the mempool after missing UTXO failures

[conradoplg]

Motivation

ZecWallet Lite creates transactions that spend transparent outputs from mempool transactions. But Zebra's mempool validation requires all UTXOs to be mined.

So we want send_raw_transaction to support transactions based on unmined UTXOs in the mempool.

We only need to support one level of unmined transaction dependencies, because that is what ZecWallet creates.

Designs

Do what Bitcoin does:

• put transactions received via send_raw_transaction in a RPC-specific queue (we don't need to queue gossiped transactions)
• re-submit failed transactions after receiving each new block
• add a timeout of a few blocks, to clear transactions that will never succeed

Related Work

[ftm1000]

Hey team! Please add your planning poker estimate with ZenHub @conradoplg @dconnolly @gustavovalverde @jvff @oxarbitrage @teor2345 @upbqdn

[oxarbitrage]

How can we reproduce this problem in a test case and where it is happening exactly in the codebase ? If i understand this issue correctly the mempool will reject a transaction if it has utxos referencing another transaction inside the mempool. I can't find where is this is enforced.

I think the first step will be somehow reproduce and then proceed with the work on building the queue

[oxarbitrage]

This ticket might be blocked on #3512

[conradoplg]

@oxarbitrage I think the idea is to:

• Send a transaction A to the mempool using sendrawtransaction
• Send a transaction B to the mempool, which spends an output from A, using sendrawtransaction, right after the previous (i.e. before the previous one is mined)

Since we don't actually have code for generating transactions, we would either need to mock this somehow (e.g. we can mock the transaction verifier to always return true, and thus we can create a transaction with dummy signatures), or do a integration test with a real wallet. Probably both, but the mock test seems to be a good start, if it's doable. I'm not sure how much work it will be, though.

You're correct that this is blocked on #3512 but that's just for testing. If we run out of stuff to implement than it's possible to implement this and leave testing for later.

[oxarbitrage]

Thank you, that is pretty much what i had in mind.

The problem is right now if we do this from a unit/prop test, all transactions will be accepted because we don't verify them (mocked mempool). I think we are going to need a mempool in the tests that implement verification in order to be rejected.

Do you know where in the mempool verification the transactions are rejected by this issue?

[conradoplg]

Thank you, that is pretty much what i had in mind.

The problem is right now if we do this from a unit/prop test, all transactions will be accepted because we don't verify them (mocked mempool). I think we are going to need a mempool in the tests that implement verification in order to be rejected.

Do you know where in the mempool verification the transactions are rejected by this issue?

Good question. I think it will fail transaction validation, not the mempool itself. It will wait for the UTXO to appear in state and eventually timeout, and then be rejected by the mempool, i.e. it will pass through here:

zebra/zebrad/src/components/mempool.rs

Line 348 in 9bf5ca0

storage.reject_if_needed(txid, e);

So we'd need to mock the tx verifier to make the first tx validate but return a TransactionDownloadVerifyError::DownloadFailed on the second.

[oxarbitrage]

Ok cool, that is something to get started. Thank you very much for the input.

[teor2345]

Testing

The problem is right now if we do this from a unit/prop test, all transactions will be accepted because we don't verify them (mocked mempool).

When you use a MockService for the mempool service, you can return whatever mempool responses you like:

1. Expect a request using expect_request or expect_request_that
2. Send the response using the returned ResponseSender's respond method

Design

Currently, the mempool queues new transactions for verification internally. But it doesn't tell us the result of verification.

If we keep that design, we'd need to queue every transaction for retries.

Then there are 3 retry cases we need to handle:

1. The transaction is in the state: remove it from the queue
2. The transaction is in the mempool: remove it from the queue
3. The transaction is not in the state or mempool: re-send the transaction to the mempool

After we've retried each transaction a few times, we should give up. If we give up using a timeout, it will handle high CPU/verification load better.