Show name, format and if uses descriptors in bitcoin-wallet tool #20198

jonasschnelli commented at 3:28 PM on October 20, 2020: contributor

No description provided.

jonasschnelli added the label Wallet on Oct 20, 2020

jonatack commented at 4:01 PM on October 20, 2020: member

ACK a4b1588c6555b164fdd5e41164bf9358a5194660 except test/functional/tool_wallet.py needs to be updated

manual testing:

$ ./src/bitcoin-wallet -wallet=legacy info
Wallet info
===========
Name: legacy
Format: bdb
Descriptors: no
Encrypted: no
HD (hd seed available): yes
Keypool Size: 2000
Transactions: 28
Address Book: 54

$ ./src/bitcoin-wallet -wallet=new info
Wallet info
===========
Name: new
Format: sqlite
Descriptors: yes
Encrypted: no
HD (hd seed available): yes
Keypool Size: 6000
Transactions: 0
Address Book: 0

jonatack commented at 4:10 PM on October 20, 2020: member

Here's a minimal test update: https://github.com/jonatack/bitcoin/commits/pr-20198-tool_wallet-test

practicalswift commented at 7:19 PM on October 20, 2020: contributor

ACK a4b1588c6555b164fdd5e41164bf9358a5194660: patch looks correct :)

promag commented at 8:22 AM on October 21, 2020: member

Code review ACK, just needs to update test as @jonatack already pointed out.

MarcoFalke added this to the milestone 0.21.0 on Oct 21, 2020

MarcoFalke commented at 8:30 AM on October 21, 2020: member

Assigned 0.21. milestone for the same reason that #20125 got the milestone

jonasschnelli commented at 8:32 AM on October 21, 2020: contributor

pushed also @jonatack test (thanks!).

MarcoFalke commented at 8:34 AM on October 21, 2020: member

Needs to be squashed to not break git bisect with the test_runner ;)

jonasschnelli commented at 8:42 AM on October 21, 2020: contributor

Squashed.

jonasschnelli force-pushed on Oct 21, 2020

MarcoFalke commented at 8:44 AM on October 21, 2020: member

review ACK fa7c585f9b306f31bcaf2ee492688e618f356108

in src/wallet/wallettool.cpp:98 in fa7c585f9b outdated

  94 | @@ -95,6 +95,9 @@ static void WalletShowInfo(CWallet* wallet_instance)
  95 |      LOCK(wallet_instance->cs_wallet);
  96 |  
  97 |      tfm::format(std::cout, "Wallet info\n===========\n");
  98 | +    tfm::format(std::cout, "Name: %s\n", wallet_instance->GetName());

jonatack commented at 8:57 AM on October 21, 2020:

The linter is complaining about trailing whitespace in the test changes, but the whitespace is needed for the test to pass. Maybe print Name: without a space when wallet_instance->GetName().empty() and update the tests.

MarcoFalke commented at 9:01 AM on October 21, 2020:

An alternative would be to write

    Name: \

    Format: bdb

in the test

jonasschnelli commented at 10:12 AM on October 21, 2020:

fixed.

jonatack commented at 10:25 AM on October 21, 2020:

still one left

test/functional/tool_wallet.py:145:18: W291 trailing whitespace

jonasschnelli force-pushed on Oct 21, 2020

in test/functional/tool_wallet.py:111 in 053fbbfb83 outdated

 106 |              HD (hd seed available): yes
 107 |              Keypool Size: 2
 108 |              Transactions: 0
 109 |              Address Book: 3
 110 |          ''')
 111 | +        print(out)

MarcoFalke commented at 10:50 AM on October 21, 2020:

leftover debug print?

jonasschnelli force-pushed on Oct 21, 2020

jonatack commented at 11:26 AM on October 21, 2020: member

ACK 67f100479270ca13d9380c8c9cfe45a0ddbaca7a

The test now prints one of the test outputs.

$ test/functional/tool_wallet.py 
2020-10-21T11:24:29.497000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_9rm_zbea
2020-10-21T11:24:30.136000Z TestFramework (INFO): Testing that various invalid commands raise with specific error messages
2020-10-21T11:24:31.035000Z TestFramework (INFO): Calling wallet tool info, testing output
Wallet info
===========
Name: 
Format: bdb
Descriptors: no
Encrypted: no
HD (hd seed available): yes
Keypool Size: 2
Transactions: 0
Address Book: 3

2020-10-21T11:24:31.706000Z TestFramework (INFO): Generating transaction to mutate wallet
2020-10-21T11:24:32.014000Z TestFramework (INFO): Calling wallet tool info after generating a transaction, testing output
2020-10-21T11:24:32.165000Z TestFramework (INFO): Calling wallet tool create on an existing wallet, testing output
2020-10-21T11:24:34.577000Z TestFramework (INFO): Starting node with arg -wallet=foo
2020-10-21T11:24:35.587000Z TestFramework (INFO): Calling getwalletinfo on a different wallet ("foo"), testing output
2020-10-21T11:24:35.942000Z TestFramework (INFO): Check salvage
2020-10-21T11:24:37.247000Z TestFramework (INFO): Stopping nodes
2020-10-21T11:24:37.247000Z TestFramework (INFO): Cleaning up /tmp/bitcoin_func_test_9rm_zbea on exit
2020-10-21T11:24:37.247000Z TestFramework (INFO): Tests successful

Show name, format and if uses descriptors in bitcoin-wallet tool fa4074b395

jonasschnelli force-pushed on Oct 21, 2020

jonasschnelli commented at 11:28 AM on October 21, 2020: contributor

I removed the accidentally added test print.

MarcoFalke commented at 12:26 PM on October 21, 2020: member

ACK fa4074b395a47c54069bd9f598244701505ff11d

jonatack commented at 12:35 PM on October 21, 2020: member

re-ACK fa4074b395a47c54069bd9f598244701505ff11d

MarcoFalke merged this on Oct 21, 2020

MarcoFalke closed this on Oct 21, 2020

DrahtBot commented at 1:05 PM on October 21, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18788 (tests: Update more tests to work with descriptor wallets by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

sidhujag referenced this in commit 4897d2f08c on Oct 21, 2020

DrahtBot locked this on Feb 15, 2022