Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rpc method 'getfundingaddresses' #4299

Merged
merged 4 commits into from
Jun 25, 2020

Conversation

ghubstan
Copy link
Member

This addresses task 1 in issue 4257.

This new gRPC Wallet service method displays the BTC wallet's list of receiving addresses. The balance and number of confirmations for the most recent transaction is displayed to the right of each address. Instead of returning a gRPC data structure to the client, the service method returns a formatted String.

If the BTC wallet has no unused addresses, one will be created and included in the returned list, and it can be used to fund the wallet.

The new method required injection of the BtcWalletService into CoreWalletsService, and the usual boilerplate changes to grpc.proto, CliMain, and GrpcWalletService.

Some of the next PRs (for issue 4257) will require some common functionality within CoreWalletsService. These additional changes were made:

  • a private, class level formatSatoshis function
  • a public getNumConfirmationsForMostRecentTransaction method
  • a public getAddressBalance method
  • a private getAddressEntry method

A unit test that verifies a successful return status was added to cli/test.sh.

This PR should be reviewed/merged after PR 4296.

This change fixes the ambiguity in the original class name, which
implied it was a btc wallet service, not a bsq and btc wallets service.
This commit includes the following changes:

 * New tests for methods `lockwallet`, `unlockwallet`,
   `removewalletpassword`, and `setwalletpassword`.

 * New `getbalance` method error handing tests to verify
   error message correctness when wallet is locked.

 * Update to `getversion` method test -- now expects `1.3.4`.

 * Check for new `[params]` column header in help text.
This addresses task #1 in issue bisq-network#4257.

This new gRPC WalletService method displays the BTC wallet's list of
receiving addresses.  The balance and number of confirmations
for the most recent transaction is displayed to the right of each
address.  Instead of returning a gRPC data structure to the client,
the service method returns a formatted String.

If the BTC wallet has no unused addresses, one will be created and
included in the returned list, and it can be used to fund the wallet.

The new method required injection of the BtcWalletService into CoreWalletsService,
and the usual boilerplate changes to grpc.proto, CliMain, and GrpcWalletService.

Some of the next PRs (for bisq-network#4257) will require some common functionality within
CoreWalletsService, so these additional changes were included:

  * a private, class level formatSatoshis function
  * a public getNumConfirmationsForMostRecentTransaction method
  * a public getAddressBalance method
  * a private getAddressEntry method

A unit test that verifies a successful return status was added to cli/test.sh.
Cleaned up the method body and improved the returned string's
formatting.  Also added a line for this method in the CLI help text.
@ghubstan ghubstan marked this pull request as ready for review June 14, 2020 16:12
@ghubstan
Copy link
Member Author

TODO Re-format the result string as a table, e.g.

Address      Balance          Confirmations
=======      ===========      =============
nM5Y...      ##.########                ###

ghubstan added a commit to ghubstan/bisq that referenced this pull request Jun 16, 2020
Response to comment in PR 4299:
bisq-network#4299 (comment)

This PR should be reviewed/merged after PR 4309.
bisq-network#4309
@cbeams cbeams added this to In progress in Ship Bisq Daemon and API Jun 17, 2020
Copy link
Contributor

@dmos62 dmos62 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK. Some of the suggested changes have been introduced in later PRs.

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@sqrrm sqrrm merged commit 2e415de into bisq-network:master Jun 25, 2020
Ship Bisq Daemon and API automation moved this from In progress to Done Jun 25, 2020
eigentsmis pushed a commit to eigentsmis/bisq that referenced this pull request Jun 26, 2020
Response to comment in PR 4299:
bisq-network#4299 (comment)

This PR should be reviewed/merged after PR 4309.
bisq-network#4309
@ghubstan ghubstan deleted the 1-getfundingaddresses branch June 26, 2020 20:42
@ripcurlx ripcurlx added this to the v1.3.6 milestone Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants