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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Oct 21, 2020
Merged

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

merged 1 commit into from
Oct 21, 2020

Conversation

jonasschnelli
Copy link
Contributor

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

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

@practicalswift
Copy link
Contributor

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

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

Squashed.

@maflcko
Copy link
Member

maflcko commented Oct 21, 2020

review ACK fa7c585

maflcko
maflcko reviewed Oct 21, 2020
View reviewed changes
test/functional/tool_wallet.py Outdated
Encrypted: no
HD (hd seed available): yes
Keypool Size: 2
Transactions: 0
Address Book: 3
''')
print(out)
Copy link
Member

Choose a reason for hiding this comment

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

leftover debug print?

@jonatack
Copy link
Member

ACK 67f1004

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
Copy link
Contributor Author

I removed the accidentally added test print.

@maflcko
Copy link
Member

maflcko commented Oct 21, 2020

ACK fa4074b

@jonatack
Copy link
Member

re-ACK fa4074b

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

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

@maflcko maflcko maflcko left review comments

@jonatack jonatack jonatack left review comments

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