This commit does not change behavior. All it does is pass new function
parameters.
It is easiest to review this change with:
git log -p -n1 -U0 --word-diff-regex=.
Route calls during node initialization and shutdown that would happen between a
node process and wallet processes through the serializable `Chain::Client`
interface, rather than `WalletInitInterface` which is now simpler and only
deals with early initialization and parameter interaction.
This commit mostly does not change behavior. The only change is that the
"Wallet disabled!" and "No wallet support compiled in!" messages are now logged
earlier during startup.
This commit does not change behavior. All it does is pass new function
parameters.
It is easiest to review this change with:
git log -p -n1 -U0 --word-diff-regex=.
fef5adcc33 blockfilter: Use unordered_set instead of set in blockfilter. (Jim Posen)
4fb789e9b2 Extract CSipHasher to it's own file in crypto/ directory. (Jim Posen)
Pull request description:
Use `std::unordered_set` (hash set) instead of `std::set` (tree set) in blockfilter interface, as suggested by @ryanofsky in #12254. This may result in a very minor speedup, but I haven't measured.
This moves `CSipHasher` to it's own file `crypto/siphash.h`, so that it can be used in the libbitcoin_util library without including `hash.{h,cpp}`. I'm open to other suggestions on solving this issue if people would prefer to leave CSipHasher where it is.
Tree-SHA512: 593d1abda771e45f2860d5334272980d20df0b81925a402bb9ee875e17595c2517c0d8ac9c579218b84bbf66e15b49418241c1fe9f9265719bcd2377b0cd0d88
This is a refactoring change that doesn't affect behavior. The motivation
behind the change is give BerkeleyEnvironment objects access to
BerkeleyDatabase objects so it will be possible to simplify the duplicate
wallet check and more reliably avoid opening the same databases twice.
dfef0df840 tests: Dry run bench_bitcoin (-evals=1 -scaling=0: <1 second running time) as part "make check" to allow for quick identification of assertion/sanitizer failures in benchmarking code (practicalswift)
00c6306a61 Remove RUN_BENCH logic (practicalswift)
Pull request description:
Dry run `bench_bitcoin` (`-evals=1 -scaling=0`: <1 second running time) as part `make check` to allow for quick identification of assertion/sanitizer failures or crashes in benchmarking code.
This is already tested in Travis but it is nice to have it locally too. The cost is near zero.
Tree-SHA512: 1f51b86b34bf97f75785f2694891d80f1bfb3e050211e6f6c35d8d9bc80c75bdebaa5ebfa51855ac0cf76d8773c3026bc576f60d0227afb0e646d728b83abde7
a4edb168b6 ZMQ: add options to configure outbound message high water mark, aka SNDHWM (mruddy)
Pull request description:
ZMQ: add options to configure outbound message high water mark, aka SNDHWM
This is my attempt at https://github.com/bitcoin/bitcoin/pull/13315
Tree-SHA512: a4cc3bcf179776899261a97c8c4f31f35d1d8950fd71a09a79c5c064879b38e600b26824c89c4091d941502ed5b0255390882f7d44baf9e6dc49d685a86e8edb
4ed730802f scripted-diff: Rename misleading 'defaultPort' to 'http_port' (Murray Nesbitt)
Pull request description:
`defaultPort` in `HTTPBindAddresses()` is misleadingly named. `defaultPort ` suggests a constant, not something that might be overridden by `-rpcport`.
Tree-SHA512: f6ae8bdc2b4a4f503e44df9efdec32c854d2dede87714399f53791d50cce6bc41c46b01d1583cfc0e3e4777c244e1c74443fa39d9da50a45e53af265b74a17d1
65f3672f3b wallet: Refactor to use WalletLocation (João Barbosa)
01a4c095c8 wallet: Add WalletLocation utility class (João Barbosa)
Pull request description:
Advantages of this change:
- avoid resolving wallet absolute path and name repetitively and in multiple places;
- avoid calling `GetWalletDir` in multiple places;
- extract these details from the actual wallet implementation.
The `WalletLocation` class can be a way to represent a wallet not yet loaded that exists in the wallet directory.
Tree-SHA512: 71ec09786e038499710e7acafe92d66ab9883fc894964e267443ae9c10a6872a10995c3987a169c436a4e793dae96b28fb97bd7f78483c4b72ac930fa23f8686
04972fefd1 Remove unused `adjustedTime` parameter (Hennadii Stepanov)
Pull request description:
After merging #13622 the `adjustedTime` parameter in the `updateStatus` function is unused.
Tree-SHA512: 1d0e03e7343f076ee0032fb721f8ba50571d579958001aab372a43e45b4de24c2bf3bd18c245071cbd69f61ef38182e19666c6f936d55c9085b73c848ba62626
2068f089c8 scripted-diff: Move util files to separate directory. (Jim Posen)
Pull request description:
As discussed [here](https://github.com/bitcoin/bitcoin/pull/14074#issuecomment-429090584), this establishes a `util/` directory to introduce more organizational structure and have a clear place for new util files. It's really not scary to review, it's just one big scripted diff.
Tree-SHA512: 39cf15480d7d35e987b6088d52a857a2d5b1802e36c6b815eb42718d80cd95e669757af9bcc7c04426cd8523662cb1050b8da1e2377d3730672820ed298b894b
Problem:
- IS_TRIVIALLY_CONSTRUCTIBLE macro does not work correctly resulting
in `memset()` usage to set a non-trivial type to 0 when
`nontrivial_t` is passed in from the tests.
- Warning reported by GCC when compiling with `--enable-werror`.
Solution:
- Use the standard algorithm `std::fill_n()` and let the compiler
determine the optimal way of looping or using `memset()`.
14a06525b2 tests: add test for 'getaddressinfo' RPC result 'ischange' field (whythat)
93d1aa9abc rpcwallet: add 'ischange' field to 'getaddressinfo' response (whythat)
Pull request description:
Implementation of proposal in #14396.
This introduces `CWallet::IsChange(CScript&)` method and replaces original `CWallet::IsChange(CTxOut&)` method with overloaded version that delegates to the new method with *txout*'s `scriptPubKey`. In this way `TODO` note from the original method can still be addressed in a single place.
Tree-SHA512: ef5dbc82d76b4b9b2fa6a70abc3385a677c55021f79e187ee2f392ee32bc6b406191f4129acae5c17b0206e72b6712e7e0cad574a4bbd966871c2e656c45e041
This moves the Dock icon click reaction code to the common place and
allows some cleanup in obj_c code.
According to the Apple's docs `class_replaceMethod` behaves as
`class_addMethod`, if the method identified by name does not yet exist;
or as `method_setImplementation`, if it does exist.
Refactor the process of PSBTInput signing to enforce the invariant that
a PSBTInput always has _either_ a witness_utxo or a non_witness_utxo,
never both.
This simplifies the logic of SignPSBTInput slightly, since it no longer
has to deal with the "both" case. When calling it, we now give it, in
order of preference: (1) whichever of the utxo fields was already
present in the PSBT we received, or (2) if neither, the
non_witness_utxo field, which is just a copy of the input transaction,
which we get from the wallet.
SignPSBTInput no longer has to remove one of the two fields; instead, it
will check if we have a witness signature, and if so, it will replace
the non_witness_utxo with the witness_utxo (which is smaller, as it is
just a copy of the output being spent.)
Add PSBTInput::IsSane checks in two more places, which checks for
both utxo fields being present; we will now give an RPC error early on
if we are supplied such a malformed PSBT to fill in.
Also add a check to FillPSBT, to avoid touching any input that is
already signed. (This is now redundant, since we should no longer
potentially harm an already-signed input, but it's harmless.)
fixes#14473
Remove redundant arguments to SignPSBTInput -- since it needs several
bits of the PartiallySignedTransaction, pass in a reference instead of
doing it piecemeal. This saves us having to pass in both a PSBTInput and
its index, as well as having to pass in the CTransaction. Also avoid
redundantly passing the sighash_type, which is contained in the
PSBTInput already.
Switches to named constants, because numeric_limits calls can be harder to read
and less portable.
Change was suggested by James O'Beirne <james.obeirne@gmail.com> in
https://github.com/bitcoin/bitcoin/pull/10973#discussion_r213473620
There are no changes in behavior except on some platforms we don't support
(ILP64, IP16L32, I16LP32), where SignalsOptInRBF() and MutateTxAddInput()
functions would now work correctly.
4fb3388db9 check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow)
Pull request description:
Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it.
It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent.
Tree-SHA512: 50c0c08e201ba02494b369a4d36ddb73e6634eb5a4e4e201c4ef38fd2dbeea2c642b8a04d50c91615da61ecbfade37309e47431368f4b1064539c42015766b50
862d159d63 Add test for conversion from non-witness to witness UTXO (Pieter Wuille)
f8c1714634 Convert non-witness UTXOs to witness if witness sig created (Andrew Chow)
Pull request description:
If a witness signature was created when a non-witness UTXO is used, convert the non-witness UTXO to a witness one.
Port of #14196 to master.
Tree-SHA512: 2235eeb008ffa48e821628032d689e4a83bff6c29b93fa050ab2ee492b0e67b3a30f29a680d4a0e574e05c3a2f9edf0005e161fbe25b7aef2acd034a2424e2f2
c11875c590 Add segwit address tests for importmulti (MeshCollider)
201451b1ca Make getaddressinfo return solvability (MeshCollider)
1753d217ea Add release notes for importmulti segwit change (MeshCollider)
353c064596 Fix typo in test_framework/blocktools (MeshCollider)
f6ed748cf0 Add SegWit support to importmulti with some ProcessImport cleanup (MeshCollider)
Pull request description:
Add support for segwit to importmulti, supports P2WSH, P2WPKH, P2SH-P2WPKH, P2SH-P2WSH. Adds a new `witnessscript` parameter which must be used for the witness scripts in the relevant situations.
Also includes some tests for the various import types.
~Also makes the change in #14019 redundant, but cherry-picks the test from that PR to test the behavior (@achow101).~
Fixes#12253, also addresses the second point in #12703, and fixes#14407
Tree-SHA512: 775a755c524d1c387a99acddd772f677d2073876b72403dcfb92c59f9b405ae13ceedcf4dbd2ee1d7a8db91c494f67ca137161032ee3a2071282eeb411be090a
The custom fee input box now has a minimum value equal to the minimum
required fee. Before a value below the minimum fee could be entered
which was confusing since the minimum fee would still be paid even
though a lower amount was entered.
This adds functions for specifing a min/max value for a
BitcoinAmountField. These options only affect user input, so it's still
possible to use setValue to set values outside of the min/max range. The
existing value will not be changed when calling these functions even if
it's out of range. The min/max range will be reinforced when the field
loses focus.
This also adds `SetAllowEmpty` function which specifies if the field is
allowed to be left empty by the user. If set to false the field will be
set to the minimum allowed value if it's empty when focus is lost.
fa511e8dad Pass tx pool reference into CheckSequenceLocks (MarcoFalke)
Pull request description:
`CheckSequenceLocks` is called from ATMP and the member function `CTxMemPool::removeForReorg` without passing in the tx pool object that is used in those function's scope and instead using the global `::mempool` instance.
This fix should be refactoring only, since currently there is only one (global) tx pool in normal operation. Though, it fixes hard to track down issues in future settings where more than one mempool exists at a time. (E.g. for tests, rpc or p2p tx relay purposes)
Tree-SHA512: f0804588c7d29bb6ff05ec14f22a16422b89ab31ae714f38cd07f811d7dc7907bfd14e799c4c1c3121144ff22711019bbe9212b39e2fd4531936a4119950fa49
ed2e18398b Remove fs::relative call and fix listwalletdir tests (João Barbosa)
Pull request description:
The implementation of `fs::relative` resolves symlinks which is not intended
in ListWalletDir. The replacement does what is required, and `listwalletdir` RPC
tests are fixed accordingly.
Also, `fs::recursive_directory_iterator` iteration is fixed to build with boost 1.47.
Based on #14559
Tree-SHA512: 1da516226073f195285d10d9d9648c90cce0158c5d1eb9c31217bb4abb575cd37f07c00787c5a850554d6120bbc5a3cbc5cb47d4488b32ac6bcb52bc1882d600
3be209d103 rpc: Always throw in getblockstats if -txindex is required (João Barbosa)
Pull request description:
Previously blocks with only the coinbase transaction didn't cause
the RPC error even if the requested stats required -txindex and
it wasn't enabled.
Fixes#14499.
Tree-SHA512: d3a6402889e3ce7199632e79eba66d7d471ff7de5c564d35312e2340cc6d84ef544a8172548fbc2eedf5e637b56dc57bbf7a9815ab798c7f226755f897fd8f3e
The implementation of fs::relative resolves symlinks which is not intended
in ListWalletDir. The replacement does what is required, and listwalletdir
tests are fixed accordingly.
Also, building with boost 1.47 required 2 changes:
- replace fs::relative with an alternative implementation;
- fix fs::recursive_directory_iterator iteration.
4ea77320c5 tests: add test case for loading copied wallet twice (Chun Kuan Lee)
2d796faf62 wallet: Fix duplicate fileid (Chun Kuan Lee)
Pull request description:
The implementation in current master can not detect if the file ID is duplicate with flushed `BerkeleyEnvironment`. This PR would store the file ID in a global variable `g_fileids` and release it when the `BerkeleyDatabase` close. So it won't have to rely on a `Db*`.
Fix#14304
Tree-SHA512: 0632254b696bb4c671b5e2e5781e9012df54ba3c6ab0f919d9f6d31f374d3b0f8bd968b90b537884ac8c3d2906afdd58c2ce258666263464c7dbd636960b0e8f
8907df9e02 qa: Ensure wallet unload during walletpassphrase timeout (João Barbosa)
321decffa1 rpc: Fix wallet unload during walletpassphrase timeout (João Barbosa)
Pull request description:
Replaces the raw wallet pointer in the `RPCRunLater` callback with a `std::weak_ptr` to check if the wallet is not expired.
To test:
```
bitcoind -regtest
bitcoin-cli -regtest encryptwallet foobar
bitcoin-cli -regtest walletpassphrase foobar 5 && bitcoin-cli -regtest unloadwallet ""
```
Fixes#14452.
Tree-SHA512: 311e839234f5fb7955ab5412a2cfc1903ee7132ea56a8ab992ede3614586834886bd65192b76531ae0aa3a526b38e70ca2e1cdbabe52995906ff97b49d93c268
48439b3c10 Don't link SSL_LIBS with GUI unless BIP70 is enabled (James Hilliard)
fbb643d2a5 Add BIP70 deprecation warning (James Hilliard)
38b98507cd qt: cleanup: Move BIP70 functions together in paymentserver (Wladimir J. van der Laan)
9dcf6c0dfe build: Add --disable-bip70 configure option (Wladimir J. van der Laan)
Pull request description:
This is based off of #11622 and adds a deprecation warning when a BIP70 URL is used.
Rational:
- BIP70 increases attack surface in multiple ways and is difficult for third party wallets to implement in a secure manner
- Very few merchants use the standard BIP70 variant supported by Bitcoin Core
- The one major payment processor that doesn't support BIP21 and currently uses a customized non-standard version of BIP70 has indicated that "Unfortunately the original BIP70 is not useful for us."
Tree-SHA512: 1e16ee8d2cdac9499f751ee7b50d058278150f9e38a87a47ddb5105dd0353cdedabe462903f54ead6209b249b249fe5e6a10d29631531be27400f2f69c25b9b9
0089905361 Add compile time checking for cs_main locks which we assert at run time (practicalswift)
Pull request description:
Assert locking requirements at compile-time (`EXCLUSIVE_LOCKS_REQUIRED(foo)`) instead of at run-time (`AssertLockHeld(…)`).
Tree-SHA512: f4965ebf4bb5dbf5e7ed738cacf82c0f6cd55134fb968860bf84a84e29806485617f223910bb8c5461213f1829b0137c64ba1f6d6a2008b3cac3bb3a28df9324
d4d70eda33 Fix listreceivedbyaddress not taking address as a string (Eric Scrivner)
Pull request description:
Fixes#14173. Add the patch in #14173 and include a regression test.
Tree-SHA512: 5a9794e0c43e90d18c899841afbaf15eb9129d7d2f6570fccf0a1793697fe170d224c3c3995b1a35c536fac19819042823d9e3bd23b019d0f03434499243d2f5
97ddc6026b validation: Pass chainparams in AcceptToMemoryPoolWorker(...) (practicalswift)
Pull request description:
Remove unused `CChainParams` argument in `AcceptToMemoryPoolWorker(...)`.
After the merge of #13527 ("policy: Remove promiscuousmempoolflags") yesterday the `CChainParams` argument is no longer used in `AcceptToMemoryPoolWorker(...)`.
Tree-SHA512: f1bab4498b64f0ab5230b8172f860df8fa8a302e4ee7385be4ba9c65a37cbc3ef640df78348c477169b9414e5c6a160a0b6471a11f4bb27921500ec208ef5340
fadffae17f Revert "Make qt wallet test compatible with qt4" (MarcoFalke)
Pull request description:
This removes the file added in #10098
Tree-SHA512: 4aa887ec02aec943f3d74f469c935acf57e43ddbea6f979bb37d3a14670571fa2276cf578e8978de92b5854d8b462f24e5e0addfb27b92fbedf90726eb5d985e
29ed2d64f6 Improve CAmount tests (Hennadii Stepanov)
Pull request description:
This provides:
- more `MoneyRange` tests;
- new `CFeeRate` constructor tests with zero byte size;
- explicit using of the `CAmount` type.
Tree-SHA512: ca0ad6ccb37909a2a5c11034dc07b316a84c32fb40c6f8b6cfc28ebec72a1de157f31d22e767ae80d70ed06d7296f23870cc5ed0689f34a754ae763d50e23d43
a3197c5294 Disable wallet and address book Qt tests on macOS minimal platform (Russell Yanofsky)
Pull request description:
macOS minimal platform is frequently broken, and these are currently failing with Qt 5.11.1.
The tests do pass when run on the full cocoa platform (with `test_bitcoin-qt -platform cocoa`).
Stack trace from test crash: https://gist.github.com/ryanofsky/3401fb63c52d13d5585e7fc777361f1e
Tree-SHA512: a05644ef15d75ea7d7f85ea804c6a5fe78e4e7358b189cbab639d9f7dc46163a35f77f7a2b4ae2fd6be5b9fb22898386b4d88069d5ee8d5fdbd995157c6f0846
1f01fe0257 bitcoin-tx: Use constant for n pubkeys check (Antoine Le Calvez)
Pull request description:
Use the constant for the maximum number of public keys in a multisig script defined in script/script.h instead of hardcoding it.
Tree-SHA512: 83e6c46df907944d0d993159955e402784415536d61fdb5a5becba2b042e37ad2a291b27301c1b169416cb71c823a571d82257512cd4a64848a27a24c875fcc6
369244f654 utils: Fix broken Windows filelock (Chun Kuan Lee)
Pull request description:
Fix broken filelock on Windows, also add a test for this. It's a regression introduced by #13862.
Tree-SHA512: 15665b1930cf39ec71f3ab07def8e2897659f6fd4d2de749d63a5a8ec920e4a04282f12bc262f242b1b3d14d2dd9fa191ddbcf16a46fb927b5b2b14d9f6b5d01
d56a068935 docs: Add release notes for listwalletdir RPC (João Barbosa)
0cb3cad166 qa: Add tests for listwalletdir RPC (João Barbosa)
cc3377360c rpc: Add listwalletdir RPC (João Barbosa)
d1b03b8e5f interfaces: Add getWalletDir and listWalletDir to Node (João Barbosa)
fc4db35bfd wallet: Add ListWalletDir utility (João Barbosa)
Pull request description:
`ListWalletDir` returns all available wallets in the current wallet directory.
Based on MeshCollider work in pull #11485.
Tree-SHA512: 5843e3dbd1e0449f55bb8ea7c241a536078ff6ffcaad88ce5fcf8963971d48c78600fbc4f44919523b8a92329d5d8a5f567a3e0ccb0270fdd27366e19603a716
2ab9140c92 Add tooltips for both datadir and blocksdir (Hennadii Stepanov)
3045704502 Add "Blocksdir" to Debug window (Hennadii Stepanov)
Pull request description:
To get the current `blocksdir` is valuable for debug purposes after
merging #12653.
Tree-SHA512: a93f2c00ee19cf6acb499d3bd9bccf4be8ef01c53c44d917ad401aa4797db02cbccb71a9c24e05262ea09345e15f9299381367fdc6951f21dd3788a4a58d2132
b0510d78ae Set C locale for amountWidget (Hennadii Stepanov)
Pull request description:
Fix#13873
Tree-SHA512: ef26b35ef83c3a87ebd90650f6d833b00a24f6c114b68fe01acd4a14d1f5bdec066f438eb7781c1e55c32640838c54e00b8f082c390639ade8d9a58830833d4a
2d471636eb wallet: Remove trailing separators from -walletdir arg (Pierre Rochard)
ea3009ee94 wallet: Add walletdir arg unit tests (Pierre Rochard)
Pull request description:
If a user passes in a path with a trailing separator as the `walletdir`, multiple BerkeleyEnvironments may be created in the same directory which can lead to data corruption.
Discovered while reviewing https://github.com/bitcoin/bitcoin/pull/12493#issuecomment-417147646
Tree-SHA512: f2bbf1749d904fd3f326b88f2ead58c8386034355910906d7faea155d518642e9cd4ceb3cae272f2d9d8feb61f126523e1c97502799d24e4315bb53e49fd7c09
43c7fbb1e7 Make MSVC compiler read the source code using utf-8 (Chun Kuan Lee)
f86a571edb tests: Add test case for std::ios_base::ate (Chun Kuan Lee)
a554cc901a Move boost/std fstream to fsbridge (Chun Kuan Lee)
86eb3b3f1a utils: Add fsbridge fstream function wrapper (Chun Kuan Lee)
Pull request description:
If compiled with mingw, use glibc++ extension `stdio_filebuf` to open the file by `FILE*` instead of filename.
In other condition, we can use boost::fstream.
Tree-SHA512: b5dbd83e347fb9b2a0c8b1c2c7bd71a272e839ec0617883b2a0ec12506ae9e825373cf6e95b9bcc91d7edc85bf51580a7716b56a9ecaad776bc3ae61638cb3da
d562027079 [doc] getblocktemplate: use SegWit in example (Sjors Provoost)
Pull request description:
Make it less likely for new miners to accidentally mine non-SegWit blocks.
Suggest backport to 0.17 so the docs on bitcoincore.org get updated at the next minor release.
Tree-SHA512: fc8c51690559d3e64d03c0eec433c75e5581d616d36b529e16f89ff2433457386feba7ef3b0d22f245d3f725183a2204fef7ee13e25cb92b945ab61f99068dee
081cc02a9 Fix QCompleter popup regression (Hennadii Stepanov)
Pull request description:
The PR #8129 has introduced a regression with the `QCompleter` popup in the Debug window.
How to reproduce:
1. open the Debug window;
2. go to the 'Console' tab;
3. start writing some RPC command and try to pick it from the list using arrow keys, press Enter.
Note that the popup used to display completions is not being closed. To close it they should mouse click somewhere outside of the popup.
The wrong behaviour of the `QCompleter` popup is observed on Linux Mint 19 and Windows 10.
This PR fixes this regression.
Refs:
- #7613
- #7772
- #8129
Tree-SHA512: f3ba8d08e1c07619d4ef307544306b57be43e4e726770976cf0c2af95082bd66e2eefe8aabb9a3fad0601cd9e6e4dea0459b6a63eba512023234feb308484655
When we receive invalid txs for the first time, we mark the sender as
misbehaving. If we receive the same tx before a new block is seen, we *don't*
punish the second sender (in the same way we do the original sender). It wasn't
initially clear to me that this is intentional, so add a clarifying comment.
032488e6e7 Move SocketHandler logic to private method. (Patrick Strateman)
2af9cff11a Move InactivityCheck logic to private method. (Patrick Strateman)
7479b63d91 Move DisconnectNodes logic to private method. (Patrick Strateman)
edb5350c32 Move NotifyNumConnectionsChanged logic to private method. (Patrick Strateman)
Pull request description:
Working towards using poll() on unix like systems.
A number of small changes designed to separate the actual socket handling from the rest of the logic in ThreadSocketHandler.
This is a simpler version of #14147
Tree-SHA512: 72f35c8ef7649019dcbfe19537d8c9f7e3d0fc5854dc691a70c5573352230fc31c3f55565820c632e9b8cb3c55b878bed19e0ad9423100762197ac35967d8067
7d173c4cd1 qt: Revert "Force TLS1.0+ for SSL connections" (Tim Ruffing)
Pull request description:
This reverts commit 15e26a6a9a, whose
purpose was to tweak the Qt configuration to force TLS, i.e., to
disable SSLv3, in Qt versions >= 5.5. However, the default behavior
of Qt >= 5.4 is to disable SSLv3 anyway [1], so the configuration
tweak is redundant.
With Qt 5.11.2, the configuration tweak is not only redundant but in
fact provokes a deadlock due to a bug in Qt 5.11.2. Since the deadlock
occurs at the early startup stage of bitcoin-qt, it renders bitcoin-qt
entirely non-functional when compiled against Qt 5.11.2 (and maybe
other Qt versions).
Fixes#14359.
[1] https://code.qt.io/cgit/qt/qtbase.git/commit/?id=3fd2d9eff8c1f948306ee5fbfe364ccded1c4b84
Tree-SHA512: 9dd86557b8d265dfa56592924778a736590f2e6a0b2acf77d4f9f4200206a9edaa79b144b0085ea59ac0cc1bc66d9740402fd02f9298ff74c8d6f526f3f725d6
36323e2ac6 Clean systray icon menu for -disablewallet mode (Hennadii Stepanov)
Pull request description:
There is a `Debug window` leftover in the system tray icon menu after #3392 merging.
This PR makes both the app menu and the systray icon menu consistent.
Tree-SHA512: c9ef58785fe2a54bc6f778140a16001748ed8c46da948656822b86fdc2e224203cd467857f71d00ce56fc73ff2590c46d8c234a54c261c1141d83039de6fee1e
2f6b466ae Stop requiring imported pubkey to sign non-PKH schemes (Pieter Wuille)
Pull request description:
...
This is a fix for #14415
Tree-SHA512: 113b4ddfbdfcce7dbaa15c565ac7ec68d16127aa4d47628e0801f2437cbe369e0fa8beb0de191d43dcb2f8cbb6f1bf8d79a5db0f4e352f38fe7280124475710a
The PR #8129 has introduced a regression with the QCompleter popup in
the Debug window.
How to reproduce:
1) open the Debug window;
2) go to the 'Console' tab;
3) start writing some rpc command and try to pick it from the list
using arrow keys, press Enter.
Note that the popup used to display completions is not being closed. To
close it they should mouse click somewhere outside of the popup.
'Must be one of' should always end in a ':'
'hex encoded' is now always 'hex-encoded'
Remove redundant '(defaults to CONSERVATIVE)' text from estimatesmartfee
Consistent spacing for options '( verbose )' and '( verbosity )'
'BIP125 replaceable' is now always 'BIP125-replaceable'
JSON-RPC example is now always 'As a JSON-RPC call'
* AddKeyPubKeyWithDB(...) reads encrypted_batch which potentially races with write in the same method.
* IncOrderPosNext(...) reads nOrderPosNext which potentially races with write in BlockDisconnected(...).
* LoadKeyPool(...) reads m_max_keypool_index which potentially races with write in BlockDisconnected(...).
* LoadMinVersion(...) reads nWalletMaxVersion which potentially races with write in BlockDisconnected(...).
This reverts commit 15e26a6a9a, whose
purpose was to tweak the Qt configuration to force TLS, i.e., to
disable SSLv3, in Qt versions >= 5.5. However, the default behavior
of Qt >= 5.4 is to disable SSLv3 anyway [1], so the configuration
tweak is redundant.
With Qt 5.11.2, the configuration tweak is not only redundant but in
fact provokes a deadlock (#14359) due to Qt 5.11.2 being incompatible
with OpenSSL 1.1.1 [2]. Since the deadlock occurs at the early startup
stage of bitcoin-qt, it renders bitcoin-qt entirely non-functional
when compiled against OpenSSL 1.1.1 and Qt 5.11.2 (and possible future
combinations of OpenSSL and Qt versions).
This commit fixes#14359 by removing the redundant code.
[1] https://code.qt.io/cgit/qt/qtbase.git/commit/?id=3fd2d9eff8c1f948306ee5fbfe364ccded1c4b84
[2] https://bugreports.qt.io/browse/QTBUG-70956
This patch adds a --disable-bip70 configure option that disables BIP70
payment request support. When disabled, this removes the dependency of
the GUI on OpenSSL and Protobuf.
3f5ac27205 Include some files currently missed by 'make distclean'. (murrayn)
Pull request description:
`make clean` currently leaves behind some cache and test log files that should be removed.
Tree-SHA512: a1877e776e24232f6dd1468d7f392ea0bd1e93fdd975e623897d48c4b23a080a2e84ebb199f5482abd6b8c9ddd036850325e7b7ed07e2f9fe7a32f83cc99da4a
380c843217 utils: Convert Windows args to utf-8 string (Chun Kuan Lee)
Pull request description:
Create a new class `WinCmdLineArgs` when building for Windows. It converts all command line arguments to utf8 string.
Tree-SHA512: f098520fd123a8a452bc84a55dc8c0b88f0c475410efe57f2ccc393f86c396eed59ea1575ddc1b920323792e390fdb092061d80cdcd9b682f0ac79a22a22ff82
- Creates m_assumed_blockchain_size and m_assumed_chain_state_size on CChainParams.
- Implements access to CChainParams' m_assumed_blockchain_size and m_assumed_chain_state_size on node interface.
- Implements m_assumed_blockchain_size and m_assumed_chain_state_size on qt/intro via node interface.
- Updates release process document with the new CChainParam's values.
- changes importprivkey behavior to overwrite existent label if one
is passed and keep existing ones if no label is passed
- tests behavior of importprivkey on existing address labels and
different same key destination
fa69ac7614 doxygen: Fix member comments (MarcoFalke)
Pull request description:
Trailing comments must be indicted with the caret `//!<`.
Not all places do this right now, see for example https://dev.visucore.com/bitcoin/doxygen/txmempool_8h.html#a2bc6653552b5871101b6cbefdbaf251f, but they can be fixed with an almost-scripted-diff:
```
sed -i --regexp-extended -e 's/((,|;) *\/\/!) /\1< /g' $(git grep --extended-regexp -l '(,|;)\s*//!\s')
```
(Same as [doxygen] Fix member comments #7793)
Tree-SHA512: 451077008353ccc6fcc795f34094b2d022feb7a171b562a07ba4de0dcb0aebc137e12b03970764bd81e2da386751d042903db4c4831900f43c0cfde804c81b2b
fa84723e73 amount: Move CAmount CENT to unit test header (MarcoFalke)
Pull request description:
`CAmount` is currently not type-safe. Exporting a constant (`CENT`) that is commonly not referred to by that name might be confusing. `CENT` is only used in two places prior to this commit (`ParseMoney` and `MIN_CHANGE`). So replace these with constants relative to `COIN` and move `CENT` to the unit test header.
Tree-SHA512: 5273e96d8664ced6ae211abde2e20bc763e6e99f89404eec02c621f29e1d235e5f9b1ade933743843fae16fc24b643f883deda9221e3d9fd31229d2ab63a914f
9c5af58d51 Consolidate redundant implementations of ParseHashStr (Ben Woosley)
Pull request description:
This change:
* adds a length check to all calls to `ParseHashStr`, appropriate given its use to populate
a 256-bit number from a hex str
* allows the caller to handle the failure, which allows for the more
appropriate `JSONRPCError` on failure in `prioritisetransaction` rpc
Relative to #14288
Tree-SHA512: baa791147e5ceb3c30c70df3981aaf807bf7d4a90a0be3625540b59aa4b9a9d303a452bfef18bf167cbb833ef9591b4ef5948bf4a1ce67b421d804ae8d20ea53
66b3fc5437 Skip stale tip checking if outbound connections are off or if reindexing. (Gregory Maxwell)
Pull request description:
I got tired of the pointless stale tip notices in reindex and on nodes with connections disabled.
Tree-SHA512: eb07d9c5c787ae6dea02cdd1d67a48a36a30adc5ccc74d6f1c0c7364d404dc8848b35d2b8daf5283f7c8f36f1a3c463aacb190d70a22d1fe796a301bb1f03228
a2a04a5abb Bugfix: Only run bitcoin-tx tests when bitcoin-tx is enabled (Luke Dashjr)
92af71cea9 configure: Make it possible to build only one of bitcoin-cli or bitcoin-tx (Luke Dashjr)
Pull request description:
Includes #5618 (which the reasons for rejecting no longer hold true)
Tree-SHA512: f30a8e4a2f70166b7cabef77c4674163b3a9da14c6a547d34f00d1056a19bf4d23e22851eea726fad2afc8735d5473ae91122c770b65ac3886663dc20e2c5b70
7ac911afe7 [docs] Add release notes for removing `-usehd` (John Newbery)
25548b2958 [wallet] Remove -usehd (John Newbery)
Pull request description:
`-usehd` is no longer used (except to tell the user that they've set it incorrectly for the wallet that they're loading). Remove it (in the same spirit as #14272)
Tree-SHA512: 5bdcd2bb9bb8504a01343595bcd1bd433d97b730255152c725103c1ac3fa3a9d9e5220a4c29d4c72307cf803e1c09d31080f83603c23dc77263846e17b1826f0
2c3eade704 Make fs::path::string() always return utf-8 string (Chun Kuan Lee)
Pull request description:
Imbue `fs::path` with `std::codecvt_utf8_utf16` at `SetupEnvironment()`, so that default string encoding will be utf-8 inside `fs::path`.
Tree-SHA512: 0cb59464d777278decbf24771fc5ff0cb2caa7bc2fe8ee5cd36c97a2324873a3caad131f08f050393b488316ee7f4ab0b28b7fa4699e41839f8e51b9867d5118
This change:
* adds a length check to ParseHashStr, appropriate given its use to populate
a 256-bit number from a hex str.
* allows the caller to handle the failure, which allows for the more
appropriate JSONRPCError on failure in prioritisetransaction rpc
6fa901fb47 Don't edit Chainparams after initialization (Jorge Timón)
980b38f8a1 MOVEONLY: Move versionbits info out of versionbits.o (Jorge Timón)
Pull request description:
This encapsulates the "-vbparams" option, which is only meant for regtest, directly on CRegTestParams.
This is a refactor and doesn't change functionality.
Related to https://github.com/bitcoin/bitcoin/pull/8994
Tree-SHA512: 79771d729a63a720e743a9c77d5e2d80369f072d66202a43c1304e83a7d0ef7c6103d4968a03aea9666cc89a7203c618da972124a677b38cfe62ddaeb28f9f5d
5eb20f81d9 Consistently use ParseHashV to validate hash inputs in rpc (Ben Woosley)
Pull request description:
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)" in that case.
Split from #13420
Tree-SHA512: f0786b41c0d7793ff76e4b2bb35547873070bbf7561d510029e8edb93f59176277efcd4d183b3185532ea69fc0bbbf3dbe9e19362e8017007ae9d51266cd78ae
1eb9a9b524 [RPC] Remove warning for removed estimatefee RPC (John Newbery)
Pull request description:
The RPC was removed in a previous version, but a warning was
left for users to use the estimatesmartfee RPC. Remove that warning now
that estimatefee has been gone for over one version.
Tree-SHA512: 7fb440a354a5058f9e95930306d7fe0c1cba6563b9a44b7388a17d9e5c3cff42023f5aa1728fd94a1a11249ea4a8615a8a891afe4fa81ae46b61c2aa08e9cc47
30973e9844 [REST] improve performance for JSON calls (Antoine Le Calvez)
Pull request description:
JSON calls do not use the raw data generated for the .bin and .hex calls.
By moving the raw data creation into the .bin and .hex switch branches, JSON calls' performance is improved.
Light benchmarking indicates that fetching 2000 JSON headers is ~25% faster, fetching large JSON blocks is ~4% faster.
Tree-SHA512: 433552c89bac2469d041b48a4a991d5443e4026a3ad7dc5621685386029f22826484218642fa5130c268349a55524ecbc4e30d64c867bd6632e0edd24370cf11
a23a7f60aa wallet: Avoid potential use of unitialized value bnb_used in CWallet::CreateTransaction(...) (practicalswift)
Pull request description:
Avoid use of uninitialized value `bnb_used` in `CWallet::CreateTransaction(...)`.
Tree-SHA512: 22faf0711ae35af44d9a0ab7f251bc01661ac88b40ad7b0a87a510427b46bbc8caf16868cab2e0a05e7d8518e93ce666d6bd1d48d3707d37bab2c0fb56a0a4a2
JSON calls do not use the raw data generated for
the .bin and .hex calls. By moving the raw data
creation into the .bin and .hex switch branches,
JSON calls become faster.
The RPC was removed in a previous version, but a warning was
left for users to use the estimatesmartfee RPC. Remove that warning now
that estimatefee has been gone for over one version.
3ccfa34b32 convert C-style (void) parameter lists to C++ style () (Arvid Norberg)
Pull request description:
In C, an empty parameter list, `()`, means the function takes any arguments, and `(void)` means the function does not take any parameters.
In C++, an empty parameter list means the function does not take any parameters.
So, C++ still supports `(void)` parameter lists with the same semantics, why change to `()`?
1. removing the redundant `void` improves signal-to-noise ratio of the code
2. using `(void)` exposes a rare inconsistency in that a template taking a template `(T)` parameter list, cannot be instantiated with `T=void`
Tree-SHA512: be2897b6c5e474873aa878ed6bac098382cd21866aec33752fe40b089a6331aa6263cae749aba1b4a41e8467f1a47086d32eb74abaf09927fd5a2f44a4b2109a
b6718e373e tests: Use MakeUnique to construct objects owned by unique_ptrs (practicalswift)
Pull request description:
A subset of #14211 ("Use MakeUnique to construct objects owned by unique_ptrs") as suggested by @MarcoFalke in https://github.com/bitcoin/bitcoin/pull/14211#issuecomment-423324019.
Use `MakeUnique` to construct objects owned by `unique_ptr`s.
Rationale:
* `MakeUnique` ensures exception safety in complex expressions.
* `MakeUnique` gives a more concise statement of the construction.
Tree-SHA512: 1228ae6ce7beb178d79142c4e936b728178ccaa8aa35c6d8feeb33d1a667abfdd010c59996a9d833594611e913877ce5794e75953d11d9b1fdbac04aa491d9cf
a2eb6f5405 [rpc] Add getnodeaddresses RPC command (chris-belcher)
Pull request description:
Implements issue https://github.com/bitcoin/bitcoin/issues/9463
New getnodeaddresses call gives access via RPC to the peers known by the node. It may be useful for bitcoin wallets to broadcast their transactions over tor for improved privacy without using the centralized DNS seeds. getnodeaddresses is very similar to the getaddr p2p method.
Please advise me on the best approach for writing an automated test. By my reading the getaddr p2p method also isn't really tested.
Tree-SHA512: ad03abf518847476495b76a2f5394b8030aa86654429167fa618e21460abb505c10ef9817ec1b80472320d41d0aff5dc94a8efce023aaaaf5e81386aa92b852b
New getnodeaddresses call gives access via RPC to the peers known by
the node. It may be useful for bitcoin wallets to broadcast their
transactions over tor for improved privacy without using the
centralized DNS seeds. getnodeaddresses is very similar to the getaddr
p2p method.
Tests the new rpc call by feeding IP address to a test node via the p2p
protocol, then obtaining someone of those addresses with
getnodeaddresses and checking that they are a subset.
f1bd03eb01 [depends, zmq, doc] upgrade zeromq to 4.2.5 and avoid deprecated zeromq api functions (mruddy)
Pull request description:
Upgrade the ZeroMQ dependency from version 4.2.3 to the latest stable version 4.2.5.
This PR Follows the lead of https://github.com/bitcoin/bitcoin/pull/11986.
I upgraded both patch files to correspond to the version `4.2.5` libzmq files.
I assume doing so is still necessary and correct.
Without updating the patch line numbers, things appear to work, but you get extra log messages while building `depends` because things don't exactly match, e.g.:
```
/bitcoin/depends> make zeromq
Extracting zeromq...
/bitcoin/depends/sources/zeromq-4.2.5.tar.gz: OK
Preprocessing zeromq...
patching file src/windows.hpp
Hunk #1 succeeded at 58 (offset 3 lines).
patching file src/thread.cpp
Hunk #1 succeeded at 307 with fuzz 2 (offset 87 lines).
Hunk #2 succeeded at 323 with fuzz 2 (offset 90 lines).
```
Updating the patches seemed cleaner, so I did it. Note that libzmq had some whitespace changes, so that's why the updated patches do too.
More info: https://github.com/zeromq/libzmq/releases/tag/v4.2.5
tags: libzmq, zmq, 0mq
Tree-SHA512: 78659dd276b5311e40634b1bbebb802ddd6b69662ba3c84995ef1e3795c49a78b1635112c7fd72a405ea36e2cc3bdeb84e6d00d4e491a349bba1dafff50e2fa5
cccc362d62 build: Remove libssl from LDADD unless gui (MarcoFalke)
Pull request description:
libssl is only used by the gui, so no need to LDADD it to the other tools and binaries
Follow up of the commit which removed rpcssl: 40b556d374
Tree-SHA512: 9dbdf4faf40699cea3a37349ac83dbcacdaa062f5338416ff4ba77924c47d9e148b27218165c5aa3584a1ef4899e0fa237ff571208aa0b98803761e802d1e5dc
8e1c13e6da doc: Document -checklevel levels (Wladimir J. van der Laan)
Pull request description:
Document the various possible check levels for the command-line argument. The numbers say nothing on their own.
Tree-SHA512: 8a526c53222b55304dde1d9350dd15a50f1dd62bf452a32dc886a4521e3ab49d5f0a86a4c5cbb0d52fb76b60222101045100f93fee5e1a5d5e3ab8e79b64cbe0
c1dde3a949 No longer shutdown after encrypting the wallet (Andrew Chow)
d7637c5a3f After encrypting the wallet, reload the database environment (Andrew Chow)
5d296ac810 Add function to close all Db's and reload the databae environment (Andrew Chow)
a769461d5e Move BerkeleyEnvironment deletion from internal method to callsite (Andrew Chow)
Pull request description:
This is the replacement for #11678 which implements @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/11678#pullrequestreview-76464511).
Shutting down the software was to prevent the BDB environment from writing unencrypted private keys to disk in the database log files, as was noted [here](https://bitcointalk.org/index.php?topic=51474.msg616068#msg616068). This PR replaces the shutdown behavior with a CDBEnv flush, close, and reopen which achieves the same effect: everything is cleanly flushed and closed, the log files are removed, and then the environment reopened to continue normal operation.
To ensure that no unencrypted private keys are in the log files after encrypting the wallet, I wrote [this script](https://gist.github.com/achow101/7f7143e6c3d3fdc034d3470e72823e9d) to pull private keys from the original wallet file and searches for these keys in the log files (note that you will have to change your file paths to make it work on your own machine).
As for concerns about private keys being written to slack space or being kept in memory, these behaviors no longer exist after the original wallet encryption PR and the shutting down solution from 2011.
cc @ryanofsky
Tree-SHA512: 34b894283b0677a873d06dee46dff8424dec85a2973009ac9b84bcf3d22d05f227c494168c395219d9aee3178e420cf70d4b3eeacc9785aa86b6015d25758e75
Removes the final #ifdef ENABLE_WALLET from libbitcoin_server by calling
g_wallet_init_interface.HasWalletSupport(), and redifining GetWallets()
and MakeWallet() in dummywallet.cpp.
fa6ab8ada1 rpc: Return more specific reject reason for submitblock (MarcoFalke)
Pull request description:
The second commit in #13439 made the `TODO` in the first commit impossible to solve.
The meaning of `fNewBlock` changed from "This is the first time we process this block" to "We are about to write the new *valid* block".
So whenever `fNewBlock` is true, the block was valid. And whenever the `fNewBlock` is false, the block is either valid or invalid. If it was valid and not new, we know it is a `"duplicate"`. In all other cases, the `BIP22ValidationResult()` will return the reason why it is invalid.
Tree-SHA512: 4b6edf7a912339c3acb0fccfabbdd6d812a0321fb1639c244c2714e58dc119aa2b8c6bf8f7d61ea609a1b861bbc23f920370fcf989c48452721e259a8ce93d24
d38bf9105d Call unicode API on Windows (Chun Kuan Lee)
Pull request description:
Call Unicode API on Windows
Tree-SHA512: 93c290ee79c9d911fdada8ba45e184fc4f14d3cb56f33f39223286213878b08e8c4dd296a80099c57797d3b8589870e6cff622b22e76123d7452659d49dd8309
d0b1cee1fe doc: `-help-debug` implies `-help` (Wladimir J. van der Laan)
Pull request description:
I don't understand why `-help-debug` would be useful without requesting the help, and I've made this particular mistake one time too many.
Tree-SHA512: 370eab368d672fc09e66fcc440db3cb7e4c70c2988aab506cdc3f3e234c27c8f0fc7512c9cf86606ac43f5c6023b3618b7b0302b4b6e289b388559ba8010f27a
fae3fbd61a logging: Replace LogPrint macros with regular functions (MarcoFalke)
Pull request description:
It is not possible to run the full test suite when configured with `--enable-lcov`, since logging is disabled currently so that "unnecessary branches are not analyzed". (See c8914b9dbb)
Fix this instead by replacing the macros with functions.
Tree-SHA512: 101aa4f4a3ffcefc38faf70c9d3deb5fc63e0b11ca54a164d0463931c79eaf53ab0b0c6ae92a45355574e3b1d2c32233874a6b24293e7a09d188fc6698e212a5
I don't understand why `-help-debug` would be useful without requesting
the help, and I've made this particular mistake one time too many.
(ok, so apparently the original reason that -help-debug is an option is
to show debug options in the GUI option overview? that seems very
unlikely to ever be used, if someone wants to add a way to show debug
options in the GUI it'd be better to have a check box)
e0664f7f54 build: Move interfaces/* to libbitcoin_server (Wladimir J. van der Laan)
Pull request description:
Move interfaces/* from libbitcoin_util to libbitcoin_server.
Usage of these is shared between `bitcoind` and `bitcoin-qt`. It is unnecessary for them to be linked against the other utilities. Also semantically they belong with the server/node, I think.
Tree-SHA512: f82f3a95d362051c0eb8092520715f77c2c75409d41f9c0fab9a15445ea9e79a2a36b5d00b1f5be09b266584051934a32a8b9b28f783f4d7be78885b4a29a383
23db9546c1 utils: run commands using utf-8 string on Windows (Chun Kuan Lee)
Pull request description:
Use unicode string to call commans
Tree-SHA512: 72f84e7b56cd947ad05176f10b5ddd5610f4641ba5e93ffd67777dea8f9734ec06e6ed3a63f67ae5e766767122c0dd2c441d0bad5572bdb9fb78758f02531feb
7d038dcb41 [build] remove ENABLE_WALLET ifdef from httprpc.cpp (John Newbery)
3076556cd0 [build] Move dummy wallet into its own .cpp file. (John Newbery)
Pull request description:
This removes the final instances of ENABLE_WALLET in libbitcoin_server and so completes #7965.
Tree-SHA512: a49128b7c17f4f69940d5843e6b785f08687efb377b5157d5b267d1205e596eb5c1966f1afb8ab36bcc2491c46252099e3e844c91f5623da8ded2e358d46338d
bb6ca65f98 gui: get special folder in unicode (Chun Kuan Lee)
1c5d225853 Drop boost::scoped_array (Chun Kuan Lee)
Pull request description:
Drop boost::scoped_array and simplify the code.
`TCHAR` should be defined as `wchar_t` if `UNICODE` is defined. So we can use `.toStdWString().c_str()` to get wchar_t C-style string.
Fix#13819
Tree-SHA512: 3fd4aa784129c9d1576b01e6ee27faa42d793e152d132f2dde504d917dad3a8e95e065fcbc54a3895d74fb6b2a9ed4f5ec67d893395552f585e225486a84a454
a679109be4 Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)
Pull request description:
Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.
On my (slow) machine:
```
before: 9.8s
after: 6.2s
--------------------
saved: 3.6s (36%)
```
This PR was split from #13050. Also see #10026.
Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f
893628be01 Drop minor GetSerializeSize template (Ben Woosley)
da74db0940 Drop unused GetType() from CSizeComputer (Ben Woosley)
Pull request description:
Based on conversation in #13462, it seems the serialization `GetType` has very narrow use/effect. In every case except for `CAddress`, which specifically relates to a network peer's address, not a wallet address etc., the serialized representation of an object is irrespective of its destination / type.
This removes the unused `GetType` method from `CSizeComputer` as a step to further narrowing that use.
Tree-SHA512: e72b8e9e5160396691e05aeaee3aba5a57935a75bd5005cfcc7fb51c936f3d1728a397f999da5c36696506dd815fafa5c738f3894df8864f25f91f639eba9c3d
b9babc82dd utils: Use _wfopen and _wreopen on Windows (Chun Kuan Lee)
Pull request description:
The fopen function does not support unicode filename on Windows, so use Windows specific function do deal with it.
Tree-SHA512: 4dcf14dcf9ec6307b9fdf95404e5b6b6b3df640949fd4b0c4ac7fecf8ea03a64fa25285fc319c4ff8a28e586eee106f1861116c181694955497402b2bf575f22
fa7e9694e1 qt: Also log and print messages or questions like bitcoind (MarcoFalke)
dd031e3839 noui: Move handlers to header file (MarcoFalke)
Pull request description:
Testing and debugging after shutdown are harder if the node was run through the gui, because errors and warnings would not be logged to the debug.log or written to the stderr (as is the case for bitcoind).
Tree-SHA512: 1154e2bf02e3c2616c8d28609569d6c3c7344c5877ad5c1303245044cc7aced9eaec9627f1e1258ed087b49c2a2e6f99bc6c1ad0abe0a855b61e737bdf2059bc
98ea64cf23 Let wallet importmulti RPC accept labels for standard scriptPubKeys (Russell Yanofsky)
Pull request description:
Allow importmulti RPC to apply address labels when importing standard scriptPubKeys. This makes the importmulti RPC less finnicky about import formats and also simpler internally.
Tree-SHA512: 102426b21239f1fa5f38162dc3f4145572caef76e63906afd786b7aff1670d6cd93456f8d85f737588eedc49c11bef2e1e8019b8b2cbf6097c77b3501b0cab1f
faa1a74942 tx pool: Use class methods to hide raw map iterator impl details (MarcoFalke)
Pull request description:
ATMP et al would often use map iterator implementation details such as `end()` or `find()`, which is acceptable in current code.
However, this not only makes it impossible to turn the maps into private members in the future but also makes it harder to replace the maps with different data structures.
This is required for and split off of #13804
Tree-SHA512: 4f9017fd1d98d9df49d25bba92655a4a97755eea161fd1cbb565ceb81bbc2b4924129d214f8a29563a77e3d8eef85a67c81245ecdc9a9e5292d419922a93cb88
d9d79576f4 Preserve a format of RPC command definitions (Kostiantyn Stepaniuk)
Pull request description:
Currently, RPC commands are formatted in a way that it's easy to read
and that `test/lint/check-rpc-mappings.py` can parse it.
To void breaking `test/lint/check-rpc-mappings.py` script by running
`clang-format`, RPC command definitions should be disabled for clang-format.
Tree-SHA512: e17d20ec0e6c4e19410198b55687ebbe6fa01654d214d4578cd16c00b872bf8b0b306594a45523685cd2e9d9280702e00471d9366e87954428e8bbeacd8cad60
9256f7d13f build: avoid getifaddrs when unavailable (Cory Fields)
Pull request description:
These changes from @theuni help building when targeting platforms that don't always have getifaddrs available like Android < 24
Tree-SHA512: dbfeb83297bd6f00b7991f53eef8a04948d2d739bf47c0524d9ae5335b843b8a5c06ff98c109fe5e6192665e6d0cf58700b8aa7e2a0b410281d3c052881973ff
fa462b3657 wallet: Set encrypted_batch to nullptr after delete. Avoid double free in the case of NDEBUG. (practicalswift)
Pull request description:
Set `encrypted_batch` to `nullptr` after delete. Avoid double free in the case of `NDEBUG`.
Tree-SHA512: 6f5ab40c82dd8c8713bbf1aacacdc837277c04769807f985248546be1c7ea269813c95379fbef982ac5683a45af0225613460a7446c39673b033f5f5edde2f5a
faea5bfc5a doc: release notes for -enablebip61 default change (MarcoFalke)
fa14b54a87 p2p: Disable BIP 61 by default (MarcoFalke)
Pull request description:
The live p2p network should not be used for debugging or as development aid when implementing the p2p protocol. Instead, applications should be tested locally (e.g. by inspecting the debug log of a validating node on the local network)
Using the p2p network for this purpose seems wasteful and even dangerous, as peers can not be trusted to send the correct reject messages or a reject message at all.
Tree-SHA512: 9c91ad035b5110942172a3b4b8a332a84e0c0aa9ee80f8134aeab63e66ac604841e68b04038681c288b716e5e0cbe38065eb5c7eb63fa72c3bdb3255a4b2d99d
dc287c98f8 Squashed 'src/univalue/' changes from 51d3ab34ba..7890db99d6 (MarcoFalke)
Pull request description:
This removes the deprecated `std::pair` wrappers from univalue, so that they are not accidentally re-introduced in our code base.
Tree-SHA512: 46f6f7c7c7942a9e131d971c425cbde4159abcc1214235b61139ce97b174024e47b9c52e832cde89fbab9879f43d12c26b64d6def9907bd47883f1e574cf2e2e
946107a68f Only log "Using PATH_TO_bitcoin.conf" message on startup if conf file exists. (Alexander Leishman)
Pull request description:
Currently we log a message indicating that a bitcoin.conf file is being used even if one does not exist. This PR changes the logic to:
**If config file does not exist and no -conf flag passed, log:**
`Config file: FILE_PATH (not found, skipping)`. Where `FILE_PATH` is the default or the path passed in with the `-conf` flag.
**If config file does not exist and -conf flag passed with incorrect path, log warning:**
`Warning: The specified config file FILE_PATH does not exist`
**If config file exists, log**:
`Config file: FILE_PATH`
Note: This is a (modified) subset of changes introduced in https://github.com/bitcoin/bitcoin/pull/13761 which creates a default example config file. I think it makes sense to extract this small bit out into a separate PR.
Tree-SHA512: be0f0ae6a0c9041e2d6acb54d2563bbcc79786fb2f8bf9a963fe01bc54cd4e388b89079fde1eb79f7f17099776428e5e984bf7107590a3d1ecfc0562dbc6e3f5
b2f49bd732 Integration of property based testing into Bitcoin Core (Chris Stewart)
Pull request description:
This PR is a subset of the changes in #8469. It's meant to be easier to review. This PR contains all of the build instructions needed for travis to pass. It includes one property call `key_properties.cpp` along with a generator file called `crypto_gen.{h,cpp}`.
Tree-SHA512: 895c9d9273dcd29f696b1de8dfe1ee843095831bf1f68472844181278850bec36b20f0ba7e51e796112c5cc75cd24759f9f1771906503bbf3af16f627e18c6c9
65a449f8e3 Explain when reindex-chainstate can be used instead of reindex (Sjors Provoost)
Pull request description:
Save users from having to Google this: https://bitcoin.stackexchange.com/a/60711
Tree-SHA512: 3128565d037c77265a2ecf3bce137b8d27740f513802a4e683be06f21a75b82ee6cc22eb903181c4f438a2990cb682ce1d076f4d3af33d5aaa79b783a9f664b1
f7e9e70468 [rpc] Remove deprecated sigrawtransaction rpc method. (John Newbery)
90c834089a [RPC] Remove warning about wallet addresses in createmultisig() (John Newbery)
df905e390e [rpc] Remove deprecated validateaddress usage. (John Newbery)
Pull request description:
The following rpc features were deprecated in V0.17:
- `validateaddress` returning wallet information about an address
- `signrawtransaction`
This PR fully removes those features. It can be merged once V0.17 has been branched from master.
Tree-SHA512: 28293d218cf7e348632081e362f8775f243d091f49aed54c354f017d4a12ae92b87b99f81ee592a1bbf4aebd5d8cd5119278141edde7a0399ff82917ed68b9f6
fab5267514 doxygen: Remove misleading checkpoints comment in CMainParams (MarcoFalke)
Pull request description:
This removes the checkpoints comment because it is misleading for two reasons:
* It shows up in the doxygen documentation of `CMainParams` https://dev.visucore.com/bitcoin/doxygen/class_c_main_params.html
* The comment refers to "strange transactions" in a block, which are not specified further. Transactions in blocks are always consensus-valid or rejected as consensus-invalid.
Also sort the includes with `clang-format`, as the file is touched anyway.
Tree-SHA512: b75f38dd0422b9310218307cbaa4dd5afa7579612d7dcdf781b8f25626f79c11e090dbcc83a05571f4418220c1a005f6254a9c461534d517ccecf7f1920be6be
ed2332aeff test: Add test for config file parsing errors (MarcoFalke)
a66c0f78a9 util: Report parse errors in configuration file (Wladimir J. van der Laan)
Pull request description:
Report errors while parsing the configuration file, instead of silently ignoring them.
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: sdafsdfafs
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -
(inspired by https://github.com/bitcoin/bitcoin/pull/14100#issuecomment-417264823)
Tree-SHA512: d516342b65db2969edf200390994bbbda23654c648f85dcc99f9f2d217d3d59a72e0f58227be7b4746529dcfa54ba26d8188ba9f14a57c9ab00015d7283fade2
PR #12713 changed the interpretation for negation of non-boolean options
(e.g. -noconnect) to no longer set the option to 0, but to remove it
from the options.
I think this is better because it gets rid of the special meaning of
'0'.
However it needs to be documented. I attempt to do so in this PR.
Addreses #14064.
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
e8c4a1e369 Add new regtest ports in doc following #10825 ports reattributions (Antoine Riard)
Pull request description:
Following #10825, regtest ports for p2p connections and JSON-RPC connections have been remapped from 18333 and 18332 to 18444 and 18443. This change is not documented in the wiki or nowhere else and it's puzzling to guess why your regtest JSON-rpc connections all failed even if you're following the docs.
Tree-SHA512: e2a1b9b4059060d9ed0900c1554e124ed69ae3e4648474880795128e77c7324d68aba52e4acda2f47390a9c3d36629b777e3b8c0eb10f0e08a2b120c4119dff3
8ecaee13f7 Increase signal to noise in appveyor build output by reducing the MSVC warning count from 12 to 4 (12 is assuming the changes in #14086 are also implemented). (practicalswift)
Pull request description:
Remove unreferenced local variables:
Increase signal to noise in appveyor build output by reducing the MSVC warning count from 12 to 4. 12 is the number of MSVC warnings under our current appveyor setup assuming the changes in #14086 are also implemented.
This makes it easier to spot errors or more important warnings in the verbose appveyor output. MSVC warnings are good, so having access to them in a noise free way (read: without trivial warnings) via appveyor without having to use Windows is really valuable.
See https://github.com/bitcoin/bitcoin/pull/14086#issuecomment-416610313 plus discussion for context.
Before:
```
c:\projects\bitcoin\src\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch [C:\projects\bitcoin\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj]
c:\projects\bitcoin\src\rest.cpp(467): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\libbitcoin_server\libbitcoin_server.vcxproj]
c:\projects\bitcoin\src\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(511): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(524): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(722): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\coins_tests.cpp(783): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\dbwrapper_tests.cpp(265): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\net_tests.cpp(118): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\net_tests.cpp(151): warning C4101: 'e': unreferenced local variable [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\scheduler_tests.cpp(57): warning C4305: 'argument': truncation from 'int' to 'bool' [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
```
After:
```
c:\projects\bitcoin\src\script\script.cpp(272): warning C4018: '>': signed/unsigned mismatch [C:\projects\bitcoin\build_msvc\libbitcoinconsensus\libbitcoinconsensus.vcxproj]
c:\projects\bitcoin\src\test\allocator_tests.cpp(147): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\crypto_tests.cpp(535): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
c:\projects\bitcoin\src\test\scheduler_tests.cpp(57): warning C4305: 'argument': truncation from 'int' to 'bool' [C:\projects\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
```
Tree-SHA512: 5051134126c570b8421d57c710f1f1b977600398d2b5e69f8a8bd766b3696f992bf4e3459643b99a6b7e08dee1adc92985ee4d0d52b20755954415cb6f23f2fb
68bfc0bce3 doc: correct GetDifficulty doc after #13288 (fanquake)
Pull request description:
`chain` is no longer passed to GetDifficulty, and we just return `1.0` if no `blockindex`.
Tree-SHA512: 701375d732f343200c4abfaf9039d5c12b10abff97b022e84564f81b26b5ba552f1eb0c0d0fd5370b29b53319eafcf39773a36e1c2dd04ee77e61c18c7b183fa
Call sync.h primitives "locks" and "mutexes" instead of "blocks" and "waitable
critical sections" to match current coding conventions and c++11 standard
names.
This PR does not rename the "CCriticalSection" class (though this could be done
as a followup) because it is used everywhere and would swamp the other changes
in this PR. Plain mutexes should mostly be preferred instead of recursive
mutexes in new code anyway.
-BEGIN VERIFY SCRIPT-
set -x
set -e
ren() { git grep -l $1 | xargs sed -i s/$1/$2/; }
ren CCriticalBlock UniqueLock
ren CWaitableCriticalSection Mutex
ren CConditionVariable std::condition_variable
ren cs_GenesisWait g_genesis_wait_mutex
ren condvar_GenesisWait g_genesis_wait_cv
perl -0777 -pi -e 's/.*typedef.*condition_variable.*\n\n?//g' src/sync.h
-END VERIFY SCRIPT-
9c4dc597dd Use LOCK macros for non-recursive locks (Russell Yanofsky)
1382913e61 Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection (Russell Yanofsky)
ba1f095aad MOVEONLY Move AnnotatedMixin declaration (Russell Yanofsky)
41b88e9337 Add unit test for DEBUG_LOCKORDER code (Russell Yanofsky)
Pull request description:
Make LOCK macros work with non-recursive mutexes, and use wherever possible for better deadlock detection.
Also add unit test for DEBUG_LOCKORDER code.
Tree-SHA512: 64ef209307f28ecd0813a283f15c6406138c6ffe7f6cbbd084161044db60e2c099a7d0d2edcd1c5e7770a115e9b931b486e86c9a777bdc96d2e8a9f4dc192942
ca1a093127 Add regression test: Don't assert(...) with side effects (practicalswift)
4c3c9c3869 Don't assert(...) with side effects (practicalswift)
Pull request description:
Don't `assert(...)` with side effects.
From the developer notes:
> **Assertions should not have side-effects**
>
> Rationale: Even though the source code is set to refuse to compile with assertions disabled, having side-effects in assertions is unexpected and makes the code harder to understand
These assertions were introduced quite recently (in #14069 which was merged two days ago) and since this is a recurring thing (see #13534 – "Don't assert(foo()) where foo() has side effects" from May) I added a simple regression test for the most obvious common side effect.
Tree-SHA512: be65db9d8d5d0f5752152ba73fe3fbb0531880f156d3cd7dfdf1752709979b63214e46ae64b1adbe1e09fa121278f4087f4ae49bff16cf8f5aec16ea6bde3650
75ea00f391 Remove unused fsbridge::freopen (practicalswift)
cceedbc4bf Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift)
Pull request description:
Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`).
Context: https://github.com/bitcoin/bitcoin/pull/13148#issuecomment-386288606
Thanks @ajtowns!
Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1
Currently we log a message indicating that a bitcoin.conf file is being used
even if one does not exists. This commit changes the logic to only display
this message if a config file exists and logs a separate message
if no config file exists. Additionally, a warning is now logged if the file
path passed in the -conf flag does not exist.
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
Report errors while parsing the configuration file, instead of silently
ignoring them.
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: sdafsdfafs
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -
0e534d4dca Fix incorrect Doxygen comments (practicalswift)
Pull request description:
Fix broken Doxygen comments.
This commit was taken from #13914 which now only covers `-Wdocumentation`.
Tree-SHA512: dddbca16bb792b8193e5f417151b5eace9acc942a321f1bc095b906e98889e3bd93509fe112ab6a24ee1f6a3a918db905bda7acefd53774fe3e6ebe669fb51ac
fa309dc305 validation: Log FormatStateMessage on ConnectBlock error in ConnectTip (MarcoFalke)
Pull request description:
This change additionally logs the validation state on error, which is not logged at all on current master.
Before:
```
ERROR: ConnectTip(): ConnectBlock ffffff.... failed
```
After:
```
ERROR: ConnectTip: ConnectBlock ffffff.... failed, bad-cb-amount (code 16)
```
Tree-SHA512: e69ee0266772b3f77c0193c4a959c2444bf1a51259bd29d790cf665582b037997e520c8567f70b36362c071dcfe1a8ebd7c0f2286cf1b842df5731960e7e1ba0
fa587773e5 scripted-diff: Remove unused first argument to addUnchecked (MarcoFalke)
fe5c49766c tx pool: Use the entry's hash instead of the one passed to addUnchecked (MarcoFalke)
ddd395f968 Mark CTxMemPoolEntry members that should not be modified const (MarcoFalke)
Pull request description:
Several years ago the transaction hash was not cached. For optimization the hash was instead passed into `addUnchecked` to avoid re-calculating it. See f77654a0e9
Passing in the hash is now redundant and the argument can safely be removed.
Tree-SHA512: 0206b65c7a014295f67574120e8c5397bf1b1bd70c918ae1360ab093676f7f89a6f084fd2c7000a141baebfe63fe6f515559e38c4ac71810ba64f949f9c0467f
b193d5a443 Removes the Boost case_conv.hpp dependency. (251)
7a208d9fad Implements custom tolower and toupper functions. (251)
e2ba043b8d Implements ParseNetwork unit test. (251)
Pull request description:
This pull request removes the `boost/algorithm/string/case_conv.hpp` dependency from the project.
`boost/algorithm/string/case_conv.hpp` is included for the `boost::to_lower` and `boost::to_upper` template functions.
We can replace the calls to these functions with straightforward alternative implementations that use the C++ Standard Library, because the functions are called with `std::string` objects that use standard 7-bit ASCII characters as argument.
The refactored implementation should work without the explicit `static_cast<unsigned char>` cast and `unsigned char` lambda return type. Both have been added defensively and to be explicit. Especially in case of the former, behaviour is undefined (potentially result in a crash) if the `std::toupper` argument is not an `unsigned char`.
A potential alternative, maybe even preferred, implementation to address the `boost::to_lower` function call in `ParseNetwork(std::string)` could have been:
```c++
if (net == "ipv4" || net == "IPv4") return NET_IPV4;
if (net == "ipv6" || net == "IPv6") return NET_IPV6;
```
This alternative implementation would however change the external behaviour of `ParseNetwork(std::string)`.
This pull requests includes a unit test to validate the implementation of `ParseNetwork(std::string)` prior and after the removal of the `case_conv.hpp` dependency.
`boost/algorithm/string/case_conv.hpp` has been removed from the `EXPECTED_BOOST_INCLUDES` in `test/lint/lint-includes.sh` because it is no longer required.
Tree-SHA512: d803ae709f2368a3efb223097384a722436955bce0c44a1a5cffd0abb3164be0cce85ba0e9ebd9408166df3f1a95ea0c0d29e3a2534af2fae206c0419d67fde9
1661a472b8 add unicode compatible file_lock for Windows (Chun Kuan Lee)
Pull request description:
boost::interprocess::file_lock cannot open the files that contain characters which cannot be parsed by the user's code page on Windows.
This PR is seperated from #13426 for easier review.
Tree-SHA512: e240479cda65958bf6e1319840b83928b2b50da81d99f4f002fb3b62621370bcd4bcfacd2b8c0678c443a650d6ba53d9d12618b591e5bfd67ac14388a18fd822
1cc58978b7 tests: Fix accidental trunction from int to bool (practicalswift)
Pull request description:
Fix accidental trunction from `int` to `bool`.
Context: https://github.com/bitcoin/bitcoin/pull/14086#issuecomment-416610313
Tree-SHA512: 72d209f892e580afa9c295174c206ea5ba764ff9e03613cd9bc57fd0d7118e895ee44d96db90930a29c0b4de7f51dc00101a1b32ba6b46576d34e089ff5482ba
This commit removes the `boost/algorithm/string/case_conv.hpp` dependency from the project. It replaces the `boost::to_lower` and `boost::to_upper` functions with custom functions that are locale independent and ASCII deterministic.
This commit implements custom equivalents for the C and C++ `tolower` and `toupper` Standard Library functions.
In addition it implements a utility function to capitalize the first letter of a string.
917353c8b0 Make SignPSBTInput operate on a private SignatureData object (Pieter Wuille)
cad5dd2368 Pass HD path data through SignatureData (Pieter Wuille)
03a99586a3 Implement key origin lookup in CWallet (Pieter Wuille)
3b01efa0d1 [MOVEONLY] Move ParseHDKeypath to utilstrencodings (Pieter Wuille)
81e1dd5ce1 Generalize PublicOnlySigningProvider into HidingSigningProvider (Pieter Wuille)
84f1f1bfdf Make SigningProvider expose key origin information (Pieter Wuille)
611ab307fb Introduce KeyOriginInfo for fingerprint + path (Pieter Wuille)
Pull request description:
This PR adds "key origin" (master fingeprint + key path) information to what is exposed from `SigningProvider`s, allowing this information to be used by the generic PSBT code instead of having the RPC pull it directly from the wallet.
This is also a preparation to having PSBT interact with output descriptors, which can then directly expose key origin information for the scripts they generate.
Tree-SHA512: c718382ba8ba2d6fc9a32c062bd4cff08b6f39b133838aa03115c39aeca0f654c7cc3ec72d87005bf8306e550824cd8eb9d60f0bd41784a3e22e17b2afcfe833
1ac3c983bf Mark single-argument constructors "explicit" (practicalswift)
Pull request description:
Mark single-argument constructors `explicit`.
Rationale:
* Avoid unexpected implicit promotions.
From the developer notes:
> **By default, declare single-argument constructors explicit.**
> Rationale: This is a precaution to avoid unintended conversions that might arise when single-argument constructors are used as implicit conversion functions.
Tree-SHA512: 7901ed5be808c9d0ecb5ca501e1bc0395987fe1b7941b8548cebac2ff08a14f7dab61fab374a69b9ba29a9295a04245c814325c7f95b97ae558af0780f111dfa
boost::interprocess::file_lock cannot open the files that contain characters which cannot be parsed by the user's code page on Windows.
This commit add a new class to handle those specific file for Windows.
update copyright headers
attempt to fix linting errors
Fixing issue with make check classifying generator files as actual unit tests
Wrapping gen files in ENABLE_PROPERTY_TESTS macro
Make macro better
5778bf95d9 Report minfeefilter value in getpeerinfo rpc (Anthony Towns)
Pull request description:
Lowering the minimum relay fee is only useful when many nodes in the p2p network also lower the fee, so to make it easier to understand progress on that front, this includes the value of the minfeefilter in getpeerinfo, so you at least have visibility to what fees your neighbours are currently accepting.
Tree-SHA512: 059f01bf2a32c98fce1648a13b7898701203b354d0209ee34e6683994b720eb594cf24968e66b699caae5e17e53d351e73281f042dd094decde14d3a318e9fb3
Lowering the minimum relay fee is only useful when many nodes in the
p2p network also lower the fee, so to make it easier to understand
progress on that front, this includes the value of the minfeefilter in
getpeerinfo, so you at least have visibility to what fees your neighbours
are currently accepting.
384273260a test: Add testing of value_ret for SelectCoinsBnB (Ben Woosley)
Pull request description:
Fix that the early bailout optimization tests did not test the intended
selection because their utxo pool was polluted by the make_hard_case test
preceding. Note the code was tested, just not with the constructed case.
Tree-SHA512: 95f665525f5922f70f4c17708c0c09900f38d7a652b5bdd817e017ba7ff2865a6234edbd340064ffccc20d34048c45df86a4ac5f46dd8f4aab98834e71dc9d3c
497e90c02b Remove default argument to prevector constructor to remove ambiguity (Ben Woosley)
Pull request description:
The call with this default argument is redundant with `prevector(size_type)` on line 251.
Tree-SHA512: 4d22e6f4cd56e4b700596d7f5afc945ec6684636a94690fa16a1bbb34e4f53b6340f53a6c314fea213359426474125228ba7193388789f8a13308506358e92db
f1640d093f Make IS_TRIVIALLY_CONSTRUCTIBLE consistent on GCC < 5 (Ben Woosley)
Pull request description:
`std::is_trivially_constructible<T>` is equivalent to `std::is_trivially_default_constructible<T>`
`std::has_trivial_default_constructor<T>` is the GCC < 5 name for `std::is_trivially_default_constructible<T>`
https://en.cppreference.com/w/cpp/types/is_default_constructiblehttps://www.gnu.org/software/gcc/gcc-5/changes.html
`std::is_trivial` was also used when compiling with clang, due to clang's use of `__GNUC__`. Test `__clang__` to target the intended implementations.
https://stackoverflow.com/a/28166605
All callers currently only pass one template argument to IS_TRIVIALLY_CONSTRUCTIBLE, with this change the build would fail if someone attempted passing more.
Tree-SHA512: 3e36ddf20a1c0d76ad94d7c95f3fe5b90f4ee00389d5516b35c657136205e7a3ddff60789b0b0b2375624631f15a51eaad3570ef19a7b9df1469a50ba28415d1
faf4a9b674 qa: Stop txindex thread before calling destructor (MarcoFalke)
Pull request description:
Same as #13894, but for the tests.
Tree-SHA512: a21d9f8ad8dc9703217d1808cb14bd969903c364fe30bbdc0dd2df170ddc0cbaba98b0bde28bc21ff1319222aaf6cb4f1b2c45cd6b236fe3c645a92eab6bacba
254c85b687 bench: Benchmark GCS filter creation and matching. (Jim Posen)
f33b717a85 blockfilter: Optimization on compilers with int128 support. (Jim Posen)
97b64d67da blockfilter: Unit test against BIP 158 test vectors. (Jim Posen)
a4afb9cadb blockfilter: Additional helper methods to compute hash and header. (Jim Posen)
cd09c7925b blockfilter: Serialization methods on BlockFilter. (Jim Posen)
c1855f6052 blockfilter: Construction of basic block filters. (Jim Posen)
53e7874e07 blockfilter: Simple test for GCSFilter construction and Match. (Jim Posen)
558c536e35 blockfilter: Implement GCSFilter Match methods. (Jim Posen)
cf70b55005 blockfilter: Implement GCSFilter constructors. (Jim Posen)
c454f0ac63 blockfilter: Declare GCSFilter class for BIP 158 impl. (Jim Posen)
9b622dc722 streams: Unit tests for BitStreamReader and BitStreamWriter. (Jim Posen)
fe943f99bf streams: Implement BitStreamReader/Writer classes. (Jim Posen)
87f2d9ee43 streams: Unit test for VectorReader class. (Jim Posen)
947133dec9 streams: Create VectorReader stream interface for vectors. (Jim Posen)
Pull request description:
This implements the compact block filter construction in [BIP 158](https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki). The code is not used anywhere in the Bitcoin Core code base yet. The next step towards [BIP 157](https://github.com/bitcoin/bips/blob/master/bip-0157.mediawiki) support would be to create an indexing module similar to `TxIndex` that constructs the basic and extended filters for each validated block.
### Filter Sizes
[Here](https://gateway.ipfs.io/ipfs/QmRqaAAQZ5ZX5eqxP7J2R1MzFrc2WDdKSWJEKtQzyawqog) is a CSV of filter sizes for blocks in the main chain.
As you can see below, the ratio of filter size to block size drops after the first ~150,000 blocks:
The reason for the relatively large filter sizes is that Golomb-coded sets only achieve good compression with a sufficient number of elements. Empirically, the average element size with 100 elements is 14% larger than with 10,000 elements.
The ratio of filter size to block size is computed without witness data for basic filters. Here is a summary table of filter size ratios *for blocks after height 150,000*:
| Stat | Filter Type |
|-------|--------------|
| Weighted Size Ratio Mean | 0.0198 |
| Size Ratio Mean | 0.0224 |
| Size Ratio Std Deviation | 0.0202 |
| Mean Element Size (bits) | 21.145 |
| Approx Theoretical Min Element Size (bits) | 21.025 |
Tree-SHA512: 2d045fbfc3fc45490ecb9b08d2f7e4dbbe7cd8c1c939f06bbdb8e8aacfe4c495cdb67c820e52520baebbf8a8305a0efd8e59d3fa8e367574a4b830509a39223f
9e0a514112 Add compile time checking for all cs_main runtime locking assertions (practicalswift)
Pull request description:
Add compile time checking for `cs_main` runtime locking assertions.
This PR is a subset of #12665. The PR was broken up to make reviewing easier.
The intention is that literally all `EXCLUSIVE_LOCKS_REQUIRED`/`LOCKS_EXCLUDED`:s added in this PR should follow either directly or indirectly from `AssertLockHeld(…)`/`AssertLockNotHeld(…)`:s already existing in the repo.
Consider the case where function `A(…)` contains `AssertLockHeld(cs_foo)` (without
first locking `cs_foo` in `A`), and that `B(…)` calls `A(…)` (without first locking `cs_main`):
* It _directly_ follows that: `A(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
* It _indirectly_ follows that: `B(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
Tree-SHA512: 120e7410c4c223dbc7d42030b1a19e328d01a55f041bb6fb5eaac10ac35cb0c5d469b9b3bda6444731164c73b88ac6495a00890672b107d9305e891571f64dd6
870bd4c73d Update functional RBF test to check replaceable flag (dexX7)
820d31f95f Add "bip125-replaceable" flag to mempool RPCs (dexX7)
Pull request description:
This pull request adds a flag "bip125-replaceable" to the mempool RPCs getrawmempool, getmempoolentry, getmempoolancestors and getmempooldescendants, which indicates whether an unconfirmed transaction might be replaced.
Initially the flag was added to the raw transaction RPCs, but thanks to @conscott, it was moved to the mempool RPCs, which actually have access to the mempool.
~~This pull request adds a flag "bip125-replaceable" to the RPCs "getrawtransaction" and "decoderawtransaction", which indicates, whether a transaction signals BIP 125 replaceability.~~
There was some discussion in #7817, whether showing replaceability in the UI could lead to the false assumption that transactions that don't signal BIP 125 are truely non-replaceable, but given that this PR tackles the raw transaction interface, which is a rather low level tool, I believe having this extra piece of information isn't bad.
Tree-SHA512: 1f5511957af2c20a9a6c79d80a335c3be37a2402dbf829c40cceaa01a24868eab81a9c1cdb0b3d77198fa3bb82799e3540a5c0ce7f35bbac80d73f7133ff7cbc
ddddce0e46 util: Replace boost::signals2 with std::function (MarcoFalke)
Pull request description:
This removes the `#include <boost/signals2/signal.hpp>` from `util.h` (hopefully speeding up the build time and reducing the memory usage further after #13634)
The whole translation interface is replaced by a function `G_TRANSLATION_FUN` that is set to nullptr in units that don't need translation. (Thus only set in the gui)
Tree-SHA512: 087c717358bbed8bdb409463e225239d667f1ced381abb10e7cd31a41dcdd2cebe20b43c2ee86f0f8e55d53301f75e963f07421a99a7ff4c0cad2c6a375c5ab1
Golomb-Rice coding, as specified in BIP 158, involves operations on
individual bits. These classes will be used to implement the
encoding/decoding operations.
fa6c3dea42 p2p: Clarify control flow in ProcessMessage() (MarcoFalke)
Pull request description:
`ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach.
Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162)
This patch does exactly that.
Also note that this patch is a subset of previous approaches such as #9608 and #10145.
Review suggestion: `git diff HEAD~ --function-context`
Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201
984d72ec65 Return the script type from Solver (Ben Woosley)
Pull request description:
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.
Tree-SHA512: 31864f856b8cb75f4b782d12678070e8b1cfe9665c6f57cfb25e7ac8bcea8a22f9a78d7c8cf0101c841f2a612400666fb91798bffe88de856e98b873703b0965
23f4343781 Add CMerkleTx::IsImmatureCoinBase method (Ben Woosley)
Pull request description:
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.
Tree-SHA512: 4470d07404a0707144f9827b9a94c5c4905f23ee6f9248edc5df599a59d28e21ea0201d8abe5d5d73b39cb05b60c861ea8e04767eef04433e2ee95dcfed653ee
std::is_trivially_constructible<T> is equivalent to std::is_trivially_default_constructible<T>
std::has_trivial_default_constructor<T> is the GCC < 5 name for std::is_trivially_default_constructible<T>
std::is_trivial was also used when compiling with clang, due to clang's use of __GNUC__. Test __clang__
to target the intended implementations.
317f2cb3f4 test: Check RPC settxfee errors (João Barbosa)
48618daf26 Add checks for settxfee reasonableness (Anthony Towns)
Pull request description:
When using the `settxfee` RPC, the value is silently ignored if it is less than either than minrelaytxfee or the wallet's mintxfee. This adds an error response if that's going to happen, but still allows "settxfee 0" to deliberately default to the minimum value.
Tree-SHA512: ce685584cf8d6b9ca2cc97196d494220e3892b6a804a458086e04b3a23df281da432ad0a3053106a064c90c541ddb6f6b96a27cf8376d45af1e44449baf88456
321159e53e don't report minversion wallet entry as unknown (Gregory Sanders)
Pull request description:
It is known in WalletBatch::LoadWallet
Tree-SHA512: 82f7e12f48ae7d17317074ce5b5e27c70ba8334b04adbf7cc863f8169cc1aa460b9454571e2698aa00059c8c8f669fe19c0d40c4910dcded260ddca6ce78be9d
faaac5caaa RPCTypeCheck bip32derivs arg in walletcreatefunded (Gregory Sanders)
1f0c4282e9 QA: add basic walletcreatefunded optional arg test (Gregory Sanders)
1f18d7b591 walletcreatefundedpsbt: remove duplicate replaceable arg (Gregory Sanders)
2252ec5008 Allow ConstructTransaction to not throw error with 0-input txn (Gregory Sanders)
Pull request description:
1) Previously an empty input argument transaction that is marked for replaceability fails to pass the `SignalsOptInRBF` check right before funding it. Explicitly check for that condition before throwing an error.
2) The rpc call had two separate `replaceable` arguments, each of which being used in mutually exclusive places. I preserved the `options` version to retain compatability with `fundtransaction`.
Tree-SHA512: 26eb0c9e2d38ea51d11f741d61100223253271a084adadeb7e78c6d4e9004636f089e4273c5bf64a41bd7e9ff795317acf30531cb36aeb0d8db9304b3c8270c3
macOS Qt minimal platform is frequently broken, and these are currently failing
with Qt 5.11.1.
The tests do pass when run on the full cocoa platform
(with `test_bitcoin-qt -platform cocoa`).
d795c610d3 [qt] TransactionView: highlight replacement tx after fee bump (Sjors Provoost)
Pull request description:
Consistent with #12421 which highlights the transaction after send.
<img width="747" alt="1" src="https://user-images.githubusercontent.com/10217/38036280-a7358ea4-32a6-11e8-8f92-417e9e1e3e8b.png">
<img width="685" alt="2" src="https://user-images.githubusercontent.com/10217/38036289-aac87040-32a6-11e8-9f94-81745ff6c592.png">
~I'm not too proud of the `QTimer::singleShot(10` bit; any suggestions on how to properly wait for the transactions table to become aware of the new transaction?~
Although I could have called `focusTransaction()` directly from `TransactionView::bumpFee()` I'm using the same signal as the send screen. This should make it easier to move fee bump / transaction replacement functionality around later.
Tree-SHA512: 242055b7c3d32c7b2cf871f5ceda2581221902fd53fa29e0b092713fc16d3191adbe8cbb28417d522dda9febec8cc05e07afe3489cd7caaecd33460c1dde6fbc
Currently RPC commands are formatted in a way that it's easy to read
and that test/lint/check-rpc-mappings.py can parse it.
To void breaking test/lint/check-rpc-mappings.py script by running
clang-format, RPC command definitions should be disabled for clang-format.
6d5fcad576 [gui] Make proxy icon from statusbar clickable (Cristian Mircea Messel)
Pull request description:
Clicking on the proxy icon will open settings showing the network tab
https://github.com/bitcoin/bitcoin/pull/11491#issuecomment-336685303
Tree-SHA512: c3549749296918818694a371326d1a3b1075478918aaee940b5c7119a7e2cb991dcfda78f20d44d6d001157b9b82951f0d5157b17f4f0d1a0a242795efade036
fa091b0016 qa: Add tests for submitheader (MarcoFalke)
36b1b63f20 rpc: Expose ProcessNewBlockHeaders (MarcoFalke)
Pull request description:
This exposes `ProcessNewBlockHeaders` as an rpc called `submitheader`. This can be used to check for invalid block headers and submission of valid block headers via the rpc.
Tree-SHA512: a61e850470f15465f88e450609116df0a98d5d9afadf36b2033d820933d8b6a4012f9f2b3246319c08a0e511bef517f5d808cd0f44ffca91d10895a938004f0b
b2f23c4153 [RPC] Remove getinfo deprecation warning (John Newbery)
Pull request description:
`getinfo` was removed in V0.16. A removal warning message was left in place to tell users that the method had been removed. We can remove that entirely in V0.18.
Tree-SHA512: bf93fbcf57a9be480438dcbdcab2dfd69ce277218b10628776975b093b3ffd2caa1751e0fb4cb0245443c81465693e2b8750e96d3e38632a78bae5ffa04f9212
1f6ff04e59 Use wildcard path in test_bitcoin.vcxproj (Chun Kuan Lee)
90cc69c0c7 ci: Add appveyor.yml to build on MSVC (Chun Kuan Lee)
4d0c7924d2 Make macro compatible with MSVC (Chun Kuan Lee)
Pull request description:
Introduce Appveyor CI for MSVC. This would require the owner adding appveyor to this repo. Also fix some MSVC incompatible code.
This `appveyor.yml` file is modified from @sipsorcery and @NicolasDorier 's code in #12613.
Appveyor CI result: https://ci.appveyor.com/project/ken2812221/bitcoin/build/1.0.151
Tree-SHA512: b5b0f1686a33e54325ea6de81606806a7d9a0f8d4acbb97c9ce598386e8fcb2220def264777609ed2b850ac8c490fd181303ea522c5a70487272d46995f4c52d
5df6f089b5 More tests of signer checks (Andrew Chow)
7c8bffdc24 Test that a non-witness script as witness utxo is not signed (Andrew Chow)
8254e9950f Additional sanity checks in SignPSBTInput (Pieter Wuille)
c05712cb59 Only wipe wrong UTXO type data if overwritten by wallet (Pieter Wuille)
Pull request description:
The current PSBT signing code can end up producing a non-segwit signature, while only the UTXO being spent is provided in the PSBT (as opposed to the entire transaction being spent). This may be used to trick a user to incorrectly decide a transaction has the semantics he intends to sign.
Fix this by refusing to sign if there is any mismatch between the provided data and what is being signed.
Tree-SHA512: b55790d79d8166e05513fc4c603a982a33710e79dc3c045060cddac6b48a1be3a28ebf8db63f988b6567b15dd27fd09bbaf48846e323c8635376ac20178956f4
bd19cc78cf Serialize non-witness utxo as a non-witness tx but always deserialize as witness (Andrew Chow)
43811e6338 Fix PSBT deserialization of 0-input transactions (Andrew Chow)
Pull request description:
0-input transactions can be ambiguously deserialized as being witness transactions. Since the unsigned transaction is never serialized as a witness transaction as it has no witnesses, we should always deserialize it as a non-witness transaction and set the serialization flags as such.
When a transaction is serliazed for the non-witness-utxo, it is always a valid network transaction and thus it should be always be deserialized as a witness transaction and the deserialzation flags are set as such.
Fixes#13958
Tree-SHA512: 1937b3cb2618534478d4f533541fb9efce3cb5badb5d1964bfe19400f4aacc6c8ecedaf1f20d26b20baf94f81fd07dfb15b3b08089ecbd63aeecbc18c7c48086
Strip out the witnesses when serializing the non-witness utxo. However
witness serializations are allowed, so make sure we always deserialize
as witness.
0-input transactions can be ambiguously deserialized as being witness
transactions. Since the unsigned transaction is never serialized as
a witness transaction as it has no witnesses, we should always
deserialize it as a non-witness transaction and set the serialization
flags as such.
Also always serialize the unsigned transaction as a non-witness transaction.
fa5ce27385 ui: Compile boost:signals2 only once (MarcoFalke)
Pull request description:
ui is one of the modules that poison other modules with `boost/signals2` headers. This moves the include to the cpp file and uses a forward declaration in the header.
Locally this speeds up the incremental build (building everything that uses the ui module) with gcc by ~5% for me. Gcc uses ~5% less memory.
Would be nice if someone could verify the numbers roughly.
I presume the improvements will be more pronounced if the other models would stop exposing the boost header as well.
Tree-SHA512: 078360eba330ddbca4268bd8552927eae242a239e18dfded25ec20be72650a68cd83af7ac160690249b943d33ae35d15df1313f1f60a0c28b9526853aa7d1e40
6ad0328f1c Don't assert(foo()) where foo has side effects (practicalswift)
Pull request description:
Don't `assert(foo())` where `foo` has side effects.
From `assert(3)`:
> If the macro `NDEBUG` is defined at the moment `<assert.h>` was last included, the macro `assert()` generates no code, and hence does nothing at all.
Bitcoin currently cannot be compiled without assertions, but we shouldn't rely on that.
Tree-SHA512: 28cff0c6d1c2fb612ca58c9c94142ed01c5cfd0a2fecb8e59cdb6c270374b215d952ed3491d921d84dc1b439fa49da4f0e75e080f6adcbc6b0e08be14e54c170
869193f5a6 docs: fixed bitcoin-cli -help output for help2man (Hennadii Stepanov)
Pull request description:
Currently `bitcon-cli -help` output forces help2man to produce `.TP` and `.IP` commands instead of a single `.IP` command for `-stdinrpcpass` option.
Removing an extra space fixes this issue.
This pull request is rebased from #13879
Tree-SHA512: 1c5b25ed2ef7b7de42bc6210165bdbabe63f045699487f2db4790e0d3176f6493dfd3e8e19f4ddc38b551539465d7b41aea570f20dccbc0609f00fdfee1b5180
4b7091a842 Replace median fee rate with feerate percentiles (Marcin Jachymiak)
Pull request description:
Currently, the `medianfeerate` statistic is calculated from the feerate of the middle transaction of a list of transactions sorted by feerate.
This PR instead uses the value of the 50th percentile weight unit in the block, and also calculates the feerate at the 10th, 25th, 75th, and 90th percentiles. This more accurately corresponds with what is generally meant by median feerate.
Tree-SHA512: 59255e243df90d7afbe69839408c58c9723884b8ab82c66dc24a769e89c6d539db1905374a3f025ff28272fb25a0b90e92d8101103e39a6d9c0d60423a596714
e306be7429 Use 72 byte dummy signatures when watching only inputs may be used (Andrew Chow)
48b1473c89 Use 71 byte signature for DUMMY_SIGNATURE_CREATOR (Andrew Chow)
18dfea0dd0 Always create 70 byte signatures with low R values (Andrew Chow)
Pull request description:
When creating signatures for transactions, always make one which has a 32 byte or smaller R and 32 byte or smaller S value. This results in signatures that are always less than 71 bytes (32 byte R + 32 byte S + 6 bytes DER + 1 byte sighash) with low R values. In most cases, the signature will be 71 bytes.
Because R is not mutable in the same way that S is, a low R value can only be found by trying different nonces. RFC 6979 for deterministic nonce generation has the option to specify additional entropy, so we simply use that and add a uin32_t counter which we increment in order to try different nonces. Nonces are sill deterministically generated as the nonce used will the be the first one where the counter results in a nonce that results in a low R value. Because different nonces need to be tried, time to produce a signature does increase. On average, it takes twice as long to make a signature as two signatures need to be created, on average, to find one with a low R.
Having a fixed size signature makes size calculations easier and also saves half a byte of transaction size, on average.
DUMMY_SIGNATURE_CREATOR has been modified to produce 71 byte dummy signatures instead of 72 byte signatures.
Tree-SHA512: 3cd791505126ce92da7c631856a97ba0b59e87d9c132feff6e0eef1dc47768e81fbb38bfbe970371bedf9714b7f61a13a5fe9f30f962c81734092a4d19a4ef33
18f690ec2f wallet: shuffle coins before grouping, where warranted (Karl-Johan Alm)
Pull request description:
Coins are randomly shuffled in coin selection to avoid unintentional privacy leaks regarding the user's coin set. For the case where a user has a lot of coins with the same destination, these will be grouped into groups of 10 *before* the shuffling.
It is unclear whether this has any implications at all, but this PR plugs the potential issue, if there ever is one, by shuffling the coins before they are grouped.
Issue brought up in https://github.com/bitcoin/bitcoin/pull/12257#discussion_r204554549
Tree-SHA512: fb50ed4b5fc03ab4853d45b76e1c64476ad5bcd797497179bc37b9262885c974ed6811159fd8e581f1461b6cc6d0a66146f4b70a2777c0f5e818d1322e0edb89
The `help2man` parses a string containing two spaces between words with an issue:
it gives out `.TP` and `.IP` commands instead of a single `.IP` command.
Removing an extra space fixes this issue.
Currently the `-help` output for the `-stdin` option looks without any issue due to eliminating
of two spaces between words by a `FormatParagraph` call for this particular case.
For consistency and preventing from future regressions extra spaces have been removed from the both lines.
The redundant `strprintf` call has been removed aswell.
Make sure that translations are synchronized with transifex before the
branch-off point to minimize the difference and prevent duplicate work.
Tree-SHA512: 41e71eaf14094606fd90011d035c551a635d5a715f865a49841dbe2b54a76b7fbf59a7918f86e5fd80a717e2934a9613fe463391fd01848d0a01e5c4e7e7fef0
This commit removes the `CBloomFilter::CBloomFilter(const unsigned int, const double, const unsigned int)` constructor, which became obsolete with 086ee67.
Removes medianfeerate result from getblockstats.
Adds feerate_percentiles which give the feerate of the 10th, 25th, 50th,
75th, and 90th percentile weight unit in the block.
bb5b1c0b2d [Docs] upgrade rescan time warning from minutes to >1 hour (Mason Simon)
Pull request description:
When I rescanned just now it took well over an hour. The time warning "may take minutes" didn't prepare me for that.
```
2018-08-08T03:10:17Z [wallet] Still rescanning. At block 174747. Progress=0.008341
2018-08-08T03:11:17Z [wallet] Still rescanning. At block 204233. Progress=0.024533
2018-08-08T03:12:17Z [wallet] Still rescanning. At block 221170. Progress=0.038340
...
2018-08-08T04:16:17Z [wallet] Still rescanning. At block 524815. Progress=0.957105
2018-08-08T04:17:17Z [wallet] Still rescanning. At block 528572. Progress=0.971323
2018-08-08T04:18:17Z [wallet] Still rescanning. At block 532458. Progress=0.986824
```
This is on a 4-core 4ghz system with a 7200rpm drive.
Tree-SHA512: 722ccf566bfd6a3381fa173e08849cb676fe4c1f1cb2c4b86b07df2a5dc1ca0d54797cbe8fd606cdc2c60fef2be7c98e052460decdac2132ba759cff822132e8
227d27e70c Use pushKV in some new PSBT RPCs. (Daniel Kraft)
Pull request description:
Most of the code uses `UniValue::pushKV` where appropriate, but some new RPC code related to PSBTs did not. This fixes those places - after this change, there are no remaining source files I could find that contain `push_back(Pair(`.
Tree-SHA512: d6567cf144d05d7e42276bd66ff4cd44413328f985772d11bb9d7339d32ab7c3438d4bb0040a37e75f8d193c610b08fa971073935885e0a178546aa045daf9fa
e254ff5d53 Introduce a maximum size for locators. (Gregory Maxwell)
Pull request description:
The largest sensible size for a locator is log in the number of blocks.
But, as noted by Coinr8d on BCT a maximum size message could encode a
hundred thousand locators. If height were used to limit the messages
that could open new attacks where peers on long low diff forks would
get disconnected and end up stuck.
Ideally, nodes first first learn to limit the size of locators they
send before limiting what would be processed, but common implementations
back off with an exponent of 2 and have an implicit limit of 2^32
blocks, so they already cannot produce locators over some size.
Locators are cheap to process so allowing a few more is harmless,
so this sets the maximum to 64-- which is enough for blockchains
with 2^64 blocks before the get overhead starts increasing.
Tree-SHA512: da28df9c46c988980da861046c62e6e7f93d0eaab3083d32e408d1062f45c00316d5e1754127e808c1feb424fa8e00e5a91aea2cc3b80326b71c148696f7cdb3
With watching only inputs, we do not know how large the signatures
for those inputs will be as their signers may not have implemented
71 byte signatures. Thus we estimate their fees using the 72 byte
dummy signature to ensure that we pay enough fees.
This only effects fundrawtransaction when includeWatching is true.
When extra entropy is not specified by the caller, CKey::Sign will
now always create a signature that has a low R value and is at most
70 bytes. The resulting signature on the stack will be 71 bytes when
the sighash byte is included.
Using low R signatures means that the resulting DER encoded signature
will never need to have additional padding to account for high R
values.
The largest sensible size for a locator is log in the number of blocks.
But, as noted by Coinr8d on BCT a maximum size message could encode a
hundred thousand locators. If height were used to limit the messages
that could open new attacks where peers on long low diff forks would
get disconnected and end up stuck.
Ideally, nodes first first learn to limit the size of locators they
send before limiting what would be processed, but common implementations
back off with an exponent of 2 and have an implicit limit of 2^32
blocks, so they already cannot produce locators over some size.
This sets the limit to an absurdly high amount of 101 in order to
maximize compatibility with existing software.
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.
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.
faab63111d shutdown: Stop threads before resetting ptrs (MarcoFalke)
Pull request description:
On shutdown some threads would continue to run after or during a pointer reset. This leads to occasional segfaults on shutdown.
Fix this by resetting the smart pointers after all threads that might read from them have been stopped.
This should fix:
* A segfault in the txindex thread, that occurs when the txindex destructor is done, but the thread was not yet stopped (as this is done in the base index destructor)
* A segfault in the scheduler thread, which dereferences conman. (e.g. CheckForStaleTipAndEvictPeers)
Tree-SHA512: abbcf67fadd088e10fe8c384fadfb90bb115d5317145ccb5363603583b320efc18131e46384f55a9bc574969013dfcbd08c49e0d42c004ed7212eca193858ab2
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
When verifying blocks at startup, the progress is printed in 10%
increments to logs. When -checklevel=4, however, the second half
of the verification (connecting the blocks again) does not log the
progress anymore. (It is still computed and shown in the UI, but
not printed to logs.)
This change makes the behaviour consistent, by adding the missing
progress logging also for level-4 checks.
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)"
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
13bb5cae31 Docs: fix GetWarnings docs to reflect behavior (Ben Woosley)
Pull request description:
In "gui", it returns all warnings, joined by a separator
In "statusbar", it returns the last warning set which seems notionally to be the most important, though that is debatable
Tree-SHA512: 5fc0dc68d143a040b7b893b7176188e2b064c2cf1d559420906e4de636e16e9ab7451a1b87603020a7a8f66d6b94f4ee6c7da2697efad879f9e6de9c0e0c9ac1
faa24441ec policy: Remove promiscuousmempoolflags (MarcoFalke)
Pull request description:
It seems odd to clutter validation code with features that can only ever be used for testing (testnet or regtest). Removing that test-only code makes the mempool logic less painful to understand and easier to reason about when changed or refactored in the future.
Tree-SHA512: 3b897aa9604ac8d82ebe9573c6efd468c93ddaa08d378ebc902e247b7aa6c68fcde71e5b449c08f17a067146cdc66dc50a67ce06d07607c27e5189a49c3fba3f
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
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
This commit slightly changes the format of the "Usage" strings in CLI
`-help` messages to meet the expection of the help2man tool, which we
use to generate man pages. On the way, we remove a few calls to
`strprintf()`, which became superficial after commit 32fbfda.
They should also work with any other mutex type which std::unique_lock
supports.
There is no change in behavior for current code that calls these macros with
CCriticalSection mutexes.
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.
7bf22bf0c2 gui: Reject options dialog when key escape is pressed (João Barbosa)
4a43306a4f gui: Reject edit address dialog when key escape is pressed (João Barbosa)
f7a553177d gui: Add GUIUtil::ItemDelegate with keyEscapePressed signal (João Barbosa)
Pull request description:
Currently `EditAddressDialog` and `OptionsDialog` don't close when the escape key is pressed. The `QDataWidgetMapper` instances prevents closing the dialogs because the escape key is used to reset the widgets values. More details and workarounds in https://stackoverflow.com/a/51487847 and http://qtramblings.blogspot.com/2010/10/qdatawidgetmapper-annoyances.html.
The adopted solution is different from the above references. It turns out that `QDataWidgetMapper::setItemDelegate` sets the event filter for all mapped widgets. So in this PR the mapper's delegate are changed to a custom `GUIUtil::ItemDelegate` that offers the signal `keyEscapePressed`, which is connected to the `QDialog::reject` slot.
Note that the installed event filter lets all events pass, so the current behaviour isn't changed, meaning that widgets values are reset in addition to closing the dialog.
Tree-SHA512: 9c961d488480b4ccc3880a11a8f1824b65f77570ee8918c7302c62775a1a73e52ae988a31a55ffff87b4170ddbecf833c2f09b66095c00eb6854a4d43f030f1f
312ff01ee5 -prune option -help output aligned with code (Hennadii Stepanov)
Pull request description:
The -help output for -prune is aligned with the code.
In the code (.../src/init.cpp#L1063):
```
if (nPruneTarget < MIN_DISK_SPACE_FOR_BLOCK_FILES) {
return InitError(strprintf(_("Prune configured below the minimum of %d MiB. Please use a higher number."), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024));
}
```
So correct value of nPruneTarget is **greater than or equal to** MIN_DISK_SPACE_FOR_BLOCK_FILES.
Tree-SHA512: 8e55aa99c8f5a9d020677b0f1b016215e2dbda5fa4ee7c8504b12a3abef226bc21beca118fa332c0bf206a4aff913a5a717b55bb5b2ecdba38423e9c0161209e
fa365021bb doc: Remove outdated net comment (MarcoFalke)
Pull request description:
`mapAddresses` and the corresponding "critsect" has been removed in 5fee401fe1 more than 6 years ago. Now is probably a good time to remove this confusing comment.
Tree-SHA512: 498a403d5703da395c18a7ebb776aa6e693e59fe43a839fefd261e0a5af58621763813979d4cfbd8d1728ce73b325b82002e393cde79bdbff33e0fbf68ab6747
fe7180c5b2 [trivial,doc] Fix memory consistency model in comment (Jesse Cohen)
Pull request description:
Updating a comment overlooked during review in #13247
Tree-SHA512: 0bd54ba1c265fdd77fd6e12ad0be46dd422348f7d926ce9abaca53fdb3a3c55c0d1cd90b4382321352076f4a81e2249c0014cd789f47a3637cb93bd983cb4657
f6b7fc349c Support h instead of ' in hardened descriptor paths (Pieter Wuille)
fddea672eb Add experimental warning to scantxoutset (Jonas Schnelli)
6495849bfd [QA] Extend tests to more combinations (Pieter Wuille)
1af237faef [QA] Add xpub range tests in scantxoutset tests (Jonas Schnelli)
151600bb49 Swap in descriptors support into scantxoutset (Pieter Wuille)
0652c3284f Descriptor tests (Pieter Wuille)
fe8a7dcd78 Output descriptors module (Pieter Wuille)
e54d76044b Add simple FlatSigningProvider (Pieter Wuille)
29943a904a Add more methods to Span class (Pieter Wuille)
Pull request description:
As promised, here is an implementation of my output descriptor concept (https://gist.github.com/sipa/e3d23d498c430bb601c5bca83523fa82) and integration within the `scantxoutset` RPC that was just added through #12196.
It changes the RPC to use descriptors for everything; I hope the interface is simple enough to encompass all use cases. It includes support for P2PK, P2PKH, P2WPKH, P2SH, P2WSH, multisig, xpubs, xprvs, and chains of keys - combined in every possible way.
Tree-SHA512: 63b54a96e7a72f5b04a8d645b8517d43ecd6a65a41f9f4e593931ce725a8845ab0baa1e9db6a7243190d8ac841f6e7e2f520d98c539312d78f7fd687d2c7b88f
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
cbeaa91dbb Update ValidationInterface() documentation to explicitly specify threading and memory model (Jesse Cohen)
b296b425a7 Update documentation for SingleThreadedSchedulerClient() to specify the memory model (Jesse Cohen)
9994d01d8b Add Unit Test for SingleThreadedSchedulerClient (Jesse Cohen)
Pull request description:
As discussed in #13023 I've split this test out into a separate pr
This test (and documentation update) makes explicit the guarantee (previously undefined, but implied by the 'SingleThreaded' in `SingleThreadedSchedulerClient()`) - that callbacks pushed to the `SingleThreadedSchedulerClient()` obey the single threaded model for memory and execution - specifically, the callbacks are executed fully and in order, and even in cases where a subsequent callback is executed by a different thread, sequential consistency of memory for all threads executing these callbacks is maintained.
Maintaining memory consistency should make the api more developer friendly - especially for users of the validationinterface. To the extent that there are performance implications from this decision, these are not currently present in practice because all use of this scheduler happens on a single thread currently, furthermore the lock should guarantee consistency across callback executions even when callbacks are executed by multiple threads (as the test does).
Tree-SHA512: 5d95a7682c402e5ad76b05bc9dfbca99ca64105f62ab9e78f6fc0f6ea8c5277aa399fbb94298e35cc677b0c2181ff17259584bb7ae230e38aa68b85ecbc22856
494634a052 bench: Make CoinSelection output groups pass eligibility filter (Andrew Chow)
Pull request description:
Set the depth of the output groups used in the CoinSelection benchmark to be 6 in order to pass the eligibility filter for the benchmark.
Fixes#13813
Tree-SHA512: 55fc6aeda0127f5e155efb982aec211b70dfd3257808dce627886af6866ffa25de4df3c9b10f8c45b6c298a42542c54654f36e59efb208e9055885361f0e501c
247d5740d2 Ignore unknown config file options for now (Pieter Wuille)
04ce0d88ca Report when unknown config file options are ignored (Pieter Wuille)
Pull request description:
As reported by @satwo on IRC a few days ago, the current mechanism of treating unknown config file options as errors is problematic for options like `-rpcclienttimeout` which aren't defined for `bitcoind`.
A full solution would be to either make all binaries be aware of each other's options, or to permit config file options that only apply to specific binaries (`bitcoind`, `bitcoin-qt`, `bitcoin-cli`). Both of these seem too invasive to introduce for 0.17.
As a compromise, this PR makes it ignores those options, but still warn about it in the log file.
Tree-SHA512: dfddc771b91df3031a9c98d9f3292f8f4fcd1b97ebb7317b2f457e12d9f205dc63f42721302e7258dbb53f273d7cc041a65a0a9120972769555784e1f1cc9aef
fa8f2d826c doc: Fix chainTxData comment (MarcoFalke)
fa6094f152 chainparams: Update with data from assumed valid chain (MarcoFalke)
Pull request description:
Can be reviewed by using the `getblock` and `getchaintxstats` rpcs of a synced node. Reviewers get extra points when their full node has checkpoints and assumevalid disabled.
Tree-SHA512: cedd61fde129ae4c16fc12275b61e4c5659b6d72dd801c608efc294188561bc986d94652fe9bea71ada48654258e2a074d2d2da78036c69608ccff3a6cc1ccf5
fa5ed4f8d2 refactor: Avoid locking tx pool cs thrice (MarcoFalke)
Pull request description:
`addUnchecked` is (outside the tests) only called by ATMP, which already takes the tx pool read lock. So locking it twice more in both `addUnchecked` methods seems redundant.
Similarly `CalculateMemPoolAncestors` is (beside once in the wallet) only called in contexts, where the tx pool lock is already taken. So remove the lock there as well.
Tree-SHA512: fcf603b570da0fc529fe6db8add218663eae52845510732bee0d4611263d2429d3d3c9c8ae68493d67287d13504500ed51905ccbe711eb15a0af3b019edad543
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.
620361fce8 Fix accidental use of the addition assignment operator ("+="). Remove newlines from error message. (practicalswift)
Pull request description:
Fix accidental use of the addition assignment operator (`+=`).
_Note to reviewers:_ Perhaps the `\n`:s should be removed too?
Tree-SHA512: 4e8c2dfd6025d78ef9d60522297994829dacc447e6b6782e15c0bdd5dd2daa17ca9a8948bfa9a15be57d9286092356381d7e6747980303852d273eb0df0dd76b
46340b3337 [bench] Add benchmark for unserialize prevector (Akio Nakamura)
Pull request description:
This PR adds benchmarks for the unserialization of the prevector.
Note: Separated from #12324.
Tree-SHA512: c055a283328cc2634c01eb60f26604a8665939bbf77d367b6ba6b4e01e77d4511fab69cc3ddb1e62969adb3c48752ed870f45ceba153eee192302601341e18a7
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
fad231ad41 Fix merging of global unknown data in PSBTs (Andrew Chow)
41df035ee1 Check that PSBT keys are the correct length (Andrew Chow)
Pull request description:
This PR fixes a few bugs that were found and adds tests checking for these errors.
Specifically:
- Single byte keys are checked to actually be one byte.
- Unknown global data must be merged when combining two PSBTs.
Tree-SHA512: c0e7b4bc607d510cc005aaa7c0813ee58c5467ab7ce4adce485522dfeee92b1af3d29fe89df778b0ea812bb3827e085b30e04d4f4ebcefd8364d809573991332
12dd101345 scripted-diff: Remove trailing whitespaces (João Barbosa)
Pull request description:
The script test/lint/lint-whitespace.sh should prevent new cases.
This happens in some pulls where the code editor and the author 'git add's them, so this would fix it all.
Tree-SHA512: bcdd3472fcd01a2754e52212c7db1de2fdc422728b06785481954a27162fb72001cb73708329cc56e95bcc5e45c1348ebc4eacc2ccfa6aa12413c7ec450b6a33
Clicking on the proxy icon will open settings showing the network tab
Create enum Tab in OptionsModel
Use new connect syntax
Use lambda for private slots
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
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
84547fa6d4 Avoid creating a temporary vector for size-prefixed elements (Pieter Wuille)
Pull request description:
This is a simple improvement to the PSBT serialization code, avoiding the need for temporary vectors everywhere.
Tree-SHA512: 9f7243b7169ec8ba00ffad31af03c016ab84e4f76ebac810167f91f5e8008f3827ad59fbcee0cb2bd2334fc26466eb222404af24e7fb6ec040fd78229ebe0fd1
fa451511a1 doc: Adjust bitcoincore.org links (MarcoFalke)
Pull request description:
Minor adjustments to our https://bitcoincore.org/ links:
Mainly adding links of the Bitcoin Core Github mirror and Bitcoin Core website to the doxygen introduction. (See e.g. current master: https://dev.visucore.com/bitcoin/doxygen/index.html). Also removing the link to bitcoin.org, to not imply there is only one resource that educates about bitcoin.
Tree-SHA512: d4d50f6de4d4b412203934e947889ebc0f564747e26f9190a2cff64234bf9fc3d56b8f056651550ce568170fba448559fa005959ef4e504d990e2fbc96a2ed77
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.
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
0454b56d8a trivial: remove unneeded include (Nikolay Mitev)
Pull request description:
Remove dead include
Tree-SHA512: 66380fe25259d37a19f955142ad53da24d4927064a84249989f54bebc21d9d688236fb60979acc79f219b05692c4c73b3ebab0872b8d03ab2447b0b44a06c8ed
793290f940 Net: Fixed a race condition when disabling the network. (lmanners)
Pull request description:
This change addresses a race condition where setnetworkactive=false wouldn't always disconnect all peers.
Before this change, the following could happen:
1. Thread A -- Begins connecting to a node.
2. Thread B -- Sets kNetworkActive=false and disconnects connected nodes.
3. Thread A -- Finishes connecting and adds node to list of connected nodes.
The node that was connected from Thread A remains connected and active,
even though kNetworkActive=false.
To fix the race, disconnections when kNetworkActive=false are now handled in the main network loop.
fixes#13038
Tree-SHA512: 6d0b7a78ae956358e796efcc034cb532c2e0d824a52ae822a3899eefc7df76327519d1f2d77362c9fddf34ef860abd095d7490d7cc02d0ba7543bf1e8c8f2535
5617840392 Drop dead code from Stacks (Ben Woosley)
Pull request description:
Stacks is local to this file, and only used in DataFromTransaction, so
it's easy to confirm this code is unused.
Tree-SHA512: cc680c99f9b31cb56db70f453087d642f83906ce594c07a6bf3e61427cfbee41441495d440b240419ba3386582cf0670c0999b2f51e7fd56b00e0a0f3f618845
9544a3f3fc tiny refactor for ArgsManager (AtsukiTak)
Pull request description:
This PR contains some small refactors for `ArgsManager`.
1. Mark `const` on member function if it possible.
2. Remove unused `error` argument from `ArgsManager::IsArgKnown`.
I'm not sure whether these refactors should be separated into another PR. If so, I will do that.
Tree-SHA512: 7df09e7f7c4cdd2e7cd60e34137faa7ca7463be66490f8552fdea3656da6ccc98886e258bdeef21820d197fc2fde06ab2d3405205f5de53f258df7c191d206b0
01a06d6686 Avoid locking mutexes that are already held by the same thread (practicalswift)
Pull request description:
Avoid locking mutexes that are already held by the same thread. These are reentrant mutexes, but still no need to lock them more than once per thread :-)
Tree-SHA512: e2fb85882e8800892fd8e8170f3c13128d6acfeb14d7b69fb9555f2b7ad0884fb201cf945b8144ffaf6fb1253c28af7c8c6c435319a7ae30ca003f28aa645a98
ac8a1d092e [RPC] Remove field in getblocktemplate help that has never been used (Conor Scott)
Pull request description:
[BIP 22 - getblocktemplate](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki#Transactions%20Object%20Format) specifies an optional flag, `required` if the transaction must be in the block.
Luke's implementation #936 did not include this flag, and it was later added to the help description in #3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a `getblocktemplate` call, so it seems logical to remove it from the help description.
If I am missing something or this is considered harmless - I can close the PR.
Tree-SHA512: f25dda51cc4e1512aff69309be04e3053bdccc1cf03c8d58e8866aa1fdf9d86cc57df872e85528351fc8a8d6d64a8f46a36c513680834762d854f368fbeb0f44
6755569840 trivial: Replace CPubKey::operator[] with CPubKey::vch where possible (Nikolay Mitev)
Pull request description:
Use variable name instead of calling operator[] through &(*this)[0]
Tree-SHA512: 7054ffda0fa33fb45d4d9f3b29698643f02fd1421d78d5197a0881f2c368dc410647fd2e1a6feb8048e30f8ab8bc2fa8749bf42b9ccbe42c30de8ff80ac45274
5f019d5354 Removes the boost/algorithm/string/join dependency (251)
Pull request description:
This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in https://github.com/bitcoin/bitcoin/pull/13726#discussion_r204159967
Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
This commit contains 2 refactors.
1. mark "const" on ArgsManager::GetHelpMessage and IsArgKnown.
2. remove unused "error" argument from ArgsManager::IsArgKnown.
Firstly, I mark "const" on where it is possible to. It is mentioned
before (e.g. https://github.com/bitcoin/bitcoin/pull/13190#pullrequestreview-118823133).
And about 2nd change, ArgsManager::IsArgKnown was added at commit #4f8704d which was
merged at PR #13112. But from its beggining, "error" argument never be used.
I think it should be refactored.
f447a0a707 Remove program options from build system (Chun Kuan Lee)
11588c639e Replace boost program_options (Chun Kuan Lee)
Pull request description:
Concept from #12744, but without parsing negated options.
Tree-SHA512: 7f418744bb8934e313d77a5f162633746ef5d043de802b9c9cd9f7c1842e7e566eb5f171cd9e2cc13317281b2449c6fbd553fa4f09b837e6af2f5d2b2aabdca2
fa4bf92be9 Remove dead service bits code (MarcoFalke)
Pull request description:
Seems fine to remove for the upcoming 0.17 release
Fixes#10993
Tree-SHA512: 3a4664b787e3da399bcaaba693619bd384826df14f469dbdfbbfffc540d9da3f2b322cda262b43388376785f77907c2540541c239ab0fca82bd7eb69d02b6b7a
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
Checks that all of the one byte type keys are actually one byte and
throw an error if they are not.
Add tests for each type to check for this behavior.
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
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
cd3f4aa808 Decouple wallet version from client version (Andrew Chow)
Pull request description:
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.
Tree-SHA512: 69c3e1f45a40bde01d622d504a803fea32fc14e2e27b14b0729725349d8592d56ebca26fd06f117fd6f5164fb4ce980122751b6370f6e25f1a947dbdf4143ddd
020628e3a4 Tests for PSBT (Andrew Chow)
a4b06fb42e Create wallet RPCs for PSBT (Andrew Chow)
c27fe419ef Create utility RPCs for PSBT (Andrew Chow)
8b5ef27937 SignPSBTInput wrapper function (Andrew Chow)
58a8e28918 Refactor transaction creation and transaction funding logic (Andrew Chow)
e9d86a43ad Methods for interacting with PSBT structs (Andrew Chow)
12bcc64f27 Add pubkeys and whether input was witness to SignatureData (Andrew Chow)
41c607f09b Implement PSBT Structures and un/serialization methods per BIP 174 (Andrew Chow)
Pull request description:
This Pull Request fully implements the [updated](https://github.com/bitcoin/bips/pull/694) BIP 174 specification. It is based upon #13425 which implements the majority of the signing logic.
BIP 174 specifies a binary transaction format which contains the information necessary for a signer to produce signatures for the transaction and holds the signatures for an input while the input does not have a complete set of signatures.
This PR contains structs for PSBT, serialization, and deserialzation code. Some changes to `SignatureData` have been made to support detection of UTXO type and storing public keys.
***
Many RPCs have been added to handle PSBTs.
`walletprocesspsbt` takes a PSBT format transaction, updates the PSBT with any inputs related to this wallet, signs, and finalizes the transaction. There is also an option to not sign and just update.
`walletcreatefundedpsbt` creates a PSBT from user provided data in the same form as createrawtransaction. It also funds the transaction and takes an options argument in the same form as `fundrawtransaction`. The resulting PSBT is blank with no input or output data filled in. It is analogous to a combination of `createrawtransaction` and `fundrawtransaction`
`decodepsbt` takes a PSBT and decodes it to JSON. It is analogous to `decoderawtransaction`
`combinepsbt` takes multiple PSBTs for the same tx and combines them. It is analogous to `combinerawtransaction`
`finalizepsbt` takes a PSBT and finalizes the inputs. If all inputs are final, it extracts the network serialized transaction and returns that instead of a PSBT unless instructed otherwise.
`createpsbt` is like `createrawtransaction` but for PSBTs instead of raw transactions.
`convertpsbt` takes a network serialized transaction and converts it into a psbt. The resulting psbt will lose all signature data and an explicit flag must be set to allow transactions with signature data to be converted.
***
This supersedes #12136
Tree-SHA512: 1ac7a79e5bc669933f0a6fcc93ded55263fdde9e8c144a30266b13ef9f62aacf43edd4cbca1ffbe003090b067e9643c9298c79be69d7c1b10231b32acafb6338
417b6c1d29 bitcoinconsensus: invalid flags should be set to bitcoinconsensus_error type, add test cases covering bitcoinconsensus error codes (Thomas Kerin)
Pull request description:
A check was added to the bitcoinconsensus verify_script codepath to ensure that callers only used _exposed_ interpreter flags. I think this error should be written to `bitcoinconsensus_err* err` and not returned by verify_script?
I modified the check so it indicates the error using *err like the others, and added tests covering the error codes.
Tree-SHA512: 8ab370e56956a7d4740f83475e6078774affd663ac92383a02b85295da550f1b4f7a7a68f32ed5c5bcb39d98e2f15ec0b76de8399887e7763eb7c1e21d131093
822a2a33a7 Modified in_addr6 cast in CConman class to work with msvc. (Aaron Clauson)
Pull request description:
Fix to allow net.cpp to compile with MSVC. Without this fix the `(in6_addr)IN6ADDR_ANY_INIT` implicit cast generates a compilation error.
Tree-SHA512: f21c5002401dc93564dcf8d49fbafe7c03ad4182df1616d2ee201e2e172f1d696ca7982fb5b42a3b7d6878c8649823044a858401b4172239fb4b0cc2a38db282
