1967d2a qt: Increase BLOCK_CHAIN_SIZE constants (Wladimir J. van der Laan)
Pull request description:
- Increase `BLOCK_CHAIN_SIZE` from 120GB to 150GB
- Increase `CHAIN_STATE_SIZE` from 2GB to 4GB
I took the local sizes of the blocks and chainstate directory, and added a bit extra to accomodate the near future (15GB for the chain and 1GB for the chainstate).
Tree-SHA512: 76ec7770bd3a30380b0224a0f307cdad14c8227ef726dd55738cebe9d894430865aff11e05a793fd3e60d8fe019dbb392f574c1fb63ec746618b4460ed64bd0c
11dd29b [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest& request) (practicalswift)
Pull request description:
When running `test_bitcoin` under Valgrind I found the following issue:
```
$ valgrind src/test/test_bitcoin
...
==10465== Use of uninitialised value of size 8
==10465== at 0x6D09B61: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B1BB: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B36C: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D17699: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x4CAAD7: operator<< (ostream:171)
==10465== by 0x4CAAD7: formatValue<ServiceFlags> (tinyformat.h:345)
==10465== by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl<ServiceFlags>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:523)
==10465== by 0x1924D4: format (tinyformat.h:510)
==10465== by 0x1924D4: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:803)
==10465== by 0x553A55: vformat (tinyformat.h:947)
==10465== by 0x553A55: format<ServiceFlags> (tinyformat.h:957)
==10465== by 0x553A55: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<ServiceFlags>(char const*, ServiceFlags const&) (tinyformat.h:966)
==10465== by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462)
==10465== by 0x28EDB5: CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_tests.cpp:31)
==10465== by 0x293947: rpc_tests::rpc_togglenetwork::test_method() (rpc_tests.cpp:88)
==10465== by 0x2950E5: rpc_tests::rpc_togglenetwork_invoker() (rpc_tests.cpp:84)
==10465== by 0x182496: invoke<void (*)()> (callback.hpp:56)
==10465== by 0x182496: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
...
```
The read of the uninitialized variable `nLocalServices` is triggered by `g_connman->GetLocalServices()` in `getnetworkinfo(const JSONRPCRequest& request)` (`net.cpp:462`):
```c++
UniValue getnetworkinfo(const JSONRPCRequest& request)
{
...
if(g_connman)
obj.push_back(Pair("localservices", strprintf("%016x", g_connman->GetLocalServices())));
...
}
```
The reason for the uninitialized `nLocalServices` is that `CConnman::Start(...)` is not called
by the tests, and hence the initialization normally performed by `CConnman::Start(...)` is
not done.
This commit adds a method `Init(const Options& connOptions)` which is called by both the
constructor and `CConnman::Start(...)`. This method initializes `nLocalServices` and the other
relevant values from the supplied `Options` object.
Tree-SHA512: d8742363acffd03b2ee081cc56840275569e17edc6fa4bb1dee4a5971ffe4b8ab1d2fe7b68f98a086bf133b7ec46f4e471243ca08b45bf82356e8c831a5a5f21
- Increase `BLOCK_CHAIN_SIZE` from 120GB to 150GB
- Increase `CHAIN_STATE_SIZE` from 2GB to 4GB
I took the local sizes of the blocks and chainstate directory, and added
a bit extra to accomodate the near future (15GB for the chain and 1GB
for the chainstate).
49d903e Eliminate fee overpaying edge case when subtracting fee from recipients (Alex Morcos)
Pull request description:
I'm not sure if this is the cause of the issue in #10034 , but this was a known edge case. I just didn't realize how simple the fix is.
Could use a couple more eyes to make sure nothing silly can go wrong here, but if we all agree it's this simple, we can add this as another 0.15 bug fix.
Tree-SHA512: db1dd1e83363a3c231267b626d3a388893ee70ba1972056fe2c339c5c9e4fbfd30f7fe837c30cc7be884d454797fd4c619b9d631a8d5eeb55cdb07402a83acb3
When running test_bitcoin under Valgrind I found the following issue:
```
$ valgrind src/test/test_bitcoin
...
==10465== Use of uninitialised value of size 8
==10465== at 0x6D09B61: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B1BB: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D0B36C: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x6D17699: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==10465== by 0x4CAAD7: operator<< (ostream:171)
==10465== by 0x4CAAD7: formatValue<ServiceFlags> (tinyformat.h:345)
==10465== by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl<ServiceFlags>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:523)
==10465== by 0x1924D4: format (tinyformat.h:510)
==10465== by 0x1924D4: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:803)
==10465== by 0x553A55: vformat (tinyformat.h:947)
==10465== by 0x553A55: format<ServiceFlags> (tinyformat.h:957)
==10465== by 0x553A55: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<ServiceFlags>(char const*, ServiceFlags const&) (tinyformat.h:966)
==10465== by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462)
==10465== by 0x28EDB5: CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_tests.cpp:31)
==10465== by 0x293947: rpc_tests::rpc_togglenetwork::test_method() (rpc_tests.cpp:88)
==10465== by 0x2950E5: rpc_tests::rpc_togglenetwork_invoker() (rpc_tests.cpp:84)
==10465== by 0x182496: invoke<void (*)()> (callback.hpp:56)
==10465== by 0x182496: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
...
```
The read of the uninitialized variable nLocalServices is triggered by g_connman->GetLocalServices()
in getnetworkinfo(const JSONRPCRequest& request) (net.cpp:462):
```c++
UniValue getnetworkinfo(const JSONRPCRequest& request)
{
...
if(g_connman)
obj.push_back(Pair("localservices", strprintf("%016x", g_connman->GetLocalServices())));
...
}
```
The reason for the uninitialized nLocalServices is that CConnman::Start(...) is not called
by the tests, and hence the initialization normally performed by CConnman::Start(...) is
not done.
This commit adds a method Init(const Options& connOptions) which is called by both the
constructor and CConnman::Start(...). This method initializes nLocalServices and the other
relevant values from the supplied Options object.
This fixes a few cases where we should be treating a restart-after-
coinsviewdb-reset identically to a just-reset-coinsviewdb.
Thanks to @morcos for identifying the bug.
This more clearly uses fReindex vs fReset to make sure we're not
clearing our coinsdb needlessly when restarting after a reindex.
It also makes it so that restarting after shutting down mid-reindex
isn't treates specially at all during txdb loading code, as it
shouldn't be.
This resolves a possible-assert-on-shutdown race introduced in
1f668b6468 when early shutdown
occurs.
Previously this was not done to avoid any cases where the
threadGroup might not exit due to a blocking thread, but at this
point the threadGroup isn't used all that much, plus Qt already
does this, and its good to keep their init/shutdown consistent.
For those curious, the threadGroup is only used in a few places:
* Its used to run the CCheckQueues in script validation, but these
use the boost mutex/condition variable primitives, so they
respect the interrupt pretty trivially.
* Its used for the import thread, which should exit rather quickly
as mostly it just calls LoadExternalBlockFile, which has an
interruption_point right before each block loaded.
* Its used in the scheduler thread, which is only used for:
* validationinterface has an effectively-dummy reference to it.
* wallet compaction, which should not last long
* addr/banlist dumping from CConnman, which should also be fast
e222dc2 Replace ismine with producesignature check in witnessifier (Andrew Chow)
Pull request description:
Instead of using ismine to check whether an address can be spent by us, make the witness version of the script or address first and then use ProduceSignature with the DummySignatureCreator to check if we can
solve for the script.
This is to fix cases where we don't have all of the private keys (for something like a multisig address) but have the redeemscript so we can witnessify it.
Tree-SHA512: 371777aee839cceb41f099109a13689120d35cf3880cde39216596cc2aac5cc1096af7d9cf07ad9306c3b05c073897f4518a7e97f0b88642f1e3b80b799f481e
efeb273 Force on-the-fly compaction during pertxout upgrade (Pieter Wuille)
Pull request description:
It seems that LevelDB tends to leave the old "per txid" UTXO entries in the database lying around for a significant amount of time during and after the per-txout upgrade. This introduces a `CompactRange` function in the database wrapper, and invokes it after every batch of updates in `CCoinsViewDB::Upgrade()`. This lowers temporary disk usage during and after the upgrade.
Tree-SHA512: fbf964c0a33f4e73709c999c8a2bfdef974779c15820907398a2f8828f5fa3e4e153ddd9031d6fc5083be81e22b999b9bd826fd063ad8b88f55c5e8342503290
c0025d0 Fix segfault when shutting down before fully loading (Matt Corallo)
1385697 Order chainstate init more logically. (Matt Corallo)
ff3a219 Call RewindBlockIndex even if we're about to run -reindex-chainstate (Matt Corallo)
b0f3249 More user-friendly error message if UTXO DB runs ahead of block DB (Matt Corallo)
eda888e Fix some LoadChainTip-related init-order bugs. (Matt Corallo)
Pull request description:
This does a number of things to clean up chainstate init order,
fixing some issues as it goes:
* Order chainstate init more logically - first all of the
blocktree-related loading, then coinsdb, then
pcoinsTip/chainActive. Only create objects as needed.
* More clearly document exactly what is and isn't called in
-reindex and -reindex-chainstate both with comments noting
calls as no-ops and by adding if guards.
* Move the writing of fTxIndex to LoadBlockIndex - this fixes a
bug introduced in d6af06d68a where
InitBlockIndex was writing to fTxIndex which had not yet been
checked (because LoadChainTip hadn't yet initialized the
chainActive, which would otherwise have resulted in
InitBlockIndex being a NOP), allowing you to modify -txindex
without reindex, potentially corrupting your chainstate!
* Rename InitBlockIndex to LoadGenesisBlock, which is now a more
natural name for it. Also check mapBlockIndex instead of
chainActive, fixing a bug where we'd write the genesis block out
on every start.
* Move LoadGenesisBlock further down in init. This is a more logical
location for it, as it is after all of the blockindex-related
loading and checking, but before any of the UTXO-related loading
and checking.
* Give LoadChainTip a return value - allowing it to indicate that
the UTXO DB ran ahead of the block DB. This just provides a nicer
error message instead of the previous mysterious
assert(!setBlockIndexCandidates.empty()) error.
* Calls ActivateBestChain in case we just loaded the genesis
block in LoadChainTip, avoiding relying on the ActivateBestChain
in ThreadImport before continuing init process.
* Move all of the VerifyDB()-related stuff into a -reindex +
-reindex-chainstate if guard. It couldn't do anything useful
as chainActive.Tip() would be null at this point anyway.
Tree-SHA512: 3c96ee7ed44f4130bee3479a40c5cd99a619fda5e309c26d60b54feab9f6ec60fabab8cf47a049c9cf15e88999b2edb7f16cbe6819e97273560b201a89d90762
Instead of using ismine to check whether an address can be spent by us,
make the witness version of the script or address first and then use
ProduceSignature with the DummySignatureCreator to check if we can
solve for the script.
Also fixes test cases to reflect this change.
* Order chainstate init more logically - first all of the
blocktree-related loading, then coinsdb, then
pcoinsTip/chainActive. Only create objects as needed.
* More clearly document exactly what is and isn't called in
-reindex and -reindex-chainstate both with comments noting
calls as no-ops and by adding if guards.
* Move LoadGenesisBlock further down in init. This is a more logical
location for it, as it is after all of the blockindex-related
loading and checking, but before any of the UTXO-related loading
and checking.
* Move all of the VerifyDB()-related stuff into a -reindex +
-reindex-chainstate if guard. It couldn't do anything useful
as chainActive.Tip() would be null at this point anyway.
RewindBlockIndex works over both chainActive - disconnecting blocks
from the tip that need witness verification - and mapBlockIndex -
requiring redownload of blocks missing witness data.
It should never have been the case that the second half is skipped
if we're about to run -reindex-chainstate.
This gives LoadChainTip a return value - allowing it to indicate that
the UTXO DB ran ahead of the block DB. This just provides a nicer
error message instead of the previous mysterious
assert(!setBlockIndexCandidates.empty()) error.
This also calls ActivateBestChain in case we just loaded the genesis
block in LoadChainTip, avoiding relying on the ActivateBestChain
in ThreadImport before continuing init process.
* Move the writing of fTxIndex to LoadBlockIndex - this fixes a
bug introduced in d6af06d68a where
InitBlockIndex was writing to fTxIndex which had not yet been
checked (because LoadChainTip hadn't yet initialized the
chainActive, which would otherwise have resulted in
InitBlockIndex being a NOP), allowing you to modify -txindex
without reindex, potentially corrupting your chainstate!
* Rename InitBlockIndex to LoadGenesisBlock, which is now a more
natural name for it. Also check mapBlockIndex instead of
chainActive, fixing a bug where we'd write the genesis block out
on every start.
df389bc Change wallet method disabled error text (Russell Yanofsky)
e526b3d Fix misleading "Method not found" multiwallet errors (Russell Yanofsky)
Pull request description:
Raise RPC_WALLET_NOT_SPECIFIED instead of RPC_METHOD_NOT_FOUND when a required
wallet filename was not specified in an RPC call.
Also raise more specific RPC_WALLET_NOT_FOUND error instead of
RPC_INVALID_PARAMETER in case an invalid wallet was specified, for consistency.
Tree-SHA512: 6a8d885283f69bcfc28f2e08ac03eff02f9f8160a312ce2a90d868aa52533434fc0b4c4ab86547c2f09392338956df915637eaf7136a4fc105e6c8179f2d0ac8
72f0060 Replace traditional for with ranged for in primitives (Dag Robole)
Pull request description:
Replace traditional for with ranged for in block and transaction primitives to improve readability
Tree-SHA512: c0fff603d2939149ca48b6aa72b59738a3658d49bd58b2d4ffbc85bdb774d8d5bb808fe526fe22bb9eb214de632834d373e2aab44f6019a83c0b09440cea6528
f228b8e remove some unused functions (Marko Bencun)
Pull request description:
Identified with `cppcheck --enable=unusedFunction .`.
- GetSendBufferSize()'s last use removed in
991955e
- SetPort()'s last use removed in
7e195e8
- GetfLargeWorkInvalidChainFound() was introduced in
e3ba0ef and never used
Tree-SHA512: ea8e5498bec981e42e1342c171c37723c2f5e575c7d6c1a524d9c6cd9b332bdd0d84fddf9e14ca011bb49fb82bd037386382c9afc546b3c2231ae548358bd4f4
065039d [tests] Fix incorrect memory_cleanse(…) call in crypto_tests.cpp (practicalswift)
Pull request description:
`chKey` and `chIV` are pointers, not arrays :-)
Probably the result of copy-pasting of old code where the code was operating on arrays instead of pointers.
If I'm reading the code correctly the absence/presence of these `memory_cleanse(…)` calls won't alter the outcome of the test in question (`TestPassphraseSingle`) even if fixed. Therefore removing.
Tree-SHA512: a053b2817bedf6ef889744e546ce9a0f165dee94aef6850d9d6a6bb05b0018789597371ecf154a4aec8588c0ef5626ef08c23c35e35927f6b0497b5f086146fe
a2420ae Avoid unnecessary work in SetNetworkActive (João Barbosa)
Pull request description:
This PR adds an early return to avoid unnecessary notifications when the status doesn't change.
Tree-SHA512: 85d05ca6fa36cb581f94bc154d08bd72cd53f6a857173c6fb2f184058f9c0208c4cf5e5d196825a78339902d8f256688eb6793f99abc7be9c7cfac85136180d9
Raise RPC_WALLET_NOT_SPECIFIED instead of RPC_METHOD_NOT_FOUND when a required
wallet filename was not specified in an RPC call.
Also raise more specific RPC_WALLET_NOT_FOUND error instead of
RPC_INVALID_PARAMETER in case an invalid wallet was specified, for consistency.
095b917 Avoid using sizes on non-fixed-width types to derive protocol constants. (Gregory Maxwell)
Pull request description:
Thanks to awemany for pointing this out.
This replaces #10172 which appears to be abandoned, but uses the constants as requested on that PR.
Tree-SHA512: 032c0d75b3aaf807a7d0c7fb8ff5515acc45ad58bd00fe81413f900fe02bad900534a970403b9bb568e132c9eddea6043e958daf625e8acc84375bd41ee2e2ef
9f8a46f Properly document target_confirmations in listsinceblock (Ryan Havar)
Pull request description:
There seems to be some misunderstandings about this, but it's a heavily used function so I'd like to make sure the docs are clear about how it works.
For a later issue:
* Change the default of target_confirmations to 6 (1 is a pretty silly default)
* Change the name of target_confirmations (it's really a horrible name)
Tree-SHA512: a2fba2fab30019cea9db56cd7e31de95ba31090617ab336bdf130f9591bfcf3fc5fbd9e7e1e40b6c7bd2f74b9b4658afb1fdc7fc44e1f79520d1319758982a1c
a56f8b0 Add missing lock in CScheduler::AreThreadsServicingQueue() (Matt Corallo)
Pull request description:
Not an actual bug as this is only used in asserts right now, but
nice to not have a missing lock.
Tree-SHA512: 7e542b150a0be716783e196493d239f2ad15e5376abf54b67d735dc3ef1b10849c090337b849f530c9f7497ddcfb8389b47d64a5dcf6382b7d38838f88cc1100
0be03c7 Qt: Use _putenv_s instead of setenv on Windows builds (Brian McMichael)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/10836
Error message I would get on `make`:
```
...
CXXLD bench/bench_bitcoin.exe
OBJCXXLD qt/bitcoin-qt.exe
qt/test/test_main.cpp: In function ‘int main(int, char**)’:
qt/test/test_main.cpp:64:43: error: ‘setenv’ was not declared in this scope
setenv("QT_QPA_PLATFORM", "minimal", 0);
^
make[2]: *** [qt/test/qt_test_test_bitcoin_qt-test_main.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/home/bmcmichael/Projects/bcoin/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/bmcmichael/Projects/bcoin/src'
make: *** [all-recursive] Error 1
```
`setenv` function is not available from the Microsoft runtime library. Need to use `_putenv_s` instead.
This solution tells the compiler to use `_putenv_s` on `WIN32` compilation (Note: this also works on 64-bit Windows instances.) and `setenv` everywhere else.
I've tested builds on Windows 10 x64 and Ubuntu 16.04 with this code.
Tree-SHA512: d53c996c890e3c6f22b4f2dcca718bef9168f19a6d4a29b8ff13391bfc0c8ea9c1cd16782b47c25b156dcbdff18bb19e23bfd5f6fefb1f373c9d5454a13fc969
99c7fc3 Prevent user from specifying conflicting parameters to fundrawtx (Matt Corallo)
Pull request description:
estimate_mode/conf_target both are overridden by feeRate, so should
not be specified together with feeRate.
Based on #10706
Tree-SHA512: 8ccd08575fd1f2a0d45112538ffbbc73983ee172963230b0cc7ac41d13c6f3c740917f82b212c41ded3a64d873452e7f2c7af49f3b47cab897f8e85117f21333
This assures that we don't overwrite a random file called
`wallet.backup` that happens to be in the current directory. It also
assures that the temporary file will be cleaned up.
Noticed by Evan Klitzke, came up in discussion here:
https://github.com/bitcoin/bitcoin/pull/10880#discussion_r128460722
9737572 [Qt] Use wallet 0 in rpc console if running with multiple wallets (Jonas Schnelli)
Pull request description:
Current master with multiwallet results in accessing wallet 0 in QT (send / receive / tx history / etc.), **but** the RPC console cannot access that wallet (only non-wallet commands work).
This is a quick solution to re-allow accessing the same wallet (Index 0) via RPC console in multiwallet.
The solutions design is not "state of the art" (should go over WalletModel). Ideally we work on an overall multiwallet support for the GUI (which then would remove this change).
I think we should consider this as a bugfix.
Tree-SHA512: 16cf844662248ffd3d82c7d0cbe5879f231fbc7d4f5a4aab4180a9087018519c98301e4ac311eaec2cc39dddf25d3edf9be99a6622ea682c138a820a9b21fd0c
a5ecaf1 Fix misspellings and remove safety verbiage (Steven D. Lander)
Pull request description:
Standardizing punctuation on CLI output and also including a few fixes for grammer. This PR is for text only changes and includes no code edits.
Tree-SHA512: afde551bf1212838822188b6723f2bf1b7222decfa1cd7aa6b04967489108a29f80833af6059252af028c53437755f258275af0614e0d4d0311e09421cd8e131
05e023f Move CloseSocket out of SetSocketNonBlocking and pass SOCKET by const reference in SetSocket* functions (Dag Robole)
Pull request description:
Rationale:
Readability, SetSocketNonBlocking does what it says on the tin.
Consistency, More consistent with the rest of the API in this unit.
Reusability, SetSocketNonBlocking can also be used by clients that may not want to close the socket on failure.
This also moves the responsibility of closing the socket back to the caller that opened it, which in general should know better how and when to close it.
Tree-SHA512: 85027137f1b626e2b636549ee38cc757a587adcf464c84be6e65ca16e3b75d7ed1a1b21dd70dbe34c7c5d599af39e53b89932dfe3c74f91a22341ff3af5ea80a
876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)
Pull request description:
The following scenario will not notify the caller of the fact `tx0` has been dropped:
1. User 1 receives BTC in tx0 from utxo1 in block aa1.
2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
3. User 1 sees 2 confirmations at block aa3.
4. Reorg into bb chain.
5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.
See `listsinceblock.py` commit for related test.
The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.
Example output:
```Python
{
'transactions': [],
'replaced': [
{
'walletconflicts': [],
'vout': 1,
'account': '',
'timereceived': 1485234857,
'time': 1485234857,
'amount': '1.00000000',
'bip125-replaceable': 'unknown',
'trusted': False,
'category': 'receive',
'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
'label': '',
'confirmations': -7
}
],
'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
}
```
I believe this addresses the comment by @luke-jr in https://github.com/bitcoin/bitcoin/pull/9516#issuecomment-274190081 but I could be wrong..
Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35
Identified with `cppcheck --enable=unusedFunction .`.
- GetSendBufferSize()'s last use removed in
991955ee81
- SetPort()'s last use removed in
7e195e8459
- GetfLargeWorkInvalidChainFound() was introduced in
e3ba0ef956 and never used
This commit adds a listwallets RPC, which lists the names of the
currently loaded wallets. This command intentionally shows no
information about the wallet other then the name. Information on
individual wallets can be obtained using the getwalletinfo RPC.
6b8d872 Protect SSE4 code behind a compile-time flag (Pieter Wuille)
fa9be90 Add selftest for SHA256 transform (Pieter Wuille)
c1ccb15 Add SSE4 based SHA256 (Pieter Wuille)
2991c91 Add SHA256 dispatcher (Pieter Wuille)
4d50f38 Support multi-block SHA256 transforms (Pieter Wuille)
Pull request description:
This adds an SSE4 assembly version of the SHA256 transform by Intel, and uses it at run time if SSE4 instructions are available, and use a fallback C++ implementation otherwise. Nearly every x86_64 CPU supports SSE4. The feature is only enabled when compiled with `--enable-experimental-asm`.
In order to avoid build dependencies and other complications, the original Intel YASM code was translated to GCC extended asm syntax.
This gives around a 50% speedup on the SHA256 benchmark for me.
It is based on an earlier patch by @laanwj, though only includes a single assembly version (for now), and removes the YASM dependency.
Tree-SHA512: d31c50695ceb45264291537b93c0d7497670be38edf021ca5402eaa7d4e1e0e1ae492326e28d4e93979d066168129e62d1825e0384b1b906d36f85d93dfcb43c
d9d1bd3 nCheckDepth chain height fix (romanornr)
Pull request description:
````
if (nCheckDepth <= 0)
nCheckDepth = 1000000000; // suffices until the year 19000
if (nCheckDepth > chainActive.Height())
nCheckDepth = chainActive.Height();
````
These lines confuse me.
Correct me if I am wrong, but we can't check any more blocks than we have right?
If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement.
````
if (nCheckDepth > chainActive.Height())
nCheckDepth = chainActive.Height();
````
when using ````--checkblocks=Z```` When Z is ````0```` or any other negative number, it will check all blocks.
I think it should be changed to this maybe.
````
if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height())
nCheckDepth = chainActive.Height();
````
Which gets rid of that huge number which is confusing for any other altcoins that have a different block time.
Tree-SHA512: 8ee0ae5f33b399fa74dc16926709694ccfe1fc8a043cba2f5d00884220ac1b9b13f2df4588041f4133be634e5c7b14f4eebe24294028dafe91581a97dbe627f3
4dc1915 check for null values in rpc args and handle appropriately (Gregory Sanders)
999ef20 importmulti options are optional (Gregory Sanders)
a70d025 fixup some rpc param counting for rpc help (Gregory Sanders)
Pull request description:
Audited where named args will fail to use correct default values or may fail when additional optional arguments are added.
Previously for these parameters, it was fine to omit them as positional arguments, but it would trigger UniValue runtime errors to set them to null, or to omit them while passing named parameters with greater positions (which would internally set earlier missing arguments to null). Now null values are treated the same as missing values so these errors do not occur.
Included a few other small fixes while working on it.
I didn't bother fixing account-based rpc calls.
Tree-SHA512: 8baf781a35bd48de7878d4726850a580dab80323d3416c1c146b4fa9062f8a233c03f37e8ae3f3159e9d04a8f39c326627ca64c14e1cb7ce72538f934ab2ae1e
6b4f231 Move transaction combining from signrawtransaction to new RPC (Andrew Chow)
Pull request description:
Create a combinerawtransaction RPC which accepts a json array of hex raw transactions to combine them into one transaction. Signrawtransaction is changed to no longer combine transactions and only accept one transaction at a time.
The tests have been updated to test this. Tests for the signrawtransaction merge have also been removed.
This is part of #10570
Tree-SHA512: 035aebbd6537c1c017d5c8e06d309228b4c23fe52d5b31ffde19741c81a11a6346ddbbdc582b77b02a47f4c22b1952b69d3c2ee1109c29b3f0f1b612d8de53ed
1c9b818 getinfo deprecation warning (Andrew Chow)
Pull request description:
This is an alternative to #10841
This PR implements @gmaxwell's suggestion of a `nag` field for getinfo which warns about the deprecation. Instead of calling it `nag`, I have named it `deprecation-warning`. The output of `getinfo` will look like this:
```
{
"version": 149900,
"protocolversion": 70015,
"walletversion": 139900,
"balance": 0.00000000,
"blocks": 476281,
"timeoffset": 0,
"connections": 2,
"proxy": "",
"difficulty": 804525194568.1318,
"testnet": false,
"keypoololdest": 1496858803,
"keypoolsize": 197,
"unlocked_until": 0,
"paytxfee": 0.00000000,
"relayfee": 0.00001000,
"errors": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications",
"deprecation-warning": "WARNING: getinfo is deprecated and will be fully removed in 0.16. Projects should transition to using getblockchaininfo, getnetworkinfo, and getwalletinfo before upgrading to 0.16"
}
```
I think this should be tagged for 0.15
Tree-SHA512: ea1bac96a67f797519e8748ddd661cf0a1127cbc38f145b98f10cf9b54dcf0519b353062ce9888e1f51875497299c75ff5147566944451bc3fc117620e773489
f4d00e6 Add a discard_rate (Alex Morcos)
b138585 Remove factor of 3 from definition of dust. (Alex Morcos)
Pull request description:
The definition of dust is redefined to remove the factor of 3.
Dust is redefined to be the value of an output such that it would
cost that value in fees to (create and) spend the output at the dust
relay rate. The previous definition was that it would cost 1/3 of the
value. The default dust relay rate is correspondingly increased to
3000 sat/kB so the actual default dust output value of 546 satoshis
for a non-segwit output remains unchanged. This commit is a refactor
only unless a dustrelayfee is passed on the commandline in which case
that number now needs to be increased by a factor of 3 to get the same
behavior. -dustrelayfee is a hidden command line option.
Note: It's not exactly a refactor due to edge case changes in rounding
as evidenced by the required change to the unit test.
A discard_rate is added which defaults to 10,000 sat/kB
Any change output which would be dust at the discard_rate you are
willing to discard completely and add to fee (as well as continuing to
pay the fee that would have been needed for creating the change)
This would be a nice addition for 0.15 and I think will remain useful for 0.16 with the new coin selection algorithms in discussion, but its not crucial.
It does add translation strings, but we could (should?) avoid that by hiding the option
Tree-SHA512: 5b6f655354d0ab6b8b6cac1e8d1fe3136d10beb15c6d948fb15bfb105155a9d03684c6240624039b3eed6428b7e60e54216cc8b2f90c4600701e39f646284a9b
e0d459264 Avoid redundant redeclaration of GetWarnings(const string&) (practicalswift)
Pull request description:
Avoid redundant redeclaration of `GetWarnings(const string&)`.
`std::string GetWarnings(const std::string& strFor)` is declared in `warnings.h` and defined in `warnings.cpp`.
Tree-SHA512: d1503e00a2073cf080d66eafa303dc9c660a7ac15d4d2abcf2e4aa69cf9622d89a8e3f09324139bb7b8debaa6d1ee4a1c1681d347cebd99b1d3672a4da6d1ace
Create a combinerawtransaction RPC which accepts a json array of hex raw
transactions to combine them into one transaction. Signrawtransaction is changed
to no longer combine transactions and only accept one transaction at a time.
cf82a9e Do not allow users to get keys from keypool without reserving them (Matt Corallo)
Pull request description:
fundrawtransaction allows users to add a change output and then
not have it removed from keypool. While it would be nice to have
users follow the normal CreateTransaction/CommitTransaction process
we use internally, there isnt much benefit in exposing this option,
especially with HD wallets, while there is ample room for users to
misunderstand or misuse this option.
This partially reverts #9377. Would be nice to get this for 15 since its kinda crazy we have this option to begin with IMO, will need release notes as an RPC option is now ignored.
Tree-SHA512: 72b5ee9c4a229b84d799dfb00c56fe80d8bba914ce81a433c3f5ab325bf9bf2b839ee658c261734f0ee183ab19435039481014d09c41dbe155e6323e63beb01d
fundrawtransaction allows users to add a change output and then
not have it removed from keypool. While it would be nice to have
users follow the normal CreateTransaction/CommitTransaction process
we use internally, there isnt much benefit in exposing this option,
especially with HD wallets, while there is ample room for users to
misunderstand or misuse this option.
This could be particularly nasty in some use-cases (especially
pre-HD-split) - eg a user might fundrawtransaction, then call
getnewaddress, hand out the address for someone to pay them, then
sendrawtransaction. This may result in the user thinking they have
received payment, even though it was really just their own change!
This could obviously result in needless key-reuse.
6b9faf7 [QA] add basic multiwallet test (Jonas Schnelli)
979d0b8 [tests] [wallet] Add wallet endpoint support to authproxy (John Newbery)
76603b1 Select wallet based on the given endpoint (Jonas Schnelli)
32c9710 Fix test_bitcoin circular dependency issue (Jonas Schnelli)
31e0720 Add wallet endpoint support to bitcoin-cli (-usewallet) (Jonas Schnelli)
dd2185c Register wallet endpoint (Jonas Schnelli)
Pull request description:
Alternative for #10829 and #10650.
It adds the most simplest form of wallet based endpoint support (`/wallet/<filename>`).
No v1 and no node/wallet endpoint split.
Tree-SHA512: 23de1fd2f9b48d94682928b582fb6909e16ca507c2ee19e1f989d5a4f3aa706194c4b1fe8854d1d79ba531b7092434239776cae1ae715ff536e829424f59f9be
1fc8c3d No longer ever reuse keypool indexes (Matt Corallo)
Pull request description:
This fixes an issue where you could reserve a keypool entry, then
top up the keypool, writing out a new key at the given index, then
return they key from the pool. This isnt likely to cause issues,
but given there is no reason to ever re-use keypool indexes
(they're 64 bits...), best to avoid it alltogether.
Builds on #10235, should probably get a 15 tag.
Tree-SHA512: c13a18a90f1076fb74307f2d64e9d80149811524c6bda259698ff2c65adaf8c6c3f2a3a07a5f4bf03251bc942ba8f5fd33a4427aa4256748c40b062991682caf
077d01f random: only use getentropy on openbsd (Cory Fields)
Pull request description:
Follow-up from #10335. I can confirm that this fixes my issue when building against a new glibc + old linux headers for back-compat.
Tree-SHA512: a0fcf26995fbd3636f970e729a172c6e1d7c0de371e703f0653cd9776600f438ec43acd2b1eb92f2678a011968da8fbbeef8a54599434851f4c6ffe78291c172
06bcdb8da Convert named argument from nblocks to conf_target (Alex Morcos)
439c4e8ad Improve api to estimatesmartfee (Alex Morcos)
Pull request description:
Through 0.14 branch, the estimatesmartfee API was tagged "WARNING: This interface is unstable and may disappear or change!" and this warning is removed for 0.15, so any wanted API updates should happen now.
The changes here are to make the additional parameter for conservative estimates a more general estimate_mode string , to omit the feerate and include an error string instead of returning -1 on error, and to do better parameter checking initially.
~It is only the last 2 commits, but it's built on #10706 and #10543~.
See https://github.com/bitcoin/bitcoin/pull/10707#issuecomment-314869251 for renaming of nblocks argument to conf_target. Will need to be included before string freeze.
PR description edited for clarity
Tree-SHA512: 6d8ebee8bb410e2950ffd59663eebfed8d1611d995dc935bb91e430d9da7e2f306796f45631458376027d26341c660f09e825e61748103d2f2736ec6dc3df3ae
8276e70de Adding assert to avoid a memory access violation inside of PartialMerkleTree::CalcHash() (Chris Stewart)
Pull request description:
Fixing a possible memory access violation in CPartialMerkleTree::CalcHash().
This can happen if we some how a merkle tree with zero txids. I don't think this can happen in practice as we only send merkle block messages on the p2p network as of now -- we cannot receive them.
This was found with #8469, specifically using this [generator](https://github.com/Christewart/bitcoin/blob/rapidcheck/src/test/gen/merkleblock_gen.h#L52-L77) which will cause a memory access violation on [this test case](https://github.com/Christewart/bitcoin/blob/rapidcheck/src/test/merkleblock_properties.cpp#L48).
Tree-SHA512: b95904ec45ea3f082c7722161d93ee06b24c706fbffa909a6e995ed14788aed2830f91b626da6f0347660c45874a0735dab61c9440b59c949c690af4165c83fb
Any change output which would be dust at the discard_rate you are
willing to discard completely and add to fee (as well as continuing to
pay the fee that would have been needed for creating the change).
This fixes an issue where you could reserve a keypool entry, then
top up the keypool, writing out a new key at the given index, then
return they key from the pool. This isnt likely to cause issues,
but given there is no reason to ever re-use keypool indexes
(they're 64 bits...), best to avoid it alltogether.
b0e8e2d Print one log message per keypool top-up, not one per key. (Gregory Maxwell)
41dc163 Increase wallet default keypool size to 1000. (Gregory Maxwell)
30d8f3a Pushdown walletdb though CWallet::AddKeyPubKey to avoid flushes. (Gregory Maxwell)
3a53f19 Pushdown walletdb object through GenerateNewKey/DeriveNewChildKey. (Gregory Maxwell)
Pull request description:
This carries the walletdb object from top-up through GenerateNewKey/DeriveNewChildKey/CWallet::AddKeyPubKey, which allows us to avoid the flush on destruction until the top up finishes instead of flushing the wallet for every key.
This speeds up adding keys by well over 10x on my laptop (actually something like 17x), I wouldn't be surprised if it were an even bigger speedup on spinning rust.
Then it increases the keypool size to 1000. I would have preferred to use 10,000 but in the case where the user creates a new wallet and then turns on encryption it seems kind of dumb to have >400KB of marked-used born unencrypted keys just laying around.
(Thanks to Matt for cluesticking me on how to bypass the crypter spaghetti)
Tree-SHA512: 868303de38fce4c3f67d7fe133f765f15435c94b39d252d7450b5fee5c607a3cc2f5e531861a69d8c8877bf130e0ff4c539f97500a6bc0ff6d67e4a42c9385c7
dba485d init: Factor out AppInitLockDataDirectory (Wladimir J. van der Laan)
Pull request description:
Alternative to #10818, alternative solution to #10815.
After this change: All the AppInit steps before and inclusive AppInitLockDataDirectory must not have Shutdown() called in case of failure. Only when AppInitMain fails, Shutdown should be called.
Changes the GUI and bitcoind code to consistently do this.
Tree-SHA512: 393e1a0ae05eb8e791025069e3ac4f6f3cdeb459ec63feda85d01cf6696ab3fed7632b6a0ac3641b8c7015af51d46756b5bba77f5e5f0c446f0c2dea58bbc92e
Change parameter for conservative estimates to be an estimate_mode string.
Change to never return a -1 for failure but to instead omit the feerate and
return an error string. Throw JSONRPC error on invalid nblocks parameter.
Alternative to #10818, alternative solution to #10815.
After this change: All the AppInit steps before and inclusive
AppInitLockDataDirectory must not have Shutdown() called in case of
failure. Only when AppInitMain fails, Shutdown should be called.
Changes the GUI and bitcoind code to consistently do this.
This redefines dust to be the value of an output such that it would
cost that value in fees to (create and) spend the output at the dust
relay rate. The previous definition was that it would cost 1/3 of the
value. The default dust relay rate is correspondingly increased to
3000 sat/kB so the actual default dust output value of 546 satoshis
for a non-segwit output remains unchanged. This commit is a refactor
only unless a dustrelayfee is passed on the commandline in which case
that number now needs to be increased by a factor of 3 to get the same
behavior. -dustrelayfee is a hidden command line option.
Note: It's not exactly a refactor due to edge case changes in rounding
as evidenced by the required change to the unit test.
11590d3 Properly bound check conf_target in wallet RPC calls (Alex Morcos)
fd29d3d Remove checking of mempool min fee from estimateSmartFee. (Alex Morcos)
2fffaa9 Make QT fee displays use GetMinimumFee instead of estimateSmartFee (Alex Morcos)
1983ca6 Use CoinControl to pass custom fee setting from QT. (Alex Morcos)
03ee701 Refactor to use CoinControl in GetMinimumFee and FeeBumper (Alex Morcos)
ecd81df Make CoinControl a required argument to CreateTransaction (Alex Morcos)
Pull request description:
This builds on #10589 (first 5 commits from that PR, last 5 commits are new)
The first couple commits refactor to use the CCoinControl class to pass fee calculation parameters around.
This allows for fixing the buggy interaction in QT between the global payTxFee which can be modified by the RPC call settxfee or temporarily modified by the QT custom fee settings. Before these changes the GUI could sometimes send a transaction with a recently set payTxFee and not respect the settings displayed in the GUI. After these changes, using the GUI does not involve the global transaction confirm target or payTxFee.
The prospective fee displays in the smart fee slider and the coin control dialog are changed to use the fee calculation from GetMinimumFee, this simplifies the code and makes them slightly more correct in edge cases.
Maxing the fee calculation with the mempool min fee is move from estimateSmartFee to GetMinimumFee.
This fixes a long standing bug, and should be tagged for 0.15 as it is holding up finalizing the estimatesmartfee RPC API before release.
Tree-SHA512: 4d36a1bd5934aa62f3806d380fcafbef73e9fe5bdf190fc5259a3e3a13349e5ce796e50e7068c46dc630ccf56d061bce5804f0bfe2e082bb01ca725b63efd4c1
4c3b538 [logs] fix zapwallettxes startup logs (John Newbery)
e7a2181 [wallet] fix zapwallettxes interaction with persistent mempool (John Newbery)
ff7365e [tests] fix flake8 warnings in zapwallettxes.py (John Newbery)
Pull request description:
zapwallettxes previously did not interact well with persistent mempool.
zapwallettxes would cause wallet transactions to be zapped, but they
would then be reloaded from the mempool on startup. This commit softsets
persistmempool to false if zapwallettxes is enabled so transactions are
actually zapped.
This PR also fixes the zapwallettxes.py functional test, which did not properly test this feature. The test line:
```py
assert_raises(JSONRPCException, self.nodes[0].gettransaction, [txid3])
#there must be a expection because the unconfirmed wallettx0 must be gone by now
```
is not actually testing the presence of the transaction since the RPC is being called incorrectly (with an array instead of a string). The `assert_raises()` passes since an assert is raised, but it's not the one the test writer had in mind!
Fixes#9710 .
Tree-SHA512: e3236efc7a2fd2b3bf1d9e2e8a7726d470c57f5d95cf41b7bde264edc8817bd36a6f3feff52f8de8db0ef64b7247c88b24e7ff7cefaa706cba86fe4e2135a508
5618b7d Do not shadow upper local variable `state`. (Pavel Janík)
Pull request description:
Tests added in #10192 emit few shadowing warnings:
```
test/txvalidationcache_tests.cpp:268:26: warning: declaration shadows a local variable [-Wshadow]
test/txvalidationcache_tests.cpp:296:26: warning: declaration shadows a local variable [-Wshadow]
test/txvalidationcache_tests.cpp:357:26: warning: declaration shadows a local variable [-Wshadow]
```
Remove shadowing declarations and reuse the upper local declaration as in other already present test cases.
Tree-SHA512: 1e3c52cf963f8f33e729900c8ecdcd5cc6fe28caa441ba53c4636df9cc3d1a351ca231966d36384589f1340ae8ddd447424c2ee3e8527d334d0412f0d1a10c8f
6835cb0ab Avoid static analyzer warnings regarding uninitialized arguments (practicalswift)
Pull request description:
Avoid static analyzer warnings regarding _"Function call argument is a pointer to uninitialized value"_ in cases where we are intentionally using such arguments.
This is achieved by using `f(b.begin(), b.end())` (`std::array<char, N>`) instead of `f(b, b + N)` (`char b[N]`).
Rationale:
* Reduce false positives by guiding static analyzers regarding our intentions.
Before this commit:
```shell
$ clang-tidy-3.5 -checks=* src/bench/base58.cpp
bench/base58.cpp:23:9: warning: Function call argument is a pointer to uninitialized value [clang-analyzer-core.CallAndMessage]
EncodeBase58(b, b + 32);
^
$ clang-tidy-3.5 -checks=* src/bench/verify_script.cpp
bench/verify_script.cpp:59:5: warning: Function call argument is a pointer to uninitialized value [clang-analyzer-core.CallAndMessage]
key.Set(vchKey, vchKey + 32, false);
^
$
```
After this commit:
```shell
$ clang-tidy-3.5 -checks=* src/bench/base58.cpp
$ clang-tidy-3.5 -checks=* src/bench/verify_script.cpp
$
```
Tree-SHA512: 5814a320ca8b959d0954bb64393424bcad73f942d2e988de1cd6788f39153b93900325532f2e340de02d740a3953385d212ae08e7ec72bb4c394a40475f251df
d0413c670 Use range based for loop (René Nyffenegger)
Pull request description:
Instead of iterating over 0 .. 1 and then deciding on an actual desired
value, use a range based for loop for the desired value.
Tree-SHA512: 0a7a4a80516c9f16cf97fa7d257088b8386360e19b93c4deac3d745b6270ea452c513821686d7d14a159a235763e034f9b14eef222ca15f7eb71c37bd1c2c380
d40a72ccb Clarify *(--.end()) iterator semantics in CWallet::TopUpKeyPool (Matt Corallo)
28301b978 Meet code style on lines changed in the previous commit (Matt Corallo)
4a3fc3562 Track keypool entries as internal vs external in memory (Matt Corallo)
Pull request description:
This is an alternative version of #10184. As @jonasschnelli points out there, the performance regressions are pretty minimal, but given that this is a pretty simple, mechanical change, its probably worth doing.
Tree-SHA512: e83f9ebf2998f8164d1b2eebe5e6dcdeadea8c30b7612861f830758c08bf4093cd6a67b3bcfa9cfcb139e5e0b106fc8898a975fc69f334981aefc756568ab613
5c643241e [utils] allow square brackets for ipv6 addresses in bitcoin-cli (John Newbery)
fe4fabaf1 [refactor] move SplitHostPort() into utilstrencodings (John Newbery)
Pull request description:
bitcoin-cli's `-rpcconnect` can accept ipv6 addresses (as long as the libevent version is new enough), but fails to parse ipv6 with square brackets. This PR makes `bitcoin-cli` parse ipv6 in square brackets correctly.
`bitcoin-cli -rpcconnect=[::1] <command>`
should now be equivalent to
`bitcoin-cli -rpcconnect=::1 <command>`
This is useful so the `bitcoin-cli` option can now be in the same format as the `bitcoind` option.
Doesn't include tests. I have a branch that fully tests `bitcoin-cli`, but that's queued behind several intermediate PRs.
- first commit moves `SplitHostPort()` from libbitcoin_common into libbitcoin_util
- second commit adds proper ipv6 parsing to bitcoin-cli
Tree-SHA512: 249d409f10360c989474283341f458cc97364a56a7d004ae6d5f13d8bffe3a51b5dc2484d42218848e2d42cd9c0b13a1b92e94ea19b209f7e91c875c208d8409
0aadc11fd Avoid dereference-of-casted-pointer (Pieter Wuille)
Pull request description:
And prefer a static_cast to the intended reference type.
Tree-SHA512: e83b20023a4dca6029b46f7040a8a6fd54e1b42112ec0c87c3c3b567ed641de97a9e2335b57a2efb075491f641e5b977bc226a474276bea0c3c3c71d8d6ac54d
zapwallettxes previously did not interact well with persistent mempool.
zapwallettxes would cause wallet transactions to be zapped, but they
would then be reloaded from the mempool on startup. This commit softsets
persistmempool to false if zapwallettxes is enabled so transactions are
actually zapped.
228987d84 getbalance example covers at least 6 confirms (Gregory Sanders)
Tree-SHA512: 328d60b007ee75d809f4d28a7d9e5537d3c1446bd30c4c2ae57c690b8e83f6287cbcd3d8c955e8ba07ab62e27f9d27497c55219ff14fd5af7759dec465673fa2
Avoid static analyzer warnings regarding "Function call argument
is a pointer to uninitialized value" in cases where we are
intentionally using such arguments.
This is achieved by using ...
`f(b.begin(), b.end())` (`std::array<char, N>`)
... instead of ...
`f(b, b + N)` (`char b[N]`)
Rationale:
* Reduce false positives by guiding static analyzers regarding our
intentions.
Before this commit:
```
$ clang-tidy-3.5 -checks=* src/bench/base58.cpp
bench/base58.cpp:23:9: warning: Function call argument is a pointer to uninitialized value [clang-analyzer-core.CallAndMessage]
EncodeBase58(b, b + 32);
^
$ clang-tidy-3.5 -checks=* src/bench/verify_script.cpp
bench/verify_script.cpp:59:5: warning: Function call argument is a pointer to uninitialized value [clang-analyzer-core.CallAndMessage]
key.Set(vchKey, vchKey + 32, false);
^
$
```
After this commit:
```
$ clang-tidy-3.5 -checks=* src/bench/base58.cpp
$ clang-tidy-3.5 -checks=* src/bench/verify_script.cpp
$
```
This check has been moved to the wallet logic GetMinimumFee. The rpc call to
estimatesmartfee will now no longer return a result maxed with the mempool min
fee, but automated fee calculations from the wallet will produce the same result
as before and coincontrol and sendcoins dialogs in the GUI will correctly
display the right prospective fee.
changes to policy/fees.cpp include a big whitespace indentation change.
Remove helper function (CalculateEstimateType) for determining whether
estimates should be conservative or not, now that this is only called
once from GetMinimumFee and incorporate the logic directly there.
This fixes buggy behavior where we were temporarily setting and unsetting the
global payTxFee when trying to send a transaction with a custom fee from the
GUI. The previous behavior was inconsistent depending on the order of using the
RPC call settxfee and clicking various radio buttons in the sendcoinsdialog.
The new behavior is that transactions sent with the GUI will always use either
the smartfee slider value or the custom fee set on the GUI and they will not
affect the global defaults which are only for RPC and initial GUI values.
2aef1f182 [Qt] migrate old fee slider value to new dropbown Always round up (conservative) (Jonas Schnelli)
bc1be90e3 [Qt] replace fee slider with a Dropdown, extend conf. targets (Jonas Schnelli)
Tree-SHA512: 53796cf0b434dd3db5d4680dbeb6231a7df8f15d88187178fd4db8917cd7fc60091ce2c1589fd93668fc94bb13f989aba5b7ef3792fa95ee1f9f21a15709e2d3
dd97a529a Properly forbid -salvagewallet and -zapwallettxes for multi wallet. (Alex Morcos)
Tree-SHA512: dcde8f854ae957b4d3af4bcf1b811e0b6e9b93602764f86499e46a28d304cd4ee93ba058c03f6ca74ccb60e1310c83e53b698c64d93e5503115377655b80d44d
Warning from gcc 7.1 is ./prevector.h:450:25: warning:
'*((void*)(&<anonymous>)+8).prevector<28, unsigned char>::_union.prevector<28, unsigned char>::direct_or_indirect::<anonymous>.prevector<28, unsigned char>::direct_or_indirect::<unnamed struct>::indirect'
may be used uninitialized in this function [-Wmaybe-uninitialized]
Some people keep thinking that MAX_BLOCK_BASE_SIZE is a separate
size limit from the weight limit when it fact it is superfluous,
and used in early tests before the witness data has been
validated or just to compute worst case sizes. The size checks
that use it would not behave any differently consensus wise
if they were eliminated completely.
Its correct value is not independently settable but is a function
of the weight limit and weight formula.
This patch just eliminates it and uses the scale factor as
required to compute the worse case constants.
It also moves the weight factor out of primitives into consensus,
which is a more logical place for it.
18bacec6c Make check to distinguish between orphan txs and old txs more efficient. (Alex Morcos)
Tree-SHA512: b6b4bad89aa561975dce7b68b2fdad5623af5ebcb9c38fd6a72b5f6d0544ed441df4865591ac018f7ae0df9b5c60820cb4d9e55664f5667c9268458df70fd554
d34d77a build: verify that the assembler can handle crc32 functions (Cory Fields)
Pull request description:
Also, enable crc32 even if -msse4.2 wasn't added by us, as long as it works. This allows custom flags (such as -march=native) to work as expected.
Addresses #10670.
Tree-SHA512: e1a41a87b078d270bc645814315b229ad9c16556a4d14fb66b27a65b28d0caf9bf324f8c1e221854992aa17f53466eece06faebbf74d59b3d4ff2e6db6c614a4
69a4339ef missing white space in function arg (Lawrence Nahum)
Pull request description:
Tree-SHA512: 94d832eca0b5429cf48c7c1d4489942b53182c5b7ad9f24264867e7631301173c23a48f7b2e8fd22ff1b3137bafacb3f9d4320f4df235668c9096aaefff7afe8
959dd8781 Avoid printing incorrect block indexing time due to uninitialized variable (practicalswift)
Tree-SHA512: a76e43c3ffa734ed5c7eadf363f345f268aa0e6ce775aba8f856fe3bbc82f240dc7c734c5ca3ac500a12eb41fae00623413e79f484d5acf809b6e400851d771d
Adding comment to assert in PartialMerkleTree::CalcHash()
Adding comment on CMerkleBlock indicating it calls something that contains an assert
Removing EOL whitespace
Due to the iterative process of selecting new coins in each loop a new fee is
calculated that needs to be met each time. In the typical case if the most
recent iteration of the loop produced a much smaller transaction and we have now
gathered inputs with too many fees, we can just reduce the change. However in
the case where there is no change output, it is possible to end up with a
transaction which drastically overpays fees. This commit addresses that case,
by creating a change output if the overpayment is large enough to support it,
this is accomplished by rerunning the transaction creation loop without
selecting new coins.
Thanks to instagibbs for working on this as well
5e3b7b5 Improve error reporting for estimaterawfee (Alex Morcos)
1fafd70 Add function to report highest estimate target tracked per horizon (Alex Morcos)
9c85b91 Change API to estimaterawfee (Alex Morcos)
Tree-SHA512: e624c6e7967e9e48abe49f5818bd674e5710e571cc093029d2f90d39fdfba3c1f30e83bf89f6dce97052b59a7d9636a64642ccfb26effd149c417d0afbed0c0b
That OP_NOP10 is the last executable opcode is a bit of a obscure trivia, and the MAX_OPCODE constant already exists. This merely standardizes use of MAX_OPCODE over OP_NOP10 where it makes sense and is more clear.
1f668b6 Expose if CScheduler is being serviced, assert its not in EmptyQueue (Matt Corallo)
3192975 Flush CValidationInterface callbacks prior to destruction (Matt Corallo)
08096bb Support more than one CScheduler thread for serial clients (Matt Corallo)
2fbf2db Add default arg to CScheduler to schedule() a callback now (Matt Corallo)
cda1429 Give CMainSignals a reference to the global scheduler (Matt Corallo)
3a19fed Make ValidationInterface signals-type-agnostic (Matt Corallo)
ff6a834 Use TestingSetup to DRY qt rpcnestedtests (Matt Corallo)
Tree-SHA512: fab91e34e30b080ed4d0a6d8c1214910e383c45440676e37be61d0bde6ae98d61e8903d22b846e95ba4e73a6ce788798350266feba246d8a2ab357e8523e4ac5
Add support for setting each of these attributes on a per RPC call basis to sendtoaddress, sendmany, fundrawtransaction (already had RBF), and bumpfee (already had RBF and conf target).
Note that the CScheduler thread cant be running at this point,
it has already been stopped with the rest of the init threadgroup.
Thus, just calling any remaining loose callbacks during Shutdown()
is sane.
This will be used by CValidationInterface soon.
This requires a bit of work as we need to ensure that most of our
callbacks happen in-order (to avoid synchronization issues in
wallet) - we keep our own internal queue and push things onto it,
scheduling a queue-draining function immediately upon new
callbacks.
9bbf600 Use method name from __func__ macro (darksh1ne)
Tree-SHA512: da6d0714f458b538189bbc2b53252ba353dcc1ef15fa780cb7f690a034b58ab0dbaa3a89f83f044c746241ee265a70fc092449f1cc7be4f190775423fbca5fc5
GetMinimumFee now passes the conservative argument into estimateSmartFee.
Call CalculateEstimateType(mode) before calling GetMinimumFee or estimateSmartFee to determine the value of this argument.
CCoinControl can now be used to control this mode.
73c942e Use "replaceable" instead of "rbfoptin" in bitcoin-tx. (Matt Corallo)
fb915d5 Use "replaceable" instead of "optIntoRbf" in fundrawtransaction. (Matt Corallo)
928c681 Use "replaceable" instead of "optintorbf" in createrawtransaction. (Matt Corallo)
Tree-SHA512: 8922451c00abb63aaa08b4a9e314e89c22233b32f207259fbc25367f7d5b67efbaccc7e2a4958c18611ad498da302296242860c7be965a0e996dcde3e89efa07
Use __func__ macro in std::runtime_exception to:
1. fix method name in CWalletTx::GetAvailableWatchOnlyCredit()
2. refactor CWalletTx::GetAvailableCredit()
(by hiding boost::signals stuff in the .cpp)
This allows us to give it a bit more intelligence as we move
forward, including routing some signals through CScheduler. While
the introduction of a "internals" pointer in the class is pretty
ugly, the fact that we no longer need to include boost/signals
directly from validationinterface.h is very much worth the loss.
ecb4fc382 fix typo in help text for removeprunedfunds (Akio Nakamura)
Tree-SHA512: 2603851f1ac90bc0b90ced6355b0056e4cb658303cb2cd03ee0827ed0053157ebb87de48076f4d4f556991bfdbdb65d0a68a8dbd275c501cee4c9b5746a9562b
2a96283 rpc: Update `generate` for developer notes (Wladimir J. van der Laan)
df7e2f0 rpc: Move the `generate` RPC call to rpcwallet (Wladimir J. van der Laan)
Tree-SHA512: ec658d6178f8435dc54b9d9c6dd59f873055a8ae0c3f177c02049d77b93107dd5fc17a1ff56d50f051810d52fdf306846eaba2ef4fc8d2a6cfa831f57a1045c4
Fixes:
init.cpp: In function ‘bool AppInitMain(boost::thread_group&, CScheduler&)’:
init.cpp:1499:56: warning: ‘nStart’ may be used uninitialized in this function [-Wmaybe-uninitialized]
LogPrintf(" block index %15dms\n", GetTimeMillis() - nStart);
^
542ce6e Report [CANCELLED] instead of [DONE] when shut down during txdb upgrade (Jonas Schnelli)
83fbea3 Report txdb upgrade not more often then every 10% (Jonas Schnelli)
06c5b6e Show txdb upgrade progress in debug log (Jonas Schnelli)
316fcb5 Allow to cancel the txdb upgrade via splashscreen callback (Jonas Schnelli)
ae09d45 Allow to shut down during txdb upgrade (Jonas Schnelli)
00cb69b [Qt] allow to execute a callback during splashscreen progress (Jonas Schnelli)
Tree-SHA512: 23190f23f441bfd60821e49f8b3698a6bef97eb0e0ee659328e4a7395769ecd1616420eacc38aa1fa0ff62b9de5f13a0098dc798cdec6bff649575cefebc0db2
This makes it possible to mine to any wallet when multi-wallet mode is added.
Solves the same problem as #10649, but IMO in a cleaner way.
It also gets rid of the circuitous `ScriptForMining` method on
`CValidationInterface`, which really doesn't belong there.
After this change it's still possible to mine without wallet through
`generatetoaddress`.
176c021 [qa] Test non-atomic chainstate writes (Suhas Daftuar)
d6af06d Dont create pcoinsTip until after ReplayBlocks. (Matt Corallo)
eaca1b7 Random db flush crash simulator (Pieter Wuille)
0580ee0 Adapt memory usage estimation for flushing (Pieter Wuille)
013a56a Non-atomic flushing using the blockchain as replay journal (Pieter Wuille)
b3a279c [MOVEONLY] Move LastCommonAncestor to chain (Pieter Wuille)
Tree-SHA512: 47ccc62303f9075c44d2a914be75bd6969ff881a857a2ff1227f05ec7def6f4c71c46680c5a28cb150c814999526797dc05cf2701fde1369c06169f46eccddee
aa95947 Use the override specifier (C++11) where we expect to be overriding the virtual function of a base class (practicalswift)
Tree-SHA512: 3835e9b4ceaa4b2db485a25dfa3e5fe50c2e3ecb22ca9d5331aed7728aa496d4378fb84c0a1a3c47b0adecc10a00bca99cc239cbaf94cf2ce5b4cda497db6023
Check that cached script execution results are only valid for the same
script flags; that script execution checks are returned for non-cached
transactions; and that cached results are only valid for transactions
with the same witness hash.
Checking for the existence in the CCoinsViewCache of the outputs of a new tx
will result in a disk hit for every output since they will not be found. On the
other hand if those outputs exist already, then the inputs must also have been
missing, so we can move this check inside the input existence check so in the
common case of a new tx it doesn't need to run.
The purpose of the check is to avoid spamming the orphanMap with slightly old
txs which we have already seen in a block, but it is already only optimistic
(depending on the outputs not being spent), so make it even more efficient by
only checking the cache and not the entire pcoinsTip.
c074752 [RPC] Add an uptime command that displays the amount of time that bitcoind has been running (Ricardo Velhote)
Tree-SHA512: 8f59d4205042885f23f5b87a0eae0f5d386e9c6134e5324598e7ee304728d4275f383cd154bf1fb25350f5a88cc0ed9f97edb099e9b50c4a0ba72d63ec5ca5b4
This removes the possibility for GetCoin/HaveCoin/HaveCoinInCache to return
true while the respective coin is spent. By doing it across all calls, some
extra checks can be eliminated.
coins_tests is modified to call HaveCoin sometimes before and sometimes
after AccessCoin. A further change is needed because the semantics for
GetCoin slightly changed, causing a pruned entry in the parent cache to not
be pulled into the child in FetchCoin.
Adds new functional test, dbcrash.py, which uses -dbcrashratio to exercise the
logic for recovering from a crash during chainstate flush.
dbcrash.py is added to the extended tests, as it may take ~10 minutes to run
Use _Exit() instead of exit() for crash simulation
This eliminates stderr output such as:
terminate called without an active exception
or
Assertion failed: (!pthread_mutex_destroy(&m)), function ~recursive_mutex, file /usr/local/include/boost/thread/pthread/recursive_mutex.hpp, line 104.
Eliminating the stderr output on crash simulation allows testing with
test_runner.py, which reports a test as failed if stderr is produced.
This requires that we not access pcoinsTip in InitBlockIndex's
FlushStateToDisk (so we just skip it until later in AppInitMain)
and the LoadChainTip in LoadBlockIndex (which there is already one
later in AppinitMain, after ReplayBlocks, so skipping it there is
fine).
Includes some simplifications by Suhas Daftuar and Pieter Wuille.
5257698 Change semantics of HaveCoinInCache to match HaveCoin (Alex Morcos)
Tree-SHA512: 397e9ba28646b81fffa53e55064735d4d242aaffdf8484506825f785b0e414f334e4c5cd1e4e1dd9a4b6d1f6954c7ecad15429934a1c4e8d39f596cbd9f5dd80
5a9b508 [trivial] Add end of namespace comments (practicalswift)
Tree-SHA512: 92b0fcae4d1d3f4da9e97569ae84ef2d6e09625a5815cd0e5f0eb6dd2ecba9852fa85c184c5ae9de5117050330ce995e9867b451fa8cd5512169025990541a2b
deaf48b Handle TIMESTAMP_WINDOW within CWallet::RescanFromTime (Russell Yanofsky)
5b2be2b Make CWallet::RescanFromTime comment less ambiguous (Russell Yanofsky)
9bb66ab Add RescanFromTime method and use from rpcdump (Russell Yanofsky)
ccf84bb Move birthday optimization out of ScanForWalletTransactions (Russell Yanofsky)
Tree-SHA512: cd38433b8f5c5e44ecfba830a6a26bd9a9d0f4a22ae42bce17773d1a6fb25e1ee4289484996dad2d7acfa03059917ff062459f25030a761da7083ba5fbc87bc9
1887337 Add a comment explaining the use of MAX_BLOCK_BASE_SIZE. (Gregory Maxwell)
Tree-SHA512: 4c643a3696241fbf4eac8d58bb26586f319338b28ee86d20394a8ea362911b467853eb40c43487ede753209a3c7bee2e576d2ca80627e9fc924fabefbcaea34b
c1be285 chainparams: make supported service bits option explicit (Cory Fields)
d5c7c1c net: use an internal address for fixed seeds (Cory Fields)
6cdc488 net: switch to dummy internal ip for dns seed source (Cory Fields)
6d0bd5b net: do not allow resolving to an internal address (Cory Fields)
7f31762 net: add an internal subnet for representing unresolved hostnames (Cory Fields)
Tree-SHA512: 9bf1042bef546ac3ef0e0d3a9a5555eb21628ff2674a0cf8c6367194b22bfdab477adf452c0e7c56f44e0fb37debc5e14bdb623452e076fb9c492c7702601d7a
dd869c6 Add an explanation of quickly hashing onto a non-power of two range. (Gregory Maxwell)
Tree-SHA512: 8b362e396206a4ee2e825908dcff6fe4525c12b9c85a6e6ed809d75f03d42edcfba5e460a002e5d17cc70c103792f84d99693563b638057e4e97946dd1d800b2
d5711f4 Filter subtrees and and benchmarks from coverage report (Andrew Chow)
405b86a Replace lcov -r commands with faster way (Andrew Chow)
c8914b9 Have `make cov` optionally include branch coverage statistics (Andrew Chow)
Tree-SHA512: 9c349a7baeb7430ea586617c52f91177df58e3546d6dc573e26815ddb79e30ab1873542d85ac1daca5e1fb2c6d6c8965824b42d027b6b0496a744af57b095852
cf68a48 Deduplicate addrdb.cpp and use CHashWriter/Verifier (Pieter Wuille)
Tree-SHA512: 0301332e797f64da3a1588c9ebaf533af58da41e38f8a64206bff20102c5e82c2a7c630ca3150cf451b2ccf4acb3dd45e44259b6ba15e92786e9e9a2b225bd2f
1ae86ec Changed event RAII helper functions to inline to deal with duplicate symbol linker errors. (Karl-Johan Alm)
fd369d2 Switched httpserver.cpp to use RAII wrapped libevents. (Kalle Alm)
Tree-SHA512: 877e431f211024d42a3b0800e860e02833398611433e8393f8d5d4970f47f4bd670b900443678c067fec110c087aaab7dc1981ccbf17f6057676fdbbda89aed9
6171826 Don't create change at the dust limit, even if it means paying more than expected (Alex Morcos)
Tree-SHA512: d4cf939b97a92d63d4703ad152093c9dc60e74af590697bc21adbcdf17fc9e6ffb37d46d5eefafbd69327d1bcb79911f1b826ce1d960d895d464c4408c4995c3
This wraps CheckInputs in ATMP's cache-inputs call to check that
each scriptPubKey the CCoinsViewCache provides is the one which
was committed to by the input's transaction hash.
A few "a->an" and "an->a".
"Shows, if the supplied default SOCKS5 proxy" -> "Shows if the supplied default SOCKS5 proxy". Change made on 3 occurrences.
"without fully understanding the ramification of a command" -> "without fully understanding the ramifications of a command".
Removed duplicate words such as "the the".
e5c6168 Fix instantiation and array accesses in class base_uint<BITS> (Pavlos Antoniou)
Tree-SHA512: e4d39510d776c5ae8814cd5fb5c5d183cd8da937e339bff95caff68a84492fbec68bf513c5a6267446a564d39093e0c7fc703c645b511caab80f7baf7955b804
9417d7a33 Be much more agressive in AccessCoin docs. (Matt Corallo)
f58349ca8 Restore some assert semantics in sigop cost calculations (Matt Corallo)
3533fb4d3 Return a bool in SpendCoin to restore pre-per-utxo assert semantics (Matt Corallo)
ec1271f2b Remove useless mapNextTx lookup in CTxMemPool::TrimToSize. (Matt Corallo)
Tree-SHA512: 158a4bce063eac93e1d50709500a10a7cb1fb3271f10ed445d701852fce713e2bf0da3456088e530ab005f194ef4a2adf0c7cb23226b160cecb37a79561f29ca
d9bec888f Use REJECT_DUPLICATE for already known and conflicted txn (Pieter Wuille)
Tree-SHA512: adc6dc5caed731c3fd5c8784e8820a074e320360cdb5579c5b9299f9799dd99de60b7382d336ab1909dab8b23e744456d78aa0c3b1c8dd1af3d1b779314cf8fa
131a8ceb7 Make clang-format use C++11 features (e.g. A<A<int>> instead of A<A<int> >) (practicalswift)
Tree-SHA512: e3c0ee683b654eae638deb41c52cf3187fa958dc5fa67778eaf8a83946b63f5b1d24c47bb965eaa910e3fcdcaf9eebf461eb3fc8e3a73ebaf03c7904521fda00
rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash
in particular, it was holding a stack canary which was not properly restored
after the cpuid.
Split out the x86+PIC case so that x86_64 doesn't have to worry about it.
Part of a series of changes to clean up the instantiation of connman
by decoupling the command line arguments.
We also now abort with an error when explicit binds are set with
-listen=0.
e9cd778 Pass in smart fee slider value to coin control dialog (Alex Morcos)
Tree-SHA512: 3899c3eb89b06e9cc842b33fabcce40a84fcc3a88ac2b02861f63419925312ac2a9f632567c02b0a060f5c5cd55f337e35e99b80535d1c1b8fcb7fd0c539f3c0
In order to prevent mixups, our internal range is never allowed as a resolve
result. This means that no user-provided string will ever be confused with an
internal address.
We currently do two resolves for dns seeds: one for the results, and one to
serve in addrman as the source for those addresses.
There's no requirement that the source hostname resolves to the stored
identifier, only that the mapping is unique. So rather than incurring the
second lookup, combine a private subnet with a hash of the hostname.
The resulting v6 ip is guaranteed not to be publicy routable, and has only a
negligible chance of colliding with a user's internal network (which would be
of no consequence anyway).
Since cfe77ef41 the global nTxConfirmTarget wasn't being updated by the smart
fee slider and thus the coin control dialog and labels were not being updated.
6294f32 gettxoutproof() should return consistent result (John Newbery)
Tree-SHA512: 1c36f78ea07a3bdde09e9494207b4372d54bcd94ed2d56e339e78281f6693e26a93e4c3123453d5c0f6e994d0069d5a1c806786c4af71864f87ea4841611c379
cb24c85 Use rdrand as entropy source on supported platforms (Pieter Wuille)
Tree-SHA512: c42eaa01a14e6bc097c70b6bf8540d61854c2f76cb32be69c2a3c411a126f7b4bf4a4486e4493c4cc367cc689319abde0d4adb799d29a54fd3e81767ce0766fc
Use case: TryCreateDirectory(GetDataDir() / "blocks" / "index") would
fail if the blocks directory was not explicitly created before.
The line that did so was in a weird location and could be removed as a
result.
bf376eacc Return early in IsBanned. (Gregory Maxwell)
Tree-SHA512: d8ed4aaf9a7523b00effa4ac17cec3be1ec1f5c5ce64d89833fbc8f3d73d13b022043354fbcf2682b2af05070d115e1fc0cc0b122197e9ddee5959c3fb9dd16d
In Olaoluwa Osuntokun's recent protocol proposal they were using a
mod in an inner loop. I wanted to suggest a normative protocol
change to use the trick we use here, but to find an explanation
of it I had to dig up the PR on github. After I posted about it
several other developers commented that it was very interesting
and they were unaware of it.
I think ideally the code should be self documenting and help
educate other contributors about non-obvious techniques that
we use. So I've written a description of the technique with
citations for future reference.
3ff1fa8 Use override keyword on CCoinsView overrides (Russell Yanofsky)
24e44c3 Don't return stale data from CCoinsViewCache::Cursor() (Russell Yanofsky)
Tree-SHA512: 08699dae0925ffb9c018f02612ac6b7eaf73ec331e2f4f934f1fe25a2ce120735fa38596926e924897c203f7470e99f0a99cf70d2ce31ff428b105e16583a861
c237bd7 wallet: Update formatting (Luke Dashjr)
9cbe8c8 wallet: Forbid -salvagewallet, -zapwallettxes, and -upgradewallet with multiple wallets (Luke Dashjr)
a2a5f3f wallet: Base backup filenames on original wallet filename (Luke Dashjr)
b823a4c wallet: Include actual backup filename in recovery warning message (Luke Dashjr)
84dcb45 Bugfix: wallet: Fix warningStr, errorStr argument order (Luke Dashjr)
008c360 Wallet: Move multiwallet sanity checks to CWallet::Verify, and do other checks on all wallets (Luke Dashjr)
0f08575 Wallet: Support loading multiple wallets if -wallet used more than once (Luke Dashjr)
b124cf0 Wallet: Replace pwalletMain with a vector of wallet pointers (Luke Dashjr)
19b3648 CWalletDB: Store the update counter per wallet (Luke Dashjr)
74e8738 Bugfix: ForceSetArg should replace entr(ies) in mapMultiArgs, not append (Luke Dashjr)
23fb9ad wallet: Move nAccountingEntryNumber from static/global to CWallet (Luke Dashjr)
9d15d55 Bugfix: wallet: Increment "update counter" when modifying account stuff (Luke Dashjr)
f28eb80 Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races (Luke Dashjr)
Tree-SHA512: 23f5dda58477307bc07997010740f1dc729164cdddefd2f9a2c9c7a877111eb1516d3e2ad4f9b104621f0b7f17369c69fcef13d28b85cb6c01d35f09a8845f23
40796e1 Remove references to priority that snuck back in in 870824e9. (Matt Corallo)
Tree-SHA512: fd6f772a9fdf14b3b125e84a79059d7ab34b3571b35dc48f8d4b9f22ea71c6cdd4ae88c2e135ae317a16744c28dd23cf7f7dd88ea9d8b2d408e57845ef87d03b
While the current implementation is pretty free, there is a lot
of possibility for this to blow up in our face with future changes,
especially as the backing map gets tweaked.
There are some similar asserts which are left removed in policy
and ATMP (policy code being broken isn't a huge deal, but if we
fail to verify some consensus rules, we should most definitely
crash).
Previously it was possible for HaveCoinInCache to return true for a spent
coin. It is more clear to keep the semantics the same. HaveCoinInCache is
used for two reasons:
- tracking coins we may want to uncache, in which case it is unlikely there
would be spent coins we could uncache (not dirty)
- optimistically checking whether we have already included a tx in the
blockchain, in which case a spent coin is not a reliable indicator that we have.
3fb81a8 Use list initialization (C++11) for maps/vectors instead of boost::assign::map_list_of/list_of (practicalswift)
Tree-SHA512: 63a9ac9ec5799472943dce1cd92a4b14e7f1fe12758a5fc4b1efceaf2c85a4ba71dad5ccc50813527f18b192e7714c076e2478ecd6ca0d452b24e88416f872f7
246a02f Use std::unordered_{map,set} (C++11) instead of boost::unordered_{map,set} (practicalswift)
Tree-SHA512: 35cd42012248174751e4b87efbc78689957f731d7710dea7e369974c6ec31b15b32d1537fb0d875c94c7ffb5046d9784735e218e5baeed96d525861dab6d4252
227ae9b [tests] Use FastRandomContext instead of boost::random::{mt19937,uniform_int_distribution} (practicalswift)
Tree-SHA512: 1bde6c8b9498051fa2eae4913eb1f5411adea8dea1511c0df859aea57a2a7db6f5839945ddf2eccdddfa322bceacad35a5d875742db7d15e40dbea83185307bb
e94584858 scripted-diff: Use new naming style for insecure_rand* functions (Pieter Wuille)
2fcd9cc86 scripted-diff: Use randbits/bool instead of randrange where possible (Pieter Wuille)
2ada67852 Use randbits instead of ad-hoc emulation in prevector tests (Pieter Wuille)
5f0b04eed Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille)
3ecabae36 Replace more rand() % NUM by randranges (Pieter Wuille)
efee1db21 scripted-diff: use insecure_rand256/randrange more (Pieter Wuille)
1119927df Add various insecure_rand wrappers for tests (Pieter Wuille)
124d13a58 Merge test_random.h into test_bitcoin.h (Pieter Wuille)
90620d66c scripted-diff: Rename cuckoo tests' local rand context (Pieter Wuille)
37e864eb9 Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille)
Tree-SHA512: d09705a3ec718ae792f7d66a75401903ba7b9c9d3fc36669d6e3b9242f0194738106be26baefc8a8e3fa6df7c9a35978c71c0c430278a028b331df23a3ea3070
We can call gettxoutproof() with a list of transactions. Currently, if
the first transaction is unspent (and all other transactions are in the
same block), then the call will succeed. If the first transaction has
been spent, then the call will fail. The means that the following two
calls will return different results:
gettxoutproof(unspent_tx1, spent_tx1)
gettxoutproof(spent_tx1, unspent_tx1)
This commit makes behaviour independent of transaction ordering by looping
through all transactions provided and trying to find which block they're in.
This commit also increases the test coverage and tests more failure
cases for gettxoutproof()
Added an option to configure to allow for branch coverage statistics gathering.
Disabled logprint macro when coverage testing is on so that unnecessary branches are not analyzed.
The implementation of base_uint::operator++(int) and base_uint::operator--(int) is now safer.
Array pn is accessed via index i after bounds checking has been performed on the index, rather than before.
The logic of the while loops has also been made more clear.
A compile time assertion has been added in the class constructors to ensure that BITS is a positive multiple of 32.
CCoinsViewCache doesn't actually support cursor iteration returning the
current contents of the cache, so raise an error when the cursor method is
called instead of returning a cursor that iterates over stale data.
Also update the gettxoutsetinfo RPC which was relying on the old behavior to be
explicit about which view it is returning data about.
This adds a new CuckooCache in validation, caching whether all of a
transaction's scripts were valid with a given set of script flags.
Unlike previous attempts at caching an entire transaction's
validity, which have nearly universally introduced consensus
failures, this only caches the validity of a transaction's
scriptSigs. As these are pure functions of the transaction and
data it commits to, this should be much safer.
This is somewhat duplicative with the sigcache, as entries in the
new cache will also have several entries in the sigcache. However,
the sigcache is kept both as ATMP relies on it and because it
prevents malleability-based DoS attacks on the new higher-level
cache. Instead, the -sigcachesize option is re-used - cutting the
sigcache size in half and using the newly freed memory for the
script execution cache.
Transactions which match the script execution cache never even have
entries in the script check thread's workqueue created.
Note that the cache is indexed only on the script execution flags
and the transaction's witness hash. While this is sufficient to
make the CScriptCheck() calls pure functions, this introduces
dependancies on the mempool calculating things such as the
PrecomputedTransactionData object, filling the CCoinsViewCache, etc
in the exact same way as ConnectBlock. I belive this is a reasonable
assumption, but should be noted carefully.
In a rather naive benchmark (reindex-chainstate up to block 284k
with cuckoocache always returning true for contains(),
-assumevalid=0 and a very large dbcache), this connected blocks
~1.7x faster.
9a5a1d7 RPC/rawtransaction: createrawtransaction: Check opt_into_rbf when provided with either value (Luke Dashjr)
23b0fe3 bitcoin-tx: rbfoptin: Avoid touching nSequence if the value is already opting in (Luke Dashjr)
b005bf2 Introduce MAX_BIP125_RBF_SEQUENCE constant (Luke Dashjr)
575cde4 [bitcoin-tx] add rbfoptin command (Jonas Schnelli)
5d26244 [Tests] extend the replace-by-fee test to cover RPC rawtx features (Jonas Schnelli)
36bcab2 RPC/Wallet: Add RBF support for fundrawtransaction (Luke Dashjr)
891c5ee Wallet: Refactor FundTransaction to accept parameters via CCoinControl (Luke Dashjr)
578ec80 RPC: rawtransaction: Add RBF support for createrawtransaction (Luke Dashjr)
Tree-SHA512: 446e37c617c188cc3b3fd1e2841c98eda6f4869e71cb3249c4a9e54002607d0f1e6bef92187f7894d4e0746ab449cfee89be9f6a1a8831e25c70cf912eac1570
656dbd871 Perform member initialization in initialization lists where possible (practicalswift)
Tree-SHA512: 048380f4da23ab1eaaf471801a01dbd76f2235afb686c1489b30a6bac109195134afc83414b8378d3482a9042d537ec62d30136dadb9347cf06b07fb5c693208
8ad5bde Merge bctest.py into bitcoin-util-test.py (John Newbery)
95836c5 Use shared config file for functional and util tests (John Newbery)
89fcd35 Use an .ini config file for environment vars in bitcoin-util-test.py (John Newbery)
e9265df Change help_text in bitcoin-util-test.py to a docstring. (John Newbery)
ce58e93 Change bitcoin-util-test.py to use Python3 (John Newbery)
Tree-SHA512: 66dab0b4a8546aee0dfaef134a165f1447aff4c0ec335754bbc7d9e55909721c62f09cdbf4b22d02ac1fcd5a9b66780f91e1cc4d8687fae7288cc9072a23a78f
Prior to per-utxo CCoins, we checked that no other in-mempool tx
spent any of the given transaction's outputs, as we don't want to
uncache that entire tx in such a case. However, we now are checking
only that there exists no other mempool spends of the same output,
which should clearly be impossible after we removed the transaction
which was spending said output (barring massive mempool
inconsistency).
Thanks to @sdaftuar for the suggestion.
At startup, we choose one peer to serve us the headers chain, until
our best header is close to caught up. Disconnect this peer if more
than 15 minutes + 1ms/expected_header passes and our best header
is still more than 1 day away from current time.
This change has no effect on wallet behavior.
On wallet startup, the transaction scan avoids reading any blocks with
timestamps older than the wallet birthday (less than nTimeFirstKey -
TIMESTAMP_WINDOW). This block skipping code currently resides in
CWallet::ScanForWalletTransactions but it doesn't really belong there because
it makes the implementation unnecessarily fragile and hard to understand, and
it never has any effect except at startup (because all other callers do their
rescans based on timestamps other than, but always greater or equal to,
nTimeFirstKey).
This adds the listening address on which incoming connections were received to the
CNode and CNodeStats structures.
The address is reported in `getpeerinfo`.
This can be useful for distinguishing connections received on different listening ports
(e.g. when using a different listening port for Tor hidden service connections)
or different networks.
8906a9a0d Fix bumpfee test after #10449 (Russell Yanofsky)
Tree-SHA512: 0838c7696499baf0fb5ee6edf0b081752d6c37578360a7f24a7e9c700598cbc14ff95826f2f5124cca805d2609470a052bc7309211874b13be7ac1ff9e911a34
589827975 scripted-diff: various renames for per-utxo consistency (Pieter Wuille)
a5e02bc7f Increase travis unit test timeout (Pieter Wuille)
73de2c1ff Rename CCoinsCacheEntry::coins to coin (Pieter Wuille)
119e552f7 Merge CCoinsViewCache's GetOutputFor and AccessCoin (Pieter Wuille)
580b02309 [MOVEONLY] Move old CCoins class to txdb.cpp (Pieter Wuille)
8b25d2c0c Upgrade from per-tx database to per-txout (Pieter Wuille)
b2af357f3 Reduce reserved memory space for flushing (Pieter Wuille)
41aa5b79a Pack Coin more tightly (Pieter Wuille)
97072d668 Remove unused CCoins methods (Pieter Wuille)
ce23efaa5 Extend coins_tests (Pieter Wuille)
508307968 Switch CCoinsView and chainstate db from per-txid to per-txout (Pieter Wuille)
4ec0d9e79 Refactor GetUTXOStats in preparation for per-COutPoint iteration (Pieter Wuille)
13870b56f Replace CCoins-based CTxMemPool::pruneSpent with isSpent (Pieter Wuille)
05293f3cb Remove ModifyCoins/ModifyNewCoins (Pieter Wuille)
961e48397 Switch tests from ModifyCoins to AddCoin/SpendCoin (Pieter Wuille)
8b3868c1b Switch CScriptCheck to use Coin instead of CCoins (Pieter Wuille)
c87b957a3 Only pass things committed to by tx's witness hash to CScriptCheck (Matt Corallo)
f68cdfe92 Switch from per-tx to per-txout CCoinsViewCache methods in some places (Pieter Wuille)
000391132 Introduce new per-txout CCoinsViewCache functions (Pieter Wuille)
bd83111a0 Optimization: Coin&& to ApplyTxInUndo (Pieter Wuille)
cb2c7fdac Replace CTxInUndo with Coin (Pieter Wuille)
422634e2f Introduce Coin, a single unspent output (Pieter Wuille)
7d991b55d Store/allow tx metadata in all undo records (Pieter Wuille)
c3aa0c119 Report on-disk size in gettxoutsetinfo (Pieter Wuille)
d34242430 Remove/ignore tx version in utxo and undo (Pieter Wuille)
7e0032290 Add specialization of SipHash for 256 + 32 bit data (Pieter Wuille)
e484652fc Introduce CHashVerifier to hash read data (Pieter Wuille)
f54580e7e error() in disconnect for disk corruption, not inconsistency (Pieter Wuille)
e66dbde6d Add SizeEstimate to CDBBatch (Pieter Wuille)
Tree-SHA512: ce1fb1e40c77d38915cd02189fab7a8b125c7f44d425c85579d872c3bede3a437760997907c99d7b3017ced1c2de54b2ac7223d99d83a6658fe5ef61edef1de3
As the maximum amount of data that can be pulled into the cache due to
a block validation is much lower now (at most one CCoin entry per input
and per output), reduce the conservative estimate used to determine
flushing time.
This patch makes several related changes:
* Changes the CCoinsView virtual methods (GetCoins, HaveCoins, ...)
to be COutPoint/Coin-based rather than txid/CCoins-based.
* Changes the chainstate db to a new incompatible format that is also
COutPoint/Coin based.
* Implements reconstruction code for hash_serialized_2.
* Adapts the coins_tests unit tests (thanks to Russell Yanofsky).
A side effect of the new CCoinsView model is that we can no longer
use the (unreliable) test for transaction outputs in the UTXO set
to determine whether we already have a particular transaction.
This clarifies a bit more the ways in which the new script execution
cache could break consensus in the future if additional data from
the CCoins object were to be used as a part of script execution.
After this change, any such consensus breaks should be very visible
to reviewers, hopefully ensuring no such changes can be made.
The new functions are:
* CCoinsViewCache::AddCoin: Add a single COutPoint/Coin pair.
* CCoinsViewCache::SpendCoin: Remove a single COutPoint.
* AddCoins: utility function that invokes CCoinsViewCache::AddCoin for
each output in a CTransaction.
* AccessByTxid: utility function that searches for any output with
a given txid.
* CCoinsViewCache::AccessCoin: retrieve the Coin for a COutPoint.
* CCoinsViewCache::HaveCoins: check whether a non-empty Coin exists
for a given COutPoint.
The AddCoin and SpendCoin methods will eventually replace ModifyCoins
and ModifyNewCoins, AddCoins will replace CCoins::FromTx, and the new
AccessCoins and HaveCoins functions will replace their per-txid
counterparts.
Note that AccessCoin for now returns a copy of the Coin object. In a
later commit it will be change to returning a const reference (which
keeps working in all call sites).
