Neptune-Crypto / neptune-core

anonymous peer-to-peer cash
Apache License 2.0
25 stars 7 forks source link

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

Open dan-da opened 3 days ago

dan-da commented 3 days ago

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 commented 9 hours ago

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.