Wallet re-lock can happen in the middle of RPC call #3673

issue laanwj opened this issue on February 14, 2014
  1. laanwj commented at 11:58 AM on February 14, 2014: member

    Not sure if this is expected: when the wallet unlock times out, this can be in the middle of an operation, resulting in different errors,

    <***> Ik zie zo een sendfrom
    <***>     [code] => -4
    <***>     [message] => Signing transaction failed
    <***> Die komt regelmatig voor
    <***> Deze ook, ook met sendfrom
    <***>     [code] => -13
    <***>     [message] => Error: Please enter the wallet passphrase with walletpassphrase first.
    <***> Of deze, ook met sendfrom
    <***>     [code] => -1
    <***>     [message] => CWallet::GenerateNewKey() : AddKey failed
    <wumpus> ja die -13 zou je verwachten idd
    

    As the cs_wallet lock is always held before calling wallet RPCs, a possible solution would be to aquire cs_wallet in LockWallet (https://github.com/bitcoin/bitcoin/blob/master/src/rpcwallet.cpp#L1545).

  2. laanwj added the label Bug on Feb 14, 2014
  3. laanwj added the label Priority Medium on Feb 14, 2014
  4. laanwj added the label Wallet on Feb 14, 2014
  5. laanwj added this to the milestone 0.9.0 on Feb 14, 2014
  6. laanwj removed this from the milestone 0.9.0 on Feb 24, 2014
  7. MattFaus referenced this in commit 8eb82b4447 on Mar 21, 2014
  8. MattFaus commented at 5:08 PM on March 21, 2014: none

    I took a quick look at this and figured that'd it be easier to add LOCK(cs_nWalletUnlockTime); before all of the EnsureWalletIsUnlocked(); calls. I tried putting this statement inside EnsureWalletIsUnlocked();, but that would complicate the API considerably since the caller would have to receive the lock object and then make sure to unlock it appropriately afterwards.

    I also refactored walletlock to simply call LockWallet() since this looked like duplicate code to me (well, two lines are swapped in order, but that shouldn't matter).

    The changes are here, but I have not been able to get make check to work, so I'm still looking into that.

    https://github.com/MattFaus/bitcoin/commits/wallet_lock2

  9. laanwj removed the label Priority Medium on Feb 16, 2016
  10. laanwj commented at 3:20 PM on September 29, 2016: member

    I'm not convinced that this is a problem. Besides causing RPC to spit out different error messages, it doesn't seem to ever cause real problems such as crashes. Trying to solve it is probably more risky than leaving it as-is.

  11. laanwj closed this on Sep 29, 2016

  12. sidhujag referenced this in commit 67084c6396 on Aug 30, 2020
  13. MarcoFalke locked this on Sep 8, 2021
Contributors
Labels

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 18:16 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me