Commit graph

396 commits

Author SHA1 Message Date
MarcoFalke
1b8c88451b
Merge #11376: Ensure backupwallet fails when attempting to backup to source file
5d465e396 Ensure backupwallet fails when attempting to backup to source file (Tomas van der Wansem)

Pull request description:

  Previous behaviour was to destroy the wallet (to zero-length)

  This fixes #11375

Tree-SHA512: bfd1738659b15e3f23b6bbdf55ec12269c62c820bf701daec19500b52bd5845bb5516733c6f76f36197eb155182a8a35dc239ad4de2ef1e59bbb0f124a455759
2017-11-01 12:27:02 -04:00
Wladimir J. van der Laan
cffa5ee132
Merge #11531: Check that new headers are not a descendant of an invalid block (more effeciently)
f3d4adf Make p2p-acceptablock not an extended test (Matt Corallo)
00dcda6 [qa] test that invalid blocks on an invalid chain get a disconnect (Matt Corallo)
015a525 Reject headers building on invalid chains by tracking invalidity (Matt Corallo)
932f118 Accept unrequested blocks with work equal to our tip (Matt Corallo)
3d9c70c Stop always storing blocks from whitelisted peers (Matt Corallo)
3b4ac43 Rewrite p2p-acceptblock in preparation for slight behavior changes (Matt Corallo)

Pull request description:

  @sdaftuar pointed out that the version in #11487 was somewhat DoS-able as someone could feed you a valid chain that forked off the the last checkpoint block and force you to do lots of work just walking backwards across blocks for each new block they gave you. We came up with a few proposals but settled on the one implemented here as likely the simplest without obvious DoS issues. It uses our existing on-load mapBlockIndex walk to make sure everything that descends from an invalid block is marked as such, and then simply caches blocks which we attempted to connect but which were found to be invalid. To avoid DoS issues during IBD, this will need to depend on #11458.

  Includes tests from #11487.

Tree-SHA512: 46aff8332908e122dae72ceb5fe8cd241902c2281a87f58a5fb486bf69d46458d84a096fdcb5f3e8e07fbcf7466232b10c429f4d67855425f11b38ac0bf612e1
2017-11-01 14:42:08 +01:00
Wladimir J. van der Laan
e1f6a2a801
Merge #11565: Make listsinceblock refuse unknown block hash
659b206 Make listsinceblock refuse unknown block hash (Russell Yanofsky)

Pull request description:

  Change suggested by @theuni  who noticed listsinceblock would ignore invalid block hashes causing it to return a completely unfiltered list of transactions.

Tree-SHA512: 3c8fb160265780d1334e856e853ab48e2e18372b8f1fc71ae480c3f45317048cc1fee0055d5c58031981a91b9c2bdbeb8e49a889d04ecba61729ce8109f2ce3f
2017-11-01 14:12:54 +01:00
Matt Corallo
f3d4adfa6f Make p2p-acceptablock not an extended test 2017-10-31 13:51:34 -04:00
Matt Corallo
00dcda60f6 [qa] test that invalid blocks on an invalid chain get a disconnect 2017-10-31 13:51:34 -04:00
Matt Corallo
932f118e6a Accept unrequested blocks with work equal to our tip
This is a simple change that makes our accept requirements the
same as our request requirements, (ever so slightly) further
decoupling our consensus logic from our FindNextBlocksToDownload
logic in net_processing.
2017-10-31 13:36:06 -04:00
Matt Corallo
3b4ac43bc3 Rewrite p2p-acceptblock in preparation for slight behavior changes
Removes checking whitelisted behavior (which will be removed, the
difference in behavior here makes little sense) and no longer
requires that blocks at the same work as our tip be dropped if not
requested (in part because we *do* request those blocks).
2017-10-31 13:36:02 -04:00
Wladimir J. van der Laan
d93fa261f0
Merge #11490: Disconnect from outbound peers with bad headers chains
e065249 Add unit test for outbound peer eviction (Suhas Daftuar)
5a6d00c Permit disconnection of outbound peers on bad/slow chains (Suhas Daftuar)
c60fd71 Disconnecting from bad outbound peers in IBD (Suhas Daftuar)

Pull request description:

  The first commit will disconnect an outbound peer that serves us a headers chain with insufficient work while we're in IBD.

  The second commit introduces a way to disconnect outbound peers whose chains fall out of sync with ours:

  For a given outbound peer, we check whether their best known block (which is known from the blocks they announce to us) has at least as much work as our tip.  If it doesn't, we set a 20 minute timeout, and if we still haven't heard about a block with as much work as our tip had when we set the timeout, then we send a single getheaders message, and wait 2 more minutes.  If after two minutes their best known block has insufficient work, we disconnect that peer.

  We protect 4 of our outbound peers (who provide some "good" headers chains, ie a chain with at least as much work as our tip at some point) from being subject to this logic, to prevent excessive network topology changes as a result of this algorithm, while still ensuring that we have a reasonable number of nodes not known to be on bogus chains.

  We also don't require our peers to be on the same chain as us, to prevent accidental partitioning of the network in the event of a chain split.  Note that if our peers are ever on a more work chain than our tip, then we will download and validate it, and then either reorg to it, or learn of a consensus incompatibility with that peer and disconnect.  This PR is designed to protect against peers that are on a less work chain which we may never try to download and validate.

Tree-SHA512: 2e0169a1dd8a7fb95980573ac4a201924bffdd724c19afcab5efcef076fdbe1f2cec7dc5f5d7e0a6327216f56d3828884f73642e00c8534b56ec2bb4c854a656
2017-10-26 21:53:41 +02:00
Suhas Daftuar
c60fd71a65 Disconnecting from bad outbound peers in IBD
When in IBD, we'd like to use all our outbound peers to help us
sync the chain.  Disconnect any outbound peers whose headers have
insufficient work.
2017-10-26 13:43:53 -04:00
Russell Yanofsky
659b2061c4 Make listsinceblock refuse unknown block hash
Change suggested by Cory Fields <cory-nospam-@coryfields.com> who noticed
listsinceblock would ignore invalid block hashes causing it to return a
completely unfiltered list of transactions.
2017-10-26 07:10:59 -04:00
MarcoFalke
57ee73990f
Merge #11538: qa: Fix race condition failures in replace-by-fee.py, sendheaders.py
6d51eaefe qa: Fix race condition in sendheaders.py (Suhas Daftuar)
c96b2e4f0 qa: Fix replace-by-fee race condition failures (Suhas Daftuar)

Pull request description:

  I think #11407 broke replace-by-fee by introducing a race condition.  I was observing frequent failures of replace-by-fee locally, always with a mempool sync failure (the sync call was added in #11407).

  It appeared to me like there were two causes: sometimes the node would be in IBD and not request the transaction that was relayed; other times the blocks generated in make_utxo wouldn't have relayed quickly enough for the spend of the transaction to be accepted.  I believe I've fixed both potential errors.

  ping @instagibbs

  Edit: I found a race condition in the sendheaders.py test, where if the verack from the python node wasn't processed before the first block in the test was generated, then no block announcement would go out to that peer, breaking the test.  Fixed by adding a sync_with_ping after waiting for verack.

Tree-SHA512: 6ad160966e432c151c1ce6e88ae67e60e47123523bda3755cf7697a00e1a5ba38de8561751826e3d7cf0e492f8c2aec298e1b4de8424ebbaf497f099a1ef1d07
2017-10-23 17:19:26 +02:00
Suhas Daftuar
6d51eaefe9 qa: Fix race condition in sendheaders.py 2017-10-23 08:55:21 -04:00
Suhas Daftuar
c96b2e4f09 qa: Fix replace-by-fee race condition failures 2017-10-20 16:10:12 -04:00
Suhas Daftuar
08fd822771 qa: add test for minchainwork use in acceptblock 2017-10-19 20:38:22 -04:00
Russell Yanofsky
478a89c1ef Avoid opening copied wallet databases simultaneously
Make sure wallet databases have unique fileids. If they don't, throw an error.
BDB caches do not work properly when more than one open database has the same
fileid, because values written to one database may show up in reads to other
databases.

Bitcoin will never create different databases with the same fileid, but users
can create them by manually copying database files.

BDB caching bug was reported by Chris Moore <dooglus@gmail.com>
https://github.com/bitcoin/bitcoin/issues/11429

Fixes #11429
2017-10-19 09:01:43 -04:00
Wladimir J. van der Laan
02ac8c892b
Merge #11452: Improve ZMQ functional test
cc9ee80 Improve ZMQ functional test (João Barbosa)

Pull request description:

  After #11439, this PR only improves:
   - test comments;
   - simplicity by removing *duplicate* tests;
   - also removes duplicate code.

Tree-SHA512: 3636fa9694c827128128742ad31e635d19670c3645aef8e7b1cb46069c21631e8b0db059486a6f6e7eee237a23d93bce6df95190394b5a8dcfce31a49a72d17f
2017-10-18 19:00:05 +02:00
Wladimir J. van der Laan
6759a24eaa
Merge #11472: qa: Make tmpdir option an absolute path, misc cleanup
fafa003 qa: Remove never used return value of sync_with_ping (MarcoFalke)
fa9de37 qa: Make tmpdir option an absolute path (MarcoFalke)

Pull request description:

  This should fix issues with the multiwallet test and its symlinks
  when the tmpdir is a relative path.

  Rather than fixing os.symlink to work with paths relative to a
  directory descriptor, which does not work on Windows, normalize
  the path instead.

Tree-SHA512: 189690f3d065ea2f0f48e06775c86d513d0916c7c86312432e8e16df160e65539e288c2bd53d49a4180735fa940f6fcd52b506ccd7d9815651a9b1a69850dda6
2017-10-18 16:52:48 +02:00
Wladimir J. van der Laan
2ca518deff
Merge #11486: [tests] Add uacomment tests
c5dfa90 [tests] Add uacomment tests (Cristian Mircea Messel)

Pull request description:

  Checks for setting the value, max length and reserved characters

Tree-SHA512: a62e2cf8e455a3cd3987c0855f7bfc49de47504c01263e3573366e3cbff400c5678224773d4f1e4ac684fff34d987994e490a0978c4da05ff2a4bfa972c84723
2017-10-18 15:49:37 +02:00
MarcoFalke
808c84f89d
Merge #11483: Fix importmulti bug when importing an already imported key
a44a21517 Fix importmulti bug when importing an already imported key (Pedro Branco)

Pull request description:

  This PR fixes a bug in `importmulti` RPC call where it returns an invalid response when importing an already imported key.

  Before:
  ```sh
  ❯ bitcoin-cli -regtest importmulti '[{ "keys": ["cNcMUunXhVK1dXJ5riixtpYSxPXZnUAMGS4vpzwChdKmYY3Rz99v"], "scriptPubKey": { "address": "n4YZAf4WE2XF3t4BfeYS2nHAhb8CVx91BR" }, "timestamp": 1507655239 }]'
  [{ "success": true }]

  ❯ bitcoin-cli -regtest importmulti '[{ "keys": ["cNcMUunXhVK1dXJ5riixtpYSxPXZnUAMGS4vpzwChdKmYY3Rz99v"], "scriptPubKey": { "address": "n4YZAf4WE2XF3t4BfeYS2nHAhb8CVx91BR" }, "timestamp": 1507655239 }]' '{ "rescan": false }'
  [ false ]

  ❯ bitcoin-cli -regtest importmulti '[{ "keys": ["cNcMUunXhVK1dXJ5riixtpYSxPXZnUAMGS4vpzwChdKmYY3Rz99v"], "scriptPubKey": { "address": "n4YZAf4WE2XF3t4BfeYS2nHAhb8CVx91BR" }, "timestamp": 1507655239 }]' '{ "rescan": true }'
  error code: -1
  error message:
  JSON value is not a boolean as expected
  ```

  After this fix:
  ```sh
  ❯ bitcoin-cli -rpcuser=u -rpcpassword=p -regtest importmulti '[{ "keys": ["cNcMUunXhVK1dXJ5riixtpYSxPXZnUAMGS4vpzwChdKmYY3Rz99v"], "scriptPubKey": { "address": "n4YZAf4WE2XF3t4BfeYS2nHAhb8CVx91BR" }, "timestamp": 1507655139 }]'
  [{ "success": true }]

  ❯ bitcoin-cli -rpcuser=u -rpcpassword=p -regtest importmulti '[{ "keys": ["cNcMUunXhVK1dXJ5riixtpYSxPXZnUAMGS4vpzwChdKmYY3Rz99v"], "scriptPubKey": { "address": "n4YZAf4WE2XF3t4BfeYS2nHAhb8CVx91BR" }, "timestamp": 1507655139 }]'
  [{ "success": false, "error": { "code": -4, "message": "The wallet already contains the private key for this address or script" } }]
  ```

Tree-SHA512: 4acebdfb7d0ebd7cd48e943b93ed1cec072db1ace5c42b3f5cc225603764b6e804e4b823b0710965826aafc2f0c615c53d5aefcfdb9bc9c379f5221b798a318c
2017-10-17 21:40:54 +02:00
John Newbery
f893085325 [tests] Don't subclass from object for Python 3 2017-10-16 21:46:23 -04:00
John Newbery
8f9e3627ef [tests] authproxy.py: tidy up __init__() 2017-10-16 21:45:49 -04:00
John Newbery
323d8f61e9 [tests] fix flake8 warnings in authproxy.py 2017-10-16 21:45:49 -04:00
John Newbery
fc0176d01e [tests] use python3 for authproxy.py 2017-10-16 21:45:49 -04:00
Jonas Schnelli
8c2de827e9
Merge #7061: [Wallet] Add RPC call "rescanblockchain <startheight> <stopheight>"
7a91ceb5e [QA] Add RPC based rescan test (Jonas Schnelli)
c77170fbd [Wallet] add rescanblockchain <start_height> <stop_height> RPC command (Jonas Schnelli)

Pull request description:

  A RPC rescan command is much more flexible for the following reasons:
  * You can define the start and end-height
  * It can be called during runtime
  * It can work in multiwallet environment

Tree-SHA512: df67177bad6ad1d08e5a621f095564524fa3eb87204c2048ef7265e77013e4b1b29f991708f807002329a507a254f35e79a4ed28a2d18d4b3da7a75d57ce0ea5
2017-10-13 15:23:22 -07:00
MarcoFalke
fafa003970 qa: Remove never used return value of sync_with_ping 2017-10-13 15:29:18 +02:00
MarcoFalke
fa9de370b1 qa: Make tmpdir option an absolute path
This should fix issues with the multiwallet test and symlinks
when the tmpdir is a relative path.

Rather than fixing os.symlink to work with paths relative to a
directory descriptor, which does not work on Windows, normalize
the path instead.
2017-10-13 15:28:56 +02:00
Jonas Schnelli
7a91ceb5e0
[QA] Add RPC based rescan test 2017-10-12 11:59:39 -07:00
Wladimir J. van der Laan
f74459dba6
Merge #11277: Fix uninitialized URI in batch RPC requests
4526d21 Add test for multiwallet batch RPC calls (Russell Yanofsky)
74182f2 Add missing batch rpc calls to python coverage logs (Russell Yanofsky)
505530c Add missing multiwallet rpc calls to python coverage logs (Russell Yanofsky)
9f67646 Make AuthServiceProxy._batch method usable (Russell Yanofsky)
e02007a Limit AuthServiceProxyWrapper.__getattr__ wrapping (Russell Yanofsky)
edafc71 Fix uninitialized URI in batch RPC requests (Russell Yanofsky)

Pull request description:

  This fixes "Wallet file not specified" errors when making batch wallet RPC calls with more than one wallet loaded. This issue was reported by @NicolasDorier in https://github.com/bitcoin/bitcoin/issues/11257

  Request URI is not used for anything except multiwallet request dispatching, so this change has no other effect.

Tree-SHA512: b3907af48a6323f864bb045ee2fa56b604188b835025ef82ba3d81673244c04228d796323cec208a676e7cd578a95ec7c7ba1e84d0158b93844d5dda8f6589b9
2017-10-12 14:54:26 +02:00
Cristian Mircea Messel
c5dfa90aab [tests] Add uacomment tests
Checks for setting the value, max length and reserved characters
2017-10-11 23:19:16 +03:00
Pedro Branco
a44a215177 Fix importmulti bug when importing an already imported key 2017-10-11 17:20:42 +01:00
Wladimir J. van der Laan
364da2c529
Merge #10941: Add blocknotify and walletnotify functional tests
857b32b [tests] Add -walletnotify functional test (João Barbosa)
df18d29 [tests] Add -blocknotify functional test (João Barbosa)
9c72a46 [tests] Tidy up forknotify.py (John Newbery)

Pull request description:

  This patch adds the missing functional tests for `-blocknotify` and `-walletnotify` notifications. The `-alertnotify` test file `forknotify.py` is renamed to `notifications.py` to accommodate the new tests. Credits to @jnewbery for this cleanup and unification.

Tree-SHA512: ee7b28b9f1bd225587efaefa6427c6d080ccb50ead390b23d94717c137a317183b37be00da0b2bffedd4192a363c971dea510d78d29278eb9fa76374f7855c09
2017-10-11 11:26:10 +02:00
Wladimir J. van der Laan
fef65c4f5e
Merge #11113: [net] Ignore getheaders requests for very old side blocks
eff4bd8 [test] P2P functional test for certain fingerprinting protections (Jim Posen)
a2be3b6 [net] Ignore getheaders requests for very old side blocks (Jim Posen)

Pull request description:

  Sending a getheaders message with an empty locator and a stop hash is a request for a single header by hash. The node will respond with headers for blocks not in the main chain as well as those in the main chain. To avoid fingerprinting, the node should, however, ignore requests for headers on side branches that are too old. This replicates the logic that currently exists for `getdata` requests for blocks.

Tree-SHA512: e04ef61e2b73945be6ec5977b3c5680b6dc3667246f8bfb67afae1ecaba900c0b49b18bbbb74869f7a37ef70b6ed99e78ebe0ea0a1569369fad9e447d720ffc4
2017-10-11 10:54:05 +02:00
João Barbosa
cc9ee809ad Improve ZMQ functional test 2017-10-10 23:49:16 +01:00
João Barbosa
857b32b4b2 [tests] Add -walletnotify functional test 2017-10-10 23:46:13 +01:00
João Barbosa
df18d29a02 [tests] Add -blocknotify functional test 2017-10-10 23:46:04 +01:00
John Newbery
9c72a464f8 [tests] Tidy up forknotify.py 2017-10-10 23:44:59 +01:00
Tomas van der Wansem
5d465e3962 Ensure backupwallet fails when attempting to backup to source file
Previous behaviour was to destroy the wallet (to zero-length)
2017-10-10 14:49:47 +02:00
MarcoFalke
c63364610f
Merge #10853: [tests] Fix RPC failure testing (again)
47ba8cf71 scripted-diff: rename assert_raises_jsonrpc to assert_raises_rpc error (John Newbery)
677d893ff [tests] do not allow assert_raises_message to be called with JSONRPCException (John Newbery)
5864e9c16 [tests] remove direct testing on JSONRPCException from individual test cases (John Newbery)

Pull request description:

  I did this a few months ago (here: #9707), but a few new examples have crept back in.

  When testing RPC failures, the test case should always assert the error value and message, to ensure that the failure was for the correct reason. Not doing that can hide bugs in the test code and mean that the test is not testing the correct behaviour.

  RPC failure testing should use the utility function `assert_raises_jsonrpc()` (renamed in the final commit of this PR to `assert_raises_rpc_error()`.

  This PR does the following:
  - changes all remaining instances of tests directly testing on `JSONRPCException` to calls to `assert_raises_jsonrpc()`
  - prevents `assert_raises_message()` from being called with `JSONRPCException`
  - scripted-diff changes `assert_raises_jsonrpc()` to `assert_raises_rpc_error()`

Tree-SHA512: 2cc5e320704ec623a6e5a27d3c2c81cea86b502e29896f03bb5bf92cc36725132c1144410aecdf49e90d4577d512ee467d50d8184e9d5c5d0870bfc931316a5a
2017-10-09 21:00:04 +02:00
MarcoFalke
92eadc3950
Merge #11465: rpc: Update named args documentation for importprivkey
aa57590d7 Update importprivkey named args documentation (Dusty Williams)

Pull request description:

  Addresses issue #11462 by updating the documentation for the importprivkey arguments to the correct names, and updates the functional test importprunedfunds.py to use named arguments when calling importprivkey.

Tree-SHA512: 64e14bf89c8c6eec9c37f6ec0c9fc0012fdb035d9ec32cd652110c75abaa922ec5c7523d6ec5098c8a7b42124159b5e330e070974eb79b8b92816f8d61074523
2017-10-09 19:09:45 +02:00
Wladimir J. van der Laan
3a93270c55
Merge #11367: [rpc] getblockchaininfo: add size_on_disk, prune_target_size
b7dfc6c [rpc] getblockchaininfo: add size_on_disk, prune_target_size, automatic_pruning (Daniel Edgecumbe)

Pull request description:

Tree-SHA512: c255c27d6c922434d203ffdefda0dd3dddbd765b6a9cce5f80f5af5cb0b1c11c8aff6f4d00e96a326701d0bc81aace2f216fd1985675aa979f76c16f564a6cf6
2017-10-09 17:04:18 +02:00
Dusty Williams
aa57590d7c Update importprivkey named args documentation
Fixes #11462. Updated documentation for importprivkey function to use the correct name for the first argument.
Also updates a call to importprivkey to use named args in functional test.
2017-10-09 07:40:42 -04:00
John Newbery
47ba8cf71e scripted-diff: rename assert_raises_jsonrpc to assert_raises_rpc error
-BEGIN VERIFY SCRIPT-
sed -i 's/assert_raises_jsonrpc/assert_raises_rpc_error/g' test/functional/*py test/functional/test_framework/*py
-END VERIFY SCRIPT-
2017-10-05 09:57:19 -04:00
John Newbery
677d893ff7 [tests] do not allow assert_raises_message to be called with JSONRPCException 2017-10-05 09:48:46 -04:00
John Newbery
5864e9c161 [tests] remove direct testing on JSONRPCException from individual test cases 2017-10-05 09:48:46 -04:00
Wladimir J. van der Laan
7f11ef2608
Merge #9937: rpc: Prevent dumpwallet from overwriting files
0cd9273 rpc: Prevent `dumpwallet` from overwriting files (Wladimir J. van der Laan)

Pull request description:

  Prevent arbitrary files from being overwritten by `dumpwallet`. There have been reports that users have overwritten wallet files this way. It may also avoid other security issues.

  Fixes #9934. Adds mention to release notes and adds a test.

Tree-SHA512: 268c98636d40924d793b55a685a0b419bafd834ad369edaec08227ebe26ed4470ddea73008d1c4beb10ea445db1b0bb8e3546ba8fc2d1a411ebd4a0de8ce9120
2017-10-04 15:01:24 +02:00
Russell Yanofsky
4526d21e52 Add test for multiwallet batch RPC calls
Tests bug reported in https://github.com/bitcoin/bitcoin/issues/11257
2017-10-04 03:07:01 -04:00
Russell Yanofsky
74182f235c Add missing batch rpc calls to python coverage logs
Without this change, batch RPC calls are not included in coverage logs.
2017-10-04 03:07:01 -04:00
Russell Yanofsky
505530c6cf Add missing multiwallet rpc calls to python coverage logs
This fixes a bug in coverage logging that's been around since the logging was
introduced.
2017-10-04 03:03:07 -04:00
Russell Yanofsky
9f67646f17 Make AuthServiceProxy._batch method usable
Split off AuthServiceProxy.get_request method to make it easier to batch RPC
requests without duplicating code and remove leading underscore from _batch
method.

This does not change any existing behavior.
2017-10-03 15:25:00 -04:00
Russell Yanofsky
e02007aade Limit AuthServiceProxyWrapper.__getattr__ wrapping
Change AuthServiceProxyWrapper.__getattr__ to only wrap proxied attributes, not
real attributes. This way AuthServiceProxyWrapper can continue logging RPC
calls without complicating other object usages, and special case handling for
the .url property can be dropped.
2017-10-03 15:25:00 -04:00