Commit graph

1879 commits

Author SHA1 Message Date
Wladimir J. van der Laan
08bb4c3156
Merge #16285: rpc: Improve scantxoutset response and help message
bdd6a4fd5d qa: Check scantxoutset result against gettxoutsetinfo (João Barbosa)
fc0c410d6e rpc: Improve scantxoutset response and help message (João Barbosa)

Pull request description:

  The new response keys `height` and `bestblock` allow the client to know at what point the scan took place.

  The help message now has all the response keys (`result` and `txouts` were missing) and it's improved a bit. Note that `searched_items` key is renamed to `txouts`, considering `scantxoutset` is marked experimental.

ACKs for top commit:
  laanwj:
    ACK bdd6a4fd5d

Tree-SHA512: 6bb7c3464b19857b756b8bc491ab7c58b0d948aad8c005b26ed27c55a1278f5639217e11a315bb505b4f44ebe86f413068c1e539c8a5f7a4007735586cc6443c
2019-09-09 08:08:51 +02:00
João Barbosa
bdd6a4fd5d qa: Check scantxoutset result against gettxoutsetinfo 2019-09-09 00:51:31 +01:00
fanquake
189c19e012
Merge #15759: p2p: Add 2 outbound block-relay-only connections
0ba08020c9 Disconnect peers violating blocks-only mode (Suhas Daftuar)
937eba91e1 doc: improve comments relating to block-relay-only peers (Suhas Daftuar)
430f489027 Don't relay addr messages to block-relay-only peers (Suhas Daftuar)
3a5e885306 Add 2 outbound block-relay-only connections (Suhas Daftuar)
b83f51a4bb Add comment explaining intended use of m_tx_relay (Suhas Daftuar)
e75c39cd42 Check that tx_relay is initialized before access (Suhas Daftuar)
c4aa2ba822 [refactor] Change tx_relay structure to be unique_ptr (Suhas Daftuar)
4de0dbac9b [refactor] Move tx relay state to separate structure (Suhas Daftuar)
26a93bce29 Remove unused variable (Suhas Daftuar)

Pull request description:

  Transaction relay is optimized for a combination of redundancy/robustness as well as bandwidth minimization -- as a result transaction relay leaks information that adversaries can use to infer the network topology.

  Network topology is better kept private for (at least) two reasons:

  (a) Knowledge of the network graph can make it easier to find the source IP of a given transaction.

  (b) Knowledge of the network graph could be used to split a target node or nodes from the honest network (eg by knowing which peers to attack in order to achieve a network split).

  We can eliminate the risks of (b) by separating block relay from transaction relay; inferring network connectivity from the relay of blocks/block headers is much more expensive for an adversary.

  After this commit, bitcoind will make 2 additional outbound connections that are only used for block relay. (In the future, we might consider rotating our transaction-relay peers to help limit the effects of (a).)

ACKs for top commit:
  sipa:
    ACK 0ba08020c9
  ajtowns:
    ACK 0ba08020c9 -- code review, ran tests. ran it on mainnet for a couple of days with MAX_BLOCKS_ONLY_CONNECTIONS upped from 2 to 16 and didn't observe any unexpected behaviour: it disconnected a couple of peers that tried sending inv's, and it successfully did compact block relay with some block relay peers.
  TheBlueMatt:
    re-utACK 0ba08020c9. Pointed out that stats.fRelayTxes was sometimes uninitialized for blocksonly peers (though its not a big deal and only effects RPC), which has since been fixed here. Otherwise changes are pretty trivial so looks good.
  jnewbery:
    utACK 0ba08020c9
  jamesob:
    ACK 0ba08020c9

Tree-SHA512: 4c3629434472c7dd4125253417b1be41967a508c3cfec8af5a34cad685464fbebbb6558f0f8f5c0d4463e3ffa4fa3aabd58247692cb9ab8395f4993078b9bcdf
2019-09-07 17:45:03 +08:00
fanquake
0d20c42a01
Merge #16421: Conservatively accept RBF bumps bumping one tx at the package limits
5ce822efbe Conservatively accept RBF bumps bumping one tx at the package limits (Matt Corallo)

Pull request description:

  Based on #15681, this adds support for some simple cases of RBF inside of large packages. Issue pointed out by sdaftuar in #15681, and this fix (or a broader one) is required ot make #15681 fully useful.

  Accept RBF bumps of single transactions (ie which evict exactly one
  transaction) even when that transaction is a member of a package
  which is currently at the package limit iff the new transaction
  does not add any additional mempool dependencies from the original.

  This could be made a bit looser in the future and still be safe,
  but for now this fixes the case that a transaction which was
  accepted by the carve-out rule will not be directly RBF'able

ACKs for top commit:
  instagibbs:
    re-ACK 5ce822efbe
  ajtowns:
    ACK 5ce822efbe ; GetSizeWithDescendants is only change and makes sense
  sipa:
    Code review ACK 5ce822efbe. I haven't thought hard about the effect on potential DoS issues this policy change may have.

Tree-SHA512: 1cee3bc57393940a30206679eb60c3ec8cb4f4825d27d40d1f062c86bd22542dd5944fa5567601c74c8d9fd425333ed3e686195170925cfc68777e861844bd55
2019-09-07 10:15:43 +08:00
MeshCollider
5e202382a9
Merge #16624: wallet: encapsulate transactions state
442a87cc0a Add a test wallet_reorgsrestore (Antoine Riard)
40ede992d9 Modify wallet tx status if has been reorged out (Antoine Riard)
7e89994133 Remove SyncTransaction for conflicted txn in CWallet::BlockConnected (Antoine Riard)
a31be09bfd Encapsulate tx status in a Confirmation struct (Antoine Riard)

Pull request description:

  While working on #15931, I've tried to rationalize tx state management to ease integration of block height tracking per-wallet tx. We currently rely on a combination of `hashBlock` and `nIndex` with magic value to determine tx confirmation, conflicted or abandoned state. It's hard to reason and error-prone.  To solve that, we encapsulate these fields in a `TxConfirmation` struct and introduce a `TxState` member that we update accordingly at block connection/disconnection.

  Following jnewbery [recommendation](https://github.com/bitcoin/bitcoin/pull/15931#discussion_r312576506), I've taken these changes in its own commit, and open a PR to get them first. It would ease review of aforementioned PR, but above all should ease fixing of long-term issues like :
  * https://github.com/bitcoin/bitcoin/issues/7315 (but maybe we should abandon abandontransaction or relieve it to only free outpoints not track the transaction as abandoned in itself, need its own discussion)
  * https://github.com/bitcoin/bitcoin/issues/8692 where we should cancel conflicted state of transactions chain smoothly
  * `MarkConflicted` in `LoadToWallet` is likely useless if we track conflicts rights at block connection

  Main changes of this PR to get right are tx update in `AddToWallet` and serialization/deserialization logic.

ACKs for top commit:
  meshcollider:
    Light re-Code Review ACK 442a87cc0a
  ryanofsky:
    utACK 442a87cc0a. Changes since last review are switching from `hasChain` to `LockChain` and removing chain lock in `WalletBatch::LoadWallet` that's redundant with the new lock still added in `CWallet::LoadWallet`, and fixing python test race condition.

Tree-SHA512: 029209e006de0240436817204e69e548c5665e2b0721b214510e7aba7eba130a5eab441d3a1ad95bd6426114dd27390492c77bf4560a9610009b32cd0a1f72f7
2019-09-06 01:28:54 +12:00
Wladimir J. van der Laan
cbde2bc806
Merge #16804: test: Remove unused try-block in assert_debug_log
fae91a09c4 test: Remove incorrect and unused try-block in assert_debug_log (MarcoFalke)

Pull request description:

  This try block has accidentally been added by me in fa3e9f7627.
  It was unused all the time, but commit 6011c9d72d added a `return` in the finally block, muting all exceptions.

  This can be tested by adding an `assert False` after any `with ...assert_debug_log...:` line.

ACKs for top commit:
  laanwj:
    ACK fae91a09c4
  ryanofsky:
    utACK fae91a09c4. I didn't know returning inside a `finally` block would cancel pending exceptions or return values, but I guess this makes sense and is a good thing to be aware of.

Tree-SHA512: 47ed0165062060e9af055a3e92f1a529cd41d00476bfad64e3cd141ae084d22f926a343bb1257717e164e15459a59ab66aed198c95d18bf780d8cb0b76aa3298
2019-09-05 13:28:51 +02:00
MarcoFalke
45be44cce4
Merge #15257: Scripts and tools: Bump flake8 to 3.7.8
3d0a82cff8 devtools: Accomodate block-style copyright blocks (Ben Woosley)
0ef0e51fe4 lint: Bump flake8 to 3.7.8 (Ben Woosley)
838920704a lint: Disable flake8 W504 warning (Ben Woosley)
b21680baf5 test/contrib: Fix invalid escapes in regex strings (Ben Woosley)

Pull request description:

  This is a second go at #15221, fixing new lints in:
  W504 line break after binary operator
  W605 invalid escape sequence
  F841 local variable 'e' is assigned to but never used

  This time around:
  * One commit per rule, for easier review
  * I went with the PEP-8 style of breaking before binary operators
  * I looked into the raw regex newline issue, and found that raw strings with newlines embedded do work appropriately. E.g. run `re.match(r" \n ", " \n ")` to check this for yourself. `re.MULTILINE` exists to modify `^` and `$` in multiline scenarios, but  all of these searches are per-line.

ACKs for top commit:
  practicalswift:
    ACK 3d0a82cff8 -- diff looks correct

Tree-SHA512: bea0c144cadd72e4adf2e9a4b4ee0535dd91a8e694206924cf8a389dc9253f364a717edfe9abda88108fbb67fda19b9e823f46822d7303c0aaa72e48909a6105
2019-09-05 02:43:13 +02:00
MarcoFalke
761fe07ba9
Merge #16768: test: Make lint-includes.sh work from any directory
490da639cb Make lint-includes.sh work from any directory (Kristaps Kaupe)

Pull request description:

  Before this change it works from root folder of bitcoin git repo, but if you do `cd test/lint; ./test-includes.sh`, you will have a lot of false positive messages like this:
  ```
  Good job! The circular dependency "chainparamsbase -> util/system -> chainparamsbase" is no longer present.
  Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in ./lint-circular-dependencies.sh
  to make sure this circular dependency is not accidentally reintroduced.

  Good job! The circular dependency "index/txindex -> validation -> index/txindex" is no longer present.
  Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in ./lint-circular-dependencies.sh
  to make sure this circular dependency is not accidentally reintroduced.

  ```

Top commit has no ACKs.

Tree-SHA512: 07fa69cb2883181dcee922191acac4b242722eeb2916cdffdc7163421302b22f3c9525aaf4c754a9dba1c307032c05285e38191d5c6aabc894321f8a27bbceaa
2019-09-05 02:00:44 +02:00
Matt Corallo
5ce822efbe Conservatively accept RBF bumps bumping one tx at the package limits
Accept RBF bumps of single transactions (ie which conflict with one
transaction) even when that transaction is a member of a package
which is currently at the package limit iff the new transaction
does not add any additional mempool dependencies from the original.

This could be made a bit looser in the future and still be safe,
but for now this fixes the case that a transaction which was
accepted by the carve-out rule will not be directly RBF'able.
2019-09-04 15:53:14 -04:00
Kristaps Kaupe
490da639cb
Make lint-includes.sh work from any directory 2019-09-04 22:36:09 +03:00
Suhas Daftuar
0ba08020c9 Disconnect peers violating blocks-only mode
If we set fRelay=false in our VERSION message, and a peer sends an INV or TX
message anyway, disconnect. Since we use fRelay=false to minimize bandwidth,
we should not tolerate remaining connected to a peer violating the protocol.
2019-09-04 14:58:36 -04:00
MarcoFalke
fae91a09c4
test: Remove incorrect and unused try-block in assert_debug_log 2019-09-04 13:10:37 -04:00
Ben Woosley
838920704a
lint: Disable flake8 W504 warning
In the words of MarcoFalke:
"W504 should be disabled. This is not a critical error that should be blocking a merge"
https://github.com/bitcoin/bitcoin/pull/15257#discussion_r302017280
2019-09-03 14:40:56 -04:00
Ben Woosley
b21680baf5
test/contrib: Fix invalid escapes in regex strings
Flagged by flake8 v3.6.0, as W605, plus a few others identified
incidentally, e.g. 59ffecf66cf4d08c4b431e457b083878d66a3fd6.

Note that r"\n" matches to "\n" under re.match/search.
2019-09-03 14:38:38 -04:00
MeshCollider
33f9750b1b
Merge #16185: gettransaction: add an argument to decode the transaction
9965940e35 doc: Add release note for the new gettransaction argument (darosior)
b8b3f0435a tests: Add a new functional test for gettransaction (darosior)
7f3bb247a8 gettransaction: add an argument to decode the transaction (darosior)

Pull request description:

  This PR adds a new parameter to the `gettransaction` call : `decode`. If set to `true`, it will add a new `decoded` field to the response. This mimics the behavior of `getrawtransaction`'s `verbose` argument to avoid using 2 calls if we want to decode a wallet transaction (`gettransaction` then `decoderawtransaction`).

  Fix #16181 .

ACKs for top commit:
  meshcollider:
    re-utACK 9965940e35

Tree-SHA512: bcb6b4bd252b3488d6afc77659c499c2ad99fd58661eb24b6a2e17014c74f22e47fde70e00fedb4f4754915786622ad02483b2cf2c4dea0ab0eb4ac8276dbeee
2019-09-02 23:31:41 +12:00
Kristaps Kaupe
8340763dc3
Check for codespell in lint-spelling.sh
Similar check for spellcheck already exists in lint-shell.sh
2019-08-31 00:00:11 +03:00
darosior
b8b3f0435a
tests: Add a new functional test for gettransaction 2019-08-30 11:40:39 +02:00
Antoine Riard
442a87cc0a Add a test wallet_reorgsrestore
Test we change tx status at loading in case of reorgs while wallet
was shutdown.
2019-08-29 12:01:51 -04:00
Wladimir J. van der Laan
d8fc997913
Merge #16695: rpc: Add window final block height to getchaintxstats
d48c1e837a Add window final block height to getchaintxstats (Jonathan "Duke" Leto)

Pull request description:

  This patch is motivated by the desire to make the output of `getchaintxstats` more useful and optimized for applications to consume and render the data.

  Firstly, this data is already available to the RPC, no additional work is done. Currently additional RPC calls will be needed to look up the height of the final block in the window or the block height that began the window.

  By adding the block height of the final block in the window, the JSON is "self-contained" and applications can calculate the exact block height range of the window with no additional RPC requests.
  For example, a web application which wants to render historical information for `getchaintxstats` RPC on various window sizes might call the RPC with various window lengths, once per day, and store the JSON results somewhere. Because the final block height of each dataset is included, it's no extra work to determine the exact block window range of each JSON response.

ACKs for top commit:
  promag:
    ACK d48c1e837a.

Tree-SHA512: fd4952c125f81a4ad18f7c78498c6b3e265b93cb574832166ac25596321ce84957f971f3f78f37d7e42638dc65f2a5d4d760f289873c9c2f2a82eb00a0f87c3f
2019-08-29 16:41:46 +02:00
Jonathan "Duke" Leto
d48c1e837a Add window final block height to getchaintxstats
The getchaintxstats RPC now returns the additional key of window_final_block_height
2019-08-29 06:24:44 -07:00
MarcoFalke
cc40b55da7
Merge #16726: tests: Avoid common Python default parameter gotcha when mutable dict/list:s are used as default parameter values
e4f4ea47eb lint: Catch use of [] or {} as default parameter values in Python functions (practicalswift)
25dd867150 Avoid using mutable default parameter values (practicalswift)

Pull request description:

  Avoid common Python default parameter gotcha when mutable `dict`/`list`:s are used as default parameter values.

  Examples of this gotcha caught during review:
  * https://github.com/bitcoin/bitcoin/pull/16673#discussion_r317415261
  * https://github.com/bitcoin/bitcoin/pull/14565#discussion_r241942304

  Perhaps surprisingly this is how mutable list and dictionary default parameter values behave in Python:

  ```
  >>> def f(i, j=[], k={}):
  ...     j.append(i)
  ...     k[i] = True
  ...     return j, k
  ...
  >>> f(1)
  ([1], {1: True})
  >>> f(1)
  ([1, 1], {1: True})
  >>> f(2)
  ([1, 1, 2], {1: True, 2: True})
  ```

  In contrast to:

  ```
  >>> def f(i, j=None, k=None):
  ...     if j is None:
  ...         j = []
  ...     if k is None:
  ...         k = {}
  ...     j.append(i)
  ...     k[i] = True
  ...     return j, k
  ...
  >>> f(1)
  ([1], {1: True})
  >>> f(1)
  ([1], {1: True})
  >>> f(2)
  ([2], {2: True})
  ```

  The latter is typically the intended behaviour.

  This PR fixes two instances of this and adds a check guarding against this gotcha going forward :-)

ACKs for top commit:
  Sjors:
    Oh Python... ACK e4f4ea47eb. Testing tip: swap the two commits.

Tree-SHA512: 56e14d24fc866211a20185c9fdb274ed046c3aed2dc0e07699e58b6f9fa3b79f6d0c880fb02d72b7fe5cc5eb7c0ff6da0ead33123344e1a872209370c2e49e3f
2019-08-28 13:34:31 -04:00
MarcoFalke
119e97ae2d
Merge #16740: qa: Relax so that the subscriber is ready before publishing zmq messages
403e372407 qa: Relax so that the subscriber is ready before publishing zmq messages (João Barbosa)

Pull request description:

  Prevents the syndrome "slow joiner" - see http://zguide.zeromq.org/py:all#sockets-and-patterns - by relaxing before publishing messages.

ACKs for top commit:
  MarcoFalke:
    unsigned ACK 403e372407

Tree-SHA512: 0e856accbc450a9b09160bdce5112b2103dc9436cc317d31fb1c9634ebd76823a300a2e727818057fb4d0a615271772ff23e80553a13e9aa1935500de5eeec5f
2019-08-28 13:30:59 -04:00
Sebastian Falbesoner
71e08ab22d test: add executable flag for wallet_watchonly.py 2019-08-28 18:25:00 +02:00
João Barbosa
403e372407 qa: Relax so that the subscriber is ready before publishing zmq messages 2019-08-28 00:42:29 +01:00
MarcoFalke
af4100cd6f
Merge #16404: qa: Test ZMQ notification after chain reorg
abdfc5e89b qa: Test ZMQ notification after chain reorg (João Barbosa)
aa2622a726 qa: Refactor ZMQ test (João Barbosa)
6bc1ff915d doc: Add note regarding ZMQ block notification (João Barbosa)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: b93237adc8c84b3aa72ccc28097090eabcb006cf408083218bebf6fec703bd0de2ded80b6879e77096872e14ba9402a6d3f923b146a54d4c4e41dcb862c3e765
2019-08-26 09:13:46 -04:00
practicalswift
e4f4ea47eb lint: Catch use of [] or {} as default parameter values in Python functions 2019-08-26 10:53:10 +00:00
practicalswift
25dd867150 Avoid using mutable default parameter values 2019-08-26 10:45:25 +00:00
MarcoFalke
6dfa9efa3f
Merge #16656: test: fix rpc_setban.py race
6011c9d72d QA: fix rpc_setban.py race (Jonas Schnelli)

Pull request description:

  The new `rpc_setban.py` test failes regularly on CIs due to a race between injecting the ban and testing the log "on the other side".

  The problem is, that the test immediately after the `addnode` command on node0 checks for the `dropped (banned)` entry on node1 (without giving some time).

  Adding a 2 seconds sleep seems to solve the race (I guess there is no better event-driven delay).

  Example of a failed test: https://bitcoinbuilds.org/index.php?ansilog=bf743910-103f-4b54-9a97-960c471061bd.log#l2906

Top commit has no ACKs.

Tree-SHA512: 680f8ea3e5ddb07e93f824f1aeff4a459e25e6c14715a39fc7670e50506d7cf25925348672c5c2d8ba3e1243ccf5effbc2456bcd094fb96868349f8d26e008f1
2019-08-21 08:22:25 -04:00
Jonas Schnelli
6011c9d72d
QA: fix rpc_setban.py race 2019-08-21 14:18:40 +02:00
MarcoFalke
70b12af87e
Merge #16647: rpc: add weight to getmempoolentry output
17d178fb94 doc: add release-notes for getmempoolentry weight field addition (fanquake)
9c9cc2bd20 qa: Add RPC tests for weight in mempool entry (Daniel Edgecumbe)
54aaa7883c RPC: add weight to mempool entry output (Daniel Edgecumbe)

Pull request description:

  Rebase of #14649 (which itself was a rebase of #11256).

  Squash the two test related commits, and swapped out `size` usage for `vsize`.

  Added a commit with release notes.

ACKs for top commit:
  emilengler:
    Concept ACK 17d178f
  instagibbs:
    utACK 17d178fb94
  meshcollider:
    utACK 17d178fb94

Tree-SHA512: 1d354c9837e0ad0afa40325de9329b9e62688d5eab4d9e1cf9b46d8ae29d08f42d903ab37a41751c2ea8f9034231b21095881b1f5d911cb542b8b06bc85dc7cd
2019-08-20 13:54:42 -04:00
MarcoFalke
83112db129
Merge #15864: Fix datadir handling
ffea41f530 Enable all tests in feature_config_args.py (Hennadii Stepanov)
66f5c17f8a Use CheckDataDirOption() for code uniformity (Hennadii Stepanov)
7e33a18a34 Fix datadir handling in bitcoin-cli (Hennadii Stepanov)
b28dada374 Fix datadir handling in bitcoin-qt (Hennadii Stepanov)
50824093bb Fix datadir handling in bitcoind (Hennadii Stepanov)
740d41ce9f Add CheckDataDirOption() function (Hennadii Stepanov)
c1f325126c Return absolute path early in AbsPathForConfigVal (Hennadii Stepanov)

Pull request description:

  Fix #15240, see: https://github.com/bitcoin/bitcoin/issues/15240#issuecomment-487353760
  Fix #15745
  Fix broken `feature_config_args.py` tests (disabled by MarcoFalke@fabe28a0cdcfa13e0e595a0905e3642a960d3077). All test are enabled now.
  This PR is alternative to #13621.

  User's `$HOME` directory is not touched unnecessarily now.

  ~To make reviewing easier only `bitcoind` code is modified (neither `bitcoin-cli` nor `bitcoin-qt`).~

  Refs:
  - https://github.com/bitcoin/bitcoin/issues/15745#issuecomment-479852569 by **laanwj**
  - #16220

Top commit has no ACKs.

Tree-SHA512: 4a4cda10e0b67c8f374da0c9567003d2b566d948e7f8550fe246868b5794c15010e88ea206009480b9cd2f737f310a15e984f920730448f99a895893bed351df
2019-08-19 10:01:30 -04:00
Wladimir J. van der Laan
27ee0cc5a6
Merge #16646: qa: Run tests with UPnP disabled
b168dd30cf Bugfix: QA: Run tests with UPnP disabled (Luke Dashjr)

Pull request description:

  This replaces #16560 by adding `upnp=0` to `bitcoin.conf` rather than passing it to nodes.

  > Needed for builds configured with --enable-upnp-default

  You can test this change using:
  ```bash
  ./configure --enable-upnp-default && make -j6 && test/functional/test_runner.py feature_config_args.py
  ```

  on master the test will fail without this change.

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

Tree-SHA512: e639dd480dda2cffa19a679018c4bd7e4bd4d0f5e3001d6b407b833e3c166bde98b201063e267b8e45f8a20b0d53ec8bc028bec806b2357f9a7ba314cc4e2d07
2019-08-19 11:22:30 +02:00
Wladimir J. van der Laan
ed9a2a37c1
Merge #16631: net: The default whitelistrelay should be true
3b05f0f70f Reformat p2p_permissions.py (nicolas.dorier)
ce7eac3cb0 [Fix] The default whitelistrelay should be true (nicolas.dorier)

Pull request description:

  I thought `whitelistrelay` default was `false` when it is `true`.

  The root of the issue come from the fact that all references to `DEFAULT_` are not in the scope of this file, so hard coding of default values are used everywhere in `net.cpp`. I think that in a separate PR we should fix that more fundamentally everywhere.

ACKs for top commit:
  promag:
    ACK 3b05f0f70f.
  Sjors:
    re-ACK 3b05f0f70f

Tree-SHA512: f4a75f986fa2adf1a5f1c91605e0d261f7ac5ac8535fb05437d83b8392dbcf5cc1a47d755adcf8ad8dc67a88de28060187200fd3ce06545261a5c7ec0fea831a
2019-08-19 11:20:23 +02:00
Daniel Edgecumbe
9c9cc2bd20
qa: Add RPC tests for weight in mempool entry 2019-08-19 11:45:19 +08:00
Luke Dashjr
b168dd30cf
Bugfix: QA: Run tests with UPnP disabled
Needed for builds configured with --enable-upnp-default
2019-08-19 09:58:43 +08:00
João Barbosa
abdfc5e89b qa: Test ZMQ notification after chain reorg 2019-08-19 01:25:33 +01:00
João Barbosa
aa2622a726 qa: Refactor ZMQ test 2019-08-19 01:25:31 +01:00
Andrew Chow
625534d7b1 Give more errors for specific failure conditions
Some failure conditions implicitly fail by failing some other check.
But the error messages are more helpful if they say explicitly what
actually caused the failure, so add those as failure conditions and
errors.
2019-08-16 19:34:01 -04:00
Andrew Chow
c325f619dd Return an error from descriptor Parse that gives more information about what failed 2019-08-16 19:34:01 -04:00
MeshCollider
7a960ba775
Merge #15986: Add checksum to getdescriptorinfo
26d3fad109 Add unmodified-but-with-checksum to getdescriptorinfo (Pieter Wuille)
104b3a5069 Factor out checksum checking from descriptor parsing (Pieter Wuille)

Pull request description:

ACKs for top commit:
  achow101:
    Code Review ACK 26d3fad109
  meshcollider:
    re-Code Review ACK 26d3fad109
  Sjors:
    ACK 26d3fad109

Tree-SHA512: b7a7f89b64a184927d6f9a0c183a087609983f0c5d5593f78e12db4714e930a4af655db9da4b0c407ea2e24d3b926cef6e1f2a15de502d0d1290a6e046826b99
2019-08-17 09:23:52 +12:00
nicolas.dorier
3b05f0f70f
Reformat p2p_permissions.py 2019-08-17 00:43:37 +09:00
nicolas.dorier
ce7eac3cb0
[Fix] The default whitelistrelay should be true 2019-08-17 00:43:22 +09:00
MarcoFalke
b80cdfec9a
Merge #16618: [Fix] Allow connection of a noban banned peer
d117f4541d Add test for setban (nicolas.dorier)
dc7529abf0 [Fix] Allow connection of a noban banned peer (nicolas.dorier)

Pull request description:

  Reported by @MarcoFalke on https://github.com/bitcoin/bitcoin/pull/16248#discussion_r314026195

  The bug would mean that if the peer connecting to you is banned, but whitelisted without specific permissions, it would not be able to connect to the node.

  The solution is just to move the same line below.

ACKs for top commit:
  Sjors:
    Agree inline is more clear. utACK d117f45
  MarcoFalke:
    ACK d117f4541d

Tree-SHA512: 0fed39acb1e8db67bb0bf4c4de3ad034ae776f38d55bd661f1ae0e1a4c6becaf1824ab46ed8279f2f31df3f4b29ff56461d8b167d3e9cece62cfe58b5a912811
2019-08-16 10:17:25 -04:00
nicolas.dorier
d117f4541d
Add test for setban 2019-08-16 13:40:31 +09:00
fanquake
0d65106dce
Merge #16383: rpcwallet: default include_watchonly to true for watchonly wallets
72eaab073b tests: functional watch-only wallet tests (William Casarin)
72ffbdc579 doc: add release note for include_watchonly default changes (William Casarin)
003a3c73c0 rpcwallet: document include_watchonly default for watchonly wallets (William Casarin)
a50d9e6c0b rpcwallet: default include_watchonly to true for watchonly wallets (William Casarin)

Pull request description:

  Right now it's a bit annoying to deal with watchonly wallets, many rpc commands have an `include_watchonly` argument that needs to be explicitly set.

  Wallets created with `createwallet` can have a `disable_private_keys` parameter, for those wallets we already know that they are watchonly, so there's no reason to have to explicitly ask for it for every command. Instead we check this wallet flag when the `include_watchonly` parameter isn't set.

ACKs for top commit:
  achow101:
    Code review ACK 72eaab073b
  Sjors:
    ACK 72eaab073b
  promag:
    ACK 72eaab073b, code review only, didn't look closely to the test.
  kallewoof:
    ACK 72eaab073b
  fanquake:
    ACK 72eaab073b - I've looked over the changes, they make sense to me. Compiled and ran the tests etc.

Tree-SHA512: d3646b55e97f386594d7efc994f0712f3888475c6a5dc7f131ac9f8c49bf5d4677182b88f42b34152abe1ad101ecadd152b4c20e9d3c1267190db36f77ab8bd7
2019-08-16 11:55:35 +08:00
MarcoFalke
1bf2ff2bf8
Merge #16060: Bury bip9 deployments
e78aaf41f4 [docs] Add release notes for burying bip 9 soft fork deployments (John Newbery)
8319e738f9 [tests] Add coverage for the content of getblockchaininfo.softforks (James O'Beirne)
0328dcdcfc [Consensus] Bury segwit deployment (John Newbery)
1c93b9b31c [Consensus] Bury CSV deployment height (John Newbery)
3862e473f0 [rpc] Tidy up reporting of buried and ongoing softforks (John Newbery)

Pull request description:

  This hardcodes CSV and segwit activation heights, similar to the BIP 90 buried deployments for BIPs 34, 65 and 66.

  CSV and segwit have been active for over 18 months. Hardcoding the activation height is a code simplification, makes it easier to understand segwit activation status, and reduces technical debt.

  This was originally attempted by jl2012 in #11398 and again by me in #12360.

ACKs for top commit:
  ajtowns:
    ACK e78aaf41f4 ; checked diff to previous acked commit, checked tests still work
  ariard:
    ACK e78aaf4, check diff, run the tests again and successfully activated csv/segwit heights on mainnet as expected.
  MarcoFalke:
    ACK e78aaf41f4 (still didn't check if the mainnet block heights are correct, but the code looks good now)

Tree-SHA512: 7e951829106e21a81725f7d3e236eddbb59349189740907bb47e33f5dbf95c43753ac1231f47ae7bee85c8c81b2146afcdfdc11deb1503947f23093a9c399912
2019-08-15 16:02:10 -04:00
MarcoFalke
85883a9f8e
Merge #16443: refactor: have CCoins* data managed under CChainState
582d2cd747 Cover UTXO set access with lock annotations (James O'Beirne)
5693530685 refactor: have CCoins* data managed under CChainState (James O'Beirne)
fae6ab6aed refactor: pcoinsTip -> CChainState::CoinsTip() (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/2019-04-proposal/proposal

  ---

  This change encapsulates UTXO set data within CChainState instances, removing global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set.

  We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy.

  This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations.

ACKs for top commit:
  Sjors:
    reACK 582d2cd747
  MarcoFalke:
    ACK 582d2cd747

Tree-SHA512: ec9d904fe5dca8cd2dc4b7916daa5d8bab30856dd4645987300f905e0a19f9919fce4f9d1ff03eda982943ca73e6e9a746be6cf53b46510de36e8c81a1eafba1
2019-08-15 12:47:15 -04:00
MarcoFalke
8bd5e0af99
Merge #16465: test: Test p2sh-witness and bech32 in wallet_import_rescan
fa3c6575ca lint: Add false positive to python dead code linter (MarcoFalke)
fa25668e1c test: Test p2sh-witness and bech32 in wallet_import_rescan (MarcoFalke)
fa79af2989 test: Replace fragile "rng" with call to random() (MarcoFalke)
fac3dcf7d0 test: Generate one block for each send in wallet_import_rescan (MarcoFalke)

Pull request description:

  This adds test coverage for segwit in the `wallet_import_rescan` test, among other cleanups.

ACKs for top commit:
  jnewbery:
    ACK fa3c6575ca

Tree-SHA512: 877741763c62c1bf9d868864a1e3f0699857e8c028e9fcd65c7eeb73600c22cbe97b7b51093737743d9e87bcb991c1fe1086f673e18765aef0fcfe27951402f0
2019-08-15 10:28:58 -04:00
MarcoFalke
fa3c6575ca
lint: Add false positive to python dead code linter 2019-08-15 08:05:14 -04:00
MarcoFalke
e00501e00c
Merge #16561: tests: Use colors and dots in test_runner.py output only if standard output is a terminal
37f2784952 tests: Use colors and dots in test_runner.py output only if standard output is a terminal -- allows for using the test runner output as input to other programs (practicalswift)

Pull request description:

  Use colors and dots in `test_runner.py` output only if standard output is a terminal -- allows for using the test runner output as input to other programs.

  I found the need for this when parsing `test_runner.py` output while investigating intermittent functional test failures.

  Before:

  ```
  $ test/functional/test_runner.py wallet_hd.py > output 2>&1
  $ less output
  Temporary test directory at /tmp/test_runner_₿_🏃_20190807_074115
  ESC[1mWARNING!ESC[0m There is already a bitcoind process running on this system. Tests may fail unexpectedly due to resource contention!
  Remaining jobs: [wallet_hd.py]
  .......................................^M                                       ^M1/1 - ESC[1mwallet_hd.pyESC[0m passed, Duration: 20 s

  ESC[1mTEST         | STATUS    | DURATION

  ESC[0mESC[0;32mwallet_hd.py | ✓ Passed  | 20 s
  ESC[0mESC[1m
  ALL          | ✓ Passed  | 20 s (accumulated)
  ESC[0mRuntime: 20 s
  ```

  After:

  ```
  $ test/functional/test_runner.py wallet_hd.py > output 2>&1
  $ less output
  Temporary test directory at /tmp/test_runner_₿_🏃_20190807_074244
  1/1 - wallet_hd.py passed, Duration: 20 s
  WARNING! There is already a bitcoind process running on this system. Tests may fail unexpectedly due to resource contention!
  Remaining jobs: [wallet_hd.py]

  TEST         | STATUS    | DURATION

  wallet_hd.py | ✓ Passed  | 20 s

  ALL          | ✓ Passed  | 20 s (accumulated)
  Runtime: 20 s
  ```

ACKs for top commit:
  laanwj:
    ACK 37f2784952

Tree-SHA512: f15d95f9e07de2954c326d63d7a4bcd2971faeaa00386600dec2fb915ec89475aeef1dbc968b2c12aa5e988d4b3ed1974d6da0b6a3f1e1a105cfd90e8cb97cf6
2019-08-15 07:43:01 -04:00