Move DataDirectoryGroupReadable 1 up a few lines to more clearly
communicate that it is required for the filesystem group to read the
DataDirectory.
Per the Tor documentation
https://2019.www.torproject.org/docs/tor-manual.html.en#DataDirectoryGroupReadable
"If this option is set to 0, don’t allow the filesystem group to read
the DataDirectory. If the option is set to 1, make the DataDirectory
readable by the default GID. (Default: 0)"
754fb6bb8125317575edec7c20b5617ad27a9bdd verifybinaries: fix argument type error pointed out by mypy (Cory Fields)
8a65e5145c4d128bb6c30c94e68434dd482db489 verifybinaries: catch the correct exception (Cory Fields)
4b23b488d2c5662215d78e4963ef5a2b86b4e25b verifybinaries: fix OS download filter (Cory Fields)
8cdadd17297e5f4487692eae88b1e60a42c8c4b2 verifybinaries: use recommended keyserver by default (Cory Fields)
4e0396835dd933a28446844da294040345f2e6ad verifybinaries: remove unreachable code (Cory Fields)
5668c6473a01528ac7d66b325b18b1cd2bd93063 verifybinaries: Don't delete shasums file (Cory Fields)
46c73b57c69933d7eb52e28595609e793e8eef6e verifybinaries: README cleanups (Cory Fields)
6d118302654481927e864a428950960e26eb7f4a verifybinaries: remove awkward bitcoin-core prefix handling (Cory Fields)
c44323a71705b6df9aafe90df24072e735a5c2ff verifybinaries: move all current examples to the pub subcommand (Cory Fields)
7a6e7ffd066a42c5fbb7d69effbe074fb982936b contrib: Use machine parseable GPG output in verifybinaries (Andrew Chow)
6b2cebfa2f1526f7eae31eb645c71712f0a69e97 contrib: Add verifybinaries command for specifying files to verify (Andrew Chow)
e4d577822835d4866e2ad046f23ab411b2910d59 contrib: Specify to GPG the SHA256SUMS file that is detached signed (Andrew Chow)
17575c0efa960ffb765392e3565b3861846f398e contrib: Refactor verifbinaries to support subcommands (Andrew Chow)
37c9fb7a59a3179b90ed1deaebaabb539976504b contrib: verifybinaries: allow multisig verification (James O'Beirne)
Pull request description:
Following up on #23020 from jamesob with achow101's additional features on top.
Both mentioned that they will be away for the next few weeks, so this is intended to keep review going.
All credit to the jamesob and achow101. See #23020 for the original description and [here](https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1480603300) for the added features.
I squashed the last commit from https://github.com/achow101/bitcoin/tree/pr23020-direct-bins-gpg-parse into the first commit here.
Fetching and local verification seem to work as intended for me.
ACKs for top commit:
josibake:
ACK 754fb6bb81
Tree-SHA512: b310c57518daa690a00126308a3e7e94b978ded56d13da15d5189e9e90b71c93888d854f64179150586b0a915db8dadd43c92b716613913c198128db8867257b
fa5af94de6aa4643b30ab526b6249340bff34d43 ci: Run base install at most once (MarcoFalke)
Pull request description:
This should avoid errors when running it twice. For example, network errors on the second invocation of 'apt update'; or unguarded modifications such as APPEND_APT_SOURCES_LIST, which will append the same string repeatedly.
The base install may be run twice in Cirrus CI with dockerfiles, or locally when running twice with DANGER_RUN_CI_ON_HOST specified.
ACKs for top commit:
josibake:
utACK fa5af94de6
Tree-SHA512: 020ba747e282076ddf10547025bb576ce626378d9f8313e09b1a5f99af1cf8237e44957aa4a0b8d64015bc730a090c83af0b3908a8eda15d3fb1ca7dae69dd7c
This should avoid errors when running it twice. For example, network
errors on the second invocation of 'apt update'; or unguarded
modifications such as APPEND_APT_SOURCES_LIST, which will append the
same string repeatedly.
The base install may be run twice in Cirrus CI with dockerfiles, or
locally when running twice with DANGER_RUN_CI_ON_HOST specified.
ed4a8339b8fe796b4668e206d7fb9c2b120f8eb2 ci: fix git dubious permissions error (josibake)
Pull request description:
fixes https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1496449588
this appears to be caused by a more recent version of git being sensitive to mismatched permissions on directories. we didn't notice this before because we were using two separate user accounts to fix up dir permissions in the container , but the second account was removed in #27376
there might be a more elegant way to do this, but this does the trick and seems to be the way others are fixing this issue around the internets.
ACKs for top commit:
RandyMcMillan:
ACK ed4a833
hebasto:
re-ACK ed4a8339b8fe796b4668e206d7fb9c2b120f8eb2
Tree-SHA512: dad467deca101a24f3ed34b3e26a1db5099a5bd5c3e9c9a22771c59848f7d7e7843c7386348e6fdf86d5a556e4706e5e20005d7a6637193e1c8aef7a5ff7fb19
6a9a4d13b2b22632e0acd4f86f7bac238294ba42 Fixes compile errors in MSVC build #27332 (Ethan Heilman)
Pull request description:
This PR is designed to address the issue https://github.com/bitcoin/bitcoin/issues/27332. The MSVC build is failing because of two bugs in how the build is configured.
The issue
====
When running `msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minima`l the build fails with following two errors.
* `C:\Users\e0\Documents\GitHub\bitcoin\src\httpserver.cpp(637,9): error C2664: 'void evhttp_connection_get_peer(evhttp_connection *,const char **,uint16_t *)': cannot convert argument 2 from 'char **' to 'const char **' [C:\Users\e0\Documents\GitHub\bitcoin\build_msvc\libbitcoin_node\libbitcoin_node .vcxproj]`
This error is occurs because bitcoin is using the wrong function signature for `evhttp_connection_get_peer` in libevent. In automake builds, configure.ac inspects the version of libevent it is building against and then defines `HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR` to flag the source code to use the correct signature. In MSVC build there does not appear to be a mechanism to do this. So it uses the wrong signature and fails. See the PR https://github.com/bitcoin/bitcoin/pull/23607 for when this logic was added to automake builds.
* `event.lib(evutil_rand.c.obj) : error LNK2019: unresolved external symbol BCryptGenRandom referenced in function arc4_seed [C:\Users\e0\Documents\GitHub\bitcoin\build_msvc\bitcoin-cli\bitcoin-cli.vcxproj]
C:\Users\e0\Documents\GitHub\bitcoin\build_msvc\x64\Release\bitcoin-cli.exe : fatal error LNK1120: 1 unresolved externals [C:\Users\e0\Documents\GitHub\bitcoin\build_msvc\bitcoin-cli\bitcoin-cli.vcxproj]`
This error is caused by msbuild not being able to find the library bcrypt.lib because it has not been configured to use bcrypt.lib.
Fixes
====
While for automake builds a macro is being define to configure the current function signature for `evhttp_connection_get_peer` in libevent, this macro is not being defined for MSVC builds.
1. This PR addresses this issue by assuming more recent version of libevent is installed and always defining `HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR`. This logic is more brittle the automake logic, but someone following the MSVC build instructions should only get the more recent version of libevent.
2. This PR fixes the bcrypt.lib errors this by setting this library as a dependency in build_msvc/common.init.vcxproj.in.
ACKs for top commit:
hebasto:
re-ACK 6a9a4d13b2b22632e0acd4f86f7bac238294ba42
Tree-SHA512: 69d2cb6a62ecca976540a39e5f83a4e5386d920a564761cedc5127d82e5fa66ced7fcde3e5e5eb3bd6cde1cc78f843e94aa2789f02bd3e3414118030017d0387
a56c96507a9e943bbcd7e126bc827de9495f0ebd ci: use clang-16 in tidy task (fanquake)
Pull request description:
Follow up to https://github.com/bitcoin/bitcoin/pull/27311#issuecomment-1481020371, as IWYU now has a [clang_16 branch](https://github.com/include-what-you-use/include-what-you-use/tree/clang_16).
This also removes some workarounds for (now fixed) clang-tidy issues, and simplifies the IWYU install steps.
ACKs for top commit:
MarcoFalke:
lgtm ACK a56c96507a9e943bbcd7e126bc827de9495f0ebd
josibake:
ACK a56c96507a
hebasto:
ACK a56c96507a9e943bbcd7e126bc827de9495f0ebd
Tree-SHA512: 5bbec6cc196c3305302895c77986f3695fc6f4024363ee57503654d54e0ebf108719a7a1d7908817f84115dcaa13377493eb764b00bdf574f1290c73251426fa
436df1e826cae036caed3e983715a4ed4e441321 depends: add NO_HARDEN option (fanquake)
Pull request description:
Add an option that when passed, will disable hardening options, and pass `--disable-hardening` through to configure. Due to the way we link `libssp` for Windows builds, they now fail (after #27118), if building with depends, and configuring with `--disable-hardening` (Windows is the odd build out here). See: https://github.com/bitcoin/bitcoin/pull/27118#issuecomment-1492606272.
This change would add a depends option such that, if someone wants to build with depends, for Windows, without hardening, they can do so. This may also be useful when building for debugging.
ACKs for top commit:
hebasto:
re-ACK 436df1e826cae036caed3e983715a4ed4e441321
Tree-SHA512: 5a3ef5ec87b10a5ad0a284201988ce94789451735c7c7e20d337f7232955b0b9a0addab1c3b5725755f00d8ce6741aa9c8cb5e3d48d926515b7dde46acdbcaa0
fa584b4d01ca055488a10b280f45dca323b474d4 test: Remove windows workaround in authproxy (MarcoFalke)
Pull request description:
I wonder if the windows issues have also been fixed by bumping the server timeout in commit 88134fcee998849d9569368d6cefbfa487dc82d1.
I guess the only way to find out and try.
Note that even with the workaround, the issue would still happen occasionally: https://github.com/bitcoin/bitcoin/issues/18623
ACKs for top commit:
fanquake:
ACK fa584b4d01ca055488a10b280f45dca323b474d4
Tree-SHA512: 1c633d7d8b54fa47bb851457d48b7d80bf6d6ea353db80610231e0bd385061b795f97ea2b64022fa24e79b4d7dcb5e947e0d637da228f10998e950d84b920878
73f4eb511cf80cf52b78627347727ca02774b731 Check that the Timestamp String is valid (John Moffett)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/27233
The current `FormatISO8601DateTime` function will return an empty string if it encounters an error when converting the `int64_t` seconds-since-epoch to a formatted date time. In the unlikely case that happens, here `strStamped.pop_back()` would be undefined behavior.
ACKs for top commit:
MarcoFalke:
lgtm ACK 73f4eb511cf80cf52b78627347727ca02774b731
stickies-v:
ACK 73f4eb511cf80cf52b78627347727ca02774b731
Tree-SHA512: 089ed639c193deb98870a8385039b31b4baed821ea907937bfc6f65a5b0981bbf8284b2afec81b2d0a922e2340716b48cf55349640eb6b8c311ef7af25abc361
9fbc5fcd28eeefd3ad1932a2d94e5558deeb16d7 build: remove ancient unused define (Cory Fields)
Pull request description:
The generic define was removed in [upstream miniupnpc in 2014](f6774e3316).
Noticed while reviewing hebasto's new CMake buildsystem: https://github.com/hebasto/bitcoin/pull/12#discussion_r1156267350.
ACKs for top commit:
RandyMcMillan:
utACK 9fbc5fc
hebasto:
ACK 9fbc5fcd28eeefd3ad1932a2d94e5558deeb16d7
TheCharlatan:
ACK 9fbc5fcd28eeefd3ad1932a2d94e5558deeb16d7
Tree-SHA512: 2075a2e63441e32d939c23f30f811c73dd1ab1ddcc9e6bf079cc8060e77d9a2a6d707add7b72e9251939a53b53e28a134e9bb330ccf3556244497728f1b091ae
54e406118926c2a526455a60f67961520dfb65da refactor: don't avoid sys/types.h on when building for Windows (fanquake)
Pull request description:
This is the only place these defines are used. They may also be available when building for Windows. `sys/stat.h` is available, and we already use it unguarded in other code. So move the defines into bdb, after the stat.h include, and remove compat from bdb.cpp.
ACKs for top commit:
TheCharlatan:
re-ACK 54e406118926c2a526455a60f67961520dfb65da
hebasto:
ACK 54e406118926c2a526455a60f67961520dfb65da, I have reviewed the code and it looks OK.
Tree-SHA512: b75bb120654b4dec9ccc83aa407ee1a50969fec92f196a3722ec51282b91ac50e455af04f07211f3e93270ab83660f1efdeef43928b44b1e4296f6b06ea807c8
f842ed9a40c0db656b86f85e84dd4978865cc0a0 test: refactor: replace unnecessary `BytesIO` uses (Sebastian Falbesoner)
Pull request description:
Rather than needing to create intermediate stream variables, we can use helper functions like `tx_from_hex` instead or access the result directly, leading both to increased readability and less code.
ACKs for top commit:
stickies-v:
ACK f842ed9a40c0db656b86f85e84dd4978865cc0a0
brunoerg:
crACK f842ed9a40c0db656b86f85e84dd4978865cc0a0
aureleoules:
ACK f842ed9a40c0db656b86f85e84dd4978865cc0a0 - It seems that these are the only instances that can be changed and it simplifies test code.
Tree-SHA512: 7f4fd7a26720d1988bf27f66c817ff6cd7060350a3be62d28e7848c768fd43578719ca475193d4057ccf4f2458af18564fd513fe3a1d458a11c799927c12bd65
Add an option that when passed, will disable hardening options, and
pass `--disable-hardening` through to configure. Due to the way
we link libssp for Windows builds, they now fail (after #27118),
if building with depends, and configuring with --disable-hardening.
See:
https://github.com/bitcoin/bitcoin/pull/27118#issuecomment-1492606272.
This change would add a depends opiton such that, if someone wants to
build with, for windows, without hardening, they can do so. This may
also be useful when building for debugging.
The following cases were covered:
Case 1: No coin control selected coins.
- 'useAvailableBalance' should fill the amount edit box with the total available balance.
Case 2: With coin control selected coins.
- 'useAvailableBalance' should fill the amount edit box with the sum of the selected coins values.
The previous behavior for getAvailableBalance when coin control
has selected coins was to return the sum of them. Instead, we
are currently returning the wallet's available total balance minus
the selected coins total amount.
This turns into a GUI-only issue for the "use available balance"
button when the user manually select coins in the send screen.
Reason:
We missed to update the GetAvailableBalance function to include
the coin control selected coins on #25685.
Context:
Since #25685 we skip the selected coins inside `AvailableCoins`,
the reason is that there is no need to traverse the wallet's
txes map just to get coins that can directly be fetched by
their id.
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan)
106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan)
b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan)
18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR #25152.
#### Context
There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/27238.
#### Changes
Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files.
There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory.
Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed.
ACKs for top commit:
hebasto:
ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3
Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
c9aace2bfd180906d528cc2a80bc856685fbc6ca depends, doc: Document `NO_USDT` option (Hennadii Stepanov)
Pull request description:
A follow-up for https://github.com/bitcoin/bitcoin/pull/23724.
This also removes a stray `</dd>` from the `NO_NATPMP` docs.
ACKs for top commit:
0xB10C:
ACK c9aace2bfd180906d528cc2a80bc856685fbc6ca
Tree-SHA512: 44426f5b2bf9eaa3b8a6c54020d23c97ab04f3d75bfa93469c2c5d3bd512fc214d82652e4558d9ad33e2b2e3605991cdbf7779ba47b10a50385d4c962a598b60
fae66fceb3147385320593d1e15faf290b0f4caf test: Remove python3.5 workaround in authproxy (MarcoFalke)
Pull request description:
Remove workaround for a bug that is long fixed in a EOL python version, that isn't used by us.
If the workaround is still needed, it should at least log the exception before silently discarding it, so that debugging is possible/easier.
ACKs for top commit:
fanquake:
ACK fae66fceb3147385320593d1e15faf290b0f4caf
Tree-SHA512: 9da28e495d530b9f9c5c75eff4982ef23b3775309e1f8d509722a9e7fd8b3535942c9a9cbd2d5e43e6487d46fdec4a63114aaa104e258c261cb98cb58560872a
3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3 [net processing] Pass TxRelay to FindTxForGetData instead of Peer (dergoegge)
c85ee76a3647a42d3a59c28444c5ca52e7db7c5e [net processin] Don't take cs_main in FindTxForGetData (dergoegge)
Pull request description:
Addresses left over feedback from #26140.
* https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153498543
* https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153499627
`mapRelay` is only accessed from the message processing thread and does not need to be kept in sync with anything validation specific, it is therfore perfectly fine to have it guarded by `g_msgproc_mutex`.
ACKs for top commit:
jnewbery:
utACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3
hebasto:
ACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 3ef84bfe4abfa8d991a7e65d9184221294d80e0df0bbb47f0270ab6ca1593266c98abf83c610f9f86b4d16c7a4b62bcf83f8856c68d3c2e10894bff6ed3e88cd
e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7 qt: Update translation source file (Hennadii Stepanov)
b7800950913c47bc1a939bd7f99c6b0629aea530 qt: Adjust plural forms for translations (Hennadii Stepanov)
6ae8a24009cb006476b3be612a34e4deda30433a GUI: Send: Make feerates translatable (Luke Dashjr)
bd42f5e1cdb0cbd829c8fe1700d036b681f3930d Bugfix: GUI: Send/PSBT: Correct virtual size unit and make translatable (Luke Dashjr)
1b0407f5f1f5902f67b58c7ada9c4b0c45457244 Bugfix: GUI: transactiondesc: Translate outlier "own address" and "watch-only" (Luke Dashjr)
170f3126f220f9ffe3db471e0024a23dede3f90b GUI: Use translated external signer errors for messagebox text (Luke Dashjr)
96989599d6c7264022c188babdcd9cacac9ab69f GUI: Make messages for copying unsigned PSBTs translatable (Luke Dashjr)
08b8b287d3762dd6d4be09fad0b71c4cacfe6658 Bugfix: GUI: Debug info: Use correct "kB" case for small mempool sizes, and make translation-friendly (Luke Dashjr)
dacc322be10c6709338d2f0d7f5c4366609ad1a0 GUI: PSBTOperationsDialog: Support translating window title (Luke Dashjr)
5a4fe5527045ad3d7f1c834c3c6ea1d531c87879 GUI: Intro: Support translating caption of data directory chooser (Luke Dashjr)
3868ba3a27d594dd2968548fff3db457cbeb0080 GUI: Support translating peer network names (Luke Dashjr)
f1f981119884389f1f355bc11847a73812a2836e GUI: Support translating address type dropdown entries (Luke Dashjr)
Pull request description:
This PR updates the `src/qt/locale/bitcoin_en.xlf` translation source file according to [Release schedule for 25.0](https://github.com/bitcoin/bitcoin/issues/26549).
Some translation-related fixes have been picked from https://github.com/bitcoin-core/gui/pull/599 and https://github.com/bitcoin-core/gui/pull/716.
Note for reviewers: it is expected to get a zero diff after running `make -C src translate` locally.
ACKs for top commit:
jarolrod:
ACK e414edd8fc88b55a91f446dcc4d55cc4cd6d32e7
Tree-SHA512: 5b0c70db1e2f5871067e84f43ebea4ee4f0027fc5f2be49bbcb1d04e162ae76607b2b038e9d0622bcb5b3658d0ede8c10c4421ddaa3343f0e0be54315ca7a4f5
Rather than needing to create intermediate stream variables, we can use
helper functions like `tx_from_hex` instead or access the result
directly, leading both to increased readability and less code.
1869310f3cfa4ab26b5090d8a4002eefdc84870e refactor: remove unused param from legacy pubkey (Bushstar)
Pull request description:
Unused param present in legacy pubkey manager interface. This param will not be used and should be removed to prevent unintended usage.
ACKs for top commit:
Sjors:
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e
furszy:
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e
Tree-SHA512: 0fb41fc8f481f859262f2e8e9a93c990c1b4637e74fd9191ccc0b3c523d0e7d94217a3074bb357276e1941a10d29326f850f9b27eccc1eca57cf6b549353400c
Also, move the burden of checking for a timeout to the client and
disable the timeout on the server. This should avoid intermittent issues
in slow tests (for example mining_getblocktemplate_longpoll.py, or
feature_dbcrash.py), or possibly when the server is running slow (for
example in valgrind). There shouldn't be any downside in tests caused
by a high rpcservertimeout.
fafe3a8e38bdb6d12b2654b066b8036ba3e9455d ci: Remove second user account (MarcoFalke)
Pull request description:
The rationale for the second (nonroot) account no longer applies. See also https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148898438
ACKs for top commit:
josibake:
utACK fafe3a8e38
Tree-SHA512: 4464e50bc6499e012ff33c591f20c5989b99e223c476cefea669aedb79a0e731734db9469d4c7d800642191b43b57ea32cda187093395f100dec719fea17724d
