e5b6aef612 Move CBlockFileInfo::ToString method where class is declared (Russell Yanofsky)
f7086fd8ff Add src/wallet/* code to wallet:: namespace (Russell Yanofsky)
90fc8b089d Add src/node/* code to node:: namespace (Russell Yanofsky)
Pull request description:
There are no code changes, this is just adding `namespace` and `using` declarations and `node::` or `wallet::` qualifiers in some places.
Motivations for this change are:
- To make it easier to see when node and wallet code is being accessed places where it shouldn't be. For example if GUI code is accessing node and wallet internals or if wallet and node code are referencing each other.
- To make source code organization clearer ([#15732](https://github.com/bitcoin/bitcoin/issues/15732)), being able to know that `wallet::` code is in `src/wallet/`, `node::` code is in `src/node/`, `init::` code is in `src/init/`, `util::` code is in `src/util/`, etc.
Reviewing with `git log -p -n1 -U0 --word-diff-regex=.` can be helpful to verify this is only updating declarations, not changing code.
ACKs for top commit:
achow101:
ACK e5b6aef612
MarcoFalke:
Concept ACK e5b6aef612🍨
Tree-SHA512: 3797745c90246794e2d55a2ee6e8b0ad5c811e4e03a242d3fdfeb68032f8787f0d48ed4097f6b7730f540220c0af99ef423cd9dbe7f76b2ec12e769a757a2c8d
`fs::path` looks more native than `std::string` for a parameter which
represents a backup file. This change eliminates back-and-forth type
conversions.
Name has been confusing since it was introduced, and it was pointed in
recent review club as https://bitcoincore.reviews/10102 that it was
particularly unclear how interfaces::WalletClient was different from
interfaces::Wallet.
-BEGIN VERIFY SCRIPT-
ren() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
ren WalletClient WalletLoader
ren walletClient walletLoader
ren wallet_client wallet_loader
ren "wallet clients release the wallet" "wallet pointer owners release the wallet"
ren "wallet client" "wallet loader"
ren "Wallet client" "Wallet loader"
-END VERIFY SCRIPT-
Currently restorewallet() logic is written in the RPC layer
and it can´t be reused by GUI. So it reimplements this in the
wallet and interface sections and then, GUI can access it.
Move fillPSBT input-output argument before output-only arguments. This is a
temporary workaround which can go away with improvements to libmultiprocess
code generator. Currently code generator figures out order of input-output
parameters by looking at input list, but it would make more sense for it to
take order from output list, so input-only parameters still have to be first
but there is more flexibility for the other parameters.
1c4b456e1a gui: send using external signer (Sjors Provoost)
24815c6309 gui: wallet creation detects external signer (Sjors Provoost)
3f845ea299 node: add externalSigners to interface (Sjors Provoost)
62ac119f91 gui: display address on external signer (Sjors Provoost)
450cb40a34 wallet: add displayAddress to interface (Sjors Provoost)
eef8d64529 gui: create wallet with external signer (Sjors Provoost)
6cdbc83e93 gui: add external signer path to options dialog (Sjors Provoost)
Pull request description:
Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).
This PR adds GUI support for external signers, based on the since merged bitcoin/bitcoin#16546 (RPC).
The UX isn't amazing - especially the blocking calls - but it works.
First we adds a GUI setting for the signer script (e.g. path to HWI):
<img width="625" alt="Schermafbeelding 2019-08-05 om 19 32 59" src="https://user-images.githubusercontent.com/10217/62483415-e1ff1680-b7b7-11e9-97ca-8d2ce54ca1cb.png">
Then we add an external signer checkbox to the wallet creation dialog:
<img width="374" alt="Schermafbeelding 2019-11-07 om 19 17 23" src="https://user-images.githubusercontent.com/10217/68416387-b57ee000-0194-11ea-9730-127d60273008.png">
It's checked by default if HWI detects a device. It also grabs the name. It then creates a fresh wallet and imports the keys.
You can verify an address on the device (blocking...):
<img width="673" alt="Schermafbeelding 2019-08-05 om 19 29 22" src="https://user-images.githubusercontent.com/10217/62483560-43bf8080-b7b8-11e9-9902-8a036116dc4b.png">
Sending, including coin selection, Just Works(tm) as long the device is present.
~External signer support is enabled by default when the GUI is configured and Boost::Process is present.~
External signer support remains disabled by default, see https://github.com/bitcoin/bitcoin/pull/21935.
ACKs for top commit:
achow101:
Code Review ACK 1c4b456e1a
hebasto:
ACK 1c4b456e1a, tested on Linux Mint 20.1 (Qt 5.12.8) with HWW `2.0.2-rc.1`.
promag:
Tested ACK 1c4b456e1a but rebased with e033ca1379, with HWI 2.0.2, with Nano S and Nano X.
meshcollider:
re-code-review ACK 1c4b456e1a
Tree-SHA512: 3503113c5c69d40adb6ce364d8e7cae23ce82d032a00474ba9aeb6202eb70f496ef4a6bf2e623e5171e524ad31ade7941a4e0e89539c64518aaec74f4562d86b
Stop giving GUI access to destdata rows in database. Replace with narrow
API just for saving and reading receive request information.
This simplifies code and should prevent the GUI from interfering with
other destdata like address-used status.
Note: No user-visible behavior is changing in this commit. New
CWallet::SetAddressReceiveRequest() implementation avoids a bug in
CWallet::AddDestData() where a modification would leave the previous
value in memory while writing the new value to disk. But it doesn't
matter because the GUI doesn't currently expose the ability to modify
receive requests, only to add and erase them.
This is the 43-commit descriptor wallet PR. It was remarkably easy to merge, given
its magnitude. With this commit Elements supports importing Bitcoin descriptors
and deriving (Bitcoin) addresses, though of course it does not support blinding
yet. That is a post-rebase project.
The material changes were:
1. Changing constants in the tests (super annoying but nothing surprising)
2. Adding a missing "skip if this coin is not ours" check in src/script/sign.cpp
which was causing us to erroneously remove existing witnesses from transactions.
This wasn't a problem before this commit since we would only ask specific
scriptpubkeymans to sign, and we'd never ask any to sign inputs we didn't
own. Andy simplified the logic here to always try every scriptpubkeyman,
which means they have to play a bit more nicely with each other.
Other than that, this was a big diff with many conflicts but literally all of
them were "we both added code" and the resolution was to take both sides.
I really like this PR, but it wound up being pretty nontrivial to merge.
The crux is that it pulls PSBT signing logic into scriptpubkey manager,
which is where it belongs, but for us this means reasoning about pegins
inside script/sign.cpp.
However, sign.cpp is part of libbitcoin_common, which does not include
anything for reasoning about PoW or RPC (lol) or anything heavy about
that. This means that some pegin validation had to remain split between
the wallet/rpc layer and sign.cpp. I added a new file script/pegins.cpp
which has the (one) method we actually need in sign.cpp.
Aside from that, this diff is very large but is mostly just moving our
code changes to wallet/psbtwallet.* into wallet/wallet.* where those
functions now live. As far as review, it's probably not worthwhile to
spend too much too much time on this since it's going to be change
again in #16528 and others. The test coverage is pretty extensive.
