5b10ab0 [trivial] Add newlines to end of log messages. (John Newbery)
Pull request description:
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.
Tree-SHA512: 88677afe85c88ce9f58312430e8881916bd76bbc8cd353ff81c97b3de8356680503160992c0ef3ea192b4694e848e9ca2480dbc38fea1776903b3784497f1af6
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.
db983beba6 tests: Add lint-tests.sh which checks the test suite naming convention (practicalswift)
5fd864fe8a tests: Rename test suits not following the test suite naming convention (practicalswift)
7b4a296a71 tests: Add note about test suite naming convention (practicalswift)
Pull request description:
Changes:
* Add note about test suite naming convention
* Fix exceptions
* Add regression test
Rationale:
* Consistent naming of test suites makes programmatic test running of specific tests/subsets of tests easier
* Explicit is better than implicit
Before this commit:
```
$ contrib/devtools/lint-tests.sh
The test suite in file src/test/foo_tests.cpp should be named
"foo_tests". Please make sure the following test suites follow
that convention:
src/test/blockchain_tests.cpp:BOOST_FIXTURE_TEST_SUITE(blockchain_difficulty_tests, BasicTestingSetup)
src/test/prevector_tests.cpp:BOOST_FIXTURE_TEST_SUITE(PrevectorTests, TestingSetup)
src/wallet/test/coinselector_tests.cpp:BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup)
src/wallet/test/crypto_tests.cpp:BOOST_FIXTURE_TEST_SUITE(wallet_crypto, BasicTestingSetup)
$
```
After this commit:
```
$ contrib/devtools/lint-tests.sh
$
```
Tree-SHA512: 7258ab9a6b9b8fc1939efadc619e2f2f02cfce8034c7f2e5dc5ecc769aa12e17f6fb8e363817feaf15c026c5b958b2574525b8d2d3f6be69658679bf8ceea9e9
c7ec524 [wallet] Add dummy wallet init class (John Newbery)
49baa4a [wallet] Use global g_wallet_init_interface to init/destroy the wallet. (John Newbery)
caaf972 [wallet] Create wallet init interface. (John Newbery)
5fb5421 [wallet] Move wallet init functions into WalletInit class. (John Newbery)
Pull request description:
This continues the work of #7965. This PR, along with several others, would remove the remaining dependencies from libbitcoin_server.a on libbitcoin_wallet.a.
To create the interface, I've just translated all the old init.cpp wallet function calls into an interface class. I've not done any thinking about whether it makes sense to change that interface by combining/splitting those calls. This is a purely internal interface, so there's no problem in changing it later.
Tree-SHA512: 32ea57615229c33fd1a7f2f29ebc11bf30337685f7211baffa899823ef74b65dcbf068289c557a161c5afffb51fdc38a2ee8180720371f64d433b12b0615cf3f
This commit creates a global g_wallet_init_interface, which is created
in bitcoind and bitcoin-qt. g_wallet_init_interface is used to init
and destroy the wallet.
This removes the dependency from init.cpp on the wallet library.
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
2fb9c1e shuffle selected coins before transaction finalization (Gregory Sanders)
Pull request description:
Currently inputs are ordered based on COutPoint ordering, which while doesn't leak additional internal wallet state, likely further fingerprints the wallet as a Core wallet to observers.
Note: This slightly changed behavior of `fundrawtransaction` in that the newly-appended inputs will now be shuffled rather than in outpoint-order. This does not break API compatibility.
Simple shuffling of the coins being returned will hopefully allow the wallet to blend in a bit more, in lieu of additional data to find what other wallets are doing, or another standard, ala @gmaxwell's suggested of ordering via scriptPubKey.
Tree-SHA512: 70689a6eccf9fa7fc6e3d884f2eba4b482446a1e6128beff7a98f446d0c60f7966c5a6c55e9b0b3d73a9b539ce54889a26c7efe78ab7f34af386d5e4f3fa6df2
d2527bd Rename wallet_accounts.py test (Russell Yanofsky)
045eeb8 Rename account to label where appropriate (Russell Yanofsky)
Pull request description:
Rename account to label where appropriate
This change only updates strings and adds RPC aliases, but should simplify the implementation of address labels in https://github.com/bitcoin/bitcoin/pull/7729, by getting renaming out of the way and letting that change focus on semantics.
The difference between accounts and labels is that labels apply only to addresses, while accounts apply to both addresses and transactions (transactions have "from" and "to" accounts). The code associating accounts with transactions is clumsy and unreliable so we would like get rid of it.
---
There is a rebased version of #7729 atop this PR at https://github.com/ryanofsky/bitcoin/commits/pr/label, see https://github.com/bitcoin/bitcoin/pull/7729#issuecomment-338417139.
Tree-SHA512: b3f934e612922d6290f50137f8ba71ddfaea4485713c7d97e89400a8b73b09b254f9186dffa462c77f5847721f5af9852b5572ade5443d8ee95dd150b3edb7ff
081bf54 Test that BnB is not used when there are preset inputs (Andrew Chow)
6ef9982 Actually disable BnB when there are preset inputs (Andrew Chow)
Pull request description:
We don't want to use BnB when there are preset inputs because there
is some weirdness with making that work with using the KnapsackSolver
as the fallback. Currently we say that we haven't used bnb when
there are preset inputs, but we don't actually disable BnB. This fixes
that.
I thought this was done originally. I guess it got lost in a rebase somewhere.
Tree-SHA512: 9792c0cdd0736866bddbed20f10b8050104955dc589fba49a0bd61a582ba491c921af2cdcc2269678b7b69275dad5fcf89c71b75c28733c7bacbe52e55891b9c
4d9b4256d8 Fix typos (Dimitris Apostolou)
Pull request description:
Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.
Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06
e5468a19d1 Remove unreachable help conditions (lutangar)
Pull request description:
These conditions on `request.fHelp`, which appears in the body of the following functions are never reached:
* `walletpassphrase`
* `walletpassphrasechange`
* `encryptwallet`
```
...
if (request.fHelp || request.params.size() != 0) {
throw std::runtime_error("");
}
...
if (request.fHelp)
return true;
...
```
The first condition would throw if `request.fHelp` evaluates to `true`.
Tree-SHA512: 1aa41ed233c6bebae27151ab5cc67144d2a408335a3acef3c103e144d6343685f360b1146e14bc8dc1d53d00fcfc6ff1ab6a0eeb0805191172a23b306ab50b79
This change only updates strings and adds RPC aliases, but should simplify the
implementation of address labels in
https://github.com/bitcoin/bitcoin/pull/7729, by getting renaming out of the
way and letting it focus on semantics.
The difference between accounts and labels is that labels apply only to
addresses, while accounts apply to both addresses and transactions
(transactions have "from" and "to" accounts). The code associating accounts
with transactions is clumsy and unreliable so we would like get rid of it.
We don't want to use BnB when there are preset inputs because there
is some weirdness with making that work with using the KnapsackSolver
as the fallback. Currently we say that we haven't used bnb when
there are preset inputs, but we don't actually disable BnB. This fixes
that.
73b5bf2cb Add a test to make sure that negative effective values are filtered (Andrew Chow)
76d2f068a Benchmark BnB in the worst case where it exhausts (Andrew Chow)
6a34ff533 Have SelectCoinsMinConf and SelectCoins use BnB or Knapsack and use it (Andrew Chow)
fab04887c Add a GetMinimumFeeRate function which is wrapped by GetMinimumFee (Andrew Chow)
cd927ff32 Move original knapsack solver tests to coinselector_tests.cpp (Andrew Chow)
fb716f7b2 Move current coin selection algorithm to coinselection.{cpp,h} (Andrew Chow)
4566ab75f Add tests for the Branch and Bound algorithm (Andrew Chow)
4b2716da4 Remove coinselection.h -> wallet.h circular dependency (Andrew Chow)
7d77eb1a5 Use a struct for output eligibility (Andrew Chow)
ce7435cf1 Move output eligibility to a separate function (Andrew Chow)
0185939be Implement Branch and Bound coin selection in a new file (Andrew Chow)
f84fed8eb Store effective value, fee, and long term fee in CInputCoin (Andrew Chow)
12ec29d3b Calculate and store the number of bytes required to spend an input (Andrew Chow)
Pull request description:
This is an implementation of the [Branch and Bound coin selection algorithm written by Murch](http://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf) (@xekyo). I have it set so this algorithm will run first and if it fails, it will fall back to the current coin selection algorithm. The coin selection algorithms and tests have been refactored to separate files instead of having them all in wallet.cpp.
I have added some tests for the new algorithm and a test for all of coin selection in general. However, more tests may be needed, but I will need help with coming up with more test cases.
This PR uses some code borrowed from #10360 to use effective values when selecting coins.
Tree-SHA512: b0500f406bf671e74984fae78e2d0fbc5e321ddf4f06182c5855e9d1984c4ef2764c7586d03e16fa4b578c340b21710324926f9ca472d5447a0d1ed43eb4357e
b4bc32a451 [wallet] Get rid of CWalletTx default constructor (Russell Yanofsky)
a128bdc9e1 [wallet] Construct CWalletTx objects in CommitTransaction (Russell Yanofsky)
Pull request description:
Two commits:
- `Construct CWalletTx objects in CommitTransaction` moves a bunch of CWalletTx initialization into CWallet::CommitTransaction to dedup some code and avoid future inconsistencies in how wallet transactions are created.
- `Get rid of CWalletTx default constructor` does what is described and eliminates the possibility of empty transaction entries being inadvertently created by mapWallet[hash] accesses.
Both of these changes were originally part of #9381
Tree-SHA512: af3841c4f0539e0662d81b33c5369fc70aa06ddde1c59cb00fb21c9e4c7d9ff47f1edc5040cb463af1333838802c56b3ef875b939e2b804ee45b8e0294a4371c
92fabcd44 Add LookupBlockIndex function (João Barbosa)
43a32b739 Add missing cs_lock in CreateWalletFromFile (João Barbosa)
f814a3e8f Fix cs_main lock in LoadExternalBlockFile (João Barbosa)
c651df8b3 Lock cs_main while loading block index in AppInitMain (João Barbosa)
02de6a6bc Assert cs_main is held when accessing mapBlockIndex (João Barbosa)
Pull request description:
Replace all `mapBlockIndex` lookups with the new `LookupBlockIndex()`. In some cases it avoids a second lookup.
Tree-SHA512: ca31118f028a19721f2191d86f2dd398144d04df345694575a64aeb293be2f85785201480c3c578a0ec99690516205708558c0fd4168b09313378fd4e60a8412
42343c748 Split up and sanitize CAccountingEntry serialization (Pieter Wuille)
029ecac1b Split up and sanitize CWalletTx serialization (Pieter Wuille)
Pull request description:
This is a small subset of changes taken from #10785, fixing a few of the craziest constness violations in the serialization code.
`CWalletTx` currently serializes some of its fields by embedding them in a key-value `mapValue`, which is modified (and then fixed up) even from the `Serialize` method (for which `mapValue` is const). `CAccountingEntry` goes even further in that it stores such a map by appending it into `strComment` after a null char, which is again later fixed up again.
Fix this by splitting the serialization and deserialization code, and making the serialization act on a copy of `mapValue` / `strComment`.
Tree-SHA512: 487e04996dea6aba5b9b8bdaf2c4e680808f111a15afc557b8d078e14b01e4f40f8ef27588869be62f9a87052117c17e0a0c26c59150f83472a9076936af035e
Allows SelectCoinsMinConf and SelectCoins be able to switch between
using BnB or Knapsack for choosing coins.
Has SelectCoinsMinConf do the preprocessing necessary to support either
BnB or Knapsack. This includes calculating the filtering the effective
values for each input.
Uses BnB in CreateTransaction to find an exact match for the output.
If BnB fails, it will fallback to the Knapsack solver.
Moves the current coin selection algorithm out of SelectCoinsMinConf
and puts it in coinselection.{cpp,h}. The new function, KnapsackSolver,
instead of taking a vector of COutputs, will take a vector of CInputCoins
that is prepared by SelectCoinsMinConf.
Changes CInputCoin to coinselection and to use CTransactionRef in
order to avoid a circular dependency. Also moves other coin selection
specific variables out of wallet.h to coinselectoin.h
* Z is the zone designator for the zero UTC offset.
* T is the delimiter used to separate date and time.
This makes it clear for the end-user that the date/time logged is
specified in UTC and not in the local time zone.
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.
Construct CWalletTx objects in CWallet::CommitTransaction, instead of having
callers do it. This ensures CWalletTx objects are constructed in a uniform way
and all fields are set.
This also makes it possible to avoid confusing and wasteful CWalletTx copies in
https://github.com/bitcoin/bitcoin/pull/9381
There is no change in behavior.
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
