8931a95bec Include util/strencodings.h which is required for IsSpace(...) (practicalswift)
7c9f790761 Update KNOWN_VIOLATIONS: Remove fixed violations (practicalswift)
587924f000 Use IsSpace(...) instead of boost::is_space (practicalswift)
c5fd143edb Use ToLower(...) instead of std::tolower (practicalswift)
e70cc8983c Use IsDigit(...) instead of std::isdigit (practicalswift)
Pull request description:
* Use `ToLower(...)` instead of `std::tolower`. `std::tolower` is locale dependent.
* Use `IsDigit(...)` instead of `std::isdigit`. Some implementations (e.g. Microsoft in 1252 codepage) may classify single-byte characters other than `[0-9]` as digits.
* Update `KNOWN_VIOLATIONS`: Remove fixed violations.
* ~~Replace use of locale dependent Boost trim (`boost::trim`) with locale independent `TrimString`.~~
* Use` IsSpace(...)` instead of `boost::is_space`
Tree-SHA512: defed016136b530b723fa185afdbd00410925a748856ba3afa4cee60f61a67617e30f304f2b9991a67b5fe075d9624f051e14342aee176f45fbc024d59e1aa82
59e387705c test: add invalid tx templates for use in functional tests (James O'Beirne)
Pull request description:
This change adds a list of `CTransaction`-generating templates which each correspond to a specific type of invalid transaction. We then use this list to test for a wider variety of invalid tx types in `p2p_invalid_tx.py` and `feature_block.py`.
Consolidating all invalid tx types will allow us to more easily cover all tx reject cases from a variety of tests without repeating ourselves. Validation logic doesn't differ much between mempool and block acceptance, but there *is* a difference and we should be sure we're testing both comprehensively.
Right now, I've only added templates covering the tx reject types listed below but if this approach seems worthwhile I will expand the list to be fully comprehensive.
```
bad-txns-in-belowout
bad-txns-inputs-duplicate
bad-txns-too-many-sigops
bad-txns-vin-empty
bad-txns-vout-empty
bad-txns-vout-negative
```
Tree-SHA512: 05407f4a953fbd7c44c08bb49bb989cefd39a2b05ea00f5b3c92197a3f05e1b302f789e33832445734220e1c333d133aba385740b77b84139b170c583471ce20
cb53b825c2 scripted-diff: Replace boost::bind with std::bind (Chun Kuan Lee)
2196c51821 refactor: Use boost::scoped_connection in signal/slot, also prefer range-based loop instead of std::transform (Chun Kuan Lee)
Pull request description:
Replace boost::bind with std::bind
- In `src/rpc/server.cpp`, replace `std::transform` with simple loop.
- In `src/validation.cpp`, store the `boost::signals2::connection` object and use it to disconnect.
- In `src/validationinterface.cpp`, use 2 map to store the `boost::signals2::scoped_connection` object.
Tree-SHA512: 6653cbe00036fecfc495340618efcba6d7be0227c752b37b81a27184433330f817e8de9257774e9b35828026cb55f11ee7f17d6c388aebe22c4a3df13b5092f0
31926ee8cf [test] functional framework: add CScript hex() for Python 3.4 (Sjors Provoost)
74ce326831 [test] Travis: enforce Python 3.4 support in functional tests (Sjors Provoost)
Pull request description:
The minimum supported version of Python is 3.4 according to [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md). This PR makes the Travis linter use this version in order to catch accidental use of modern syntax.
Tree-SHA512: 71b2c102be72b135a8ba049378d66875760f20a04a657102a399240c5c2b2ddbdfa7d5ab4c0c0242ecc3259e0ee8eb2273f331bc5eb824f4ae4c3cc58aea37ac
Make lint/check-doc.py Python 3.4 compatible.
Also add .python-version for pyenv which will cause tests with too
modern syntax to fail on developer machine rather than on Travis.
688f665a5e Scripts and tools & Docs: Used #!/usr/bin/env bash instead of obsolete #!/bin/bash, added linting for .sh files shebang and updated the Developer Notes. (vim88)
Pull request description:
As it was discussed in [#13510](https://github.com/bitcoin/bitcoin/pull/13510), it is better to use `#!/usr/bin/env bash` instead of `#!/bin/bash`.
Tree-SHA512: 25f71eb9a6a0cdc91568b5c6863205c5fe095f77a69e633503a2ac7805bd9013af8538e538c0c666ce96a28e3f43ce7a8df5f08d4ff007723bb588d85674f2da
65f3672f3b wallet: Refactor to use WalletLocation (João Barbosa)
01a4c095c8 wallet: Add WalletLocation utility class (João Barbosa)
Pull request description:
Advantages of this change:
- avoid resolving wallet absolute path and name repetitively and in multiple places;
- avoid calling `GetWalletDir` in multiple places;
- extract these details from the actual wallet implementation.
The `WalletLocation` class can be a way to represent a wallet not yet loaded that exists in the wallet directory.
Tree-SHA512: 71ec09786e038499710e7acafe92d66ab9883fc894964e267443ae9c10a6872a10995c3987a169c436a4e793dae96b28fb97bd7f78483c4b72ac930fa23f8686
854c85ae90 test: allow arguments to be forwarded to flake8 in lint-python.sh (James O'Beirne)
Pull request description:
In order to use `lint-python.sh` from within various in-editor linting frameworks (e.g. [ALE](https://github.com/w0rp/ale), [flycheck](https://github.com/flycheck/flycheck)), we need to allow its arguments to be forwarded to the wrapped flake8 invocation.
For what it's worth, here's my bitcoin-specific ALE vim config for doing so (requires this changeset):
```vim
$ grep python /home/james/src/bitcoin/.exrc
let g:ale_python_flake8_executable="/home/james/src/bitcoin/test/lint/lint-python.sh"
```
Tree-SHA512: 0d5500238ea5fde26ee9c21f6518a3a3dc8409c77ad1271ff7e7a94ef45a8c8d2e1b8ad3df3075dd4062ee0fff534625b1bc79613f869cd3c2d9260814ffc7ee
7ac911afe7 [docs] Add release notes for removing `-usehd` (John Newbery)
25548b2958 [wallet] Remove -usehd (John Newbery)
Pull request description:
`-usehd` is no longer used (except to tell the user that they've set it incorrectly for the wallet that they're loading). Remove it (in the same spirit as #14272)
Tree-SHA512: 5bdcd2bb9bb8504a01343595bcd1bd433d97b730255152c725103c1ac3fa3a9d9e5220a4c29d4c72307cf803e1c09d31080f83603c23dc77263846e17b1826f0
fa8433e379 qa: Remove unneded import_deterministic_coinbase_privkeys overwrite, add comments (MarcoFalke)
e413c2ddd1 qa: Fix codespell error and have lint-spelling error instead of warn (MarcoFalke)
Pull request description:
Currently the functional tests require the wallet module to be compiled into the Bitcoin Core executable. For example the premine (or datadir cache) to speed up tests when run in parallel would mine a bunch of blocks and store the private keys to sign the coinbase tx outputs in a wallet. There is no need to have the overhead of the whole wallet module by using keys that are deterministic for all runs.
Note that this change most likely requires the `./test/cache/` to be cleared.
Tree-SHA512: 9ce26036b0e10f0f888f66a1e50be6a357343f9ffb302ae24a7bb3df2f083a31702ef308b738a03b08a1b623aeddac5d6563dc1b15078c0357b7dafad7808ec3
bb6ca65f98 gui: get special folder in unicode (Chun Kuan Lee)
1c5d225853 Drop boost::scoped_array (Chun Kuan Lee)
Pull request description:
Drop boost::scoped_array and simplify the code.
`TCHAR` should be defined as `wchar_t` if `UNICODE` is defined. So we can use `.toStdWString().c_str()` to get wchar_t C-style string.
Fix#13819
Tree-SHA512: 3fd4aa784129c9d1576b01e6ee27faa42d793e152d132f2dde504d917dad3a8e95e065fcbc54a3895d74fb6b2a9ed4f5ec67d893395552f585e225486a84a454
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
341f7c7b0e macOS fix: Check for correct version of flake8 to avoid spurious warnings. The brew installed flake8 version is Python 2 based and does not work. (practicalswift)
908a559f33 macOS fix: Add excludes for checks added in the newer shellcheck version installed by brew (practicalswift)
ec4d57bbb3 macOS fix: Work around empty (sub)expression error when using BSD grep (practicalswift)
b57d7d92fe macOS fix: Avoid mapfile due to ancient version of bash shipped with macOS (practicalswift)
Pull request description:
The linters are thoroughly tested under Ubuntu which is what we use in Travis. When reading #14041 I understood that some developers were experiencing problems when running the linters on their local machines.
Assuming these local machines were running macOS I installed a fresh macOS VM, followed the instructions in `build-osx.md` and ran the linters.
This PR contains the changes needed to make `lint-all.sh` run as expected.
Ideally the linters would continuously run also under a Travis macOS environment to make sure we catch these kind of issues before merge.
Tree-SHA512: b39c9a970d14d27db1fb592539923c0bc676b5217f415d02fda3f17bf54d46faa172376e8a3ecab07ca68a3acba9aebe00b2b1b2161b2a36b85fbb672e7efb5c
5d62dcf9cf lint: Make sure we read the command line inputs using utf-8 decoding in python (Chun Kuan Lee)
Pull request description:
Make sure we read the command line inputs using utf-8 decoding in python
occurred from travis cron job:
contrib/verify-commits/verify-commits.py should run with utf-8, otherwise it would raise UnicodeDecodeError
`UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 744: ordinal not in range(128)`
Tree-SHA512: 90e4ad57fdbbbecb0a21fc2d2b03a04f5ef125e54124719ef36e5a85326930b732b47534757a7c3a8730096f3947b009ec898191928b5c2d38f9f4b3e37db48d
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
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
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.
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.
68400d8b96 tests: Use explicit imports (practicalswift)
Pull request description:
Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports.
Wildcard imports make it unclear which names are present in the namespace, confusing both readers and many automated tools.
An additional benefit of not using wildcard imports in tests scripts is that readers of a test script then can infer the rough testing scope just by looking at the imports.
Before this commit:
```
$ contrib/devtools/lint-python.sh | head -10
./test/functional/feature_rbf.py:8:1: F403 'from test_framework.util import *' used; unable to detect undefined names
./test/functional/feature_rbf.py:9:1: F403 'from test_framework.script import *' used; unable to detect undefined names
./test/functional/feature_rbf.py:10:1: F403 'from test_framework.mininode import *' used; unable to detect undefined names
./test/functional/feature_rbf.py:15:12: F405 bytes_to_hex_str may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:17:58: F405 CScript may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:25:13: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:26:31: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:26:60: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:30:41: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:30:68: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
$
```
After this commit:
```
$ contrib/devtools/lint-python.sh | head -10
$
```
Tree-SHA512: 3f826d39cffb6438388e5efcb20a9622ff8238247e882d68f7b38609877421b2a8e10e9229575f8eb6a8fa42dec4256986692e92922c86171f750a0e887438d9
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
This linter checks that the number of arguments passed to each variadic format
string function matches the number of format specifiers in the format string.
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
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
Protects against added circular depencies, makes it explicit in the
code when circular dependencies have been removed.
Modeled after EXPECTED_BOOST_INCLUDES in lint-includes.sh
927e1150bc Test: Harden lint-filenames.sh (wodry)
Pull request description:
- This fixes that only files with lower case file name suffix where found before, which contradicted the Regex to find uppercase characters in file names (including suffixes I guess).
- `--full-name` switch was added to git ls-files, to define that the found file always includes it's full path in the git project.
- since we know now that the file name includes the full path, we can harden the Regex to exclude the secp256k1 and univalue sub folders.
- use backslash line break to make code easier to read and avoid too long line.
Tree-SHA512: 9b55fe4965ae2084112b9f8a81bf9c657756c2cb5004986e7b6102a76adaf62c7d7a53257d9f13c5d8a1c75870b52c744d13830e3edd454a099e810357c914e5
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
c8176b3cc7 Add linter: Make sure we explicitly open all text files using UTF-8 or ASCII encoding in Python (practicalswift)
634bd97001 Explicitly specify encoding when opening text files in Python code (practicalswift)
Pull request description:
Add linter: Make sure we explicitly open all text files using UTF-8 encoding in Python.
As requested by @laanwj in #13440.
Tree-SHA512: 1651c00fe220ceb273324abd6703aee504029b96c7ef0e3029145901762c733c9b9d24927da281394fd4681a5bff774336c04eed01fafea997bb32192c334c06
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
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
698cfd0811 docs: Mention lint-locale-dependence.sh in developer-notes.md (practicalswift)
0a4ea2f458 build: Add linter for checking accidental locale dependence (practicalswift)
Pull request description:
This linter will check for code accidentally introducing locale dependencies.
Unnecessary locale dependence can cause bugs that are very tricky to isolate and fix. We should avoid using locale dependent functions if possible.
Context: https://github.com/bitcoin/bitcoin/pull/12881#issuecomment-378564722
Example output:
```
$ contrib/devtools/lint-locale-dependence.sh
The locale dependent function tolower(...) appears to be used:
src/init.cpp: if (s[0] == '0' && std::tolower(s[1]) == 'x') {
Unnecessary locale dependence can cause bugs that are very
tricky to isolate and fix. Please avoid using locale dependent
functions if possible.
Advice not applicable in this specific case? Add an exception
by updating the ignore list in contrib/devtools/lint-locale-dependence.sh
```
**Note to reviewers:** What is the most appropriate `LOCALE_DEPENDENT_FUNCTIONS` function list? What should be added or removed?
Tree-SHA512: 14e448828804bb02bf59070647e38b52fce120c700c903a4a8472769a2cee5dd529bd3fc182386993cb8720482cf4250b63a0a477db61b941ae4babe5c65025f