Commit graph

18667 commits

Author SHA1 Message Date
Jonas Schnelli
5778d44aa8
Merge #13079: Fix rescanblockchain rpc to properly report progress
16be13345 Fix rescanblockchain rpc to property report progress (Ben Woosley)

Pull request description:

  Previously it assumed tip in all cases. This also extracts a RescanVerificationProgress helper object whose role is to manage reporting, in order to simplify ScanForWalletTransactions - more lines in total, but much simpler to follow the core logic.

Tree-SHA512: 5ebed0c56fae4ccfe613ff1d7082cb6da5a86635a8993ed3af70b500a4ea43074121aea9219b2f0321fbfeb7efcb964bdc2199297a64ca0fa85d9d07aa637d40
2018-05-07 15:03:38 +02:00
Wladimir J. van der Laan
57aae632e2
Merge #13131: Add Windows shutdown handler
ddebde7 Add Windows shutdown handler (Chun Kuan Lee)

Pull request description:

  Exit properly when clicked the red X of Windows Console

Tree-SHA512: f030edd08868390662b42abfa1dc6bd702166c6c19f5b1f8e7482e202451e79fb6f37ea672c26c2eb0d32c367bfca86160fbee624696c53828f280b7070be6a0
2018-05-07 14:33:03 +02:00
practicalswift
d8c4998f31 Fix typos 2018-05-07 14:32:50 +02:00
Wladimir J. van der Laan
bd83704ec6
Merge #13149: Handle unsuccessful fseek(...):s
20ce5af Print a log message if we fail to shrink the debug log file (practicalswift)
29c9bdc Handle unsuccessful fseek(...):s (practicalswift)

Pull request description:

  Handle unsuccessful `fseek(...)`:s.

  **Note to reviewers:** What is the most appropriate course of actions for each of these unsuccessful `fseek(...)`:s?

Tree-SHA512: 5b3d82dbdd15d434d3f08dcb4df62888da4df8541d2586f56a4e529083005f6782c39e10645acd1ec403da83061bbfd8dbf2dddc66e09268d410ad0918c61876
2018-05-07 13:39:19 +02:00
Jonas Schnelli
6a01a50f49
Merge #13180: Fix re-declared scoped enum as unscoped (Causes issues with some compilers)
43f3dec00 Remove enum specifier (to avoid re-declare scoped enum as unscoped) (donaloconnor)

Pull request description:

  MSVC fails to compile with the changes made in #10742

  The problem is enum types were changed to scoped (`enum class`) but in some places `enum` as an unscoped is used.

  This is a very simple fix and I've tested it.

  Edit: Had to remove enum altogether - `enum class` doesn't compile on clang.

Tree-SHA512: 13e21666243585a133c74c81249a1fa4098d6b7aa3cda06be871fa017c0ad9bb7b0725f801160b9d31678448d668718197941fd84702ebdef15128c27d92cd70
2018-05-07 13:28:33 +02:00
Wladimir J. van der Laan
a174702bad
Merge #13162: [net] Don't incorrectly log that REJECT messages are unknown.
fad63eb [logging] Don't incorrectly log that REJECT messages are unknown. (John Newbery)

Pull request description:

  Reject messages are logged to debug.log if NET debug logging is enabled.

  Because of the way the `ProcessMessages()` function is structured,
  processing for REJECT messages will also drop through to the default
  branch and incorrectly log `Unknown command "reject" from peer-?`. Fix
  that by exiting from `ProcessMessages()` early.

  without this PR:
  ```
  2018-05-03T17:37:00.930600Z received: reject (21 bytes) peer=0
  2018-05-03T17:37:00.930620Z Reject message code 16: spammy spam
  2018-05-03T17:37:00.930656Z Unknown command "reject" from peer=0
  ```
  with this PR:
  ```
  2018-05-03T17:35:04.751246Z received: reject (21 bytes) peer=0
  2018-05-03T17:35:04.751274Z Reject message code 16: spammy spam
  ```

Tree-SHA512: 5c84c98433ab99e0db2dd481f9c2db6f87ff0d39022ff317a791737e918714bbcb4a23e81118212ed8e594ebcf098ab7f52f7fd5e21ebc3f07b1efb279b9b30b
2018-05-07 12:49:11 +02:00
Jorge Timón
cda8e36f01
Refactor: RPC: Separate GetBlockChecked() from getblock()
This does not change functionality
2018-05-07 12:22:42 +02:00
donaloconnor
43f3dec009 Remove enum specifier (to avoid re-declare scoped enum as unscoped) 2018-05-06 22:50:39 +01:00
Martin Ankerl
9aac9f90d5 replace modulus with FastMod
Replaces the slow modulo operation with a much faster 32bit multiplication & shift. This works
because the hash should be uniformly distributed between 0 and 2^32-1. This speeds up the benchmark
by a factor of about 1.3:

RollingBloom, 5, 1500000, 3.73733, 4.97569e-07, 4.99002e-07, 4.98372e-07 # before
RollingBloom, 5, 1500000, 2.86842, 3.81630e-07, 3.83730e-07, 3.82473e-07 # FastMod

Be aware that this changes the position of the bits that are toggled, so this should probably
not be used for CBloomFilter which is serialized.
2018-05-06 13:55:33 +02:00
practicalswift
47782b49e6 Add Clang thread safety analysis annotations 2018-05-05 16:51:22 +02:00
practicalswift
0e2dfa8a65 Fix missing locking in CTxMemPool::setSanityCheck(double dFrequency)
* writing variable 'nCheckFrequency' requires holding mutex 'cs'
2018-05-05 16:50:58 +02:00
practicalswift
6bc5b7100b Fix missing locking in CTxMemPool::check(const CCoinsViewCache *pcoins)
* reading variable 'mapTx' requires holding mutex 'cs'
* reading variable 'mapNextTx' requires holding mutex 'cs'
* reading variable 'nCheckFrequency' requires holding mutex 'cs'
2018-05-05 16:50:58 +02:00
MarcoFalke
66cc47be98
Merge #13080: mempool: Add compile time checking for ::mempool.cs runtime locking assertions
cbba1d2da4 Add compile time checking for all ::mempool.cs runtime locking assertions (practicalswift)

Pull request description:

  Add compile time checking for `::mempool.cs` runtime locking assertions.

  This PR is a subset of #12665. The PR was broken up to make reviewing easier.

  The intention is that literally all `EXCLUSIVE_LOCKS_REQUIRED`/`LOCKS_EXCLUDED`:s added in this PR should follow either directly or indirectly from `AssertLockHeld(…)`/`AssertLockNotHeld(…)`:s already existing in the repo.

  Consider the case where function `A(…)` contains `AssertLockHeld(cs_foo)` (without
  first locking `cs_foo` in `A`), and that `B(…)` calls `A(…)` (without first locking `cs_main`):
  * It _directly_ follows that: `A(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
  * It _indirectly_ follows that: `B(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.

Tree-SHA512: 1b5ec1cfca6be67edd1298fea1a52b5572ce833dd4ad05c4583f753c2d3229402663373675df87e950151d5c41aeb3ee02f0ad935ed83fe2f45ca8e4d55d901e
2018-05-05 10:32:32 -04:00
practicalswift
cbba1d2da4 Add compile time checking for all ::mempool.cs runtime locking assertions 2018-05-05 08:11:05 +02:00
Andrew Chow
dd3c07acce Separate HaveKey function that checks whether a key is in a keystore 2018-05-05 00:43:04 -04:00
MarcoFalke
fac1223a56
Cache witness hash in CTransaction 2018-05-04 17:55:12 -04:00
MarcoFalke
faab55fbb1
Make CMutableTransaction constructor explicit
Silently converting to a CMutableTransaction will drop all caches
and should thus be done explicitly
2018-05-04 17:40:52 -04:00
MarcoFalke
364bae5f7a qa: Pad scriptPubKeys to get minimum sized txs 2018-05-05 04:59:55 +08:00
MarcoFalke
f82e1c9482
Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53
2018-05-04 16:42:50 -04:00
Johnson Lau
7485488e90 Policy to reject extremely small transactions
A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. Anything smaller than this have unnecessary malloc overhead and are not relayed/mined.
2018-05-05 04:26:13 +08:00
Johnson Lau
0f8719bb03 Add transaction tests for constant scriptCode
Tests showing that CONST_SCRIPTCODE is applied only to non-segwit transactions
2018-05-05 04:26:13 +08:00
Johnson Lau
9dabfe49c0 Add constant scriptCode policy in non-segwit scripts
This disables OP_CODESEPARATOR in non-segwit scripts (even in an unexecuted branch), and makes a positive FindAndDelete result invalid. This ensures that the scriptCode serialized in SignatureHash() is always the same as the script passing to the EvalScript.
2018-05-05 04:26:12 +08:00
Johnson Lau
2f1a30c63e Fix MAX_STANDARD_TX_WEIGHT check
As suggested by the constant name and its comment in policy.h, a transaction with a weight of exactly MAX_STANDARD_TX_WEIGHT should be allowed
2018-05-05 00:00:28 +08:00
MarcoFalke
627c3762ce
Merge #13165: doc: Mention good first issue list in CONTRIBUTING.md
a508091c59 doc: Mention good first issue list in CONTRIBUTING.md (fanquake)

Pull request description:

Tree-SHA512: a954cda62d6dc8ab78809565be392a1c9f55c543e8d5c1a5a8588c0062e1a11bd4968991646e4aa5bd9b9761b1765fde4ceab8fea7a973148122ff249d728b92
2018-05-04 10:21:22 -04:00
fanquake
a508091c59
doc: Mention good first issue list in CONTRIBUTING.md 2018-05-04 11:23:38 +08:00
practicalswift
c3f34d06be Make it clear which functions that are intended to be translation unit local
Do not share functions that are meant to be translation unit local with
other translation units. Use internal linkage for those consistently.
2018-05-03 21:47:40 +02:00
Pieter Wuille
a53f0feff8 Add some checks for invalid recursion in IsMine 2018-05-03 11:02:01 -07:00
Pieter Wuille
b5802a9f5f Simplify IsMine logic 2018-05-03 11:02:01 -07:00
Pieter Wuille
4e91820531 Make IsMine stop distinguishing solvable/unsolvable 2018-05-03 11:02:01 -07:00
Pieter Wuille
6d714c3419 Make coincontrol use IsSolvable to determine solvability 2018-05-03 11:01:57 -07:00
John Newbery
fad63ebe0f [logging] Don't incorrectly log that REJECT messages are unknown.
Reject messages are logged to debug.log if NET debug logging is enabled.

Because of the way the `ProcessMessages()` function is structured,
processing for REJECT messages will also drop through to the default
branch and incorrectly log `Unknown command "reject" from peer-?`. Fix
that by exiting from `ProcessMessages()` early.

without this PR:
```
2018-05-03T17:37:00.930600Z received: reject (21 bytes) peer=0
2018-05-03T17:37:00.930620Z Reject message code 16: spammy spam
2018-05-03T17:37:00.930656Z Unknown command "reject" from peer=0
```
with this PR:
```
2018-05-03T17:35:04.751246Z received: reject (21 bytes) peer=0
2018-05-03T17:35:04.751274Z Reject message code 16: spammy spam
```
2018-05-03 13:41:03 -04:00
Ben Woosley
16be13345e
Fix rescanblockchain rpc to property report progress
CWallet::ScanForWalletTransactions did not previously take into account
pindexStop when calculating progress.

Renamed progress vars to progress_*.

rescanblockchain is the only rpc that uses this parameter.
2018-05-03 13:18:10 -04:00
Tim Ruffing
264c643809
wallet: Reset BerkeleyDB handle after connection fails
According to the BerkeleyDB docs, the DbEnv handle may not be accessed
after close() has been called. This change ensures that we create a new
handle after close() is called. This avoids a segfault when the first
connection attempt fails and then a second connection attempt tries to
call open() on the already closed DbEnv handle.
2018-05-03 19:15:11 +02:00
practicalswift
20ce5af4c6 Print a log message if we fail to shrink the debug log file 2018-05-03 15:44:04 +02:00
Wladimir J. van der Laan
7eb7076f70
Merge #12639: Reduce cs_main lock in listunspent
a59dac3 refactor: Avoid extra lookups of mapAddressBook in listunspent RPC (João Barbosa)
d76962e rpc: Reduce cs_main lock in listunspent (João Barbosa)

Pull request description:

  On my system, where the wallet has 10000 unspents, the `cs_main` lock duration changed from 191ms to 36ms. The loop that generates the response takes around 155ms. So, the lock duration is reduced to around 20%.

Tree-SHA512: ddaae591f39da59a9d1a8e9ffe773d857687789476f566ca273d310ad531da6dacff80cac69f3334c601c251ac7c5ed4136656c725aa3d611c6bbf734111946e
2018-05-03 15:37:41 +02:00
Wladimir J. van der Laan
b62b437acd
Merge #13148: logging: Fix potential use-after-free in LogPrintStr(...)
0bd4cd3 logging: remove unused return value from LogPrintStr (practicalswift)
76f344d logging: Fix potential use-after-free in LogPrintStr(...) (practicalswift)

Pull request description:

  Fix potential use-after-free in `LogPrintStr(...)`.

  `freopen(…)` frees `m_fileout`.

Tree-SHA512: ceee1f659c10a21525aa648377afeea0a37016339f5269dea54850ba3b475aa316f4931081655717b65f981598fdc9d79a1e79e55f7084c242eeb7bf372bc4b6
2018-05-03 12:53:57 +02:00
Wladimir J. van der Laan
11adab39e6
Merge #13154: Trivial: s/SetBestChain/ChainStateFlushed in comments after #13106
21f5680 Trivial: s/SetBestChain/ChainStateFlushed in comments after #13106 (Russell Yanofsky)

Pull request description:

Tree-SHA512: 373896dd73c3ba202739433b22320a4b7ea6dc80ef84547b4ed72c7ae0d6746b109c8b1879359c43374d1816fe384cc31b24b87879ddefe993887140c2f0df9c
2018-05-03 12:40:08 +02:00
João Barbosa
a59dac35ab refactor: Avoid extra lookups of mapAddressBook in listunspent RPC 2018-05-03 11:32:57 +01:00
João Barbosa
d76962e056 rpc: Reduce cs_main lock in listunspent 2018-05-03 11:31:21 +01:00
Wladimir J. van der Laan
979150bc23
Merge #12729: Get rid of ambiguous OutputType::NONE value
1e46d8a Get rid of ambiguous OutputType::NONE value (Russell Yanofsky)

Pull request description:

  Based on suggestion by @sipa https://github.com/bitcoin/bitcoin/pull/12119#issuecomment-357982763

  After #12119, the NONE output type was overloaded to refer to either an output type that couldn't be parsed, or to an automatic change output mode.  This change drops the NONE enum and uses a simple bool to indicate parse failure, and a new CHANGE_AUTO enum to refer the change output type.

  This change is almost a pure refactoring except it makes RPCs reject empty string ("") address types instead of treating them like they were unset. This simplifies the parsing code a little bit and could prevent RPC usage mistakes. It's noted in the release notes.

  Follows up #12408 by @MarcoFalke

  Followups for future PRs:

  - [ ] Add explicit support for specifying "auto" in `ParseOutputType` as suggested by promag and sipa: https://github.com/bitcoin/bitcoin/pull/12729#issuecomment-374799567 and https://github.com/bitcoin/bitcoin/pull/12729#discussion_r175969481
  - [ ] Add wallet `AddressChangeType` method to complement `TransactionChangeType`:  https://github.com/bitcoin/bitcoin/pull/12729#discussion_r175969618.

Tree-SHA512: 8b08b272bcb177a0a9e556dcd965840a7fe601ef83ca97938b879c9b1a33b5b3f96939e1bceef11ba7c644ac21bfd6c1dbc6ca715cd1da4ace50475240e4ee48
2018-05-03 11:53:30 +02:00
Wladimir J. van der Laan
2afdc29403
Merge #12507: Interrupt rescan on shutdown request
c4fda76 wallet: Interrupt rescan on shutdown request (João Barbosa)

Pull request description:

  Fixes #10987.

  Here are the steps to test the feature:

  1. start bitcoind, generate a couple of transactions and then stop:
  ```
  bitcoind -regtest -printtoconsole
  bitcoin-cli -regtest generate 100
  ```
  2. apply the following patch
  ```diff
  diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
  index 2478d67ce..8f8cea40c 100644
  --- a/src/wallet/wallet.cpp
  +++ b/src/wallet/wallet.cpp
  @@ -1671,6 +1671,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock
           }
           while (pindex && !fAbortRescan && !ShutdownRequested())
           {
  +            MilliSleep(500);
               if (pindex->nHeight % 100 == 0 && dProgressTip - dProgressStart > 0.0) {
                   double gvp = 0;
                   {
  ```
  3. start bitcoind with rescan flag, interrupt with CTRL+C and the output should look like:
  ```
  bitcoind -regtest -printtoconsole -rescan
  ...
  ^C2018-02-22 01:00:55 AddToWallet e8bfb4501b630ad2acb91e88ab0112a779766536d2c564b04faae45ae90e18f7
  2018-02-22 01:00:55 Rescan interrupted by shutdown request at block 5. Progress=1.000000
  2018-02-22 01:00:55  rescan                 1774ms
  2018-02-22 01:00:55 setKeyPool.size() = 1995
  2018-02-22 01:00:55 mapWallet.size() = 10145
  2018-02-22 01:00:55 mapAddressBook.size() = 3
  2018-02-22 01:00:55 Shutdown: In progress...
  2018-02-22 01:00:55 scheduler thread interrupt
  2018-02-22 01:00:55 Shutdown: done
  ```

Tree-SHA512: f9bebe2cdacf0359b6cbfcbc48ac2818a3ae7aa7822ff0c2c0de4ca2fff7c88493380b74a1c5ff2ce1de01fe605b0e5ef3576f124ea9cff8ef25a9e762477b92
2018-05-03 11:27:36 +02:00
John Newbery
5d536619ab [tests] Remove 'account' API from wallet functional tests
Removes usage of account API from the following functional tests:

- wallet_listreceivedby.py
- wallet_basic.py
- wallet_keypool_topup.py
- wallet_txn_clone.py
- wallet_listsinceblock.py
- wallet_import_rescan.py
- wallet_listtransactions.py
- wallet_txn_doublespend.py
2018-05-02 17:53:23 -04:00
Wladimir J. van der Laan
ef006d9284
Merge #12928: qt: Initialize non-static class members that were previously neither initialized where defined nor in constructor
3fdc5fe Make sure initialization occurs in the constructor (practicalswift)
1e7813e Remove redundant initializations from the constructor (practicalswift)
f131872 Initialize non-static class members where they are defined (practicalswift)
73bc1b7 Initialize editStatus and autoCompleter. Previously not initialized where defined or in constructor. (practicalswift)

Pull request description:

  Initialize variables previously neither defined where defined nor in constructor:
  * `editStatus`
  * `autoCompleter`

  Also; initialize non-static class members where they are defined in accordance with developer notes.

Tree-SHA512: 84f0cb87ec8394ed7641bfa0731be2ec72e6a920e00ae206ff89e2e7c960358f603c52878311b24601a33aa7cba6ea4f9a78a8ade88112dea0f41efb08e84e25
2018-05-02 16:49:59 +02:00
Russell Yanofsky
21f5680553 Trivial: s/SetBestChain/ChainStateFlushed in comments after #13106 2018-05-02 10:02:48 -04:00
Wladimir J. van der Laan
ef46c9904b
Merge #13153: Add missing newlines to debug logging
4b75dcf devtools: Make linter check LogPrint calls (MarcoFalke)
ff2ad2d Add missing newlines to LogPrint debug logging (Wladimir J. van der Laan)

Pull request description:

  ~~Don't we have a linter that should catch these?~~

Tree-SHA512: 1a58eca01ded9c1719e943c09447deeb59bb06dba00528cf460eefe857fdf95b42671fbdebc87cdd2f51e931e86942d06587ffd097cbb0d8dd9eb7a0ba17a8f0
2018-05-02 15:56:58 +02:00
MarcoFalke
4b75dcf0ec devtools: Make linter check LogPrint calls 2018-05-02 15:14:37 +02:00
Wladimir J. van der Laan
ff2ad2d569 Add missing newlines to LogPrint debug logging
The linter only checked `LogPrintf`, not `LogPrint`.
Fix the remaining cases.
2018-05-02 15:14:04 +02:00
practicalswift
3fdc5fee18 Make sure initialization occurs in the constructor 2018-05-02 13:56:20 +02:00
Wladimir J. van der Laan
598db389c3
Merge #13106: Simplify semantics of ChainStateFlushed callback
9cb6cdc Simplify semantics of ChainStateFlushed callback (Matt Corallo)
50b6533 scripted-diff: Rename SetBestChain callback ChainStateFlushed (Matt Corallo)

Pull request description:

  Previously, ChainStateFlushed would fire either if a full flush
  completed (which can happen due to memory limits, forced flush, or
  on its own DATABASE_WRITE_INTERVAL timer) *or* on a
  ChainStateFlushed-specific DATABASE_WRITE_INTERVAL timer. This is
  both less clear for clients (as there are no guarantees about a
  flush having actually happened prior to the call), and reults in
  extra flushes not clearly intended by the code. We drop the second
  case, providing a strong guarantee without removing the periodit
  timer-based flushing.

  This is a follow-up to discussion in #11857.

Tree-SHA512: 22ba3a0954d265d28413dbf87040790ca5b439820ee7bbadab14028295ec190de82ce5cd664426c82e58b706dc84278868026fa8d066702eb6e6962c9ace1f8e
2018-05-02 13:11:52 +02:00
João Barbosa
c4fda7672a wallet: Interrupt rescan on shutdown request 2018-05-02 11:58:02 +01:00