This moves CWallet members and methods dealing with keys to a new
LegacyScriptPubKeyMan class, and updates calling code to reference the new
class instead of CWallet.
Most of the changes are simple text replacements and variable substitutions
easily verified with:
git log -p -n1 -U0 --word-diff-regex=.
The only nontrivial chunk of code added is the new LegacyScriptPubKeyMan class
declaration, but this code isn't new and is just selectively copied and moved
from the previous CWallet class declaration. This can be verified with:
git log -p -n1 --color-moved=dimmed_zebra src/wallet/scriptpubkeyman.h src/wallet/wallet.h
or
git diff HEAD~1:src/wallet/wallet.h HEAD:src/wallet/scriptpubkeyman.h
This commit does not change behavior.
Start moving wallet and ismine code to scriptpubkeyman.h, scriptpubkeyman.cpp
The easiest way to review this commit is to run:
git log -p -n1 --color-moved=dimmed_zebra
And check that everything is a move (other than includes and copyrights comments).
This commit is move-only and doesn't change code or affect behavior.
cd68594dcd Only check the hash of transactions loaded from disk (Andrew Chow)
Pull request description:
It feels unnecessary to do a full `CheckTransaction` for every transaction saved in the wallet. It should not be possible for an invalid transaction to get into the wallet in the first place, and if there is any disk corruption, the hash check will catch it.
ACKs for top commit:
MarcoFalke:
ACK cd68594dcd
laanwj:
ACK cd68594dcd
promag:
ACK cd68594dcd, AFAICT the check is not needed, hash comparison gives data integrity.
Tree-SHA512: 5b2e719f76097cfbf125392db6cc6c764355c81f0b7a5b60aee4b06af1afcca80cfd38a3cf5307fd9e2c1afc405f8321929a4552943099a8161e6762965451fb
35e60e790f Remove ReadVersion and WriteVersion (Andrew Chow)
b3d4f6c961 Log the actual wallet file version (Andrew Chow)
c88e87c3b2 Remove nFileVersion from CWalletScanState (Andrew Chow)
Pull request description:
The wallet file version is stored in the "minversion" record, not the "version" record. However "version" is no longer used anywhere except to record the highest versioned client which has opened a wallet file (which is currently only used to check whether this was most recently opened by a 0.4.0 or 0.5.0rc1 client which had a broken wallet encryption implementation). Furthermore, "version" was logged to the debug.log which is confusing because it is not the actual wallet file version.
This PR changes it so that this confusion largely no longer exists. The wallet file version logging is changed to use "minversion" and reading and writing the "version" record is no longer publicly exposed to prevent potential confusion about whether the actual file version is being read or written. Lastly, in the one place it is actually used, the variable name is changed from nFileVersion to last_client to better reflect what that record actually represents.
ACKs for top commit:
jb55:
ACK 35e60e7, I compiled locally as a quick sanity check.
ryanofsky:
utACK 35e60e790f. This code still pretty confusing, but a little simpler now. And the previous log statement was really misleading and useless compared to the new one here.
meshcollider:
Looks good, thanks! utACK 35e60e790f
Tree-SHA512: f782b2f215d07fbc9b806322bda8085445b81c02b65ca674a8c6a3e1de505a0abd050669afe0ead4778816144a1c18462e13930071cedb7227a058aeb39493f7
The "version" record that these functions read and write are not
used anywhere in the code except for one place. There is no reason
to expose these functions publicly. Furthermore, this avoids potential
confusion as developers may mistake these functions for actually
reading and writing the wallet version when they do not.
nFileVersion is not the actual file version and is not used except
in one place. So it is removed from CWalletScanState and changed so
that it is just read at the place it is needed. Furthermore, the
"version" record now only indicates the version of the highest
versioned client that has opened a wallet file so the variable
name is changed accordingly
CheckTransaction is a context-free function that does not require access
to the blockchain or mempool. Move it from src/consensus/tx_verify in
lib_server to a new unit src/consensus/tx_check in lib_consensus so that
it can be called by non-server libraries.
When loading the wallet, display the entire path in error messages, instead of
the name (which, for the default wallet, is the empty string.)
When an exception occurs during wallet loading, display e.what() if possible,
instead of nothing.
Store the master key fingerprint and derivation path in the
key metadata. hdKeypath is kept to indicate the seed and for
backwards compatibility, but all key derivation path output
uses the key origin info instead of hdKeypath.
f34c8c466a Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations. (practicalswift)
Pull request description:
Make objects in range declarations immutable by default.
Rationale:
* Immutable objects are easier to reason about.
* Prevents accidental or hard-to-notice change of value.
Tree-SHA512: cad69d35f0cf8a938b848e65dd537c621d96fe3369be306b65ef0cd1baf6cc0a9f28bc230e1e383d810c555a6743d08cb6b2b0bd51856d4611f537a12e5abb8b
This commit does the following changes:
- [wallet] Remove 'account' argument from GetLegacyBalance()
- GetLegacyBalance() is never called with an account argument.
Remove the argument and helper functions.
- [wallet] Remove CWallet::ListAccountCreditDebit()
- Function no longer used.
- [wallet] Remove AccountMove()
- Function no longer used.
- [wallet] Remove AddAccountingEntry()
- Function no longer used.
- [wallet] Remove GetAccountCreditDebit()
- Function no longer used.
- [wallet] Don't rewrite accounting entries when reordering wallet transactions.
- Accounting entries are deprecated. Don't rewrite them to the wallet
database when re-ordering transactions.
- [wallet] Remove WriteAccountingEntry()
- Function no longer used.
- [wallet] Don't read acentry key-values from wallet on load.
- [wallet] Remove ListAccountCreditDebit()
- Function no longer used.
- [wallet] Remove CAccountingEntry class
- No longer used
- [wallet] Remove GetLabelDestination
- Function no longer used.
- [wallet] Delete unused account functions
- ReadAccount
- WriteAccount
- EraseAccount
- DeleteLabel
- [wallet] Remove fromAccount argument from CommitTransaction()
- [wallet] Remove strFromAccount.
- No longer used.
- [wallet] Remove strSentAccount from GetAmounts().
- No longer used.
- [wallet] Update zapwallettxes comment to remove accounts.
- [wallet] Remove CAccount
- No longer used
- [docs] fix typo in release notes for PR 14023
3fc20632a3 qt: Set BLOCK_CHAIN_SIZE = 220 (DrahtBot)
2b6a2f4a28 Regenerate manpages (DrahtBot)
eb7daf4d60 Update copyright headers to 2018 (DrahtBot)
Pull request description:
Some trivial maintenance to avoid having to do it again after the 0.17 branch off.
(The scripts to do this are in `./contrib/`)
Tree-SHA512: 16b2af45e0351b1c691c5311d48025dc6828079e98c2aa2e600dc5910ee8aa01858ca6c356538150dc46fe14c8819ed8ec8e4ec9a0f682b9950dd41bc50518fa
a3fa4d6a6a QA: Fix bug in -usecli logic that converts booleans to non-lowercase strings (Jonas Schnelli)
4704e5f074 [QA] add createwallet disableprivatekey test (Jonas Schnelli)
c7b8f343e9 [Qt] Disable creating receive addresses when private keys are disabled (Jonas Schnelli)
2f15c2bc20 Add disable privatekeys option to createwallet (Jonas Schnelli)
cebefba085 Add option to disable private keys during internal wallet creation (Jonas Schnelli)
9995a602a6 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
Instead of comparing version numbers in the wallet to the client
version number, compare them to the latest supported wallet version
in the client. This allows for wallet version numbers to be unrelated
to the client version number.
72ec5b7 debug log number of unknown wallet records on load (Gregory Sanders)
Pull request description:
This would have saved me some time during wallet debugging, with minimal logging clutter.
Tree-SHA512: e11a4d73a5b1d2bd73fe7b75b62fdfa127e21b8641c5b0c76f14ecd292ab374c0d4749f6bd99919b2b3e9cb00c3b5e8179386eb39ac656698306b3b545ee79f1
Add label API to wallet RPC.
This is one step towards #3816 ("Remove bolt-on account system") although it doesn't
actually remove anything yet.
These initially mirror the account functions, with the following differences:
- These functions aren't DEPRECATED in the help
- Help mentions 'label' instead of accounts. In the language used, labels are
associated with addresses, instead of addresses associated with labels. (unlike
with accounts.)
- Labels have no balance
- No balances in `listlabels`
- `listlabels` has no minconf or watchonly argument
- Like in the GUI, labels can be set on any address, not just receiving addreses
- Unlike accounts, labels can be deleted.
Being unable to delete them is a common annoyance (see #1231).
Currently only by reassigning all addresses using `setlabel`, but an explicit
call `deletelabel` which assigns all address to the default label may make
sense.
Thanks to Pierre Rochard for test fixes.
1f45e21 scripted-diff: Convert 11 enums into scoped enums (C++11) (practicalswift)
Pull request description:
Rationale (from Bjarne Stroustrup's ["C++11 FAQ"](http://www.stroustrup.com/C++11FAQ.html#enum)):
>
> The enum classes ("new enums", "strong enums") address three problems with traditional C++ enumerations:
>
> * conventional enums implicitly convert to int, causing errors when someone does not want an enumeration to act as an integer.
> * conventional enums export their enumerators to the surrounding scope, causing name clashes.
> * the underlying type of an enum cannot be specified, causing confusion, compatibility problems, and makes forward declaration impossible.
>
> The new enums are "enum class" because they combine aspects of traditional enumerations (names values) with aspects of classes (scoped members and absence of conversions).
Tree-SHA512: 9656e1cf4c3cabd4378c7a38d0c2eaf79e4a54d204a3c5762330840e55ee7e141e188a3efb2b4daf0ef3110bbaff80d8b9253abf2a9b015cdc4d60b49ac2b914
No change in behavior in the normal case. But buggy mapWallet lookups with
invalid txids will now throw exceptions instead of inserting dummy entries into
the map, and potentially causing segfaults and other failures.
This also makes it a compiler error to use the mapWallet[hash] syntax which
could create dummy entries.
be8ab7d08 Create new wallet databases as directories rather than files (Russell Yanofsky)
26c06f24e Allow wallet files not in -walletdir directory (Russell Yanofsky)
d8a99f65e Allow wallet files in multiple directories (Russell Yanofsky)
Pull request description:
This change consists of three commits:
* The first commit is a pure refactoring that removes the restriction that two wallets can only be opened at the same time if they are contained in the same directory.
* The second commit removes the restriction that `-wallet` filenames can only refer to files in the `-walletdir` directory.
* The third commit makes second commit a little safer by changing bitcoin to create wallet databases as directories rather than files, so they can be safely backed up.
All three commits should be straightforward:
* The first commit adds around 20 lines of new code and then updates a bunch of function signatures (generally updating them to take plain fs::path parameters, instead of combinations of strings, fs::paths, and objects like CDBEnv and CWalletDBWrapper).
* The second commit removes two `-wallet` filename checks and adds some test cases to the multiwallet unit test.
* The third commit just changes the mapping from specified wallet paths to bdb environment & data paths.
---
**Note:** For anybody looking at this PR for the first time, I think you can skip the comments before _20 Nov_ and start reading at https://github.com/bitcoin/bitcoin/pull/11687#issuecomment-345625565. Comments before _20 Nov_ were about an earlier version of the PR that didn't include the third commit, and then confusion from not seeing the first commit.
Tree-SHA512: 00bbb120fe0df847cf57014f75f1f7f1f58b0b62fa0b3adab4560163ebdfe06ccdfff33b4231693f03c5dc23601cb41954a07bcea9a4919c8d42f7d62bcf6024
