Skip to content

Commit

Permalink
wallet: Support %w in -walletnotify script, on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
promag authored and luke-jr committed May 29, 2024
1 parent 6c7e9e9 commit 2bad09b
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 19 deletions.
4 changes: 1 addition & 3 deletions src/common/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@
// 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)
Expand Down
2 changes: 0 additions & 2 deletions src/common/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,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
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,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);

Expand Down
9 changes: 0 additions & 9 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1149,16 +1149,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>()) {
Expand Down
8 changes: 4 additions & 4 deletions test/functional/feature_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@

# 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):
Expand All @@ -33,7 +33,7 @@ def set_test_params(self):
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")
Expand Down

0 comments on commit 2bad09b

Please sign in to comment.