fa27623 qt: Initialize members in WalletModel (MarcoFalke)
Pull request description:
This prevents segfaults (or errors when running qt in valgrind)
```
Conditional jump or move depends on uninitialised value(s)
WalletModel::checkBalanceChanged() (walletmodel.cpp:156)
Tree-SHA512: 38c8c03c7fa947edb3f1c13eab2ac7a62ef8f8141603c2329a7dc5821a887a349af8014dc739b762e046f410f44a9c6653b6930f08b53496cf66381cadc06246
d6f3a73 Remove redundant locks (practicalswift)
Pull request description:
Remove redundant locks:
* ~~`FindNode(...)` is locking `cs_vNodes` internally~~
* `SetAddressBook(...)` is locking `cs_wallet` internally
* `DelAddressBook(...)` is locking `cs_wallet` internally
**Note to reviewers:** From what I can tell these locks are redundantly held from a data integrity perspective (guarding specific variables), and they do not appear to be needed from a data consistency perspective (ensuring a consistent state at the right points). Review thoroughly and please let me know if I'm mistaken :-)
Tree-SHA512: 7e3ca2d52fecb16385dc65051b5b20d81b502c0025d70b0c489eb3881866bdd57947a9c96931f7b213f5a8a76b6d2c7b084dff0ef2028a1e9ca9ccfd83e5b91e
004f999 boost: drop boost threads for [alert|block|wallet]notify (Cory Fields)
0827267 boost: drop boost threads from torcontrol (Cory Fields)
ba91724 boost: remove useless threadGroup parameter from Discover (Cory Fields)
f26866b boost: drop boost threads for upnp (Cory Fields)
Pull request description:
This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining.
Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w'
Tree-SHA512: 5a356798d0785f93ed143d1f0afafe890bc82f0d470bc969473da2d2aa78bcb9b096f7ba11b92564d546fb447d4bd0d347e7842994ea0170aafd53fda7e0a66e
c8edc2c [docs] initial QT documentation, move Qt Creator instructions (Sjors Provoost)
Pull request description:
I'll update this as I figure out how everything is tied together, but I think it's a useful enough start.
Tree-SHA512: d96e5c9ba8ccc3a1b92a0894a8a8449317100eebb14e5d390b51793534458f50eac296cf2945fccf81b85aff23fa32d91d6015a0a76ada4f7091a400d7508ae5
1e5d14b qt: Clarify some comments (Wladimir J. van der Laan)
f5a4c3d qt: Make sure splash screen is freed on AppInitMain fail (Wladimir J. van der Laan)
Pull request description:
The `splashFinished` event was never sent if AppInitMain fails, causing the splash screen to stick around, causing problems later.
This bug has existed for a while but is now trigging potential crashed because the splash screen subscribes to wallet events.
Meant to fix#12372.
Tree-SHA512: 192a7e3a528015e771d7860dd95fd7b772292fd8064abf2a3cf3a8ea0d375cd43a6e8ed37ca1a38962fe1410c934599e557adf6a8ef9d87ec7f61b6e5fd8db7e
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)
Pull request description:
The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
e5033a5c9b/src/corelib/thread/qthread.cpp (L412-L415)
Potential fix for https://github.com/bitcoin/bitcoin/issues/12372#issuecomment-363642332
This reverts #11831 for now and hopefully restores the previous behaviour.
Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
9ad6746ccd Use static_cast instead of C-style casts for non-fundamental types (practicalswift)
Pull request description:
A C-style cast is equivalent to try casting in the following order:
1. `const_cast(...)`
2. `static_cast(...)`
3. `const_cast(static_cast(...))`
4. `reinterpret_cast(...)`
5. `const_cast(reinterpret_cast(...))`
By using `static_cast<T>(...)` explicitly we avoid the possibility of an unintentional and dangerous `reinterpret_cast`. Furthermore `static_cast<T>(...)` allows for easier grepping of casts.
For a more thorough discussion, see ["ES.49: If you must use a cast, use a named cast"](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es49-if-you-must-use-a-cast-use-a-named-cast) in the C++ Core Guidelines (Stroustrup & Sutter).
Tree-SHA512: bd6349b7ea157da93a47b8cf238932af5dff84731374ccfd69b9f732fabdad1f9b1cdfca67497040f14eaa85346391404f4c0495e22c467f26ca883cd2de4d3c
The `splashFinished` event was never sent if AppInitMain fails,
causing the splash screen to stick around, causing problems
later.
This bug has existed for a while but is now trigging potential crashed
because the splash screen subscribes to wallet events.
Meant to fix#12372.
Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
d3e467520f Properly alphabetize output of CLI --help option. (murrayn)
Pull request description:
The --help output of bitcoind, bitcoin-cli, bitcoin-tx, qt/bitcoin-qt, et al. is only about 90% alphabetized by option, which is kind of sloppy and occasionally misleading. This change (mostly) organizes the output alphabetically.
Tree-SHA512: 3029900dbe99f03397c1cbdb5e4ac09a13bc99bafe73c6855097206e4cdd9ad70d0b5cedb5e1e520005c3e9ef1c4cd32bb9d8c98ce6918d8434fec8bf06e56c8
Pull 0.16 translations before forking, to avoid having to do it twice.
Tree-SHA512: 9c093885f03783e0f64718985c5f9d385d2a8592e2acc87d922ca973d07c756a6b7fff585388094f0e1b673c41e792ce918c1f594b45e82a262acd93e1b91a8e
This resolves#12229 which pointed out a shutdown deadlock due to
scheduler/checkqueue having been shut down while network message
processing is still running.
ba490d2460 qt: Bump BLOCK_CHAIN_SIZE to 200GB (Wladimir J. van der Laan)
Pull request description:
Part of the release process for 0.16.
Value is open for discussion, my blocks/ directory is 163GB but this leaves some slack.
Tree-SHA512: 4dff81740992bf9de90427934afeb223ea5216f5682c9f07cb5c47aea33980a4c682fe3fd43c3dfa2c4d66ad0e7434dbce6cb252e56d63b36df605e12af9b10a
596c446 [wallet] use P2WPKH change output if any destination is P2WPKH or P2WSH (Sjors Provoost)
Pull request description:
If `-changetype` is not explicitly set, then regardless of `-addresstype`, the wallet will use a ~`bech32` change address~ `P2WPKH` change output if any destination is `P2WPKH` or `P2WSH`.
This seems more intuitive to me and more in line with the spirit of [BIP-69](https://github.com/bitcoin/bips/blob/master/bip-0069.mediawiki).
When combined with #11991 a QT user could opt to use `bech32` exclusively without having to figure out how to launch with `-changetype=bech32`, although so would #11937.
Tree-SHA512: 9238d3ccd1f3be8dfdd43444ccf45d6bdc6584ced3172a3045f3ecfec4a7cc8999db0cdb76ae49236492a84e6dbf3a1fdf18544d3eaf6d518e1f8bd241db33e7
7f81250 Mention that other RPC calls report keys as "imported" while txns are still missing (Jonas Schnelli)
ccd8ef6 Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock (Jonas Schnelli)
bc356b4 Make sure WalletRescanReserver has successfully reserved the rescan (Jonas Schnelli)
dbf8556 Add RAII wallet rescan reserver (Jonas Schnelli)
8d0b610 Avoid pemanent cs_main/cs_wallet lock during wallet rescans (Jonas Schnelli)
Pull request description:
Right now, we are holding `cs_main`/`cs_wallet` during the whole rescan process (which can take a couple of hours).
This was probably only done because of laziness and it is an important show-stopper for #11200 (GUI rescan abort).
Tree-SHA512: 0fc3f82d0ee9b2f013e6bacba8d59f7334306660cd676cd64c47bb305c4cb7c7a36219d6a6f76023b74e5fe87f3ab9fc7fd2439e939f71aef653fddb0a1e23b1
63ac8907c [qt] receive tab: bech32 address opt-in checkbox (Sjors Provoost)
Pull request description:
<img width="647" alt="schermafbeelding 2018-01-12 om 18 34 48" src="https://user-images.githubusercontent.com/10217/34887691-a6a796fe-f7c7-11e7-8b89-87ce07c61ce3.png">
Checkbox does what you would expect. Press tab from the amount field to get there.
It's unchecked by default.
When launched with `-addresstype=bech32` it's checked by default. When launched with `-addresstype=legacy` it unchecked and disabled.
The change in `receivecoinsdialog.ui` is smaller than it looks, due to the way git handles XML diffs. I had to add a horizontal spacer to make it look decent, see https://github.com/bitcoin/bitcoin/issues/11950#issuecomment-352870909. This causes column numbers to change in the rest of the grid.
I recommend testing on at least one other OS than OSX to be on the safe side.
Tree-SHA512: ec4b733b796d9a94278a5d8040a69d9574ef50021e68f94f61f2da75d1bb57f39272cbc9f1f7d34f733a19640daf666a23844fcd132f83bfdaf327d9d1d6f105
b224a47a1 Add address_types test (Pieter Wuille)
7ee54fd7c Support downgrading after recovered keypool witness keys (Pieter Wuille)
940a21932 SegWit wallet support (Pieter Wuille)
f37c64e47 Implicitly know about P2WPKH redeemscripts (Pieter Wuille)
57273f2b3 [test] Serialize CTransaction with witness by default (Pieter Wuille)
cf2c0b6f5 Support P2WPKH and P2SH-P2WPKH in dumpprivkey (Pieter Wuille)
37c03d3e0 Support P2WPKH addresses in create/addmultisig (Pieter Wuille)
3eaa003c8 Extend validateaddress information for P2SH-embedded witness (Pieter Wuille)
30a27dc5b Expose method to find key for a single-key destination (Pieter Wuille)
985c79552 Improve witness destination types and use them more (Pieter Wuille)
cbe197470 [refactor] GetAccount{PubKey,Address} -> GetAccountDestination (Pieter Wuille)
0c8ea6380 Abstract out IsSolvable from Witnessifier (Pieter Wuille)
Pull request description:
This implements a minimum viable implementation of SegWit wallet support, based on top of #11389, and includes part of the functionality from #11089.
Two new configuration options are added:
* `-addresstype`, with options `legacy`, `p2sh`, and `bech32`. It controls what kind of addresses are produced by `getnewaddress`, `getaccountaddress`, and `createmultisigaddress`.
* `-changetype`, with the same options, and by default equal to `-addresstype`, that controls what kind of change is used.
All wallet private and public keys can be used for any type of address. Support for address types dependent on different derivation paths will need a major overhaul of how our internal detection of outputs work. I expect that that will happen for a next major version.
The above also applies to imported keys, as having a distinction there but not for normal operations is a disaster for testing, and probably for comprehension of users. This has some ugly effects, like needing to associate the provided label to `importprivkey` with each style address for the corresponding key.
To deal with witness outputs requiring a corresponding redeemscript in wallet, three approaches are used:
* All SegWit addresses created through `getnewaddress` or multisig RPCs explicitly get their redeemscripts added to the wallet file. This means that downgrading after creating a witness address will work, as long as the wallet file is up to date.
* All SegWit keys in the wallet get an _implicit_ redeemscript added, without it being written to the file. This means recovery of an old backup will work, as long as you use new software.
* All keypool keys that are seen used in transactions explicitly get their redeemscripts added to the wallet files. This means that downgrading after recovering from a backup that includes a witness address will work.
These approaches correspond to solutions 3a, 1a, and 5a respectively from https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2. As argued there, there is no full solution for dealing with the case where you both downgrade and restore a backup, so that's also not implemented.
`dumpwallet`, `importwallet`, `importmulti`, `signmessage` and `verifymessage` don't work with SegWit addresses yet. They're remaining TODOs, for this PR or a follow-up. Because of that, several tests unexpectedly run with `-addresstype=legacy` for now.
Tree-SHA512: d425dbe517c0422061ab8dacdc3a6ae47da071450932ed992c79559d922dff7b2574a31a8c94feccd3761c1dffb6422c50055e6dca8e3cf94a169bc95e39e959
This introduces two command line flags (-addresstype and -changetype) which control
the type of addresses/outputs created by the GUI and RPCs. Certain RPCs allow
overriding these (`getnewaddress` and `getrawchangeaddress`). Supported types
are "legacy" (P2PKH and P2SH-multisig), "p2sh-segwit" (P2SH-P2WPKH and P2SH-P2WSH-multisig),
and "bech32" (P2WPKH and P2WSH-multisig).
A few utility functions are added to the wallet to construct different address type
and to add the necessary entries to the wallet file to be compatible with earlier
versions (see `CWallet::LearnRelatedScripts`, `GetDestinationForKey`,
`GetAllDestinationsForKey`, `CWallet::AddAndGetDestinationForScript`).
ebcee1de2 bips: add bip176 (Bits Denomination) (William Casarin)
275b2eeed [qt] change µBTC to bits (William Casarin)
Pull request description:
Now that we have bip176, change "µBTC" to the more colloquial "bits"
Tree-SHA512: eba5e5f89c392728a4f0a3bd81a9779a117b8d72a490390fd031d4e7cc56c2bfee0016aba7ef9535903e8cf2262ce46497283424e378906d0e3bf5b0d2d981c7
6dda059bd [qt] Simplifies boolean expression model && model->haveWatchOnly() (251)
Pull request description:
This PR optimizes the boolean expression `model && model->haveWatchOnly()` to `model->haveWatchOnly()`.
The boolean expression can be optimized because the method `TransactionView::exportClicked` already guards against a potential dereferenced null pointer by returning early if `model` is null.
63a4dc1087/src/qt/transactionview.cpp (L351-L353)
Tree-SHA512: 8bdd0d05bf879745fa39d3ca7524471720ae08ceee9427d5a08776e7b56d18542ae87a6991cd6779e232305f504fdfc77223702b72ecbe231f5f5e98453456dd
The boolean expression model && model->haveWatchOnly() can be simplified to model->haveWatchOnly(), because if (!model || !model->getOptionsModel()) { return; } guards against a potential dereferenced null pointer.
* Now that we have bip176, change "µBTC" to the more colloquial "bits"
* We retain the `µBTC (bits)` description in dropdowns and status bars.
The more concise "bits" is used when appended to numbers.
Signed-off-by: William Casarin <jb55@jb55.com>
GUI wallet uses RBF by default, regardless of -walletrbf.
RPC and debug console in the GUI remain unchanged; they don't
use RBF by default, unless launched with -walletrbf=1.
99ba0c3 Don't use pass by reference to const for cheaply-copied types (bool, char, etc.). (practicalswift)
Pull request description:
Don't use pass by reference to const for cheaply-copied types (`bool`, `char`, etc.).
Tree-SHA512: ccad5e2695dff0b3d6de3e713ff3448f2981168cdac72d73bee10ad346b9919d8d4d588933369e54657a244b8b222fa0bef919bc56d983e1fa64b2004e51b225
a720b92 Remove includes in .cpp files for things the corresponding .h file already included (practicalswift)
Pull request description:
Remove includes in .cpp files for things the corresponding .h file already included.
Example case:
* `addrdb.cpp` includes `addrdb.h` and `fs.h`
* `addrdb.h` includes `fs.h`
Then remove the direct inclusion of `fs.h` in `addrman.cpp` and rely on the indirect inclusion of `fs.h` via the included `addrdb.h`.
In line with the header include guideline (see #10575).
Tree-SHA512: 8704b9de3011a4c234db336a39f7d2c139e741cf0f7aef08a5d3e05197e1e18286b863fdab25ae9638af4ff86b3d52e5cab9eed66bfa2476063aa5c79f9b0346
f05d349 gui: Fix proxy setting options dialog crash (Wladimir J. van der Laan)
Pull request description:
This fixes a crash bug when opening the options dialog.
- Check the return value of split() to avoid segmentation faults due to out of bounds when the user manages to enter invalid proxy settings. This is reported resonably often.
- Move the default proxy/port to a constant instead of hardcoding magic values.
- Factor out some common code.
- Revert #11448 because this proves a more robust replacement, it is no longer necessary and didn't generally solve the issue.
No attempt is made to do full sanity checking on the proxy, so it can still be rejected by the core with an InitError message.
Tree-SHA512: 72b700b7d6c4d3e3410f0c60e9e4facf93d7c6c1a1b6b23957c48b074a045970f518166952859d1ebca8620062cb70d222670a7310bbd6fe50550ec6d04417b5
This fixes a crash bug when opening the options dialog.
- Check the return value of split() to avoid segmentation faults due to
out of bounds when the user manages to enter invalid proxy settings.
This is reported resonably often.
- Move the default proxy/port to a constant instead of hardcoding magic
values.
- Factor out some common code.
- Revert #11448 because this proves a more robust replacement, it is no
longer necessary and didn't generally solve the issue.
No attempt is made to do full sanity checking on the proxy, so it can
still be rejected by the core with an InitError message.
680bc2cbb Use range-based for loops (C++11) when looping over map elements (practicalswift)
Pull request description:
Before this commit:
```c++
for (std::map<T1, T2>::iterator x = y.begin(); x != y.end(); ++x) {
T1 z = (*x).first;
…
}
```
After this commit:
```c++
for (auto& x : y) {
T1 z = x.first;
…
}
```
Tree-SHA512: 954b136b7f5e6df09f39248a6b530fd9baa9ab59d7c2c7eb369fd4afbb591b7a52c92ee25f87f1745f47b41d6828b7abfd395b43daf84a55b4e6a3d45015e3a0
c3055bb Add help-console command to Qt debug console (Luke Mlsna)
Pull request description:
This PR would close issue #9195 by adding documentation for the debug console features (mainly nested commands) which were added in [PR #7783](https://github.com/bitcoin/bitcoin/pull/7783).
The following changes were made to QT debug console code:
- Added a line to the initial message text at the top of the debug console:
> For more information on using this console type **help-console**.
- Added a pseudo-command `help-console` which is hooked after parsing the request, but before actually executing the RPC thread. It prints the following text to the console as if it were a valid RPC response.
> This console accepts RPC commands using the standard syntax.
> example: getblockhash 8
> This console can also accept RPC commands using bracketed syntax.
> example: getblockhash(8)
> A space or a comma can be used to separate arguments for either syntax.
> example: sendtoaddress \<address\> \<amount\>
> sendtoaddress,\<address\>,\<amount\>
> Commands may be nested when specified with the bracketed syntax.
> example: getblockinfo(getblockhash(0),true).
> Result values can be queried with a non-quoted string in brackets.
> example: getblock(getblockhash(0) true)[height]
This seemed like a reasonably sane way to introduce a fake RPC help command, but
Tree-SHA512: 35d73dcef9c4936b8be99e80978169f117c22b94f4400c91097bf7e0e1489060202dcd738d9debdf4c8a7bd10709e2c19d4f625f19e47c4a034f1d6019c0e0f2
c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)
Pull request description:
Closes#11348
Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.
Includes tests and release notes. Things which might need to be considered more:
- there is no 'lock' on the wallets directory, which might be needed?
- because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
- jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in https://github.com/bitcoin/bitcoin/pull/11687
- doc/files.md needs updating (will do soon)
I also considered including a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. #3073), but decided it wasn't super relevant here will just complicate review.
Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
Opt-in RBF checkbox uses less technical jargon and emphasises
the fee bump functionality (at the expense of not mentioning
other uses of RBF).
The transaction confirmation screen uses copy consistent with this.
- Added `help-console` to the list of autocompletion strings
- Implemented requested changes to help message:
- Added an example that uses access-by-index `getblock(getblockhash(0) true)[tx][0]`
- Replace "bracketed syntax" to "parenthesized syntax" where applicable
- Replace "separate" with "delimit"
- Removed `<br>` and `<b>help/help-console</b>` from translation strings, since these parts don't change between languages
- Changed examples to be based off `getblock 0` so they will work even with pruned/no blockchain and `disablewallet` if copied and pasted
- Clarified syntax for queries of named/unnamed result objects.
This makes all include paths in the GUI absolute.
Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.
aed1d90ac [wallet] Change feebumper from class to functions (Russell Yanofsky)
37bdcca3c [refactor] Make feebumper namespace (Russell Yanofsky)
7c4f00919 [trivial] Rename feebumper variables according to project code style (Russell Yanofsky)
Pull request description:
Make feebumper methods static and remove stored state in the class.
Having the results of feebumper calls persist in an object makes process
separation between Qt and wallet awkward, because it means the feebumper object
either has to be serialized back and forth between Qt and wallet processes
between fee bump calls, or that the feebumper object needs to stay alive in the
wallet process with an object reference passed back to Qt. It's simpler just to
have fee bumper calls return their results immediately instead of storing them
in an object with an extended lifetime.
In addition to making feebumper methods static, also:
- Move LOCK calls from Qt code to feebumper
- Move TransactionCanBeBumped implementation from Qt code to feebumper
- Rename CFeeBumper class to FeeBumper (every CFeeBumper reference had to be
updated in this PR anyway so this doesn't increase the size of the diff)
This change was originally part of https://github.com/bitcoin/bitcoin/pull/10244
Tree-SHA512: bf75e0c741b4e9c8912e66cc1dedf0ff715f77ea65fc33f7020d97d9099b0f6448f5852236dac63eea649de7d6fc03b0b21492e2c5140fb7560a39cf085506fd
927f4ff5a GUI: Receive: Remove option to reuse a previous address (Luke Dashjr)
Pull request description:
This was justified by the need to "resent" an invoice, but now that we have the request history, that need should be gone.
Tree-SHA512: 4ade4eb84a21bbbd8dcc3a2c9580d416e113284b5bdf350c22051c233101fe0ee31659c54a7a46e7136f9c999acb61efbbb3f97aeb2fa7b2b1e1daec02ca0837
Change feebumper from a stateful class into a namespace of stateless
functions.
Having the results of feebumper calls persist in an object makes process
separation between Qt and wallet awkward, because it means the feebumper object
either has to be serialized back and forth between Qt and wallet processes
between fee bump calls, or that the feebumper object needs to stay alive in the
wallet process with an object reference passed back to Qt. It's simpler just to
have fee bumper calls return their results immediately instead of storing them
in an object with an extended lifetime.
In addition to making feebumper stateless, also:
- Move LOCK calls from Qt code to feebumper
- Move TransactionCanBeBumped implementation from Qt code to feebumper
Future commit will remove the FeeBumper class. This commit simply places
everything into a feebumper namespace, and changes the enum class name
from BumpeFeeResult to feebumper::Result.
Future PRs will completely refactor this translation unit and touch all
this code so we rename the variables to follow project stlye guidelines
in this preparation commit.
Don't use m_ prefixes for member variables since we're going to remove
the class entirely in the next commits.
d052e3847 [qt] Add use available balance in send coins dialog (CryptAxe)
Pull request description:
This is an alternative to #11098 to handle #11033 where a new button `Use available balance` is added to each entry. When activated, the available balance is calculated by using the coin control (if any) and then it's subtracted the remaining recipient amounts. If this amount is positive then the `Subtract fee from amount` is automatically selected.
Comparing to #11098, this has the advantage to avoid the fair amount division over the recipients and allows to fine adjust the amounts in multiple iterations.
Started from @CryptAxe commit 89e9eda to credit some code.
<img width="965" alt="screen shot 2017-09-13 at 01 32 44" src="https://user-images.githubusercontent.com/3534524/30354518-e1bee31c-9824-11e7-9354-300aa63cdfd0.png">
<img width="964" alt="screen shot 2017-09-13 at 01 44 57" src="https://user-images.githubusercontent.com/3534524/30354598-5731ac9c-9825-11e7-9d5f-8781988ed219.png">
Tree-SHA512: 01d20c13fd8b6c2a0ca1d74d3a9027c6922e6dccd3b08e59d5a72636be7072ed5eca7ebc5d431299497dd3374e83753220ad4174d8bc46dadb4b2f54973036a5
a357293 Use MakeUnique<Db>(...) (practicalswift)
3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift)
8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift)
d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift)
b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift)
29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift)
f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift)
8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift)
73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift)
0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift)
fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift)
5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift)
860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift)
Pull request description:
Use `std::unique_ptr` (C++11) where possible.
Rationale:
1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`)
2. Avoid undefined behaviour (specifically: double `delete`:s)
**Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases.
Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c
5a5e4e9 [wallet] Remove CTransaction&() helper conversion operator from wallet implementation. (Karl-Johan Alm)
Pull request description:
The `CTransaction&()` operator in `CMerkleTx` makes conversion into `CTransaction`s transparent, but was marked as to-be-removed in favor of explicitly getting the `tx` ivar, presumably as the operator can lead to ambiguous behavior and makes the code harder to follow.
This PR removes the operator and adapts callers. This includes some cases of `static_cast<CTransaction>(wtx)` → `*wtx.tx`, which is definitely an improvement.
Tree-SHA512: 95856fec7194d6a79615ea1c322abfcd6bcedf6ffd0cfa89bbdd332ce13035fa52dd4b828d20df673072dde1be64b79c513529a6f422dd5f0961ce722a32d56a
b109a1c Remove redundant nullptr checks before deallocation (practicalswift)
Pull request description:
Rationale:
* `delete ptr` is a no-op if `ptr` is `nullptr`
Tree-SHA512: c98ce769125c4912186a8403cc08a59cfba85b7141af645c709b4c4eb90dd9cbdd6ed8076d50099d1e4ec2bf75917d1af6844082ec42bbb4d94d229a710e051c
ff35de8 [ui] Add toggle for unblinding password fields (Thomas Snider)
Pull request description:
Proposed change for adding the ability to toggle password visibility in the password dialog. This is similar to functionality in most password managers and is specifically added with the use case of password managers in mind - the password in that case is likely pasted twice into both the new password and confirm password fields.
If this is a welcome change, I am open to suggestions on rearranging the layout.
Tree-SHA512: 1823f356f8f941cc584c44de264433e9a573cb8a358efa300a412c4458b5564d8d193969be40859195cf9c8d6768eee895ee22440d51db4f09175f9b4e28bced
6b1891e2c Add Sent and Received information to the debug menu peer list (Aaron Golliver)
8e4aa35ff move human-readable byte formatting to guiutil (Aaron Golliver)
Pull request description:
Makes the peer list display how much you've uploaded/downloaded from each peer.
Here's a screenshot ~~[outdated](https://i.imgur.com/MhPbItp.png)~~, [current](https://i.imgur.com/K1htrVv.png) of how it looks. You can now sort to see who are the peers you've uploaded the most too.
I also moved `RPCConsole::FormatBytes` to `guiutil::formatBytes` so I could use it in the peerlist
Tree-SHA512: 8845ef406e4cbe7f981879a78c063542ce90f50f45c8fa3514ba3e6e1164b4c70bb2093c4e1cac268aef0328b7b63545bc1dfa435c227f28fdb4cb0a596800f5
7a91ceb5e [QA] Add RPC based rescan test (Jonas Schnelli)
c77170fbd [Wallet] add rescanblockchain <start_height> <stop_height> RPC command (Jonas Schnelli)
Pull request description:
A RPC rescan command is much more flexible for the following reasons:
* You can define the start and end-height
* It can be called during runtime
* It can work in multiwallet environment
Tree-SHA512: df67177bad6ad1d08e5a621f095564524fa3eb87204c2048ef7265e77013e4b1b29f991708f807002329a507a254f35e79a4ed28a2d18d4b3da7a75d57ce0ea5
55509f1 Document assumptions that are being made to avoid division by zero (practicalswift)
Pull request description:
Document assumptions (via `assert(…)`:s) that are being made to avoid division by zero.
Rationale:
* Make it clear to human reviewers and non-human static analyzers that what might look like potential division by zero cases are written the way they are intentionally (these cases are currently flagged by various static analyzers).
Tree-SHA512: bbb67b1370afd8f39bda35f9e3a20f4325f017d94cc1bfac3b0d36c9f34c2d95a9efe11efe44db29fb4aadd25d8276d8f0e03c8806ac64f0d21d821912e13b8e
If addrProxy or addrSeparateProxyTor do not have a colon in the string
somewhere in the QSettings storage, then attempting to open the options
dialog will cause the entire program to crash.
634e38ca7 [Tests] Add Qt GUI tests to Overview and ReceiveCoin Page (Anditto Heristyo)
Pull request description:
I've added some Qt wallet tests based on #9974, namely the input & buttons on ReceiveCoin.
Tree-SHA512: f4223827145e35c2abee83a6ca777498bebcff3825fece10fbb1dbfd1f6bb017d3f2c0521662854b4407cdeee9c6a527269ab9cc28e0dc85c11b668155fcd195
3a4401a [Qt] Terminate string *pszExePath after readlink and without using memset (practicalswift)
Pull request description:
Terminate string `*pszExePath` after `readlink` and before passing to operator `<<`.
* `ssize_t readlink(const char *pathname, char *buf, size_t bufsiz)` does not append a null byte to `buf`.
* Operator `<<` expects a null-terminated string.
Tree-SHA512: fc18844bb23059fead8db0cb9b4b4ba6188f58e3f19ab4719c2737cc5dd6df23ae7d4804ef2820d39b334204a48ee3de1d202c272bcd156e60761af2fcb9349d
8213838 [Qt] tolerate BIP173/bech32 addresses during input validation (Jonas Schnelli)
06eaca6 [RPC] Wallet: test importing of native witness scripts (NicolasDorier)
fd0041a Use BIP173 addresses in segwit.py test (Pieter Wuille)
e278f12 Support BIP173 in addwitnessaddress (Pieter Wuille)
c091b99 Implement BIP173 addresses and tests (Pieter Wuille)
bd355b8 Add regtest testing to base58_tests (Pieter Wuille)
6565c55 Convert base58_tests from type/payload to scriptPubKey comparison (Pieter Wuille)
8fd2267 Import Bech32 C++ reference code & tests (Pieter Wuille)
1e46ebd Implement {Encode,Decode}Destination without CBitcoinAddress (Pieter Wuille)
Pull request description:
Builds on top of #11117.
This adds support for:
* Creating BIP173 addresses for testing (through `addwitnessaddress`, though by default it still produces P2SH versions)
* Sending to BIP173 addresses (including non-v0 ones)
* Analysing BIP173 addresses (through `validateaddress`)
It includes a reformatted version of the [C++ Bech32 reference code](https://github.com/sipa/bech32/tree/master/ref/c%2B%2B) and an independent implementation of the address encoding/decoding logic (integrated with CTxDestination). All BIP173 test vectors are included.
Not included (and intended for other PRs):
* Full wallet support for SegWit (which would include automatically adding witness scripts to the wallet during automatic keypool topup, SegWit change outputs, ...) [see #11403]
* Splitting base58.cpp and tests/base58_tests.cpp up into base58-specific code, and "address encoding"-code [see #11372]
* Error locating in UI for BIP173 addresses.
Tree-SHA512: 238031185fd07f3ac873c586043970cc2db91bf7735c3c168cb33a3db39a7bda81d4891b649685bb17ef90dc63af0328e7705d8cd3e8dafd6c4d3c08fb230341
This eases the during-type validation to allow Bech32 chars.
Once the focus has been lost, the address will be properly verified through IsValidDestinationString
7b137aced [Qt] Add delay before filtering transactions Fixes 3141 (Lucas Betschart)
Pull request description:
As discussed in https://github.com/bitcoin/bitcoin/issues/3141.
This adds a QTimer pause of 200ms before start to filter so it should be possible to filter big data sets easier.
Tree-SHA512: ee599367794eac2c5b8bc7ecac47f44295e40c0ff543ff2f2c4860590f917b59b1cfb273fa564e6eb4c44016c0ef412d49f1a8f1b36b07e034022f51bb76653c
13baf72 Replace save|restoreWindowGeometry with Qt functions (MeshCollider)
Pull request description:
Alternative to https://github.com/bitcoin/bitcoin/pull/11208, closes https://github.com/bitcoin/bitcoin/issues/11207
According to the [Qt documentation](https://doc.qt.io/qt-4.8/qwidget.html#restoreGeometry), restoreGeometry does all the checks we need, so it would be better to rely on them instead of doing it ourselves.
~Haven't tested this properly yet, hence the WIP.~
Gives expected behavior exactly as the other system apps do based on my tests. Only potential issue is the case when the GUI is almost entirely offscreen with only a single strip of pixels, its not really possible to see the GUI, but if you know it's there you can bring it back onscreen with just the mouse. And that's exactly how notepad behaves on Windows so I don't think its a real issue.
This also gives much better behavior when closing a maximized window, currently (0.15.0 release) a maximized window will save the window size on close, and then reopen as a not-maximized but still that size, which is really annoying. This reopens as maximized.
Gitian build here: https://bitcoin.jonasschnelli.ch/build/305
Tree-SHA512: a8bde14793b4316192df1fa2eaaeb32b44d5ebc5219c35252379840056cd737a9fd162625fd715987f275fec8375334ec1ec328dbc671563f084c611a938985c
723aa1b qt: Backup former GUI settings on `-resetguisettings` (Wladimir J. van der Laan)
Pull request description:
Writes the GUI settings to `guisettings.bak` in the data directory before wiping them. This can be used to retroactively troubleshoot issues (e.g. #11262) where `-resetguisettings` solves the problem.
(as discussed in yesterday's IRC meeting)
Tree-SHA512: c64f5052d992eb02057ba285435f143c42d0cc456144a4c565e1c87be833737f9df750d0aee10810f85047c820d9b4f9f22fd94a6f09f4b28a9cf41b63a56586
Writes the GUI settings to `guisettings.bak` in the data directory
before wiping them. This can be used to retroactively troubleshoot
issues (e.g. #11262) where `-resetguisettings` solves the problem.
A C-style cast is equivalent to try casting in the following order:
1. const_cast(...)
2. static_cast(...)
3. const_cast(static_cast(...))
4. reinterpret_cast(...)
5. const_cast(reinterpret_cast(...))
By using static_cast<T>(...) explicitly we avoid the possibility
of an unintentional and dangerous reinterpret_cast. Furthermore
static_cast<T>(...) allows for easier grepping of casts.
fdc3293 Document assumptions that are being made to avoid NULL pointer dereferences (practicalswift)
Pull request description:
Document assumptions (via `assert(…)`:s) that are being made avoid `NULL` pointer dereferences.
Rationale:
* Make it clear to human reviewers and non-human static analyzers that what might look like potential `NULL` pointer dereferences are written the way they are intentionally (these cases are currently flagged by various static analyzers).
Tree-SHA512: b424328195e2680e1e4ec546298f718c49e5ad182147dc004de580693db1b50eec4065e1c4f232bdb302baa12954265a50ba21cb5ba4ff30248535b2de778672
f151f5f50 [macOS] remove Growl support, remove unused code (Jonas Schnelli)
Pull request description:
There is no longer a reason to support Growl.
A) It went to pay-ware since a couple of years
B) Since OSX 10.8, the operating system has its own modal notification options (Notification Center).
This PR removes support for Growl.
OSX notification centre is still supported after this PR.
Tree-SHA512: eee18098d7354c4e98f927bca9963d4843ff6bceee74795f73a66c27eed33efaac00ec2cabde8807efcbc936b16ab712249006fa13f5a3f55e4d44d163f5f9a0
592404f03 Changing &vec[0] to vec.data(), what 9804 missed (MeshCollider)
Pull request description:
This just continues the work of https://github.com/bitcoin/bitcoin/pull/9804
Modifies a lot of `&vector[]`'s to `vector.data()`'s across all the files including tests, just the stuff that 9804 missed
Tree-SHA512: dd1a9dffb999dea4fba78dcc91fe02f90250db86f5c74948e1ff3e8b4036b2154b600555eaa04dece5368920aae3513bc36425dc96e4319ca1041b0928a6b656
c8d38abd6 Refactor tipUpdate as per style guide (MeshCollider)
3b69a08c5 Fix division by zero in time remaining (MeshCollider)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/10291, https://github.com/bitcoin/bitcoin/issues/11265
progressDelta may be 0 (or even negative according to 11265), this checks for that and prints unknown if it is, because we cannot calculate an estimate for the time remaining (would be infinite or negative).
Tree-SHA512: bc5708e5ed6e4670d008219558c5fbb25709bd99a32c98ec39bb74f94a0b7fa058f3d03389ccdd39e6723e6b5b48e34b13ceee7c051c2db631e51d8ec3e1d68c
9b348ff9e Fix memory leaks in qt/guiutil.cpp (Dan Raviv)
Pull request description:
on macOS:
`listSnapshot` was leaking in `findStartupItemInList()`
`bitcoinAppUrl` was leaking in `[Get|Set]StartOnSystemStartup()`
Tree-SHA512: dd49e1166336cf4f20035d21930f2f99f21f1d9f91a1101b1434a23dd0b92d402ac7efb177473c758d8af1dbab8d8750485583231c5b5854203d2493f0b43e73
ee4d1493e Drop upgrade-cancel callback registration for a generic "resumeable" (Matt Corallo)
Pull request description:
Instead of passing a StartShutdown reference all the way up from
txdb, give ShowProgress a "cancelable" boolean, as StartShutdown
is pretty much always what you'll want to use to cancel. Use the
same boolean to allow cancel during initial block verification.
Tree-SHA512: 515817aaa4b9e3e856200e00be9c2d44ecfa2d4f288fe3e02116105fe85de2650c13076ee7e45396ec1ce6ab45e53b0477cddda7cfdee5b3bd0589cb81a4c346
d2be7b25b Typo in optionsdialog.ui Tooltip displayed ampersand incorrectly, & should be in text. (James Evans)
Pull request description:
Tooltip displayed ampersand incorrectly, & should be in text property rather than tooltip so that access key is correctly displayed for accessibility.
Tree-SHA512: 331848207317d37d4d9db40119d0b7ae9a276d06cd1b057cd0e87d508e1aa769b785246ca30ca9156db632798ec9f68ba8bf78cf42904267b4187bd27cfced35
864cd2787 Move CBitcoinAddress to base58.cpp (Pieter Wuille)
5c8ff0d44 Introduce wrappers around CBitcoinAddress (Pieter Wuille)
Pull request description:
This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`.
As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions.
This change is far from complete. In follow-ups I'd like to:
* Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so.
* Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere).
* Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`.
However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction.
Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e
This patch removes the need for the intermediary Base58 type
CBitcoinAddress, by providing {Encode,Decode,IsValid}Destination
function that directly operate on the conversion between strings
and CTxDestination.
dea086f49 Stop test_bitcoin-qt touching ~/.bitcoin (MeshCollider)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/11192
The directory remains unused, but this stops the tests touching ~/.bitcoin at all (namely creating it if it doesn't exist)
Tree-SHA512: e59ad6b83dbc5ea2fb2761994c09933721d29668b0eef09b9d938a4ee1c67871c5125c57483ee0ea25f2385e308d275d86bcb9087dd4d502923013b4f3dbac82
a1ea1cfbd qt: Use IsMine to validate custom change address (Chris Moore)
Pull request description:
Fixes#11137Closes#11184 (which was accidentally opened against 0.15 branch)
Tree-SHA512: a20a59b4f36c1471a9c84bcc7c69048576d1f413104c299a7ed9ba221f28eddf93d727fca2926420ea5d0dd9aba582924f26a5acd44d995039b7202c73eb53bc
d1138e362 Remove redundant testutil files (MeshCollider)
Pull request description:
The only function in testutil.cpp, `GetTempPath()` simply called `fs::temp_directory_path()` directly. This just tidies things up by removing that redundant function and the file containing it
I can understand wanting a general util file for tests to use, but if there's nothing in it, we might as well remove it, it can always be added back later when it's put to use.
Tree-SHA512: b923f99acf33328743755368a1aa90f5da4a7d5f61b163a4b0b894275c98db80a91edf8f051fbfb4893d970fda5a9078aae78a2672867ff521c4ca4b653c71c0
ec6902d0e rpc: Push down safe mode checks (Andrew Chow)
Pull request description:
This contains most of the changes of #10563 "remove safe mode" by @achow101, but doesn't remove the safe mode yet, but put an `ObserveSafeMode()` check in (all 23) individual calls which used to have okSafeMode=false.
This cleans up the ugly "okSafeMode" flag from the dispatch tables, which is not a concern for the RPC server.
Extra-author: Wladimir J. van der Laan <laanwj@gmail.com>
Tree-SHA512: eee0f251fe2f38f122e7391e3c4e98d6a1e2757f3b718d6b560ad835ae94f11490865a0aef893e90b5fe298165932c8dd8298224173ac2677a5245cd532bac6e
This contains most of the changes of 10563 "remove safe mode", but doesn't
remove the safe mode yet, but put an `ObserveSafeMode()` check in
individual calls with okSafeMode=false.
This cleans up the ugly "okSafeMode" flag from the dispatch tables,
which is not a concern for the RPC server.
Extra-author: Wladimir J. van der Laan <laanwj@gmail.com>
f01103c MOVEONLY: Init functions wallet/wallet.cpp -> wallet/init.cpp (Russell Yanofsky)
e7fe320 MOVEONLY: Fee functions wallet/wallet.cpp -> wallet/fees.cpp (Russell Yanofsky)
d97fe20 Move some static functions out of wallet.h/cpp (Russell Yanofsky)
Pull request description:
This just moves some static wallet fee and init functions out of `wallet/wallet.cpp` and into new `wallet/fees.cpp` and `wallet/init.cpp` source files. There is one commit updating declarations and callers, followed by two MOVEONLY commits actually moving the function bodies.
This change is desirable because wallet.h/cpp are monolithic and hard to navigate, so pulling things out and grouping together pieces of related functionality should improve the organization.
Another motivation is the wallet process separation work in https://github.com/bitcoin/bitcoin/pull/10973, where (at least initially) parameter parsing and fee estimation are still done in the main process rather than the wallet process, and having functions that run in different processes scrambled up throughout wallet.cpp is unnecessarily confusing.
Tree-SHA512: 6e6982ff82b2ab4e681c043907e2b1801ceb9513394730070f16c46ad338278a863f5b3759aa13db76a259b268b1c919c81f4e339f0796a3cfb990161e8c316d
Instead of passing a StartShutdown reference all the way up from
txdb, give ShowProgress a "resumeable" boolean, which is used to
inform the user if the action will be resumed, but cancel is always
allowed by just calling StartShutdown().
8f2f1e0 wallet: Avoid second mapWallet lookup (João Barbosa)
Pull request description:
All calls to `mapWallet.count()` have the intent to detect if a `txid` exists and most are followed by a second lookup to retrieve the `CWalletTx`.
This PR replaces all `mapWallet.count()` calls with `mapWallet.find()` to avoid the second lookup.
Tree-SHA512: 96b7de7f5520ebf789a1aec1949a4e9c74e13683869cee012f717e5be8e51097d068e2347a36e89097c9a89f1ed1a1529db71760dac9b572e36a3e9ac1155f29
6e8c48dc5 Add const to methods that do not modify the object for which it is called (practicalswift)
Pull request description:
Tree-SHA512: a6888111ba16fb796e320e60806e1a77d36f545989b5405dc7319992291800109eab0b8e8c286b784778f41f1ff5289e7cb6b4afd7aec77f385fbcafc02cffc1
f42fc1d50 doc: spelling fixes (klemens)
Pull request description:
patch contains some spelling fixes ( just in comments ) as found by a bot ( http://www.misfix.org, https://github.com/ka7/misspell_fixer ).
Tree-SHA512: ba6046cfcd81b0783420daae7d776be92dd7b85a593e212f8f1b4403aca9b1b6af12cef7080d4ea5ed4a14952fd25e4300109a59c414e08f5395cdb9947bb750
