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

[laanwj]

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).

[MattFaus]

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

[laanwj]

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.