Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGINGd36def6deacb: Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for…
rABCd36def6deacb: Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for…

Summary

a3fa4d6a6acf19d640a1d5879a00aa1f059e2380 QA: Fix bug in -usecli logic that converts booleans to non-lowercase strings (Jonas Schnelli)
4704e5f074e57782d058404a594a7313cf170cf0 [QA] add createwallet disableprivatekey test (Jonas Schnelli)
c7b8f343e99d9d53ea353ddce9a977f1886caf30 [Qt] Disable creating receive addresses when private keys are disabled (Jonas Schnelli)
2f15c2bc20d583b4c1788da78c9c635c36e03ed0 Add disable privatekeys option to createwallet (Jonas Schnelli)
cebefba0855cee7fbcb9474b34e6779369e8e9ce Add option to disable private keys during internal wallet creation (Jonas Schnelli)
9995a602a639b64a749545b7c3bafbf67f97324f Add facility to store wallet flags (64 bits) (Jonas Schnelli)

Pull request description:

This mode ('createwallet {"disableprivatekeys": true}') is intended for a sane pure watch-only mode, ideal for a use-case where one likes to use Bitcoin-Core in conjunction with a hardware-wallet or another solutions for cold-storage.

Since we have support for custom change addresses in `fundrawtransaction`, pure watch-only wallets including coin-selection are possible and do make sense for some use cases.

This new mode disables all forms of private key generation and ensure that no mix between hot and cold keys are possible.

Tree-SHA512: 3ebe7e8d54c4d4e5f790c348d4c292d456f573960a5b04d69ca5ef43a9217c7e7671761c6968cdc56f9a8bc235f3badd358576651af9f10855a0eb731f3fc508

Backport of Core PR9662
https://github.com/bitcoin/bitcoin/pull/9662/

Depends on D4175

Test Plan

make check
test_runner.py
bitcoin-qt
Help -> Debug -> Console
createwallet noprivkey true

Select noprivkey wallet in the main window
In the Receive menu, the Request Payment button should be greyed out and not clickable.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR9662

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7965
Build 13933: Bitcoin ABC Buildbot (legacy)
Build 13932: arc lint + arc unit

Event Timeline

nakihito created this revision.Oct 29 2019, 19:25

Owners added a reviewer: Restricted Owners Package.Oct 29 2019, 19:25

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 29 2019, 19:25

nakihito planned changes to this revision.Oct 29 2019, 19:25

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito requested review of this revision.Oct 29 2019, 19:26

nakihito edited the test plan for this revision. (Show Details)

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7897: Diff 13745.Oct 29 2019, 19:40

nakihito added a child revision: D4305: qt: Use new Qt5 connect syntax.Oct 29 2019, 20:18

You are missing the usecli version of the test.

src/wallet/rpcwallet.cpp
3510 ↗	(On Diff #13745)	C++ style cast
src/wallet/wallet.cpp
1616 ↗	(On Diff #13745)	Fix braces
4564 ↗	(On Diff #13745)	...

This revision now requires changes to proceed.Oct 30 2019, 16:36

Fixed nits, added --usecli version of new test, and updated timing.json with new tests.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7915: Diff 13793.Oct 30 2019, 20:55

nakihito added a child revision: D4343: Update qt/walletmodel.cpp with Qt5 connect syntax.Oct 31 2019, 18:41

nakihito removed a child revision: D4343: Update qt/walletmodel.cpp with Qt5 connect syntax.Oct 31 2019, 20:27

deadalnix requested changes to this revision.Nov 2 2019, 18:49

deadalnix added inline comments.

src/wallet/rpcwallet.cpp
3511 ↗	(On Diff #13793)	Why is that a static cast ? `uint64_t(WALLET_FLAG_DISABLE_PRIVATE_KEYS)` doesn't work ?
src/wallet/wallet.h
1366 ↗	(On Diff #13793)	Relayout

This revision now requires changes to proceed.Nov 2 2019, 18:49

nakihito planned changes to this revision.Nov 4 2019, 07:38

nakihito added inline comments.

src/wallet/rpcwallet.cpp
3511 ↗	(On Diff #13793)	The functional cast does work, but to my knowledge its a generally less safe casting method because it tries all cast types and uses which ever one works first (I believe the first three it tries are const, static, and then dynamic casts). In this case, it would likely end up as a static cast. I will be reverting to the functional cast because this is a rather trivial change and there is no need to give ourselves ownership of this code on the off chance that Core makes some changes that affect this code.

Reverted static_cast to functional cast to prevent issues of code ownership and reformatted a comment.

nakihito planned changes to this revision.Nov 4 2019, 07:39

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7965: Diff 13883.Nov 4 2019, 07:50

nakihito requested review of this revision.Nov 4 2019, 19:34

deadalnix added inline comments.Nov 18 2019, 00:23

src/wallet/rpcwallet.cpp
3511 ↗	(On Diff #13793)	Your answer is applicable to C-style casts. That is not what I suggest you to use.

Fabien requested changes to this revision.Nov 19 2019, 15:04

Fabien added inline comments.

src/wallet/wallet.cpp
4548	It's not what is in the PR.
src/wallet/wallet.h
108	This block of comments and the one above need some love: sentences start capitalized and end with a dot. Edit: there a several places like this in this diff.
1359	Dito.
1362	Dito.
1368	Dito.

This revision now requires changes to proceed.Nov 19 2019, 15:04

Fixed comment formatting.

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito added inline comments.Nov 19 2019, 19:28

src/wallet/wallet.cpp
4548	Sorry, forgot to make a note about this one. This change was made in D4179 (out of order backport). See https://github.com/bitcoin/bitcoin/pull/13774/files#diff-b2bb174788c7409b671c46ccc86034bdR4082

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8164: Diff 14210.Nov 19 2019, 19:38

Fabien accepted this revision.Nov 19 2019, 20:19

deadalnix accepted this revision.Nov 20 2019, 14:30

This revision is now accepted and ready to land.Nov 20 2019, 14:30

Closed by commit rABCd36def6deacb: Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for… (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by nakihito). · Explain WhyNov 20 2019, 19:30

This revision was automatically updated to reflect the committed changes.

nakihito added a commit: rABCd36def6deacb: Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for….

nakihito added a commit: rSTAGINGd36def6deacb: Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for….Apr 7 2020, 21:45

Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for watchonly-wallets
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 13883

src/interfaces/wallet.h

src/interfaces/wallet.cpp

src/qt/receivecoinsdialog.cpp

src/qt/walletmodel.h

src/qt/walletmodel.cpp

src/rpc/client.cpp

src/wallet/rpcwallet.cpp

src/wallet/test/wallet_tests.cpp

src/wallet/wallet.h

src/wallet/wallet.cpp

src/wallet/walletdb.h

src/wallet/walletdb.cpp

test/functional/test_framework/test_node.py

test/functional/test_runner.py

test/functional/timing.json

test/functional/wallet_disableprivatekeys.py

Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for watchonly-walletsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 13883

src/interfaces/wallet.h

src/interfaces/wallet.cpp

src/qt/receivecoinsdialog.cpp

src/qt/walletmodel.h

src/qt/walletmodel.cpp

src/rpc/client.cpp

src/wallet/rpcwallet.cpp

src/wallet/test/wallet_tests.cpp

src/wallet/wallet.h

src/wallet/wallet.cpp

src/wallet/walletdb.h

src/wallet/walletdb.cpp

test/functional/test_framework/test_node.py

test/functional/test_runner.py

test/functional/timing.json

test/functional/wallet_disableprivatekeys.py

Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for watchonly-wallets
ClosedPublic
Actions

Revision Contents
Changeset List