[bisq-network/bisq] Implement simple password-based gRPC authentication (#4189)

Stan notifications at github.com
Sun Apr 26 14:46:10 UTC 2020


> 2\. Implement a `setwalletpassword` method that, given a single argument, attempts to encrypt a previously unencrypted wallet using the argument as the wallet password. The method will fail with an appropriate error message if the wallet password has already been set. Given two arguments, the method will attempt to change the wallet password from its old value (1st argument) to a new value (2nd argument). In this way, the `setwalletpassword` method is like combining Bitcoin Core's https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/encryptwallet/ and https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletpassphrasechange/ methods into one.

This brings up the question of method names.  In another branch, I created protos (and impls):

```
service EncryptWallet {
    rpc EncryptWallet (EncryptWalletRequest) returns (EncryptWalletReply) {
    }
}

message EncryptWalletRequest {
    string passphrase = 1;
}

message EncryptWalletReply {
    bool success = 1;
    string error_message = 2; // optional error message
}

service WalletPassphraseChange {
    rpc WalletPassphraseChange (WalletPassphraseChangeRequest) returns (WalletPassphraseChangeReply) {
    }
}

message WalletPassphraseChangeRequest {
    string old_passphrase = 1;
    string new_passphrase = 2;
}

message WalletPassphraseChangeReply {
    bool success = 1;
    string error_message = 2; // optional error message
}
```

They do not combine the two methods into one `setwalletpassword` as you wish, but they are named and work exactly like the eponymously named bitcoind methods.

I would implement the `setwalletpassword` method from this proto:
```
service SetWalletPassword {
    rpc SetWalletPassword (SetWalletPasswordRequest) returns (SetWalletPasswordReply) {
    }
}

message SetWalletPasswordRequest {
    string passphrase = 1;
    string new_passphrase = 2;
}

message SetWalletPasswordReply {
    bool success = 1;
    string error_message = 2; // optional error message
}
```
However you want this done is OK with me, and I think `setwalletpassword` would be consistent with text in the UI.  But I want to bring up the point that if the cli is for developers, and many of those developers are familiar with bitcoin-cli, wouldn't it be nice if we stuck to bitcoin-cli's method and parameter names where appropriate?  If we did, many new users would intuitively know how to use bisq-cli without even having to look at --help.

If you don't want to use existing bitcoind method names (that's OK), I would just like to be informed of what you wanted before I went off to add a bunch of new endpoints.    Beyond that, I am just looking at this from the perspective of someone who has been using bitcoin-cli a lot lately (unfortunately, not wheelin' and deelin' BTC).  

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/4189#issuecomment-619563711
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200426/551a2010/attachment.html>


More information about the bisq-github mailing list