Makes AddAndGetDestinationForScript use a generic CKeyStore rather than
the wallet, and makes it always add the script to the keystore, rather
than only adding related (redeem) scripts.
63c16ed507 Use __cpuid_count for gnu C to avoid gitian build fail. (Chun Kuan Lee)
Pull request description:
Fixes#13538
Tree-SHA512: 161ae4db022288ae8631a166eaea2d08cf2c90bcd27218a094a754276de30b92ca9cfb5a79aa899c5a9d0534c5d7261037e7e915e1b92bc7067ab1539dc2b51e
#13004 changed the default behavior for printtoconsole but this has not been reflected in the command line help.
This fixes the description of -printtoconsole to reflect this change, and also provides the user with missing information on how to explicitly disable logging to debug.log.
At present I have made the latter update to two separate places (-printtoconsole and -debuglogfile) because a user looking for information on how to disable logging is probably going to look in the "Debugging/Testing Options" section. Moving -debuglogfile from the "General" options category to the "Debugging/Testing" section could potentially remove the need for this redundancy but may be out of the scope of this PR.
ea65182f03 [wallet] loadwallet shouldn't create new wallets. (John Newbery)
Pull request description:
A bug in the initial implementation of loadwallet meant that if the
arguement was a directory that didn't contain a wallet.dat file, a new
wallet would be created in that directory. Fix that so that if a
directory is passed in, it must contain a wallet.dat file.
Bug reported by promag (João Barbosa).
Tree-SHA512: 0a59fa8a33fde51a88544ad288b00e4995284fe16424f643076aaba42b8244fff362145217650ee53d518dfab7efbed4237632c34cdd3dcbbecaa9ecaab5fd7b
Just calling atoi to convert strings to integers does not check for
valid integers very thoroughly; in particular, it just ignores
everything starting from the first non-numeral character. Even a string
like "foo" is fine and silently returns 0.
This meant that bitcoin-tx would not fail if such a string was passed in
various places where an integer is expected (like the locktime or an
input/output index); this means that it would, for instance, silently
accept a typo and interpret it in an unexpected way.
In this change, we use ParseInt64 for parsing strings to integers,
which actually verifies that the full string is valid as number.
New tests in the bitcoin-util-test cover the new error paths.
1fc605a8ae fix bench/prevector.cpp (Akio Nakamura)
Pull request description:
This patch intends to fix some incorrect action of bench/prevector.cpp.
1. PrevectorClear()
2nd call of ```clear()``` should to operate t1 instead of t0.
This patch changes t0 to t1.
2. PREVECTOR_TEST()
PREVECTOR_TEST macro should to call both
```PrevectorXX<nontrivial_t>(state)``` and ```PrevectorXX<trivial_t>(state)```
by specific ```"name"``` which given by parameter instead of calling
```PrevectorResize<>()``` regardless of ```"name"```.
This patch changes ```"PrevectorResize<"``` of this macro to
```"Prevector ## name<"```.
Tree-SHA512: d0498c6d627d7e96fc8ccfb329ca0be2641535b1ce1923d9b1fc720825f9bf4d7281dc8d5ae929038e37b3e625189af9807cb62e6d20933d73832a6dff4b5596
98b181323 [build] Tune wildcards for LIBSECP256K1 target (Karl-Johan Alm)
Pull request description:
Automake would think the target was out of date every time because e.g. '.deps' was updated.
Note: I am assuming that secp256k1 depends on `*.h`, `*.c`, ~~and `libsecp256k1-config.h`~~ (it's `.h` so already included), aside from pre-existing `include/*`. If there are other files that would require a rebuild of the `LIBSECP256K1` target, they should probably be added.
It would be neat if you could exclude specific files, rather than split it up like this, but it doesn't seem possible (https://www.gnu.org/software/make/manual/html_node/Wildcard-Function.html#Wildcard-Function)
Should probably note this:
```Bash
$ V=1 make check VERBOSE=1
Making check in src
make[1]: Entering directory '/home/user/workspace/bitcoin/src'
make[2]: Entering directory '/home/user/workspace/bitcoin/src'
make -C secp256k1 libsecp256k1.la
make[3]: Entering directory '/home/user/workspace/bitcoin/src/secp256k1'
make[3]: 'libsecp256k1.la' is up to date.
make[3]: Leaving directory '/home/user/workspace/bitcoin/src/secp256k1'
make check-TESTS check-local
make[3]: Entering directory '/home/user/workspace/bitcoin/src'
make[4]: Entering directory '/home/user/workspace/bitcoin/src'
make -C secp256k1 libsecp256k1.la
make[5]: Entering directory '/home/user/workspace/bitcoin/src/secp256k1'
make[5]: 'libsecp256k1.la' is up to date.
make[5]: Leaving directory '/home/user/workspace/bitcoin/src/secp256k1'
PASS: test/test_bitcoin.exe
```
Tree-SHA512: 62b133c76e882788dae0c14208a9f5acdbd731c2e7a248f9e01f488b8ec13f9d637d7ad0d63e18d324bb4e088f1836a936649b0fb97bee679eaadedbeed5c981
2f1a30c63 Fix MAX_STANDARD_TX_WEIGHT check (Johnson Lau)
Pull request description:
As suggested by the constant name and its comment in policy.h, a transaction with a weight of exactly MAX_STANDARD_TX_WEIGHT should be allowed. Users could be confused.
Tree-SHA512: af417de1c6a2e6796ebbb39aa0caad8764302ded155cb1bbfbe457e4567c199cc53256189832b17d4aeec369e190b3edd4c6116d5f0b8cf0ede6dfb4ed83bdd3
2dcd7b4ec logging: avoid nStart may be used uninitialized in AppInitMain warning (mruddy)
Pull request description:
Was getting the following compiler warning:
```
init.cpp: In function ‘bool AppInitMain()’:
init.cpp:1616:60: warning: ‘nStart’ may be used uninitialized in this function [-Wmaybe-uninitialized]
LogPrintf(" block index %15dms\n", GetTimeMillis() - nStart);
```
It's ok without this PR, but this PR renames `nStart` to `load_block_index_start_time`, makes it `const`, and also reduces the scope of the variable.
The logging line is moved such that the the time spent will be logged even if a shutdown is requested while the index is being loaded.
Having the log message output even when a shutdown is requested may be how this was intended to work before anyways. That could explain the leading space, as such a log message now looks like:
```
2018-06-30T11:34:05Z [0%]...[16%]...[33%]...[50%]... block index 25750ms
2018-06-30T11:34:17Z Shutdown requested. Exiting.
```
Tree-SHA512: 967048afbc31f2ce8f80ae7d33fee0bdcbe94550cf2b5b662087e2a7cff14a8bf43d909b30f930660c184ec6c3c7e1302a84e3e54fc1723f7412827f4bf2c518
b81560029 Remove CombineSignatures and replace tests (Andrew Chow)
ed94c8b55 Replace CombineSignatures with ProduceSignature (Andrew Chow)
0422beb9b Make SignatureData able to store signatures and scripts (Andrew Chow)
b6edb4f5e Inline Sign1 and SignN (Andrew Chow)
Pull request description:
Currently CombineSignatures is used to create the final scriptSig or an input. However ProduceSignature is capable of doing this itself. Using both CombineSignatures and ProduceSignature results in code duplication which is unnecessary.
To move the scriptSig construction to ProduceSignatures, the SignatureData class contains two maps to hold pubkeys mapped to signatures, and script ids mapped to scripts. DataFromTransaction is extended to be able to extract signatures, their public keys, and scripts from existing ScriptSigs.
The SignaureData are then passed down to SignStep which can use the aforementioned maps to get the signatures, pubkeys, and scripts that it needs, falling back to the actual SigningProvider and SignatureCreator if the data are not available in the SignatureData.
Additionally, Sign1 and SignN have been removed and their functionality inlined into SignStep since Sign1 is really just a wrapper around CreateSig.
Since ProduceSignature can produce the final scriptSig or scriptWitness by using SignatureData which has extracted data from the transaction, CombineSignatures is unnecessary as ProduceSignature is able to replicate all of CombineSignatures' functionality.
This also furthers BIP 174 support and begins moving towards a BIP 174 style backend.
The tests have also been updated to use the new combining methodology.
Tree-SHA512: 78cd58a4ebe37f79229bd5eee2958a0bb45cd7f36d0e993eee13ff685b3665dd76ef2dfd5f47d34678995bb587f5594100ee5f6c09b1c69ee96d3684d470d01e
49d1f4cdd Detect if char equals int8_t (Chun Kuan Lee)
Pull request description:
Probably fixes#13576. I'm not able to test this. @stacepellegrino, can you test this?
Tree-SHA512: b750e00e11e6b6f6341fec668ec2254cc101c8ebdd4878f320d6cb3b07cf326761146e4ceff0b6405b7e503ff64c093a8274bd524a097e2c49382dc296972c4f
1. PrevectorClear()
2nd call of clear() should to operate t1 instead of t0.
This patch changes t0 to t1.
2. PREVECTOR_TEST()
PREVECTOR_TEST macro should to call both
PrevectorXX<nontrivial_t>(state) and PrevectorXX<trivial_t>(state)
by specific "name" which given by parameter instead of calling
PrevectorResize<>() regardless of "name".
This patch changes "PrevectorResize<" of this macro to
"Prevector ## name<".
This adds a new RPC method "getzmqnotifications", which returns
information about all active ZMQ notification endpoints. This is useful
for software that layers on top of bitcoind, so it can verify that
ZeroMQ is enabled and also figure out where it should listen.
See https://github.com/bitcoin/bitcoin/issues/13526.
b330c3001 Docs: Improve doc of options addnode, connect, seednode (wodry)
Pull request description:
Just clarify that options `addnode`, `connect` and `seednode` can be specified multiple times.
Tree-SHA512: ed149cabe7fc1d40f2fb6ad8b643656e0ec49cfae1834c157c89170eac1241efa3c5683d97266ff921f5229f28d732c9f7ee030e7902d9a79db1e0c8716fa3db
1fabd59e7 Break circular dependency: init -> * -> init by extracting shutdown.h (Ben Woosley)
e62fdfeea Drop unused init.h includes (Ben Woosley)
Pull request description:
Most includers just wanted to react to pending shutdown.
This isolates access to `fRequestShutdown` and limits access to the shutdown api functions, including the new `CancelShutdown` for setting it to `false`.
Tree-SHA512: df42f75dfbba163576710e9a67cf1228531fd99d70a2f187bfba0bcc476d6749cf88180a97e66a81bb5b6c3c7f0917de7402d26039ba7b644cb7509b02f7e267
07c493f2d scripted-diff: Replace NET_TOR with NET_ONION (wodry)
Pull request description:
This is a follow-up to #13532, where @promag already asked if this renaming would make sense.
If network shall be named _Onion_ instead of _Tor_ (like in the option `onlynet`), renaming the network enum NET_TOR to NET_ONION maybe would make sense and be stringent.
Change was produced with the following script:
```
#!/bin/bash
for file in $(grep --exclude-dir='.git' --files-with-matches --binary-files=without-match --recursive NET_TOR bitcoin/)
do
sed --in-place --expression='s/NET_TOR/NET_ONION/g' $file
done
```
_Tor_ is used at many other places in the code, though.
Tree-SHA512: 4ffdeca8115031465eb64e1c76694fb77b5900c4ea465d3c13d9b6b75a1eb04c45913f83cdc8bdbef28936aeec4655f1d4905b3b98407da3263632a2128a8d23
bb582a59c Add P2WSH destination helper and use it instead of manual hashing (Pieter Wuille)
eaba1c111 Add additional unit tests for invalid IsMine combinations (Pieter Wuille)
e6b9730c4 Do not expose invalidity from IsMine (Pieter Wuille)
Pull request description:
This improves the handling of INVALID in IsMine:
* Extra INVALID conditions were added to `IsMine` (following https://github.com/bitcoin/bitcoin/pull/13142/files#r185349057), but these were untested. Add unit tests for them.
* In https://github.com/bitcoin/bitcoin/pull/13142#issuecomment-386396975 it was suggested to merge `isInvalid` into the return status. This PR takes a different approach, and removes the `isInvalid` entirely. It was only ever used inside tests, as normal users of IsMine don't care about the reason for non-mine-ness, only whether it is or not. As the unit tests are extensive enough, it seems sufficient to have a black box text (with tests for both compressed and uncompressed keys).
Some addition code simplification is done as well.
Tree-SHA512: 3267f8846f3fa4e994f57504b155b0e1bbdf13808c4c04dab7c6886c2c0b88716169cee9c5b350513297e0ca2a00812e3401acf30ac9cde5d892f9fb59ad7fef
Instead of using CombineSignatures to create the final scriptSig or
scriptWitness of an input, use ProduceSignature itself.
To allow for ProduceSignature to place signatures, pubkeys, and scripts
that it does not know about, we pass down the SignatureData to SignStep
which pulls out the information that it needs from the SignatureData.
In addition to having the scriptSig and scriptWitness, have SignatureData
also be able to store just the signatures (pubkeys mapped to sigs) and
scripts (script ids mapped to scripts).
Also have DataFromTransaction be able to extract signatures and scripts
from the scriptSig and scriptWitness of an input to put them in SignatureData.
Adds a new SignatureChecker which takes a SignatureData and puts pubkeys
and signatures into it when it successfully verifies a signature.
Adds a new field in SignatureData which stores whether the SignatureData
was complete. This allows us to also update the scriptSig and
scriptWitness to the final one when updating a SignatureData with another
one.
-BEGIN VERIFY SCRIPT-
sed --in-place'' --expression='s/NET_TOR/NET_ONION/g' $(git grep -I --files-with-matches 'NET_TOR')
-END VERIFY SCRIPT-
The --in-place'' hack is required for sed on macOS to edit files in-place without passing a backup extension.
f618ebc4e4 validation: count blocks correctly for check level < 3 (Karl-Johan Alm)
Pull request description:
As noted in https://github.com/bitcoin/bitcoin/pull/13428#issuecomment-396129295 there is a bug where if check level < 3, the resulting count for blocks is wrong, because `pindexState` is never updated.
Post-commit `./bitcoin-cli verifychain 1 3`:
```
2018-06-11T07:12:28Z Verifying last 3 blocks at level 1
2018-06-11T07:12:28Z [0%]...[33%]...[66%]...[99%]...[DONE].
2018-06-11T07:12:28Z No coin database inconsistencies in last 3 blocks (0 transactions)
```
Pre-commit `./bitcoin-cli verifychain 1 3`:
```
2018-06-11T07:13:34Z Verifying last 3 blocks at level 1
2018-06-11T07:13:34Z [0%]...[33%]...[66%]...[99%]...[DONE].
2018-06-11T07:13:34Z No coin database inconsistencies in last 0 blocks (0 transactions)
```
Tree-SHA512: 3d82ed26665162c9615fb0e6e91a46ed4d229a5e6797c6c420e6b0bf1be6e5e02401c6e9a93b7a5aec503a2650d8c20d1b45fe300a922379e4cef8ee26e18d96
4132ad3bf Show symbol for inbound/outbound in peer table (wodry)
Pull request description:
Fixes#13483
The address in the network peer table is prefixed with an up-arrow symbolizing an outbound connection, or an down-array symbolizing an inbound connection. See screenshot.
The user has an easy visual confirmation about the connection direction state. I really like it :)
Impact to columns sorting is grouping by inbound/outbound first, which in my opinion is an advantage, too.
Tree-SHA512: d355f679d34c3006743c06750be5f36a083c1a8376da8f5f35045fcd9df964153409946fdde5007734f23bd692c91355962dc42df31122cdcf88e4affce8bc0e
This moves the used instance of CZMQNotificationInterface from a static
variable in init.cpp to a globally-accessible one declared in
zmq/zmqnotificationinterface.h. The variable is also renamed to
g_zmq_notification_interface, to be consistent with other globals.
We need this to implement a new RPC method "getzmqnotifications" (see
https://github.com/bitcoin/bitcoin/issues/13526) in a follow up.
c2e4fc84ec bench: Simplify CoinSelection (João Barbosa)
Pull request description:
Closes#13549.
As pointed by @MarcoFalke:
- `SelectCoinsMinConf` should always succeed as there are enough coins in the wallet.
- Removed creating the coins in the wallet.
Tree-SHA512: 965c363bcaf0ca7a1dec35b5cf4866abcf190c53eb7012dc4aeb4d29830f13a7465644bfb5a47f6ea3eaa86e4d4a57fe41e7b2593bf5094b76a551c4c71625bb
This value doesn't affect the outcome of the test, because the values are
properly set on line 351, but this makes the test values internally coherent.
A bug in the initial implementation of loadwallet meant that if the
arguement was a directory that didn't contain a wallet.dat file, a new
wallet would be created in that directory. Fix that so that if a
directory is passed in, it must contain a wallet.dat file.
Bug reported by promag (João Barbosa).
Signatures using segregated witness commit to the amount being spent,
so that value must be passed into signrawtransactionwithkey and
signrawtransactionwithwallet. This ensures an error is issued if that
doesn't happen, rather than just assuming the value is 0 and producing
a signature that is almost certainly invalid.
9fdf05d70c tests: Fix lock-order-inversion (potential deadlock) in DoS_tests. Reported by TSAN. (practicalswift)
Pull request description:
Fix lock-order-inversion (potential deadlock) in `DoS_tests`. Reported by Clang's TSAN.
Makes `src/test/test_bitcoin` pass also when compiled with TreadSanitizer (`./configure --with-sanitizers=thread` with `clang`).
Tree-SHA512: 41403bb7b6e26bdf1b830b5699e27c637d522bae1799d2a19ed4b68b21b2555438b42170d8b1189613beb32a69b76a65175d29a83f5f4e493896c3d0d94ae26d
9f8c54b1b5 Log warning message when deprecated network name 'tor' is used (e.g. option onlynet=tor) (wodry)
Pull request description:
As @laanwj mentioned [here](https://github.com/bitcoin/bitcoin/pull/13418#discussion_r197645385), using option `onlynet=tor` is deprecated.
I think it would be good to give the user a depcreaction warning feedback, so users can switch to `onlynet=onion` so there is a perspective for removing the deprecated `tor` in the future to decrease confusion.
Currently, users maybe just wonder that they can use a undocumented option, or they are not aware that they use a deprecated option.
Alternatively for the log warning message, I think at least this deprecetaion should be documented in the source code in a comment for readers of the source code.
Tree-SHA512: f4889793cdd62a0a13353e13994ed50ca7d367fa9da9897ce909f86cf0b0ce6151b3c484c8e514b8ac332949c6bbc71001e06e918248a1089f73756bd4840602
df10f07db1 [wallet] Don't use accounts when checking balance in sendmany (John Newbery)
e209184101 [wallet] deprecate sendfrom RPC method. (John Newbery)
Pull request description:
A couple of fixups from the accounts API deprecation PR (#12953):
- properly deprecate `sendfrom`
- don't use accounts when calculating balance in `sendmany` (unless the `-deprecatedrpc=accounts` flag is being used)
Tree-SHA512: 1befde055067438c4c3391bbff1aaed0e6249efd708c567db3f1faad40a0f28e64f95e5bad0679ae826d24a0239e4bc8a1c392dc93e2e7502343a7f6b1d1845c
faca0a8625 doc: Clarify that mempool txiter is const_iterator (MarcoFalke)
Pull request description:
`iterator` and `const_iterator` are the same type for multi indexed transaction sets, but `const_iterator` should be preferred for documentation purposes.
Tree-SHA512: 83e8af36d15aa1e9fc59b3c2279504fd6f6ea3188dc43e36dec279ee0613ff07947d7143fd112bade7868b0dba59ecab3fd246cbde82e376ef965b646d9f8c4d
3f72d04e29 Fix parameter count check for importpubkey. (Kristaps Kaupe)
Pull request description:
Found this while working on #13464. Parameter count check for `importpubkey` was wrong.
Tree-SHA512: aba41b666c6493379f320be5e3e438a6cad1a96429102ff4428c092c48f29c2eead2195792c0b018296f20e1c42eb091dd5b9886c42cecbb1f0d03d5def14705
faa2cf685a [qt] coincontrol: Remove unused qt4 workaround (MarcoFalke)
Pull request description:
This reverts 55eade9d46 since it is no longer required.
Tree-SHA512: ec523d505b410ab72ce9fdee86dfcfe96011472fb386744bb585169724270426ee65da2b527ae47928d604e1f21f54aa2b4b82f9a9d3fbfea1a6516478d81d11
bb3de15ad8 qt: Move BitcoinGUI initializers to class, fix initializer order warning (Wladimir J. van der Laan)
Pull request description:
- C++11-ize the code (move initializers to class, change `0` to `nullptr` where appropriate)
- Make sure `m_wallet_selector` is initialized
- And fix the following warning:
bitcoin/src/qt/bitcoingui.cpp:122:5: warning: field 'spinnerFrame' will be initialized after field 'm_wallet_selector_label' [-Wreorder]
spinnerFrame(0),
Tree-SHA512: b81c8d4ac31b712c8dfaf941ba43b235eb466eb5528535d69d68c26d8706d2a658581513a413050e5dee08b72a4e7fc08bd8936ef5beb052059d2467eaeff84b
- C++11-ize the code (move initializers to class, change 0 to `nullptr` where appropriate)
- Make sure `m_wallet_selector` is initialized
- And fix the following warning:
bitcoin/src/qt/bitcoingui.cpp:122:5⚠️ field 'spinnerFrame' will be initialized after field 'm_wallet_selector_label' [-Wreorder]
spinnerFrame(0),
Most includers just wanted to react to pending shutdown.
This isolates access to `fRequestShutdown` and limits access to the shutdown
api functions, including the new `AbortShutdown` for setting it to `false`.
Note I originally called `AbortShutdown` `CancelShutdown` but that name was
already taken by winuser.h
https://travis-ci.org/bitcoin/bitcoin/jobs/386913329
This change also triggered a build error in bench. Fixing it required moving LIBBITCOIN_SERVER after LIBBITCOIN_WALLET in bench_bench_bitcoin_LDADD To make
server definitions in src/net.cpp available to wallet methods in
src/wallet/wallet.cpp. Specifically, solving:
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `CWalletTx::RelayWalletTransaction(CConnman*)':
wallet.cpp:(.text+0x3f0e): undefined reference to `CConnman::NodeFullyConnected(CNode const*)'
collect2: error: ld returned 1 exit status
https://travis-ci.org/bitcoin/bitcoin/jobs/392133581
Need for remaining init.h includes confirmed via a thorough search with a more
specific regex:
\bInterrupt\(\)|\bShutdown\(\)|\bInitLogging\(\)|\bInitParameterInteraction\(\)|\bAppInitBasicSetup\(\)|\bAppInitParameterInteraction\(\)|\bAppInitSanityChecks\(\)|\bAppInitLockDataDirectory\(\)|\bAppInitMain\(\)|\bSetupServerArgs\(\)|\bLicenseInfo\(\)|g_wallet_init_interface|init.h
These were entirely unused, as based on successful compilation
and a grep for:
\bStartShutdown\(\)|\bShutdownRequested\(\)|\bInterrupt\(\)|\bShutdown\(\)|\bInitLogging\(\)|\bInitParameterInteraction\(\)|\bAppInitBasicSetup\(\)|\bAppInitParameterInteraction\(\)|\bAppInitSanityChecks\(\)|\bAppInitLockDataDirectory\(\)|\bAppInitMain\(\)|\bSetupServerArgs\(\)|\bLicenseInfo\(\)|g_wallet_init_interface|init.h
Fix a build error introduced in #13219.
```
.../bitcoin/src/bench/block_assemble.cpp:42:13:error: use of undeclared identifier 'CheckProofOfWork'
while (!CheckProofOfWork(block->GetHash(), block->nBits, Params().GetConsensus())) {
```
471a4992d4 Move rpc/util.cpp from libbitcoin-util to libbitcoin-server (Chun Kuan Lee)
Pull request description:
The functions in `rpc/util.cpp` would call functions in `script/standard.cpp` which in libbitcoin-common. This could cause problem if the linker does not strip out unused function while linking `bitcoin-cli`.
Tree-SHA512: 2f8335c880eeb00a29a359d5398a93d9f2909094b8febf2ad0a1e01388d077634fb5e72a638671bae8de89e1936c234d3f47ff445f1e456de723389bdc22d089
d92204c900 build: add warning to detect hidden copies in range-for loops (Cory Fields)
466e16e0e8 cleanup: avoid hidden copies in range-for loops (Cory Fields)
Pull request description:
Following-up on #13241, which was itself a follow-up of #12169.
See title. Fixing these would otherwise be a continuous process, adding the warning should keep them from cropping up.
Note that the warning seems to be Clang-only for now.
Tree-SHA512: ccfb769c3128b3f92c95715abcf21ee2496fe2aa384f80efead1529a28eeb56b98995b531b49a089f8142601389e63f7bb935963d724eacde4f5e1b4a024934b
af6ac3b677 doc: Remove mention of Qt4 from build docs (Wladimir J. van der Laan)
462c71f71b test: Update travis to not test Qt4 anymore (Wladimir J. van der Laan)
907f73bbc5 gui: Remove QT_VERSION fallbacks for Qt < 5 (Wladimir J. van der Laan)
bad068ad9f build: Build system changes to support only Qt5 (Wladimir J. van der Laan)
Pull request description:
Implements #8263.
Qt4.x has been EOL since 2015, and at least Gentoo has, or is going to drop support for it. I wouldn't be surprised if other Linux distributions follow.
This removes Qt4 detection from the build system, as well as removes all Qt4 fallbacks from the code. Turns out there's more than I expected: this is going to make maintenance of the GUI code, as well as adding new features significantly easier.
(I know there's still some references left to qt4 in RPM and Debian build script, but I don't have the knowledge how to fix them)
Tree-SHA512: d495924fd4dda6f6566ba44ee96be7cbe62e69ba1ca993b80a8449f78da852b7f1bd3e8200d57cfa1d72233c340eeff4596fb0032ecbddc715d99aea63817d3f
32d153fa36 For AVX2 code, also check for AVX, XSAVE, and OS support (Pieter Wuille)
Pull request description:
Fixes#12903.
Tree-SHA512: 01e71efb5d3a43c49a145a5b1dc4fe7d0a491e1e78479e7df830a2aaac57c3dcfc316e28984c695206c76f93b68e4350fc037ca36756ca579b7070e39c835da2
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
f74894480 Only set fNewBlock to true in AcceptBlock when we write to disk (Matt Corallo)
fa6e49731 rpc: Avoid "duplicate" return value for invalid submitblock (MarcoFalke)
Pull request description:
This is #13395 with one more commit tacked on. MarcoFalke got tired of dealing with the stupidity of fixing a return code with too many rounds of review (not that I blame him). Honestly we should probably have no return whatsoever, but for now, this fixes it (as well as nLastBlockTime for eviction purposes).
Original description:
When `submitblock` of an invalid block, the return value should not be `"duplicate"`.
This is only seen when the header was previously found (denoted by the incorrectly named boolean `fBlockPresent`). Fix this bug by removing `fBlockPresent`.
Tree-SHA512: 0ce3092655d5d904b4c8c5ff7479f73ce387144a738f20472b8af132564005c6db5594ae366e589508f6258506ee7a28b1c7995a83a8328b334f99316006bf2d
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.
faa18ca046 wallet: Erase wtxOrderd wtx pointer on removeprunedfunds (MarcoFalke)
Pull request description:
This prevents segfaults, when reading from the freed memory.
Tree-SHA512: 04f8190dea7901cf1cc298d5db98c83b02858f27114c5ef4da738accd176d6647d6b81f3dc39f3d5912b1a981cf0599370fd391c4154ffbde97afc1fac389123
fafa270328 Make ReceivedBlockTransactions return void (MarcoFalke)
Pull request description:
Instead of always returning `bool{true}` and forcing the caller to handle the return code, make it void and remove "a bunch" of dead code at the call sites.
Tree-SHA512: 10e41461c0516c0441d8b8eedcf6385874355c224b9e9d65e89addb142b4cf3e3be2d4ca0a7f2bd95c76aecdaa8537b6bd2d25631bf804bc42863ad5e84fa271
1e1eb6367f Improve coverage of SHA256 SelfTest code (Pieter Wuille)
Pull request description:
The existing SelfTest code does not cover the specialized double-SHA256-for-64-byte-inputs transforms added in #13191. Fix this.
Tree-SHA512: 593c7ee5dc9e77fc4c89e0a7753a63529b0d3d32ddbc015ae3895b52be77bee8a80bf16b754b30a22c01625a68db83fb77fa945a543143542bebb5b0f017ec5b
55771b7c6a Removed unused == operator from CMutableTransaction. (lucash.dev@gmail.com)
Pull request description:
This removes the unused == operator from `CMutableTransaction`.
The motivation is that unused code has a cost but offers no benefit (in general), while also adding the risk of introducing silent bugs. On top of that this particular code is quite inefficient, unnecessarily calculating the hash (it could, say, compare serializations). So if anyone ever needs to use a == comparison on `CMutableTransaction`, they'd be better of having to reimplement it (and add tests) than relying on code that's not being maintained.
Note: after this, trying to use the == operator on CMutableTransactions results in a compilation error:
```
./primitives/transaction.h:405:15: error: invalid operands to binary expression ('CMutableTransaction' and
'CMutableTransaction')
```
Tree-SHA512: a565af563e09d99347b6fe419f6d48c750b1377295af293a3e0c3c0d815e58aede8d7058987a68d66cfa7ed023e5d3285b12afabd17d0ff9cf11322ba3ce20fe
47776a958b Add linter: Make sure all shell scripts opt out of locale dependence using "export LC_ALL=C" (practicalswift)
3352da8da1 Add "export LC_ALL=C" to all shell scripts (practicalswift)
Pull request description:
~~Make sure `LC_ALL=C` is set when using `grep` range expressions.~~
Make sure `LC_ALL=C` is set in all shell scripts.
From the `grep(1)` documentation:
> Within a bracket expression, a range expression consists of two characters separated by a hyphen. It matches any single character that sorts between the two characters, inclusive, using the locale's collating sequence and character set. For example, in the default C locale, `[a-d]` is equivalent to `[abcd]`. Many locales sort characters in dictionary order, and in these locales `[a-d]` is typically not equivalent to `[abcd]`; it might be equivalent to `[aBbCcDd]`, for example. To obtain the traditional interpretation of bracket expressions, you can use the C locale by setting the `LC_ALL` environment variable to the value C.
Context: [Locale issue found when reviewing #13450](https://github.com/bitcoin/bitcoin/pull/13450/files#r194877736)
Tree-SHA512: fd74d2612998f9b49ef9be24410e505d8c842716f84d085157fc7f9799d40e8a7b4969de783afcf99b7fae4f91bbb4559651f7dd6578a6a081a50bdea29f0909
9b72c988a0 scripted-diff: Avoid temporary copies when looping over std::map (Ben Woosley)
Pull request description:
The ::value_type of the std::map/std::multimap/std::unordered_map containers is
std::pair<const Key, T>. Dropping the const results in an unnecessary copy,
for example in C++11 range-based loops.
For this I started with a more general scripted diff, then narrowed it down
based on the inspection showing that all actual map/multimap/unordered_map
variables used in loops start with m or have map in the name.
Tree-SHA512: b656d66b69ffa1eb954124aa8ae2bc5436ca50262abefa93bdda55cfcdaffc5ff90cd40539051a2bd06355ba69ddf245265cc8764eebff66d761b3aec06155a9
25bc9615b7 Document validationinterace callback blocking deadlock potential. (Matt Corallo)
Pull request description:
From the branches-I've-had-lying-around-and-forgot-to-PR department...
This is a comment-only PR, but the comments point out an API quirk that isn't exactly trivial. None of our use-cases right now hit this, but if we were to call SyncWithValidationInterfaceQueue (eg to limit queue depth) in ATMP, I'm pretty sure we'd hit a deadlock there.
Tree-SHA512: 889dd8fc9eb15d1f2aa5ca467e783bc8f07bc543b166b032741795b0db7a0df11a2846d3cb7c69bafa8d1acf970021001b742f52be06725a932813230c5b4a7b
86edf4a2a5 expose CBlockIndex::nTx in getblock(header) (Gregory Sanders)
Pull request description:
Recent publication of a weakness in Bitcoin's merkle tree construction demonstrates many SPV applications vulnerable to an expensive to pull off yet still plausible attack: https://bitslog.wordpress.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/
Including the coinbase in the txoutproof seems the most effective fix, however results in a significant efficiency downgrade. Transactors will not even know a priori what the size of their proof will be within a couple orders of magnitude, unless they use the mid-state of SHA2 as detailed in the blog post.
Some applications, like Elements blockchain platform that take SPV-style proofs have optional access to a bitcoind to verify these proofs of inclusion and check depth in the chain. Returning `CBlockIndex::nTx` would allow an extremely easy and compact way of checking the depth of the tree, with no additional overhead to the codebase, and works with pruned nodes.
`getblockheader` is arguably not the place for it, but as mentioned before, is a natural workflow for us checking depth of a block in a possibly pruned node.
We should also ensure that `verifytxoutproof` ends up validating this depth fact as well, but left this for another PR.
Tree-SHA512: af4cf48e704c6088f8da06a477fda1aaa6f8770cee9b876c4465d1075966d6a95831a88817673fe5a0d6bbcdc1ffcbc1892e2be0d838c60fc6958d33eacdcc14
c2dfbb4a97 Add unavailable options to hidden options category (Andrew Chow)
Pull request description:
From IRC:
```
<ossifrage> FYI, bitcoin-qt from the head I built today won't start if you have "daemon=0" in the config file, so you can't use the same config for either bitcoind or bitcoin-qt
<ossifrage> Seems like bitcoin-qt should ignore this option?
<provoostenator> ossifrage: probably caused by 13112. Another problem is disablewallet=1 will prevent a launch if you compile bitcoind without wallet. It probably needs to be relaxed slightly.
```
Adds all of the options that are unavailable due to compiling options to the hidden category so that shared config files do not break with the alternative binaries.
Tree-SHA512: 1ef43f5f7ad46ecc2865d22ee683ef22831e8f131ec99b732bb36d90381f7964bf64829595e993c2d435823fe4425a20323c8e65307cf2463a9e40b8049ab559
faf52f953b tests: Drop variadic macro (MarcoFalke)
Pull request description:
The C++11 constructor of `std::vector` that takes an initializer list, is not `explicit`. Thus, the macro is not required and can be dropped.
Hopefully fixes#13456
Tree-SHA512: 4095ed205f88138a7cd5b14790cc426899966f622a924a9b3f7de646a0d801a48ffb8921da760f1f93d5481298477c8a64dbec291381bb9aa77b075bdd2659f2
f6f8026e40 validation: check the specified number of blocks (off-by-one) (Karl-Johan Alm)
Pull request description:
```
echeveria | 2018-06-11 02:03:03.384975 Verifying last 3 blocks at level 3
echeveria | 2018-06-11 02:03:23.676793 No coin database inconsistencies in last 4 blocks (6564 transactions)
echeveria | off by one?
sipa | echeveria: possibly!
kallewoof | Looks like it checks one more block than suggested. `if (pindex->nHeight < chainActive.Height()-nCheckDepth) break;` should probably be `<=`.
sipa | kallewoof: agree
```
Post-commit:
```
2018-06-11T05:24:02Z Verifying last 6 blocks at level 3
2018-06-11T05:24:02Z [0%]...[16%]...[33%]...[50%]...[66%]...[83%]...[99%]...[DONE].
2018-06-11T05:25:07Z No coin database inconsistencies in last 6 blocks (7258 transactions)
```
Pre-commit:
```
2018-06-11T05:27:11Z Verifying last 6 blocks at level 3
2018-06-11T05:27:11Z [0%]...[16%]...[33%]...[50%]...[66%]...[83%]...[99%]...[DONE].
2018-06-11T05:27:12Z No coin database inconsistencies in last 7 blocks (9832 transactions)
```
Tree-SHA512: 6e68dc4ba74232518c2ba8ea624d65893534f3619d43ccdf0b9c65992f25b68cb52cf54fa35e6e3d092d1eee5c9a8887057828895f1acdafc0ebb48f683fffdc
Options that are not available (but known in the source code) will
cause an error if they are specified.
Make these options "available" by adding them to the hidden options
category to prevent conf files from failing when shared between binaries
that have different options available.
57ba401abc Enable double-SHA256-for-64-byte code on 32-bit x86 (Pieter Wuille)
Pull request description:
The SSE4 and AVX2 double-SHA256-for-64-byte input code from #13191 compiles fine on 32-bit x86 systems, but the autodetection logic in sha256.cpp doesn't enable it. Fix this.
Note that these instruction sets are only available on CPUs that support 64-bit mode as well, so it is only beneficial in the (perhaps unlikely) scenario where a 64-bit CPU is running a 32-bit Bitcoin Core binary.
Tree-SHA512: 39d5963c1ba8c33932549d5fe98bd184932689a40aeba95043eca31dd6824f566197c546b60905555eccaf407408a5f0f200247bb0907450d309b0a70b245102
fa7a6cf1b3 policy: Treat segwit as always active (MarcoFalke)
Pull request description:
Now that segwit is active for a long time, there is no need to reject transactions with the reason that segwit hasn't activated.
Strictly speaking, this is a bug fix, because with the release of 0.16, we create segwit transactions in our wallet by default without checking if they are allowed by local policy.
More broadly, this simplifies the code as if "premature witness" was always set to true with the corresponding command line args.
Tree-SHA512: 484c26aa3a66faba6b41e8554a91a29bfc15fbf6caae3d5363a3966283143189c4bd5333a610b0669c1238f75620691264e73f6b9f1161cdacf7574d946436da
e56771365b Do not use uppercase characters in source code filenames (practicalswift)
419a1983ca docs: Add a note about the source code filename naming convention (practicalswift)
Pull request description:
Add a note about the source code filename naming convention.
Tree-SHA512: 8d329bd9e19bcd26e74b0862fb0bc2369b46095dbd3e69d34859908632763abd7c3d00ccc44ee059772ad4bae4460c2bcc1c0e22fd9d8876d57e5fcd346cea4b
The only affect this should have is fixing the return code in
submitblock in cases where a block fails ContextualCheckBlock and
not setting nLastBlockTime on peers that provide blocks which fail
ContextualCheckBlock (which is only used in eviction and cosmetic).
The ::value_type of the std::map/std::multimap/std::unordered_map containers is
std::pair<const Key, T>. Dropping the const results in an unnecessary copy,
for example in C++11 range-based loops.
For this I started with a more general scripted diff, then narrowed it down
based on the inspection showing that all actual map/multimap/unordered_map
variables used in loops start with m or have map in the name.
-BEGIN VERIFY SCRIPT-
sed -i -E 's/for \(([^<]*)std::pair<([^c])(.+) : m/for (\1std::pair<const \2\3 : m/' src/*.cpp src/**/*.cpp
sed -i -E 's/for \(([^<]*)std::pair<([^c])(.+) : (.*)map/for (\1std::pair<const \2\3 : \4map/' src/*.cpp src/**/*.cpp
-END VERIFY SCRIPT-
16e3cd380a Clarify include recommendation (practicalswift)
6d10f43738 Enforce the use of bracket syntax includes ("#include <foo.h>") (practicalswift)
906bee8e5f Use bracket syntax includes ("#include <foo.h>") (practicalswift)
Pull request description:
When analysing includes in the project it is often assumed that the preferred bracket include syntax (`#include <foo.h>`) mentioned in `developer-docs.md` is used consistently. @sipa:s excellent circular dependencies script [`circular-dependencies.py`](50c69b7801/contrib/devtools/circular-dependencies.py) (#13228) is an example of a script making this reasonable assumption.
This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (`#include <foo.h>`) is used consistently.
Tree-SHA512: a414921aabe8e487ebed42f3f1cbd02fecd1add385065c1f2244cd602c31889e61fea5a801507ec501ef9bd309b05d3c999f915cec1c2b44f085bb0d2835c182
f77e1d34fd test: Add MempoolAncestryTests (Karl-Johan Alm)
a08d76bcfe mempool: Calculate descendant maximum thoroughly (Karl-Johan Alm)
6d3568371e wallet: Switch to using ancestor/descendant limits (Karl-Johan Alm)
6888195b06 wallet: Strictly greater than for ancestor caps (Karl-Johan Alm)
322b12ac4e Remove deprecated TransactionWithinChainLimit (Karl-Johan Alm)
4784751547 Switch to GetTransactionAncestry() in OutputEligibleForSpending (Karl-Johan Alm)
475a385a80 Add GetTransactionAncestry to CTxMemPool for general purpose chain limit checking (Karl-Johan Alm)
46847d69d2 mempool: Fix max descendants check (Karl-Johan Alm)
b9ef21dd72 mempool: Add explicit max_descendants (Karl-Johan Alm)
Pull request description:
Currently, `TransactionWithinChainLimit` is restricted to single-output use, and needs to be called every time for different limits. If it is replaced with a chain limit value calculator, that can be called once and reused, and is generally more flexible (see e.g. #12257).
Update: this PR now corrects usage of max ancestors / max descendants, including calculating the correct max descendant value, as advertised for the two limits.
~~This change also makes `nMaxAncestors` signed, as the replacement method will return `-1` for "not in the mempool", which is different from "0", which means "no ancestors/descendants in mempool".~~
~~This is a subset of #12257.~~
Tree-SHA512: aa59c849360542362b3126c0e29d44d3d58f11898e277d38c034dc4b86a5b4500f77ac61767599ce878c876b5c446fec9c02699797eb2fa41e530ec863a00cf9
67e0e04140 [wallet] [docs] Update release notes for removing `getlabeladdress` (John Newbery)
81608178cf [wallet] [rpc] Remove getlabeladdress RPC (John Newbery)
Pull request description:
labels are associated with addresses (rather than addresses being
associated with labels, as was the case with accounts). The
getlabeladdress does not make sense in this model, so remove it.
getaccountaddress is still supported for one release as the accounts
API is deprecated.
Tree-SHA512: 7f45d0456248ebcc4e54dd34e2578a09a8ea8e4fceda75238ccea9d731dc99a3f3c0519b18a9739de17d2e6e59c9c2259ba67c9ae2e3cb2a40ddb14b9193fe29
f68049dd87 crypto: cleanup sha256 build (Cory Fields)
Pull request description:
Requested by @sipa in #13386.
Rather than appending all possible cpu variants to all targets, create a convenience variable that encompasses all.
Tree-SHA512: 8e9ab2185515672b79bb7925afa4f3fbfe921bfcbe61456833d15457de4feba95290de17514344ce42ee81cc38b252476cd0c29432ac48c737c2225ed515a4bd
cbede7dbfd [qt] OptionsDialog: add prune setting (Sjors Provoost)
Pull request description:
The default suggested value is 2 GB. Minimum is 1 GB (550 MB rounded up).
When the user toggles this setting, a strong warning appears that undoing requires re-downloading the chain:
<img width="478" alt="schermafbeelding 2018-05-15 om 12 35 24" src="https://user-images.githubusercontent.com/10217/40051858-7939cc20-583c-11e8-9120-327a75376732.png">
Tooltip points out that actual disk usage can be higher. It's a bit vague on the "advanced features", because I'm assuming anyone who needs to use `-rescan` and `-txindex` will read the documentation, and a more detailed text would needlessly confuse everyone else.
<img width="450" alt="schermafbeelding 2018-05-15 om 12 33 51" src="https://user-images.githubusercontent.com/10217/40051791-49d6156a-583c-11e8-97b9-7de6dfd8c481.png">
The UI uses gigabytes for readability and easy of use. There is also no manual pruning UI (`prune=1`). The user will have to use `bitcoin.conf` for those things.
Fixes#6461. When combined with #13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit `bitcoin.conf`. However I don't think that's an essential prerequisite.
Tree-SHA512: e17aff276d7235fbd40796adb6431d430620788a753ee13bc064abd35d2edc4280a3d3cddc18e42b4e00edff13ed18fd4f2a966c6f0b43b689afd13673e0c4bf
Instead of combining the -limitancestorcount and -limitdescendantcount into a nMaxChainLength, this commit uses each one separately in the coin eligibility filters.
The chain limits check for max descendants would check the descendants of the transaction itself even though the description for -limitdescendantcount says 'any ancestor'. This commit corrects the descendant count check by finding the top parent transaction in the mempool and comparing against that.
TransactionWithinChainLimits would take a 'limit' and check it against ascendants and descendants. This is changed to take an explicit
max ancestors and max descendants value, and to test the corresponding value against its corresponding max.
e9a1881b90 refactor: add a function for determining if a block is pruned or not (Karl-Johan Alm)
Pull request description:
The check for whether a block is pruned or not is sufficiently obscure that it deserves a macro. It is also used in 2 places, ~~with more coming, e.g. #10757~~ (turns out it was a move, not an addition).
Tree-SHA512: b9aeb60663e1d1196df5371d5aa00b32ff5d4cdea6a77e4b566f28115cce09570c18e45e4b81a4033f67c4135c8e32c027f67bae3b75c2ea4564285578a3f4dd
2acd1d6716 Drop uint 256 not operator (Ben Woosley)
Pull request description:
All the other operators are integer or bitwise operations, and this is unused
apart from tests.
Note attempting to call `!` on `arith_uint256` results in a build error after this change:
```
test/arith_uint256_tests.cpp:201:17: error: invalid argument type 'const arith_uint256' to unary expression
BOOST_CHECK(!ZeroL);
```
Tree-SHA512: 5791b643f426dac9829e9499d678786f1ad294edb2d840879252a1b642bda55941632114f64048660a5991a984aeba49eeb5dfe64ba0a6275cbe7b1c049d7095
ec3073a274 index: Move index DBs into index/ directory. (Jim Posen)
89eddcd365 index: Remove TxIndexDB from public interface of TxIndex. (Jim Posen)
2318affd27 MOVEONLY: Move BaseIndex to its own file. (Jim Posen)
f376a49241 index: Generalize logged statements in BaseIndex. (Jim Posen)
61a1226d87 index: Extract logic from TxIndex into reusable base class. (Jim Posen)
e5af5fc6fb db: Make reusable base class for index databases. (Jim Posen)
9b0ec1a7f9 db: Remove obsolete methods from CBlockTreeDB. (Jim Posen)
Pull request description:
This refactors most of the logic in TxIndex into a reusable base class for other indices. There are two commits moving code between files, which may be be more easily reviewed using `git diff --color-moved` (https://blog.github.com/2018-04-05-git-217-released/).
The motivation for this is to support BIP 157 by indexing block filters.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/bitcoin/bitcoin/13243)
<!-- Reviewable:end -->
Tree-SHA512: 0857f04df2aa920178dab2eb8e57984d8eb4d5010deca9971190358479e05b6672ccca2a08af0a7ac9fe02afb947be84cf35a3693204d0667263c6add2959cbf
ebebedce20 speed up of tx_validationcache_tests by reusing of CTransaction. (lucash.dev@gmail.com)
Pull request description:
The code was converting CMutableTransaction to CTransaction multiple times, which implies recalculating the hash multiple times. This commit fixes this by reusing a single CTransaction.
Run-time results:
```
Before: 6.7s
After: 5.5s
--------------
Saved: 1.2s
```
This PR was split from #13050. Also, see #10026.
Tree-SHA512: 61fb81972a08299085a7d3d0060485b265aefc7a4f82ab548e5f94371c8643cfb97bf0ef34f4e1211bf853d0217fa1c3338e4117f36fda1b37d203f690e86d60
0231ef6c6d cli: Ignore libevent warnings (Cory Fields)
Pull request description:
Should fix rpc tests that fail due to an unclean stderr.
Untested as I'm not seeing these warnings. @promag mind seeing if this fixes your problem?
Tree-SHA512: fba5ae3f239b515e93e19f9c3eca659eb7fb21f1b1fec25b68285695bfd1ecbdcd9b2235543689aaf97bff85cbb762840f65365a67e791314e9a6b8db2c9e246
The code was converting CMutableTransaction to CTransaction multiple times, which implies recalculating the hash multiple times. This commit fixes this by reusing a single CTransaction.
ebec7317ca Drop the chain argument to GetDifficulty (Ben Woosley)
Pull request description:
By dropping the chain argument to `GetDifficulty`. `GetDifficulty` was called in two ways:
* with a guaranteed non-null blockindex
* with no argument
Change the latter case to be provided `chainActive.Tip()` explicitly.
Introduced in: #11748
Tree-SHA512: f2c97014be185f3e3de92db15848548650e4a67fab20a41bcfa851c5c63c245915cbe9380f84d9da2081e8756d31a41de417db1d35cfecf41ddb4f25070eb525
fa4760fbb3 qa: Increase includeconf test coverage (MarcoFalke)
Pull request description:
This adds some missing `return false` for error conditions and adds test coverage [1] for those.
Also, extend recursion warning when the chain was set in one of the includeconfs.
[1] See the red lines in https://marcofalke.github.io/btc_cov/total.coverage/src/util.cpp.gcov.html for missing coverage.
Tree-SHA512: d32563c9bb277879895a173e699034db5ecdb4061a1ec8890c566d61e36a09efa5eda19a029baf952ff6d568f8b9684a13a0bb90827850075470975e2088fee4
6aa33feadb Drop UpdateTransaction in favor of UpdateInput (Ben Woosley)
Pull request description:
Updating the input explicitly requires the caller to present a mutable
input, which more clearly communicates the effects and intent of the call
(and, often, the enclosing loop).
In most cases, this input is already immediately available and need not be
looked up.
Tree-SHA512: 8c7914a8b7ae975d8ad0e9d760e3c5da65776a5f79d060b8ffb6b3ff7a32235f71ad705f2185b368d9263742d7796bb562395d22b806d90e8502d8c496011e57
fa36aa7965 wallet: Prevent segfault when sending to unspendable witness (MarcoFalke)
Pull request description:
Previously we wouldn't care about the `txnouttype`, but after 4e91820531 we `switch` on the type.
Tree-SHA512: 6b597aba80cb43881671ad7b3a4ad97753864e8005a05c23fdd8ee79953483c08f241b5c392a9b494298eadc5cfba895b0480d916ef4f11d122fd6196f31b84a
4defdfab94 [MOVEONLY] Move unused Merkle branch code to tests (Pieter Wuille)
4437d6e1f3 8-way AVX2 implementation for double SHA256 on 64-byte inputs (Pieter Wuille)
230294bf5f 4-way SSE4.1 implementation for double SHA256 on 64-byte inputs (Pieter Wuille)
1f0e7ca09c Use SHA256D64 in Merkle root computation (Pieter Wuille)
d0c9632883 Specialized double sha256 for 64 byte inputs (Pieter Wuille)
57f34630fb Refactor SHA256 code (Pieter Wuille)
0df017889b Benchmark Merkle root computation (Pieter Wuille)
Pull request description:
This introduces a framework for specialized double-SHA256 with 64 byte inputs. 4 different implementations are provided:
* Generic C++ (reusing the normal SHA256 code)
* Specialized C++ for 64-byte inputs, but no special instructions
* 4-way using SSE4.1 intrinsics
* 8-way using AVX2 intrinsics
On my own system (AVX2 capable), I get these benchmarks for computing the Merkle root of 9001 leaves (supported lengths / special instructions / parallellism):
* 7.2 ms with varsize/naive/1way (master, non-SSE4 hardware)
* 5.8 ms with size64/naive/1way (this PR, non-SSE4 capable systems)
* 4.8 ms with varsize/SSE4/1way (master, SSE4 hardware)
* 2.9 ms with size64/SSE4/4way (this PR, SSE4 hardware)
* 1.1 ms with size64/AVX2/8way (this PR, AVX2 hardware)
Tree-SHA512: efa32d48b32820d9ce788ead4eb583949265be8c2e5f538c94bc914e92d131a57f8c1ee26c6f998e81fb0e30675d4e2eddc3360bcf632676249036018cff343e
db56755ca4 Fix "gmake check" under OpenBSD 6.3 (probably *BSD): Avoid using GNU grep specific regexp handling (practicalswift)
Pull request description:
Fixes#13337 (!)
GNU grep and BSD grep differs in the way they handle regexps when extended regular expressions are not enabled via the `-E` flag:
```
$ grep --version | head -1
grep (GNU grep) 3.1
$ echo "BOOST_FIXTURE_TEST_SUITE(foo)" | grep "BOOST_FIXTURE_TEST_SUITE(\|BOOST_AUTO_TEST_SUITE("
BOOST_FIXTURE_TEST_SUITE(foo)
$
```
```
$ grep --version | head -1
grep version 0.9
$ echo "BOOST_FIXTURE_TEST_SUITE(foo)" | grep "BOOST_FIXTURE_TEST_SUITE(\|BOOST_AUTO_TEST_SUITE("
$
```
The portable way to do it is:
```
$ echo "BOOST_FIXTURE_TEST_SUITE(foo)" | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()"
BOOST_FIXTURE_TEST_SUITE(foo)
$
```
Tree-SHA512: d83c78f34421504dd8efc3921c98527f499045b702bd34715a5bc78e04ef2a5f49f601a55ad08632e870f137b1edada94a3f530291bc9107d8d6b16fe11e640b
493a166948 bench: Don't return a bool from main (Wladimir J. van der Laan)
Pull request description:
Return `1` from `main()` on error, not the bool `false` (introduced in #13112). This is the correct value to return on error, and also shuts up a clang warning.
Tree-SHA512: 52a0f1b2f6ae2697555f71ee2019ce657046f7f379f1f4faf3cce9d5f3fb21fcdc43a4c84895a2a8b6929997ba70bbe87c231f2f9553215b84c22333810d58d9
6b8b63af14 Generic TransactionSignatureCreator works with both CTransaction and CMutableTransaction (Martin Ankerl)
Pull request description:
Refactored `TransactionSignatureCreator` into a templated `GenericTransactionSignatureCreator` that works with both `CMutableTransaction` and `CTransaction`.
The advantage is that now in `SignSignature`, the `MutableTransactionSignatureCreator` can now operate directly with the `CMutableTransaction` without the need to copy the data into a `CTransaction`.
Running all unit tests brings a very noticable speedup on my machine:
48.4 sec before this change
36.4 sec with this change
--------
12.0 seconds saved
running only `--run_test=transaction_tests/test_big_witness_transaction`:
16.7 sec before this change
5.9 sec with this change
--------
10.8 seconds saved
This relates to my first attempt with the const_cast hack #13202, and to the slow unit test issue #10026.
Also see #13050 which modifies the tests but not the production code (like this PR) to get a speedup.
Tree-SHA512: 2cff0e9699f484f26120a40e431a24c8bc8f9e780fd89cb0ecf20c5be3eab6c43f9c359cde244abd9f3620d06c7c354e3b9dd3da41fa2ca1ac1e09386fea25fb
Return `EXIT_SUCCESS` from `main()` on error, not the bool `false`
(introduced in #13112). This is the correct value to return on error,
and also shuts up a clang warning.
Also add a final return for clarity.
903055730b Test gArgs erroring on unknown args (Andrew Chow)
4f8704d57f Give an error and exit if there are unknown parameters (Andrew Chow)
174f7c8080 Use a struct for arguments and nested map for categories (Andrew Chow)
Pull request description:
Following #13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist.
Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior.
Closes#1044
Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7
4b62bdf513 Wallet: Refactor ReserveKeyFromKeyPool for safety (Ben Woosley)
Pull request description:
ReserveKeyFromKeyPool's previous behaviour is to set nIndex to -1 if the keypool is
empty, OR throw an exception for technical failures. Instead, we now return false
if the keypool is empty, true if the operation succeeded.
This is to make failure more easily detectable by calling code.
Tree-SHA512: 753f057ad13bd4c28d121f426bf0967ed72b827d97fb24582f9326ec60072abc5482e3db69ccada7c5fc66de9957fc59098432dd223fc4116991cab44c6d7aef
If an unknown option is given via either the command line args or
the conf file, throw an error and exit
Update tests for ArgsManager knowing args
Ignore unknown options in the config file for bitcoin-cli
Fix tests and bitcoin-cli to match actual options used
Instead of a single map with the category and name as the key,
make m_available_args contain maps. The key will be the category and
the value is a map which actually contains the arguments for that
category. The nested map's key is the argument name, while the value
is a struct that contains the help text and whether the argument is
a debug only argument.
c814e2e7e8 Remove template matching and pseudo opcodes (Pieter Wuille)
Pull request description:
The current code contains a rather complex script template matching engine, which is only used for 3 particular script types (P2PK, P2PKH, multisig). The first two of these are trivial to match for otherwise, and a specialized matcher for multisig is both more compact and more efficient than a generic one.
The goal is being more flexible, so that for example larger standard multisigs inside SegWit outputs are easier to implement.
As a side-effect, it also gets rid of the pseudo opcodes hack.
Tree-SHA512: 643b409c5c36821519f613a43efd399af0ec99b6131f35cd4024decfb2d483d719e0e921cd088bc9832a7ac797cb4a6b1158b8574c82f7fbebb75f1b31b359df
Templated version so that no copying of CMutableTransaction into a CTransaction is
necessary. This speeds up the test case transaction_tests/test_big_witness_transaction
from 7.9 seconds to 3.1 seconds on my machine.
Many options are extremely technical, and refer internals, making it
difficult to translate usefully. This came up in discussion of e.g.
#10949. If a message is not understood by translators (which are
typically end-users, not developers) they'll either translate it
literally, making it harder to understand instead of easier, with the
added drawback of the user no longer being able to google it.
Also the translation was only working for bitcoin-qt as with
the console programs, there is no translation backend. So it was
injecting never-used translation messages for bitcoin-cli, -tx.
For these reasons, stop translating options help completely. This should
not affect the output **in any way** except for bitcoin-qt when a
non-English language is configured in the locale.
This implements #10962.
