Report better transactions errors to lightwalletd

[teor2345]

Motivation

When transactions don't validate, we want to provide wallet users with fast and accurate errors, so they can decide if they need to retry the transaction.

Scheduling

This ticket is optional, because the wallet will show a generic error to the user after 10 blocks.

We should only do this ticket if transaction errors are common during wallet testing.

Specifications

Aditya Kulkarni says that ZecWallet expects the following behaviour:

ZecWallet will expect an error if the tx is malformed or is using the wrong branchID or something like that synchronously. Even though wallets try their best not to construct invalid transactions, it can sometimes happen due to, for example if the user is using another wallet at the same time, and spends the note in another tx constructed in the other wallet.

In the various rollback scenarios you describe the way ZecWallet handles it is that it waits for the txid to show up in a block. If it doesn’t show up within 10 blocks, then it will try to notify the user that the tx failed and ask the user to retry (no automatic retries in ZecWallet)

So in summary, it would be good for zebra to return an error if the tx is invalid so ZecWallet can show a meaningful error message, but ZecWallet will detect if the txid was not mined and show a generic error asking the user to retry.

Designs

Here are some possible ways to make this change:

Wait for errors

This quick change should work:

1. Send the transaction to the mempool, and return any errors to lightwalletd
2. Sleep for a short time, to allow the mempool to do the quick transaction checks
3. Re-send the transaction to the mempool, and return any errors to lightwalletd, ignoring "duplicate queued transaction" errors

Split out the quick checks

This is a more complicated alternative:

1. Split transaction validation into quick and slow checks
2. If requested by the caller, do the quick checks before inserting a transaction into the mempool queue, and return any errors immediately.

The quick checks should include branch IDs, and transparent and shielded spend conflicts. They might take some time, because they have to check the state. So we only want to do them for RPCs.

One drawback of this approach is that it slows down inbound transaction handling a bit. Another is potential timing attacks. But we expect that any transactions sent to the mempool are public anyway.

Related Work

• sendrawtransaction JSON-RPC method #3146
• Send transactions in lightwalletd integration tests #3512

[ftm1000]

Please add your planning poker estimate with ZenHub @conradoplg

[teor2345]

This ticket is blocked by our manual wallet testing - we won't know if we need to fix anything until we've actually tested real wallets.

[ftm1000]

to be reassigned, re-estimated and possibly split. We need to do more testing to work out if anything needs to be fixed

[teor2345]

I think we should open specific tickets for any error handling bugs that we find.