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
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
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
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
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
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
97c112d4ca Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)
Pull request description:
These methods are standalone string parsing methods which were included
into test via an include of torcontrol.cpp, which is bad practice.
~~Splitting them out reveals that they were the only torcontrol.cpp
methods under test, so the test file is renamed tor_reply_tests.cpp.~~
Introduced in #10408
Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac
fac1223a56 Cache witness hash in CTransaction (MarcoFalke)
faab55fbb1 Make CMutableTransaction constructor explicit (MarcoFalke)
Pull request description:
This speeds up:
* compactblocks (v2)
* ATMP
* validation and miner (via `BlockWitnessMerkleRoot`)
* sigcache (see also unrelated #13204)
* rpc and rest (nice, but irrelevant)
This presumably slows down rescan, which uses a `CTransaction` and its `GetHash`, but never uses the `GetWitnessHash`. The slow down is proportional to the number of witness transactions in the rescan window. I.e. early in the chain there should be no measurable slow down. Later in the chain, there should be a slow down, but acceptable given the speedups in the modules mentioned above.
Tree-SHA512: 443e86acfcceb5af2163e68840c581d44159af3fd1fce266cab3504b29fcd74c50812b69a00d41582e7e1c5ea292f420ce5e892cdfab691da9c24ed1c44536c7
60ebc7da4c trivial: Mark overrides as such. (Daniel Kraft)
Pull request description:
This trivial change adds the `override` keyword to some methods that override virtual base class / interface methods. This ensures that any future changes to the interface's method signatures which are not correctly mirrored in the subclasses will break at compile time with a clear error message, rather than at runtime.
Tree-SHA512: cc1bfa5f03b5e29d20e3eab07b0b5fa2f77b47f79e08263dbff43e4f463e9dd8f4f537e2c8c9b6cb3663220dcf40cfd77723cd9fcbd623c9efc90a4cd44facfc
This removes the need to include rpc/blockchain.cpp in order to put
GetDifficulty under test. 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.
This trivial change adds the "override" keyword to some methods of
subclasses meant to override interface methods. This ensures that any
future change to the interface' method signatures which are not correctly
mirrored in the subclass will break at compile time with a clear error message,
rather than fail at runtime (which is harder to debug).
Updating the input explicitly requires the caller to present a mutable
input, which more clearly communicates the effects and intent of the method.
In most cases, this input is already immediately available and need not be
looked up.
dd435ad Add unit tests for signals generated by ProcessNewBlock() (Jesse Cohen)
a3ae8e6 Fix concurrency-related bugs in ActivateBestChain (Jesse Cohen)
ecc3c4a Do not unlock cs_main in ABC unless we've actually made progress. (Matt Corallo)
Pull request description:
Originally this PR was just to add tests around concurrency in block validation - those tests seem to have uncovered another bug in ActivateBestChain - this now fixes that bug and adds tests.
ActivateBestChain (invoked after a new block is validated) proceeds in steps - acquiring and releasing cs_main while incrementally disconnecting and connecting blocks to sync to the most work chain known (FindMostWorkChain()). Every time cs_main is released the result of FindMostWorkChain() can change - but currently that value is cached across acquisitions of cs_main and only refreshed when an invalid chain is explored. It needs to be refreshed every time cs_main is reacquired. The test added in 6094ce7304 will occasionally fail without the commit fixing this issue 26bfdbaddb
Original description below
--
After a bug discovered where UpdatedBlockTip() notifications could be triggered out of order (#12978), these unit tests check certain invariants about these signals.
The scheduler test asserts that a SingleThreadedSchedulerClient processes callbacks fully and sequentially.
The block validation test generates a random chain and calls ProcessNewBlock from multiple threads at random and in parallel. ValidationInterface callbacks verify that the ordering of BlockConnected BlockDisconnected and UpdatedBlockTip events occur as expected.
Tree-SHA512: 4102423a03d2ea28580c7a70add8a6bdb22ef9e33b107c3aadef80d5af02644cdfaae516c44933924717599c81701e0b96fbf9cf38696e9e41372401a5ee1f3c
After a recent bug discovered in callback ordering in MainSignals,
this test checks invariants in ordering of
BlockConnected / BlockDisconnected / UpdatedChainTip signals
60f61f9 Tighten up bech32::Decode(); add tests. (murrayn)
Pull request description:
Just a few minor optimizations to bech32::Decode():
1) optimize the order and logic of the conditionals
2) get rid of subsequent '(c < 33 || c > 126)' check which is redundant (already performed above)
3) add a couple more bech32 tests (mixed-case)
Tree-SHA512: e41af834c8f6b7d34c22c28b724df42c60f72e00df616e70a12efbc4271d15d80627fe1bc36845caf29f615c238499a566298a863cbe119fef457287231053c8
chain.h does not actually depend on the methods defined in pow.h, just its
include of consensus/params.h, which is standalone and can be included instead.
Confirmed by inspection and successful build.
159c32d1f1 Add assertion to guide static analyzers. Clang Static Analyzer needs this guidance. (practicalswift)
fd447a6efe Fix dead stores. Values were stored but never read. Limit scope. (practicalswift)
Pull request description:
Fix Clang Static Analyzer warnings reported by @kallewoof in #12961:
* Fix dead stores. Values were stored but never read.
* Add assertion to guide static analyzers. See #12961 for details.
Tree-SHA512: 83dbec821f45217637316bee978e7543f2d2caeb7f7b0b3aec107fede0fff8baa756da8f6b761ae0d38537740839ac9752f6689109c38a4b05c0c041aaa3a1fb
364bae5 qa: Pad scriptPubKeys to get minimum sized txs (MarcoFalke)
7485488 Policy to reject extremely small transactions (Johnson Lau)
0f8719b Add transaction tests for constant scriptCode (Johnson Lau)
9dabfe4 Add constant scriptCode policy in non-segwit scripts (Johnson Lau)
Pull request description:
This disables `OP_CODESEPARATOR` in non-segwit scripts (even in an unexecuted branch), and makes a positive `FindAndDelete` result invalid. This ensures that the `scriptCode` serialized in `SignatureHash` is always the same as the script passing to the `EvalScript`.
Tree-SHA512: a0552cb920294d130251c48053fa2ff1fbdd26332e62b52147d918837852750f0ce35ce2cd1cbdb86588943312f8154ccb4925e850dbb7c2254bc353070cd5f8
This disables OP_CODESEPARATOR in non-segwit scripts (even in an unexecuted branch), and makes a positive FindAndDelete result invalid. This ensures that the scriptCode serialized in SignatureHash() is always the same as the script passing to the EvalScript.
7d0f80b Use anonymous namespace instead of static functions (Pieter Wuille)
b61fb71 Mention removal of bare multisig IsMine in release notes (Pieter Wuille)
9c2a8b8 Do not treat bare multisig as IsMine (Pieter Wuille)
08f3228 Optimization: only test for witness scripts at top level (Pieter Wuille)
3619735 Track difference between scriptPubKey and P2SH execution in IsMine (Pieter Wuille)
ac6ec62 Switch to a private version of SigVersion inside IsMine (Pieter Wuille)
19fc973 Do not expose SigVersion argument to IsMine (Pieter Wuille)
fb1dfbb Remove unused IsMine overload (Pieter Wuille)
952d821 Make CScript -> CScriptID conversion explicit (Pieter Wuille)
Pull request description:
Currently our wallet code will treat bare multisig outputs (meaning scriptPubKeys with multiple public keys + `OP_CHECKMULTISIG` operator in it) as ours without the user asking for it, as long as all private keys in it are in our wallet.
This is a pointless feature. As it only works when all private keys are in one place, it's useless compared to single key outputs (P2PK, P2PKH, P2WPKH, P2SH-P2WPKH), and worse in terms of space, cost, UTXO size, and ability to test (due to lack of address format for them).
Furthermore, they are problematic in that producing a list of all `scriptPubKeys` we accept is not tractable (it involves all combinations of all public keys that are ours). In further wallet changes I'd like to move to a model where all scriptPubKeys that are treated as ours are explicit, rather than defined by whatever keys we have. The current behavior of the wallet is very hard to model in such a design, so I'd like to get rid of it.
I think there are two options:
* Remove it entirely (do not ever accept bare multisig outputs as ours, unless watched)
* Only accept bare multisig outputs in situations where the P2SH version of that output would also be acceptable
This PR implements the first option. The second option was explored in #12874.
Tree-SHA512: 917ed45b3cac864cee53e27f9a3e900390c576277fbd6751b1250becea04d692b3b426fa09065a3399931013bd579c4f3dbeeb29d51d19ed0c64da75d430ad9a
54a5a21 [MOVEONLY] Turn CScript::GetOp2 into a function and move to cpp (Pieter Wuille)
6a7456a [MOVEONLY] Move CSCript::FindAndDelete to interpreter (Pieter Wuille)
33a8ecf Delete unused non-const-iterator CSCript::GetOp overloads (Pieter Wuille)
2fb168b Make iterators in CScript::FindAndDelete const (Pieter Wuille)
Pull request description:
This PR moves `FindAndDelete` and `GetOp2` out of CScript (the first is only used inside the interpreter and moved there, the second does not actually depend on any script specifics and works on any vector). Furthermore, all non-const-iterator versions of GetOp are replaced by const ones, removing a number of methods in the process.
The longer term goal here is making the script interpreter independent from the CScript representation.
Note for reviewers: both `FindAndDelete` and `GetScriptOp` are consensus critical.
Tree-SHA512: c4ccf91c0b33c37cff0d474aa8dd2dab25b5b7655e2ed69a9b15e29daf0a67b21d51c23e1defb3a72ec762bd6138de96f69c6db1fb9c1fe1e976e421261aedb7
Such outputs can still be watched, and signed for, but they aren't treated as valid payments.
That means they won't cause transactions to appear in listtransactions, their outputs to be
shown under listunspent, or affect balances.
