feat: Add -chainlocknotify cmd-line option, update -instantsendnotify (#5522)

## Issue being fixed or feature implemented
Execute command when the best chainlock changes (`%s` in cmd is replaced
by chainlocked block hash). Same as `-blocknotify` but for chainlocks.
Let `-instantsendnotify` replace `%w` with wallet name like
`-walletnotify` does.

## What was done?

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

src/init.cpp

@@ -529,6 +529,9 @@ void SetupServerArgs(NodeContext& node)
 #endif
     argsman.AddArg("-blockreconstructionextratxn=<n>", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless the peer has the 'forcerelay' permission. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+#if HAVE_SYSTEM
+    argsman.AddArg("-chainlocknotify=<cmd>", "Execute command when the best chainlock changes (%s in cmd is replaced by chainlocked block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+#endif
     argsman.AddArg("-coinstatsindex", strprintf("Maintain coinstats index used by the gettxoutset RPC (default: %u)", DEFAULT_COINSTATSINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-conf=<file>", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -2368,6 +2371,18 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
         };
         uiInterface.NotifyBlockTip_connect(BlockNotifyCallback);
     }
+    if (args.IsArgSet("-chainlocknotify")) {
+        const std::string chainlock_notify = args.GetArg("-chainlocknotify", "");
+        const auto ChainlockNotifyCallback = [chainlock_notify](const std::string& bestChainLockHash, int bestChainLockHeight) {
+            std::string strCmd = chainlock_notify;
+            if (!strCmd.empty()) {
+                ReplaceAll(strCmd, "%s", bestChainLockHash);
+                std::thread t(runCommand, strCmd);
+                t.detach(); // thread runs free
+            }
+        };
+        uiInterface.NotifyChainLock_connect(ChainlockNotifyCallback);
+    }
 #endif
 
     std::vector<fs::path> vImportFiles;

src/wallet/init.cpp

@@ -58,7 +58,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-createwalletbackups=<n>", strprintf("Number of automatic wallet backups (default: %u)", nWalletBackups), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-disablewallet", "Do not load the wallet and disable wallet RPC calls", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
 #if HAVE_SYSTEM
-    argsman.AddArg("-instantsendnotify=<cmd>", "Execute command when a wallet InstantSend transaction is successfully locked (%s in cmd is replaced by TxID)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
+    argsman.AddArg("-instantsendnotify=<cmd>", "Execute command when a wallet InstantSend transaction is successfully locked. %s in cmd is replaced by TxID and %w is replaced by wallet name. %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);
 #endif
     argsman.AddArg("-keypool=<n>", strprintf("Set key pool size to <n> (default: %u). Warning: Smaller sizes may increase the risk of losing funds when restoring from an old backup, if none of the addresses in the original keypool have been used.", DEFAULT_KEYPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-rescan=<mode>", "Rescan the block chain for missing wallet transactions on startup"

src/wallet/wallet.cpp

@@ -5056,6 +5056,14 @@ void CWallet::notifyTransactionLock(const CTransactionRef &tx, const std::shared
         std::string strCmd = gArgs.GetArg("-instantsendnotify", "");
         if (!strCmd.empty()) {
             ReplaceAll(strCmd, "%s", txHash.GetHex());
+#ifndef 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
+            ReplaceAll(strCmd, "%w", ShellEscape(GetName()));
+#endif
             std::thread t(runCommand, strCmd);
             t.detach(); // thread runs free
         }

test/functional/feature_notifications.py

@@ -1,14 +1,16 @@
 #!/usr/bin/env python3
 # Copyright (c) 2014-2016 The Bitcoin Core developers
+# Copyright (c) 2023 The Dash Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test the -alertnotify, -blocknotify and -walletnotify options."""
+"""Test the -alertnotify, -blocknotify, -chainlocknotify, -instantsendnotify and -walletnotify options."""
 import os
 
 from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE
-from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_framework import DashTestFramework
 from test_framework.util import (
     assert_equal,
+    force_finish_mnsync,
     wait_until,
 )
 
@@ -23,36 +25,45 @@ def notify_outputname(walletname, txid):
     return txid if os.name == 'nt' else '{}_{}'.format(walletname, txid)
 
 
-class NotificationsTest(BitcoinTestFramework):
+class NotificationsTest(DashTestFramework):
     def set_test_params(self):
-        self.num_nodes = 2
+        self.set_dash_test_params(5, 3, fast_dip3_enforcement=True)
-        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.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")
+        self.chainlocknotify_dir = os.path.join(self.options.tmpdir, "chainlocknotify")
+        self.instantsendnotify_dir = os.path.join(self.options.tmpdir, "instantsendnotify")
         os.mkdir(self.alertnotify_dir)
         os.mkdir(self.blocknotify_dir)
         os.mkdir(self.walletnotify_dir)
+        os.mkdir(self.chainlocknotify_dir)
+        os.mkdir(self.instantsendnotify_dir)
 
         # -alertnotify and -blocknotify on node0, walletnotify on node1
-        self.extra_args = [[
+        self.extra_args[0].append("-alertnotify=echo > {}".format(os.path.join(self.alertnotify_dir, '%s')))
-                            "-alertnotify=echo > {}".format(os.path.join(self.alertnotify_dir, '%s')),
+        self.extra_args[0].append("-blocknotify=echo > {}".format(os.path.join(self.blocknotify_dir, '%s')))
-                            "-blocknotify=echo > {}".format(os.path.join(self.blocknotify_dir, '%s'))],
+        self.extra_args[1].append("-blockversion=211")
-                           ["-blockversion=211",
+        self.extra_args[1].append("-rescan")
-                            "-rescan",
+        self.extra_args[1].append("-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))))
-                            "-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s')))]]
+
-        self.wallet_names = [self.default_wallet_name, self.wallet]
+        # -chainlocknotify on node0, -instantsendnotify on node1
+        self.extra_args[0].append("-chainlocknotify=echo > {}".format(os.path.join(self.chainlocknotify_dir, '%s')))
+        self.extra_args[1].append("-instantsendnotify=echo > {}".format(os.path.join(self.instantsendnotify_dir, notify_outputname('%w', '%s'))))
+
         super().setup_network()
 
     def run_test(self):
+        # remove files created during network setup
+        for block_file in os.listdir(self.blocknotify_dir):
+            os.remove(os.path.join(self.blocknotify_dir, block_file))
+        for tx_file in os.listdir(self.walletnotify_dir):
+            os.remove(os.path.join(self.walletnotify_dir, tx_file))
+
         if self.is_wallet_compiled():
-            # Make the wallets
+            self.nodes[1].createwallet(wallet_name=self.wallet, load_on_startup=True)
-            # Ensures that node 0 and node 1 share the same wallet for the conflicting transaction tests below.
-            for i, name in enumerate(self.wallet_names):
-                self.nodes[i].createwallet(wallet_name=name, load_on_startup=True)
 
         self.log.info("test -blocknotify")
         block_count = 10
@@ -80,6 +91,7 @@ class NotificationsTest(BitcoinTestFramework):
             self.log.info("test -walletnotify after rescan")
             # restart node to rescan to force wallet notifications
             self.start_node(1)
+            force_finish_mnsync(self.nodes[1])
             self.connect_nodes(0, 1)
 
             wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10)
@@ -88,6 +100,39 @@ class NotificationsTest(BitcoinTestFramework):
             txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count)))
             assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir)))
 
+        self.log.info("test -chainlocknotify")
+
+        self.activate_dip8()
+        self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)
+        self.wait_for_sporks_same()
+        self.mine_quorum()
+        self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 0)
+        self.wait_for_sporks_same()
+
+        self.log.info("Mine single block, wait for chainlock")
+        self.bump_mocktime(1)
+        tip = self.nodes[0].generate(1)[-1]
+        self.wait_for_chainlocked_block_all_nodes(tip)
+        # directory content should equal the chainlocked block hash
+        assert_equal([tip], sorted(os.listdir(self.chainlocknotify_dir)))
+
+        if self.is_wallet_compiled():
+            self.log.info("test -instantsendnotify")
+            assert_equal(len(os.listdir(self.instantsendnotify_dir)), 0)
+
+            tx_count = 10
+            for _ in range(tx_count):
+                txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1)
+                self.wait_for_instantlock(txid, self.nodes[1])
+
+            # wait at most 10 seconds for expected number of files before reading the content
+            wait_until(lambda: len(os.listdir(self.instantsendnotify_dir)) == tx_count, timeout=10)
+
+            # directory content should equal the generated transaction hashes
+            txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", tx_count)))
+            assert_equal(sorted(txids_rpc), sorted(os.listdir(self.instantsendnotify_dir)))
+
         # TODO: add test for `-alertnotify` large fork notifications
 
 if __name__ == '__main__':

test/functional/p2p_instantsend.py

@@ -24,6 +24,9 @@ class InstantSendTest(DashTestFramework):
         self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
         self.mine_quorum()
+        self.nodes[self.isolated_idx].createwallet(self.default_wallet_name)
+        self.nodes[self.receiver_idx].createwallet(self.default_wallet_name)
+        self.nodes[self.sender_idx].createwallet(self.default_wallet_name)
 
         self.test_mempool_doublespend()
         self.test_block_doublespend()

test/functional/test_framework/test_framework.py

@@ -1106,7 +1106,6 @@ class DashTestFramework(BitcoinTestFramework):
         idx = len(self.nodes)
         self.add_nodes(1, extra_args=[self.extra_args[idx]])
         self.start_node(idx)
-        self.nodes[idx].createwallet(self.default_wallet_name)
         for i in range(0, idx):
             self.connect_nodes(i, idx)