Commit graph

1581 commits

Author SHA1 Message Date
Andrew Chow
d7637c5a3f After encrypting the wallet, reload the database environment
Calls ReloadDbEnv after encrypting the wallet so that the database
environment is flushed, closed, and reopened to prevent unencrypted
keys from being saved on disk.
2018-08-09 11:27:39 -07:00
Andrew Chow
5d296ac810 Add function to close all Db's and reload the databae environment
Adds a ReloadDbEnv function to BerkeleyEnvironment in order to close all Db
instances, closes the environment, resets it, and then reopens
the BerkeleyEnvironment.

Also adds a ReloadDbEnv function to BerkeleyDatabase that calls
BerkeleyEnvironment's ReloadDbEnv.
2018-08-09 11:27:39 -07:00
Andrew Chow
a769461d5e Move BerkeleyEnvironment deletion from internal method to callsite
Instead of having the object destroy itself, having the caller
destroy it.
2018-08-09 11:27:39 -07:00
Daniel Kraft
227d27e70c Use pushKV in some new PSBT RPCs.
Most of the code uses UniValue::pushKV where appropriate, but some new
RPC code related to PSBTs did not.
2018-08-09 18:08:45 +02:00
MarcoFalke
3e3a50aeb8
Merge #13911: doc: Revert translated string change, clarify wallet log messages
c4a884d555 Trivial: Revert translated string change, clarify wallet log messages (Pierre Rochard)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/pull/12992 post-merge nits from @jnewbery

Tree-SHA512: 002d8a69b489fd216e15b7d6200d7117c489b32405d5e9f514f120d43113fd97ca2f235452b0093e0760bc03baf714edc4564ae14af8456e1b2a54f83c577bf3
2018-08-09 08:11:17 -04:00
MarcoFalke
8eb9870052
Merge #13876: wallet: Catch filesystem_error and raise InitError
fa8527ffec wallet: Catch filesystem_error and raise InitError (MarcoFalke)

Pull request description:

  Fixes #13754 by restoring the previous behaviour

Tree-SHA512: f64052e89f6b332be395df2a5ea6f227c213fa2f38e415e83f30a4fad0938e947e5cddff7902368a43c07be135955a31b90f7eac5a46875c58d54ea95f87f6e6
2018-08-09 08:03:23 -04:00
practicalswift
0e534d4dca Fix incorrect Doxygen comments 2018-08-08 22:14:45 +02:00
Pierre Rochard
c4a884d555 Trivial: Revert translated string change, clarify wallet log messages 2018-08-08 11:00:55 -04:00
Wladimir J. van der Laan
78dae8cacc
Merge #13780: 0.17: Pre-branch maintenance
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
2018-08-08 13:55:27 +02:00
Mason Simon
bb5b1c0b2d [Docs] upgrade rescan time warning from minutes to >1 hour 2018-08-07 22:39:34 -07:00
Ben Woosley
5eb20f81d9
Consistently use ParseHashV to validate hash inputs in rpc
ParseHashV validates the length and encoding of the string and throws
an informative RPC error on failure, which is as good or better than
these alternative calls.

Note I switched ParseHashV to check string length first, because
IsHex tests that the length is even, and an error like:
"must be of length 64 (not 63, for X)" is much more informative than
"must be hexadecimal string (not X)"
2018-08-07 12:47:39 -04:00
Wladimir J. van der Laan
9d86aad287
Merge #13812: wallet: sum ancestors rather than taking max in output groups
23fbbb100f wallet: sum ancestors rather than taking max in output groups (Karl-Johan Alm)

Pull request description:

  This is pointed out in https://github.com/bitcoin/bitcoin/pull/12257#discussion_r204549758.

  Basically, the ancestors gives an indication as to how many ancestors the resulting transaction will have, which is more precise when summing up the values, rather than taking the maximum, since all the coins in the group will become ancestors if selected.

Tree-SHA512: 0588c4b6059669650614817e041526a2ab89dda8c07fca8e077c7669dca1fed51cd164f7df56340840ab60285d48f3b140dcee64f64bf696b2dd4ab16d556a13
2018-08-07 17:23:04 +02:00
Wladimir J. van der Laan
e8f387f997
Merge #13843: [trivial] Add doxygen-compatible comments to CAffectedKeysVisitor
3339d84535 [trivial] add doxygen-compatible comments to CAffectedKeysVisitor (Pierre Rochard)

Pull request description:

Tree-SHA512: 0003fde198a6977d0c8988efc8f76428f9e095009fddf131b07bd9809ef76a778c86bb2b1305e33df16101b6b703cf43eb6193462bb9f3687f98c1d9b109dd96
2018-08-07 16:49:18 +02:00
Wladimir J. van der Laan
51c693d49e
Merge #13657: wallet: assert to ensure accuracy of CMerkleTx::GetBlocksToMaturity
93de2891fa wallet: assert to ensure accuracy of CMerkleTx::GetBlocksToMaturity (Ben Woosley)

Pull request description:

  According to my understanding, it should not be possible for coinbase
  transactions to be conflicting, thus it should not be possible for
  GetDepthInMainChain to return a negative result. If it did, this would
  also result in innacurate results for GetBlocksToMaturity due to the
  math therein. asserting ensures accuracy.

Tree-SHA512: 8e71c26f09fe457cfb00c362ca27066f7f018ea2af1f395090fdc7fd9f5964b76f4317c23f7a4923776f00087558511da5c1c368095be39fb1bacc614a93c32f
2018-08-07 14:19:50 +02:00
Wladimir J. van der Laan
b81a8a5ea9
Merge #13667: wallet: Fix backupwallet for multiwallets
a1a998cf24 wallet: Fix backupwallet for multiwallets (Daniel Kraft)

Pull request description:

  `backupwallet` was broken for multiwallets in their own directories (i.e. something like `DATADIR/wallets/mywallet/wallet.dat`).  In this case, the backup would use `DATADIR/wallets/wallet.dat` as source file and not take the specific wallet's directory into account.

  This led to either an error during the backup (if the wrong source file was not present) or would silently back up the wrong wallet; especially the latter behaviour can be quite bad for users.

Tree-SHA512: 7efe2450ca047e40719fcc7cc211ed94699056020ac737cada7b59e8240298675960570c45079add424d0aab520437d5050d956acd695a9c2452dd4317b4d2c4
2018-08-07 13:31:54 +02:00
practicalswift
d56b73f217 Remove redundant extern 2018-08-07 09:34:45 +02:00
practicalswift
a9e90e5002 Remove redundant redeclaration of rescanblockchain(...) in same scope 2018-08-06 23:52:35 +02:00
MarcoFalke
fa8527ffec
wallet: Catch filesystem_error and raise InitError 2018-08-04 12:04:38 -04:00
Pierre Rochard
3339d84535 [trivial] add doxygen-compatible comments to CAffectedKeysVisitor 2018-08-03 15:30:08 -04:00
Ben Woosley
384273260a
test: Add testing of value_ret for SelectCoinsBnB
Fix that the early bailout optimization tests did not test the actual
selection because their utxo pool was polluted by the make_hard_case test
preceding.
2018-08-03 02:45:20 -04:00
Pierre Rochard
909f54c80a [wallet] Add wallet name to log messages
After multiple wallets became supported, wallet-related log messages
became ambiguous as to which wallet they were being emitted by.

fixes #11317
2018-08-02 12:09:39 -04:00
Varunram
d5f745a5c7
trivial: correct typos 2018-08-02 21:27:17 +08:00
MarcoFalke
c88529a178
Merge #13805: [wallet] Correctly limit output group size
a13647b8bd [qa] Add test for too-large wallet output groups (Suhas Daftuar)
57ec1c97b2 [wallet] correctly limit output group size (Suhas Daftuar)

Pull request description:

  Also add a test to ensure that output groups are being limited, even if a wallet has many outputs corresponding to the same scriptPubKey (the test fails without the first commit).

Tree-SHA512: 2aaa82005b0910488f5cbf40690d4c5e2f46949e299ef70b4cb6e440713811443d411dcbc6d71b1701fd82423073125e21747787d70830cd021c841afb732d51
2018-08-01 12:03:20 -04:00
Karl-Johan Alm
23fbbb100f
wallet: sum ancestors rather than taking max in output groups 2018-07-31 04:54:41 +09:00
Suhas Daftuar
57ec1c97b2 [wallet] correctly limit output group size 2018-07-30 10:43:16 -04:00
Ben Woosley
23f4343781
Add CMerkleTx::IsImmatureCoinBase method
All but one call to GetBlocksToMaturity is testing it relative to 0
for the purposes of determining whether the coinbase tx is immature.
In such case, the value greater than 0 implies that the tx is coinbase,
so there is no need to separately test that status.

This names the concept for easy singular use.
2018-07-29 19:49:14 -04:00
MarcoFalke
fa5ed4f8d2
refactor: Avoid locking tx pool cs thrice 2018-07-29 08:04:06 -04:00
MarcoFalke
ad51e1372b
Merge #13774: Return void instead of bool for functions that cannot fail
d78a8dc3e8 Return void instead of bool for functions that cannot fail (practicalswift)

Pull request description:

  Return `void` instead of `bool` for functions that cannot fail:
  * `CBlockTreeDB::ReadReindexing(...)`
  * `CChainState::ResetBlockFailureFlags(...)`
  * `CTxMemPool::addUnchecked(...)`
  * `CWallet::CommitTransaction(...)`
  * `CWallet::LoadDestData(...)`
  * `CWallet::LoadKeyMetadata(...)`
  * `CWallet::LoadScriptMetadata(...)`
  * `CWallet::LoadToWallet(...)`
  * `CWallet::SetHDChain(...)`
  * `CWallet::SetHDSeed(...)`
  * `PendingWalletTx::commit(...)`
  * `RemoveLocal(...)`
  * `SetMinVersion(...)`
  * `StartHTTPServer(...)`
  * `StartRPC(...)`
  * `TorControlConnection::Disconnect(...)`

  Some of the functions can fail by throwing.

  Found by manually inspecting the following candidate functions:

  ```
  $ git grep -E '(^((static|virtual|inline|friend)[^a-z])*[^a-z]*bool [^=]*\(|return true|return false)' -- "*.cpp" "*.h"
  ```

Tree-SHA512: c0014e045362dbcd1a0cc8f69844e7b8cbae4f538e7632028daeca3a797ac11d8d3d86ebc480bedcb8626df3e96779d592747d52a12556fc49921b114fa0ccc6
2018-07-29 07:56:36 -04:00
practicalswift
620361fce8 Fix accidental use of the addition assignment operator ("+="). Remove newlines from error message. 2018-07-27 18:47:03 +02:00
practicalswift
d78a8dc3e8 Return void instead of bool for functions that cannot fail
* CBlockTreeDB::ReadReindexing(...)
* CChainState::ResetBlockFailureFlags(...)
* CTxMemPool::addUnchecked(...)
* CWallet::LoadDestData(...)
* CWallet::LoadKeyMetadata(...)
* CWallet::LoadScriptMetadata(...)
* CWallet::LoadToWallet(...)
* CWallet::SetHDChain(...)
* CWallet::SetHDSeed(...)
* RemoveLocal(...)
* SetMinVersion(...)
* StartHTTPServer(...)
* StartRPC(...)
* TorControlConnection::Disconnect(...)
2018-07-27 13:19:03 +02:00
DrahtBot
eb7daf4d60 Update copyright headers to 2018 2018-07-27 07:15:02 -04:00
Wladimir J. van der Laan
2d41af1728
Merge #13658: [moveonly] Extract RescanWallet to handle a simple rescan
3fe836b78d [moveonly] Extract RescanWallet to handle a simple rescan (Ben Woosley)

Pull request description:

  Where the outcome does not depend on the result, apart from a simple
  success check.

Tree-SHA512: e0d29c6fc0c7f99a730289e5a80deb586b2848aead56b5198a71ef01f65374812468dfd57be0b8b076eb9be4090d5101d28d979a1d5c3d2f1caeca77b303e90e
2018-07-25 15:39:04 +02:00
MarcoFalke
1211b15bf6
Merge #13656: Remove the boost/algorithm/string/predicate.hpp dependency
e3245f2e7b Removes Boost predicate.hpp dependency (251)

Pull request description:

  This pull request removes the `boost/algorithm/string/predicate.hpp` dependency from the project.

  To replace the the `predicate.hpp` dependency from the project the function calls to `boost::algorithm::starts_with` and `boost::algorithm::ends_with` have been replaced with respectively C++11's `std::basic_string::front` and `std::basic_string::back` function calls.

  Refactors that were not required, but have been done anyways:

  - The Boost function `all` was implicitly made available via the `predicate.hpp` header. Instead of including the appropriate header, function calls to `all` have been replaced with function calls to `std::all_of`.

  - The  `boost::algorithm::is_digit` predicate has been replaced with a custom `IsDigit` function that is locale independent and ASCII deterministic.

Tree-SHA512: 22dda6adfb4d7ac0cabac8cc33e8fb8330c899805acc1ae4ede402c4b11ea75a399414b389dfaa3650d23b47f41351b4650077af9005d598fbe48d5277bdc320
2018-07-24 14:50:05 -04:00
Wladimir J. van der Laan
5f7575e263
Merge #12257: [wallet] Use destination groups instead of coins in coin select
232f96f5c8 doc: Add release notes for -avoidpartialspends (Karl-Johan Alm)
e00b4699cc clean-up: Remove no longer used ivars from CInputCoin (Karl-Johan Alm)
43e04d13b1 wallet: Remove deprecated OutputEligibleForSpending (Karl-Johan Alm)
0128121101 test: Add basic testing for wallet groups (Karl-Johan Alm)
59d6f7b4e2 wallet: Switch to using output groups instead of coins in coin selection (Karl-Johan Alm)
87ebce25d6 wallet: Add output grouping (Karl-Johan Alm)
bb629cb9dc Add -avoidpartialspends and m_avoid_partial_spends (Karl-Johan Alm)
65b3eda458 wallet: Add input bytes to CInputCoin (Karl-Johan Alm)
a443d7a0ca moveonly: CoinElegibilityFilter into coinselection.h (Karl-Johan Alm)
173e18a289 utils: Add insert() convenience templates (Karl-Johan Alm)

Pull request description:

  This PR adds an optional (off by default) `-avoidpartialspends` flag, which changes coin select to use output groups rather than outputs, where each output group corresponds to all outputs with the same destination.

  It is a privacy improvement, as each time you spend some output, any other output that is publicly associated with the destination (address) will also be spent at the same time, at the cost of fee increase for cases where coin select without group restriction would find a more optimal set of coins (see example below).

  For regular use without address reuse, this PR should have no effect on the user experience whatsoever; it only affects users who, for some reason, have multiple outputs with the same destination (i.e. address reuse).

  Nodes with this turned off will still try to avoid partial spending, if the fee of the resulting transaction is not greater than the fee of the original transaction.

  Example: a node has four outputs linked to two addresses `A` and `B`:

  * 1.0 btc to `A`
  * 0.5 btc to `A`
  * 1.0 btc to `B`
  * 0.5 btc to `B`

  The node sends 0.2 btc to `C`. Without `-avoidpartialspends`, the following coin selection will occur:
  * 0.5 btc to `A` or `B` is picked
  * 0.2 btc is output to `C`
  * 0.3 - fee is output to (unique change address)

  With `-avoidpartialspends`, the following will instead happen:
  * Both of (0.5, 1.0) btc to `A` or `B` is picked (one or the other pair)
  * 0.2 btc is output to `C`
  * 1.3 - fee is output to (unique change address)

  As noted, the pro here is that, assuming nobody sends to the address after you spend from it, you will only ever use one address once. The con is that the transaction becomes slightly larger in this case, because it is overpicking outputs to adhere to the no partial spending rule.

  This complements #10386, in particular it addresses @luke-jr and @gmaxwell's concerns in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-300667926 and https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381.

  Together with `-avoidreuse`, this fully addresses the concerns in #10065 I believe.

Tree-SHA512: 24687a4490ba59cf4198ed90052944ff4996653a4257833bb52ed24d058b3e924800c9b3790aeb6be6385b653b49e304453e5d7ff960e64c682fc23bfc447621
2018-07-24 16:34:03 +02:00
Karl-Johan Alm
e00b4699cc
clean-up: Remove no longer used ivars from CInputCoin 2018-07-24 15:05:38 +09:00
Karl-Johan Alm
43e04d13b1
wallet: Remove deprecated OutputEligibleForSpending 2018-07-24 15:05:38 +09:00
Karl-Johan Alm
59d6f7b4e2
wallet: Switch to using output groups instead of coins in coin selection 2018-07-24 15:05:37 +09:00
Karl-Johan Alm
87ebce25d6
wallet: Add output grouping 2018-07-24 15:05:37 +09:00
Karl-Johan Alm
bb629cb9dc
Add -avoidpartialspends and m_avoid_partial_spends 2018-07-24 15:05:37 +09:00
Karl-Johan Alm
65b3eda458
wallet: Add input bytes to CInputCoin
With nInputBytes, coin selection can execute without a reference to the COutput
2018-07-24 15:05:36 +09:00
Karl-Johan Alm
a443d7a0ca
moveonly: CoinElegibilityFilter into coinselection.h 2018-07-24 15:05:36 +09:00
Ben Woosley
984d72ec65
Return the script type from Solver
Because false is synonymous with TX_NONSTANDARD, this conveys the same
information and makes the handling explicitly based on script type,
simplifying each call site.

Prior to this change it was common for the return value to be ignored,
or for the return value and TX_NONSTANDARD to be redundantly handled.
2018-07-22 21:37:00 -04:00
251
e3245f2e7b Removes Boost predicate.hpp dependency
This is a squashed commit that squashes the following commits:

This commit removes the `boost/algorithm/string/predicate.hpp` dependenc
from the project by replacing the function calls to `boost::algorithm::starts_with`
`boost::algorithm::ends_with` and `all` with respectively C++11'
`std::basic_string::front`, `std::basic_string::back`, `std::all_of` function calls

This commit replaces `boost::algorithm::is_digit` with  a locale independent isdigi
function, because the use of the standard library's `isdigit` and `std::isdigit
functions is discoraged in the developer notes
2018-07-22 21:34:45 +02:00
MarcoFalke
62baa28787
Merge #13691: Remove redundant variables, statements and forward declarations
3dee4cc509 Remove redundant statement (practicalswift)
99be644966 Remove redundant unused variables (practicalswift)
66ed242343 Remove redundant forward declaration (practicalswift)

Pull request description:

  Remove redundant …
  * ~access modifiers,~
  * forward declarations,
  * unused variables,
  * statements, and
  * ~return types from lambdas.~

Tree-SHA512: 328bb7d9c45398e44ecbee32095b6376879470dfddbc2180e037620d8390d524b51d7fda112fd58a078715e04432b24dd6998a2459f3550aa0498aa68de866d4
2018-07-22 09:38:10 -04:00
MarcoFalke
e8c74348d3
Merge #13683: wallet: Introduce assertion to document the assumption that cache and cache_used are always set in tandem
d06330396f wallet: Avoid potential null pointer dereference in CWalletTx::GetAvailableCredit(...) (practicalswift)

Pull request description:

  Avoid potential null pointer dereference in `CWalletTx::GetAvailableCredit(...)`.

  Introduced in 4279da4785.

Tree-SHA512: 92d1da9682f0bab11f6f96395ca30549331b0a056cbceb7e1a7f98b3d17d10082aaeed0907cafd46f4164b0e0f4b77e01f78bfd7d24e0503a66c6942ae842aa5
2018-07-22 08:34:09 -04:00
Wladimir J. van der Laan
6b6e854362
Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for watchonly-wallets
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
2018-07-20 14:28:50 +02:00
MarcoFalke
aba2e666d7
Merge #13712: wallet: Fix non-determinism in ParseHDKeypath(...). Avoid using an uninitialized variable in path calculation.
27ee53c1ae wallet: Add error handling. Check return value of ParseUInt32(...) in ParseHDKeypath(...). (practicalswift)
7223263899 wallet: Add tests for ParseHDKeypath(...) (practicalswift)

Pull request description:

  Add error handling. Check return value of `ParseUInt32(...)` in `ParseHDKeypath(...)`.

  `ParseUInt32(...)` returns `false` if the entire string could not be parsed or when an overflow or underflow occurred. In such case the uninitialized variable `number` would be used in the calculation of `path` (prior to this commit).

  An example key path triggering this is `m/0/4294967296`:

  ```
    ParseHDKeypath("m/0/4294967296", keypath);
  ```

  `4294967296` is `1` + `0xFFFFFFFF` (`uint32_t` max: `4294967295`).

  Introduced in a4b06fb42e which was merged into `master` 14 hours ago as part of #13557 ("BIP 174 PSBT Serializations and RPCs").

Tree-SHA512: e5ff423f67c18d82c1231bde6343587a453e793c32004d93dc9b61be6d9372b57a6b2c9978d9eb1000d6cc82fd180f2486013f928dca737fb92daad22c16e467
2018-07-19 17:39:59 -04:00
MarcoFalke
f281f8f755
Merge #13074: [trivial] Correct help text for importaddress RPC
2c71edc2fc [wallet] [rpc] Fix importaddress help text (John Newbery)

Pull request description:

  Help text for `importaddress` referred to the first parameter as `script`, when in fact it's `address`. Calling with a script argument fails:

  ```
  → bcli -named importaddress script=2N3qhMpHK8WNo7wv87W9eHMgvGyJU1593Ei
  error code: -8
  error message:
  Unknown named parameter script
  → bcli -named importaddress address=2N3qhMpHK8WNo7wv87W9eHMgvGyJU1593Ei
  # success!
  ```

Tree-SHA512: 24dcb2cbd0a43e25896b1c67fa0386df2453ec04d49a339e10992417b3921ce3df8a6aa5abba7d2237d6188b018948b2a21ea2f04d37120ad36c31c7b7fc9f1c
2018-07-19 17:34:12 -04:00
practicalswift
27ee53c1ae wallet: Add error handling. Check return value of ParseUInt32(...) in ParseHDKeypath(...). 2018-07-19 21:13:03 +02:00
practicalswift
7223263899 wallet: Add tests for ParseHDKeypath(...) 2018-07-19 21:13:03 +02:00