64fee48944 qt: Assert QMetaObject::invokeMethod result (João Barbosa)
f27bd96b5f gui: Fix missing qRegisterMetaType(WalletModel*) (João Barbosa)
Pull request description:
Invalid/wrong dynamic calls aren't verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the `invokeMethod` overload that allows connecting to lambdas or member pointers, which are compile checked.
For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5.
ACKs for top commit:
laanwj:
ACK 64fee48944
Tree-SHA512: d332e5d7eb2c7be5d3fe90e2e4ff20a67800b9664f6637c122a23647a964f7915703d3f086e2de440f695cfe14de268ff581d0092b7736e911952a4f4d248e25
e1a55690e6 Delete error-prone CScript constructor (Gregory Sanders)
Pull request description:
The behavior of this constructor is not the expected behavior compared to the other constructors which directly interpret the vector as a CScript, rather than serialize it into a new CScript. It has only four uses in the entire codebase. Delete this constructor and replace its four uses with the more clear serialization construction.
ACKs for top commit:
Empact:
ACK e1a55690e6
sipa:
Concept and code review ACK e1a55690e6, but I'd like to make sure we have tests covering the FindAndDelete usage.
Tree-SHA512: b6721e343c867ca401a80ec87c25939d7f1fc798f3bf7e5feb0ea6f8280eecb6bd65afc8286912c76ff8119ccea50ad7726b1a4137cae70c9d4fed7d960e10d3
fa64b947bb util: No translation of `Bitcoin Core` in the copyright (MarcoFalke)
fab85208f6 qt: Run «make translate» in ./src/ (MarcoFalke)
fabe87d2c9 scripted-diff: Avoid passing PACKAGE_NAME for translation (MarcoFalke)
fa5e9f157e build: Stop translating PACKAGE_NAME (MarcoFalke)
Pull request description:
Generally the package name is not translated, but the package description is.
E.g. `GIMP` or `Firefox` are always called that way regardless of the system language. However, "`Firefox` webbrowser" or "`GIMP` image manipulation program" are translated.
ACKs for top commit:
hebasto:
ACK fa64b947bb, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
Tree-SHA512: 626f811531182d0ba0ef1044930d32726773349bcb49b10261288a86ee6b80a183db30a87d817d5b0d501fad058ac22d6272311716b4f5a154f17c6f391a5a1a
1ac454a384 Enable ShellCheck rules (Hennadii Stepanov)
Pull request description:
Enable some simple ShellCheck rules.
Note for reviewers: `bash` and `shellcheck` on macOS are different from ones on Ubuntu.
For local tests the latest `shellcheck` version 0.6.0 should be used (see #15166).
ACKs for top commit:
practicalswift:
utACK 1ac454a384
dongcarl:
utACK 1ac454a
fanquake:
ACK 1ac454a384
Tree-SHA512: 8d0a3a5c09fe1a0c22120178f5e6b80f81f746f8c3356b7701ff301c117acb2edea8fe08f08fb54ed73f94b1617515fb239fa28e7ab4121f74872e6494b6f20e
faa1e0fb17 qt: test: Create at most one testing setup (MarcoFalke)
Pull request description:
It is assumed that ideally only one BasicTestingSetup exists at any point in time for each process (due to use of globals).
This assumption is violated in the GUI tests, as a testing setup is created as the first step of the `main` function and then (sometimes) another one for the following test cases.
So, the gui tests create two testing setups:
* `BasicTestingSetup` in `main` (added in fa4a04a5a9)
* a testing setup for individual test cases
Avoid that by destructing the testing setup in main after creation and then move the explicit `ECC_Stop` to the only places where it is needed (before and after `apptests`).
ACKs for top commit:
laanwj:
code review ACK faa1e0fb17
Tree-SHA512: b8edceb7e2a8749e1de3ea80bc20b6fb7d4390bf366bb9817206ada3dc8669a91416f4803c22a0e6c636c514e0c858dcfe04523221f8851b10deaf472f107d82
FundTransaction calls GetMinimumFee which, when the fee rate is absurdly high, quietly reduced the fee to -maxtxfee. Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.
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
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
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
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
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
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
75485ef09 gui: Enable open wallet menu on setWalletController (João Barbosa)
Pull request description:
`BitcoinApplication::initializeResult` and `BitcoinGUI::setWalletController` are only called after the startup rescan is completed. While the rescan is in progress the window menus are already available.
This PR makes the Open Wallet menu disabled until `BitcoinGUI::setWalletController` is called.
Fixes#16087
ACKs for commit 75485e:
jonasschnelli:
utACK 75485ef096
ryanofsky:
utACK 75485ef096. It's a simple, sensible fix.
Tree-SHA512: 9395ceed54bbceb6cbf1cd443f783d07a6ebb8fc5515b63c6e1b8b19b216b08d1cba7eaf872814d7c426ab7192f3b416ba0d57fc84f3bcbfebf01ce153794201
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)
Pull request description:
Make reasoning about dependencies easier by not including unused dependencies.
Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.
As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:
```
$ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
sed 's%^%src/%g' | xargs cat | wc -l
51393
```
Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)
ACKs for commit 67f4e9:
Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
6e6494b3fb qt: fix opening bitcoin.conf via Preferences on macOS; see #15409 (shannon1916)
Pull request description:
Fix#15409. The QT wallet fail to open the configuration file on Mac, when these is no default application for `*.conf` files.
Here is a feasible way to solve this bug. When `QDesktopServices::openUrl` fails to open `file:///path/bitcoin.conf` with its default application, use `QProcess::startDetached` to run `open -t /path/bitcoin.conf` command instead, so as to open the configuration file with system's default text editor.
ACKs for commit 6e6494:
hebasto:
re-ACK 6e6494b3fb
fanquake:
tACK 6e6494b3fb on macOS 10.14.x
Tree-SHA512: 60e898f4cb77cfd7b8adbc8d33fbebf46bac2a801bdcf40cae15e24b78ad56b1f32358b1879b670623d9f8651dea93961d34269358cea18f4e15b089a8ffcfbf
2d8ad2f997 gui: Enable console line edit on setClientModel (João Barbosa)
Pull request description:
Make console line edit disable by default, and only enable once `RPCConsole::setClientModel` is called.
Fixes#16119.
ACKs for commit 2d8ad2:
fanquake:
tACK 2d8ad2f997 on macOS.
Tree-SHA512: 1418ce3c120c08e5ec3e7a7a063572a24402ce0ec541bd4adc21f61d60c4e86b711e82e940ebf5f0445ab861f89c146c2a2e7990fb52bed2c65fc199a1981f71
9f85e9cb3d scripted-diff: Rename LockAnnotation to LockAssertion (practicalswift)
de9b5dbca3 Make sure the compile-time locking promises given via LockAnnotation:s hold also in practice at runtime (ifdef DEBUG_LOCKORDER) (practicalswift)
3a809446b3 Move LockAnnotation to make it reflect the truth (practicalswift)
cc2588579c Move LockAnnotation from threadsafety.h (imported code) to sync.h (our code) (practicalswift)
Pull request description:
`LockAnnotation lock(mutex);` is a guarantee to the compiler thread-analysis that `mutex` is locked (when it couldn't be determined otherwise).
Before this PR it was possible to make the mistake of adding a `LockAnnotation` where the correct mutex is _not_ held. This in turn makes the thread-analysis reasoning being based on incorrect premises.
This PR adds an assertion in the `LockAnnotation` ctor which checks that the guarantees given by us at compile-time are held also in practice (`ifdef DEBUG_LOCKORDER`).
Issues like the one described in #16028 will be discovered immediately with this PR merged.
Changes in this PR:
* Move `LockAnnotation` from `threadsafety.h` (imported code) to `sync.h` (our code)
* Move `LockAnnotation` in `wallet_tests` to make it reflect the truth
* Make sure the compile-time locking promises given via `LockAnnotation`:s hold also in practice at runtime (`ifdef DEBUG_LOCKORDER`)
* Rename `LockAnnotation` to `LockAssertion`
ACKs for commit 9f85e9:
ryanofsky:
utACK 9f85e9cb3d. No changes at all since last review except clean rebase after base PR #16033 was merged
Tree-SHA512: fb80e78fe362adfd6ea8405bcb142c09b99f834fe8be4397282b223ca2c3a2bb9719a074a47a043b44757f840b239a6fcd2f98d14771f8729204834ecf608c3a
0b09a57ae Give WalletModel::UnlockContext move semantics (Pieter Wuille)
Pull request description:
WalletModel::UnlockContext seems to implement "move upon copy" semantics; with C++11 this can be done more safely using move semantics (making attempts to actually copy fail instead).
Not a big deal if this isn't worth review time.
ACKs for commit 0b09a5:
Empact:
utACK 0b09a57aec
jonasschnelli:
utACK 0b09a57aec
jb55:
utACK 0b09a57aec
Tree-SHA512: f827856586afd03666c2d9f50320776afb3dd511ac1bcd293b330f015acd1588551b163dccc97b1351301e3295f4c74d90e5754bcee89faeadf6437d7db165c8
78e407ad0c GetKeyBirthTimes should return key ids, not destinations (Gregory Sanders)
70946e7fee Replace CScriptID and CKeyID in CTxDestination with dedicated types (Gregory Sanders)
Pull request description:
The current usage seems to be an overloading of meanings. `CScriptID` is used in the wallet as a lookup key, as well as a destination, and `CKeyID` likewise. Instead, have all destinations be dedicated types.
New types:
`CScriptID`->`ScriptHash`
`CKeyID`->`PKHash`
ACKs for commit 78e407:
ryanofsky:
utACK 78e407ad0c. Only changes are removing extra CScriptID()s and fixing the test case.
Sjors:
utACK 78e407a
meshcollider:
utACK 78e407ad0c
Tree-SHA512: 437f59fc3afb83a40540da3351507aef5aed44e3a7f15b01ddad6226854edeee762ff0b0ef336fe3654c4cd99a205cef175211de8b639abe1130c8a6313337b9
486c1eea86 refactoring: remove unused chainActive (James O'Beirne)
631940aab2 scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne)
a3a609079c refactoring: introduce unused ChainActive() (James O'Beirne)
1b6e6fcfd2 rename: CChainState.chainActive -> m_chain (James O'Beirne)
Pull request description:
This is part of the assumeutxo project:
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
---
This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data.
The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot.
This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review.
ACKs for commit 486c1e:
Sjors:
utACK 486c1ee
promag:
utACK 486c1ee.
practicalswift:
utACK 486c1eea86
Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a
fc929842c2 GUI: Move QRImageWidget to its own file-pair (Luke Dashjr)
77851ab682 GUI: Refactor actual QR code rendering into new QRImageWidget::setQR (Luke Dashjr)
Pull request description:
For at least QR-code based pairing of mobile wallets with nodes, it will be desirable to render QR codes even without wallet support.
Therefore, this prepares by moving the QRImageWidget out of a wallet-specific file into its own `qrencoder` file-pair.
ACKs for commit fc9298:
laanwj:
utACK fc929842c2
jonasschnelli:
utACK fc929842c2
Tree-SHA512: 95529a38c0573a4b3f1253fb5f11ca07a5b3a9840ec24acc7d87270212f3c9f7c5b186d9274d297517a3b80494f38a57574fb9730b1574db01688539b987bd91
Though at the moment ChainActive() simply references `g_chainstate.m_chain`,
doing this change now clears the way for multiple chainstate usage and allows
us to script the diff.
-BEGIN VERIFY SCRIPT-
git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g'
-END VERIFY SCRIPT-
This work is prerequisite to attaching thread names to log lines and deadlock
debug utilities. This code allows setting of an "internal" threadname per
thread on platforms where thread_local is available.
This commit also moves RenameThread() out of a more general module and adds a
numeric suffix to disambiguate between threads with the same name. It
explicitly names a few main threads using the new util::ThreadRename().
5c759c73b2 [wallet] Move maxTxFee to wallet (John Newbery)
Pull request description:
Closes#15355
Moves the `-maxtxfee` from the node to the wallet. See discussion in issue for details.
This is a cleanup. There is no change in behaviour.
Completes #15620
ACKs for commit 5c759c:
MarcoFalke:
utACK 5c759c73b2
ryanofsky:
utACK 5c759c73b2. Changes since last review: updated commit message and an error message and method name.
meshcollider:
utACK 5c759c73b2
Tree-SHA512: 2f9b2729da3940a5cda994d3f3bc11ee1a52fcc1c5e9842ea0ea63e4eb0300e8416853046776311298bc449ba07554aa46f0f245ce28598a5b0bd7347c12e752
This pull request attempts to resolve the `qt/guiutil` <-> `qt/optionsmodel`
circular dependency.
The circular dependency is resolved by moving the `Intro::getDefaultDataDirectory`
member function to `GUIUtil::getDefaultDataDirectory`.
418d3230f8 Resolve the checkpoints <-> validation CD. (251)
Pull request description:
This pull request attempts to resolve the `checkpoints -> validation -> checkpoints` circular dependency.
The circular dependency is resolved by moving the `CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` function to `validation.cpp` where it used exclusively by the private function `ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime)`.
ACKs for commit 418d32:
promag:
utACK 418d323, only `GetLastCheckpoint` usage is in `validation.cpp` and so makes sense to move it there.
practicalswift:
utACK 418d3230f8
MarcoFalke:
utACK 418d3230f8
sipa:
utACK 418d3230f8
Tree-SHA512: 03c3556bc192e65f5e3fa76fd545d4ee7d63d3fb06b132f7a1fa6131aa21ddd2e5b2d19e2222dfe524f422daaca30efde219bed188db8c74ff4b088876b5bc16
8a33f4d63f GUI: Options: Remove the upper-bound limit from pruning size setting (Luke Dashjr)
4ddeb2f860 GUI: Options: Set the range of pruning size before loading its value (Luke Dashjr)
Pull request description:
This fixes two bugs:
1. The prune setting range was set *after* loading the current value. If users had a prune of (eg) 200, it would get limited to 99 before the range was raised. This is fixed by setting the range first.
2. The prune setting was limited to <= the chainparams' "assumed blockchain size". There's no reason for this limit (the UX is the same either way), and there are use cases it breaks (eg, setting a prune size such that it begins pruning at some future point). Therefore, I raised it to the max value.
This is a daggy fix, so should cleanly merge to both master and 0.18 branches.
ACKs for commit 8a33f4:
MarcoFalke:
utACK 8a33f4d63f
laanwj:
utACK 8a33f4d63f
promag:
utACK 8a33f4d.
Tree-SHA512: 480570fa243ab5cc76af76fded18cb8cb2d3194b9f050fec5e03ca551edeeda72ee8b06312e200a9e49404ec1cdffa62f7150cf9982ec1b282f17d90879ce438
This commit moves the maxtxfee setting to the wallet. There is only
one minor behavior change:
- an error message in feebumper now refers to -maxtxfee instead of
maxTxFee.
faf400077d scripted-diff: Bump copyright headers in test, bench (MarcoFalke)
fa821904bf scripted-diff: Rename test_bitcoin to test/setup_common (MarcoFalke)
fa8685d49e test: Use test_bitcoin setup in bench, Add test utils (MarcoFalke)
666696b673 test: Have segwit always active in (Basic)TestingSetup (MarcoFalke)
Pull request description:
Now that the fuzz tests can use the BasicTestingSetup [1], do the same for bench.
Also move some duplicate code to a common "test/util" module.
[1]: fuzz: Link BasicTestingSetup (shared with unit tests) #15504
ACKs for commit faf400:
jonatack:
ACK faf400077d
Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4
Adds the following util units and adds them to libbitcoin_util:
- `util/url.cpp` takes `urlDecode` from `httpserver.cpp`
- `util/error.cpp` takes `TransactionErrorString` from
`node/transaction.cpp` and `AmountHighWarn` and `AmountErrMsg` from
`ui_interface.cpp`
- `util/fees.cpp` takes `StringForFeeReason` and `FeeModeFromString` from `policy/fees.cpp`
- `util/rbf.cpp` takes `SignalsOptInRBF` from `policy/rbf.cpp`
- 'util/validation.cpp` takes `FormatStateMessage` and `strMessageMagic` from 'validation.cpp`
This commit resolves the checkpoints -> validation -> checkpoints
cirular dependency by moving
`CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` from
`checkpoints.cpp` to `validation.cpp`.
signrawtransaction was deprecated in 0.17 and removed in 0.18. A warning
was left in place to tell users to migrate to using
signrawtransactionswithwallet or signrawtransactionwithkey. Remove the
warning now that it's been two releases since the method was removed.
d358466de Remove remaining wallet accesses to node globals (Russell Yanofsky)
b1b2b2389 Remove use of CCoinsViewMemPool::GetCoin in wallet code (Russell Yanofsky)
4e4d9e9f8 Remove use of CRPCTable::appendCommand in wallet code (Russell Yanofsky)
91868e628 Remove use CValidationInterface in wallet code (Russell Yanofsky)
Pull request description:
This PR is the last in a chain of PRs (#14437, #14711, and #15288) that make the wallet code access node state through an abstract [`Chain`](https://github.com/ryanofsky/bitcoin/blob/pr/wipc-sep/src/interfaces/chain.h) class in [`src/interfaces/`](https://github.com/ryanofsky/bitcoin/tree/pr/wipc-sep/src/interfaces) instead of using global variables like `cs_main`, `chainActive`, and `g_connman`. After this PR, wallet code no longer accesses global variables declared outside the wallet directory, and no longer calls functions accessing those globals (as verified by the `hide-globals` script in #10244).
This PR and the previous PRs have been refactoring changes that do not affect behavior. Previous PRs have consisted of lots of mechanical changes like:
```diff
- wtx.nTimeReceived = GetAdjustedTime();
+ wtx.nTimeReceived = m_chain->getAdjustedTime();
```
This PR is smaller, but less mechanical. It replaces last few bits of wallet code that access node state directly (through `CValidationInterface`, `CRPCTable`, and `CCoinsViewMemPool` interfaces) with code that uses the `Chain` interface.
These changes allow followup PR #10102 (multiprocess gui & wallet PR) to work without any significant updates to wallet code. Additionally they:
* Provide a single place to describe the interface between wallet and node code.
* Can make better wallet testing possible, because the `Chain` object consists of virtual methods that can be overloaded for mocking. (This could be used to test edge cases in the rescan code, for example).
Tree-SHA512: e6291d8a3c50bdff18a9c8ad11e729beb30b5b7040d7aaf31ba678800b4a97b2dd2be76340b1e5c01fe2827d67d37ed1bb4c8380cf8ed653aadfea003e9b22e7
28c86de3b gui: Drop unused return values in WalletFrame (João Barbosa)
Pull request description:
This is a small cleanup since the return value of `WalletFrame` methods are not used. This is in line with the usual async slot declaration.
Tree-SHA512: ff0ca098804118bba200a58cd796ff90e853a6430e58125bd178b7bfa9b2b763c13d17b81e8f3ebd94395cac249d80379ba1529680c47682ba6a2ed81492ba33
fd46c4c00 Bump minimum Qt version to 5.5.1 (Sjors Provoost)
Pull request description:
Fixes#13478
Compiled and lightly tested on 10.14.3 against QT 5.12.0.
Tree-SHA512: 6890331969bbf4c66dc0993b8817b1f0831d008f5863554e9c09a38f4700260b84044ff961664c377decc9fb8300e3543c267f935ec64fbc97b20f8fb396247a
84f53154e Travis: Add test without BIP70 (but still full wallet + tests) (Luke Dashjr)
113f0004b GUI: If BIP70 is disabled, give a proper error when trying to open a payment request file (Luke Dashjr)
9975282fa GUI: If BIP70 is disabled, attempt to fall back to BIP21 parsing (Luke Dashjr)
Pull request description:
Tree-SHA512: 66a684ce4336d0eac8b0107b405ff3a2cf312258a967f3e1b14734cd39db11e2db3e9b03492755583170d94d54754ef536b0776e5f19a0cc2caca8379eeb4495
1951ea434 gui: Show indeterminate progress dialog while opening walllet (João Barbosa)
8847cdaaa gui: Add OpenWalletActivity (João Barbosa)
4c8982a88 interfaces: Avoid interface instance if wallet is null (João Barbosa)
be82dea23 gui: Add thread to run background activity in WalletController (João Barbosa)
6c49a55b4 gui: Add Open Wallet menu (João Barbosa)
32a8c6abf gui: Add openWallet and getWalletsAvailableToOpen to WalletController (João Barbosa)
ab288b4e5 interfaces: Add loadWallet to Node (João Barbosa)
17abc0fd5 wallet: Factor out LoadWallet (João Barbosa)
Pull request description:
The *Open Wallet* menu has all the available wallets currently not loaded. The list of the available wallets comes from `listWalletDir`.
In the future the menu can be replaced by a custom dialog.
<img width="674" alt="screenshot 2019-01-12 at 12 17 02" src="https://user-images.githubusercontent.com/3534524/51073166-ac041480-1664-11e9-8302-be81702bc146.png">
Tree-SHA512: ebfd75eee0c8264863748899843afab67dadb7dff21313c11e3cb5b6108d954978dd1f1ae786bc07580c5a771ea4ab38d18c1643c9b9b3683ed53f0f6c582e38
7687f7873 [wallet] Support creating a blank wallet (Andrew Chow)
Pull request description:
Alternative (kind of) to #14938
This PR adds a `blank` parameter to the `createwallet` RPC to create a wallet that has no private keys initially. `sethdseed` can then be used to make a clean wallet with a custom seed. `encryptwallet` can also be used to make a wallet that is born encrypted.
Instead of changing the version number as done in #14938, a wallet flag is used to indicate that the wallet should be blank. This flag is set at creation, and then unset when the wallet is no longer blank. A wallet becomes non-blank when a HD seed is set or anything is imported. The main change to create a blank wallet is primarily taken from #14938.
Also with this, the term "blank wallet" is used instead of "empty wallet" to avoid confusion with wallets that have balance which would also be referred to as "empty".
This is built on top of #15225 in order to fix GUI issues.
Tree-SHA512: 824d685e11ac2259a26b5ece99c67a7bda94a570cd921472c464243ee356b7734595ad35cc439b34357135df041ed9cba951e6edac194935c3a55a1dc4fcbdea
A blank wallet is a wallet that has no keys, script or watch only things.
A new wallet flag indicating that it is blank will be set when the wallet
is blank. Once it is no longer blank (a seed has been generated, keys or
scripts imported, etc), the flag will be unset.
e8db6b8044 Qt: Fix update headers-count (Jonas Schnelli)
7bb45e4b7a Qt: update header count regardless of update delay (Jonas Schnelli)
Pull request description:
Update the block and header tip is constraint to have a minimal distance of 250ms between updates... which can lead to miss the last header update.
The modal overlay then assumes we are still in header sync and the view get stuck in "syncing headers,..." (while it's actually syncing blocks).
This removes the 250ms minimal delta for header updates as well as it fixes the correct display of how header updates should update the labels.
Tree-SHA512: 57608dac822b135cd604fc6ba1c80f25c0202a6e20bb140362026615d4bf243ef4fcc254a11bad36419c554a222a2f4947438d4ce44aa14041d1874751643d68
364cff1cab Fix issue #9683 "gui, wallet: random abort (segmentation fault) running master/HEAD". (Chris Moore)
Pull request description:
Patch taken from @ryanofsky's comment https://github.com/bitcoin/bitcoin/issues/9683#issuecomment-448035913.
[MarcoFalke wrote](https://github.com/bitcoin/bitcoin/issues/9683#issuecomment-454066004):
> Mind to submit this patch as a pull request?
So that's what I'm doing.
I was regularly seeing crashes on startup before applying this patch and haven't seen a single crash on startup since applying it almost a month ago.
Tree-SHA512: 3bbb2291cdf03ab7e7b5b796df68d76272491e35d473a89f4550065554c092f867659a7b8d7a1a91461ae4dc9a3b13b72541eafdbd732536463e9f3cf82300c8
0164b0f5cf build: Remove WINVER pre define in Makefile.leveldb.inlcude (Chun Kuan Lee)
d0522ec94e Drop defunct Windows compat fixes (Ben Woosley)
d8a2992067 windows: Call SetProcessDEPPolicy directly (Chun Kuan Lee)
1bd9ffdd44 windows: Set _WIN32_WINNT to 0x0601 (Windows 7) (Chun Kuan Lee)
Pull request description:
The current minimum support Windows version is Vista. So set it to 0x0600
5a88def8ad/mingw-w64-headers/include/sdkddkver.h (L19)
Tree-SHA512: 38e2afc79426ae547131c8ad3db2e0a7f54a95512f341cfa0c06e4b2fe79521ae67d2795ef96b0192e683e4f1ba6183c010d7b4b8d6b3e68b9bf48c374c59e7d
aebafd0edf Rename Chain getLocator -> getTipLocator (Russell Yanofsky)
2c1fbaa771 Drop redundant get_value_or (Russell Yanofsky)
84adb206fc Fix ScanForWalletTransactions start_block comment (Russell Yanofsky)
2efa66b464 Document rescanblockchain returned stop_height being null (Russell Yanofsky)
db2d093233 Add suggested rescanblockchain comments (Russell Yanofsky)
a8d645c934 Update ScanForWalletTransactions result comment (Russell Yanofsky)
95a812b599 Rename ScanResult stop_block field (Russell Yanofsky)
Pull request description:
This implements suggested changes from #14711 review comments that didn't make make it in before merging.
There are no changes in behavior in this PR, just documentation updates, simplifications, and variable renames.
Tree-SHA512: 39f1a5718195732b70b5e427c3b3e4295ea5af6328a5991763a422051212dfb95383186db0c0504ce2c2782fb61998dfd2fe9851645b7cb4e75d849049483cc8
- Update transifex slug
- Mention update of MSVC build in `doc/translation_process.md`
- Do a `make translate` to update English translations
- Pull current translations from transifex
This change forwards the shutdown request on the GUI (close the
application for instace) to the node as soon as possible. This way the
GUI doesn't have to wait for long operations to complete (rescan the
wallet for instance), instead those operations detect the shutdown
request and abort/interrupt.
Move only change that makes unsubscribeFromCoreSignals public. It must be
called if the event loop is not running otherwise core signals handlers
can deadlock.
2bc4c3eaf9 Notify the GUI that the keypool has changed to set the receive button (Andrew Chow)
14bcdbe09c Check for more than private keys disabled to show receive button (Andrew Chow)
Pull request description:
Currently the Receive button in the GUI is displayed enabled or disabled by the initial state of the wallet when the wallet is first loaded. The button is only enabled or disabled depending on whether the disable private keys flag is set when the wallet is loaded. However, future changes to the wallet means that this initial state and check may no longer be accurate. #14938 introduces empty wallets which do not have private keys. An empty wallet that is loaded should have the Receive button disabled, and then it should become enabled once `sethdseed` is used so that a keypool can be generated and new keys generated. Likewise, with #14075, a wallet can be loaded with no keypool initially, so the button should be disabled. Later, public keys can be imported into the keypool, at which time the button should become enabled. When the keypool runs out again (no new keys are generated as the keypool only consists of imports), the button should become disabled.
This PR makes it so that the button becomes enabled and disabled as the keypool state changes. The check for whether to enable or disable the receive button has changed to checking whether it is possible to get new keys. It now checks for whether the wallet has an HD seed and, if not, whether the private keys are disabled. When an action happens which would make it possible for a new address to be retrieved or make it possible for a no more addresses to be retrieved, a signal is emitted which has the GUI recheck the conditions for the Receive button. These actions are setting a new HD seed, topping up the keypool, retrieving a key from the keypool, and returning a key to the keypool.
Tree-SHA512: eff15a5337f4c64ecd7169414fb47053c04f6a0f0130341b6dd9799ac4d79f451e25284701c668971fca33f0909d5352a474a2c12349375bedfdb59b63077d50
6f6514a080 Correct units for "-dbcache" and "-prune" (Hennadii Stepanov)
Pull request description:
Actually, all `dbcache`-related values in the code are measured in MiB (not in megabytes, MB) or in bytes (e.g., `nTotalCache`).
See: https://github.com/bitcoin/bitcoin/blob/master/src/txdb.hba8c8b2227/src/init.cpp (L1405-L1424)
Also, "-prune" is fixed:
1. The GUI values in GB are translated to the node values in MiB correctly.
2. The maximum of the "prune" `QSpinBox` is not limited by default value of 99 (GB).
Fix: #15106
Tree-SHA512: 151ec43b31b1074db8b345fedb1dcc10bde225899a5296bfc183f57e1553d13ac27db8db100226646769ad03c9fcab29d88763065a471757c6c41ac51108459d
All dbcache-related values in the code are measured in MiB (not in
megabytes, MB) or in bytes.
The GUI "-prune" values in GB are translated to the node values in MiB
correctly. The maximum of the "-prune" QSpinBox is not limited by the
default value of 99 (GB).
Also, this improves log readability.
44de1561a Remove remaining chainActive references from CWallet (Russell Yanofsky)
db21f0264 Convert CWallet::ScanForWalletTransactions and SyncTransaction to the new Chain apis (Russell Yanofsky)
2ffb07929 Add findFork and findBlock to the Chain interface (Russell Yanofsky)
d93c4c1d6 Add time methods to the Chain interface (Russell Yanofsky)
700c42b85 Add height, depth, and hash methods to the Chain interface (Russell Yanofsky)
Pull request description:
This change removes uses of `chainActive` and `mapBlockIndex` globals in wallet code. It is a refactoring change which does not affect external behavior.
This is the next step in the larger #10973 refactoring change, which removes all other accesses to node global variables from wallet code. Doing this is useful to provide a better defined interface between the wallet and node, and necessary to allow wallet and node code to run in separate processes in #10102.
Tree-SHA512: 4dcec8a31c458f54e2ea6ecf01e430469b0994c5b41a21a2d150efa67cd209f4c93ae210a101e064b3a87c52c6edfc70b070e979992be0e3a00fd425de6230a8
Whenever the keypool changes (new keys generated, new seed set,
keypool runs out, etc.), notify the GUI that the keypool has changed. The
receive button can then be enabled and disabled as necessary.
da6011826a Fix macOS launch-at-startup memory issue (Jonas Schnelli)
516437a1b7 Qt: remove macOS launch-at-startup option when compiled with > macOS 10.11 (Jonas Schnelli)
Pull request description:
The launch-at-startup API Bitcoin Core uses on macOS where removed in macOS 10.11 leading to a segmentation-fault due to the weak-linking when not actively compiled against SDK 10.11 (`-mmacosx-version-min=10.11`)
This PR removes the launch-at-startup feature on macOS when compiled with macOS min version > 10.11 (the default is always the macOS version you compile on).
**The depends built binaries (Gitian) are not affected since we are building with min macOS 10.10.**
Users self compiling on macOS > 10.11 can re-enable the feature by compiling with min version <= 10.11 (`CXXFLAGS="-mmacosx-version-min=10.11" CFLAGS="-mmacosx-version-min=10.11" ./configure`)
**Isn't there a new API from Apple?**
Yes, [there is](https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/CreatingLoginItems.html).
It will require to create a helper application which needs to be embedded in the .app folder (needs code signing as well). Developers willing to go down that rabbit hole are welcome.
Fixes#15142
Tree-SHA512: fa9cc4e39d5a2d2559919b7e22b7766f5e0269a361719294d4a4a2df2fd9d955e5b23b5907e68023fdeee297f652f844f3c447904bf18f9c1145348ad101c432
223de8d94d Document RNG design in random.h (Pieter Wuille)
f2e60ca985 Use secure allocator for RNG state (Pieter Wuille)
cddb31bb0a Encapsulate RNGState better (Pieter Wuille)
152146e782 DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille)
a1f252eda8 Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille)
4ea8e50837 Remove hwrand_initialized. (Pieter Wuille)
9d7032e4f0 Switch all RNG code to the built-in PRNG. (Pieter Wuille)
16e40a8b56 Integrate util/system's CInit into RNGState (Pieter Wuille)
2ccc3d3aa3 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille)
aae8b9bf0f Add thread safety annotations to RNG state (Pieter Wuille)
d3f54d1c82 Rename some hardware RNG related functions (Pieter Wuille)
05fde14e3a Automatically initialize RNG on first use. (Pieter Wuille)
2d1cc50939 Don't log RandAddSeedPerfmon details (Pieter Wuille)
6a57ca91da Use FRC::randbytes instead of reading >32 bytes from RNG (Pieter Wuille)
Pull request description:
This does not remove OpenSSL, but makes our own PRNG the 'main' one; for GetStrongRandBytes, the OpenSSL RNG is still used (indirectly, by feeding its output into our PRNG state).
It includes a few policy changes (regarding what entropy is seeded when).
Before this PR:
* GetRand*:
* OpenSSL
* GetStrongRand*:
* CPU cycle counter
* Perfmon data (on Windows, once 10 min)
* /dev/urandom (or equivalent)
* rdrand (if available)
* From scheduler when idle:
* CPU cycle counter before and after 1ms sleep
* At startup:
* CPU cycle counter before and after 1ms sleep
After this PR:
* GetRand*:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* GetStrongRand*:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* /dev/urandom (or equivalent)
* OpenSSL
* CPU cycle counter again
* From scheduler when idle:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter before and after 1ms sleep
* Perfmon data (on Windows, once every 10 min)
* At startup:
* Stack pointer (which indirectly identifies thread and some call stack information)
* rdrand (if available)
* CPU cycle counter
* /dev/urandom (or equivalent)
* OpenSSL
* CPU cycle counter again
* Perfmon data (on Windows, once every 10 min)
The interface of random.h is also simplified, and documentation is added.
This implements most of #14623.
Tree-SHA512: 0120e19bd4ce80a509b5c180a4f29497d299ce8242e25755880851344b825bc2d64a222bc245e659562fb5463fb7c70fbfcf003616be4dc59d0ed6534f93dd20
18185b57c3 scripted-diff: batch-recase BanMan variables (Carl Dong)
c2e04d37f3 banman: Add, use CBanEntry ctor that takes ban reason (Carl Dong)
1ffa4ce27d banman: reformulate nBanUtil calculation (Carl Dong)
daae598feb banman: add thread annotations and mark members const where possible (Cory Fields)
84fc3fbd03 scripted-diff: batch-rename BanMan members (Cory Fields)
af3503d903 net: move BanMan to its own files (Cory Fields)
d0469b2e93 banman: pass in default ban time as a parameter (Cory Fields)
2e56702ece banman: pass the banfile path in (Cory Fields)
4c0d961eb0 banman: create and split out banman (Cory Fields)
83c1ea2e5e net: split up addresses/ban dumps in preparation for moving them (Cory Fields)
136bd7926c tests: remove member connman/peerLogic in TestingSetup (Cory Fields)
7cc2b9f678 net: Break disconnecting out of Ban() (Cory Fields)
Pull request description:
**Old English à la Beowulf**
```
Banman wæs bréme --blaéd wíde sprang--
Connmanes eafera Coreum in.
aéglaéca léodum forstandan
Swá bealdode bearn Connmanes
guma gúðum cúð gódum daédum·
dréah æfter dóme· nealles druncne slóg
```
**Modern English Translation**
```
Banman was famed --his renown spread wide--
Conman's hier, in Core-land.
against the evil creature defend the people
Thus he was bold, the son of Connman
man famed in war, for good deeds;
he led his life for glory, never, having drunk, slew
```
--
With @theuni's blessing, here is Banman, rebased. Original PR: https://github.com/bitcoin/bitcoin/pull/11457
--
Followup PRs:
1. Give `CNode` a `Disconnect` method ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248065847))
2. Add a comment to `std::atomic_bool fDisconnect` in `net.h` that setting this to true will cause the node to be disconnected the next time `DisconnectNodes()` runs ([source](https://github.com/bitcoin/bitcoin/pull/14605#discussion_r248384309))
Tree-SHA512: 9c207edbf577415c22c9811113e393322d936a843d4ff265186728152a67c057779ac4d4f27b895de9729f7a53e870f828b9ebc8bcdab757520c2aebe1e9be35
0dd9bdefa gui: Refactor to use WalletController (João Barbosa)
8fa271f08 gui: Add WalletController (João Barbosa)
cefb399e2 gui: Use AutoConnection for WalletModel::unload signal (João Barbosa)
Pull request description:
This PR is a subset of the work done in the context of #13100. This change consists in extracting from the application class the code that manages the wallet models.
The role of the `WalletController` instance is to coordinate wallet operations and the window.
Tree-SHA512: 6a824054376730eb7d16c643dd2003f5f60778e8ad3af707b82bc12c48438db179ca4446316b28fb17b206f4b9aba8998419aab8c5dd1f7c32467015732b5094
24313fbf7e Remove redundant stopExecutor() signal (Hennadii Stepanov)
1c0e0a5e38 Remove redundant stopThread() signal (Hennadii Stepanov)
Pull request description:
The `QThread::finished` signal do this work.
Tree-SHA512: 1afce23d30232276d50c3af5af79d83b88e390a2b71f7df585cc1079585d330447d179bbc34c0a89599beb2da035dfd5b9ce23238171490825cabc3a19ae6e67
These are separate events which need to be carried out by separate subsystems.
This also cleans up some whitespace and tabs in qt to avoid getting flagged by
the linter.
Current behavior is preserved.
Added a note that results can be queried in the parenthesized syntax.
Deprecated boolean `verbose` replaced with numerical `verbosity` in
`getblock` examples.
fe7048b39 gui: Show current wallet name in window title (João Barbosa)
8a7926112 gui: Keep network style in BitcoinGUI (João Barbosa)
f411c8b35 gui: Remove unused return type in some BitcoinGUI methods (João Barbosa)
Pull request description:
<img width="876" alt="screenshot 2019-01-11 at 23 58 26" src="https://user-images.githubusercontent.com/3534524/51065458-d7ebaf80-15fc-11e9-9162-e37e9a10d448.png">
Tree-SHA512: 5c43f615834983bc1c5045e07c6e119044dd78ca947fd2679d302b519d5ce1d08d29ca00b1c11e88c4bbc4d56f2e6f4a8adc42084f3503e751e642e8a13112dc
Only change in behavior is "Rescan started from block <height>" message
replaced by "Rescan started from block <hash>" message in
ScanForWalletTransactions.
Co-authored-by: Ben Woosley <ben.woosley@gmail.com>
