Add new format string placeholders for walletnotify to include relevant block information for transactions

Author: maayank

src/wallet/init.cpp

@@ -70,7 +70,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 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);
+    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);
 #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

@@ -944,6 +944,14 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio
     if (!strCmd.empty())
     {
         boost::replace_all(strCmd, "%s", hash.GetHex());
+        if (confirm.status == CWalletTx::Status::CONFIRMED)
+        {
+            boost::replace_all(strCmd, "%b", confirm.hashBlock.GetHex());
+            boost::replace_all(strCmd, "%h", ToString(confirm.block_height));
+        } else {
+            boost::replace_all(strCmd, "%b", "unconfirmed");
+            boost::replace_all(strCmd, "%h", "-1");
+        }
 #ifndef WIN32
         // Substituting the wallet name isn't currently supported on windows
         // because windows shell escaping has not been implemented yet:

test/functional/feature_notifications.py

@@ -17,7 +17,7 @@
 FILE_CHAR_START = 32 if os.name == 'nt' else 1
 FILE_CHAR_END = 128
 FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if os.name == 'nt' else '/'
-
+UNCONFIRMED_HASH_STRING = 'unconfirmed'
 
 def notify_outputname(walletname, txid):
     return txid if os.name == 'nt' else '{}_{}'.format(walletname, txid)
@@ -43,7 +43,7 @@ def setup_network(self):
             "-blocknotify=echo > {}".format(os.path.join(self.blocknotify_dir, '%s')),
         ], [
             "-rescan",
-            "-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))),
+            "-walletnotify=echo %h_%b > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))),
         ]]
         self.wallet_names = [self.default_wallet_name, self.wallet]
         super().setup_network()
@@ -90,11 +90,9 @@ def run_test(self):
             self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_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("*", block_count)))
-            assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir)))
+            tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count)))
             self.stop_node(1)
-            for tx_file in os.listdir(self.walletnotify_dir):
-                os.remove(os.path.join(self.walletnotify_dir, tx_file))
+            self.expect_wallet_notify(tx_details)
 
             self.log.info("test -walletnotify after rescan")
             # restart node to rescan to force wallet notifications
@@ -104,10 +102,8 @@ def run_test(self):
             self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_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("*", block_count)))
-            assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir)))
-            for tx_file in os.listdir(self.walletnotify_dir):
-                os.remove(os.path.join(self.walletnotify_dir, tx_file))
+            tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count)))
+            self.expect_wallet_notify(tx_details)
 
             # Conflicting transactions tests.
             # Generate spends from node 0, and check notifications
@@ -122,7 +118,7 @@ def run_test(self):
             tx1 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True)
             assert_equal(tx1 in self.nodes[0].getrawmempool(), True)
             self.sync_mempools()
-            self.expect_wallet_notify([tx1])
+            self.expect_wallet_notify([(tx1, -1, UNCONFIRMED_HASH_STRING)])
 
             # Generate bump transaction, sync mempools, and check for bump1
             # notification. In the future, per
@@ -131,39 +127,57 @@ def run_test(self):
             bump1 = self.nodes[0].bumpfee(tx1)["txid"]
             assert_equal(bump1 in self.nodes[0].getrawmempool(), True)
             self.sync_mempools()
-            self.expect_wallet_notify([bump1])
+            self.expect_wallet_notify([(bump1, -1, UNCONFIRMED_HASH_STRING)])
 
             # Add bump1 transaction to new block, checking for a notification
             # and the correct number of confirmations.
-            self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
+            blockhash1 = self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)[0]
+            blockheight1 = self.nodes[0].getblockcount()
             self.sync_blocks()
-            self.expect_wallet_notify([bump1])
+            self.expect_wallet_notify([(bump1, blockheight1, blockhash1)])
             assert_equal(self.nodes[1].gettransaction(bump1)["confirmations"], 1)
 
             # Generate a second transaction to be bumped.
             tx2 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True)
             assert_equal(tx2 in self.nodes[0].getrawmempool(), True)
             self.sync_mempools()
-            self.expect_wallet_notify([tx2])
+            self.expect_wallet_notify([(tx2, -1, UNCONFIRMED_HASH_STRING)])
 
             # Bump tx2 as bump2 and generate a block on node 0 while
             # disconnected, then reconnect and check for notifications on node 1
             # about newly confirmed bump2 and newly conflicted tx2.
             self.disconnect_nodes(0, 1)
             bump2 = self.nodes[0].bumpfee(tx2)["txid"]
-            self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
+            blockhash2 = self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)[0]
+            blockheight2 = self.nodes[0].getblockcount()
             assert_equal(self.nodes[0].gettransaction(bump2)["confirmations"], 1)
             assert_equal(tx2 in self.nodes[1].getrawmempool(), True)
             self.connect_nodes(0, 1)
             self.sync_blocks()
-            self.expect_wallet_notify([bump2, tx2])
+            self.expect_wallet_notify([(bump2, blockheight2, blockhash2), (tx2, -1, UNCONFIRMED_HASH_STRING)])
             assert_equal(self.nodes[1].gettransaction(bump2)["confirmations"], 1)
 
         # TODO: add test for `-alertnotify` large fork notifications
 
-    def expect_wallet_notify(self, tx_ids):
-        self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_ids), timeout=10)
-        assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id in tx_ids), sorted(os.listdir(self.walletnotify_dir)))
+    def expect_wallet_notify(self, tx_details):
+        self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_details), timeout=10)
+        # Should have no more and no less files than expected
+        assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id, _, _ in tx_details), sorted(os.listdir(self.walletnotify_dir)))
+        # Should now verify contents of each file
+        for tx_id, blockheight, blockhash in tx_details:
+            fname = os.path.join(self.walletnotify_dir, notify_outputname(self.wallet, tx_id))
+            with open(fname, 'rt', encoding='utf-8') as f:
+                text = f.read()
+                # Universal newline ensures '\n' on 'nt'
+                assert_equal(text[-1], '\n')
+                text = text[:-1]
+                if os.name == 'nt':
+                    # On Windows, echo as above will append a whitespace
+                    assert_equal(text[-1], ' ')
+                    text = text[:-1]
+                expected = str(blockheight) + '_' + blockhash
+                assert_equal(text, expected)
+
         for tx_file in os.listdir(self.walletnotify_dir):
             os.remove(os.path.join(self.walletnotify_dir, tx_file))