This change prevents UB in case of early g_lockstack destroying.

Co-authored-by: Wladimir J. van der Laan <[email protected]>

Some functions should be returning std::string instead of const char*.
This commit changes that.

if -peerblockfilters is configured, handle requests for cfheaders.

Also, add interruption points to scantxoutset

These haven't been updated since their addition, so this updates the list that
controls which qt base translations are bundled with the macOS binary, to all the
languages that are available with qt 5.9.8.

This could probably be improved in some way, however qt updates are infrequent,
and I didn't want to spend any more time looking at this. Also given that no-one
seems to have noticed and/or reported this it wouldn't seem high-priority.

Could be backported to 0.20.1.

This commit returns 'fee' in the testmempoolaccept rpc results.
'fee' is only returned if the transaction is accepted in mempool.

Existing functional tests are modified to reflect changed behaviour.

The only call to Salvage set fAggressive = true so remove that parameter
and always use DB_AGGRESSIVE

We need this exposed for BerkeleyBatch::Recover to be moved out.

…andalone

Instead of having these be class static functions, just make them be
standalone. Also removes WalletBatch::Recover which just passed through
to BerkeleyBatch::Recover.

69bfcac gui: update Qt base translations for macOS release (fanquake)

Pull request description:

  These haven't been updated since their addition, so this updates the list that
  controls which qt base translations are bundled with the macOS binary, to all the
  languages that are available with qt 5.9.8.

  This could probably be improved in some way, however qt updates are infrequent,
  and I didn't want to spend any more time looking at this. Also given that no-one
  seems to have noticed and/or reported this it wouldn't seem high-priority.

  Could be backported to 0.20.1.

  Master:
  ![master](https://user-images.githubusercontent.com/863730/82729428-11bce200-9d2a-11ea-8569-ee65d46c7403.png)

  This PR:
  ![fixed](https://user-images.githubusercontent.com/863730/82729427-0f5a8800-9d2a-11ea-86dd-1e6a3e211efa.png)

ACKs for top commit:
  hebasto:
    ACK 69bfcac, tested on macOS 10.15.

Tree-SHA512: df142fb16097deb514e72e005b73aafc4eb4ff0c17e423ba5040a3ec6874020a733e1c5259a88923580e71ef73c16222aed28f482b8c270a544a85b745a7b327

f9b22e3 tests: Add fuzzing harness for CCoinsViewCache (practicalswift)

Pull request description:

  Add fuzzing harness for `CCoinsViewCache`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  MarcoFalke:
    ACK f9b22e3 📫

Tree-SHA512: 4fa79aab683875eef128b672cf199909c86e4d2ed7c406f006fa27a546dafc9cb0061c4de5e660e622458072f1dab69dbf6b6b03d5b863f81c5710bf4cee6c0c

…ible

fa75692 rpc: Make gettxoutsetinfo/GetUTXOStats interruptible (MarcoFalke)
fa7fc5a rpc: factor out RpcInterruptionPoint from dumptxoutset (MarcoFalke)

Pull request description:

  Make it interruptible, so that shutdown doesn't block for up to one hour.

  Fixes (partially) bitcoin#13217

ACKs for top commit:
  Empact:
    Code Review ACK bitcoin@fa75692
  laanwj:
    Code review ACK fa75692

Tree-SHA512: 298261e0ff7d79fab542b8f6828cc0ac451cbafe396d5f0816c9d36437faba1330f5c4cb2a25c5540e202bfb9783da6ec858bd453056ce488d21e36335d3d42c

90eb027 doc: Add and fix comments about never destroyed objects (Hennadii Stepanov)
26c093a Replace thread_local g_lockstack with a mutex-protected map (Hennadii Stepanov)
58e6881 refactor: Refactor duplicated code into LockHeld() (Hennadii Stepanov)
f511f61 refactor: Add LockPair type alias (Hennadii Stepanov)
8d8921a refactor: Add LockStackItem type alias (Hennadii Stepanov)
458992b Prevent UB in DeleteLock() function (Hennadii Stepanov)

Pull request description:

  Tracking our instrumented mutexes (`Mutex` and `RecursiveMutex` types) requires that all involved objects should not be destroyed until after their last use. On master (ec79b5f) we have two problems related to the object destroying order:
  - the function-local `static` `lockdata` object that is destroyed at [program exit](https://en.cppreference.com/w/cpp/utility/program/exit)
  - the `thread_local` `g_lockstack` that is destroyed at [thread exit](https://en.cppreference.com/w/cpp/language/destructor)

  Both cases could cause UB at program exit in so far as mutexes are used in other static object destructors.

  Fix bitcoin#18824

ACKs for top commit:
  MarcoFalke:
    re-ACK 90eb027, only change is new doc commit 👠
  ryanofsky:
    Code review ACK 90eb027 because all the changes look correct and safe. But I don't know the purpose of commit  26c093a "Replace thread_local g_lockstack with a mutex-protected map (5/6)." It seems like it could have a bad impact on debug performance, and the commit message and PR description don't give a reason for the change.

Tree-SHA512: 99f29157fd1278994e3f6eebccedfd9dae540450f5f8b980518345a89d56b635f943a85b20864cef087027fd0fcdb4880b659ef59bfe5626d110452ae22031c6

71f016c Remove old serialization primitives (Pieter Wuille)
92beff1 Convert LimitedString to formatter (Pieter Wuille)
ef17c03 Convert wallet to new serialization (Pieter Wuille)
65c589e Convert Qt to new serialization (Pieter Wuille)

Pull request description:

  This is the final step 🥳 of the serialization improvements extracted from bitcoin#10785.

  It converts the LimitedString wrapper to a new-style formatter, and updates the wallet and Qt code to use the new serialization framework. Finally all remaining old primitives are removed.

ACKs for top commit:
  jonatack:
    ACK 71f016c reviewed diff, builds/tests/re-fuzzed.
  laanwj:
    Code review ACK 71f016c

Tree-SHA512: d952194bc73259f6510bd4ab1348a1febbbf9862af30f905991812fb0e1f23f15948cdb3fc662be54d648e8f6d95b11060055d2e7a8c2cb5bf008224870b1ea1

fa80b47 test: Remove global wait_until from p2p_getdata (MarcoFalke)
999922b test: Default mininode.wait_until timeout to 60s (MarcoFalke)
fab4737 test: pep-8 p2p_getdata.py (MarcoFalke)

Pull request description:

  Using the global wait_until makes it impossible to adjust the timeout based on the hardware the test is running on.

  Fix that by using the mininode member function.

  So for example, `./test/functional/p2p_getdata.py  --timeout-factor=0.04` gives a timeout of 2.4 seconds.

ACKs for top commit:
  laanwj:
    ACK fa80b47

Tree-SHA512: ebb1b7860a64451de2b8ee9a0966faddb13b84af711f6744e8260d7c9bc0b382e8fb259897df5212190821e850ed30d4d5c2d7af45a97f207fd4511b06b6674a

84ae057 Add release notes about salvage changes (Andrew Chow)
ea337f2 Move RecoverKeysOnlyFilter into RecoverDataBaseFile (Andrew Chow)
9ea2d25 Move RecoverDatabaseFile and RecoverKeysOnlyFilter into salvage.{cpp/h} (Andrew Chow)
b426c77 Make BerkeleyBatch::Recover and WalletBatch::RecoverKeysOnlyFilter standalone (Andrew Chow)
2741774 Expose a version of ReadKeyValue and use it in RecoverKeysOnlyFilter (Andrew Chow)
ced95d0 Move BerkeleyEnvironment::Salvage into BerkeleyBatch::Recover (Andrew Chow)
07250b8 walletdb: remove fAggressive from Salvage (Andrew Chow)
8ebcbc8 walletdb: don't automatically salvage when corruption is detected (Andrew Chow)
d321046 wallet: remove -salvagewallet (Andrew Chow)
cdd955e Add basic test for bitcoin-wallet salvage (Andrew Chow)
c877709 wallettool: Add a salvage command (Andrew Chow)

Pull request description:

  Removes the `-salvagewallet` startup option and adds a `salvage` command to the `bitcoin-wallet` tool. As such, `-salvagewallet` is removed. Additionally, the automatic salvage that is done if the wallet file fails to load is removed.

  Lastly the salvage code entirely is moved out entirely into `bitcoin-wallet` from `walletdb.{cpp/h}` and `db.{cpp/h}`.

ACKs for top commit:
  jonatack:
    ACK 84ae057 feedback taken, and compared to my previous review, the bitcoin-wallet salvage command now seems to run and it exits without raising. The new test passes at both 9454105 and 84ae057 so as a sanity check I'd agree there is room for improvement, if possible.
  MarcoFalke:
    re-ACK 84ae057 🏉
  Empact:
    Code Review ACK bitcoin@84ae057
  ryanofsky:
    Code review ACK 84ae057. Lot of small changes since previous review: added verify step before salvage, added basic test in new commit, removed unused scanstate variable and warnings parameter, tweaked various comments and strings, moved fsuccess variable declaration
  meshcollider:
    Concept / light code review ACK 84ae057

Tree-SHA512: 05be116b56ecade1c58faca1728c8fe4b78f0a082dbc2544a3f7507dd155f1f4f39070bd1fe90053444384337bc48b97149df5c1010230d78f8ecc08e69d93af

1c91ffe doc : add link to readme.md in the first section (pad)

Pull request description:

  I have searched how to do it in this doc for some time :-(
  I think it might help other newbies interested in building with visual studio.

ACKs for top commit:
  hebasto:
    ACK 1c91ffe, a new link works as expected :)

Tree-SHA512: 42ef3ba374bced9b4ab0010fe8c30de06f59ff8a84f8e02f8a91f33e7e403cf91d624fc7df3f45096df53171a90b9ff60277969cc30f1357d92094ad72ca9d53

4c82579 Remove outdated comment about DER encoding (Elichai Turkel)

Pull request description:

  This comment got me confused about the status of BIP66 (Thanks jnewbery for explaining)
  The comment was added in: bitcoin#3843
  But in bitcoin#5713 strict DER encoding was enforced in consensus,
  and is now it's buried and enforced by the height of the block here: https://github.com/bitcoin/bitcoin/blob/4af01b37d40246cd1fdb54719855927e36a36b46/src/validation.cpp#L1889

  P.S. This is also quite confusing: https://github.com/bitcoin/bitcoin/blob/4af01b37d40246cd1fdb54719855927e36a36b46/src/validation.cpp#L1560-L1563 But seems to be intentional: https://github.com/bitcoin/bitcoin/blob/4af01b37d40246cd1fdb54719855927e36a36b46/src/validation.cpp#L1510-L1511

ACKs for top commit:
  laanwj:
    ACK 4c82579

Tree-SHA512: 7afbbae84ed4ecfaa0a273ae024b14f2b7ffe65307f078086fe0b5b645c57722bc2952fb15d167d9e4fa5b052d1d0ac6e5e33f57e8fc881c0ea611d352bccc1e

c57f03c refactor: Replace const char* to std::string (Calvin Kim)

Pull request description:

  Rationale: Addresses bitcoin#19000
  Some functions should be returning std::string instead of const char*.
  This commit changes that.

  Main benefits/reasoning:

  1.  The functions never return nullptr, so returning a string makes code at call sites easier to review (reviewers don't have to read the source code to verify that a nullptr is never returned)
  2. All call sites convert to string anyway

ACKs for top commit:
  MarcoFalke:
    re-ACK c57f03c (no changes since previous review) 🚃
  Empact:
    Fair enough, Code Review ACK bitcoin@c57f03c
  practicalswift:
    ACK c57f03c -- patch looks correct
  hebasto:
    re-ACK c57f03c

Tree-SHA512: 9ce99bb38fe399b54844315048204cafce0f27fd8f24cae357fa7ac6f5d8094d57bbf5f5c1f5878a65f2d35e4a3f95d527eb17f49250b690c591c0df86ca84fd

This commit returns 'fee' in the testmempoolaccept rpc results.
'fee' is only returned if the transaction is accepted in mempool.

Existing functional tests are modified to reflect changed behaviour.