Merge walletnotify_w_win-27+knots

src/common/system.cpp

@@ -37,14 +37,12 @@ using util::ReplaceAll;
 // Application startup time (used for uptime calculation)
 const int64_t nStartupTime = GetTime();
 
-#ifndef WIN32
 std::string ShellEscape(const std::string& arg)
 {
     std::string escaped = arg;
-    ReplaceAll(escaped, "'", "'\"'\"'");
+    ReplaceAll(escaped, "'", "'\\''");
     return "'" + escaped + "'";
 }
-#endif
 
 #if HAVE_SYSTEM
 void runCommand(const std::string& strCommand)

src/common/system.h

@@ -16,9 +16,7 @@ int64_t GetStartupTime();
 
 void SetupEnvironment();
 [[nodiscard]] bool SetupNetworking();
-#ifndef WIN32
 std::string ShellEscape(const std::string& arg);
-#endif
 #if HAVE_SYSTEM
 void runCommand(const std::string& strCommand);
 #endif

src/wallet/init.cpp

@@ -77,7 +77,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-walletbroadcast",  strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-walletdir=<dir>", "Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
 #if HAVE_SYSTEM
-    argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
+    argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
 #endif
     argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
 

src/wallet/wallet.cpp

@@ -1232,16 +1232,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
 #if HAVE_SYSTEM
     // notify an external script when a wallet transaction comes in or is updated
     if (!m_notify_tx_changed_scripts.empty()) {
-#ifdef WIN32
-        // Substituting the wallet name isn't currently supported on windows
-        // because windows shell escaping has not been implemented yet:
-        // https://github.com/bitcoin/bitcoin/pull/13339#issuecomment-537384875
-        // A few ways it could be implemented in the future are described in:
-        // https://github.com/bitcoin/bitcoin/pull/13339#issuecomment-461288094
-        const std::string walletname_escaped = "wallet_name_substitution_is_not_available_on_Windows";
-#else
         const std::string walletname_escaped = ShellEscape(GetName());
-#endif
         const std::string txid_hex = hash.GetHex();
         std::string blockhash_hex, blockheight_str;
         if (auto* conf = wtx.state<TxStateConfirmed>()) {

test/functional/feature_notifications.py

@@ -15,13 +15,13 @@ from test_framework.util import (
 
 # Linux allow all characters other than \x00
 # Windows disallow control characters (0-31) and /\?%:|"<>
-FILE_CHAR_START = 32 if platform.system() == 'Windows' else 1
+FILE_CHAR_START = 43 if platform.system() == 'Windows' else 1
 FILE_CHAR_END = 128
-FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/'
+FILE_CHARS_DISALLOWED = '/:;<>?@[\\]^`{|}' if platform.system() == 'Windows' else '/'
 UNCONFIRMED_HASH_STRING = 'unconfirmed'
 
 def notify_outputname(walletname, txid):
-    return txid if platform.system() == 'Windows' else f'{walletname}_{txid}'
+    return f'{walletname}_{txid}'
 
 
 class NotificationsTest(BitcoinTestFramework):
@@ -33,7 +33,7 @@ class NotificationsTest(BitcoinTestFramework):
         self.setup_clean_chain = True
 
     def setup_network(self):
-        self.wallet = ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHARS_DISALLOWED)
+        self.wallet = 'wallet' + ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHARS_DISALLOWED)
         self.alertnotify_dir = os.path.join(self.options.tmpdir, "alertnotify")
         self.blocknotify_dir = os.path.join(self.options.tmpdir, "blocknotify")
         self.walletnotify_dir = os.path.join(self.options.tmpdir, "walletnotify")