Commit graph

13655 commits

Author SHA1 Message Date
Wladimir J. van der Laan
935cd6b1ec
Merge #16300: util: Explain why the path is cached
fa69c3e6ca util: Explain why the path is cached (MarcoFalke)

Pull request description:

  The rationale for caching the datadir is given as

  ```
      // This can be called during exceptions by LogPrintf(), so we cache the
      // value so we don't have to do memory allocations after that.
  ```

  Since 8c2d695c4a, the debug log location is actually cached itself in `m_file_path`.

  So explain that the caching is now only used to guard against disk access on each call. (See also #16255)

ACKs for top commit:
  promag:
    ACK fa69c3e6ca.
  laanwj:
    ACK fa69c3e6ca
  ryanofsky:
    utACK fa69c3e6ca. Good cleanup. Previous comment was confusing, and definitely not helpful if outdated.

Tree-SHA512: 02108c90026d6d7c02843aaf59a06b4e1fa63d5d4378bb7760f50767efc340dc94c259bf7afb32fa4d47952b48a4e91798d1e0ddc1b051d770405e078636793a
2019-06-28 13:41:28 +02:00
MarcoFalke
fa69c3e6ca
util: Explain why the path is cached 2019-06-27 14:42:06 -04:00
MarcoFalke
7400135b79
Merge #16278: tests: Remove unused includes
9a841696c1 tests: Reduce compilation time and unneccessary recompiles by removing unused includes in tests (practicalswift)

Pull request description:

  Reduce compilation time and unneccessary recompiles by removing unused includes in tests.

  A subset of #16273 ("refactor: Reduce total compilation time by 2% and avoid unnecessary recompiles by removing unused includes") as requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-505022643.

ACKs for top commit:
  Sjors:
    ACK 9a84169 on macOS 10.14.5 (I rebased on #16289)

Tree-SHA512: bcb6ecffef689a9839bee1a5cb93abe83db1f30819a54226c5630fee456b5a5d187507d06861454adfda939c3556a975113f97662e415cb47fa0327ea4fd09fb
2019-06-27 10:24:18 -04:00
MarcoFalke
3077f11dad
Merge #16289: test: Add missing ECC_Stop() in GUI rpcnestedtests.cpp
f466c4ce84 Add missing ECC_Stop(); in GUI rpcnestedtests.cpp (Jonas Schnelli)

Pull request description:

  Fixes #16288

  Was probably missing in #7783

ACKs for top commit:
  Sjors:
    ACK f466c4c. Tested by comparing `make check` on master and this PR with macOS 10.14.5. I also tried with and without `--enable-debug` / `--without-gui`.
  fanquake:
    ACK f466c4ce84. Tested running `make check` on macOS.

Tree-SHA512: 648e10c2e35bd01fb92e63709169a6c185ac4b62c69af0109d2cd2d7db47e56ae804c788f9a1a1845746f818764799732f9e58e9dbfca3bffeea8f14683c8c7f
2019-06-26 18:29:38 -04:00
practicalswift
9a841696c1 tests: Reduce compilation time and unneccessary recompiles by removing unused includes in tests 2019-06-26 20:37:48 +02:00
MarcoFalke
1b28bca04c
Merge #16287: refactor: remove extra CBlockIndex declaration
9824a0d6e9 Remove extra CBlockIndex declaration (RJ Rybarczyk)

Pull request description:

  Remove duplicate `class CBlockIndex;` declaration.

ACKs for top commit:
  promag:
    ACK 9824a0d. Is this a random finding or you have searched for more similar cases?
  practicalswift:
    utACK 9824a0d6e9
  fanquake:
    ACK 9824a0d6e9

Tree-SHA512: aaf88450f53cb8859778102fe971b1121808819c04e64802e5a5cf47bf1403b42531361c52b097b41b905f9fa1bb7acc82b446cfa659c6ac41d00fab29e114e4
2019-06-26 09:15:57 -04:00
Jonas Schnelli
f466c4ce84
Add missing ECC_Stop(); in GUI rpcnestedtests.cpp 2019-06-26 11:28:07 +02:00
RJ Rybarczyk
9824a0d6e9
Remove extra CBlockIndex declaration 2019-06-25 15:02:34 -04:00
Hennadii Stepanov
d8bd97d5ee
Fix GCC 7.4.0 warning
Warning: enumeral and non-enumeral type in conditional expression.
2019-06-25 20:18:12 +03:00
Wladimir J. van der Laan
332c6134bb
Merge #15894: Remove duplicated "Error: " prefix in logs
f724f31401 Make AbortNode() aware of MSG_NOPREFIX flag (Hennadii Stepanov)
96fd4ee02f Add MSG_NOPREFIX flag for user messages (Hennadii Stepanov)
f0641f274f Prepend the error/warning prefix for GUI messages (Hennadii Stepanov)

Pull request description:

  The "Error" prefix/title is set already in the next functions:
  - `noui_ThreadSafeMessageBox()`2068f089c8/src/noui.cpp (L17)
  - `ThreadSafeMessageBox()`a720a98301/src/qt/bitcoingui.cpp (L1351)

  Currently on master:
  ![Screenshot from 2019-04-25 22-08-54](https://user-images.githubusercontent.com/32963518/56763092-25ee8280-67aa-11e9-86c8-6a029dd9ab08.png)

  With this PR:
  ![Screenshot from 2019-04-25 22-26-36](https://user-images.githubusercontent.com/32963518/56763107-30108100-67aa-11e9-9021-683cbd7e2aaa.png)

ACKs for top commit:
  laanwj:
    concept and code-review ACK f724f31401

Tree-SHA512: 218a179b81cc2ac64239d833c02b4c4d4da9b976728a2dcd645966726c4c660b6f1fe43aa28f33d1cb566785a4329e7f93bf5a502bf202316db79d2ff5fce0f8
2019-06-25 13:32:53 +02:00
Wladimir J. van der Laan
c52776e6ff
Merge #16252: test: Log to debug.log in all unit tests
fabc57e07d test: Log to debug.log in all tests (MarcoFalke)
fa4a04a5a9 test: use common setup in gui tests (MarcoFalke)
fad3d2a624 test: Create data dir in BasicTestingSetup (MarcoFalke)

Pull request description:

  This makes it easier to debug a frozen test or a test that failed. To debug a failed test, remove the line `fs::remove_all(m_path_root);`.

  The pull is done in three commits:
  * Create a datadir for every unit test once (and only once). This requires the `SetDataDir` function to go away.
  * Use the common setup in the gui unit tests. Some of those tests are testing the init sequence, so we'd have to undo some of what the testing setup did.
  * Log to the debug.log in all tests

ACKs for top commit:
  laanwj:
    ACK fabc57e07d

Tree-SHA512: 73444210b88172669e2cd22c2703a1e30e105185d2d5f03decbdedcfd09c64ed208d3716c59c8bebb0e44214cee5c8095e3e995d049e1572ee98f1017e413665
2019-06-25 12:14:31 +02:00
fanquake
21bd6eb782
Merge #16188: net: Document what happens to getdata of unknown type
dddd9270f8 net: Document what happens to getdata of unknonw type (MarcoFalke)

Pull request description:

  Any getdata of unknown type will never be processed and blocks all future messages from a peer. This isn't obviously clear from reading the code, so document it.

Top commit has no ACKs.

Tree-SHA512: 4f8e43bbe6534242facfcfffae28b7a6aa2d228841fa2146a87d494e69f614b0da23cf7a5f3d4367358a7c1981fe2ec196a21c437ae1653f1c7e0351be22598a
2019-06-25 11:12:37 +08:00
MarcoFalke
44e849c35a
Merge #16254: qt: Set AA_EnableHighDpiScaling attribute early
099e4b9ad3 Set AA_EnableHighDpiScaling attribute early (Hennadii Stepanov)

Pull request description:

  Running `bitcoin-qt` compiled against Qt 5.12.4 causes a warning:
  ```
  hebasto@bionic-qt:~/bitcoin$ src/qt/bitcoin-qt
  Attribute Qt::AA_EnableHighDpiScaling must be set before QCoreApplication is created.
  ```

  This PR fixes this issue.

  From Qt docs:
  - [Qt::AA_EnableHighDpiScaling](https://doc.qt.io/qt-5/qt.html#ApplicationAttribute-enum):
  > Enables high-DPI scaling in Qt on supported platforms (see also High DPI Displays). _Supported platforms are X11, Windows and Android._ Enabling makes Qt scale the main (device independent) coordinate system according to display scale factors provided by the operating system. This corresponds to setting the `QT_AUTO_SCREEN​_SCALE_FACTOR` environment variable to 1. This attribute must be set before `QGuiApplication` is constructed. This value was added in Qt 5.6.

  - [QCoreApplication::setAttribute()](https://doc.qt.io/qt-5/qcoreapplication.html#setAttribute)

ACKs for commit 099e4b:
  MarcoFalke:
    ACK 099e4b9ad3
  jonasschnelli:
    utACK 099e4b9ad3
  fanquake:
    ACK 099e4b9ad3. Did some testing on `Bionic` and `Windows 10` (using VirtualBox). I couldn't see any obvious visual difference, but given Marco's screens above, this change is obviously better. I also checked that there wasn't any sort of regression on macOS.

Tree-SHA512: 1965a427ee14ffb3871bac317685032406cf02d1fa2b2dc11c8b643bfe4ba09195674d149d1e41752f14c0d000446b35e142f3ce60d987ba97082fd7ee39a094
2019-06-24 08:58:30 -04:00
fanquake
c8fee6769a
Merge #16263: qt: Use qInfo() if no error occurs
a2aabfb749 Use qInfo() if no error occurs (Hennadii Stepanov)

Pull request description:

  [Warning and Debugging Messages](https://doc.qt.io/qt-5/debug.html#warning-and-debugging-messages):
  > - `qInfo()` is used for informational messages.
  > - `qWarning()` is used to report warnings and recoverable errors in your
  application.
  >
  > If the `QT_FATAL_WARNINGS` environment variable is set, `qWarning()` exits after printing the warning message. This makes it easy to obtain a backtrace in the debugger.

  [`qWarning()`](https://doc.qt.io/qt-5/qtglobal.html#qWarning):
  > Calls the message handler with the warning message message... This function does nothing if `QT_NO_WARNING_OUTPUT` was defined during compilation; it exits if at the nth warning corresponding to the counter in environment variable `QT_FATAL_WARNINGS`.

  This PR allows more productive debugging using the environment variable `QT_FATAL_WARNINGS`.

  Examples:
  - https://github.com/bitcoin/bitcoin/pull/16118#issuecomment-503184695
  - https://github.com/bitcoin/bitcoin/pull/16254#issuecomment-504223404

  The behavior, when option `-debug=qt` is set/unset, remains unchanged.

ACKs for commit a2aabf:
  promag:
    ACK a2aabfb, I also have this change locally.
  Empact:
    ACK a2aabfb749
  laanwj:
    ACK a2aabfb749
  fanquake:
    ACK a2aabfb749.

Tree-SHA512: b4df300c9c00a1705b0d3a10227e3deaac19a98b0a898bb60d5a88872cf450fb131eba150d9dd6c29e021566ee04b3b86b7d486bbe28bd894743c128d2309155
2019-06-24 09:28:14 +08:00
fanquake
c1bab5052a
Merge #16231: gui: Fix open wallet menu initialization order
5224be5a33 gui: Fix open wallet menu initialization order (João Barbosa)

Pull request description:

  Fixes #16230, the menu must be created before connecting to aboutToShow signal.

ACKs for commit 5224be:
  hebasto:
    ACK 5224be5a33, I have tested the code on Bionic with Qt 5.12.4.
  ryanofsky:
    utACK 5224be5a33. Looks good, fix is simple and makes perfect sense after seeing explanation in https://github.com/bitcoin/bitcoin/pull/16118#issuecomment-503166407. Without this change (and since #16118), the menu pointer passed to `connect(m_open_wallet_action->menu(), ...)` is null and connecting has no effect. With this change, the menu is constructed earlier so the connect call can work.
  fanquake:
    ACK 5224be5a33 Testing included in a comment above. The segfaulting with QT_FATAL_WARNINGS is unrelated to this change.

Tree-SHA512: 97b42493b37b96683058bccf39a0ee93589293d4ba8f0c60aef7f4fb9dd084cc6d5608cd5ef531cadf5e03b1f01627ef96bc2d79f784fb38cb87aa6643183d41
2019-06-23 18:57:27 +08:00
MeshCollider
2cbcc55ba6
Merge #16239: wallet/rpc: follow-up clean-up/fixes to avoid_reuse
71d0344cf2 docs: release note wording (Karl-Johan Alm)
3d2ff37913 wallet/rpc: use static help text (Karl-Johan Alm)
53c3c1ea9e wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm)

Pull request description:

  This addresses a few remaining issues pointed out in #13756:

  * First commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r284907468
  * Second commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r294868973

  Ping jnewbery and achow101 as they pointed out these issues.

ACKs for commit 71d034:
  jnewbery:
    ACK 71d0344cf2
  meshcollider:
    re-utACK 71d0344cf2

Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
2019-06-22 22:00:10 +12:00
Karl-Johan Alm
3d2ff37913
wallet/rpc: use static help text
Always show the same help topic regardless of wallet flags, and explain that something is not always available, rather than runtime-modifying the help output.
2019-06-22 02:45:40 +09:00
Karl-Johan Alm
53c3c1ea9e
wallet/rpc/getbalances: add entry for 'mine.used' balance in results 2019-06-22 02:45:40 +09:00
Hennadii Stepanov
a2aabfb749
Use qInfo() if no error occurs
qWarning() is used to report warnings and recoverable errors in your 
application.
qInfo() is used for informational messages (since Qt 5.5).
2019-06-21 20:22:13 +03:00
MeshCollider
fd333e15a5
Merge #16226: Move ismine to the wallet module
e61de6306f Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2000 Move ismine to wallet module (Andrew Chow)

Pull request description:

  `IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.

  This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.

ACKs for commit e61de6:
  MarcoFalke:
    re-ACK e61de6306f (only change is rebase with git auto-merge)
  meshcollider:
    Very light code review ACK e61de6306f

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341
2019-06-21 19:59:48 +12:00
MeshCollider
303ec103ba
Merge #16026: Ensure that uncompressed public keys in a multisig always returns a legacy address
a49503402b Make and get the multisig redeemscript and destination in one function instead of two (Andrew Chow)

Pull request description:

  `CreateMultisigRedeemscript()` is changed to `AddAndGetMultisigDestination()` so that the process of constructing the redeemScript and then getting the `CTxDestination` are done in the same function. This allows that function to see what the keys in the multisig are so that the correct address type is returned from `AddAndGetDestinationForScript()`.

  This only effects the `createmultisig` and `addmultisigaddress` RPCs and does not change signing logic as #16022 does.

  Alternative to #16022 and #16012

  Fixes #16011

ACKs for commit a49503:

Tree-SHA512: 5b0154a714deea3b2cc3a54beb420c95eeeacf4ca30c40ca80940d9d640f8b03611b0fc14c2f0710bfd8a79e8d27ad7d9ae380b4b83d52b40ab201624f2a63f0
2019-06-21 19:44:08 +12:00
Hennadii Stepanov
099e4b9ad3
Set AA_EnableHighDpiScaling attribute early
Qt docs: This attribute must be set before QGuiApplication is 
constructed.
2019-06-20 21:24:22 +03:00
MarcoFalke
fabc57e07d
test: Log to debug.log in all tests 2019-06-20 12:12:24 -04:00
Andrew Chow
a49503402b Make and get the multisig redeemscript and destination in one function instead of two
Instead of creating a redeemScript with CreateMultisigRedeemscript and
then getting the destination with AddAndGetDestinationForScript, do
both in the same function.

CreateMultisigRedeemscript is changed to AddAndGetMultisigDestination.
It creates the redeemScript and returns it via an output parameter. Then
it calls AddAndGetDestinationForScript to add the destination to the
keystore and get the proper destination.

This allows us to inspect the public keys in the redeemScript before creating
the destination so that the correct destination is used when uncompressed
pubkeys are in the multisig.
2019-06-20 11:02:00 -04:00
MarcoFalke
dddd9270f8
net: Document what happens to getdata of unknonw type 2019-06-20 10:49:26 -04:00
MarcoFalke
fa4a04a5a9
test: use common setup in gui tests 2019-06-20 09:31:07 -04:00
MarcoFalke
fad3d2a624
test: Create data dir in BasicTestingSetup 2019-06-20 09:31:02 -04:00
Andrew Chow
e61de6306f Change ismine to take a CWallet instead of CKeyStore 2019-06-19 18:06:30 -04:00
Andrew Chow
7c611e2000 Move ismine to wallet module 2019-06-19 18:06:30 -04:00
Hennadii Stepanov
f724f31401
Make AbortNode() aware of MSG_NOPREFIX flag 2019-06-19 19:22:34 +03:00
Hennadii Stepanov
96fd4ee02f
Add MSG_NOPREFIX flag for user messages
It forces do not prepend error/warning prefix.
2019-06-19 19:22:34 +03:00
Hennadii Stepanov
f0641f274f
Prepend the error/warning prefix for GUI messages 2019-06-19 19:20:22 +03:00
MeshCollider
44d8172323
Merge #13756: wallet: "avoid_reuse" wallet flag for improved privacy
5ebc6b0eb2 bitcoind: update -avoidpartialspends description to account for auto-enable for avoid_reuse wallets (Karl-Johan Alm)
ada258f8c8 doc: release notes for avoid_reuse (Karl-Johan Alm)
27669551da wallet: enable avoid_partial_spends by default if avoid_reuse is set (Karl-Johan Alm)
8f2e208f7c test: add test for avoidreuse feature (Karl-Johan Alm)
0bdfbd34cf wallet/rpc: add 'avoid_reuse' option to RPC commands (Karl-Johan Alm)
f904723e0d wallet/rpc: add setwalletflag RPC and MUTABLE_WALLET_FLAGS (Karl-Johan Alm)
8247a0da3a wallet: enable avoid_reuse feature (Karl-Johan Alm)
eec15662fa wallet: avoid reuse flags (Karl-Johan Alm)
58928098c2 wallet: make IsWalletFlagSet() const (Karl-Johan Alm)
129a5bafd9 wallet: rename g_known_wallet_flags constant to KNOWN_WALLET_FLAGS (Karl-Johan Alm)

Pull request description:

  Add a new wallet flag called `avoid_reuse` which, when enabled, will keep track of when a specific destination has been spent from, and will actively "blacklist" any new UTXOs which send to an already-spent-from destination.

  This improves privacy, as a payer could otherwise begin tracking a payee's wallet by regularly peppering a known UTXO with dust outputs, which would then be scooped up and used in payments by the payee, allowing the payer to map out (1) the inputs owned by the payee and (2) the destinations to which the payee is making payments.

  This replaces #10386 and together with the (now merged) #12257 it addresses #10065 in full. The concerns raised in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381 are also addressed due to #12257.

  ~~Note: this builds on top of #15780.~~ (merged)

ACKs for commit 5ebc6b:
  jnewbery:
    ACK 5ebc6b0eb
  laanwj:
    Concept and code-review ACK 5ebc6b0eb2
  meshcollider:
    Code review ACK 5ebc6b0eb2
  achow101:
    ACK 5ebc6b0eb2 modulo above nits

Tree-SHA512: fdef45826af544cbbb45634ac367852cc467ec87081d86d08b53ca849e588617e9a0a255b7e7bb28692d15332de58d6c3d274ac003355220e4213d7d9070742e
2019-06-19 11:33:03 +12:00
MarcoFalke
0b68fca700
Merge #16092: Don't use global (external) symbols for symbols that are used in only one translation unit
0959d37e3e Don't use global (external) symbols for symbols that are used in only one translation unit (practicalswift)

Pull request description:

  Don't use global (external) symbols for symbols that are used in only one translation unit.

  Before:

  ```
  $ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do
        REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h")
        N_REFERENCES=$(wc -l <<< "${REFERENCES}")
        if [[ ${N_REFERENCES} > 1 ]]; then
            continue
        fi
        echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}"
    done
  Global symbol g_chainstate is used in only one translation unit: src/validation.cpp
  Global symbol g_ui_signals is used in only one translation unit: src/ui_interface.cpp
  Global symbol instance_of_cmaincleanup is used in only one translation unit: src/validation.cpp
  Global symbol instance_of_cnetcleanup is used in only one translation unit: src/net.cpp
  Global symbol instance_of_cnetprocessingcleanup is used in only one translation unit: src/net_processing.cpp
  Global symbol pindexBestForkBase is used in only one translation unit: src/validation.cpp
  Global symbol pindexBestForkTip is used in only one translation unit: src/validation.cpp
  $
  ```

  After:

  ```
  $ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do
        REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h")
        N_REFERENCES=$(wc -l <<< "${REFERENCES}")
        if [[ ${N_REFERENCES} > 1 ]]; then
            continue
        fi
        echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}"
    done
  $
  ```

  ♻️ Think about future generations: save the global namespace from unnecessary pollution!  ♻️

ACKs for commit 0959d3:
  Empact:
    ACK 0959d37e3e
  MarcoFalke:
    ACK 0959d37e3e
  hebasto:
    ACK 0959d37e3e
  promag:
    ACK 0959d37.

Tree-SHA512: 722f66bb50450f19b57e8a8fbe949f30cd651eb8564e5787cbb772a539bf3a288c048dc49e655fd73ece6a46f6dafade515ec4004729bf2b3ab83117b7c5d153
2019-06-18 15:59:53 -04:00
MarcoFalke
0853d8d2fd
Merge #16112: util: Log early messages
faa2a47cd7 logging: Add threadsafety comments (MarcoFalke)
0b282f9b00 Log early messages with -printtoconsole (Anthony Towns)
412987430c Replace OpenDebugLog() with StartLogging() (Anthony Towns)

Pull request description:

  Early log messages are dropped on the floor and they'd never make it to the console or debug log. This can be tested by running the test included in this pull request without re-compiling the `bitcoind`.

  Fix that by buffering early messages and flushing them as soon as all logging options have been initialized and logging has been started.

  This pull request is identical to  "Log early messages with -printtoconsole" (#13088)  by **ajtowns**, with the following changes:
  * Rebased
  * Added docstrings for `m_buffering` and `StartLogging`
  * Switch `CCriticalSection` (aka `RecursiveMutex`) to just `Mutex` in the last commit
  * Added tests

  Fixes #16098
  Fixes #13157
  Closes #13088

ACKs for commit faa2a4:
  ajtowns:
    utACK faa2a47cd7
  hebasto:
    ACK faa2a47cd7
  kristapsk:
    ACK faa2a47cd7 (ran added functional test before / after recompiling, didn't do additional testing)

Tree-SHA512: 685e2882642fe2a43ce171d42862582dadb840d03cda8236a994322c389ca2a1f3f431b179b2726c155c61793543bb340c568a5455d97f8b83bc7d307a85d387
2019-06-18 12:32:32 -04:00
Wladimir J. van der Laan
6c9d3c704f
Merge #15651: torcontrol: Use the default/standard network port for Tor hidden services, even if the internal port is set differently
8a2656702b torcontrol: Use the default/standard network port for Tor hidden services, even if the internal port is set differently (Luke Dashjr)

Pull request description:

  Currently, the hidden service is published on the same port as the public listening port.
  But if a non-standard port is configured, this can be used to guess (pretty reliably) that the public IP and the hidden service are the same node.

ACKs for top commit:
  practicalswift:
    utACK 8a2656702b
  naumenkogs:
    utACK 8a26567
  laanwj:
    utACK 8a2656702b

Tree-SHA512: 737c8da4f7c3f0bb22a338647d357987f5808156e3f38864168d0d8c2e2b171160812f7da4de11eef602902b304e357d76052950b72d7b3b83535b0fdd05fadc
2019-06-18 17:28:44 +02:00
Wladimir J. van der Laan
8777a80706
Merge #12324: speed up Unserialize_impl for prevector
86b47fa741 speed up Unserialize_impl for prevector (Akio Nakamura)

Pull request description:

  The unserializer for prevector uses `resize()` for reserve the area, but it's prefer to use `reserve()` because `resize()` have overhead to call its constructor many times.

  However, `reserve()` does not change the value of `_size` (a private member of prevector).

  This PR make the logic of read from stream to callback function, and prevector handles initilizing new values with that call-back and ajust the value of `_size`.

  The changes are as follows:
  1. prevector.h
  Add a public member function named 'append'.
  This function has 2 params, number of elemenst to append and call-back function that initilizing new appended values.

  2. serialize.h
  In the following two function:
  - `Unserialize_impl(Stream& is, prevector<N, T>& v, const unsigned char&)`
  - `Unserialize_impl(Stream& is, prevector<N, T>& v, const V&)`
  Make a callback function from each original logic of reading values from stream, and call prevector's `append()`.

  3. test/prevector_tests.cpp
  Add a test for `append()`.

  ## A benchmark result is following:
  [Machine]
  MacBook Pro (macOS 10.13.3/i7 2.2GHz/mem 16GB/SSD)

  [result]
  DeserializeAndCheckBlockTest  => 22% faster
  DeserializeBlockTest => 29% faster

  [before PR]
      # Benchmark, evals, iterations, total, min, max, median
      DeserializeAndCheckBlockTest, 60, 160, 94.4901, 0.0094644, 0.0104715, 0.0098339
      DeserializeBlockTest, 60, 130, 65.0964, 0.00800362, 0.00895134, 0.00824187

  [After PR]
      # Benchmark, evals, iterations, total, min, max, median
      DeserializeAndCheckBlockTest, 60, 160, 77.1597, 0.00767013, 0.00858959, 0.00805757
      DeserializeBlockTest, 60, 130, 49.9443, 0.00613926, 0.00691187, 0.00635527

ACKs for top commit:
  laanwj:
    utACK 86b47fa741

Tree-SHA512: 62ea121ccd45a306fefc67485a1b03a853435af762607dae2426a87b15a3033d802c8556e1923727ddd1023a1837d0e5f6720c2c77b38196907e750e15fbb902
2019-06-18 17:12:02 +02:00
MarcoFalke
e2182b02b5
Merge #16171: Remove -mempoolreplacement to prevent needless block prop slowness.
8053e5cdad Remove -mempoolreplacement to prevent needless block prop slowness. (Matt Corallo)

Pull request description:

  At this point there is no reasonable excuse to disable opt-in RBF,
  and, unlike when this option was added, there are now significant
  issues created when disabling it (in the form of compact block
  reconstruction failures). Further, it breaks a lot of modern wallet
  behavior.

  This removes an option that is:

  * (a) only useful when a large portion of (other) miners enforce it as well
  * (b) is detrimental to everyone (income for miners, RBF notifications for others) who uses it individually otherwise
  * (c) is effectively unused
  * (d) is often confused with disabling RBF (rather than just remaining stubbornly unaware of it while the rest of the network lets it through)

ACKs for commit 8053e5:
  practicalswift:
    utACK 8053e5cdad
  promag:
    Deprecation would save from unlikely rantings, still ACK 8053e5c.
  jtimon:
    utACK 8053e5cdad
  ajtowns:
    ACK 8053e5cdad -- quick code review, checked tests work
  MarcoFalke:
    ACK 8053e5cdad

Tree-SHA512: 01aee8905b2487fc38a3a86649d422d2d2345bc60f878889ebda4b8680783e1f1a97c2000c27ef086719501be2abc2911b2039a259a5e5c04f3b24ff02b0427e
2019-06-18 10:04:14 -04:00
João Barbosa
5224be5a33 gui: Fix open wallet menu initialization order
The menu must be created before connecting to aboutToShow signal.
2019-06-18 14:13:04 +01:00
MeshCollider
22b6c4ed75
Merge #15899: rpc: Document iswitness flag and fix bug in converttopsbt
fa499b5f02 rpc: bugfix: Properly use iswitness in converttopsbt (MarcoFalke)
fa5c5cd141 rpc: Switch touched RPCs to IsValidNumArgs (MarcoFalke)

Pull request description:

  When a serialized transaction has inputs, there is no risk in only trying to deserialize it with witness allowed. (This is how all transactions from p2p are deserialized.) In fact, it would avoid a common issue where a transaction with inputs can be deserialized in two ways:
  * Fixes #12989
  * Fixes #15872
  * Fixes #15701
  * Fixes #13738
  * ...

  When a serialized transaction has no inputs, there is no risk in only trying to deserialze it with witness disallowed. (A transaction without inputs can't have corresponding witness data)

ACKs for commit fa499b:
  meshcollider:
    utACK fa499b5f02
  ryanofsky:
    utACK fa499b5f02. Changes since last review: consolidating commits and making iswitness documentation the same across methods.
  PastaPastaPasta:
    utACK fa499b5f02

Tree-SHA512: a64423a3131f3f0222a40da557c8b590c9ff01b45bcd40796f77a1a64ae74c6680a6be9d01ece95c492dfbcc7e2810409d2c2b336c2894af00bb213972fc85c6
2019-06-19 00:52:39 +12:00
practicalswift
f8995807e4 tests: Make coins_tests/updatecoins_simulation_test deterministic 2019-06-17 19:13:30 +02:00
MarcoFalke
91958d66cb
Merge #16210: rpc: add 2nd arg to signrawtransactionwithkey examples
71fd628ada Add example 2nd arg to signrawtransactionwithkey (Chris Moore)

Pull request description:

  The RPC examples for signrawtransactionwithkey are missing the 2nd parameter.

  Before this change the help text showed:

      Examples:
      > bitcoin-cli signrawtransactionwithkey "myhex"
      > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "signrawtransactionwithkey", "params": ["myhex"] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

  With the change, it shows:

      Examples:
      > bitcoin-cli signrawtransactionwithkey "myhex" "[\"key1\",\"key2\"]"
      > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "signrawtransactionwithkey", "params": ["myhex", "[\"key1\",\"key2\"]"] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

ACKs for commit 71fd62:

Tree-SHA512: dadf6bf0ba64ac356b7b8f9ed4d483384b70080ac4b1664b27a2e72b97f25d7266f3dae89fbeade73c1bae802b5bae7b84d596c93a9ae9c748851ae35758d9a6
2019-06-17 13:02:47 -04:00
MarcoFalke
fce4123242
Merge #16217: getrawtransaction: inform about blockhash argument when lookup fails
c59e3a3261 getrawtransaction: inform about blockhash argument when lookup fails (darosior)

Pull request description:

  Just 4 words added on `getrawtransaction` lookup error to fix #16142

ACKs for commit c59e3a:

Tree-SHA512: 2219099c1240667527a9b1498a58818b5ff1c2ef366c498d2bb57963e828b3c87fa3e6b94be7e6463bd289ceabc13f9c9b1082134641594ba335ac400e6d63aa
2019-06-17 10:15:18 -04:00
MarcoFalke
1a274bce4b
Merge #16205: Refactor: Replace fprintf with tfm::format
fa8f195195 Replace remaining fprintf with tfm::format manually (MarcoFalke)
fac03ec43a scripted-diff: Replace fprintf with tfm::format (MarcoFalke)
fa72a64b90 tinyformat: Add doc to Bitcoin Core specific strprintf (MarcoFalke)

Pull request description:

  This should be a refactor except in the cases where we use the wrong format specifier [1], in which case this patch is a bug fix.

  [1] : e.g.  depends: Add libevent compatibility patch for windows #8730

ACKs for commit fa8f19:
  promag:
    ACK fa8f195195. Ideally this should be rebased before merge.
  practicalswift:
    utACK fa8f195195
  Empact:
    ACK fa8f195195
  laanwj:
    code review and lightly tested ACK fa8f195195
  jonatack:
    ACK fa8f195195 from light code review, building, and running linter/unit tests/extended functional tests.

Tree-SHA512: 65f648b0bc383e3266a5bdb4ad8c8a1908a719635d49e1cd321b91254be24dbc7e22290370178e29b98ddcb3fec0889de9cbae273c7140abc9793d849534a743
2019-06-17 06:06:41 -04:00
fanquake
47d981e827
Merge #16186: doc/lint: Fix spelling errors identified by codespell 1.15.0
b748bf6f50 Fix spelling errors identified by codespell 1.15.0 (Ben Woosley)

Pull request description:

  Note all changes are to comments / documentation.

  After this commit, the only remaining output is:

  ```
    $ test/lint/lint-spelling.sh
    src/test/base32_tests.cpp:14: fo  ==> of, for
    src/test/base64_tests.cpp:14: fo  ==> of, for
    ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt
  ```

  Note:
  * I ignore several valid alternative spellings ~, but changed homogenous
  to homogeneous as the latter is a more specific term according to the
  Google dictionary definitions I found~
  * homogenous is present in tinyformat, hence should be addressed upstream
  * process' is correct only if there are plural processes

ACKs for commit b748bf:
  practicalswift:
    ACK b748bf6f50
  fanquake:
    ACK b748bf6f50

Tree-SHA512: 9add7044643ce015e0a44d8b27a3f300d72c485ffff550fb6491a17f14528085289ec5caddfe02f291ea9b2cded38a0dd3079652a054e2d7fe2ff4f7b53db5d7
2019-06-16 09:57:09 +08:00
darosior
c59e3a3261 getrawtransaction: inform about blockhash argument when lookup fails 2019-06-14 23:02:19 +02:00
Chris Moore
71fd628ada
Add example 2nd arg to signrawtransactionwithkey
The RPC examples for signrawtransactionwithkey are missing the 2nd parameter.
2019-06-13 19:33:28 -07:00
MarcoFalke
fa8f195195
Replace remaining fprintf with tfm::format manually 2019-06-13 11:46:38 -04:00
MarcoFalke
fac03ec43a
scripted-diff: Replace fprintf with tfm::format
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e 's/fprintf\(std(err|out), /tfm::format(std::c\1, /g' $(git grep -l 'fprintf(' -- ':(exclude)src/crypto' ':(exclude)src/leveldb' ':(exclude)src/univalue' ':(exclude)src/secp256k1')
-END VERIFY SCRIPT-

fixup! scripted-diff: Replace fprintf with tfm::format
2019-06-13 10:32:52 -04:00
MarcoFalke
fa72a64b90
tinyformat: Add doc to Bitcoin Core specific strprintf 2019-06-13 09:30:40 -04:00