foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#24830: init: Allow -proxy="" setting values

Browse source 
1d4122dfef init: Allow -proxy="" setting values (Ryan Ofsky)

Pull request description:

  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.

ACKs for top commit:
  hebasto:
    re-ACK 1d4122dfef, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/24830#pullrequestreview-941255672).

Tree-SHA512: 753adfce199ed078a6cd9e0ea78e76c0b14070f8fcfe2a4632cd0c6dfe6b4e135ddffbe11a97e5e30520ea9e5bda00bad1493cbaef74cf425aa8613249167f53
This commit is contained in:
MacroFake 2022-05-20 08:28:02 +01:00
commit 4a8709821e
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
4 changed files with 14 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/init.cpp
View file

@ -471,7 +471,7 @@ void SetupServerArgs(ArgsManager& argsman)
// TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from
// https://github.com/bitcoin/bitcoin/pull/23542 have become widespread.
argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port>. Nodes not using the default ports (default: %u, testnet: %u, signet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION);
argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@ -1029,10 +1029,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
nMaxTipAge = args.GetIntArg("-maxtipage", DEFAULT_MAX_TIP_AGE);
if (args.IsArgSet("-proxy") && args.GetArg("-proxy", "").empty()) {
return InitError(_("No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>."));
}
if (args.GetBoolArg("-reindex-chainstate", false)) {
// indexes that must be deactivated to prevent index corruption, see #24630
if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) {

18
src/util/system.cpp
View file

@ -236,7 +236,7 @@ KeyInfo InterpretKey(std::string key)
* @return parsed settings value if it is valid, otherwise nullopt accompanied
* by a descriptive error string
*/
static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string& value,
static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value,
unsigned int flags, std::string& error)
{
// Return negated settings as false values.
@ -246,13 +246,17 @@ static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, con
return std::nullopt;
}
// Double negatives like -nofoo=0 are supported (but discouraged)
if (!InterpretBool(value)) {
LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key.name, value);
if (value && !InterpretBool(*value)) {
LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key.name, *value);
return true;
}
return false;
}
return value;
if (!value && (flags & ArgsManager::DISALLOW_ELISION)) {
error = strprintf("Can not set -%s with no value. Please specify value with -%s=value.", key.name, key.name);
return std::nullopt;
}
return value ? *value : "";
}
// Define default constructor and destructor that are not inline, so code instantiating this class doesn't need to
@ -320,7 +324,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
#endif
if (key == "-") break; //bitcoin-tx using stdin
std::string val;
std::optional<std::string> val;
size_t is_index = key.find('=');
if (is_index != std::string::npos) {
val = key.substr(is_index + 1);
@ -366,7 +370,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
return false;
}
std::optional<util::SettingsValue> value = InterpretValue(keyinfo, val, *flags, error);
std::optional<util::SettingsValue> value = InterpretValue(keyinfo, val ? &*val : nullptr, *flags, error);
if (!value) return false;
m_settings.command_line_options[keyinfo.name].push_back(*value);
@ -887,7 +891,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file
KeyInfo key = InterpretKey(option.first);
std::optional<unsigned int> flags = GetArgFlags('-' + key.name);
if (flags) {
std::optional<util::SettingsValue> value = InterpretValue(key, option.second, *flags, error);
std::optional<util::SettingsValue> value = InterpretValue(key, &option.second, *flags, error);
if (!value) {
return false;
}

1
src/util/system.h
View file

@ -175,6 +175,7 @@ public:
// ALLOW_STRING = 0x08, //!< unimplemented, draft implementation in #16545
// ALLOW_LIST = 0x10, //!< unimplemented, draft implementation in #16545
DISALLOW_NEGATION = 0x20, //!< disallow -nofoo syntax
DISALLOW_ELISION = 0x40, //!< disallow -foo syntax that doesn't assign any value
DEBUG_ONLY = 0x100,
/* Some options would cause cross-contamination if values for

2
test/functional/feature_config_args.py
View file

@ -85,7 +85,7 @@ class ConfArgsTest(BitcoinTestFramework):
def test_invalid_command_line_options(self):
self.nodes[0].assert_start_raises_init_error(
expected_msg='Error: No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>.',
expected_msg='Error: Error parsing command line arguments: Can not set -proxy with no value. Please specify value with -proxy=value.',
extra_args=['-proxy'],
)