create_transaction(), send(), should consider Tx in mempool.

[dan-da]

Presently create_transaction() will re-use the same inputs when it is called multiple times in the same block.

eg, consider this loop:

    let mut gsm = global_state_lock
        .lock_guard_mut()
        .await;

    for _ in 0..5 {
        let tx = gsm
            .create_transaction(
                &mut tx_outputs,
                change_key,
                UtxoNotifyMethod::OnChain,
                NeptuneCoins::zero(),
                Timestamp::now(),
            )
            .await?;

        gsm.mempool
            .insert(tx)?;
    }

What happens is that 5 tx get created that all spend the same input.

(but Mempool::insert() de-dups them into just one tx)

This is because create_transaction() calls WalletState::allocate_sufficient_input_funds_from_lock() which fetches unspent utxos that are valid as of tip block. It does not consider utxos in the mempool at all.

This means that:

1. create_transaction() will re-use input utxos.
2. it is not possible to chain unconfirmed tx. eg alice sends 5 to bob, and 3 change to herself. From the change alice sends 2 to carol and 1 change to herself.

Fixing this will require that create_transaction() inspects the utxos in the mempool.

[dan-da]

hmm, digging into the code, I don't immediately see how to achieve unconfirmed tx chaining. Tx validation requires that the PrimitiveWitness have one mutator-set memership proof per input. But input utxos in the mempool are represented as RemovalRecord, with no such membership proof.

I am not certain if it is possible with Neptune's design or not. a question for @aszepieniec.

Even if unconfirmed tx chaining is not presently achievable, create_transaction() can still avoid adding any input that is already in the mempool. That would prevent the input re-use.

[aszepieniec]

About chaining unconfirmed transactions: it is possible in principle, but requires a substantial amount of engineering. What would need to be done is 1) generalize inputs; and 2) write a new SingleProof path.

1. Generalize inputs. Currently, inputs can only be mutator set removal records. Eventually they should instances of an enum that that can be a) a mutator set removal record (for lockable UTXOs); b) a MMR item (for lock-free UTXOs); or c) unconfirmed transaction output.
2. Transactions circulate with a single proof testifying to their validity. There are currently three pathways towards producing such a SingleProof: a) from a collection of proofs of subclaims (such as: owners assent; no inflation; etc.) b) by updating a valid transaction; or c) by merging two valid transactions. Eventually we want to support at least two more pathways: d) by establishing membership in an integral mempool, which is itself a data structure that admits only valid transactions; and e) from chaining valid but unconfirmed transactions and cutting through UTXOs that are simultaneously inputs and outputs.

Right now I think other features are more urgently demanded.

[dan-da]

@aszepieniec somehow I missed your comment above until just now. anyway, please take a look at #197 when u have a chance and let me know your thoughts. low-ish priority -- It can wait until after the merge is completed.

If it's fatally flawed somehow I can still fix this issue by ignoring wallet inputs in the mempool when creating a tx.