denpamusic / php-bitcoinrpc

Fully unit-tested Bitcoin JSON-RPC client based on GuzzleHttp.
MIT License
283 stars 100 forks source link

Supporting wallet passphrase in configs #52

Closed mbsaberi closed 4 years ago

mbsaberi commented 4 years ago

Issue I've encrypted my wallet with a passphrase. Running commands after this change needs using passphrase before calling methods, but I did not find anywhere to set such key.

My solution: I suggest having a new option in configs to allow communicate with wallet using passphrase if it is set.

denpamusic commented 4 years ago

Hi,

There's no real benefits to having an additional config value for passphrase, but it introduces issue with multi-wallet support as well as adds additional complication of having list of methods that require passphrase in client and keeping this list up to date with current Bitcoin Core API.

Hovewer, you can just call WalletPassphrase method before issuing your other commands, that require unlocked wallet, and calling WalletLock after you done.

$bitcoind->walletPassphrase('your_passphrase', 60);
...
// do your operations, that require wallet passphrase here
...
$bitcoind->walletLock();

For convenience sake, you can wrap it in function like this:

use Denpa\Bitcoin\Client as BitcoinClient;

function walletUnlock(BitcoinClient $bitcoind, string $passphrase, callable $fn, int $timeout = 60) : void {
  if ($fn) {
    $bitcoind->walletPassphrase($passphrase, $timeout);
    $fn($bitcoind)
    $bitcoind->walletLock();
  }
}

// then call it like this
walletUnlock($bitcoind, 'your_passphrase', function($bitcoind) {
    // do your operations, that require wallet passphrase here, e. g.
    // $bitcoind->sendToMany(...);
});

That being said, having encrypted wallet and storing passphrase in plain text kind of defeats a whole purpoise of having an encrypted wallet anyway...

mbsaberi commented 4 years ago

Thank you @denpamusic for the reply. I thought using such config may let developer to have less coding for each transaction. However, it is a practical way. Regarding to your last sentence, I think having a wallet without passphrase is dangerous, and developing a platform based on an encrypted wallet has another danger of using that passphrase in the codes. The only thing I can imagine is having an encrypted file containing that passphrase.

denpamusic commented 4 years ago

Thank you for you feedback.

I understand your idea. If possible I still don't want to maintain a list of methods requiring unlocked wallet, since one of advantages of this client is that it doesn't require any update when Core API changes in any way.

If you're ok with manually telling client when you need to unlock wallet, I can implement such configuration parameter using the method I've mentioned above:

use Denpa\Bitcoin\Client as BitcoinClient;

$bitcoind = new BitcoinClient([
    ...
    'passphrase' => 'your_passphrase',
    ...
]);

// when you'll need to execute some commands that require passphrase,
// you'll do it like this:
$bitcoind->withPassphrase(function ($bitcoind) {
    $bitcoind->sendToMany(...);
    ...
});
mbsaberi commented 4 years ago

@denpamusic, thank you for your attention. As using passphrase needs a time in seconds too, and it may differ for each Bitcoin Core action, I think the best solution is unlocking the wallet for each action (Your first reply). Having a fixed time duration in the config is not practical as methods may need different time periods.

$bitcoind->walletPassphrase('your_passphrase', 60);
...
// do your operations, that require wallet passphrase here
...
$bitcoind->walletLock();