c1c6c410a66996b2d60d5172189b5a5ec8100842 test: add reason checks for non-standard txs in test_IsStandard (Sebastian Falbesoner)
Pull request description:
While taking a look at #17272 I noticed that for some reason the unit test `test_IsStandard` (which was not adapted to the policy change in the referenced PR commits) didn't fail as expected:
6a97e8a060/src/test/transaction_tests.cpp (L758-L762)
It turned out that `IsStandardTx()` returned `"dust"` as rejection reason (instead of the expected `"multi-op-return"`), leading to the conclusion that 5fe6f052bd erroneously performs the `IsDust()` check also for TX_NULL_DATA transactions. To avoid cases like this in the future, this PR makes the unit test `test_IsStandard` more strict by also checking for the concrete reason after each occurence of `IsStandardTx()` returning false.
ACKs for top commit:
instagibbs:
utACK c1c6c410a6
Tree-SHA512: c7419884cc52977c73f8f8c476eaebed80ba7bda4d03509d3f46dd977be911389f7b53daefa5ef31d2f7df9402243152e01e83f1b8a9fb300c19d1a0f69a89a9
091747b46ecf06244ce2650028f1833b2e7c5062 gui: Add shortcuts for tab tools (João Barbosa)
Pull request description:
This makes accessing the RPC console very fast/easy. It also improves accessibility.
<img width="234" alt="Screenshot 2019-10-02 at 01 30 53" src="https://user-images.githubusercontent.com/3534524/66009867-50104300-e4b4-11e9-90b5-6b8dc961a8a1.png">
ACKs for top commit:
jonasschnelli:
Tested ACK 091747b46ecf06244ce2650028f1833b2e7c5062 - this is an improvment. Further solutions to solve the interference between the console and the shortcuts (if possible) can be done upstream (Qt) or with another PR.
Tree-SHA512: 6b8bc07e8a3a75e53c05f0fdb73458d75ef025f950569e885e655de53fdac8b91dcabfb1c6e643b1d23065420fa2701847c00cc1718bc188778640aefb5bcbd8
37b8475dcf61383dffffebae374eab07c14aee80 Chainparams: Use name constants in chainparams initialization (Jorge Timón)
Pull request description:
I thought this wouldn't work for some reason, but it seems it does.
Just a little bit more consistency. I'm still not able to use them in qt/networkstyle.cpp though, not sure why.
ACKs for top commit:
MarcoFalke:
ACK 37b8475dcf61383dffffebae374eab07c14aee80
laanwj:
ACK 37b8475dcf61383dffffebae374eab07c14aee80
hebasto:
ACK 37b8475dcf61383dffffebae374eab07c14aee80, I have reviewed the code and it looks OK, I agree it can be merged.
fjahr:
ACK 37b8475
Tree-SHA512: d9fa5df5650e10c645ac1f3afe831674a47f35d4a649e18a3d2aee1d04b08e6896aff6f1bbed0630d28775c51f989f9daaa9e405c9f3d7dca30e639a6f9008f0
fa398091b7ad683dfd3cd3c2cd030eaf9f336737 Avoid unused call to GuessVerificationProgress in NotifyHeaderTip (MarcoFalke)
Pull request description:
`GuessVerificationProgress` for a header (not a block) is always 0 because the number of txs in the block can not be determined from the header alone. Anyway, this result was never used, so we can optimize this call by hardcoding 0.
This is the next commit in a series of changes toward removing nChainTx (see #14863, #13875)
ACKs for top commit:
promag:
Code review ACK fa398091b7ad683dfd3cd3c2cd030eaf9f336737, missed that.
laanwj:
ACK fa398091b7ad683dfd3cd3c2cd030eaf9f336737
Tree-SHA512: 11016f8dbb1af1cf75241948d1ad35eac0c79d1311cd0db8c6ec806df2a9e3dc5f998dbd66ccbad5d84564e6cec7fe21ce7a2a13c2b34c746e2d3b31aa1db53a
04dbdd613fe5c0a742de915e07553614cd4cbf46 [net] SocketHandler: log peer id for close and disconnect (Sjors Provoost)
Pull request description:
When combined with `-logips` this makes it easier to diagnose disconnects.
To test on macOS, find a connection you want to disrupt:
```
lsof -nP -iTCP:8333 -sTCP:ESTABLISHED
```
To shut it down gracefully you can use tcpkill or this Python script: https://github.com/google/tcp_killer
The log should say:
```
2019-10-25T13:26:55Z socket closed for peer=1
2019-10-25T13:26:55Z disconnecting peer=1
2019-10-25T13:26:55Z Cleared nodestate for peer=1
```
To shut it down ungracefully I made a patch to the above script, adding a `-force` argument. _Careful, this may result in data corruption_. Then the log should say:
```
2019-10-25T13:39:57Z socket select error Bad file descriptor (9)
2019-10-25T13:39:57Z socket recv error for peer=0: Bad file descriptor (9)
2019-10-25T13:39:57Z disconnecting peer=0
2019-10-25T13:39:57Z Socket close failed: 35. Error: Bad file descriptor (9)
2019-10-25T13:39:57Z Cleared nodestate for peer=0
```
ACKs for top commit:
MarcoFalke:
unsigned ACK 04dbdd613fe5c0a742de915e07553614cd4cbf46
TheBlueMatt:
unsigned ACK 04dbdd613fe5c0a742de915e07553614cd4cbf46 LGTM!
theuni:
unsigned ACK 04dbdd613fe5c0a742de915e07553614cd4cbf46.
Tree-SHA512: 415313908484f97ffe11a48b4ed6afab3ab0be660c788adb9ad975f88b69aa1cfd5ccbe5859350cdf19ef8fde191fd530fb22cef34e70638defdc9f3d761c71d
* chore: bump MIN_PEER_PROTO_VERSION to 70215
simplifies logic, removes branches
This protocol version is from v14 in May of 2019, should be more than safe to bump this
* drop MIN_COINJOIN_PEER_PROTO_VERSION
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Trying to use them results in "Error parsing command line arguments: Invalid parameter" error. Should either add them to hidden_args or drop them. The latter seems like a better option since no one should be using them by now anyway.
9999f891d1c9093e552492cf8ccc3168370c7a39 bench: Avoid deprecated use of volatile += (MarcoFalke)
Pull request description:
Deprecated in C++20 according to https://eel.is/c++draft/expr.ass#6 .
```
bench/examples.cpp:16:13: warning: compound assignment with ‘volatile’-qualified left operand is deprecated [-Wvolatile]
16 | sum += sin(d);
| ~~~~^~~~~~~~~
```
While C++20 is currently unsupported, I don't see any downside to a minor fixup to an example benchmark. This will also make a hypothetical C++20 patch smaller.
ACKs for top commit:
fanquake:
ACK 9999f891d1c9093e552492cf8ccc3168370c7a39
Tree-SHA512: ca7d660fa8eba347a4648408a8b97a0ecb8263a825da7abd59129d783058102581e05b273667989f95480436a66d5384bd1e92d9ae79408f5b30e2178935cc38
fadcd031390dd4588bbb1c07e5020a7131312050 Fix unsigned integer overflow in LoadMempool (MarcoFalke)
Pull request description:
It doesn't seem ideal to have an integer sanitizer enabled, but then disable it for the whole validation.cpp file.
This removes one of the two violations.
This should be a refactor.
ACKs for top commit:
prayank23:
Code Review ACK fadcd03139
Tree-SHA512: 9fb2f3d49008a59cd45b7c17be0c88c04e61183197c11c8176865af5532c8d0c940db49a351dd0fc75e1d7fd8678c3b816d34cfca170dc6b9cf8f37fdf1c8cae
ffd11ea87640c8a3d83b6f83ac18e65234fc6002 Fix typo and grammar (Heebs)
Pull request description:
Fix typo and grammar in the coin selection algorithm's description.
ACKs for top commit:
meshcollider:
ACK ffd11ea87640c8a3d83b6f83ac18e65234fc6002
Tree-SHA512: bba07c2efd5140fb3e021618739d70aaa761bbc274afb8158809492b0606773c217e42e58e58b18a2454b9c45ebc883ebece17cdc467ac60e3d3140d7a979db7
faa185bb3abe5fdaeeae14706bad9437acac6a69 Revert "Fixes Bug in Transaction generation in ComplexMempool benchmark" (MarcoFalke)
Pull request description:
Developers are reporting crashes (potentially OOM) on IRC, but I can't reproduce. Still, revert this for now, since one developer reported the bare metal this was running on crashed.
Top commit has no ACKs.
Tree-SHA512: 080db4fcfc682b68f4cc40dfabd9d3e0e3f6e6297ce4b782d5de2c83bc18f85f60efb1cda64c51e23c4fd2a05222a904e7a11853d9f9c052dcd26a53aa00b235
29e983386b0aecf99cdb7d0e08ba6b450bed313e Fixes Bug in Transaction generation in ComplexMempool benchmark (Shorya)
Pull request description:
This fixes issues with `ComplexMempool` benchmark introduced in [#17292](https://github.com/bitcoin/bitcoin/pull/17292) , this stress test benchmarks performance of ancestor and descendant tracking of mempool graph algorithms on a complex Mempool.
This Benchmark first creates 100 base transactions and stores them in `available_coins` vector. `available_coins` is used for selecting ancestor transactions while creating 800 new transactions. For this a random transaction is picked from `available_coins` and some of its outputs are mapped to the inputs of the new transaction being created.
Now in case we exhaust all the outputs of an entry in `available_coins` then we need to remove it from `available_coins` before the next iteration of choosing a potential ancestor , it is now implemented with this patch.
As the index of the entry is randomly chosen from `available_coins` , In order to remove it from the vector , if index of the selected entry is not at the end of `available_coins` vector , it is swapped with the entry at the back of the vector , then the entry at the end of `available_coins` is popped out.
Earlier the code responsible for constructing outputs of the newly created transaction was inside the loop used for assigning ancestors to the transaction , which does some unnecessary work as it creates outputs of the transaction again and again , now it is moved out of the loop so outputs of the transaction are created just once before adding it to the final list of the transactions created. This one is a minor change to save some computation.
These changes have changed the `ComplexMempool` benchmark results on `bitcoin:master` as follows :
**Before**
>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 232,881,625.00 | 4.29 | 0.7% | 2.55 | `ComplexMemPool`
**After**
>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 497,275,135.00 | 2.01 | 0.5% | 5.49 | `ComplexMemPool`
Top commit has no ACKs.
Tree-SHA512: d6946d7e65c55f54c84cc49d7abee52e59ffc8b7668b3c80b4ce15a57690ab00a600c6241cc71a2a075def9c30792a311256fed325ef162f37aeacd2cce93624
459e208276a4d1457d37bf41c977e62caf05456d Exit early for an empty vChecks in CCheckQueue::Add (Hennadii Stepanov)
c43aa623435a277a692dbde784e8a7146f5573e9 Avoid excessive lock contention in CCheckQueue::Add (Hennadii Stepanov)
Pull request description:
This PR significantly reduces lock contention in the `CCheckQueue` class by releasing a mutex before calling `std::condition_variable::notify_one` and `std::condition_variable::notify_all`.
From C++ [docs](https://en.cppreference.com/w/cpp/thread/condition_variable/notify_one):
> The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s); in fact doing so is a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock.
Related to:
- #23167
- #23223
ACKs for top commit:
martinus:
ACK 459e208, codereview and tested. I first thought this introduced a segfault in `psbt_wallet_tests/psbt_updater_test` because that test failed for me, but thats a different issue fixed in #23403.
vasild:
ACK 459e208276a4d1457d37bf41c977e62caf05456d
theStack:
Code-review ACK 459e208276a4d1457d37bf41c977e62caf05456d
Tree-SHA512: c197858656392ba3ebcd638d713cf93c9fb48b7b3bad193209490d2828f9c7e3ae4dee6f84674f2f34dceed894139562e29579ee7299e06756c8c990caddc5ed
cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c gui: fix autofocus in CreateWalletActivity::askPassphrase() (Jon Atack)
539d9403af956c76ae0149a58c07c71a6b58ac69 gui: fix passphrase labels/tooltip in createwalletdialog/askpassphrasedialog (Jon Atack)
43aa9b0d790840e30c1cd128e67946ffcbfeb721 gui: rename encrypt(), blank(), and askPasshprase() (Jon Atack)
Pull request description:
Closes#16820. The wallet [name escaping issue](https://github.com/bitcoin/bitcoin/pull/15450#pullrequestreview-282297760) in that issue predates #15450 and is fixed by #16826.
- [x] rename encrypt() to encryptWallet(), and blank() to makeBlankWallet() // EDIT: updated to
isEncryptWalletChecked()
isDisablePrivateKeysChecked()
isMakeBlankWalletChecked()
- [x] fix naming of askPasshprase() to askPassphrase()
- [x] fix passphrase labels and tooltip in createwalletdialog.ui and askpassphrasedialog.ui
- [x] fix grammar of labels in askpassphrase dialog and WalletController::closeWallet
- [x] fix autofocus in CreateWalletActivity::askPassphrase()
Squashed down to three commits.
Reviewers, to test manually: build, launch the gui wallet, and look at labels/tooltips/focus with the create wallet, encrypt wallet, change password, and close wallet commands.
ACKs for top commit:
jb55:
Approach ACK cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c
instagibbs:
code review and tACK cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c
fanquake:
ACK cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c
Tree-SHA512: b441fbf8f8cd370dd692bac24f0d3c1b32fc7d947b6c3a2c9ba7cf0bc175a72b3460440f2f10f7632c0e8e0f8e65fe15615a30c46e2c7763bf258c504b457dd6
fa61365a1368f9970fa22fb96f89f4ecc08e69f0 wallet: Translate all initErrors in CreateWalletFromFile (MarcoFalke)
fa70d199d0c2182d76b2a1cfa21f9ada4bb12913 util: Make util/error bilingual_str (refactor) (MarcoFalke)
Pull request description:
The translations are going to close in three days (#15940), so I am submitting this as a standalone pull request.
Those changes are part of a bugfix #16661, which includes a test. The first change (the refactor) is required, the second commit is not. I am happy to drop it, if needed.
ACKs for top commit:
laanwj:
utACK fa61365a1368f9970fa22fb96f89f4ecc08e69f0
hebasto:
ACK fa61365a1368f9970fa22fb96f89f4ecc08e69f0, I have tested the code on Linux Mint 19.2.
Tree-SHA512: a7616cc38b9ffd301c6b915ea808a65815c3d97e9f57ec091772eb260e5cf0d75a13a6e4dfa3913e236833677c7929b9a748cb7d7a0e406d51749944b614e11b
d0ea9bab2804928c9f40def61fd99064d2d8f9b8 walletdb: Don't remove database transaction logs and instead error (Andrew Chow)
Pull request description:
Instead of removing the database transaction logs and retrying the
wallet loading, just return an error message to the user. Additionally,
speciically for DB_RUNRECOVERY, notify the user that this could be due
to different BDB versions.
Kind of implements the suggestion from https://github.com/bitcoin/bitcoin/pull/18870#discussion_r421647964
ACKs for top commit:
Sjors:
re-utACK d0ea9bab2804928c9f40def61fd99064d2d8f9b8
ryanofsky:
Code review ACK d0ea9bab2804928c9f40def61fd99064d2d8f9b8. Only changes since last review are rebase and expanding error and commit messages.
Tree-SHA512: f6e67dc70f58188742a5c8af7cdc63a2b58779aa0d26ae7f1e75805a239f1a342433860e5a238d6577fae5ab04b9d15e7f11c55b867065dfd13781a6a62e4958
18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad util: Cleanup translation.h (Hennadii Stepanov)
e95e658b8ec6e02229691a1941d688e96d4df6af doc: Do not translate technical or extremely rare errors (Hennadii Stepanov)
7e923d47ba9891856b86bc9f718cf2f1f773bdf6 Make InitError bilingual (Hennadii Stepanov)
917ca93553917251e0fd59717a347c63cdfd8a14 Make ThreadSafe{MessageBox|Question} bilingual (Hennadii Stepanov)
23b9fa2e5ec0425980301d2eebad81e660a5ea39 gui: Add detailed text to BitcoinGUI::message (Hennadii Stepanov)
Pull request description:
This is an alternative to #15340 (it works with the `Chain` interface; see: https://github.com/bitcoin/bitcoin/pull/15340#issuecomment-502674004).
Refs:
- #16218 (partial fix)
- https://github.com/bitcoin/bitcoin/pull/15894#issuecomment-487947077
This PR:
- makes GUI error messages bilingual: user's native language + untranslated (i.e. English)
- insures that only untranslated messages are written to the debug log file and to `stderr` (that is not the case on master).
If a translated string is unavailable only an English string appears to a user.
Here are some **examples** (updated):
* `qt5ct: using qt5ct plugin` message is my local environment specific; please ignore it.
---
Note for reviewers: `InitWarning()` is out of this PR scope.
ACKs for top commit:
Sjors:
re-tACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad
MarcoFalke:
ACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad 🐦
Tree-SHA512: 3cc8ec44f84403e54b57d11714c86b0855ed90eb794b5472e432005073354b9e3f7b4e8e7bf347a4c21be47299dbc7170f2d0c4b80e308205ff09596e55a4f96
# Conflicts:
# src/dashd.cpp
# src/httpserver.cpp
# src/index/base.cpp
# src/init.cpp
# src/interfaces/chain.cpp
# src/interfaces/chain.h
# src/interfaces/node.cpp
# src/net.h
# src/qt/bitcoingui.cpp
# src/ui_interface.h
# src/wallet/init.cpp
# src/wallet/load.cpp
2fa8dc545417ab8ff54676ff22a93674d1a54797 src/init: correct a typo (darosior)
Pull request description:
Just a little typo I noticed while reading the manpage.
ACKs for top commit:
emilengler:
ACK 2fa8dc5
practicalswift:
ACK 2fa8dc545417ab8ff54676ff22a93674d1a54797 -- also small defects should be fixed
laanwj:
ACK 2fa8dc545417ab8ff54676ff22a93674d1a54797
kristapsk:
utACK 2fa8dc545417ab8ff54676ff22a93674d1a54797
Tree-SHA512: 3f25793313348f3584cd56894cc54a88bc3883c19e8e690b9624e1ab66eb2497ce270668684422c01ada2ec72f8349ec19106590739293e54fb458dbe9c561f3
15ac916642f20918f66e32729bb6b0b674e3bc24 doc: Doxygen-friendly descriptor.h comments (Jon Layton)
Pull request description:
Closes#16942.
- Make `Descriptor` overview subtext of `Interface for parsed descriptor objects.`
- Conform to `@param[in, out] argname: Info` in parameter comments. Present in code: feb162d500/src/net_processing.cpp (L1001)
- Remove redundant argument type, `in` vs `out` mentions
- Removed unnecessary backticks around `IsSolvable()`, since Doxygen builds a link to the known function's docs
- Add backticks to refer to `argname`s
`descriptor.cpp` has more documentation, but Doxygen's output doesn't include anything inside unnamed namespaces for some reason. Tried to access them via searchbar.
Top commit has no ACKs.
Tree-SHA512: 587cc7596de46358a08b0321a7cf08a08785945715dbdce8945d837e1bee0664d1e11b1e47b7be85c4f35262f7ea173fb1f6202efcacc2023e2c6b0bd44133b3
d314e8a818d4c162b1c7201533e6b600dcab2d91 refactor: Replace all uses of boost::optional with our own Optional type (Wladimir J. van der Laan)
Pull request description:
Replace all uses of boost::optional with our own Optional type. Luckily, there aren't so many.
After this:
- `boost::optional` is no longer used directly (only through `Optional` which is an alias for it)
- `boost/optional.hpp` is only included in one place
ACKs for top commit:
MarcoFalke:
ACK d314e8a818d4c162b1c7201533e6b600dcab2d91
practicalswift:
ACK d314e8a818d4c162b1c7201533e6b600dcab2d91 -- diff looks correct + satisfying to see incremental progress towards the goal of a Boost free future :)
jtimon:
ACK d314e8a818d4c162b1c7201533e6b600dcab2d91
fanquake:
ACK d314e8a818d4c162b1c7201533e6b600dcab2d91
Tree-SHA512: b43e0017af81b07b5851377cd09624f114510ac5b9018d037664b58ad0fc8e893e30946b61f8f5e21e39125925bf9998a81f2226b468aab2df653ee57ed3213d
fa1aa6c571f406a2c40282664487aca4aff9dc9d fuzz: Limit ParseISO8601DateTime fuzzing to 32-bit (MarcoFalke)
Pull request description:
2038 is more than 10 years in the future, so no need for us to waste time fuzzing a 3rd party lib that will be EOL by then.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34092
ACKs for top commit:
theStack:
Concept and code review ACK fa1aa6c571f406a2c40282664487aca4aff9dc9d
Tree-SHA512: fdd2fbc7b5c7ce33ad23b2e5431bb97eaf6ae8c2d2a55990a3ab73be79282c584b704dcd1471ba288de75283732970c70c9a03ddad059b97b66ba8b3de39effe
bd9c6ade46c8c29bea3fd5df62a35efadb77ac89 wallet: Fixed Grammatical error in bdb.h (zealsham)
Pull request description:
A comment in bdb.h file in the wallet directory contains a grammatical error that makes the underlying code harder to reason about .
The comment which says `/** Indicate the a new database user has began using the database. */` should actually be
`/** indicate that a new database user has began using the database. */`. The former is quite confusing , and leaves you wondering what "the a new database user " is refering to . This pull request thus provides value to the bitcoin codebase by improving readability .
Top commit has no ACKs.
Tree-SHA512: db07cda39f89ac344be3497c884a8c6e4b48276afae18e931a9a8e5732c58eed20645ccd18d6b1212c763f64b1300477c1de26a00b5b3b24e6141ffae3658ca7
077a875d94b51e3c87381133657be98989c8643e refactor: include a missing <limits> header in fs.cpp (Joan Karadimov)
Pull request description:
I get this compilation error on versions `0.21.1` and `22.0`:
```
fs.cpp: In member function 'bool fsbridge::FileLock::TryLock()':
fs.cpp:123:89: error: 'numeric_limits' is not a member of 'std'
123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
| ^~~~~~~~~~~~~~
fs.cpp:123:109: error: expected primary-expression before '>' token
123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
| ^
fs.cpp:123:112: error: '::max' has not been declared; did you mean 'std::max'?
123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
| ^~~
| std::max
In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39,
from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40,
from ./fs.h:9,
from fs.cpp:5:
C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here
300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp)
| ^~~
fs.cpp:123:124: error: 'numeric_limits' is not a member of 'std'
123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
| ^~~~~~~~~~~~~~
fs.cpp:123:144: error: expected primary-expression before '>' token
123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
| ^
fs.cpp:123:147: error: '::max' has not been declared; did you mean 'std::max'?
123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
| ^~~
| std::max
In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39,
from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40,
from ./fs.h:9,
from fs.cpp:5:
C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here
300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp)
| ^~~
```
It appears that `std::numeric_limits<T>::max` is used without the `limits` header being included. Probably on other STL implementations it's included transitively, but not in the one in MinGW. Including it fixes the compilation problem.
Environment:
OS: Windows 10
Compiler: gcc 11.2.0
Qt: 5.15.2 (included in msys2)
Using the latest mingw w64 shipped with msys2.
ACKs for top commit:
fanquake:
ACK 077a875d94b51e3c87381133657be98989c8643e - Thanks.
hebasto:
ACK 077a875d94b51e3c87381133657be98989c8643e
Tree-SHA512: 2289cb72fa3a28470f4250833be66079482d1392189b1e4679330dad109a8ae67b1d6d51cb635dbc1947c00c6c4cfbc4d7c9ae02269b932cfa1475583adaf73d
17ae2601c786e6863cee1bd62297d79521219295 build: remove build stubs for external leveldb (Cory Fields)
Pull request description:
Presumably these stubs indicate to packagers that external leveldb is meant to be supported in some way. It is not. Remove the stubs to avoid sending any mixed messages.
For context, this was reported on IRC:
> \<Talkless> bitcoind fails to start with undefined symbol: _ZTIN7leveldb6LoggerE in Debian Sid after leveldb upgraded from 1.22 to 1.23: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996486
ACKs for top commit:
fanquake:
ACK 17ae2601c786e6863cee1bd62297d79521219295
hebasto:
ACK 17ae2601c786e6863cee1bd62297d79521219295. I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 2f1ac2cb30dac64791933a245a2b66ce237bde3955e6f4a6b7ec181248f77a9b1b10597d865d3e2c2b6def696af70de40e905ec274e4ae7cccd1daf461473957
e3d2ba7c70b13a2165020e45abf02373a1e953f7 fuzz: Update FuzzedDataProvider.h from upstream (LLVM) (practicalswift)
Pull request description:
Update `FuzzedDataProvider.h` from upstream (LLVM).
Upstream revision: 6d0488f75b/compiler-rt/include/fuzzer/FuzzedDataProvider.h
Changes since last update:
* [[compiler-rt] FuzzedDataProvider: add ConsumeData and method.](20a604d3f5)
* [[compiler-rt] Fix a typo in a comment in FuzzedDataProvider.h.](5517d3b80b)
* [[compiler-rt] Add ConsumeRandomLengthString() version without arguments.](2136d17d8d)
* [[compiler-rt] Refactor FuzzedDataProvider for better readability.](1262db1b6a)
* [[compiler-rt] FuzzedDataProvider: make linter happy.](1e65209e04)
* [[compiler-rt] Mark FDP non-template methods inline to avoid ODR violations.](6d0488f75b)
ACKs for top commit:
MarcoFalke:
ACK e3d2ba7c70b13a2165020e45abf02373a1e953f7 🌛
Tree-SHA512: 62cb27906f08fd07983f4a8fbbd381c12ed185617a58f1ebc8564c87e638086f952417f4f6481fbd91b9a313aff00e944215393734566c219c074512991f8057
fae7a1c18803675e70b9bf66575e1e0a6e01f6f6 fuzz: Configure check for main function (MarcoFalke)
Pull request description:
Instead of the PP jungle, use a proper configure check
Fixes https://github.com/google/honggfuzz/issues/336#issuecomment-702972138
ACKs for top commit:
practicalswift:
ACK fae7a1c18803675e70b9bf66575e1e0a6e01f6f6
Tree-SHA512: 2e55457d01f9ac598bb1e119d8b49dca55a28f88ec164cee6b5f071c29e9791f5a46cc8ee2b801b3a3faf906348da964ce32e7254da981c1104b9210a3508100
fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 doc: Mention Span in developer-notes.md (Pieter Wuille)
3502a60418858a8281ddf2f9cd59daa8f01d2fa8 doc: Document Span pitfalls (Pieter Wuille)
Pull request description:
This is an attempt to document pitfalls with the use of `Span`, following up on comments like https://github.com/bitcoin/bitcoin/pull/18468#issuecomment-622846597 and https://github.com/bitcoin/bitcoin/pull/18468#discussion_r442998211
ACKs for top commit:
laanwj:
ACK fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88
Tree-SHA512: 8f6f277d6d88921852334853c2b7ced97e83d3222ce40c9fe12dfef508945f26269b90ae091439ebffddf03f939797cb28126b2387f77959069ef8909c25ab53
1087807b2bc56b9c7e7a5471c83f6ecfae79b048 tests: Provide main(...) function in fuzzer (practicalswift)
Pull request description:
Provide `main(...)` function in fuzzer. Allow building uninstrumented harnesses with only `--enable-fuzz`.
This PR restores the behaviour to how things worked prior to #18008. #18008 worked around an macOS specific issue but did it in a way which unnecessarily affected platforms not in need of the workaround :)
Before this patch:
```
# Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation)
$ ./configure --enable-fuzz
$ make
CXXLD test/fuzz/span
/usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/Scrt1.o: In function `_start':
(.text+0x20): undefined reference to `main'
collect2: error: ld returned 1 exit status
Makefile:7244: recipe for target 'test/fuzz/span' failed
make[2]: *** [test/fuzz/span] Error 1
make[2]: *** Waiting for unfinished jobs....
$
```
After this patch:
```
# Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation)
$ ./configure --enable-fuzz
$ make
$ echo foo | src/test/fuzz/span
$
```
The examples above show the change in non-macOS functionality. macOS functionality is unaffected by this patch.
ACKs for top commit:
MarcoFalke:
ACK 1087807b2bc56b9c7e7a5471c83f6ecfae79b048
Tree-SHA512: 9c16ea32ffd378057c4fae9d9124636d11e3769374d340f68a1b761b9e3e3b8a33579e60425293c96b8911405d8b96ac3ed378e669ea4c47836af06892aca73d
f32ab443a98e622afa601372454310aef1f380be Bugfix: RPC: JSON null is not "None" (Luke Dashjr)
26dcf3958187cbdc9ffc9438a5eebfcaf607f7e9 Bugfix: RPC: Don't use a continuation elipsis after an elision elipsis (Luke Dashjr)
eca65caadcddf43b2dace111bd7e4b0a2a9556c2 Bugfix: RPC: Add missing commas and correct indentation of explicit ELISION (Luke Dashjr)
Pull request description:
1. listsinceblock had a double ellipsis (elision + continuation); this looks ugly, just one is needed.
2. Elision ellipsis wasn't getting a comma, so was truncated to `".."` by comma-removal code.
3. Elision ellipsis was indented incorrectly (as if it was a subitem).
4. Similarly, type "none" would get truncated to `"Non"`, when it should really be `"null"` anyway.
ACKs for top commit:
MarcoFalke:
ACK f32ab443a98e622afa601372454310aef1f380be 🐰
Tree-SHA512: 34e1c72673790ed11cdee838d64ea5e0ac498de19258df99d54b5322e003060123c65ad27ac2fd4729a1dfe52066a0629602a132b1ef85d4154affd99a065a3f
470e2ac602ed2d6e62e5c80f27cd0a60c7cf6bce tests: Avoid hitting some known minor tinyformat issues when fuzzing strprintf(...) (practicalswift)
Pull request description:
Avoid hitting some known minor tinyformat issues when fuzzing `strprintf(...)`. These can be removed when the issues have been resolved upstreams :)
Note to reviewers: The `%c` and `%*` issues are also present for `%<some junk>c` and `%<some junk>*`. That is why simply matching on `"%c"` or `"%*"` is not enough. Note that the intentionally trivial skipping logic overshoots somewhat (`c[…]%` is filtered in addition to `%[…]c`).
Top commit has no ACKs.
Tree-SHA512: 2b002981e8b3f2ee021c3013f1260654ac7e158699313849c9e9660462bb8cd521544935799bb8daa74925959dc04d63440e647495e0b008cfe1b8a8b2202d40
b35567fe0ba3e6f8d8f9525088eb8ee62065ad01 test: only declare a main() when fuzzing with AFL (fanquake)
Pull request description:
This fixes fuzzing using [libFuzzer](https://llvm.org/docs/LibFuzzer.html) on macOS, which caused a few issues during the recent review club. macOS users could only fuzz using afl, or inside a VM.
It seems that the `__attribute__((weak))` marking is not quite enough to properly mark `main()` as weak on macOS. See Apples docs on [Frameworks and Weak Linking](https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html#//apple_ref/doc/uid/20002378-107262-CJBJAEID).
Have tested fuzzing using libFuzzer and AFL with this patch.
ACKs for top commit:
MarcoFalke:
ACK b35567fe0ba3e6f8d8f9525088eb8ee62065ad01
fjahr:
ACK b35567f
Tree-SHA512: b881fdd98c7e1587fcf44debd31f5e7a52df938059ab91c41d0785077b3329b793e051a2bf2eee64488b9f6029d9288c911052ec23ab3ab8c0561a2be1682dae
e161bc74d24a381c313aecb950d3b8411e0ed19d 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 e161bc74d24a381c313aecb950d3b8411e0ed19d -- rationale makes sense and diff looks correct :)
MarcoFalke:
Tested ACK e161bc74d24a381c313aecb950d3b8411e0ed19d 🔮
Tree-SHA512: d38754903252896dc86fac6c12ad6615d322c2744db7c02b18574a08c69e8876b2c905e1f09b324002236b111ee93479f89769c562e7b3b2e6eb2992d76464ef
e61de6306fd89fe9aae90253062e7b1b20343f8a Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e20007bf5face34d33dffa26c8db67e29ec Move ismine to wallet module (Andrew Chow)
Pull request description:
`IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.
This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.
ACKs for commit e61de6:
MarcoFalke:
re-ACK e61de6306f (only change is rebase with git auto-merge)
meshcollider:
Very light code review ACK e61de6306f
Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
f59bbb61af5055844c16939a4f8e58c4f73843c1 test: Fix bug in blockfilter_index_tests. (Jim Posen)
Pull request description:
The test case tests a chain reorganization, however the two chains were generated in the same manner and thus produced the same blocks.
This issue was [pointed out](https://github.com/bitcoin/bitcoin/pull/14121#discussion_r334282663) by MarcoFalke.
ACKs for top commit:
MarcoFalke:
Thanks! ACK f59bbb61af5055844c16939a4f8e58c4f73843c1 (looked at the diff on GitHub, didn't compile, nor run tests)
Tree-SHA512: a2f063ae9312051ffc2a3fcc1116a6a8ac09beeef261bc40aa3ff7270ff4de22a790eb19fec6b15ba1eb46e78f1f317bfd91472d8581b95bb9441a56b102554e
7777703958937ec0ae609b1ee882f1bf2d113d10 doc: Explain new test logging (MarcoFalke)
Pull request description:
Explain logging added in #18472 and #16975
ACKs for top commit:
jonatack:
ACK 7777703
Tree-SHA512: 3a0aa7bab32a6753d8894d29cf82604b044b23e512102dd275b717eefda3c2212dbf43ea7e9155267350dd9f3bc5badba2eb660152db3efeab30a04f52126c95
fa37e0a68bea65979f9f8f2e5258fe608acf2bdf test: Show debug log on unit test failure (MarcoFalke)
Pull request description:
Often, it is hard to debug unit test failures without the debug log. Especially when the failure happens remotely (e.g. on a ci system).
Fix that by printing the log on failure.
ACKs for top commit:
jamesob:
ACK fa37e0a68bea65979f9f8f2e5258fe608acf2bdf ([`jamesob/ackr/16975.1.MarcoFalke.test_show_debug_log_on_u`](https://github.com/jamesob/bitcoin/tree/ackr/16975.1.MarcoFalke.test_show_debug_log_on_u))
Tree-SHA512: 2ca4150c4ae3d4ad47e03b5e5e70da2baffec928ddef1fdf53a3ebc061f14aee249205387cb1b12ef6d4eb55711ef0080c0b41d9d18000b5da124ca80299793b
9743432034586385cfef87df4b377c255ed0cba8 Fix bug where duplicate PSBT keys are accepted (John L. Jegutanis)
Pull request description:
As per the BIP 174 spec a PSBT key cannot be duplicated,
however the current code accepts key duplication.
The PSBT key/value entries can be duplicated when the value
is `empty()` or `IsNull()` for `CScript` or `CTxOut` respectively
and if those key/value entries are serialized before the non-empty ones.
For example, the following PSBT, included in the test vectors,
contains a duplicate field:
```
// magic
70736274ff
// global tx
//// key
0100
//// value
2a02000000000140420f000000000017a9146e91b72d5593e7d4391e2ff44e91e985c31641f08700000000
//// separator
00
// no inputs
// outputs
//// key PSBT_OUT_WITNESSSCRIPT
0101
//// value (empty script)
00
//// key PSBT_OUT_WITNESSSCRIPT (same as the above)
0101
//// value (an OP_RETURN script)
016a
//// separator
00
```
ACKs for top commit:
achow101:
ACK 9743432034586385cfef87df4b377c255ed0cba8
instagibbs:
code review ACK 9743432034
Tree-SHA512: 34f4b34c8e6561c6a6ab745cdd319f6687eac6f7cecc735c94035eeca8c5157e17a27f2ae853dbaa6634fcd5a8f4e1c6cc13d1ebd7e563459665d72bb147cc1e
621e86ee8d0102e2bf41f7656a368083b89b2f83 Update -blocksonly documentation (glowang)
Pull request description:
When -blocksonly is set to 1, it interacts with the -walletbroadcast
parameter and sets it to 0.
This behavior is not captured by the current documentation, which
claims that -blocksonly does not impact any wallet transactions at
all.
Fixes#17294
ACKs for top commit:
MarcoFalke:
ACK 621e86ee8d0102e2bf41f7656a368083b89b2f83
Tree-SHA512: f47bfb40a196c23e62505e1d4f79094011ac7c21fc9b920fad60cdadb5c4f48e993be1f015e26e568ce329967c24848fd7b665a6cffd3881f4cfcd2fd0081ed8
50c4afa3c420f11329cffb091b62beeb96b39183 add newline after -stdin* (Karl-Johan Alm)
7f11fba2e3c7b1e00ffb98504cdcfc2ded9c09cf cli: add -stdinwalletpassphrase for (slightly more) secure CLI (Karl-Johan Alm)
0da503e9475fea5644168544668581796bf66334 add stdin helpers for password input support (Karl-Johan Alm)
Pull request description:
This PR
* adds `-stdinwalletpassphrase` for use with `walletpasshprase(change)`
* adds no-echo for passwords (`-stdinrpcpass` and above)
It may not be ideal, but it's better than having to clear the screen whenever you unlock the wallet.
ACKs for top commit:
laanwj:
code review ACK 50c4afa3c420f11329cffb091b62beeb96b39183
Tree-SHA512: 473db8a303ff360ffaa36ac81a2f82be2136fa82696df0bc4f33cb44033a3ae258b5aa5bbcc1f101f88ae9abe9598ed564ce52877ab139bd5d709833f5275ec6
fac52253f81a4f0d04b25eb4b6c28d04b85ef10b rpc: Document an RPCResult for all calls; Enforce at compile time (MarcoFalke)
fadd99f61032c0b75ad9b198eb5d8e307b0518ee rpc: Add missing newline in RPCResult description (MarcoFalke)
Pull request description:
This documents the RPC Result (type and description, if applicable) everywhere it was missing. The patch can be reviewed with the `git diff` option `-W`/`--function-context`.
Also, code won't compile without having an RPCResult documented.
ACKs for top commit:
laanwj:
Lightly tested ACK fac52253f81a4f0d04b25eb4b6c28d04b85ef10b
promag:
Tested ACK fac52253f81a4f0d04b25eb4b6c28d04b85ef10b, built and verified listunspent help output.
Tree-SHA512: af2c1af1432beb944993776026c320814bfaecaf202f47359f5758849096ca7051ec6560395a2cc6678dcc111e7c9cf4917d0f0b221bdcf3ed1642e14d0e5b3c
9e95931865186d7a9a6dc54b64bd96507e9fea4b [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery)
d1734f9a3b138ab046f38ee44a09bc3847bf938a [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a02b463ffeaf82ec11fc6f74f439c037ae [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b22d22a8dfea7c947b542b1022bfc1c0ea [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)
Pull request description:
`CommitTransaction()` returns a bool to indicate success, but since commit
b3a7410 (#9302) it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.
All dead code in `if (!CommitTransaction())` branches has been removed.
Two additional commits fix up the idiosyncratic whitespace in `CommitTransaction` and add a doxygen comment for the function.
ACKs for top commit:
laanwj:
ACK 9e95931865186d7a9a6dc54b64bd96507e9fea4b
Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
fac86ac7b3ceac2f884412c7a9f4bd5bab5e3916 scripted-diff: Add missed copyright headers (Hennadii Stepanov)
6fde9d5e47fc9a1042b3fb68031eab5bf55e508d script: Update EXLUDE list in copyright_header.py (Hennadii Stepanov)
1998152f15fd2b0e83f5068c375a34feaf73db8c script: Add empty line after C++ copyright (Hennadii Stepanov)
071f2fc204f542c5a287ca8835115a2ee0bf2f50 script: Add ability to insert copyright to *.sh (Hennadii Stepanov)
Pull request description:
This PR improves `contrib/devtools/copyright_header.py` script and adds copyright headers to the files in `src` and `test` directories with two exceptions:
- [`src/reverse_iterator.h`](https://github.com/bitcoin/bitcoin/blob/master/src/reverse_iterator.h) (added to exceptions)
- [`src/test/fuzz/FuzzedDataProvider.h`](https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/FuzzedDataProvider.h) (added to exceptions)
On master 5622d8f3156a293e61d0964c33d4b21d8c9fd5e0:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
25 with zero copyrights
```
With this PR:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
2 with zero copyrights
```
~I am uncertain about our copyright policy with `build_msvc` and `contrib` directories content, so they are out of scope of this PR.~
ACKs for top commit:
MarcoFalke:
ACK fac86ac7b3ceac2f884412c7a9f4bd5bab5e3916
Tree-SHA512: d7832c4a7a1a3b7806119775b40ec35d7982f49ff0e6199b8cee4c0e0a36e68d51728b6ee9924b1c161df4bc6105bd93391b79d42914357fa522f499cb113fa8
85016e52f6adc01735beefe5a8aefcc4f0c4aa25 [rpc] Fix broken bitcoin-cli examples (Andrew Toth)
Pull request description:
This fixes the `bitcoin-cli` examples for `combinerawtransaction`, `combinepsbt` and `testmempoolaccept`. They currently return `Error parsing JSON`.
ACKs for top commit:
laanwj:
ACK 85016e52f6adc01735beefe5a8aefcc4f0c4aa25
Tree-SHA512: b561f68f7a188dc91dab1ceb98da3ac3e232143ab2b906c90f95c6b74b584599d0f3b51f067cdd3b1153931f95b3dc385e453b1a0dde86f9cb549b94560f219d
* Add function to validate DSTX message
* Addressed review comment: Remove text left due to merge
* Apply suggestions from code review
* Update src/net_processing.cpp
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
8a6810d0d2759c69f63b53c48aa79e0cfdd88ffb Add a 'logpath' field to getrpcinfo (darosior)
Pull request description:
as discussed in #15438
ACKs for top commit:
laanwj:
Tested ACK 8a6810d0d2759c69f63b53c48aa79e0cfdd88ffb
Tree-SHA512: 752c7d90f670677c8144efb338c5c97c2264f85f1e65e031fd5a44f04230b6eafbabd0f634db263eb42c25642ecc1c4b1b602d4735e3fab07ec00b566134ddab
119d360aab travis: Document whether functional tests are run in the job name (Ben Woosley)
64f28545e3 Revert "travis: Compile trusty with depends for now" (Ben Woosley)
267eac00f9 Prefer boost::optional#get_value_or over #value_or (Ben Woosley)
1971f5ba04 Piecewise construct to avoid invalid construction (Ben Woosley)
Pull request description:
In light of #14979, I realized that only qt 5.5+ was being tested under CI, while compatibility lists 5.2+.
In #15276, Marco added Trusty to CI, building with depends. This changes that build to system libraries, in order to ensure ongoing compatibility with our claimed minimum required versions.
Fixes#14983, previously open as #14998
Tree-SHA512: 6cff5e28c756ecb8bf797c8f6eb77c1944ba61a8dd6d7d4984e63eef384f6429dc79c505da3241c05b9c4db31c72b2a9846c7365aba9280f2e0620e5f3998d07
* refactor: optimizations for processing specialtransaction, specifcally IDB / reindex
GetValidMNsCount -> GetAllMNsCount for reservation as GetValidMNsCount is a slow operation, while GetAllMNsCount is simply .size
Reduce some locking
Pass const Consensus::Params& to CleanupCache. I saw in benchmarks that a substancial
all CheckProRegTx ish methods should take check_sigs
introduce new CDeterministicMNManager thread for handling cleanup
* use main scheduler
* Update src/evo/deterministicmns.cpp
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
aaa1d03d3acebeb44fdd40a302f086aad3d329ce Add optimized sha256d64_arm_shani::Transform_2way (Pieter Wuille)
fe0629852aaf3a26f291bfa535e7e455fe7bea06 Implement sha256_arm_shani::Transform (Pavol Rusnak)
48a72fa81f80c8a3c7c6de8339b5feb361dece1c Add sha256_arm_shani to build system (Pavol Rusnak)
c2b79342506e24e9b7100fb7a6025dc870375ef6 Rename SHANI to X86_SHANI to allow future implementation of ARM_SHANI (Pavol Rusnak)
Pull request description:
This PR adds support for ARMv8 SHA2 Intrinsics.
Fixes https://github.com/bitcoin/bitcoin/issues/13401 and https://github.com/bitcoin/bitcoin/issues/17414
* Integration part was done by me.
* The original SHA2 NI code comes from https://github.com/noloader/SHA-Intrinsics/blob/master/sha256-arm.c
* Minor optimizations from https://github.com/rollmeister/bitcoin-armv8/blob/master/src/crypto/sha256.cpp are applied too.
* The 2-way transform added by @sipa
ACKs for top commit:
laanwj:
Code review and lightly tested ACK aaa1d03d3acebeb44fdd40a302f086aad3d329ce
Tree-SHA512: 9689d6390c004269cb1ee79ed05430d7d35a6efef2554a2b6732f7258a11e7e959b3306c04b4e8637a9623fb4c12d1c1b3592da0ff0dc6d737932db302509669
# Conflicts:
# configure.ac
# src/Makefile.am
# src/crypto/sha256.cpp
* Merge #16291: gui: Stop translating PACKAGE_NAME
fa64b947bb3075ff8737656706b941af691908ab util: No translation of `Bitcoin Core` in the copyright (MarcoFalke)
fab85208f678ba1be53bdb73a73ce3c5c937d448 qt: Run «make translate» in ./src/ (MarcoFalke)
fabe87d2c923ab3a70b8cde2666a4d1cda8b22fb scripted-diff: Avoid passing PACKAGE_NAME for translation (MarcoFalke)
fa5e9f157e568b7fbbea1482b393181f0733f2ba build: Stop translating PACKAGE_NAME (MarcoFalke)
Pull request description:
Generally the package name is not translated, but the package description is.
E.g. `GIMP` or `Firefox` are always called that way regardless of the system language. However, "`Firefox` webbrowser" or "`GIMP` image manipulation program" are translated.
ACKs for top commit:
hebasto:
ACK fa64b947bb3075ff8737656706b941af691908ab, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
Tree-SHA512: 626f811531182d0ba0ef1044930d32726773349bcb49b10261288a86ee6b80a183db30a87d817d5b0d501fad058ac22d6272311716b4f5a154f17c6f391a5a1a
* more
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
7fc487afd1249eee97651a0eec0630d7e421924f refactor: use `{Read,Write}BE32` helpers for BIP32 nChild (de)serialization (Sebastian Falbesoner)
Pull request description:
This small refactoring PR replaces manual bit-fiddling (de)serialization of the BIP32 child number (nChild) by the helpers `ReadBE32`/`WriteBE32`. Note that those were first introduced in #4100, almost one year _after_ the BIP32 derivation implementation has been merged (#2829, eb2c9990).
ACKs for top commit:
sipa:
utACK 7fc487afd1249eee97651a0eec0630d7e421924f
laanwj:
Code review ACK 7fc487afd1249eee97651a0eec0630d7e421924f
Tree-SHA512: bbe3e411fb0429fa74c8a5705a91f4d6ed704dac9d6623ecb633563f22acf8e21f3189a16f1d0cf1aeedfc56a5b695df54ae51e9577e34eb6d7dc335de2da6de
1d44513f9b34f2a59c794474752025430285a68b Squashed 'src/crc32c/' changes from b5ef9be675..0d624261ef (MarcoFalke)
Pull request description:
Only change is a warning fix for arm.
```
CXX crc32c/src/crc32c_libcrc32c_a-crc32c.o
In file included from crc32c/src/crc32c.cc:11:0:
crc32c/src/./crc32c_arm64_check.h: In function ‘bool crc32c::CanUseArm64Crc32()’:
crc32c/src/./crc32c_arm64_check.h:43:37: warning: the address of ‘long unsigned int getauxval(long unsigned int)’ will never be NULL [-Waddress]
unsigned long hwcap = (&getauxval != nullptr) ? getauxval(AT_HWCAP) : 0;
~~~~~~~~~~~^~~~~~~~~~
ACKs for top commit:
laanwj:
Code review ACK fac1c13ead8da2414acdd3202040f00a21435b8f
fanquake:
ACK fac1c13ead8da2414acdd3202040f00a21435b8f
Tree-SHA512: 22a52caf67dd89092eff1f075fbf5c5d16bdca9146ba042ce5d3fcc10ce1485e950964089f8536c938ebe650676e03a789d3597fe45b19920fd2c5e72f1391ad
330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad refactor: net: avoid duplicate map lookups to `mapLocalHost` (Sebastian Falbesoner)
Pull request description:
This simple refactoring PR aims to avoid duplicate lookups to `mapLocalHost`: instead of calling `count()` (to first find out whether a key is in the map) and then `operator[]` (to get the value to the passed key, or default-construct one if not found), use either
* `find()` and dereference the returned iterator (for simple lookups), see https://www.cplusplus.com/reference/map/map/find/
* `emplace()` and use the returned <iterator, inserted> pair (for lookups where a new element should be inserted if the key isn't found), see https://www.cplusplus.com/reference/map/map/emplace/
ACKs for top commit:
naumenkogs:
ACK 330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad
jonatack:
Code review ACK 330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad plus rebase to master + debug build
Tree-SHA512: d13da6a927ff561eee8ac6b093bf3586dfe31d6c94173a5a6d8f3698e0ee224fb394d3635155d5141c165da59d2c2c37260122eb4f2e8bcda3e8a29b901d213e
5008dd87b2c9a8eeee21f2a11367a7ada9c324ed doc: Remove stale comment for CPrivKey (Calvin Kim)
Pull request description:
Removes stale doc about `secure_allocator` being defined in `allocators.h`.
ACKs for top commit:
laanwj:
ACK 5008dd87b2c9a8eeee21f2a11367a7ada9c324ed
theStack:
Code-review ACK 5008dd87b2c9a8eeee21f2a11367a7ada9c324ed
Tree-SHA512: eb65aff6db5b27d0db2b86f1d1dc6e066daccdaf00f7f9f95b5bee507167fcea2601316cdbd70da4ba32f1fab1e28e440a7e3cabd7b1a72c07dd20b1367361f0
fdd71448e78f442ffd93a3a3398a5062eaba9f1b system: skip trying to set the locale on NetBSD (fanquake)
Pull request description:
Just treat it the same as the other BSDs.
Fixes#17379.
ACKs for top commit:
laanwj:
Code review ACK fdd71448e78f442ffd93a3a3398a5062eaba9f1b
practicalswift:
cr ACK fdd71448e78f442ffd93a3a3398a5062eaba9f1b
Tree-SHA512: 5fe0a66f014279ad2683b548692a36af493377fb92d1f28b15dc4feef871190fe08ef40dcc4f5ba21a525fe365c42fb429fe4be0673a1e96db163af587c23204
69a439b8807cb150068d196a0e2fd57fd80057df doc: add missing copyright header to getuniquepath.cpp (fanquake)
Pull request description:
This was missed in #21052.
ACKs for top commit:
hebasto:
ACK 69a439b8807cb150068d196a0e2fd57fd80057df, but a scripted-diff using `copyright_header.py insert` looks preferable.
Tree-SHA512: 1a75ffd93fa8e5e83821e1fe984353422740ebf9e203dc873debf4dffec0e23f55a1e31f806dd2d66087d3620f6dc447af69f9124f0bcc6676ef91ee35374832
acb7aad27ec8a184808aa7905887e3b2c5d54e9c Fix build with Boost 1.77.0 (Rafael Sadowski)
Pull request description:
BOOST_FILESYSTEM_C_STR changed to accept the path as an argument.
ACKs for top commit:
hebasto:
ACK acb7aad27ec8a184808aa7905887e3b2c5d54e9c
benthecarman:
ACK acb7aad27ec8a184808aa7905887e3b2c5d54e9c
fanquake:
ACK acb7aad27ec8a184808aa7905887e3b2c5d54e9c - tested the fix with Boost 1.77.0 and 1.71.0.
Tree-SHA512: c25fcb56971ee7a448cfb074f8a13696b32c16c63f81076f8a76911f93aa849c8f3637555b0b4215fa0d8b958641d7e4e60d10e103b833545cbc6b1f4009b526
fae814c9a6c8ce4822f1fc6b88cfbbde7cc2d49c fuzz: Remove incorrect float round-trip serialization test (MarcoFalke)
Pull request description:
It tests the wrong way of the round-trip: `int -> float -> int`, but only `float -> int -> float` is allowed and used. See also `src/test/fuzz/float.cpp`.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34118
ACKs for top commit:
laanwj:
Anyhow, ACK fae814c9a6c8ce4822f1fc6b88cfbbde7cc2d49c
Tree-SHA512: 8412a7985be2225109f382b7c7ea6d6fcfbea15711671fdf2f41dd1a9adbb3b4489592863751d78bedaff98e9b0b13571d9cae06ffd92db8fbf7ce0f47874a41
