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
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
