cc5d38f4b Add option to attach a python debugger if test fails (John Newbery)
Pull request description:
Adds a simple option to the test_framework to attach pdb if the test fails.
Helpful for catching and debugging intermittent failures: Run the test in a loop with this option. The first failure will cause execution to pause and nodes will be left running for interactive debugging.
@sdaftuar
Tree-SHA512: 01cfae15fa3f04ed6ec6a99fef60a6c6a59723429309e81eacd6767caf12f5758f59b337804291ecab33a38a2958f36e2b513d201bee72a2eeb207a67046f952
c5ebddd11 Tests: address placement should be deterministic by default (René Nyffenegger)
Pull request description:
Better version of wrong and closed pull request https://github.com/bitcoin/bitcoin/pull/10764
Tree-SHA512: dfda6ea4a9dd0f4c8b96212ad43a716ff1dddf115cd2712a2a7e42c97fc9494079c746906b39d880a9827c05d2b75c728afd4ca4519ce4d365f0dae0c4aec24c
faa76d1b7 qa: Fix inv race in example_test (MarcoFalke)
Pull request description:
There have been intermittent test failures on this script.
```py
File "./test/functional/example_test.py", line 216, in run_test
assert_equal(block, 1)
AssertionError: not(2 == 1)
```
Probably the simplest way to fix them is overriding the `on_inv` method, so that no "colliding" getdata for the blocks are sent out.
Additionally, all getdata are now sent in a single message.
Tree-SHA512: 809c2bbfa90a67fc97905769fcbe90ba9abe1aac1f145530934f56a364835973b94d3302b6be68f4f2987acf333bce146bcc4c878c283301871ba5bb1a9bedb6
86279464b [RPC] trivial: gettxout no longer shows version of tx (Felix Weis)
Pull request description:
Since the switch to a per-txout chainstate db in #10195, the tx version information is no longer stored. Updated `gettxout` rpc help text accordingly.
Tree-SHA512: 3d7f42ef0f649056ece98bf22a1e972d1876324733adc81fa31bc2cd160550c5b6cb8682209fb8e8dbc56a8139ed5f5f0e740945f709039e69d52997ddbca7b8
4d4fb33fc Rename member field according to the style guide. (Pavel Janík)
Pull request description:
After #10193, approx. five instances of this warning are printed when compiling with `-Wshadow`:
```
In file included from txmempool.cpp:14:
./reverse_iterator.h:20:22: warning: declaration shadows a field of 'reverse_range<T>' [-Wshadow]
reverse_range(T &x) : x(x) {}
^
./reverse_iterator.h:17:8: note: previous declaration is here
T &x;
^
1 warning generated.
```
Tree-SHA512: 6c07c2ed6f4f232a3a8bdcdd6057040967c74552fd29d80f42e8a453b95baf203c410aa31dccc08ff2e765cbba02b1a282f6ea7804955f09b31ab20ef383792e
fd05132e5 Restore default format state of cout after printing with std::fixed/setprecision (practicalswift)
Pull request description:
Restore default format state of `std::cout` after printing with `std::fixed`/`std::setprecision`.
Tree-SHA512: 445b5b42aff58e2350939e8febc9b4a6fff478616abfe831aec42bee906cefac7a153c93d506407fb213d04dae9c7afbb5bfd344be63ca0f40ae39b331a4144f
13b1e9a16 Capitalize bullet points in CONTRIBUTING guide (Evan Klitzke)
Pull request description:
English grammar dictates that these bullet points should be capitalized.
This also makes the capitalization style consistent with the rest of the
document, e.g. the "Decision Making Process" section.
Tree-SHA512: 59f5a8941180ff3862ba63d364c27fd83d2e144299a71b2e784d58f806e8a02d7951dcc80fcc7152d0c78c2d1f5a22db1236af7ea6b9abece8dbe93533e4b65c
4ccc12a54 [qa] Rewrite BIP66 functional tests (Suhas Daftuar)
d4f0d87b6 [qa] Rewrite BIP65 functional tests (Suhas Daftuar)
Pull request description:
After 122786d0e0, BIP65 and BIP66 activate at
particular fixed heights (without regard to version numbers of blocks
below those heights). Rewrite the functional tests to take
this into account, and remove two tests that weren't really testing anything.
Moves the rewritten functional tests out of the extended test suite, so that they run in travis regularly.
Note: I discovered that the ComparisonTestFramework (which the original versions of these p2p tests were written is, has a bug that caused them to not catch obvious errors, eg if you just comment out setting the script flags for these softforks in ConnectBlock, the versions of these tests in master do not fail(!) -- will separately PR a fix for the comparison test framework).
Tree-SHA512: 2108b79951f2e980854d0d14ccc0fd1810a43dd4251fd6e3b203e7f104ea55e00650bb1a60a28c2801b4249f46563a8559ac3e3cd39db220914acfed0b3b163d
English grammar dictates that these bullet points should be capitalized.
This also makes the capitalization style consistent with the rest of the
document, e.g. the "Decision Making Process" section.
861f9a2 Skip remainder of init if upgrade is cancelled (Matt Corallo)
Pull request description:
Based on #10919.
Without this, if you cancel upgrade, you get a needless error:
ERROR: VerifyDB(): *** irrecoverable inconsistency in block data at
Tree-SHA512: aa47665682c6605ada376f1c100ce17cf8c4312427929eb2e75306f2199b47cbcdb4e0d98d5efcfefff03947b2c0fcbd3aab487a4ed14d50607df685c91a03d0
055d95f [wallet] return correct error code from resendwallettransaction (John Newbery)
Pull request description:
New code in #10995 uses `RPC_INVALID_REQUEST`. According to the comment in rpc/protocol.h:
```
// RPC_INVALID_REQUEST is internally mapped to HTTP_BAD_REQUEST (400).
// It should not be used for application-layer errors.
```
Change the returned error code to `RPC_WALLET_ERROR`
#11000 will need to be updated to test for the correct error code.
Tree-SHA512: 0201b3a2091adf17ad301825da5bd29f0ea7e284b5394cbef80483fc293a558acc849f74a0780bb8501acab324fc722e41ae049cffec7afb76884e26df4b809e
ce07638 doc: Add comment to use ValueFromAmount/AmountFromValue for JSON, not utilmoneystr (Wladimir J. van der Laan)
ec05c50 rpc: Use ValueFromAmount instead of FormatMoney in TxToUniv (Wladimir J. van der Laan)
46347ad rpc: Move ValueFromAmount to core_write (Wladimir J. van der Laan)
dac3782 doc: Correct AmountFromValue/ValueFromAmount names (Wladimir J. van der Laan)
Pull request description:
With this, the amounts returned in `decoderawtransaction` will be padded to 8 digits like anywhere else in the API.
This is accomplished by using `ValueFromAmount` in `TxToUniv`, instead of `FormatMoney` which it currently (mistakingly) uses. The `FormatMoney` function is only for debugging/logging use!
To avoid dependency issues, `ValueFromAmount` is moved to `core_write.cpp`, where it also fits better. I don't move `AmountFromValue` to `core_read.cpp` at the same time, as this would have more impact due to the RPCError dependency there.
(n.b.: large number of changed files is solely due to the util_tests JSONs needing update)
Tree-SHA512: 10fc2d27d33a77dbcb57aa7eccd4f53110c05d38eb7df6d40f10f14c08fad4274472e93af75aa59fe68ad0720fdf0930f0108124abef518e0dd162b3d2b2b292
5e35cd94c [tests] Test disconnecting unsupported service bits logic. (John Newbery)
Pull request description:
In v0.15, we disconnect nodes that send us version messages with
unsupported service bits (1 << 5 and 1 << 7). This commit adds a test
that bitcoind will disconnect those nodes before August 1st 2018, and won't
disconnect those nodes after August 1st 2018.
@sdaftuar @TheBlueMatt
Tree-SHA512: cb1bf311f9edc4aa5acf621e392543fd92952b2462ae2a8c20339e5c41e67ece1a2a9ede38db5dcd16563faa9ee58318e118f54024dd282c580132202d2df01b
In v0.15, we disconnect nodes that send us version messages with
unsupported service bits (1 << 5 and 1 << 7). This commit adds a test
that bitcoind will disconnect those nodes before August 1st 2018, and won't
disconnect those nodes after August 1st 2018.
ee2d10a Check if sys/random.h is required for getentropy on OSX. (James Hilliard)
Pull request description:
This should check and include sys/random.h if required for osx as mentioned [here](https://github.com/bitcoin/bitcoin/pull/9821#issuecomment-290936636).
Tree-SHA512: e9491f67f2e8b2e6bcdbcbb8063295e844d5627daf5336e3e17b4a8027d888fa65a08e4580a745abdc35ffd8d86b4fc7434daaac172c4a06ab7566a2ed0bfb92
This is necessary because core_write has to write amounts in
TxToUniv, and mistakingly uses FormatMoney for that
(which is only for debugging).
We don't move AmountFromValue at the same time, as
this is more challenging due to the RPCError depencency
there.
01699fb Fix resendwallettransactions assert failure if -walletbroadcast=0 (Matt Corallo)
Pull request description:
This fixes#10981 in my preferred way.
Tree-SHA512: 2e43d3ac78d13c5d59db23a82c76c722cc3344767a8237617080e489296d27a98bb1b3bd469b2c9b289b57a9da3709c90448d7a23bcc2e1dfb791c4fd16be015
e7539f8 Fix some broken init-time prints/constants (Matt Corallo)
13ab353 Check for empty coinsview instead of just-reset coinsview in init (Matt Corallo)
fce3f4f Fix resume-of-reindex-after-restart (Matt Corallo)
efac91e Always wait for threadGroup to exit in bitcoind shutdown (Matt Corallo)
Pull request description:
This is a follow-on to #10758 to help move 10758 along. The first fixes a regression in master that was partially fixed in 10758, the second I'm not sure if its a regression or not, but its clearly a bug that should be fixed.
Tree-SHA512: aca7b97a97dca66e1a218a33cc6f4aa002292ff1bb0af64e35b81fbaa91b9504f2605375808b43e93a63fc73634ad079b30ef6c9f4ba338d3b5f72d816dfeaff
1de73f4 Disconnect network service bits 6 and 8 until Aug 1, 2018 (Matt Corallo)
Pull request description:
Immediately disconnect peers that use service bits 6 and 8 until August 1st, 2018
These bits have been used as a flag to indicate that a node is running incompatible
consensus rules instead of changing the network magic, so we're stuck disconnecting
based on the service bits, at least for a while.
Staying connected to nodes on other networks only prevents both sides from reaching consensus quickly, wastes network resources on both sides, etc.
Didn't add constants to protocol.h as the code there notes that "service bits should be allocated via the BIP process".
Tree-SHA512: 2d887774fcf20357019ffc2a8398464c76c1cff2c4e448c92bd5f391d630312301977fea841e0534df6641c7c5547605a5aad82859c59c4bd68be865e6d5a4c6
1967d2a qt: Increase BLOCK_CHAIN_SIZE constants (Wladimir J. van der Laan)
Pull request description:
- Increase `BLOCK_CHAIN_SIZE` from 120GB to 150GB
- Increase `CHAIN_STATE_SIZE` from 2GB to 4GB
I took the local sizes of the blocks and chainstate directory, and added a bit extra to accomodate the near future (15GB for the chain and 1GB for the chainstate).
Tree-SHA512: 76ec7770bd3a30380b0224a0f307cdad14c8227ef726dd55738cebe9d894430865aff11e05a793fd3e60d8fe019dbb392f574c1fb63ec746618b4460ed64bd0c
11dd29b [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest& request) (practicalswift)
Pull request description:
When running `test_bitcoin` under Valgrind I found the following issue:
```
$ valgrind src/test/test_bitcoin
...
==10465== Use of uninitialised value of size 8
==10465== at 0x6D09B61: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B1BB: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B36C: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D17699: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x4CAAD7: operator<< (ostream:171)
==10465== by 0x4CAAD7: formatValue<ServiceFlags> (tinyformat.h:345)
==10465== by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl<ServiceFlags>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:523)
==10465== by 0x1924D4: format (tinyformat.h:510)
==10465== by 0x1924D4: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:803)
==10465== by 0x553A55: vformat (tinyformat.h:947)
==10465== by 0x553A55: format<ServiceFlags> (tinyformat.h:957)
==10465== by 0x553A55: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<ServiceFlags>(char const*, ServiceFlags const&) (tinyformat.h:966)
==10465== by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462)
==10465== by 0x28EDB5: CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_tests.cpp:31)
==10465== by 0x293947: rpc_tests::rpc_togglenetwork::test_method() (rpc_tests.cpp:88)
==10465== by 0x2950E5: rpc_tests::rpc_togglenetwork_invoker() (rpc_tests.cpp:84)
==10465== by 0x182496: invoke<void (*)()> (callback.hpp:56)
==10465== by 0x182496: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
...
```
The read of the uninitialized variable `nLocalServices` is triggered by `g_connman->GetLocalServices()` in `getnetworkinfo(const JSONRPCRequest& request)` (`net.cpp:462`):
```c++
UniValue getnetworkinfo(const JSONRPCRequest& request)
{
...
if(g_connman)
obj.push_back(Pair("localservices", strprintf("%016x", g_connman->GetLocalServices())));
...
}
```
The reason for the uninitialized `nLocalServices` is that `CConnman::Start(...)` is not called
by the tests, and hence the initialization normally performed by `CConnman::Start(...)` is
not done.
This commit adds a method `Init(const Options& connOptions)` which is called by both the
constructor and `CConnman::Start(...)`. This method initializes `nLocalServices` and the other
relevant values from the supplied `Options` object.
Tree-SHA512: d8742363acffd03b2ee081cc56840275569e17edc6fa4bb1dee4a5971ffe4b8ab1d2fe7b68f98a086bf133b7ec46f4e471243ca08b45bf82356e8c831a5a5f21
9baca41 build: always attempt to enable targeted sse42 cxxflags (Cory Fields)
Pull request description:
For depends builds without this, configure thinks that the user has overridden CXXFLAGS manually, when really they've just been set by the config.site.
The effect is that warnings and extra cxxflags (sse4.2 for example) were not being added.
Tree-SHA512: 9fd615ad0e926bd9d6b541ffcf7fc555e2147e8761f57ff3b5fb5d196c9cef0f26aa99681ff72db8c83c0f9a7ed91f4253f46bab09f2c835044b68047358fa47
- Increase `BLOCK_CHAIN_SIZE` from 120GB to 150GB
- Increase `CHAIN_STATE_SIZE` from 2GB to 4GB
I took the local sizes of the blocks and chainstate directory, and added
a bit extra to accomodate the near future (15GB for the chain and 1GB
for the chainstate).
49d903e Eliminate fee overpaying edge case when subtracting fee from recipients (Alex Morcos)
Pull request description:
I'm not sure if this is the cause of the issue in #10034 , but this was a known edge case. I just didn't realize how simple the fix is.
Could use a couple more eyes to make sure nothing silly can go wrong here, but if we all agree it's this simple, we can add this as another 0.15 bug fix.
Tree-SHA512: db1dd1e83363a3c231267b626d3a388893ee70ba1972056fe2c339c5c9e4fbfd30f7fe837c30cc7be884d454797fd4c619b9d631a8d5eeb55cdb07402a83acb3
When running test_bitcoin under Valgrind I found the following issue:
```
$ valgrind src/test/test_bitcoin
...
==10465== Use of uninitialised value of size 8
==10465== at 0x6D09B61: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B1BB: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B36C: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D17699: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x4CAAD7: operator<< (ostream:171)
==10465== by 0x4CAAD7: formatValue<ServiceFlags> (tinyformat.h:345)
==10465== by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl<ServiceFlags>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:523)
==10465== by 0x1924D4: format (tinyformat.h:510)
==10465== by 0x1924D4: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:803)
==10465== by 0x553A55: vformat (tinyformat.h:947)
==10465== by 0x553A55: format<ServiceFlags> (tinyformat.h:957)
==10465== by 0x553A55: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<ServiceFlags>(char const*, ServiceFlags const&) (tinyformat.h:966)
==10465== by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462)
==10465== by 0x28EDB5: CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_tests.cpp:31)
==10465== by 0x293947: rpc_tests::rpc_togglenetwork::test_method() (rpc_tests.cpp:88)
==10465== by 0x2950E5: rpc_tests::rpc_togglenetwork_invoker() (rpc_tests.cpp:84)
==10465== by 0x182496: invoke<void (*)()> (callback.hpp:56)
==10465== by 0x182496: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
...
```
The read of the uninitialized variable nLocalServices is triggered by g_connman->GetLocalServices()
in getnetworkinfo(const JSONRPCRequest& request) (net.cpp:462):
```c++
UniValue getnetworkinfo(const JSONRPCRequest& request)
{
...
if(g_connman)
obj.push_back(Pair("localservices", strprintf("%016x", g_connman->GetLocalServices())));
...
}
```
The reason for the uninitialized nLocalServices is that CConnman::Start(...) is not called
by the tests, and hence the initialization normally performed by CConnman::Start(...) is
not done.
This commit adds a method Init(const Options& connOptions) which is called by both the
constructor and CConnman::Start(...). This method initializes nLocalServices and the other
relevant values from the supplied Options object.
This fixes a few cases where we should be treating a restart-after-
coinsviewdb-reset identically to a just-reset-coinsviewdb.
Thanks to @morcos for identifying the bug.
This more clearly uses fReindex vs fReset to make sure we're not
clearing our coinsdb needlessly when restarting after a reindex.
It also makes it so that restarting after shutting down mid-reindex
isn't treates specially at all during txdb loading code, as it
shouldn't be.
This resolves a possible-assert-on-shutdown race introduced in
1f668b6468 when early shutdown
occurs.
Previously this was not done to avoid any cases where the
threadGroup might not exit due to a blocking thread, but at this
point the threadGroup isn't used all that much, plus Qt already
does this, and its good to keep their init/shutdown consistent.
For those curious, the threadGroup is only used in a few places:
* Its used to run the CCheckQueues in script validation, but these
use the boost mutex/condition variable primitives, so they
respect the interrupt pretty trivially.
* Its used for the import thread, which should exit rather quickly
as mostly it just calls LoadExternalBlockFile, which has an
interruption_point right before each block loaded.
* Its used in the scheduler thread, which is only used for:
* validationinterface has an effectively-dummy reference to it.
* wallet compaction, which should not last long
* addr/banlist dumping from CConnman, which should also be fast
