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
