886f1731be Key pool: Fix omitted pre-split count in GetKeyPoolSize (Andrew Chow)
386a994b85 Key pool: Change ReturnDestination interface to take address instead of key (Andrew Chow)
ba41aa4969 Key pool: Move LearnRelated and GetDestination calls (Andrew Chow)
65833a7407 Add OutputType and CPubKey parameters to KeepDestination (Andrew Chow)
9fcf8ce7ae Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper (Andrew Chow)
596f6460f9 Key pool: Move CanGetAddresses call (Andrew Chow)
Pull request description:
* The `pwallet->CanGetAddresses()` call in `ReserveDestination::GetReservedDestination` to `LegacyScriptPubKeyMan::GetReservedDestination` so that the sanity check results in a failure when a `ScriptPubKeyMan` individually cannot get a destination, not when any of the `ScriptPubKeyMan`s can't.
* `ScriptPubKeyMan::GetReservedDestination` is changed to return the destination so that future `ScriptPubKeyMan`s can return destinations constructed in other ways. This is implemented for `LegacyScriptPubKeyMan` by moving key-to-destination code from `CWallet` to `LegacyScriptPubKeyMan`
* In order for `ScriptPubKeyMan` to be generic and work with future `ScriptPubKeyMan`s, `ScriptPubKeyMan::ReturnDestination` is changed to take a `CTxDestination` instead of a `CPubKey`. Since `LegacyScriptPubKeyMan` still deals with keys internally, a new map `m_reserved_key_to_index` is added in order to track the keypool indexes that have been reserved.
* A bug is fixed in how the total keypool size is calculated as it was omitting `set_pre_split_keypool` which is a bug.
Split from #17261
ACKs for top commit:
ryanofsky:
Code review ACK 886f1731be. Only change is moving earlier fix to a better commit (same end result).
promag:
Code review ACK 886f1731be.
instagibbs:
code review re-ACK 886f1731be
Sjors:
Code review re-ACK 886f1731be
Tree-SHA512: f4be290759f63fdc920d5c02bd0d09acc4b06a5f053787d4afcd3c921b2e35d2bd97617fadae015da853dc189f559fb8d2c6e58d53e4cabfac9af151cd97ad19
d8daa8f371 pubkey: Assert CPubKey's ECCVerifyHandle precondition (practicalswift)
Pull request description:
Assert `CPubKey`'s `ECCVerifyHandle` precondition.
This makes it more clear for fuzzing harness writers and others that `ECCVerifyHandle` is expected to be held when interacting with `CPubKey`.
Related PR #17274.
ACKs for top commit:
sipa:
ACK d8daa8f371
Tree-SHA512: 9e74086599799dc9b5c3fb8357445b662e5bf896d826af63d6d6b6ddb616612966f3bb5de3bd3ae0e692c47de85672f64b8ab6d3a1c45899dc25ba46990b5ec7
Last update was in 2017.
Updates tinyformat to upstream commit 705e3f4e1de922069bf715746d35bd2364b1f98f.
Re-apply bitcoin core specific changes.
No changes that affect our use, as far as I can see, but this gets rid
of the gcc `-Wimplicit-fallthrough` warnings, at least.
Currently they are identical, but in the future we might want to turn
the mempool into a unique_ptr. Replacing the global with the mempool
pointer from the node context simplifies this step.
55b2cb199c random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake)
461e547877 doc: correct random.h docs after #17270 (fanquake)
Pull request description:
The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was
[removed](d61f2bb076) in #17270, meaning it, and its users can now be marked `noexcept`.
This also corrects the docs in random.h for some of the changes in #17270.
ACKs for top commit:
practicalswift:
ACK 55b2cb199c
laanwj:
ACK 55b2cb199c
sipa:
ACK 55b2cb199c
Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a
fa40e48c50 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke)
fa1bfc476c ci: ubsan report_error_type=1 and add suppressions (MarcoFalke)
fa69cef13e test: Print stderr when subprocess fails (MarcoFalke)
2222c30586 test: Use char instead of unsigned char (MarcoFalke)
faa8023ce9 ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke)
Pull request description:
Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le
ACKs for top commit:
practicalswift:
ACK fa40e48c50 assuming Travis is happy -- diff looks correct :)
Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
02d8c56a18 Seed RNG with precision timestamps on receipt of net messages. (Matt Corallo)
Pull request description:
See title. Exposes a generic dead-simple "SeedEvent" interface, but currently just used for net messages.
ACKs for top commit:
sipa:
utACK 02d8c56a18
laanwj:
ACK 02d8c56a18
meshcollider:
utACK 02d8c56a18
Tree-SHA512: 28eb39a201ee2b13393c5c64dbf7c1913f3482f095969ef5141bfe549ce77dd63bb5f14738f6eedb296c686ea36014aa157b9c5e8059710a318590f30e9caa14
e7ad4a2f8c doc: rename wallet-tool references to bitcoin-wallet (Wilson Ccasihue S)
Pull request description:
Fix. text reference to executable bitcoin-wallet instead of wallet-tool, there is not a wallet-tool at bin/ folder.
ACKs for top commit:
fanquake:
ACK e7ad4a2f8c - thanks for following up.
Tree-SHA512: aed41b08947728a4ff3a97a62858ee7c86e2e5d57dcbbd0aab492dae3d8a548bb60541924e68cf3a0aa3d53d7db0012b489462b466919cd83f05b2aa88b7fff7
In accordance with #17314, Removing noisy fields from -getinfo. Fields removed: protocolversion, walletversion and keypoololdest. In addition to changing bitcoin-cli -getinfo, there is another change to test/functional/interface_bitcoin_cli.py. This change deletes tests that utilize removed -getinfo calls.
76303f65f9 test: add unit test for non-standard txs with wrong nVersion (Dominik Spicher)
Pull request description:
Takes care of one of the missing cases of #17394: nVersion must be within the allowed range.
ACKs for top commit:
instagibbs:
ACK 76303f65f9
Tree-SHA512: 94464f781cf70a5616f7cea2014ae0a97a338c34411cc989c60389de2ce00368374811db78c919bda30e0ebf341fb830998a5e97c124dd8afc8feb726cedfd3a
70ed2ab7ef Add unit test for DB creation with unicode path (Aaron Clauson)
Pull request description:
An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755.
ACKs for top commit:
laanwj:
ACK 70ed2ab7ef
Tree-SHA512: fc6dbd3aa26a439016e63e8d4d931f218ce99094fc7887a13b54562ad4133047020288ecbcd622a8309f422ee1eda5df50bcb8c8e44442af36ed57b22c069004
02afb0c550 Fix origfee return for bumpfee with feerate arg (Gregory Sanders)
Pull request description:
fixes https://github.com/bitcoin/bitcoin/issues/17642 and adds a simple test that would have caught it
ACKs for top commit:
achow101:
ACK 02afb0c550
Tree-SHA512: 303e392e05407f204dffe360689b5bb5dc77fd462dd0e489bc0b6c8f94f89ab7fe2bd8cb47e4dc6dc5c23a619826d15f3bf6b02b2c8e96402fbb51953c462e2d
In order for ScriptPubKeyMan to be generic and work with future
ScriptPubKeyMans, ScriptPubKeyMan::ReturnDestination is changed to
take a CTxDestination instead of a CPubKey. Since LegacyScriptPubKeyMan
still deals with keys internally, a new map m_reserved_key_to_index is
added in order to track the keypool indexes that have been reserved.
The CPubKey argument of KeepDestination is also removed so that it is
more generic. Instead of taking a CPubKey or a CTxDestination, we just use
the nIndex given to find the pubkey.
Addresses are determined by LegacyScriptPubKeyMan::GetReservedDestination
instead of ReserveDestination::GetReservedDestination as other ScriptPubKeyMan
implementations may construct addresses differently
This does not change behavior.
98fbd1cdff Use correct C++11 header for std::swap() (Hennadii Stepanov)
b66861e2e5 Fix comparison function signature (Hennadii Stepanov)
Pull request description:
This PR fixes build on CentOS 7 with GCC 4.8.5:
```
...
In file included from /usr/include/c++/4.8.2/algorithm:62:0,
from ./serialize.h:11,
from ./qt/sendcoinsrecipient.h:13,
from ./qt/recentrequeststablemodel.h:8,
from qt/recentrequeststablemodel.cpp:5:
/usr/include/c++/4.8.2/bits/stl_algo.h: In instantiation of ‘_RandomAccessIterator std::__unguarded_partition(_RandomAccessIterator, _RandomAccessIterator, const _Tp&, _Compare) [with _RandomAccessIterator = QList<RecentRequestEntry>::iterator; _Tp = RecentRequestEntry; _Compare = RecentRequestEntryLessThan]’:
/usr/include/c++/4.8.2/bits/stl_algo.h:2296:78: required from ‘_RandomAccessIterator std::__unguarded_partition_pivot(_RandomAccessIterator, _RandomAccessIterator, _Compare) [with _RandomAccessIterator = QList<RecentRequestEntry>::iterator; _Compare = RecentRequestEntryLessThan]’
/usr/include/c++/4.8.2/bits/stl_algo.h:2337:62: required from ‘void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size, _Compare) [with _RandomAccessIterator = QList<RecentRequestEntry>::iterator; _Size = int; _Compare = RecentRequestEntryLessThan]’
/usr/include/c++/4.8.2/bits/stl_algo.h:5499:44: required from ‘void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter = QList<RecentRequestEntry>::iterator; _Compare = RecentRequestEntryLessThan]’
qt/recentrequeststablemodel.cpp:208:82: required from here
/usr/include/c++/4.8.2/bits/stl_algo.h:2263:35: error: no match for call to ‘(RecentRequestEntryLessThan) (RecentRequestEntry&, const RecentRequestEntry&)’
while (__comp(*__first, __pivot))
^
In file included from qt/recentrequeststablemodel.cpp:5:0:
./qt/recentrequeststablemodel.h:43:7: note: candidate is:
class RecentRequestEntryLessThan
^
qt/recentrequeststablemodel.cpp:217:6: note: bool RecentRequestEntryLessThan::operator()(RecentRequestEntry&, RecentRequestEntry&) const
bool RecentRequestEntryLessThan::operator()(RecentRequestEntry &left, RecentRequestEntry &right) const
^
qt/recentrequeststablemodel.cpp:217:6: note: no known conversion for argument 2 from ‘const RecentRequestEntry’ to ‘RecentRequestEntry&’
In file included from /usr/include/c++/4.8.2/algorithm:62:0,
from ./serialize.h:11,
from ./qt/sendcoinsrecipient.h:13,
from ./qt/recentrequeststablemodel.h:8,
from qt/recentrequeststablemodel.cpp:5:
/usr/include/c++/4.8.2/bits/stl_algo.h:2266:34: error: no match for call to ‘(RecentRequestEntryLessThan) (const RecentRequestEntry&, RecentRequestEntry&)’
while (__comp(__pivot, *__last))
^
In file included from qt/recentrequeststablemodel.cpp:5:0:
./qt/recentrequeststablemodel.h:43:7: note: candidate is:
class RecentRequestEntryLessThan
^
qt/recentrequeststablemodel.cpp:217:6: note: bool RecentRequestEntryLessThan::operator()(RecentRequestEntry&, RecentRequestEntry&) const
bool RecentRequestEntryLessThan::operator()(RecentRequestEntry &left, RecentRequestEntry &right) const
^
qt/recentrequeststablemodel.cpp:217:6: note: no known conversion for argument 1 from ‘const RecentRequestEntry’ to ‘RecentRequestEntry&’
CXX qt/qt_libbitcoinqt_a-sendcoinsentry.o
make[2]: *** [qt/qt_libbitcoinqt_a-recentrequeststablemodel.o] Error 1
```
Also for `std::swap()` header `<algorithm>` is replaced with `<utility>` one.
Refs:
- [`std::swap()`](https://en.cppreference.com/w/cpp/algorithm/swap)
- [standard library header `<utility>`](https://en.cppreference.com/w/cpp/header/utility)
ACKs for top commit:
promag:
Code review ACK 98fbd1cdff.
jonasschnelli:
utACK 98fbd1cdff
fanquake:
ACK 98fbd1cdff
Tree-SHA512: 91324490c1bdb98f186d233418e7e72ae7bee507876e94fb8c038bee031cea9e1046900f21156da4b7c33abcd726796867b124c4132d9ae3759877e90a8527db
eadd1304c8 tests: Add a test for funding with sufficient preset inputs and subtractFeeFromOutputs (Andrew Chow)
ff330badd4 Default to bnb_used = false as there are many cases where BnB is not used (Andrew Chow)
Pull request description:
#17290 introduced a bug where, when we had preset inputs that covered the amount being sent and subtractFeeFrromOutputs was being used, transaction funding would result in a `Fee exceeds maximum configured by -maxtxfee` error. This was happening because we weren't setting `bnb_used = false` when the preset inputs were used as it should have been. This resulted in a too high fee because the change would go to fees accidentally.
Apparently this particular case doesn't have a test, so I've added one as well.
ACKs for top commit:
Sjors:
ACK eadd130. I can't get this new test to fail on macOS (without this PR). It passes whether or not I compile with `--enable-debug`. It does fail on Ubuntu. Yay undefined behavior... Anyway, it's a useful test.
fanquake:
ACK eadd1304c8
instagibbs:
utACK eadd1304c8
Tree-SHA512: 7286c321f78666eea558cc591174630d210263594df41cab1065417510591ee514ade0e1d0cec8af09a785757da68de82592b013e8fe8d4966cec3254368706e
An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755.
4a96e459d7 [gui] send: show watch-only balance in send screen (Sjors Provoost)
2689c8fd71 [test] qt: add send screen balance test (Sjors Provoost)
Pull request description:
Now that we can create a PSBT from a watch-only wallet (#16944), we should also display the watch-only balance on the send screen.
Before:
<img width="1008" alt="before" src="https://user-images.githubusercontent.com/10217/69533384-030e9180-0f78-11ea-9748-c32c957e822e.png">
After:
<img width="1009" alt="Schermafbeelding 2019-11-26 om 11 44 17" src="https://user-images.githubusercontent.com/10217/69622879-19811f80-1042-11ea-8279-091012f39b38.png">
I added a test to check the balance on the send screen, but it only covers regular wallets. A better would add a watch-only only wallet.
ACKs for top commit:
meshcollider:
utACK 4a96e459d7
jb55:
utACK 4a96e459d7
promag:
reACK 4a96e45, rebased and label change since last review.
instagibbs:
code review and light test ACK 4a96e459d7
Tree-SHA512: 4213549888bd309f72bdbba1453218f4a2b07e809100d786a3791897c75468f9092b06fe4b971942b1c228aa75ee7c04971f262ca9a478b42756e056eb534620
73b96c94cb net: Fix uninitialized read in ProcessMessage(...) (practicalswift)
Pull request description:
Fix an uninitialized read in `ProcessMessage(…, "tx", …)` when receiving a transaction we already have.
The uninitialized value is read and used on [L2526 in the case of `AlreadyHave(inv) == true`](d8a66626d6/src/net_processing.cpp (L2494-L2526)).
Proof of concept being run against a `bitcoind` built with MemorySanitizer (`-fsanitize=memory`):
```
$ ./p2p-uninit-read-in-conditional-poc.py
Usage: ./p2p-uninit-read-in-conditional-poc.py <dstaddr> <dstport> <net>
$ bitcoind -regtest &
$ ./p2p-uninit-read-in-conditional-poc.py 127.0.0.1 18444 regtest
SUMMARY: MemorySanitizer: use-of-uninitialized-value
[1]+ Exit 77 bitcoind -regtest
$
```
Proof of concept being run against a `bitcoind` running under Valgrind (`valgrind --exit-on-first-error`):
```
$ valgrind -q --exit-on-first-error=yes --error-exitcode=1 bitcoind -regtest &
$ ./p2p-uninit-read-in-conditional-poc.py 127.0.0.1 18444 regtest
==27351== Conditional jump or move depends on uninitialised value(s)
[1]+ Exit 1 valgrind -q --exit-on-first-error=yes --error-exitcode=1 bitcoind -regtest
$
```
Proof of concept script:
```
#!/usr/bin/env python3
import sys
from test_framework.mininode import NetworkThread
from test_framework.mininode import P2PDataStore
from test_framework.messages import CTransaction, CTxIn, CTxOut, msg_tx
def send_duplicate_tx(dstaddr="127.0.0.1", dstport=18444, net="regtest"):
network_thread = NetworkThread()
network_thread.start()
node = P2PDataStore()
node.peer_connect(dstaddr=dstaddr, dstport=dstport, net=net)()
node.wait_for_verack()
tx = CTransaction()
tx.vin.append(CTxIn())
tx.vout.append(CTxOut())
node.send_message(msg_tx(tx))
node.send_message(msg_tx(tx))
node.peer_disconnect()
network_thread.close()
if __name__ == "__main__":
if len(sys.argv) != 4:
print("Usage: {} <dstaddr> <dstport> <net>".format(sys.argv[0]))
sys.exit(0)
send_duplicate_tx(sys.argv[1], int(sys.argv[2]), sys.argv[3])
```
Note that the transaction in the proof of concept is the simplest possible, but really any transaction can be used. It does not have to be a valid transaction.
This bug was introduced in #15921 ("validation: Tidy up ValidationState interface") which was merged in to `master` 28 days ago.
Luckily this bug was caught before being part of any Bitcoin Core release :)
ACKs for top commit:
jnewbery:
utACK 73b96c94cb
laanwj:
ACK 73b96c94cb, thanks for discovering and reporting this before it ended up in a release.
Tree-SHA512: 7ce6b8f260bcdd9b2ec4ff4b941a891bbef578acf4456df33b7a8d42b248237ec4949e65e2445b24851d1639b10681c701ad500b1c0b776ff050ef8c3812c795
d2a3a5cadb util: make ScheduleBatchPriority advisory only (fanquake)
Pull request description:
ACKs for top commit:
laanwj:
ACK d2a3a5cadb
Tree-SHA512: 14e44360bc6b0c0bfd794cb8a744af7d64fb01aa5602fdb392d6c54799a721ef04426e8379b157dd40f2a33c0b6a5248b09d59c865c453ff1f6e3abbafff524e
33f5fc32e5 test: add rpc getaddressinfo labels test coverage (Jon Atack)
0f3539ac6d test: add listlabels test in wallet_labels.py (Jon Atack)
1388de8390 rpc: add getaddressinfo code documentation (Jon Atack)
2ee0cb3330 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack)
8d1ed0c263 rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack)
5a0ed85070 rpc: improve getaddressinfo RPCHelpman content (Jon Atack)
70cda342cd rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack)
Pull request description:
This PR is a continuation of the work in https://github.com/bitcoin/bitcoin/pull/12892.
Main motivations:
- There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address.
- `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers.
Changes by order of commits:
- [x] improve/fix getaddressinfo RPCHelpman layout formatting
- [x] improve/fix getaddressinfo RPCHelpman content
- [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman
- [x] update getaddressinfo RPCExamples addresses to bech32
- [x] add getaddressinfo code docs
- [x] add a `listlabels` test assertion in wallet_labels.py
- [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests
Here are gists of the CLI help output:
[`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810)
[`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba)
It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups #17578 and #17585 now build on this PR._
ACKs for top commit:
fjahr:
Re-ACK 33f5fc32e5
jnewbery:
ACK 33f5fc32e5.
Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07
27d82b63fb gui: remove macOS start on login code (fanquake)
Pull request description:
The macOS startup item code was disabled for builds targeting macOS >
`10.11` in #15208. Now that we require macOS `10.12` as a minimum (#17550),
we can remove the startup item code entirely. The API we were using, `LSSharedFileListItemCopyResolvedURL`, `LSSharedFileListCopySnapshot` etc,
was removed in macOS `10.12` SDK.
ACKs for top commit:
jonasschnelli:
utACK 27d82b63fb
jonasschnelli:
Tested ACK 27d82b63fb - successfully compiled on 10.15.1
Tree-SHA512: 7420757b91c7820e6a63280887155394547134a9cebcf3721af0284da23292627f94cd431241e033075b3fd86d79ace3ebf1b25d17763acbf71e07a742395409
93352d261f qt: Use proper class for Ui::ReceiveCoinsDialog (Hennadii Stepanov)
8781904643 qt: Fix class name of Ui::ModalOverlay (Hennadii Stepanov)
Pull request description:
Use proper classes for:
- `Ui::ModalOverlay` to remove `<customwidget>` entry
- `Ui::ReceiveCoinsDialog` to be consistent with the code base
This PR does not change behavior.
ACKs for top commit:
jonasschnelli:
Tested ACK 93352d261f - ran this on top of master and tested the modal overlay on initial mainnet sync.
laanwj:
code review ACK 93352d261f
Tree-SHA512: faeed8e86dbf5355505defcdb7e1db07d6a6005ee5eb07367b00f6aa122dd8ad34f8372d4bae7b29c0eac87b538a33157e19328be2876135e8a6376a3197f1bc
f93fc61c65 Put bounds on the number of CPUID leaves explored (Pieter Wuille)
ba2c5fe147 Fix CPUID subleaf iteration (Pieter Wuille)
Pull request description:
This fixes#17523.
The code to determine which CPUID subleaves to explore was incorrect in #17270. The new code here is based on Intel's reference documentation for CPUID (a document called "Intel® Processor Identification and the CPUID Instruction - Application Note 485", which I cannot actually find on their own website).
ACKs for top commit:
laanwj:
ACK f93fc61c65
jonatack:
ACK f93fc61c65 code review, tested rebased on current master bb862d7 with Debian 4.19 x86_64
mzumsande:
ACK f93fc61, reviewed code and compared with the intel doc, tested on an AMD and an Intel processor.
Tree-SHA512: 2790b326fa397b736c0f39f25807bea57de2752fdd58bf6693d044b8cb26df36c11cce165a334b471f8e33724f10e3b76edab5cc4e0e7776601aabda13277245
An opportunistic TopUp is moved from LegacyScriptPubKeyMan::GetNewDestination
to CWallet::GetNewDestination. Another opportunistic TopUp is moved from
LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination)
to ReserveDestination::GetReservedDestination.
Moving opportunistic TopUps ensures that ScriptPubKeyMans will always
be topped up before requesting Destinations from them as we cannot
always rely on future ScriptPubKeyMan implementaions topping up internally.
As such, it is also unnecessary to keep the TopUp calls in the
LegacyScriptPubKeyMan functions so they are moved.
This does not change behavior as TopUp calls are moved up the call stack.
This does not change behavior. This TopUp() is unnecessary as currently
m_spk_man calls TopUp further down the call stack inside
LegacyScriptPubKeyMan::ReserveKeyFromKeyPool (called by LegacyScriptPubKeyMan::GetReservedDestination)
By removing this here, we also prepare for future changes where CWallet
has multiple ScriptPubKeyMans instead of m_spk_man.
Call LegacyScriptPubKeyMan::CanGetAddresses directly instead of calling
CWallet::CanGetAddresses to only query the relevant key manager
This is a minor change in behavior: call now only happens if a new key needs to
be reserved, since if a key is already reserved it might fail unnecessarily.
This change also serves as a sanity check
https://github.com/bitcoin/bitcoin/pull/16341#discussion_r331238394
The macOS startup item code was disabled for builds targeting macOS >
10.11 in #15208. Now that we require macOS 10.12 as a minimum, #17550,
we can remove the startup item code entirely, as the API we were using
was removed in macOS 10.12.
3c2c439dcd wallet: Make -walletdir network only (João Barbosa)
Pull request description:
With this PR `bitcoind -regtest` doesn't run if bitcoin.conf has
```
walletdir=/mnt/mydisk/wallets
```
But works with
```
[regtest]
walletdir=/mnt/mydisk/wallets
```
Doesn't change mainnet behavior.
Closes#15630.
ACKs for top commit:
ryanofsky:
ACK 3c2c439dcd
MarcoFalke:
ACK 3c2c439dcd🍈
meshcollider:
Tested ACK 3c2c439dcd
Tree-SHA512: 8ab3b2db5f3f9cab78b36baaf490c80f7330372cfd8f73fe6536c8fb4c6e55e09f62296feb70617075838b3bcd7101abebbef3b228b6c3dbd42ce8c7a5c372d9
6a2e6b0600 Remove out of date comments for CalculateMaximumSignedTxSize (Gregory Sanders)
Pull request description:
These paths can be hit for probably a number of reasons, and ISMINE spendability is not a requirement to call it.
For example: During watch-only transaction creation, previous transaction in wallet, pubkey imported, but not the witnessscript associated with the prevout.
In this case I think no/minimal comment is better than specific and soon to be out of date.
ACKs for top commit:
achow101:
ACK 6a2e6b0600
darosior:
ACK 6a2e6b0600
Tree-SHA512: ad4c26fd2409eb5aed19d67c19cb5479d226bd11e9298630309c4344f6562ace2e10c2850ebe22770331d71e91320a606e79619b9fe52dd478ce1f589a740122
3958295bc8 wallet: LearnRelatedScripts only if KeepDestination (João Barbosa)
55295fba4c wallet: Lock address type in ReserveDestination (João Barbosa)
Pull request description:
Only mutates the wallet if the reserved key is kept.
First commit is a refactor that makes the address type a class member.
The second commit moves `LearnRelatedScripts` from `GetReservedDestination` to `KeepDestination` to avoid an unnecessary call to `AddCScript` - which in turn prevents multiple entries of the same script in the wallet DB.
ACKs for top commit:
achow101:
Re-ACK 3958295bc8
Sjors:
ACK 3958295bc8
ryanofsky:
Code review ACK 3958295bc8. I like this change. The new behavior makes more sense, and the change makes the code clearer, since the current LearnRelatedScripts call is hard to understand and explain. (Personally, I'd like it if this PR were merged before #17373 or that PR was rebased on top of this one so it would be less confusing.)
meshcollider:
utACK 3958295bc8
Tree-SHA512: 49a5f4b022b28042ad37ea309b28378a3983cb904e234a25795b5a360356652e0f8e60f15e3e64d85094ea63af9be01812d90ccfc08ca4f1dd927fdd8566e33f
c6dd565c88 [gui] watch-only wallet: copy PSBT to clipboard (Sjors Provoost)
39465d545d [wallet] add fillPSBT to interface (Sjors Provoost)
848f889208 [gui] send: include watch-only (Sjors Provoost)
40537f0909 [wallet] ListCoins: include watch-only for wallets without private keys (Sjors Provoost)
Pull request description:
For wallets with `WALLET_FLAG_DISABLE_PRIVATE_KEYS` this makes the watch-only balance available on the send screen (including coin selection). Instead of sending a transaction it generates a PSBT.
The user can take this PSBT and process it with [HWI](https://github.com/bitcoin-core/HWI) or put it an SD card for hardware wallets that support that.
The PSBT is copied to the clipboard. This was the easiest approach; we can add a dialog later to display it, as well as an option to save to disk.
ACKs for top commit:
instagibbs:
test and code review ACK c6dd565c88
meshcollider:
re-ACK c6dd565c88
Tree-SHA512: ebc3da0737e33b255ed926191b84569aedb6097d14868662bd5dce726ce3048e86e9a31eba987b10dffe1482b35c21ae1cd595c2caa4634bc4cf78a826a83852
d0dab897af Refactor: Require scriptPubKey to get wallet SigningProvider (Andrew Chow)
4b0c718f8f Accumulate result UniValue in SignTransaction (Andrew Chow)
Pull request description:
Easier to review ignoring whitespace:
git log -p -n1 -w
This commit does not change behavior. It passes new CScript arguments to
signing functions, but the arguments aren't currently used.
Split from #17261
ACKs for top commit:
instagibbs:
utACK d0dab897af
ryanofsky:
Code review ACK d0dab897af. Thanks for the SignTransaction update. No other changes since last review
Sjors:
Code review ACK d0dab897af
promag:
Code review ACK d0dab897af.
meshcollider:
Code review ACK d0dab897af
Tree-SHA512: c3f52df20fd9d6b3b5aa65562cf5f7dce7b7f44c148b0f988f8b578fce2a28e9b7bf010f5f04bb5bf60f5272b2899f1dbbfb8aee81579c21c9cba559d1d2bb70
b007efdf19 Allow BnB when subtract fee from outputs (Andrew Chow)
db15e71e79 Use BnB when preset inputs are selected (Andrew Chow)
Pull request description:
Currently we explicitly disable BnB when there are preset inputs selected or when the subtract fee from outputs option is enabled. This PR enables BnB for both cases.
Kind of an alternative to #17246 (implements the subtract fee from outputs part of it) and borrows a test from there too.
ACKs for top commit:
instagibbs:
reACK b007efdf19
Sjors:
re-ACK b007efdf19
Tree-SHA512: 933276b09b2fa2ab43db7f0b98762f06f6f5fa8606195f96aca9fa1cb71ae4ee7156028dd482b1cada82ddd0996a9daf12ea5c152589fdf192cd96cbc51e99df
e20c72f9f0 Fire TransactionRemovedFromMempool from mempool (251)
Pull request description:
This pull request fires TransactionRemovedFromMempool callbacks from the mempool and cleans up a bunch of code.
It also resolves the `txmempool -> validation -> validationinterface -> txmempool` circular dependency.
Ideally, `validationinterface` is a dumb component that doesn't have any knowledge of the sub-systems it sends its notifications to. The commit that aims to resolve this circular dependency by moving `txmempool` specific code out of `validationinterface` to `txmempool` where it belongs.
ACKs for top commit:
jnewbery:
ACK e20c72f9f0
Tree-SHA512: 354c3ff1113b21a0b511d80d604edfe3846dddae3355e43d1387f68906e54bf5dc01e7c029edc0b8e635b500b2ab97ee50362e2486eb4319f7347ee9a9e6cef3
a2e581de94 build: Create test utility library from src/test/util/ (Harris)
Pull request description:
This PR creates a static **test utility library** that replaces repetitive compilations of sources from *src/test/util* in **unit**, **gui** and **bench** **tests**.
The original issue is here: https://github.com/bitcoin/bitcoin/issues/17401
The changes are:
* a new *Makefile.test_util.include*
* a new entry in *Makefile.am* that includes *Makefile.test_util.include* when testing is enabled
* removal of all *src/test/util* headers & sources from unit, gui and bench Makefiles
* addition of *libtest_util.a* at LDADD's of every test
ACKs for top commit:
MarcoFalke:
ACK a2e581de94🍞
Tree-SHA512: d172127a26ee70d16625e17d7d94337a65472c57bb97f910c357c52d3dc082ea478ee586ee9074d9ebfeb05b75027e5e15f5bcd2aa35962dadfd9ac6bfd55ab9
e9a27cf338 refactor: Remove unused COINBASE_FLAGS (Neha Narula)
Pull request description:
Commit d449772cf6 stopped setting
COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.
Following up on #17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.
ACKs for top commit:
laanwj:
ACK e9a27cf338
MarcoFalke:
ACK e9a27cf338💻
Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
fa538813b1 scripted-diff: Replace ::mempool with m_node.mempool in tests (MarcoFalke)
8888ad02e2 test: Replace recursive lock with locking annotations (MarcoFalke)
fac07f2038 node: Add reference to mempool in NodeContext (MarcoFalke)
Pull request description:
This is the first step toward making the mempool a global that is not initialized before main.
#### Motivation
Currently the mempool is a global that is initialized before the `main` function. This is confusing and easy to get wrong. E.g. the mempool constructor queries state that has not been initialized, like randomness (fixed), or command line arguments (not an issue last time I checked). Also without having the chainstate (chain tip) initialized first, it doesn't make conceptually sense to have a mempool, since the mempool builds txs on top of the utxo set (chain tip).
Finally, in the future someone might want to run a consensus-only full node (`-nowallet -noblockfilter -no... -nomempool` command line options) that only verifies blocks and updates the utxo set.
This is conceptually the same change that has already been done for the connection manager `CConnman`.
ACKs for top commit:
jnewbery:
utACK fa538813b1
ariard:
Tested ACK fa53881.
Tree-SHA512: 2c446a8a51476354aad7126c2b833500d36b24490caa94f847b2bdc622054de0dae28980f23e3d91b1b492dc32931656d98dbd019af9e4e58f2f8c5375aac694
The function IsStandardTx() returns rejection reason "bare-multisig" if the
transaction has a bare multisig output and the policy flag fIsBareMultisigStd
is false (set by the boolean command-line argument "-permitbaremultisig" -- for
the unit test, we simply set the global flag variable directly).
Commit d449772cf6 stopped setting
COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.
Update the help string to remove "flags", which is not specified in
BIP 22.
faffa7f0dc wallet: Avoid showing GUI popups on RPC errors (take 2) (MarcoFalke)
Pull request description:
Commit 8b0d82bb42 claims "This commit does not change behavior." However, it re-introduced the bug I tried to fix in #17070
ACKs for top commit:
ryanofsky:
Code review ACK faffa7f0dc
Tree-SHA512: 99987f80c76414dca40c7d76b2fe4ea853debbe3c49e7acdeab2596c726a2935c468f4484d49212e65ecc9c8b0d861c0c2b83c1ddfc07670540699199dbfecb0
30fb598737 Fix segfault in allocator_tests/arena_tests (Jeffrey Czyz)
15c84f53f4 Define ARENA_DEBUG in Travis test runs (Jeffrey Czyz)
ad71548822 Fix compilation errors in support/lockedpool.cpp (Jeffrey Czyz)
Pull request description:
Changes in #12048 cause a compilation error in Arena::walk() when
ARENA_DEBUG is defined. Specifically, Arena's chunks_free map was
changed to have a different value type.
Additionally, missing includes cause other compilation errors when
ARENA_DEBUG is defined.
Reproduced with:
make CPPFLAGS=-DARENA_DEBUG
ACKs for top commit:
laanwj:
ACK 30fb598737
fanquake:
ACK 30fb598737 - thanks for following up jkczyz.
Tree-SHA512: 4eec368a4e9c67e4e2a27bc05608a807c2892d50c60d06ed21490cd274c0369f9671bc05b3006acc2a193316caf4896454c9c299603bfed29bd488f1987ec446
8944c1d340 Changed tooltips of receive form to highlight difference between Label and Message (dannmat)
Pull request description:
I have changed the tooltips for 'Label' & 'Message' text fields to be more clear, stating the difference between the two (#17173)
ACKs for top commit:
MarcoFalke:
ACK 8944c1d340
laanwj:
ACK 8944c1d340
Tree-SHA512: 7fbea4d3c4416264ae6c146d51d29958c418a278bdd6744133db0b684ad7a9413178c005592aa21a81d127f3f3a8583fc5de00078239db08e6f101f657a5dd3a
Also remove a needless loop in DecodeBase58 to prune zeroes in the base256
output of the conversion. The number of zeroes is implied by keeping track
explicitly of the length during the loop.
Invalid PSBTs need to be re-created, so the next role is the
Creator (new PSBTRole). Additionally, we need to know what went
wrong so an error field was added to PSBTAnalysis.
A PSBTAnalysis indicating invalid will have empty everything,
next will be set to PSBTRole::CREATOR, and an error message.
e5a0bece6e doc: add OpenSSL removal to release-notes.md (fanquake)
397dbae070 ci: remove OpenSSL installation (fanquake)
a4eb839619 doc: remove OpenSSL from build instructions and licensing info (fanquake)
648b2e3c32 depends: remove OpenSSL package (fanquake)
8983ee3e6d build: remove OpenSSL detection and libs (fanquake)
b49b6b0f70 random: Remove remaining OpenSSL calls and locking infrastructure (fanquake)
4fcfcc294e random: stop retrieving random bytes from OpenSSL (fanquake)
5624ab0b4f random: stop feeding RNG output back into OpenSSL (fanquake)
Pull request description:
Now that #17165 has been merged, removing our remaining OpenSSL usage is possible.
That remaining usage was a call to [`RAND_bytes`](https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html) during the ::SLOW path of [ProcRand](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L616). As well as feeding output from our RNG back into OpenSSL via [`RAND_add`](https://www.openssl.org/docs/manmaster/man3/RAND_add.html) during the ::SLOW and ::SLEEP paths.
Optimistically tagged for `0.20.0`. Needs discussion, potentially in an upcoming weekly meeting?
Closes#12530.
ACKs for top commit:
MarcoFalke:
ACK e5a0bece6e
laanwj:
ACK e5a0bece6e
Tree-SHA512: 02fce08ec91d20e0da51e9314eec53dcf8699cded02f0a005417d627520c20b826332cb42bdae132af283d4903aa3088a9f613f3aea915d655a51532a4d4796c
Easier to review ignoring whitespace:
git log -p -n1 -w
This commit does not change behavior. It passes new CScript arguments to
signing functions, but the arguments aren't currently used.
SignTransaction will be called multiple times in the future. Pass
it a result UniValue so that it can accumulate the results of multiple
SignTransaction passes.
49f4c7f069 tests: Add fuzzing harness for various PSBT related functions (practicalswift)
Pull request description:
Add fuzzing harness for various PSBT related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/psbt
```
ACKs for top commit:
MarcoFalke:
re-ACK 49f4c7f069🐟
Tree-SHA512: 4cebe62bd8c244ee40a43e829f5bd175ab40e1dfbbab1affb1529374858225820d6c9fa9ba45862bf56c1522845422fd96d620cedbdec52a67ac1449dec4e1b2
e161bc74d2 doc: Remove bitness from bitcoin-qt help message and manpage (Wladimir J. van der Laan)
Pull request description:
Remove the `(64-bit)` from the bitcoin-qt help message.
Since removing the Windows 32-bit builds, it is no longer information that is often useful for troubleshooting. This never worked for other architectures than x86, and the only 32-bit x86 build left is the Linux one. Linux users tend to know what architecture they are using.
It also accidentally ends up in the bitcoin-qt manpage (if you happen to be generating them on a x86 machine), which gets checked in. See for example 1bc9988993 (diff-e4b84be382c8ea33b83203ceb8c85296)
ACKs for top commit:
practicalswift:
ACK e161bc74d2 -- rationale makes sense and diff looks correct :)
MarcoFalke:
Tested ACK e161bc74d2🔮
Tree-SHA512: d38754903252896dc86fac6c12ad6615d322c2744db7c02b18574a08c69e8876b2c905e1f09b324002236b111ee93479f89769c562e7b3b2e6eb2992d76464ef
On the ::SLOW path we would use OpenSSL as an additional source of
random bytes. This commit removes that functionality. Note that this was
always only an additional source, and that we never checked the return
value
RAND_bytes(): https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html
RAND_bytes() puts num cryptographically strong pseudo-random bytes into buf.
On the ::SLOW or ::SLEEP paths, we would feed our RNG output back into
OpenSSL using RAND_add. This commit removes that functionality.
RAND_add(): https://www.openssl.org/docs/manmaster/man3/RAND_add.html
RAND_add() mixes the num bytes at buf into the internal state of the
random generator. This function will not normally be needed, as
mentioned above. The randomness argument is an estimate of how much
randomness is contained in buf, in bytes, and should be a number
between zero and num.
2f5f7d6b13 GuessVerificationProgress: cap the ratio to 1 (darosior)
Pull request description:
Noticed `getblockchaininfo` would return a `verificationprogress` > 1, especially while generating. This caps the verification progress to `1`.
Tried to append a check to functional tests but this would pass even without the patch, so it seems better to not add a superfluous check (but this can easily be reproduced by trying to generate blocks in the background and `watch`ing `getblockchainfo`).
ACKs for top commit:
laanwj:
ACK 2f5f7d6b13
promag:
ACK 2f5f7d6b13.
Tree-SHA512: fa3aca12acab9c14dab3b2cc94351082f548ea6e6c588987cd86e928a00feb023e8112433658a0e85084e294bfd940eaafa33fb46c4add94146a0901bc1c4f80
d1c02775aa Report amount of data gathered from environment (Pieter Wuille)
64e1e022ce Use thread-safe atomic in perfmon seeder (Pieter Wuille)
d61f2bb076 Run background seeding periodically instead of unpredictably (Pieter Wuille)
483b94292e Add information gathered through getauxval() (Pieter Wuille)
11793ea22e Feed CPUID data into RNG (Pieter Wuille)
a81c494b4c Use sysctl for seeding on MacOS/BSD (Pieter Wuille)
2554c1b81b Gather additional entropy from the environment (Pieter Wuille)
c2a262a78c Seed randomness with process id / thread id / various clocks (Pieter Wuille)
723c796667 [MOVEONLY] Move cpuid code from random & sha256 to compat/cpuid (Pieter Wuille)
cea3902015 [MOVEONLY] Move perfmon data gathering to new randomenv module (Pieter Wuille)
b51bae1a5a doc: minor corrections in random.cpp (fanquake)
Pull request description:
This introduces a new `randomenv` module that queries varies non-cryptographic (and non-RNG) sources of entropy available on the system; things like user IDs, system configuration, time, statistics, CPUID data.
The idea is that these provide a fallback in scenarios where system entropy is somehow broken (note that if system entropy *fails* we will abort regardless; this is only meant to function as a last resort against undetected failure). It includes some data sources OpenSSL currently uses, and more.
The separation between random and randomenv is a bit arbitrary, but I felt that all this "non-essential" functionality deserved to be separated from the core random module.
ACKs for top commit:
TheBlueMatt:
utACK d1c02775aa. Certainly no longer measuring the time elapsed between a 1ms sleep (which got removed in the latest change) is a fair tradeoff for adding about 2 million other actually-higher-entropy bits :).
laanwj:
ACK d1c02775aa
Tree-SHA512: d290a8db6538a164348118ee02079e4f4c8551749ea78fa44b2aad57f5df2ccbc2a12dc7d80d8f3e916d68cdd8e204faf9e1bcbec15f9054eba6b22f17c66ae3
Remove the `(64-bit)` from the bitcoin-qt help message.
Since removing the Windows 32-bit builds, it is no longer information
that is often useful for troubleshooting. This never worked for other
architectures than x86, and the only 32-bit x86 build left is the Linux
one. Linux users tend to know what architecture they are using.
It also accidentally ends up in the bitcoin-qt manpage.
The test uses reinterpret_cast<void*> on unallocated memory. Using this
memory in printchunk as char* causes a segfault, so have printchunk take
void* instead.
Changes in #12048 cause a compilation error in Arena::walk() when
ARENA_DEBUG is defined. Specifically, Arena's chunks_free map was
changed to have a different value type.
Additionally, missing includes cause other compilation errors when
ARENA_DEBUG is defined.
Reproduced with:
make CPPFLAGS=-DARENA_DEBUG
5e8a56348b test: add unit test for non-standard txs with too large scriptSig (Sebastian Falbesoner)
Pull request description:
Approaches the first missing test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason `"scriptsig-size"` if any one the inputs' scriptSig is larger than 1650 bytes.
ACKs for top commit:
MarcoFalke:
ACK 5e8a56348b
instagibbs:
ACK 5e8a56348b
Tree-SHA512: 79977b12ddea9438a37cefdbb48cc551e4ad02a8ccfaa2d2837ced9f3a185e2e07cc366c243b9e3c7736245e90e315d7b4110efc6b440c63dbef7ee2c9d78a73
-BEGIN VERIFY SCRIPT-
# tx pool member access (mempool followed by dot)
sed --regexp-extended -i -e 's/(::)?\<mempool\>\.([a-zA-Z])/m_node.mempool->\2/g' $(git grep -l mempool ./src/test)
# plain global (mempool not preceeded by dot, but followed by comma)
sed --regexp-extended -i -e 's/([^\.])(::)?\<mempool\>,/\1*m_node.mempool,/g' $(git grep -l mempool ./src/test)
-END VERIFY SCRIPT-
Currently it is an alias to the global ::mempool and should be used as
follows.
* Node code (validation and transaction relay) can use either ::mempool
or node.mempool, whichever seems a better fit.
* RPC code should use the added convenience getter EnsureMempool, which
makes sure the mempool exists before use. This prepares the RPC code
to a future where the mempool might be disabled at runtime or compile
time.
* Test code should use m_node.mempool directly, as the mempool is always
initialized for tests.
edb6b768a4 fix uninitialized variable nMinerConfirmationWindow (NullFunctor)
Pull request description:
It is used for the computation of `BIP9WarningHeight`, and by that time it isn't initialized.
ACKs for top commit:
jnewbery:
utACK edb6b768a
promag:
ACK edb6b768a4, commit description could be cleaned up though.
MarcoFalke:
ACK edb6b768a4, used python3 to do the addition locally 📍
practicalswift:
ACK edb6b768a4, used `clang++ -O2` on the previous version^W^W^W^W^W^W`bc` to verify the addition locally 🏓
Sjors:
Code review ACK edb6b76. Nit: commit description has duplicate text.
Tree-SHA512: 6fa0be0ecfbfd5d537f2c5b4a9333c76530c1f3182f777330cc7939b0496e37b75d8f8810cdaf471a9bd3247b425f2e239578300dfa0d5a87cd14a6ccfafa619
Also rename the "result_complete" variable in GetSettingsList() to "done" to be
more consistent with GetSetting().
This change doesn't affect current behavior but could be useful in the future
to support dynamically changing settings at runtime and adding new settings
sources, because it lets high priority sources reset settings back to default
(see test).
By removing a special case for null, this change also helps merge code treat
settings values more like black boxes, and interfere less with settings parsing
and retrieval.
The bool/int/string flags were added speculatively in #16097 and trigger errors
when type checking is actually implemented in
https://github.com/bitcoin/bitcoin/pull/16545
-BEGIN VERIFY SCRIPT-
sed -i 's/ALLOW_\(BOOL\|INT\|STRING\)/ALLOW_ANY/g' src/test/util_tests.cpp src/test/getarg_tests.cpp
-END VERIFY SCRIPT-
This commit does not change behavior.
Test GetSetting and GetArg type coercion, negation, and default value handling.
Test is expanded later to cover other flags besides ALLOW_ANY when they are
implemented in https://github.com/bitcoin/bitcoin/pull/16545
This commit does not change behavior.
Add for consistency with ArgsManager::GetSetting method and to make setting
types accessible to ArgsManager callers and tests (test added next commit).
This commit does not change behavior.
* Instead of calling RandAddSeedSleep anytime the scheduler goes
idle, call its replacement (RandAddSeedPeriodic) just once per
minute. This has better guarantees of actually being run, and
helps limit how frequently the dynamic env data is gathered.
* Since this code runs once per minute regardless now, we no
longer need to keep track of the last time strengthening was
run; just do it always.
* Make strengthening time context dependent (100 ms at startup,
10 ms once per minute afterwards).
