RpcUnlockWalletImplicitlyWhenRequired about bitcoinlib HOT 3 CLOSED

Hi Boris, thanks for reporting this. You are correct, this feature hasn't been implemented yet.

The idea was to include this parameter in the .config at the inception of this feature so when implemented existing clients won't break, as it's one of the few parameters required to instantiate the CoinParameters class.

The problem is that each request that needs an unlocked wallet takes a different amount of time to be fulfilled, which is totally unpredictable, for example a SendToAddress() request will take a few milliseconds in a wallet that contains a single unspent output but can take many seconds in a wallet that contains several unspent outputs of small value that have to be combined in an optimal way by the coin selection algorithm to form the requested (by SendToAddress()) amount.

Setting the WalletPassphrase() timeout too high will allow for all requests that make use of the RpcUnlockWalletImplicitlyWhenRequired to be fulfilled but can cause security issues as the wallet will be left unlocked for a longer period of time than needed. Setting it too low can result in failing requests.

And while every app that makes use of this library can adjust the WalletPassphrase timeouts depending on its type, environment, implementation etc, it's very difficult for this library to come up with a timeout value that will fit all cases, so I suggest we remove the RpcUnlockWalletImplicitlyWhenRequired parameter from the code. Feel free to perform this change and submit a pull request, in other case I will remove it in the next release.

from bitcoinlib.

Hi George!

What do you think of using RPC timeout as a value for unlock time? And manual locking after method is finished.

Also, do you know internals of bitcoind? Maybe it's engineered that if unlock timeout expires after you invoked SendToAddress() it would still succeed even if method has not returned?

from bitcoinlib.

We could give it a go, although it could be challenging.

A sample implementation could be:

    private enum Wallet
    {
        Lock,
        Unlock
    }

    private void ImplicitlyLockUnlockWalletIfEnabled(Wallet command)
    {
        if (Parameters.RpcUnlockWalletImplicitlyWhenRequired)
        {
            switch (command)
            {
                case Wallet.Lock:
                    WalletLock();
                    break;

                case Wallet.Unlock:
                    WalletPassphrase(Parameters.WalletPassword, Parameters.RpcRequestTimeoutInSeconds);
                    break;

                default:
                    throw new Exception("Unkown WalletSecurity command!");
            }
        }
    }

And then for every command that required an unlocked wallet we could replace the existing implementation in RpcService.cs with something like that:

    public String DumpPrivKey(String bitcoinAddress)
    {
        ImplicitlyLockUnlockWalletIfEnabled(Wallet.Unlock);
        String response = _rpcConnector.MakeRequest<String>(RpcMethods.dumpprivkey, bitcoinAddress);
        ImplicitlyLockUnlockWalletIfEnabled(Wallet.Lock);
        return response;
    }

but this would require lots of repetition inside the code, so we should keep on looking for a more elegant approach. Thoughts?

from bitcoinlib.