init: Allow -proxy="" setting values
ClosedPublic
Actions

Authored by PiRK on Jun 16 2025, 13:11.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC23736ac049b1: init: Allow -proxy="" setting values

Summary

This drops the No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>
error when a empty -proxy= command line argument, bitcoin.conf value, or
settings.json value is specified, and just makes bitcoin connect and listen
normally in these cases.

The error was originally added in https://github.com/bitcoin/bitcoin/pull/20003
to prevent a bare -proxy command line argument with no assignment from
clearing proxy settings. But it was implemented in an overbroad way breaking
empty -proxy= assignments as well.

The motivation for this change is to prevent a GUI bug that happens with
https://github.com/bitcoin/bitcoin/pull/15936, reported in
https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-937685759 by
vasild, that happens after a proxy setting is enabled and disabled in the GUI.
But this change also makes sense on its own to remove a potentially confusing
error message.

See D4496 and D8637 for changes related to ParseKeyValue

This concludes backport of core#24830
Depends on D18260

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Jun 16 2025, 13:11

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 16 2025, 13:11

PiRK requested review of this revision.Jun 16 2025, 13:11

PiRK edited the summary of this revision. (Show Details)Jun 16 2025, 13:12

Harbormaster completed remote builds in B33638: Diff 54497.Jun 16 2025, 13:16

Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

2025-06-16T13:16:05.947780Z [/work/src/random.cpp:106] [ReportHardwareRand] Using RdSeed as additional entropy source
2025-06-16T13:16:05.947791Z [/work/src/random.cpp:109] [ReportHardwareRand] Using RdRand as an additional entropy source
2025-06-16T13:16:05.956725Z [/work/src/script/sigcache.cpp:94] [InitSignatureCache] Using 32 MiB out of 32 MiB requested for signature cache, able to store 1048576 elements
2025-06-16T13:16:05.957437Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements
2025-06-16T13:16:05.958295Z [/work/src/test/util/random.cpp:39] [Seed] Seed: Setting random seed for current tests to RANDOM_CTX_SEED=def0795c5ffceefcc904e2ad82d69b0f58b6813e3a3a16cbd08699b7b544a84f
2025-06-16T13:16:05.958328Z [/work/src/init/common.cpp:199] [LogPackageVersion] Bitcoin ABC version v0.31.6-d347f91ae9a3 (release build)
2025-06-16T13:16:05.958477Z [/work/src/kernel/context.cpp:19] [Context] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2025-06-16T13:16:05.958497Z [/work/src/random.cpp:106] [ReportHardwareRand] Using RdSeed as additional entropy source
2025-06-16T13:16:05.958517Z [/work/src/random.cpp:109] [ReportHardwareRand] Using RdRand as an additional entropy source
2025-06-16T13:16:05.967679Z [/work/src/script/sigcache.cpp:94] [InitSignatureCache] Using 32 MiB out of 32 MiB requested for signature cache, able to store 1048576 elements
2025-06-16T13:16:05.968707Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements
2025-06-16T13:16:05.969557Z [/work/src/test/util/random.cpp:39] [Seed] Seed: Setting random seed for current tests to RANDOM_CTX_SEED=def0795c5ffceefcc904e2ad82d69b0f58b6813e3a3a16cbd08699b7b544a84f
2025-06-16T13:16:05.969578Z [/work/src/init/common.cpp:199] [LogPackageVersion] Bitcoin ABC version v0.31.6-d347f91ae9a3 (release build)
2025-06-16T13:16:05.969739Z [/work/src/kernel/context.cpp:19] [Context] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2025-06-16T13:16:05.969759Z [/work/src/random.cpp:106] [ReportHardwareRand] Using RdSeed as additional entropy source
2025-06-16T13:16:05.969774Z [/work/src/random.cpp:109] [ReportHardwareRand] Using RdRand as an additional entropy source
2025-06-16T13:16:05.978861Z [/work/src/script/sigcache.cpp:94] [InitSignatureCache] Using 32 MiB out of 32 MiB requested for signature cache, able to store 1048576 elements
2025-06-16T13:16:05.979489Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements
2025-06-16T13:16:05.980615Z [/work/src/test/util/random.cpp:39] [Seed] Seed: Setting random seed for current tests to RANDOM_CTX_SEED=def0795c5ffceefcc904e2ad82d69b0f58b6813e3a3a16cbd08699b7b544a84f
2025-06-16T13:16:05.980640Z [/work/src/init/common.cpp:199] [LogPackageVersion] Bitcoin ABC version v0.31.6-d347f91ae9a3 (release build)
2025-06-16T13:16:05.980815Z [/work/src/kernel/context.cpp:19] [Context] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2025-06-16T13:16:05.980830Z [/work/src/random.cpp:106] [ReportHardwareRand] Using RdSeed as additional entropy source
2025-06-16T13:16:05.980840Z [/work/src/random.cpp:109] [ReportHardwareRand] Using RdRand as an additional entropy source
2025-06-16T13:16:05.989743Z [/work/src/script/sigcache.cpp:94] [InitSignatureCache] Using 32 MiB out of 32 MiB requested for signature cache, able to store 1048576 elements
2025-06-16T13:16:05.990378Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
[497/528] bitcoin: testing validation_tests
[498/528] bitcoin: testing scriptpubkeyman_tests
[499/528] Running utility command for check-bitcoin-validationinterface_tests
[500/528] bitcoin: testing txrequest_tests
[501/528] Running utility command for check-bitcoin-validation_tests
[502/528] Running utility command for check-bitcoin-txrequest_tests
[503/528] bitcoin: testing txvalidationcache_tests
[504/528] Running utility command for check-bitcoin-scriptpubkeyman_tests
[505/528] Running utility command for check-bitcoin-txvalidationcache_tests
[506/528] bitcoin: testing walletdb_tests
[507/528] Running utility command for check-bitcoin-walletdb_tests
[508/528] Linking CXX executable src/qt/test/test_bitcoin-qt
[509/528] bitcoin: testing psbt_wallet_tests
[510/528] Running utility command for check-bitcoin-psbt_wallet_tests
[511/528] bitcoin: testing validation_block_tests
[512/528] Running utility command for check-bitcoin-validation_block_tests
[513/528] bitcoin: testing validation_chainstatemanager_tests
[514/528] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[515/528] bitcoin: testing wallet_crypto_tests
[516/528] Running utility command for check-bitcoin-wallet_crypto_tests
[517/528] bitcoin-qt: testing test_bitcoin-qt
[518/528] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[519/528] bitcoin: testing transaction_tests
[520/528] Running utility command for check-bitcoin-transaction_tests
[521/528] bitcoin: testing wallet_tests
[522/528] Running utility command for check-bitcoin-wallet_tests
[523/528] bitcoin: testing coins_tests
[524/528] Running utility command for check-bitcoin-coins_tests
[525/528] bitcoin: testing coinselector_tests
[526/528] Running utility command for check-bitcoin-coinselector_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

2025-06-16T13:19:48.679193Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements
2025-06-16T13:19:48.680306Z [/work/src/test/util/random.cpp:39] [Seed] Seed: Setting random seed for current tests to RANDOM_CTX_SEED=8b1f9fdea3652573488f2cf42a8d075ff4b298c91bc5ba54a6ca2cdb3a0bd804
2025-06-16T13:19:48.680339Z [/work/src/init/common.cpp:199] [LogPackageVersion] Bitcoin ABC version v0.31.6-d347f91ae9a3 (release build)
2025-06-16T13:19:48.680507Z [/work/src/kernel/context.cpp:19] [Context] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2025-06-16T13:19:48.680525Z [/work/src/random.cpp:106] [ReportHardwareRand] Using RdSeed as additional entropy source
2025-06-16T13:19:48.680536Z [/work/src/random.cpp:109] [ReportHardwareRand] Using RdRand as an additional entropy source
2025-06-16T13:19:48.689556Z [/work/src/script/sigcache.cpp:94] [InitSignatureCache] Using 32 MiB out of 32 MiB requested for signature cache, able to store 1048576 elements
2025-06-16T13:19:48.690279Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements
2025-06-16T13:19:48.691102Z [/work/src/test/util/random.cpp:39] [Seed] Seed: Setting random seed for current tests to RANDOM_CTX_SEED=8b1f9fdea3652573488f2cf42a8d075ff4b298c91bc5ba54a6ca2cdb3a0bd804
2025-06-16T13:19:48.691134Z [/work/src/init/common.cpp:199] [LogPackageVersion] Bitcoin ABC version v0.31.6-d347f91ae9a3 (release build)
2025-06-16T13:19:48.691289Z [/work/src/kernel/context.cpp:19] [Context] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2025-06-16T13:19:48.691305Z [/work/src/random.cpp:106] [ReportHardwareRand] Using RdSeed as additional entropy source
2025-06-16T13:19:48.691317Z [/work/src/random.cpp:109] [ReportHardwareRand] Using RdRand as an additional entropy source
2025-06-16T13:19:48.700128Z [/work/src/script/sigcache.cpp:94] [InitSignatureCache] Using 32 MiB out of 32 MiB requested for signature cache, able to store 1048576 elements
2025-06-16T13:19:48.700804Z [/work/src/script/scriptcache.cpp:93] [InitScriptExecutionCache] Using 32 MiB out of 32 MiB requested for script execution cache, able to store 1048576 elements

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
[481/521] bitcoin: testing txrequest_tests
[482/521] bitcoin: testing txvalidationcache_tests
[483/521] bitcoin: testing validation_tests
[484/521] bitcoin: testing init_tests
[485/521] bitcoin: testing ismine_tests
[486/521] Running utility command for check-bitcoin-validationinterface_tests
[487/521] Running utility command for check-bitcoin-txrequest_tests
[488/521] Running utility command for check-bitcoin-ismine_tests
[489/521] Running utility command for check-bitcoin-init_tests
[490/521] Running utility command for check-bitcoin-txvalidationcache_tests
[491/521] Running utility command for check-bitcoin-validation_tests
[492/521] Running utility command for check-pow-aserti32d_tests
[493/521] Running utility command for check-pow-grasberg_tests
[494/521] Running pow test suite
PASSED: pow test suite
[495/521] bitcoin: testing scriptpubkeyman_tests
[496/521] Running utility command for check-bitcoin-scriptpubkeyman_tests
[497/521] bitcoin: testing walletdb_tests
[498/521] Running utility command for check-bitcoin-walletdb_tests
[499/521] bitcoin: testing validation_block_tests
[500/521] Running utility command for check-bitcoin-validation_block_tests
[501/521] bitcoin: testing psbt_wallet_tests
[502/521] Running utility command for check-bitcoin-psbt_wallet_tests
[503/521] bitcoin: testing wallet_crypto_tests
[504/521] Running utility command for check-bitcoin-wallet_crypto_tests
[505/521] bitcoin: testing coins_tests
[506/521] Running utility command for check-bitcoin-coins_tests
[507/521] bitcoin: testing transaction_tests
[508/521] Running utility command for check-bitcoin-transaction_tests
[509/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[510/521] bitcoin: testing wallet_tests
[511/521] Running utility command for check-bitcoin-wallet_tests
[512/521] bitcoin: testing coinselector_tests
[513/521] bitcoin: testing validation_chainstatemanager_tests
[514/521] Running utility command for check-bitcoin-coinselector_tests
[515/521] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[516/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[517/521] Linking CXX executable src/qt/test/test_bitcoin-qt
[518/521] bitcoin-qt: testing test_bitcoin-qt
[519/521] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

hard to see what the error is, in the logs of the clang build. Will have to reproduce locally

There is something weird happening with clang. The test failures show that sometimes the value is the one from the previous test when we expect it to be the empty string. And the previous test does not reuse the same var, so somehow clang does not initialize std::optional<std::string> to std::nullopt but jsut reuses the same memory buffer and keeps the data to initialize the value var.

/work/src/test/util_tests.cpp(592): error: in "util_tests/util_ParseKeyValue": check *value == "" has failed [v != ]
/work/src/test/util_tests.cpp(606): error: in "util_tests/util_ParseKeyValue": check *value == "" has failed [1 != ]

fix test by not dereferencing std::nullopt, use .value() in that test instead of the deref operator so that we get an error is value is unexpectedly std::nullopt

Harbormaster completed remote builds in B33642: Diff 54504.Jun 17 2025, 06:52

Fabien added a subscriber: Fabien.Jun 17 2025, 08:26

Fabien added inline comments.

src/common/args.h
88 ↗	(On Diff #54504)	I don't get it. One one hand you have a std::optional and on the other hand you have a pointer, requiring a conversion in between with no benefit afaict. Why not use the std::optional (that convey the api behavior btw, it's self documenting) everywhere ?

PiRK added inline comments.Jun 17 2025, 13:16

src/common/args.h
88 ↗	(On Diff #54504)	I think I agree with you, though I generally try to avoid having an opinion on design choices when it is a backport of code that we are unlikely to touch very often in the future. The std::optional option was discussed in the review of the original PR: https://github.com/bitcoin/bitcoin/pull/24830/files#r849563238 The only rationale that was mentioned is related to performance: I generally don't like making copies of things that can be pointed to, especially since val in this case is a user-supplied value that doesn't have a predetermined size". I don't think it is a very good rationale, because users are unlikely to DoS themselves via command lines args.