Wallet unconfirmed and mempool is empty

[bokobza]

The wallet gets in a state where the coins are unconfirmed, but the mempool is empty, meaning that nothing has been broadcasted.

I believe this is due to the CrossChaintransferStore changing the state of the wallet. This is not the right place to do it. The wallet should only get changed when a transaction has been boradcasted to the network.
This needs changing asap.

[quantumagi]

@bokobza, this would happen when a transaction had been built and ProcessTransaction had been called to register the transaction to reserve the UTXO's in the wallet. This is so that subsequent BuildTransaction calls do not re-use the UTXO's.

There won't be anything in the mempool due to the fact that all the signatures would not yet have been collected at that point. Once all signatures are collected the transaction would then be broadcast and the mempool would become non-empty.

Unfortunately, in the code that the store has to support, the signature collection is asynchronous so that we need to protect the transaction's UTXO's while signatures are being collected. We also have the situation where the transaction hash changes each time a signature is added so we may need to update the wallet to keep things aligned. This is one of the things I had on my mind when I mentioned that we will probably be doing a lot of simplification in the future. ;)

[dangershony]

I think we should not use the wallet for the pending UTXOsat all, they should get a separate store for that, once a trx is signed and broadcast you can push it out and update the wallet.

This is also to avoid that the member be stuck on a UTXO that got processed by another member

[quantumagi]

Not sure why we should not use the wallet, seeing that its the wallet we need to affect. Why have another store and a parallel set of logic?

Not sure how that will happen seeing that these transactions remain local until ready. Are you sure this is happening vs some signing issue?

Having said that, here is how I think it should work:

1. Let go of the idea of broadcasting transactions between members in a loose asynchronous manner.
2. Have a single method (it could call other methods) used by the leader that:
   • Builds a transaction
   • Passes the transaction among members via parallel synchronous calls for signing.
   • Broadcasts the transaction if enough signatures were collected.
   • This will require a quorum to be online simultaneously but should not be too big an ask.
3. In addition:
   • Consider building a withdrawal transaction for multiple deposits - not just one. The advantages here relate to coin maturity considerations and implicit change re-use.
   • Alternatively send a batch of transactions, related to consecutive deposits, between members for signing.
   • Assuming the first option, change the meaning of the OP_RETURN data to be the FIRST deposit id of the deposits served by the withdrawal. Since we are processing deposits in a deterministic way we should be able to infer the other deposit ids. Alternatively find a way to record those deposit ids in the withdrawal transaction.

[dangershony]

The reason it feels wrong to use the wallet is that, normally when there is an unconfirmed balance then there is an equivalent trx in mempool, not having it this way feels a bit odd.
But I am not strict on it either of it works using the wallet then ok, just make sure it does not break anything in the wallet.