Skip to content

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

Merged
maflcko merged 1 commit intobitcoin:masterfrom
jonasschnelli:2020/10/wallet_tool_sqlite
Oct 21, 2020
Merged

Show name, format and if uses descriptors in bitcoin-wallet tool#20198
maflcko merged 1 commit intobitcoin:masterfrom
jonasschnelli:2020/10/wallet_tool_sqlite

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Oct 20, 2020

No description provided.

jonatack
jonatack reviewed Oct 20, 2020
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK a4b1588 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
Copy link
Member

jonatack commented Oct 20, 2020

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

@practicalswift
Copy link
Contributor

practicalswift commented Oct 20, 2020

ACK a4b1588: patch looks correct :)

@promag
Copy link
Contributor

promag commented Oct 21, 2020

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

@maflcko maflcko added this to the 0.21.0 milestone Oct 21, 2020
@maflcko
Copy link
Member

maflcko commented Oct 21, 2020

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

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Oct 21, 2020

pushed also @jonatack test (thanks!).

@maflcko
Copy link
Member

maflcko commented Oct 21, 2020

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

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Oct 21, 2020

Squashed.

@jonasschnelli jonasschnelli force-pushed the 2020/10/wallet_tool_sqlite branch from 471dc7c to fa7c585 Compare October 21, 2020 08:42
@maflcko
Copy link
Member

maflcko commented Oct 21, 2020

review ACK fa7c585f9b306f31bcaf2ee492688e618f356108

jonatack
jonatack reviewed Oct 21, 2020
View reviewed changes
@jonasschnelli jonasschnelli force-pushed the 2020/10/wallet_tool_sqlite branch from fa7c585 to 053fbbf Compare October 21, 2020 10:12
maflcko
maflcko reviewed Oct 21, 2020
View reviewed changes
test/functional/tool_wallet.py Outdated
Copy link
Member

@maflcko maflcko Oct 21, 2020

Choose a reason for hiding this comment

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

leftover debug print?

@jonasschnelli jonasschnelli force-pushed the 2020/10/wallet_tool_sqlite branch from 053fbbf to 67f1004 Compare October 21, 2020 11:20
@jonatack
Copy link
Member

jonatack commented Oct 21, 2020

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

@jonasschnelli jonasschnelli force-pushed the 2020/10/wallet_tool_sqlite branch from 67f1004 to fa4074b Compare October 21, 2020 11:28
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Oct 21, 2020

I removed the accidentally added test print.

@maflcko
Copy link
Member

maflcko commented Oct 21, 2020

ACK fa4074b

@jonatack
Copy link
Member

jonatack commented Oct 21, 2020

re-ACK fa4074b

@maflcko maflcko merged commit b46f37b into bitcoin:master Oct 21, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 21, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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 pushed a commit to syscoin/syscoin that referenced this pull request Oct 21, 2020
Merge bitcoin#20198: Show name, format and if uses descriptors in bit…
4897d2f 
…coin-wallet tool

fa4074b Show name, format and if uses descriptors in bitcoin-wallet tool (Jonas Schnelli)

Pull request description:

ACKs for top commit:
  MarcoFalke:
    ACK fa4074b
  jonatack:
    re-ACK fa4074b

Tree-SHA512: cf6ee96ff21532fc4b0ba7a0fdfdc1fa485c9b1495447350fe65cd0bd919e0e0280613933265cdee069b8c29ccf015ac374535a70cac3d4fb89f4d08b3a03519
@DrahtBot DrahtBot mentioned this pull request Oct 21, 2020
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@jonatack jonatack jonatack left review comments

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Wallet

Projects

None yet

Milestone

0.21.0

Development

Successfully merging this pull request may close these issues.

6 participants

@jonasschnelli @jonatack @practicalswift @promag @maflcko @DrahtBot