wallet: Fully process previous RPCs before accepting new ones #18840

1. 
   MarcoFalke commented at 1:23 pm on May 1, 2020: member

   The wallet may process calls without waiting on the result and effects of previous calls. This causes failures in user scripts, because later RPCs may depend on the state changes from previous RPCs.

   For example, the following might fail on current master:

   0txid = sendtoaddress(...)
   1bumpfee(txid)
   2abandontransaction(txid)  # fails because tx is still in the mempool
   

   Fixes #18831

2. wallet: Fully process previous wallet calls before accepting new ones fa3f147713
3. MarcoFalke renamed this:
   wallet: Fully process previous wallet calls before accepting new ones
   wallet: Fully process previous RPCs before accepting new ones
   on May 1, 2020
4. fanquake added the label Wallet on May 1, 2020
5. 
   ryanofsky commented at 2:29 pm on May 1, 2020: member

   I think the description of this change is misleading. With this change the wallet isn’t just waiting to “fuly process previous RPCs” it’s also waiting for the node to process it’s own internal events, and is waiting for other external listeners like the GUI, the transaction index, p2p networking code, and other wallets that are loaded.

   I’m also not sure if the change reliably fixes the reported problem, or just fixes it by accidentally by adding a delay. How does SyncWithValidationInterfaceQueue() ensure the new transaction is added to the mempool? And could there be a more direct fix for the abandontransaction error like having bumpfee update the old transaction’s fInMempool flag directly after the new transaction is added?

6. 
   MarcoFalke commented at 2:52 pm on May 1, 2020: member

   ok, closing then
7. MarcoFalke closed this on May 1, 2020

   ---

8. MarcoFalke deleted the branch on May 1, 2020
9. 
   MarcoFalke commented at 3:32 pm on May 1, 2020: member

   How does SyncWithValidationInterfaceQueue() ensure the new transaction is added to the mempool?

   This is not about the new transaction, it is about the replaced transaction (txid in my example that fails). So SyncWithValidationInterfaceQueue ensures that the transaction is properly marked as removed from the mempool when it has in fact been removed.

   And could there be a more direct fix for the abandontransaction error like having bumpfee update the old transaction’s fInMempool flag directly after the new transaction is added?

   Sure, see #18842. But keep in mind that #18842 is less correct, as a transaction that is still in the mempool and has been marked as removed will never be correctly marked as in mempool again.

10. 
    kanzure commented at 7:17 pm on May 1, 2020: contributor

    I think something like this pull request could help with some (but not all) RPC request isolation issues like #9197 (comment)
11. DrahtBot locked this on Feb 15, 2022

Contributors
MarcoFalke ryanofsky kanzure

Labels
Wallet