Avoid locking cs_main in some wallet RPC #12559

1. 
   promag commented at 4:24 pm on February 27, 2018: member

   Avoid locking cs_main in the folllowing wallet RPC:

   • decoderawtransaction
   • getnewaddress
   • getrawchangeaddress
   • setlabel
2. promag force-pushed on Feb 27, 2018
3. fanquake added the label RPC/REST/ZMQ on Feb 27, 2018
4. laanwj added the label Wallet on Mar 5, 2018
5. 
   laanwj commented at 9:40 pm on March 5, 2018: member

   Can you please add some rationale as to how you checked that the cs_main lock can be safely removed from these functions?
6. 
   promag commented at 9:55 pm on March 5, 2018: member

   I’ve manually checked what calls were made in each RPC, and checked that none uses something protected by cs_main. I guess this could be checked by AssertLockIsHeld or similar, but it’s not widely adopted yet.
7. 
   laanwj commented at 10:32 pm on March 5, 2018: member

   Thanks. What I’m a little bit worried about is that some of the CWallet calls might internally lock cs_main, which will cause potential deadlocks due to lock ordering if cs_wallet is already held. Or maybe not now, but that someone will do so in the future.
8. 
   promag commented at 2:42 pm on April 24, 2018: member

   @laanwj that would be detected either by review or by lock order check?
9. 
   MarcoFalke commented at 7:26 pm on April 27, 2018: member

   This +6/-7 change should go into a single commit
10. promag force-pushed on Apr 27, 2018
11. 
    promag commented at 8:06 pm on April 27, 2018: member

    @MarcoFalke Squashed 909cad7…eb29e2536. Updated PR description.
12. 
    MarcoFalke commented at 7:09 pm on June 24, 2018: member

    Needs rebase due to travis bug
13. promag force-pushed on Jun 24, 2018
14. 
    promag commented at 7:13 pm on June 24, 2018: member

    Thanks @MarcoFalke, rebased.
15. 
    achow101 commented at 10:14 pm on July 9, 2018: member

    utACK ca7a86acbc3bd0c9f24962e41b9aed36a7a229f4
16. 
    MarcoFalke commented at 5:32 pm on July 10, 2018: member

    utACK ca7a86a. Agree that lock-order inversion is caught by travis or review.
17. DrahtBot added the label Needs rebase on Jul 20, 2018
18. promag force-pushed on Aug 12, 2018
19. 
    promag commented at 0:41 am on August 12, 2018: member

    Rebased.
20. DrahtBot removed the label Needs rebase on Aug 12, 2018
21. 
    MarcoFalke commented at 6:35 pm on August 20, 2018: member

    re-utACK 8c00f6569fb1f69c63a62025f07f7f821ed63064
22. rpc: Avoid locking cs_main in some wallet RPC 00f58f8c48
23. promag force-pushed on Aug 23, 2018
24. 
    promag commented at 0:48 am on August 23, 2018: member

    Removed changes from RPC’s related to accounts to avoid conflict with #14023. Updated OP.
25. 
    MarcoFalke commented at 1:17 am on August 23, 2018: member

    re-utACK 00f58f8c48db05dce9dceed73a0028482e037f0f
26. 
    laanwj commented at 5:38 pm on August 23, 2018: member

    utACK 00f58f8c48db05dce9dceed73a0028482e037f0f
27. laanwj merged this on Aug 23, 2018
28. laanwj closed this on Aug 23, 2018

    ---

29. laanwj referenced this in commit 540bf8aacc on Aug 23, 2018
30. promag deleted the branch on Aug 23, 2018
31. deadalnix referenced this in commit 24d55d31a9 on Jan 14, 2020
32. Munkybooty referenced this in commit ceaf7cde1c on Sep 8, 2021
33. MarcoFalke locked this on Sep 8, 2021

Contributors
promag laanwj MarcoFalke achow101

Labels
Wallet RPC/REST/ZMQ