Commit Graph

22008 Commits

Author SHA1 Message Date
Gr0kchain
582e66b6e7 doc: Added regtest config for linearize script
Updated the example-linearize.cfg file to include support for the regtest chain network config which is used by the ./linearize-data.py

Problem:

Without the regtest magic, genesis hash and path config, the `linearize-data.py` script cannot generate a bootstrap.dat file.

Example of error:

./linearize-data.py ./linearize.cfg
Read 102 hashes
Genesis block not found in hashlist

Solution:

Added netmagic, genesis and input example parameters to file.

Resolution

1. Starting bitcoind in regtest mode
2. bitcoin-cli generatetoaddress 101 $(bitcoin-cli getnewaddress)
3. ./linearize-hashes.py ./linearize.cfg > ./hashlist.txt
4. ./linearize-data.py ./linearize.cfg

Example after fix:
$ ./linearize-data.py ./linearize.cfg
Read 102 hashes
Input file /Users/gr0kchain/.bitcoin/regtest/blocks/blk00000.dat
Output file /Users/gr0kchain/Downloads/bootstrap.dat
Done (102 blocks written)
2019-11-07 08:07:10 +02:00
Wladimir J. van der Laan
224c19645f
Merge #17388: Add missing newline in util_ChainMerge test
3645e4ca00 Add missing newline in util_ChainMerge test (Russell Yanofsky)

Pull request description:

  This was causing a lot of test cases not not be very meaningful because
  multiple configuration options were combined into one line.

  The changes in test output with this fix make sense and look like:

  ```diff
  - testnet=1 regtest=1 || test
  + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one.
  ```

  Issue was reported and debugged by
  Wladimir J. van der Laan <laanwj@protonmail.com> in
  https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222

  <!--
  *** Please remove the following help text before submitting: ***

  Pull requests without a rationale and clear improvement may be closed
  immediately.
  -->

  <!--
  Please provide clear motivation for your patch and explain how it improves
  Bitcoin Core user experience or Bitcoin Core developer experience
  significantly:

  * Any test improvements or new tests that improve coverage are always welcome.
  * All other changes should have accompanying unit tests (see `src/test/`) or
    functional tests (see `test/`). Contributors should note which tests cover
    modified code. If no tests exist for a region of modified code, new tests
    should accompany the change.
  * Bug fixes are most welcome when they come with steps to reproduce or an
    explanation of the potential issue as well as reasoning for the way the bug
    was fixed.
  * Features are welcome, but might be rejected due to design or scope issues.
    If a feature is based on a lot of dependencies, contributors should first
    consider building the system outside of Bitcoin Core, if possible.
  * Refactoring changes are only accepted if they are required for a feature or
    bug fix or otherwise improve developer experience significantly. For example,
    most "code style" refactoring changes require a thorough explanation why they
    are useful, what downsides they have and why they *significantly* improve
    developer experience or avoid serious programming bugs. Note that code style
    is often a subjective matter. Unless they are explicitly mentioned to be
    preferred in the [developer notes](/doc/developer-notes.md), stylistic code
    changes are usually rejected.
  -->

  <!--
  Bitcoin Core has a thorough review process and even the most trivial change
  needs to pass a lot of eyes and requires non-zero or even substantial time
  effort to review. There is a huge lack of active reviewers on the project, so
  patches often sit for a long time.
  -->

ACKs for top commit:
  laanwj:
    ACK 3645e4ca00
  practicalswift:
    ACK 3645e4ca00 -- diff looks correct

Tree-SHA512: ca5bde9b9f553811d4827113f4880d15d7b8f4f1455b95bbf34c9a1512fdd53062f1a2133c50d9b54f94160a1ee77a54bc82681a5f3bf25d2b0d01f8a8e95165
2019-11-06 09:53:38 +01:00
fanquake
4e21f72980
Merge #17370: doc: Update doc/bips.md with recent changes in master
fa7f5a4d2a doc: Update doc/bips.md with recent changes in master (MarcoFalke)

Pull request description:

  Follow-up to #17165

ACKs for top commit:
  jonatack:
    ACK fa7f5a4d2a. Verified markdown view at https://github.com/MarcoFalke/bitcoin-core/blob/1911-docBips/doc/bips.md and the urls in the links. Some of the PRs are indicated with # and some without, but this is the case over the whole document.
  laanwj:
    ACK fa7f5a4d2a
  fanquake:
    ACK fa7f5a4d2a

Tree-SHA512: 31782b5f1f2f10b1189f05f010f908c183dbe723477ca1c46ad1d3bee5ea483335847008a7fe48d72373ccd39b84e0b950d0d1b23e457cb70f34210c5f2dc6aa
2019-11-05 21:37:02 -05:00
Wladimir J. van der Laan
45e65376ac
Merge #17382: rpc: Remove unused boost::this_thread::interruption_point
fa5facd3e7 rpc: Remove unused boost::this_thread::interruption_point (MarcoFalke)

Pull request description:

  There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points

  However, the rpc threads are `std::thread`, which does not have an `std:🧵:interrupt` member function to request interruption: https://dev.visucore.com/bitcoin/doxygen/httpserver_8cpp.html#ae1a63374e18b9abd348eb74e4243ea34

  Thus, the interruption points can be removed.

ACKs for top commit:
  laanwj:
    ACK fa5facd3e7, this does nothing.
  practicalswift:
    ACK fa5facd3e7
  jamesob:
    ACK fa5facd3e7

Tree-SHA512: 4e29a44df1f2702cbd1ffdffa559440a8bb800baab64b4116e2c3d27cd64d8d1e8aafe1dc21b1a4e3988470d03be19cae294bd5669f7abf6d487685dc8fd8d7e
2019-11-06 00:04:38 +01:00
Wladimir J. van der Laan
40b6070ad7
Merge #16805: logs: add timing information to FlushStateToDisk()
dcef9a2922 logs: add timing information to FlushStateToDisk() (James O'Beirne)
41edaf227a logs: add BCLog::Timer and related macros (James O'Beirne)

Pull request description:

  It's currently annoying to detect FlushStateToDisk() calls when benchmarking since they have to be inferred from a drop in coins count from the `UpdateTip: ` log messages. This adds a new logging utility, `BCLog::Timer`, and some related macros that are generally useful for printing timing-related logging messages, and a message that is unconditionally written when the coins cache is flushed to disk.

  ```
  2019-09-04T20:17:51Z FlushStateToDisk: write block and undo data to disk completed (3ms)
  2019-09-04T20:17:51Z FlushStateToDisk: write block index to disk completed (370ms)
  2019-09-04T20:17:51Z FlushStateToDisk: write coins cache to disk (2068451 coins, 294967kB) completed (21481ms)
  ```

ACKs for top commit:
  laanwj:
    Thanks, ACK dcef9a2922
  ryanofsky:
    Code review ACK dcef9a2922. No changes since last review other than moving code to new timer.h header

Tree-SHA512: 6d61e48a062d3edb48d0e056a6f0b1f8031773cc99289ee4544f8349d24526b88519e1e304009d56e428f1eaf76c857bf8e7e1c0b6873a6f270306accb5edc3d
2019-11-05 23:45:30 +01:00
Russell Yanofsky
3645e4ca00 Add missing newline in util_ChainMerge test
This was causing a lot of test cases not not be very meaningful because
multiple configuration options were combined into one line.

The changes in test output with this fix make sense and look like:

```diff
- testnet=1 regtest=1 || test
+ testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one.
```

Issue was reported and debugged by
Wladimir J. van der Laan <laanwj@protonmail.com> in
https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222
2019-11-05 17:25:16 -05:00
Wladimir J. van der Laan
d9a4550001
Merge #17360: gui: Improve "Hide" button tool-tip message
1c26c16065 Improve "Hide" button tool-tip message (Danny-Scott)

Pull request description:

  Cleaned up the tool tip text, it looks as though it just got included back in 2014 when the whole section was added.

  Changed hide button tool tip within transaction fee settings area from "collapse fee-settings" to "Hide transaction fee settings" to be more user friendly and fit with other tool tips.

  ![hide-transaction-fee-tool-tip](https://user-images.githubusercontent.com/17258195/68086415-b7b70680-fe43-11e9-82cb-567b9730c1b9.png)

ACKs for top commit:
  laanwj:
    ACK 1c26c16065

Tree-SHA512: e2c83271c273f785ac625da9f88e095076043e21a9c59792049c271747837d19483e0cae5466c26ef3231947b6245680c4c136a530ba6f1885f9ddc18f2560d6
2019-11-05 20:42:37 +01:00
MarcoFalke
fea532a5f2
Merge #16540: test: Add ASSERT_DEBUG_LOG to unit test framework
fa2c44c3cc test: Add ASSERT_DEBUG_LOG to unit test framework (MarcoFalke)
fa1936f57b logging: Add member for arbitrary print callbacks (MarcoFalke)

Pull request description:

  Similar to `assert_debug_log` in the functional test framework

Top commit has no ACKs.

Tree-SHA512: aa9eaeca386b61d806867c04a33275f6eb4624fa5bf50f2928d16c83f5634bac96bcac46f9e8eda3b00b4251c5f12d7b01d6ffd84ba8e05c09eeec810cc31251
2019-11-05 14:34:42 -05:00
Danny-Scott
1c26c16065 Improve "Hide" button tool-tip message 2019-11-05 19:15:42 +00:00
MarcoFalke
fa5facd3e7
rpc: Remove unused boost::this_thread::interruption_point 2019-11-05 14:00:03 -05:00
Wladimir J. van der Laan
b05b28183c
Merge #16899: UTXO snapshot creation (dumptxoutset)
92b2f5306b test: add dumptxoutset RPC test (James O'Beirne)
c1ccbc3dde devtools: add utxo_snapshot.sh (James O'Beirne)
57cf74c991 rpc: add dumptxoutset (James O'Beirne)
92fafb3a7d coinstats: add coins_count (James O'Beirne)
707fde7b9b add unused SnapshotMetadata class (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

  ---

  This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`.

  All of this is unused at the moment.

ACKs for top commit:
  laanwj:
    ACK 92b2f5306b

Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31
2019-11-05 19:40:18 +01:00
James O'Beirne
92b2f5306b test: add dumptxoutset RPC test 2019-11-05 13:36:04 -05:00
James O'Beirne
c1ccbc3dde devtools: add utxo_snapshot.sh
to allow easy (if not time-consuming) generation and verification of
snapshots.
2019-11-05 13:36:04 -05:00
James O'Beirne
57cf74c991 rpc: add dumptxoutset
Allows the creation of a UTXO snapshot to disk.
2019-11-05 13:35:57 -05:00
Wladimir J. van der Laan
d35b12107e
Merge #17044: init: Remove auto-import of bootstrap.dat and associated code
104f7de593 remove old bootstrap relevant code (tryphe)

Pull request description:

  This picks up #15954

  I fixed the code and added at a functional test utilizing the scripts in `contrib/linearize` as suggested by @MarcoFalke .

ACKs for top commit:
  laanwj:
    ACK 104f7de593

Tree-SHA512: acac9f285f9785fcbc3afc78118461e45bec2962f90ab90e9f82f3ad28adc90a44f0443b712458ccf486e46d891eb8a67f53e7bee5fa6d89e4387814fe03f117
2019-11-05 19:25:10 +01:00
MarcoFalke
22e7eea629
Merge #17363: test: add "diamond" unit test to MempoolAncestryTests
b2ff500fb3 test: add "diamond" unit test to MempoolAncestryTests (Sebastian Falbesoner)

Pull request description:

  Approaches #17271 (_Missing Unit Test for Ancestors "diamond"_).
  If ancestors are represented more than once (in this case `ta` and `tb`), check that those are not overcounted.

ACKs for top commit:
  laanwj:
    ACK b2ff500fb3

Tree-SHA512: 82a6573cc7f0e82bf6fcfe207d7ddecbf297d2a203d22e95b73d887e3cb280f45a3c5f649161561c1be1eb560ff81b9b385868f205d1c12284211c2377e5ad99
2019-11-05 13:19:50 -05:00
MarcoFalke
50591f6ec6
Merge #17357: tests: Add fuzzing harness for Bech32 encoding/decoding
b7541705d0 tests: Add fuzzing harness for Bech32 encoding/decoding (practicalswift)
85a34b1683 tests: Move CaseInsensitiveEqual to test/util/str (practicalswift)

Pull request description:

  Add fuzzing harness for Bech32 encoding/decoding.

  **Testing this PR**

  Run:

  ```
  $ make distclean
  $ ./autogen.sh
  $ CC=clang CXX=clang++ ./configure --enable-fuzz \
        --with-sanitizers=address,fuzzer,undefined
  $ make
  $ src/test/fuzz/bech32 -max_total_time=60
  …
  ```

ACKs for top commit:
  jonatack:
    ACK b7541705d0

Tree-SHA512: ade01d30c6886a083b806dbfff08999cc0d08e687701c670c895e261ed242c789e8a0062d4ebbe8f82676b8f168dc37e83351a88822c9c0eab478572a9e1ec02
2019-11-05 12:54:50 -05:00
MarcoFalke
c7e6b3b343
Merge #17243: p2p: add PoissonNextSend method that returns mockable time
1a8f0d5a74 [tools] update nNextInvSend to use mockable time (Amiti Uttarwar)
4de630354f [tools] add PoissonNextSend method that returns mockable time (Amiti Uttarwar)

Pull request description:

  Introduce a Poisson helper method that wraps the existing method to return `std::chrono::duration` type, which is mockable.

  Needed for https://github.com/bitcoin/bitcoin/pull/16698.

ACKs for top commit:
  ajtowns:
    ACK 1a8f0d5a74
  MarcoFalke:
    re-ACK 1a8f0d5a74
  naumenkogs:
    ACK 1a8f0d5, and let's merge it and come back to it later.

Tree-SHA512: 7e2325d7c55fc0b4357cb86b83e0c218ba269f678c1786342d8bc380bfd9696373bc24ff124b9ff17a6e761c62b2b44ff5247c3911e2afdc7cc5c20417e8290b
2019-11-05 12:38:28 -05:00
MarcoFalke
54a337478d
Merge #17378: TestShell: Fix typos & implement cleanups
2493770e36 TestShell: Return self from setup() (James Chiang)
a8dea45524 TestShell: Simplify default setting of num_nodes (James Chiang)
9c7806e4bf Doc: Remove backticks in test-shell.md code block (James Chiang)
d3ed06e2cd TestShell: Fix typo in TestShell warning printout (James Chiang)

Pull request description:

  This PR follows up on #17288 and fixes typos and implements code clean-ups suggested by reviewers of 19139ee.

  - Typo in `test_shell.py` warning
  - Typo in `test-shell.md` code block
  - Simplified default setting of `num_nodes` in `TestShell.setup()`
  - Enable initializer chaining: `TestShell().setup()`

ACKs for top commit:
  MarcoFalke:
    ACK 2493770e36
  instagibbs:
    tACK 2493770e36
  jnewbery:
    utACK 2493770e36

Tree-SHA512: 8fa7c2c550dbc3ec899de9dc328cd55cfa6daafe3b888aa5427e72fea69f064d938ec68e15bfa57109c0f6c3583e627ac4bd69303a11575d056941bd253adee0
2019-11-05 12:36:16 -05:00
James O'Beirne
92fafb3a7d coinstats: add coins_count
Also changes existing CCoinsStats attributes to be C++11 initialized.
2019-11-05 10:54:00 -05:00
tryphe
104f7de593 remove old bootstrap relevant code
- only load blockfiles when we have paths
- add release notes for modified bootstrap functionality
- amend documentation on ThreadImport
2019-11-05 16:47:26 +01:00
MarcoFalke
fa7f5a4d2a
doc: Update doc/bips.md with recent changes in master 2019-11-05 10:20:16 -05:00
MarcoFalke
e204dc11b5
Merge #17365: depends: update README.md with working Android targets and API levels
3fe1aba601 depends: move README.md Android instructions to a separate section (Igor Cota)
aa9b84acee depends: update README.md with working Android targets and API levels (Igor Cota)

Pull request description:

  Per @Sjors comments in https://github.com/bitcoin/bitcoin/pull/16110#pullrequestreview-310821810

ACKs for top commit:
  Sjors:
    ACK 3fe1aba

Tree-SHA512: 7a2e676070d51c7a4291b0d4b638f52321c08cc6ebe2bd2c02ba62f6cc3dd8a73227df4693c6ce9201863eb0bf26e0133805347b9016cb0f9a389a49cc9492aa
2019-11-05 09:42:57 -05:00
James Chiang
2493770e36 TestShell: Return self from setup()
This allows user to chain setup() to the initializer. test-shell.md code
examples have been updated to reflect this.
2019-11-05 12:55:52 +01:00
James Chiang
a8dea45524 TestShell: Simplify default setting of num_nodes 2019-11-05 12:55:52 +01:00
James Chiang
9c7806e4bf Doc: Remove backticks in test-shell.md code block 2019-11-05 12:55:52 +01:00
James Chiang
d3ed06e2cd TestShell: Fix typo in TestShell warning printout 2019-11-05 12:55:52 +01:00
Amiti Uttarwar
1a8f0d5a74 [tools] update nNextInvSend to use mockable time 2019-11-05 11:12:10 +01:00
Amiti Uttarwar
4de630354f [tools] add PoissonNextSend method that returns mockable time 2019-11-05 11:06:53 +01:00
Igor Cota
3fe1aba601 depends: move README.md Android instructions to a separate section 2019-11-05 10:35:53 +01:00
practicalswift
b7541705d0 tests: Add fuzzing harness for Bech32 encoding/decoding 2019-11-05 09:23:44 +00:00
practicalswift
85a34b1683 tests: Move CaseInsensitiveEqual to test/util/str 2019-11-05 09:23:44 +00:00
Samuel Dobson
bdda137878
Merge #16766: wallet: Make IsTrusted scan parents recursively
4671fc3d9e Expand on wallet_balance.py comment from https://github.com/bitcoin/bitcoin/pull/16766\#issuecomment-527563982 (Jeremy Rubin)
91f3073f08 Update release notes to mention changes to IsTrusted and impact on wallet (Jeremy Rubin)
8f174ef112 Systematize style of IsTrusted single line if (Jeremy Rubin)
b49dcbedf7 update variable naming conventions for IsTrusted (Jeremy Rubin)
5ffe0d1449 Update comment in test/functional/wallet_balance.py (Jeremy Rubin)
a550c58267 Update wallet_balance.py test to reflect new behavior (Jeremy Rubin)
5dd7da4ccd Reuse trustedParents in looped calls to IsTrusted (Jeremy Rubin)
595f09d6de Cache tx Trust per-call to avoid DoS (Jeremy Rubin)
dce032ce29 Make IsTrusted scan parents recursively (Jeremy Rubin)

Pull request description:

  This slightly modifies the behavior of IsTrusted to recursively check the parents of a transaction. Otherwise, it's possible that a parent is not IsTrusted but a child is. If a parent is not trusted, then a child should not be either.

  This recursive scan can be a little expensive, so ~it might be beneficial to have a way of caching IsTrusted state, but this is a little complex because various conditions can change between calls to IsTrusted (e.g., re-org).~ I added a cache which works per call/across calls, but does not store the results semi-permanently. Which reduces DoS risk of this change. There is no risk of untrusted parents causing a resource exploitation, as we immediately return once that is detected.

  This is a change that came up as a bug-fix esque change while working on OP_SECURETHEBAG. You can see the branch where this change is important here: https://github.com/bitcoin/bitcoin/compare/master...JeremyRubin:stb-with-rpc?expand=1. Essentially, without this change, we can be tricked into accepting an OP_SECURETHEBAG output because we don't properly check the parents. As this was a change which, on its own, was not dependent on OP_SECURETHEBAG, I broke it out as I felt the change stands on its own by fixing a long standing wallet bug.

  The test wallet_balance.py has been corrected to meet the new behavior. The below comment, reproduced, explains what the issue is and the edge cases that can arise before this change.

          # Before `test_balance()`, we have had two nodes with a balance of 50
          # each and then we:
          #
          # 1) Sent 40 from node A to node B with fee 0.01
          # 2) Sent 60 from node B to node A with fee 0.01
          #
          # Then we check the balances:
          #
          # 1) As is
          # 2) With transaction 2 from above with 2x the fee
          #
          # Prior to #16766, in this situation, the node would immediately report
          # a balance of 30 on node B as unconfirmed and trusted.
          #
          # After #16766, we show that balance as unconfirmed.
          #
          # The balance is indeed "trusted" and "confirmed" insofar as removing
          # the mempool transactions would return at least that much money. But
          # the algorithm after #16766 marks it as unconfirmed because the 'taint'
          # tracking of transaction trust for summing balances doesn't consider
          # which inputs belong to a user. In this case, the change output in
          # question could be "destroyed" by replace the 1st transaction above.
          #
          # The post #16766 behavior is correct; we shouldn't be treating those
          # funds as confirmed. If you want to rely on that specific UTXO existing
          # which has given you that balance, you cannot, as a third party
          # spending the other input would destroy that unconfirmed.
          #
          # For example, if the test transactions were:
          #
          # 1) Sent 40 from node A to node B with fee 0.01
          # 2) Sent 10 from node B to node A with fee 0.01
          #
          # Then our node would report a confirmed balance of 40 + 50 - 10 = 80
          # BTC, which is more than would be available if transaction 1 were
          # replaced.

  The release notes have been updated to note the new behavior.

ACKs for top commit:
  ariard:
    Code Review ACK 4671fc3, maybe extend DoS protection in a follow-up PR.
  fjahr:
    Code review ACK 4671fc3d9e
  ryanofsky:
    Code review ACK 4671fc3d9e. Changes since last review: 2 new commits adding suggested release note and python test comment, also a clean rebase with no changes to the earlier commits. The PR description is more comprehensive now, too. Looks good!
  promag:
    Code review ACK 4671fc3d9e.

Tree-SHA512: 6b183ff425304fef49724290053514cb2770f4a2350dcb83660ef24af5c54f7c4c2c345b0f62bba60eb2d2f70625ee61a7fab76a7f491bb5a84be5c4cc86b92f
2019-11-05 21:59:27 +13:00
Samuel Dobson
bfc4c896d6
Merge #17258: Fix issue with conflicted mempool tx in listsinceblock
436ad43643 Fix issue with conflicted mempool tx in listsinceblock (Adam Jonas)

Pull request description:

  Closes #8752 by bringing back abandoned #10470.

  This now checks that returned transactions are not conflicting with any transactions that are filtered out by the given blockhash and add a functional test to prevent this in the future.

  For more context, #8757 was closed in favor of #10470.

ACKs for top commit:
  instagibbs:
    utACK 436ad43643
  kallewoof:
    utACK 436ad43643
  jonatack:
    I'm not qualifed to give an ACK here but 436ad43643 appears reasonable. Built/ran tests/verified that this test fails without the change in rpcwallet.cpp:

Tree-SHA512: 63d75cd3d3f19fc84dc38899b200c96179b82b24db263cd0116ee5b715265be647157855c2e35912d2fbc49c7b37db9375d6aab0ac672f0f09bece8431de5ea9
2019-11-05 21:56:34 +13:00
MarcoFalke
bc38bb9a60
Merge #17288: Added TestShell class for interactive Python environments.
19139ee034 Add documentation for test_shell submodule (JamesC)
f5112369cf Add TestShell class (James Chiang)
5155602a63 Move argparse() to init() (JamesC)
2ab01462f4 Move assert num_nodes is set into main() (JamesC)
614c645643 Clear TestNode objects after shutdown (JamesC)
6f40820757 Add closing and flushing of logging handlers (JamesC)
6b71241291 Refactor TestFramework main() into setup/shutdown (JamesC)
ede8b7608e Remove network_event_loop instance in close() (JamesC)

Pull request description:

  This PR refactors BitcoinTestFramework to encapsulate setup and shutdown logic into dedicated methods, and adds a  ~~TestWrapper~~ TestShell child class. This wrapper allows the underlying BitcoinTestFramework to run _between user inputs_ in a REPL environment, such as a Jupyter notebook or any interactive Python3 interpreter.

  The ~~TestWrapper~~ TestShell is motivated by the opportunity to expose the test-framework as a prototyping and educational toolkit. Examples of code prototypes enabled by  ~~TestWrapper~~ TestShell can be found in the Optech [Taproot/Schnorr](https://github.com/bitcoinops/taproot-workshop) workshop repository.

  Usage example:
  ```
  >>> import sys
  >>> sys.path.insert(0, "/path/to/bitcoin/test/functional")
  ```
  ```
  >>> from test_framework.test_wrapper import TestShell
  >>> test = TestShell()
  >>> test.setup(num_nodes=2)
  20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Initializing test directory /path/to/bitcoin_func_test_XXXXXXX
  ```
  ```
  >>> test.nodes[0].generate(101)
  >>> test.nodes[0].getblockchaininfo()["blocks"]
  101
  ```
  ```
  >>> test.shutdown()
  20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Stopping nodes
  20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Cleaning up /path/to/bitcoin_func_test_XXXXXXX on exit
  20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Tests successful
  ```

  **Overview of changes to BitcoinTestFramework:**

  - Code moved to `setup()/shutdown()` methods.
  - Argument parsing logic encapsulated by `parse_args` method.
  - Success state moved to `BitcoinTestFramework.success`.

  _During Shutdown_

  - `BitcoinTestFramework` logging handlers are flushed and removed.
  - `BitcoinTestFrameowork.nodes` list is cleared.
  - `NetworkThread.network_event_loop` is reset. (NetworkThread class).

  **Behavioural changes:**
  - Test parameters can now also be set when overriding BitcoinTestFramework.setup() in addition to overriding `set_test_params` method.
  - Potential exceptions raised in BitcoinTestFramework.setup() will be handled in main().

  **Added files:**
  - ~~test_wrapper.py~~ `test_shell.py`
  - ~~test-wrapper.md~~ `test-shell.md`

ACKs for top commit:
  jamesob:
    ACK 19139ee034
  jonatack:
    ACK 19139ee034
  jnewbery:
    Rather than invalidate the three ACKs for a minor nit, can you force push back to 19139ee034 please? I think this PR was ready to merge before your last force push.
  jachiang:
    > Rather than invalidate the three ACKs for a minor nit, can you force push back to [19139ee](19139ee034) please? I think this PR was ready to merge before your last force push.
  jnewbery:
    ACK 19139ee034

Tree-SHA512: 0c24f405f295a8580a9c8f1b9e0182b5d753eb08cc331424616dd50a062fb773d3719db4d08943365b1f42ccb965cc363b4bcc5beae27ac90b3460b349ed46b2
2019-11-04 14:54:14 -05:00
MarcoFalke
33b155f287
Merge #17366: test: Reset global args between test suites
fa07b8beb5 test: Reset global args between test suites (MarcoFalke)

Pull request description:

  Ideally there wouldn't be any globals in Bitcoin Core. However, as we still have globals, they need to be reset between runs of test cases. One way to do this is to run each suite in a different process. `make check` does that. However, `./src/test/test_bitcoin` when run manually or on appveyor is a single process, where all globals are preserved between test cases.

  This leads to hard to debug issues such as https://github.com/bitcoin/bitcoin/pull/15845#pullrequestreview-310852164.

  Fix that by resetting the global arg for each test suite. Note that this wont reset the arg between test cases, as the constructor/destructor is not called for them.

  Addendum: This is not a general fix, only for `-segwitheight`. I don't know if clearing all args can be done with today's argsmanager.  Nor do I know if it makes sense. Maybe we want datadir set to a temp path to not risk accidentally corrupting the default data dir?

ACKs for top commit:
  laanwj:
    ACK fa07b8beb5
  practicalswift:
    ACK fa07b8beb5
  mzumsande:
    ACK fa07b8beb5, I also tested that this fixes the issue in #15845.

Tree-SHA512: 1e30b06f0d2829144a61cc1bc9bdd6a694cbd911afff83dd3ad2a3f15b577fd30acdf9f1469f8cb724d0642ad5d297364fd5a8a2a9c8619a7a71fa9ae2837cdc
2019-11-04 14:20:53 -05:00
James O'Beirne
dcef9a2922 logs: add timing information to FlushStateToDisk() 2019-11-04 14:13:54 -05:00
James O'Beirne
41edaf227a logs: add BCLog::Timer and related macros
Makes logging timing information about a block of code easier.
2019-11-04 14:13:52 -05:00
MarcoFalke
94a26b192f
Merge #17318: replace asserts in RPC code with CHECK_NONFATAL and add linter
c98bd13e67 replace asserts in RPC code with CHECK_NONFATAL and add linter (Adam Jonas)

Pull request description:

  - Replace instances of assert in /rpc files and rpcwallet with CHECK_NONFATAL(condition)
  - Add a linter to prevent future usage of assert being used in RPC code

  ref https://github.com/bitcoin/bitcoin/pull/17192

ACKs for top commit:
  practicalswift:
    ACK c98bd13e67 -- diff looks correct

Tree-SHA512: a16036b6bbcca73a5334665f66e17e1756377d582317568291da1d727fc9cf8c84bac9d9bd099534e1be315345336e5f7b66b93793135155f320dc5862a2d875
2019-11-04 11:33:41 -05:00
MarcoFalke
6cb10c14c6
Merge #17199: test: use default address type (bech32) for wallet_bumpfee tests
8d8e5a79d0 test: use default address type (bech32) for wallet_bumpfee tests (Sebastian Falbesoner)

Pull request description:

  The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads to smaller transaction sizes, needing adaption of some constants in the following test cases:
  - `test_dust_to_fee()`: adaption of dust calculation (p2wpkh spend estimate of 67 is taken from `src/policy/policy.cpp:GetDustThreshold()`)
  - `test_maxtxfee_fails()`: lowering `-maxtxfee` setting to trigger fail

Top commit has no ACKs.

Tree-SHA512: b4163700d56c11955f811bc5fe6edaf7aec69931d7db741c03b055fb518bb9825c031fb931c513b37a1968085cb8c2f263adf664b357aff8ee42795fd0f88d2d
2019-11-04 11:27:48 -05:00
MarcoFalke
8f9df2ed88
Merge #17164: p2p: Avoid allocating memory for addrKnown where we don't need it
b6d2183858 Minor refactoring to remove implied m_addr_relay_peer. (User)
a552e8477c added asserts to check m_addr_known when it's used (User)
090b75c14b p2p: Avoid allocating memory for addrKnown where we don't need it (User)

Pull request description:

  We should allocate memory for addrKnown filter only for those peers which are expected to participate in address relay.

  Currently, we do it for all peers (including SPV and block-relay-only),  which results in extra RAM where it's not needed.

  Upd:
  In future, we would still allow SPVs to ask for addrs, so allocation still will be done by default.
  However, they will be able to opt-out via [this proposal](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-October/017428.html) and then we could save some more memory.
  This PR still saves memory for block-relay-only peers immediately after merging.

Top commit has no ACKs.

Tree-SHA512: e84d93b2615556d466f5ca0e543580fde763911a3bfea3127c493ddfaba8f05c8605cb94ff795d165af542b594400995a2c51338185c298581408687e7812463
2019-11-04 11:17:20 -05:00
MarcoFalke
fa07b8beb5
test: Reset global args between test suites 2019-11-04 10:59:55 -05:00
MarcoFalke
fa2c44c3cc
test: Add ASSERT_DEBUG_LOG to unit test framework 2019-11-04 10:42:33 -05:00
MarcoFalke
fa1936f57b
logging: Add member for arbitrary print callbacks 2019-11-04 10:42:29 -05:00
Igor Cota
aa9b84acee depends: update README.md with working Android targets and API levels 2019-11-04 16:13:08 +01:00
JamesC
19139ee034 Add documentation for test_shell submodule 2019-11-04 16:02:28 +01:00
Wladimir J. van der Laan
bbc9e4133c
Merge #17304: refactor: Move many functions into LegacyScriptPubKeyMan and further separate it from CWallet
152b0a00d8 Refactor: Move nTimeFirstKey accesses out of CWallet (Andrew Chow)
7ef47b88e6 Refactor: Move GetKeypoolSize code out of CWallet (Andrew Chow)
089e17d45c Refactor: Move RewriteDB code out of CWallet (Andrew Chow)
0eac7088ab Refactor: Move SetupGeneration code out of CWallet (Andrew Chow)
f45d12b36c Refactor: Move HavePrivateKeys code out of CWallet::CreateWalletFromFile (Andrew Chow)
8b0d82bb42 Refactor: Move Upgrade code out of CWallet::CreateWalletFromFile (Andrew Chow)
46865ec958 Refactor: Move MarkUnusedAddresses code out of CWallet::AddToWalletIfInvolvingMe (Andrew Chow)
a18edd7b38 Refactor: Move GetMetadata code out of getaddressinfo (Andrew Chow)
9716bbe0f8 Refactor: Move LoadKey LegacyScriptPubKeyMan method definition (Andrew Chow)
67be6b9e21 Refactor: Move SetAddressBookWithDB call out of LegacyScriptPubKeyMan::ImportScriptPubKeys (Andrew Chow)
fc2867fdf5 refactor: Replace UnsetWalletFlagWithDB with UnsetBlankWalletFlag in ScriptPubKeyMan (Andrew Chow)
78e7cbc7ba Refactor: Remove UnsetWalletFlag call from LegacyScriptPubKeyMan::SetHDSeed (Andrew Chow)
0391aba52d Remove SetWalletFlag from WalletStorage (Andrew Chow)
4c5491f99c Refactor: Move SetWalletFlag out of LegacyScriptPubKeyMan::UpgradeKeyMetadata (Andrew Chow)
769acef857 Refactor: Move SetAddressBook call out of LegacyScriptPubKeyMan::GetNewDestination (Andrew Chow)
acedc5b823 Refactor: Add new ScriptPubKeyMan virtual methods (Andrew Chow)
533d8b364f Refactor: Declare LegacyScriptPubKeyMan methods as virtual (Andrew Chow)
b4cb18bce3 MOVEONLY: Reorder LegacyScriptPubKeyMan methods (Andrew Chow)

Pull request description:

  Moves several more key management and metadata functions into LegacyScriptPubKeyMan from CWallet to further separate the two.

  Note to reviewers: All of the `if (auto spk_man = walletInstance->m_spk_man.get()) {` blocks will be replaced with for loops in the next PR so you may see some things in those blocks that don't necessarily make sense with an `if` but will with a `for`.

ACKs for top commit:
  laanwj:
    code review ACK 152b0a00d8
  Sjors:
    re-ACK 152b0a00d8
  promag:
    Code review ACK 152b0a00d8.

Tree-SHA512: ff9872a3ef818922166cb15d72363004ec184e1015a3928a66091bddf48995423602ccd7e55b814de85d25ad7c69058280b1fde2e633570c680dc7d6084b3122
2019-11-04 16:01:42 +01:00
Sebastian Falbesoner
b2ff500fb3 test: add "diamond" unit test to MempoolAncestryTests
Approaches #17271.
If ancestors are represented more than once, check that those are not
overcounted.
2019-11-04 15:05:47 +01:00
MarcoFalke
fba574c908
Merge #17349: Remove redundant copy constructors
fa8919889f bench: Remove redundant copy constructor in mempool_stress (MarcoFalke)
29f8434368 refactor: Remove redundant PSBT copy constructor (Hennadii Stepanov)

Pull request description:

  I fail to see why people add these copy constructors manually without explanation, when the compiler can generate them at least as good automatically with less code.

ACKs for top commit:
  promag:
    ACK fa8919889f.
  hebasto:
    ACK fa8919889f, nit s/constructor/operator/ in commit fa8919889f message, as @promag [mentioned](https://github.com/bitcoin/bitcoin/pull/17349#discussion_r341776389) above.
  jonatack:
    ACK fa8919889f

Tree-SHA512: ce024fdb894328f41037420b881169b8b1b48c87fbae5f432edf371a35c82e77e21468ef97cda6f54d34f1cf9bb010235d62904bb0669793457ed1c3b2a89723
2019-11-04 08:32:22 -05:00
MarcoFalke
def69e5a86
Merge #17233: travis: Run unit and functional tests on native arm
facc0da63a travis: Run unit and functional tests on native arm (MarcoFalke)
fafa064d2a ci: Remove ccache requirement on the host (MarcoFalke)

Pull request description:

  This keeps the cross-compilation to make it easy to run the ci on non-arm hardware. To run this locally in qemu-user as it used to be, just `export QEMU_USER_CMD="qemu-arm -L /usr/arm-linux-gnueabihf/"`.

ACKs for top commit:
  laanwj:
    LGTM ACK facc0da63a
  practicalswift:
    ACK facc0da63a -- diff looks correct and Travis seems happy

Tree-SHA512: 0dc1bc82eb93e2bd8b159e044f20fe3055f8cdfd73aaa238bd2e178397582144dfc0c6a87bd8270115dafea1a623e642bde5d5f30254f94140f1a2cdb12fc2da
2019-11-04 08:23:21 -05:00