907f142fc7e1d35f443be076367739faf11cc2cc rpc: change no wallet loaded message to be clearer (Andrew Chow)
Pull request description:
Changes the no wallet is loaded rpc error message to be clearer that no wallet is loaded and how the user can load or create a wallet. Also changes the error code from METHOD_NOT_FOUND to RPC_WALLET_NOT_FOUND as that makes more sense.
ACKs for top commit:
MarcoFalke:
review ACK 907f142fc7e1d35f443be076367739faf11cc2cc
kristapsk:
ACK 907f142fc7e1d35f443be076367739faf11cc2cc. In addition to standard tests, just in case tested that this doesn't break anything with JoinMarket.
meshcollider:
utACK 907f142fc7e1d35f443be076367739faf11cc2cc
Tree-SHA512: 4b413e6ab5430ec75a79de9db6583f2f3f38ccdf71aa373d8386a56e64f07f92200c8107c8c82c92c7c431d739615977c208b771a24c5960fa8676789b5497a2
2f79e9d00206a5230377f49be7b2f6da70f80417 refactor: remove unused header <arpa/inet.h> in protocol.cpp (Sebastian Falbesoner)
Pull request description:
There is no code using types or functions related to "internet operations" anymore in protocol.cpp (since #735, more than 8 years ago!), hence the header include can be removed.
ACKs for top commit:
practicalswift:
ACK 2f79e9d00206a5230377f49be7b2f6da70f80417 -- patch looks correct and CI is happy
epson121:
Code review ACK 2f79e9d00206a5230377f49be7b2f6da70f80417
laanwj:
ACK 2f79e9d00206a5230377f49be7b2f6da70f80417
promag:
Code review ACK 2f79e9d00206a5230377f49be7b2f6da70f80417.
Tree-SHA512: b3f75fa080125a34ce224f11eb13ec7b914cd9930e3bbed24f550031ce92a7e0830e8ff20159d737ffe487dfd28c24c273ad5e89c6932c8c6960d7fadb6c5e54
56b018ca7f37d25041b74f1bec305bdf54a55b9b test: Fix flaky wallet_basic test (Fabian Jahr)
Pull request description:
Fixes#19853
I investigated the issue in #19876 and I still intend to fix the underlying issue of a race when using wallet RPCs right after starting a node in that PR. However, since that is a bit more complicated than I initially thought it makes sense to merge the fix of the test so the intermittent test failures stop. This fix in the test is going to be needed, either way, #19876 will only provide an error where before it was reporting a false balance.
Top commit has no ACKs.
Tree-SHA512: 52bb2388a3e77aa20d26ab0fd45796bc1781483b1cffe49cbb44e2488a72e76998edfb1198495373f9c6fd2ec26064d4176bd1a64dd59806622d5e50a4f4e870
86d4cf42d97abf4c436d1eabf29e2ed150f69c1e Increase the ip address relay branching factor for unreachable networks (Pieter Wuille)
Pull request description:
Onion addresses propagate very badly among the IPv4/IPv6 network, resulting
in difficulty for those to find each other.
The branching factor 1 is probably so low that propagations die out before
they reach another onion peer. Increase it to 1.5 on average.
ACKs for top commit:
practicalswift:
ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e -- patch looks correct
naumenkogs:
ACK 86d4cf4
jonatack:
ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e. Code review, built and running with some sanity check logging. `RelayAddress()` is called by `ProcessMessage() ADDR` msg handling, from within the loop while processing each new address to relay it to a limited number of other nodes. According to git blame, the line setting `nRelayNodes` hasn't been touched since 2016 in e736772c56 *Move network-msg-processing code out of main to its own file*, which moved the line but otherwise did not change it. Running a mixed clearnet/onion node with this patch and the logging below, I'm only seeing values of `fReachable 1, nRelayNodes 2`. IIUC, I need to use the settings in `init.cpp` that call `SetReachable(*, false)`. *Edit:* with `onlynet=onion` am now seeing entries of `fReachable 0` with `nRelayNodes` values of 1 and 2.
vasild:
ACK 86d4cf42d
Tree-SHA512: 22391e16d60bcfdec9a9336728da39d68a24a183b3d1b0e8fbc038d265ca6ddf71d16db018f3678745fd9f3e9281049e42197fa0a29124833c50a9170ed6f793
4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e1f3a4f94a5a4cb7adeecf967e14156bde qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)
Pull request description:
Fix#24
The first commit:
- visual improvement with no behavior change
The second commit:
- removes a bunch of LOCs
- slightly change behavior and makes it standard
With this PR:
ACKs for top commit:
Saibato:
Concept tACK 4227a8e1f34ec49f8d1e
promag:
Tested ACK 4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 on macos.
Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce Remove old check for 3-byte shifted IP addresses from pre-0.2.9 node messages (Raúl Martínez (RME))
Pull request description:
The change removes an old check for IPv6 addresses in range ::ff:ff00:0:0:0/72 that were created due to a bug in size field of addr messages for 0.2.8 nodes and before.
This check is no longer needed as they are no more pre 0.2.9 nodes on the network (as per bitnodes network snapshot).
Credits for discovering this go to sipa in https://github.com/bitcoin/bitcoin/pull/19628#discussion_r475907453
Thanks for the attention!
ACKs for top commit:
sipa:
utACK 7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce
vasild:
ACK 7b6d0f1
Tree-SHA512: c5fab59dda2acafe143f607a4c5b636a54ac76fba651cad1ad1b09c94e88ab39503a31c2244c8f2664da68456c2a870c601d8894139c55cde9ece8161913ed2e
df536883d263781c2abe944afc85f681cda635ed chain: Remove UB CChain comparison (Carl Dong)
Pull request description:
Comparing two empty `CChain`s is currently undefined behaviour, and resulted in false assertion failures when comparing identical empty `CChain`s in local testing.
Let's just remove this comparison operator since it doesn't seem to be used anywhere.
ACKs for top commit:
practicalswift:
ACK df536883d263781c2abe944afc85f681cda635ed -- patch is guaranteed to be correct :)
MarcoFalke:
cr ACK df536883d263781c2abe944afc85f681cda635ed
Tree-SHA512: db10bac364fc965b56abf7a5bac48018786b14806ffe107e3e8eb24d5004a29331f3387dfe3409a3452a6750d3329e3f354265d787ebb3abfccabe77b28a54d5
b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8 gui: Clarify block height label (Hennadii Stepanov)
Pull request description:
Prefer "block height" instead of "number of blocks".
This was done while testing https://github.com/bitcoin/bitcoin/pull/16981.
ACKs for top commit:
michaelfolkson:
ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8. I don't think there are any other obvious examples in the GUI where "block height" should replace "number of blocks" except for translations.
MarcoFalke:
cr ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8
Tree-SHA512: ec3b48c1af5d613ed657ad51f2caddea774376736ecc02343d54518986e35ec37f1745b059814b5be92b5e5c2bb2970d17159b24c6e88b9316803d4de5327c31
366913e307d2dc13bc00d6bf7b6b2426c359ac30 build: AX_BOOST_THREAD serial 33 (Igor Cota)
cf0681133ae7301ead7091eaee55c945da5cdfcc build: disable D-Bus on Android by default (Igor Cota)
Pull request description:
I've been trying to build for Android on different OSes/Gitian with varying success. Build system is quite the beast and sometimes it doesn't get it right. To make sure it does these three little tweaks make the Android build more robust:
- disable D-Bus (Android doesn't support it and has its own way to trigger notifications)
- don't flag `-lpthread` when linking Boost, [Bionic has built-in support](https://stackoverflow.com/questions/30801752/android-ndk-and-pthread)
- ~~add `-static-libstdc++` to linker flags. This avoids having to bundle `libc++_shared` with CLI apps, still necessary with `bitcoin-qt` though (thanks Sjors)~~
I think these are small and fairly straightforward so I put them all into this one PR.
ACKs for top commit:
fanquake:
ACK 366913e307d2dc13bc00d6bf7b6b2426c359ac30
Tree-SHA512: 31465fd228a5877c20aa2a05f98242d4eeb328b9b35bd1a7a3dcfb1ef51379d84053a81ade5a65436ffc1bc8ccd21f11ed0539eb10e827d182c0c04394629af0
5067c5acc30c5cf87496c1bf8eb03712cc66b206 [test] Add test for getblockheader verboseness (Torhte Butler)
Pull request description:
Improve test coverage by adding a test for getblockheader with verbose argument set to false.
ACKs for top commit:
theStack:
ACK 5067c5acc3
Tree-SHA512: e55593f1026a89dc7b796fa985b4cbcdb596e91d80d42dfb0660bda1692aaa35749ec29f9cd7032803f6225afb323f085df1ef6a9982de87be8e098f7253cdd5
ca2e47437277ef6851a739f247b44e73a53f21a1 Fix a compiler warning: unused GetDevURandom() (Vasil Dimov)
Pull request description:
~~Only define GetDevURandom() if it is going to be used.~~
Silence by planting a dummy reference to the `GetDevURandom` symbol
in the places where we don't call the function.
ACKs for top commit:
practicalswift:
ACK ca2e47437277ef6851a739f247b44e73a53f21a1 -- increased signal to noise in compiler diagnostics is good
sipa:
utACK ca2e47437277ef6851a739f247b44e73a53f21a1
hebasto:
re-ACK ca2e47437277ef6851a739f247b44e73a53f21a1, tested on macOS 10.15.6 + llvm clang 10.0.0
Tree-SHA512: 03c98f00dad5d9a3c5c9f68553d72ad5489ec02f18b9769108a22003ec7be7819a731b1eab6a9f64dafb5be0efddccf6980de7e3bb90cd20d4f4d72f74124675
72351784b3df21a89f79076f4b814a6e700b6469 lint: Remove travis env var from commit linter (Fabian Jahr)
Pull request description:
#19439 was recently merged and seemed to work fine but I now noticed strange behavior when it was running in Travis, which I could not reproduce locally. It turns out `TRAVIS_COMMIT_RANGE` which is used in Travis to get the commits for the linter, uses all the commits that were in a push, which includes all rebase commits for example. This means that the linter can fail on a commit that the developer has never even seen before, which can be very confusing. See an example here which caused me to look into this: https://travis-ci.org/github/bitcoin/bitcoin/jobs/714296381 The commit that is reported as failing in my PR is not part of my PR.
I think we rather want to use something like `git merge-base` to get the commit range by default and in Travis. I am leaving the env variable functionality in place with a different name but this is not a variable that can be expected to be present in the CI environments so the `merge-base` range should be used there by default.
ACKs for top commit:
hebasto:
ACK 72351784b3df21a89f79076f4b814a6e700b6469, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: afb27bb386855cb8d5cf84fd3a6c11ef1160b25af6175ed0aa146bf04b9a26eb77298df70df0a855f8c46f19f08b3f62c49872c12974fcfa5526a15ee05b3c10
334de75885dd0fb1ca51c6ec4536d9f665957095 scripted-diff: Remove Reference Links (Robert)
Pull request description:
Removed all reference links.
Found this issue from #19582.
The decision to remove links instead of update them was made in #19584
The author of that PR was slow to resolve his commit to use scripted diff so I made this PR instead.
ACKs for top commit:
laanwj:
ACK 334de75885dd0fb1ca51c6ec4536d9f665957095
MarcoFalke:
ACK 334de75885dd0fb1ca51c6ec4536d9f665957095
Tree-SHA512: a337116379912b27974867bd86ec7799a1d41d67b51771885467fbe1be003b415cb37ce8e521568bf3eae190ab2f6af0d6e29fd3ea25f2689b8fb31def8fec96
284a969cc082ae3c63ab523f22e71da86ad4ab20 Linter to check commit message formatting (Amir Ghorbanian)
Pull request description:
Write linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091.
ACKs for top commit:
troygiorshev:
ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 Reviewed, manually tested. Works great!
fjahr:
tested ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
adamjonas:
utACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
Tree-SHA512: fa278f090780b54e4fa6e2967a62b4c1a4da55d112ec1ad6dd7e1181ac490c5c1af0165524b5781b463fdd6d0f79fd3d95b5160184e6eca432ccff1189f77390
ef3d4ce4c301caa57946f772f554678cd872fca8 build: call AC_PATH_TOOL for dsymutil in macOS cross-compile (fanquake)
Pull request description:
While testing #19530 I noticed that we couldn't call [`dsymutil`](https://www.llvm.org/docs/CommandGuide/dsymutil.html) after LTO:
```bash
../libtool: line 10643: x86_64-apple-darwin16-dsymutil: command not found
```
This updates configure to call `AC_PATH_TOOL` so that we end up with the
full path to dsymutil, similar to `otool` and `install_name_tool`, ie:
`/bitcoin/depends/x86_64-apple-darwin16/share/../native/bin/x86_64-apple-darwin16-dsymutil`.
ACKs for top commit:
laanwj:
Code review ACK ef3d4ce4c301caa57946f772f554678cd872fca8
theuni:
ACK ef3d4ce4c301caa57946f772f554678cd872fca8.
Tree-SHA512: e4fa93e7f9f7945289143dfe2a6645ad8ee7f3bee0793412b3509901a30566d6f952e3b39e0e525a54f8dbd0c480f8da70fc6cb80b07800d11b0c6071fbb7466
5fa067a27d709a8a24b798cbd2459bf5b291c885 Remove unnecessary blockfile SetPos (Tom Harding)
Pull request description:
Nothing could have changed the position since we retrieved it a few statements earlier. This dates from commit 16d5194165.
ACKs for top commit:
LarryRuane:
ACK 5fa067a27d709a8a24b798cbd2459bf5b291c885
Tree-SHA512: 459cc7226e186c231ffb67f0613f550e8eb940f1b8933c3bc4a4e8dd519c8d5d45884e8cfd9347039dab90a093644bbbb31be063baed1c6fc7984b6cb4f17c9f
fa6d5ab67444013f9db696cca2257c871e002594 refactor: Remove unused BlockAssembler::pblock member var (MarcoFalke)
Pull request description:
It seems odd to have a confusing and fragile "convenience pointer" member variable to be able to write `pblock->vtx` instead of `pblocktemplate->block.vtx` in a single place.
ACKs for top commit:
promag:
Code review ACK fa6d5ab67444013f9db696cca2257c871e002594.
Tree-SHA512: e9f032b5ab702dbefffd370db3768ebfb95c13acc732972b695281ea34c91d70cd0a1700bc2c6f106dbc9de68e81bc6bb06c68c2afd53c17cba8ebee4f9931b9
fc6a637a013daeb14b2f93652d7f494f3b8462aa qt: increase console command max length (10xcryptodev)
Pull request description:
fix#17618
Tested the examples https://github.com/bitcoin/bitcoin/issues/17618#issuecomment-559538070 and works
ACKs for top commit:
MarcoFalke:
Approach ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa
hebasto:
ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 4975d7fa4c13a6b0f50f5754c3e04eb5a42b1411c385dc883d9948b6fc0dee38900ba2a418218a9a30ce39988a27d22f3ff3a02f0fa44f4136f01eef473efeca
4b5ac258812a1e8848862689ff333587cf274892 Drop unused CDBWrapper methods (Hennadii Stepanov)
Pull request description:
`CDBWrapper::Flush()` and `CDBWrapper::Sync()` are not used in the code.
ACKs for top commit:
promag:
ACK 4b5ac258812a1e8848862689ff333587cf274892.
laanwj:
ACK 4b5ac258812a1e8848862689ff333587cf274892
Tree-SHA512: 06115c59e75995d496173a64ceea1b9bb1b4fe3eac8bf4f59df68b87b112b5b3e8065298dcd5c4c7408544f76ee62922325acc2208619d830fd5dbb420cdda5c
b9253c7d2089d3d159dcc10118ce5a219d9a6881 tools: clang-format 6 compatibility (Jon Atack)
Pull request description:
Our `.clang-format` settings inadvertently lost compatibility with Clang versions < 9 in #19095, including for Debian stable. This patch returns compatibility in the interim until the distros update. See discussion from https://github.com/bitcoin/bitcoin/pull/19095#issuecomment-651926138.
ACKs for top commit:
MarcoFalke:
Approach ACK b9253c7d2089d3d159dcc10118ce5a219d9a6881 , haven't tested
Tree-SHA512: 4af541a195f48d84ffb80e23aaefb624c66bc78f087c8d92b4af5a654420b69fedf25272c6e4fde2688ff88412d306b7a990ce1e15d8b24180374c625a253fb6
faebb60b8d009e52d585cffd0f124a0f2fd1a66d doc: Remove outdated comment in TransactionTablePriv (MarcoFalke)
Pull request description:
Locks are no longer taken upfront, so remove the outdated comment
ACKs for top commit:
hebasto:
ACK faebb60b8d009e52d585cffd0f124a0f2fd1a66d, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: cd6df24d49d17e58049ac9b261c5e07c8e85ed1aacb547b13c0e55139339d7fcc3b1f766ea2e27d758ea77deadc01f7e28781be1515323c82b9012cee8fd488b
14bc2a17dd03ccd89f65a302328763ff22c710c2 Trivial: add doxygen-compatible comments relating to BerkeleyEnvironment (Pierre Rochard)
88b1d956fe3e38f2d2dd805feee9dadb0be9e8a9 Tests: add unit tests for GetWalletEnv (Pierre Rochard)
f1f4bb7345b90853ec5037478173601035593d26 Free BerkeleyEnvironment instances when not in use (Russell Yanofsky)
Pull request description:
Instead of adding BerkeleyEnvironment objects permanently to the g_dbenvs map, use reference counted shared pointers and remove map entries when the last BerkeleyEnvironment reference goes out of scope.
This change was requested by @TheBlueMatt and makes code that sets up mock databases cleaner. The mock database environment will now go out of scope and be reset on destruction so there is no need to call BerkeleyEnvironment::Reset() during wallet construction to clear out prior state.
This change does affect bitcoin behavior slightly. On startup, instead of same wallet environments staying open throughout VerifyWallets() and OpenWallets() calls, VerifyWallets() will open and close an environment once for each wallet, and OpenWallets() will create its own environment(s) later.
Tree-SHA512: 219d77a9e2268298435b86088f998795e059fdab1d2050ba284a9ab8d8a44961c9b5cf96e94ee521688108d23c6db680e3e3a999b8cb2ac2a8590f691d50668b
4e353cb618745cdb5d98e58e7dcd400ded01299a http: Release work queue after event base finish (João Barbosa)
Pull request description:
This fixes a race between `http_request_cb` and `StopHTTPServer` where
the work queue is used after release.
Fixes#18856.
ACKs for top commit:
fjahr:
Code review ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
achow101:
ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
LarryRuane:
ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
hebasto:
ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a, tested (rebased on top of master 9313c4e6aa4b707c06a86b33d5d2753cd8383340) on Linux Mint 20.1 (x86_64) using MarcoFalke's [patch](https://github.com/bitcoin/bitcoin/pull/19033#issuecomment-640106647), including different `-rpcthreads`/`-rpcworkqueue` cases. The bug is fixed. The code is correct.
Tree-SHA512: 185d2a9744d0d5134d782bf321ac9958ba17b11a5b3d70b4897c8243e6b146dfd3f23c57aef8e10ae9484374120b64389c1949a9cf0a21dccc47ffc934c20930
9a0969585fce03b45be7004bba865bc15909904c build: Add /opt/homebrew to path to look for boost libraries (Fu Yong Quah)
Pull request description:
Following the instruction in https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md for building on the M1 Macs don't work out of the box, because homebrew now defaults to /opt/homebrew instead of /usr/local. This PR fixes that.
ACKs for top commit:
jonasschnelli:
utACK 9a0969585fce03b45be7004bba865bc15909904c
promag:
Tested ACK 9a0969585fce03b45be7004bba865bc15909904c.
Tree-SHA512: 472568b97fbd8623481fe6fd43b0509fa32fe7f1c1d8090321a6a6a5bdc7343d4ad4122c10dcc7c9c93068db8a3f009a73befaf1ba11e4af54a66afd2c2dbe14
2be35725069fd4c589497b93e09e1c6db6946372 test: Fix IPv6 check on BSD systems (nthumann)
Pull request description:
I noticed that `test_ipv6_local()` always returns `False` on macOS or FreeBSD, even though IPv6 is working perfectly fine. This causes `test/functional/rpc_bind.py --ipv6` and `test/functional/feature_proxy.py` to skip their run.
Apparently, there's a check if the port number is `0` (see [here](64881da478/sys/netinet6/udp6_usrreq.c (L248)) or [here](8f02f2a044/bsd/netinet6/udp6_usrreq.c (L282))), while Linux has no problem with this.
This is fixed by specifying any other port number than `0`, e.g. `1`. Still, because of `SOCK_DGRAM`, no actual connection is made.
ACKs for top commit:
fanquake:
ACK 2be35725069fd4c589497b93e09e1c6db6946372 - nice improvement. I checked that with this change ipv6 related tests in `feature_proxy.py` are being run.
theStack:
ACK 2be35725069fd4c589497b93e09e1c6db6946372
Tree-SHA512: 8417c2d3cf71050529f3fa409a03872040fe5d249eae4172f276e62156e505a20d375b963712a186c9ad7967d8a497b5900d327c74a9693f68c33063871d4691
7d07192ddeaad28a26e3c4a54757a4f27f2a08ff Add src/qt/android/.gitignore (Hennadii Stepanov)
Pull request description:
This PR makes `git` ignore files created by `make apk`.
ACKs for top commit:
icota:
ACK 7d07192dde
Tree-SHA512: 4be20bd84830217a10d8ea7634799e71ed50be73f4f60c91c56311a2c95b22ff1f28d3b7bc077f1417318bb75e446e3fc3bdbf9dbc037b4cbc8428f0875f2c77
74bf850ac47735f2ef4306059d3e664d40cac85e faster HexStr => 13% faster blockToJSON (Martin Ankerl)
Pull request description:
`std::string`'s push_back is rather slow because it needs to check & update the string size. For
`HexStr` the output string size is already easily know, so we can initially create the string with
the correct size and then just assign the data.
`HexStr` is heavily usd in `blockToJSON`, so this change is a noticeable benefit. Benchmark on an i7-8700 @3.2GHz:
* 71,315,461.00 ns/op master
* 62,842,490.00 ns/op this commit
So this little change makes `blockToJSON` about ~13% faster.
ACKs for top commit:
laanwj:
Code review ACK 74bf850ac47735f2ef4306059d3e664d40cac85e
theStack:
re-ACK 74bf850ac47735f2ef4306059d3e664d40cac85e
Tree-SHA512: fc99105123edc11f4e40ed77aea80cf7f32e49c53369aa364b38395dcb48575e15040b0489ed30d0fe857c032a04e225c33e9d95cdfa109a3cb5a6ec9a972415
bc4538806e3c53e7821e01d5db896f65dd3358ad build: add *~ to .gitignore (Sjors Provoost)
Pull request description:
The file `configure~` recently started appearing for me on macOS (11.3.1) whenever configure is (re)run.
ACKs for top commit:
hebasto:
ACK bc4538806e3c53e7821e01d5db896f65dd3358ad, tested on Linux Mint 20.1 with different build scenarios including cross-compiling for Windows and macOS.
Tree-SHA512: 830c7baf392ff6d66250a79c6ed0a98dac3daaace54a6d2e7940b9a72e3bac79ab44bbecd7642c931fde8a446654e2260d6afdecc679a1743fae6ec5eeda79f1
34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0 refactor: Add TSA annotations to the WorkQueue class members (Hennadii Stepanov)
Pull request description:
Noted while reviewing #19033, and hoping this will not conflict with it :)
ACKs for top commit:
promag:
Code review ACK 34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0.
Tree-SHA512: 4c15729acd95223263c19bc0dd64b9e7960872b48edee6eee97a5d0c2b99b8838185ac3a2ccd5bee992cb3a12498633427fe9919be5a12da9949fcf69a6275a0
fa2204f6adef493079d1ca5148b0fdc2b55816e6 streams: Accept URef obj for VectorReader unserialize (MarcoFalke)
Pull request description:
Missed in commit 172f5fa738. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in `VectorReader::operator>>`, so add it for consistency.
ACKs for top commit:
ryanofsky:
Code review ACK fa2204f6adef493079d1ca5148b0fdc2b55816e6, just expanded test since last review
Tree-SHA512: 09ff4e8a918e15b08cebd8c125d37e78bfb3a635c38546fc8454a97a882b2c81c55ef552243617e78744799d31127e6fbf78c4e319c030480b370aab6f38b645
fa9249aaccc3ef7a0a91a822e1cb666c4c9716ec depends: Add missing -D_LIBCPP_DEBUG=1 to debug flags (MarcoFalke)
Pull request description:
Commands that can be used for testing:
```
$ cat 1.cpp
#include <vector>
int main() {
std::vector<int> foo;
foo.begin() + 7;
}
```
```
clang++ -stdlib=libc++ -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -Wall 1.cpp -o exe && ./exe
g++ -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -Wall 1.cpp -o exe && ./exe
ACKs for top commit:
practicalswift:
cr ACK fa9249aaccc3ef7a0a91a822e1cb666c4c9716ec: patch looks correct
fanquake:
ACK fa9249aaccc3ef7a0a91a822e1cb666c4c9716ec - was going to suggest adding this to the macOS CPP flags as well, however it seems doing that is less straight forward. Could be looked at by someone in a followup.
Tree-SHA512: 2ffbaaf0ccb36bcc9fa1a15426566406c6115c8878ff211a4794d982c5d198672d444a20f6c7ae9f341193f6d8118c7cc50896daf98af9553834379e47ddb39e
cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7 [tools] Update clang-format config (John Newbery)
Pull request description:
In some cases, running clang-format has made code _less_ readable by joining declarations and calls for functions with many arguments into very long lines. For example:
```
- size_t getQueueInfo(std::chrono::system_clock::time_point &first,
- std::chrono::system_clock::time_point &last) const;
+ size_t getQueueInfo(std::chrono::system_clock::time_point& first, std::chrono::system_clock::time_point& last) const;
```
(https://github.com/bitcoin/bitcoin/pull/19090#discussion_r431961148)
This change to clang-format would allow arguments/parameters for func declarations/calls to be split over multiple lines, aligned with the opening parens. It does not force args/params to be on new lines (that setting is `BinPackParameters : true`).
ACKs for top commit:
MarcoFalke:
ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7 fine with me
practicalswift:
ACK cc29d1e2c46e01c544ef44c79d72b7bcc5d39ba7
Tree-SHA512: a62474925e71aaff41bdce7960fd5ffd64317da810f694d8084080b054708cf71c2ab2ce3111db5a9260d1c1f9e02d59a2ecb5543b1b6172ce085cb42432160a
fa84edb93c85f7709fc53abf9c6daae5d1bb3b28 build: don't warn when doxygen isn't found (fanquake)
Pull request description:
Doxygen isn't so important that we need to warn when it is missing. I'd
assume it might even be missing more often than not for most builds.
ACKs for top commit:
MarcoFalke:
Fine with me ACK fa84edb93c85f7709fc53abf9c6daae5d1bb3b28
hebasto:
ACK fa84edb93c85f7709fc53abf9c6daae5d1bb3b28, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 793ebf01a8a5d48b78a70fdef0022633fca59b30074c960ebb21589e3bd98992b8304621a2d999195d12172ed30fe9eefeeb2a952d58853cf58e8d9902b0090c
* Update to leveldb upstream using subtree merge
* Import crc32c using subtree merge as as 'src/crc32c'
* build: Update build system for new leveldb
Upstream leveldb switched build systems, which means we need to define
a few different values.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* doc: Add crc32c subtree to developer notes
* test: Add crc32c to subtree check linter
* test: Add crc32c exception to various linters and generation scripts
* build: Add LCOV exception for crc32c
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* build: CRC32C build system integration
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Removes all (besides one) usages of "Params().GetConsensus().llmqs.at" and instead uses the wrapper in quorum_utils.cpp
Rename all params to llmq_params for consistency and not conflict with non-llmq params
make some llmq_params const where possible
remove unneeded llmq_params variables where it's only used once
Signed-off-by: pasta <pasta@dashboost.org>
9a19c9ada53e84d1ee8521b48f656faad48b737a Always define the raii_event_tests test suite (Craig Andrews)
Pull request description:
The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn't fail due to not being able to find the raii_event_tests test.
This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493
ACKs for top commit:
MarcoFalke:
ACK 9a19c9ada53e84d1ee8521b48f656faad48b737a 🎹
Tree-SHA512: 3c42f17a9b5d56c8841f3aa9ac19da91c10aff210026266f31f7eb98a62528740d7c518c121452b68e8f801d6c80ecfb627d137ec6ed533289fa3beb08b4f176
9760293ce632e09f0175368ebf0c8502ac9b10d4 wallet: Fix for exported confirmation field in payment to self transactions (Ben Carman)
Pull request description:
Closes#3455
ACKs for top commit:
jonasschnelli:
Tested ACK 9760293ce632e09f0175368ebf0c8502ac9b10d4
Tree-SHA512: 8207768771ad787f716b966c4aa7aeef2da8a602e32e3510e41c7b49ec5ec679a3835d248be5016d4b37764f9914846f7c41c11cf48cddb617cb7ef831318fd7
e8fa0a3d2025509fcddc59fc618e91371542cf87 Fix WSL file locking by using flock instead of fcntl (Samuel Dobson)
Pull request description:
Fixes#18622
A bug in WSL means that fcntl does not exclusively lock files, allowing multiple instances of bitcoin to use the same datadir. If we instead use flock, it works correctly. Passes Travis, but testing on some OS variety would be sensible.
From what I can tell, flock and fcntl don't work with each other on linux, so it would still be possible to run a node with this code change and a node before it with the same datadir (this isn't true for Mac/FreeBSD). flock also doesn't support NFS on MacOS and linux<2.6.12 while fcntl did. See here for example: https://gavv.github.io/articles/file-locks/
If changing to flock for all systems is inadvisable, it would also be possible to just detect WSL and use flock when on that platform to avoid the bug.
ACKs for top commit:
laanwj:
Code review ACK e8fa0a3d2025509fcddc59fc618e91371542cf87
Tree-SHA512: ca1009e171970101f1dc2332c5e998717aee00eebc80bb586b826927a74bd0d4c94712e46d1396821bc30533d76deac391b6e1c406c406865661f57fa062c702
4c825792dd9f4eaf4936c3e376ac7a5c177528e2 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: https://github.com/bitcoin/bitcoin/pull/3843
But in https://github.com/bitcoin/bitcoin/pull/5713 strict DER encoding was enforced in consensus,
and is now it's buried and enforced by the height of the block here: 4af01b37d4/src/validation.cpp (L1889)
P.S. This is also quite confusing: 4af01b37d4/src/validation.cpp (L1560-L1563) But seems to be intentional: 4af01b37d4/src/validation.cpp (L1510-L1511)
ACKs for top commit:
laanwj:
ACK 4c825792dd9f4eaf4936c3e376ac7a5c177528e2
Tree-SHA512: 7afbbae84ed4ecfaa0a273ae024b14f2b7ffe65307f078086fe0b5b645c57722bc2952fb15d167d9e4fa5b052d1d0ac6e5e33f57e8fc881c0ea611d352bccc1e
69bfcac27a83440092bc6e61904ded910ed4baf4 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:
This PR:
ACKs for top commit:
hebasto:
ACK 69bfcac27a83440092bc6e61904ded910ed4baf4, tested on macOS 10.15.
Tree-SHA512: df142fb16097deb514e72e005b73aafc4eb4ff0c17e423ba5040a3ec6874020a733e1c5259a88923580e71ef73c16222aed28f482b8c270a544a85b745a7b327
fa243be1dc49385fff847f8a784c7a9c9f07c939 log: Remove "No rpcpassword set" from logs (MarcoFalke)
Pull request description:
rpcpassword is deprecated and not recommended anymore. So remove it from the logs, which indicate that an rpcpassword should be set and cause confusion. See #18998.
ACKs for top commit:
ryanofsky:
Code review ACK fa243be1dc49385fff847f8a784c7a9c9f07c939. New log message makes more sense
elichai:
Re Code Review ACK (Checked the diff) fa243be1dc49385fff847f8a784c7a9c9f07c939
Tree-SHA512: de3e0800a204b15a59a59a7e6f345013ee9d38e8c5d0c9a94d6142780faa9cce672ed358c7571f53c1eb843bf5afb0b7bcbfd289d3b9e2e0bf8ff2fd361e98a9
8d306862ef077f2a71931372dd6a2efa05188c84 ci: Add fuzzbuzz integration (practicalswift)
Pull request description:
Add fuzzbuzz integration.
Just like #15338 enabled optional FreeBSD building via Cirrus CI (`.cirrus.yml`) this PR adds optional fuzzing via fuzzbuzz (`.fuzzbuzz.yml`).
Having this merged makes is easier for people to fuzz Bitcoin Core (via their forked repos) using their fuzzbuzz account and then hopefully submit coverage increasing corpus additions upstreams to to https://github.com/bitcoin-core/qa-assets.
Historically it has been mostly been me and MarcoFalke who submit test cases to `qa-assets`, but with this change hopefully more people will join the hunt for coverage increasing fuzzing inputs :)
Top commit has no ACKs.
Tree-SHA512: c7d8e354996c673da36cc9add260383c82a5325bfaa7ce6141ad6cd6b7d6adf3a6c900ea2db17fb70147b3625fa7f6a1ff8ba813aeaa299f316d8f6cabb3a65c
