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
4105a543817c47bda21739d5ad3269677f84e861 lint: remove boost::bind linter (fanquake)
Pull request description:
I don't think we need to maintain a linter for reintroducing boost::bind at this point.
ACKs for top commit:
hebasto:
ACK 4105a543817c47bda21739d5ad3269677f84e861, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 86bda91ee7ed11f0aa7ac95e9e7b62dbba626dcea75444d2851a3d40e794ab16bef09a1f0c956a716d43602b23c1cf67e1ff3a51184ea1ee7d686fbb76316cb0
58d0393bec7680933702b76cd3f1d1e33030ed16 build: update retry to current version (randymcmillann)
Pull request description:
This commit eliminates spelling and white space
errors that are flagged in the linting process
ACKs for top commit:
practicalswift:
ACK 58d0393bec7680933702b76cd3f1d1e33030ed16
Tree-SHA512: c241ed0775026c890dd29d1f7231c5540e9c9285867a99844605753a3007d08f0bd4f7a59f078e4c65b741301ff7fa8a871e2e3c64b9a9fe47b3ea74c4228498
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
7ba962276e3e540a15eb0bdbbaaa101958229bd4 doc: Minor grammatical changes and flow improvements (Travin Keith)
Pull request description:
**Grammar:**
Line 49: There shouldn't be a period at the end of a phrase.
Lines 56, 57, 116, 137, and 177: Adding necessary commas
Lines 103 and 136: Run-on sentence issues fixed.
Line 176: Fixed punctuation and added necessary conjunction
Line 178: Singular noun when it should be plural
**Flow:**
Line 49: Adding "for" makes it more natural.
Line 54: Though it's not grammatically incorrect to end a sentence with a preposition in an informal document such as this, the word "followed" is much easier to understand anyway, especially for those who don't have English as their native language.
Top commit has no ACKs.
Tree-SHA512: 17fdb5fc6146e025f160e860dbcbdbfa07b7608b8cb611c3b9d4ed91c426100ef772915251bc1f6bacb3a62df57b72c2003fb72cb2c8542454638545985313da
d695eb4c2164fc8777dc014e1a30c014cf04982a doc: note the costs of fetching all pull requests (Vasil Dimov)
Pull request description:
Also mention that it is possible to fetch just one pull request.
ACKs for top commit:
MarcoFalke:
ACK d695eb4c2164fc8777dc014e1a30c014cf04982a
fanquake:
ACK d695eb4c2164fc8777dc014e1a30c014cf04982a
Tree-SHA512: afe080fd018b2e773fb974956937e819085831bf0c1c5623f7f12c728639906b80666b785234058ee39fd98115a53a2fad431c54ee0840667e60bb317e4a828d
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
fa330d8fed5a02349440be170af3b443c1321b4b ci: Avoid invoking curl on the host (MarcoFalke)
Pull request description:
The only requirement for the ci system are the programs `docker.io` and `bash`. However, the mac cross build invokes `curl` on the host. Fix that.
Before:
```
$ FILE_ENV="./ci/test/00_setup_env_mac.sh" ./ci/test_run_all.sh
...
./ci/test/05_before_script.sh: line 22: curl: command not found
```
After:
```
... (command passes)
ACKs for top commit:
promag:
ACK fa330d8fed5a02349440be170af3b443c1321b4b.
Tree-SHA512: 49120fd671a48a6599dd6c34f6d3502a6e9f84b4476061cab06f55cba374d8188f53b9b41363e90f5fafb0074767b581f30bd2545c0b6934580a7eccfa1ef5c4
fac4be30482c21ac330e09ef8756c49e37faa6fa fuzz: Configure check for main function (take 2) (MarcoFalke)
Pull request description:
Actually fix https://github.com/google/honggfuzz/issues/336#issuecomment-702972138
Follow-up to #20065
Steps to test: `honggfuzz` section in doc/fuzzing.md
ACKs for top commit:
practicalswift:
cr ACK fac4be30482c21ac330e09ef8756c49e37faa6fa: patch looks correct!
Tree-SHA512: 893768c80439fe5d90b883ade89dc02f5bb80e27637916cf5575b6a9ed0b1c04942ff851342f5bbabb8666e6696715427feeb98f5301ad23c7b87b09e5872f97
05cdceb19dcc21822fe39a987ce43fe54bdc8634 build: don't set PORT=no in config.site (fanquake)
Pull request description:
This should have been a part of dropping macports support in #15175.
ACKs for top commit:
hebasto:
ACK 05cdceb19dcc21822fe39a987ce43fe54bdc8634, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 278b2f308e55aad5524530e654ac08462714676b71e01d31e5f152f69e02f916afc553510ac8352ed0bb0184f7fdb2ee7038ab5ac2de6923f366829859e0b6ee
655d52a0cd6b3df738dc9843e25a95b8930b4628 contrib: Specify wb mode when creating mac sdk (João Barbosa)
Pull request description:
Fix the warning:
```
./contrib/macdeploy/gen-sdk:84: FutureWarning: GzipFile was opened for writing, but this will change in future Python releases. Specify the mode argument for opening it for writing.
```
ACKs for top commit:
fanquake:
Tested ACK 655d52a0cd6b3df738dc9843e25a95b8930b4628 with Python 3.10.
Tree-SHA512: 095cd301f211531a8b8f50e7915fe13c7ab0b278fb23dc50a03625cfae9e2fd7a0d8c315fced4af3011552e3e455ce562b7f717a0ed096c4433ddcf24f22b2c9
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
90f1f849e9f5a0c1855b72824af38b9aa24d5287 doc: Suggest `keys.openpgp.org` as keyserver in SECURITY.md (Tim Ruffing)
Pull request description:
`--recv-keys` without a `--keyserver` arg simply failed for me on a fresh Arch Linux installation, so I think it's a good idea to suggest a keyserver. OpenPGP ecosystem is broken in a number of ways, so the right way to approach this issue has some potential for bikeshedding. But the only thing that this PR does is to keep `SECURITY.md` in line with the instructions for builder keys, where there was agreement on switching to `keys.openpgp.org` (#22688).
ACKs for top commit:
MarcoFalke:
review ACK 90f1f849e9f5a0c1855b72824af38b9aa24d5287
laanwj:
Review ACK 90f1f849e9f5a0c1855b72824af38b9aa24d5287
hebasto:
ACK 90f1f849e9f5a0c1855b72824af38b9aa24d5287, agree with arguments above.
Zero-1729:
ACK 90f1f849e9f5a0c1855b72824af38b9aa24d5287
Tree-SHA512: 1ab20c837cd952aa32b57473772cbfd33411a08db6e88b951bce38f76a3c509c0e91d6944ec0ca5eac8d5eb4d98a5489276d55691328f2e2556b2640f8e7c108
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
fa6c62f34b50818102ad58f2ffd152189f54d973 test: Replace log with assert_equal in wallet_abandonconflict (MarcoFalke)
Pull request description:
This will make the test fail as soon as the bug is fixed, forcing it to
be updated. Otherwise, the bug might be fixed (or made worse)
accidentally, leaving the test in an inconsistent state.
ACKs for top commit:
theStack:
Concept and code-review ACK fa6c62f34b50818102ad58f2ffd152189f54d973
brunoerg:
tACK fa6c62f34b50818102ad58f2ffd152189f54d973
Tree-SHA512: 416f72380164bf3f93332a5cfa81a8e61d8ada3714ef6815889ed3cf2d16c09411760dee4acf19629227e565b765b3dea491a0b23efd38eb370254cfadf7c441
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
bda620aecd690004c52e550ad7de187ce0eb655d test: check abandoned tx in listsinceblock (brunoerg)
Pull request description:
This PR tests if the abandoned transaction is correct in listsinceblock return (wallet_abandonconflict.py).
ACKs for top commit:
jonatack:
ACK bda620aecd690004c52e550ad7de187ce0eb655d
theStack:
ACK bda620aecd690004c52e550ad7de187ce0eb655d
stratospher:
Tested ACK bda620a. This PR verifies whether the transaction txAB1 has been abandoned in listsinceblock and is a nice addition to the test!
Tree-SHA512: e4dce344cf621de7a8b5bd8660d252419772a293080fc881f6f448b6df85c6b1c8f0df619e855a40b6393f53c836f0d7fadbd3916c20cccd3a95830b8b502991
0bc666b053b8f4883c3f5de43959e2bbd91b95c5 doc: add info for debugging with relative paths (S3RK)
a8b515c317f0b5560f62c72a8f4eb6560d8f1c75 configure: keep relative paths in debug info (S3RK)
Pull request description:
This is a follow-up for #20353 that fixes#21885
It also adds a small section to assist debugging without absolute paths in debug info.
ACKs for top commit:
kallewoof:
Tested ACK 0bc666b053b8f4883c3f5de43959e2bbd91b95c5
Zero-1729:
Light crACK 0bc666b053b8f4883c3f5de43959e2bbd91b95c5
Tree-SHA512: d4b75183c3d3a0f59fe786841fb230581de87f6fe04cf7224e4b89c520d45513ba729d4ad8c0e62dd1dbaaa7a25741f04d036bc047f92842e76c9cc31ea47fb2
