ecf9b25 remove unused fNoncriticalErrors variable from CWalletDB::FindWalletTx (Pierre Rochard)
Pull request description:
The `CWalletDB::FindWalletTx` method was patterned after `CWalletDB::LoadWallet`, where `fNoncriticalErrors` is used when a tx check fails in `ReadKeyValue`.
Since `FindWalletTx` is only used by methods which are zapping txs, it makes sense that `ReadKeyValue` is not called and the tx is not checked, so I think that deleting the unused `fNoncriticalErrors` boolean variable and its conditional statement is appropriate.
Tree-SHA512: 0976eae97522719fdaeca1fb3f4a080561e46c06d0b8dc75e14262c6bc242998db3f7057183a230a1d7e4ac5fc348e9059f545b7d718ebbcdf6dcdfc63bcc286
6f39ac0 Add test for decoderawtransaction bool (MeshCollider)
bbdbe80 Add iswitness parameter to decode- and fundrawtransaction RPCs (MeshCollider)
Pull request description:
Suggested in https://github.com/bitcoin/bitcoin/pull/10481#issuecomment-325244946, this adds the option to explicitly choose whether a serialized transaction should be decoded as a witness or non-witness transaction rather than relying on the heuristic checks in #10481. The parameter defaults to relying on #10481 if not included, but it overrides that if included.
Tree-SHA512: d4846a5bb7d64dc19c516445488b00af329fc1f4181d9dfdf9f2382a086568edc98250a4ac7594e24a1bc231dfdee53c699b12c8380c355b920a67cc6770b7a9
c79d73d Clarify getbalance meaning a tiny bit in response to questions. (Matt Corallo)
Pull request description:
Someone was asking why getbalance "*" was more "correct" than getbalance, which should rarely be true...spendzeroconfchange was the issue.
Tree-SHA512: 90201cad1acec5161aee469fb4c6d737a0eb90f8380ac93abf0e41e0f02d120afcc3e2e873e5096d3655bb63bbd16fe99e72452f308d72e69139c7f6bb2d745e
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
03a5dc9 [wallet] Make CWallet::FundTransaction atomic (João Barbosa)
95d4450 [wallet] Tidy up CWallet::FundTransaction (João Barbosa)
Pull request description:
This PR fixes a race for `setLockedCoins` when `lockUnspents` is true. For instance, it should not be possible to use the same unspent in concurrent `fundrawtransaction` calls.
Now the `cs_main` and `cs_wallet` locks are held during `CreateTransaction` and `LockCoin`(s). Also added some style nits around the change.
Tree-SHA512: ccf383c0c5f6db775655a3e9ccd200c3bd831a83afae2b7c389564c74f7227f5bea86a4775727de2c3603b188f383f8a12d3f9d6d94f7887865c31c94ce95ef6
9c8eca7 Split up key and script metadata for better type safety (Russell Yanofsky)
Pull request description:
Suggested by @TheBlueMatt
https://github.com/bitcoin/bitcoin/pull/11403#discussion_r155599383
Combining the maps was probably never a good arrangement but is more
problematic now in presence of WitnessV0ScriptHash and WitnessV0KeyHash types.
Tree-SHA512: 9263e9c01090fb49221e91d88a88241a9691dda3e92d86041c8e284306a64d3af5e2438249f9dcc3e6e4a5c11c1a89f975a86d55690adf95bf2636f15f99f92a
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
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
49bd659 tests: move pwalletMain to wallet test fixture (Wladimir J. van der Laan)
Pull request description:
Scope the variable instead of using an external global; this is how test fixtures are intended to be used.
Followup to #11713.
Tree-SHA512: 7d5bda93cdfe1329c8fe39bd72965906e36dad72fbb5d344ebedf26e66b1857510d01a3c2872d7f718fdeb23365e6ba71991aafe68e82781c6767a086b6d1590
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
28f8b66 Diagnose unsuitable outputs in lockunspent(). (Eelis)
Pull request description:
Fixes#2667.
This is a simplified version of pull request #3574, which was abandoned by its author.
I added some tests as well.
Tree-SHA512: e63e00dec8b1b232079380183805cb0b0b18c78ea6bea769837949aab984689d7f68b2ccfe66b1873517b040b9e616ce0eb058575c3d4382aa8c26eebcf1f14e
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
89f0312 Remove redundant pwallet nullptr check (Matt Corallo)
c4784b5 Add a dev notes document describing the new wallet RPC blocking (Matt Corallo)
3ea8b75 Give ZMQ consistent order with UpdatedBlockTip on scheduler thread (Matt Corallo)
cb06edf Fix wallet RPC race by waiting for callbacks in sendrawtransaction (Matt Corallo)
e545ded Also call other wallet notify callbacks in scheduler thread (Matt Corallo)
17220d6 Use callbacks to cache whether wallet transactions are in mempool (Matt Corallo)
5d67a78 Add calls to CWallet::BlockUntilSyncedToCurrentChain() in RPCs (Matt Corallo)
5ee3172 Add CWallet::BlockUntilSyncedToCurrentChain() (Matt Corallo)
0b2f42d Add CallFunctionInQueue to wait on validation interface queue drain (Matt Corallo)
2b4b345 Add ability to assert a lock is not held in DEBUG_LOCKORDER (Matt Corallo)
0343676 Call TransactionRemovedFromMempool in the CScheduler thread (Matt Corallo)
a7d3936 Add a CValidationInterface::TransactionRemovedFromMempool (Matt Corallo)
Pull request description:
Based on #10179, this effectively reverts #9583, regaining most of the original speedups of #7946.
This concludes the work of #9725, #10178, and #10179.
See individual commit messages for more information.
Tree-SHA512: eead4809b0a75d1fb33b0765174ff52c972e45040635e38cf3686cef310859c1e6b3c00e7186cbd17374c6ae547bfbd6c1718fe36f26c76ba8a8b052d6ed7bc9
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.
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
dd9bb25 Fix code style in keystore.cpp/crypter.cpp (Jonas Schnelli)
208fda6 CCrypter: move relevant implementation out of the header (Jonas Schnelli)
3155fd2 CKeystore: move relevant implementation out of the header (Jonas Schnelli)
Pull request description:
Tree-SHA512: 4ce73cca5609199b74b8ff2614ee2b6af949545a1332a3a0135c6453c98665d2b0da171c1e390c9a2aec6b12b7fad931ec90084bb7c2defe243786bfc70daf60
6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)
Pull request description:
Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See #9573.
Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
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
7963335 Fix -disablewallet default value (João Barbosa)
b411c2a Improve -disablewallet parameter interaction (João Barbosa)
Pull request description:
The first commit logs a message for each configured wallet if `-disablewallet` is set:
```
bitcoind -printtoconsole -regtest -disablewallet -wallet=foo -wallet=bar
...
WalletParameterInteraction: parameter interaction: -disablewallet -> ignoring -wallet=foo
WalletParameterInteraction: parameter interaction: -disablewallet -> ignoring -wallet=bar
```
It also moves up the `-disablewallet` check which avoids the unnecessary `-wallet` soft set.
The second commit fixes the default value of `-disablewallet`, currently the value is correct, but it should use `DEFAULT_DISABLE_WALLET`.
The third commit can be dropped or squashed, just took the opportunity to fix the coding style there.
Tree-SHA512: bec13d2b2be5adf4680c77212020ed27dd05f15c4c73542d2005d91108bf704e2df1707ed2bec696e584ecd40eff7a63e25201fd70400222aa5a8da6aed6afeb
c098c58 Wrap dumpwallet warning and note scripts aren't dumped (MeshCollider)
a38bfbc Add wallet backup text to import*, add* and dumpwallet RPCs (MeshCollider)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/11243
Adds "Requires a new wallet backup" text to `addwitnessaddress`, `importprivkey`, `importmulti`, `importaddress`, `importpubkey`, and `addmultisigaddress`. Also adds a warning to `dumpwallet` that backing up the seed alone is not sufficient to back up non-HD addresses
Tree-SHA512: 76d7cdca54d5b458acf479154620322391b889922525fddd6153f4164cfee393ad743757400cb8f6b1b30f24947df68ea9043b4e509f7df77a8fa05dda370933
720d9e8fa [Wallet] always show help-line of wallet encryption calls (Jonas Schnelli)
Pull request description:
We do currently show/hide the wallet encryption RPC calls from the help if the current wallet.
In case of an encrypted wallet, `encryptwallet` is hidden and `walletpassphrasechange`, `walletpassphrasechange` and `walletlock` do appear in the help.
This is no longer ideal in case of multiwallet due to the fact that one may want help infos in order to target a specific wallet.
IMO its preferable to have a static help screen (show everything always). The currently show/hidden calls do handle the possible invalid encryption-state fine.
Fixes#11588
Tree-SHA512: 513fecd15248a31361f5143685e8cdeb63dfd3fa7120828917e1db54d936dc3db60d48ce46efa5c3a563a48157fe962689879856eeeed53f904686b12aec204e
5d465e396 Ensure backupwallet fails when attempting to backup to source file (Tomas van der Wansem)
Pull request description:
Previous behaviour was to destroy the wallet (to zero-length)
This fixes#11375
Tree-SHA512: bfd1738659b15e3f23b6bbdf55ec12269c62c820bf701daec19500b52bd5845bb5516733c6f76f36197eb155182a8a35dc239ad4de2ef1e59bbb0f124a455759
Change suggested by Cory Fields <cory-nospam-@coryfields.com> who noticed
listsinceblock would ignore invalid block hashes causing it to return a
completely unfiltered list of transactions.
Make sure wallet databases have unique fileids. If they don't, throw an error.
BDB caches do not work properly when more than one open database has the same
fileid, because values written to one database may show up in reads to other
databases.
Bitcoin will never create different databases with the same fileid, but users
can create them by manually copying database files.
BDB caching bug was reported by Chris Moore <dooglus@gmail.com>
https://github.com/bitcoin/bitcoin/issues/11429Fixes#11429
0aacfa4 Remove accidental stray semicolon (practicalswift)
68feb49 Use nullptr instead of NULL (practicalswift)
c6b07fd Fix a vs. an typo (practicalswift)
Pull request description:
Minor cleanups:
* Typo: Fix a vs. an typo
* Typo: Remove accidental stray semicolon (only remaining instance in repo)
* Correctness/consistency: Use `nullptr` instead of `NULL` (only remaining instance in repo)
Tree-SHA512: 47142e557da9d3fa0b532c46edeb7f356a1f6dc5973e60b0e496badff3581ff696eade542d49da777ac7f2e895129cc8487ccdb1984ff828434fa86f9a56dad0
7104de8 [wallet] Fix leak in CDB constructor (João Barbosa)
Pull request description:
First commit fixes a minor leak.
Second commit improves the constructor in the failure cases.
Tree-SHA512: 5165413d60ed9fc28203c9fe128adbba03a9ea9e9aa3734d9ea2522dafd815ba0fb8b90fd0809dbc06eb3ad360e7764de01dadf653ade3350fe86f6b8f04bc90
Now using a std::unique_ptr, the Db instance is correctly released
when CDB initialization fails.
The internal CDB state and mapFileUseCount are only mutated when
the CDB initialization succeeds.
This avoid calling out to mempool state during coin selection,
balance calculation, etc. In the next commit we ensure all wallet
callbacks from CValidationInterface happen in the same queue,
serialized with each other. This helps to avoid re-introducing one
of the issues described in #9584 [1] by further disconnecting
wallet from current chain/mempool state.
Thanks to @morcos for the suggestion to do this.
Note that there are several race conditions introduced here:
* If a user calls sendrawtransaction from RPC, adding a
transaction which is "trusted" (ie from them) and pays them
change, it may not be immediately used by coin selection until
the notification callbacks finish running. No such race is
introduced in normal transaction-sending RPCs as this case is
explicitly handled.
* Until Block{Connected,Disconnected} and
TransactionAddedToMempool calls also run in the CSceduler
background thread, there is a race where
TransactionAddedToMempool might be called after a
Block{Connected,Disconnected} call happens.
* Wallet will write a new best chain from the SetBestChain
callback prior to having processed the transaction from that
block.
[1] "you could go to select coins, need to use 0-conf change, but
such 0-conf change may have been included in a block who's
callbacks have not yet been processed - resulting in thinking they
are not in mempool and, thus, not selectable."
This prevents the wallet-RPCs-return-stale-info issue from being
re-introduced when new-block callbacks no longer happen in the
block-connection cs_main lock
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
Fixes#11462. Updated documentation for importprivkey function to use the correct name for the first argument.
Also updates a call to importprivkey to use named args in functional test.
c626dcb50 Make fUseCrypto atomic (MeshCollider)
731065b11 Consistent parameter names in txdb.h (MeshCollider)
35aeabec6 Make fReindex atomic to avoid race (MeshCollider)
58d91af59 Fix race for mapBlockIndex in AppInitMain (MeshCollider)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/11106
Also makes fReindex atomic as suggested in @TheBlueMatt comment below, and makes fUseCrypto atomic as suggested in 10916
d291e7635b just renames the parameters in the txdb header file to make them consistent with those used in the cpp file, noticed it when looking for uses of fReindex
Tree-SHA512: b378aa7289fd505b76565cd4d48dcdc04ac5540283ea1c80442170b0f13cb6df771b1a94dd54b7fec3478a7b4668c224ec9d795f16937782724c5d020edd3a42
0cd9273 rpc: Prevent `dumpwallet` from overwriting files (Wladimir J. van der Laan)
Pull request description:
Prevent arbitrary files from being overwritten by `dumpwallet`. There have been reports that users have overwritten wallet files this way. It may also avoid other security issues.
Fixes#9934. Adds mention to release notes and adds a test.
Tree-SHA512: 268c98636d40924d793b55a685a0b419bafd834ad369edaec08227ebe26ed4470ddea73008d1c4beb10ea445db1b0bb8e3546ba8fc2d1a411ebd4a0de8ce9120
cffe85f Skip sys::system(...) call in case of empty command (practicalswift)
6fb8f5f Check that -blocknotify command is non-empty before executing (practicalswift)
Pull request description:
Check that `-blocknotify` command is non-empty before executing.
To make the `BlockNotifyCallback(...)` (`-blocknotify`) behaviour consistent with that of:
* `AlertNotify(...)` (`-alertnotify`)
* `AddToWallet(...)` (`-walletnotify`)
Tree-SHA512: 18272166793a5a8b9cc2a727bfbcea53d38c329a55bc975c02db601329d608a61c20e026ce4b616193ecd3810dca4d3e2cb3bf773898a51872008a8dba96763e
1789e4675 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo)
53a6590f4 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo)
0b1b9148c Remove countMaskInv caching in bench framework (Matt Corallo)
Pull request description:
This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion.
Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
bf64c3cb3 Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos)
04f78ab5b Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos)
fd849e1b0 Change AcceptToMemoryPool function signature (Alex Morcos)
Pull request description:
First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior.
Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](https://github.com/bitcoin/bitcoin/pull/9602#issue-202197849)
Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303.
Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
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
603efe9fc Fix parameter name typo in ErasePurpose walletdb method. (Pierre Rochard)
Pull request description:
The header file has the correct method signature and the one usage in CWallet::DelAddressBook is correctly passing in EncodeDestination(address)
Tree-SHA512: ee0808a74111fd23a1c47ba5ab51de151fdd33a01d92895671e562ac184cbcb33180a3ff26c22e5717595592097b9fa33deca9878d89ce8d34687f09cfadfcf0
d01a968 wallet: update stored witness in AddToWallet (Suhas Daftuar)
Pull request description:
Replace witness-stripped wallet transactions with full transactions;
this can happen when upgrading from a pre-segwit wallet to a segwit-
aware wallet.
Tree-SHA512: a348b16b38ae738fa75cf7d3ff50ebd0d0071d5d6061c9a10dc3325fc34f6bc96a67aea21fde460ca20f6178768ee0af04d6d8785b35647f436a9083c4270b07
Prevent arbitrary files from being overwritten. There have been reports
that users have overwritten wallet files this way. It may also avoid
other security issues.
Fixes#9934. Adds mention to release notes and adds a test.
2a07f878a Refactor: Modernize disallowed copy constructors/assignment (Dan Raviv)
Pull request description:
Use C++11's better capability of expressing an interface of a non-copyable class by publicly deleting its copy ctor and assignment operator instead of just declaring them private.
Tree-SHA512: 878f446be5a136bb2a90643aaeaca62948b575e6ef71ccc5b4b8f373e66f36ced00665128f36504e0ccfee639863d969329c4276154ef9f2a9de9137f0801e01
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
Use C++11's better capability of expressing an interface of a non-copyable class by publicly deleting its copy ctor and assignment operator instead of just declaring them private.
Combine fLimitFree and fOverrideMempoolLimit into a single boolean:
bypass_limits. This is used to indicate that mempool limiting based on feerate
should be bypassed. It is used when readding transactions from a reorg and then
the mempool is trimmed to size after all transactions are added and they can be
evaluated in the context of their descendants. No changes to behavior.
713a92073 Remove usehd option and warn when it is used (Andrew Chow)
d4c18f733 Bump wallet version number to 159900 (Andrew Chow)
Pull request description:
Bump the wallet version number to 159900 so that new wallets made without a default key will no longer work on previous versions at all. Also remove the `usehd` option to avoid weird interaction with wallet version numbers and HD-ness of wallets.
Tree-SHA512: dd7965505bfad6a926c79afd423236f509229a398a8398076f8d57d90a5974243f9459a61225c4daee560c796f427445c9e55a3ad528a3a97a9123ca6a1269ab
