wallet: clean up json output creation, part 2.

It looked like we weren't printing the address on closing outputs.

But we are, because the 'scriptPubkey' field is in the 'outputs' db
table since 0.7.3 (66a47d2761).

So make the logic clearer, and remove a completely bogus comment (UTXOs
with closing_info are definitely spendable!).

We export the json_add_utxos() for future use, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

tests/test_closing.py

@@ -1568,6 +1568,34 @@ def test_onchain_their_unilateral_out(node_factory, bitcoind):
     assert account_balance(l2, channel_id) == 0
 
 
+def test_listfunds_after_their_unilateral(node_factory, bitcoind):
+    """We keep spending info around for their unilateral closes.
+
+Make sure we show the address.
+    """
+    coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py')
+    l1, l2 = node_factory.line_graph(2, opts={'plugin': coin_mvt_plugin})
+    channel_id = first_channel_id(l1, l2)
+
+    # listfunds will show 1 output change, and channels.
+    assert len(l1.rpc.listfunds()['outputs']) == 1
+
+    l1.stop()
+    l2.rpc.close(l1.info['id'], unilateraltimeout=1)
+    l2.wait_for_channel_onchain(l1.info['id'])
+    bitcoind.generate_block(100)
+
+    l1.start()
+    l2.daemon.wait_for_log('onchaind complete, forgetting peer')
+    l1.daemon.wait_for_log('onchaind complete, forgetting peer')
+    wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2)
+    assert all(['address' in o for o in l1.rpc.listfunds()['outputs']])
+
+    # Verify accounting for l1 & l2
+    assert account_balance(l1, channel_id) == 0
+    assert account_balance(l2, channel_id) == 0
+
+
 @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
 def test_onchain_feechange(node_factory, bitcoind, executor):
     """Onchain handling when we restart with different fees"""

wallet/walletrpc.c

@@ -31,6 +31,7 @@
 #include <lightningd/peer_control.h>
 #include <lightningd/subd.h>
 #include <wallet/wallet.h>
+#include <wallet/walletrpc.h>
 #include <wally_bip32.h>
 #include <wire/wire_sync.h>
 
@@ -633,7 +634,6 @@ AUTODATA(json_command, &withdraw_command);
 /* May return NULL if encoding error occurs. */
 static char *
 encode_pubkey_to_addr(const tal_t *ctx,
-		      const struct lightningd *ld,
 		      const struct pubkey *pubkey,
 		      bool is_p2sh_p2wpkh,
 		      /* Output: redeemscript to use to redeem outputs
@@ -741,8 +741,8 @@ static struct command_result *json_newaddr(struct command *cmd,
 		txfilter_add_scriptpubkey(cmd->ld->owned_txfilter,
 					  scriptpubkey_p2sh(tmpctx, b32script));
 
-	p2sh = encode_pubkey_to_addr(cmd, cmd->ld, &pubkey, true, NULL);
+	p2sh = encode_pubkey_to_addr(cmd, &pubkey, true, NULL);
-	bech32 = encode_pubkey_to_addr(cmd, cmd->ld, &pubkey, false, NULL);
+	bech32 = encode_pubkey_to_addr(cmd, &pubkey, false, NULL);
 	if (!p2sh || !bech32) {
 		return command_fail(cmd, LIGHTNINGD,
 				    "p2wpkh address encoding failure.");
@@ -801,14 +801,14 @@ static struct command_result *json_listaddrs(struct command *cmd,
 
 		// p2sh
 		u8 *redeemscript_p2sh;
-		char *out_p2sh = encode_pubkey_to_addr(cmd, cmd->ld,
+		char *out_p2sh = encode_pubkey_to_addr(cmd,
 						       &pubkey,
 						       true,
 						       &redeemscript_p2sh);
 
 		// bech32 : p2wpkh
 		u8 *redeemscript_p2wpkh;
-		char *out_p2wpkh = encode_pubkey_to_addr(cmd, cmd->ld,
+		char *out_p2wpkh = encode_pubkey_to_addr(cmd,
 							 &pubkey,
 							 false,
 							 &redeemscript_p2wpkh);
@@ -842,64 +842,71 @@ static const struct json_command listaddrs_command = {
 };
 AUTODATA(json_command, &listaddrs_command);
 
-static void json_add_utxos(struct command *cmd,
+static void json_add_utxo(struct json_stream *response,
-			   struct json_stream *response,
+			  const char *fieldname,
-			   struct utxo **utxos)
+			  struct wallet *wallet,
+			  const struct utxo *utxo)
 {
-	char* out;
+	const char *out;
-	struct pubkey funding_pubkey;
 
-	for (size_t i = 0; i < tal_count(utxos); i++) {
+	json_object_start(response, fieldname);
-		json_object_start(response, NULL);
+	json_add_txid(response, "txid", &utxo->txid);
-		json_add_txid(response, "txid", &utxos[i]->txid);
+	json_add_num(response, "output", utxo->outnum);
-		json_add_num(response, "output", utxos[i]->outnum);
+	json_add_amount_sat_compat(response, utxo->amount,
-		json_add_amount_sat_compat(response, utxos[i]->amount,
 				   "value", "amount_msat");
 
-		/* @close_info is for outputs that are not yet claimable */
+	if (utxo->scriptPubkey != NULL) {
-		if (utxos[i]->close_info == NULL) {
+		out = encode_scriptpubkey_to_addr(
-			bip32_pubkey(cmd->ld->wallet->bip32_base, &funding_pubkey,
+			tmpctx, chainparams,
-				     utxos[i]->keyindex);
+			utxo->scriptPubkey);
-			out = encode_pubkey_to_addr(cmd, cmd->ld,
+	} else {
+		out = NULL;
+#ifdef COMPAT_V072
+		/* scriptpubkey was introduced in v0.7.3.
+		 * We could handle close_info via HSM to get address,
+		 * but who cares?  We'll print a warning though. */
+		if (utxo->close_info == NULL) {
+			struct pubkey funding_pubkey;
+			bip32_pubkey(wallet->bip32_base,
 				     &funding_pubkey,
-						    utxos[i]->is_p2sh,
+				     utxo->keyindex);
+			out = encode_pubkey_to_addr(tmpctx,
+						    &funding_pubkey,
+						    utxo->is_p2sh,
 						    NULL);
+		}
+#endif
+	}
 	if (!out)
-				log_broken(cmd->ld->log,
+		log_broken(wallet->log,
-					   "Could not encode utxo %s:%u!",
+			   "Could not encode utxo %s:%u%s!",
-					   type_to_string(tmpctx,
-							  struct bitcoin_txid,
-							  &utxos[i]->txid),
-					   utxos[i]->outnum);
-			else
-				json_add_string(response, "address", out);
-		} else if (utxos[i]->scriptPubkey != NULL) {
-			out = encode_scriptpubkey_to_addr(
-			    cmd, chainparams,
-			    utxos[i]->scriptPubkey);
-			if (!out)
-				log_broken(cmd->ld->log,
-					   "Could not encode utxo %s:%u!",
 			   type_to_string(tmpctx,
 					  struct bitcoin_txid,
-							  &utxos[i]->txid),
+					  &utxo->txid),
-					   utxos[i]->outnum);
+			   utxo->outnum,
+			   utxo->close_info ? " (has close_info)" : "");
 	else
 		json_add_string(response, "address", out);
-		}
 
-		if (utxos[i]->spendheight)
+	if (utxo->spendheight)
 		json_add_string(response, "status", "spent");
-		else if (utxos[i]->blockheight) {
+	else if (utxo->blockheight) {
 		json_add_string(response, "status", "confirmed");
-			json_add_num(response, "blockheight", *utxos[i]->blockheight);
+		json_add_num(response, "blockheight", *utxo->blockheight);
 	} else
 		json_add_string(response, "status", "unconfirmed");
 
 	json_add_bool(response, "reserved",
-			      utxos[i]->status == output_state_reserved);
+		      utxo->status == output_state_reserved);
 	json_object_end(response);
-	}
+}
+
+void json_add_utxos(struct json_stream *response,
+		    struct wallet *wallet,
+		    struct utxo **utxos)
+{
+	for (size_t i = 0; i < tal_count(utxos); i++)
+		json_add_utxo(response, NULL, wallet, utxos[i]);
 }
 
 static struct command_result *json_listfunds(struct command *cmd,
@@ -919,8 +926,8 @@ static struct command_result *json_listfunds(struct command *cmd,
 	utxos = wallet_get_utxos(cmd, cmd->ld->wallet, output_state_available);
 	reserved_utxos = wallet_get_utxos(cmd, cmd->ld->wallet, output_state_reserved);
 	json_array_start(response, "outputs");
-	json_add_utxos(cmd, response, utxos);
+	json_add_utxos(response, cmd->ld->wallet, utxos);
-	json_add_utxos(cmd, response, reserved_utxos);
+	json_add_utxos(response, cmd->ld->wallet, reserved_utxos);
 	json_array_end(response);
 
 	/* Add funds that are allocated to channels */

wallet/walletrpc.h

@@ -1,11 +1,12 @@
 #ifndef LIGHTNING_WALLET_WALLETRPC_H
 #define LIGHTNING_WALLET_WALLETRPC_H
-
 #include "config.h"
 
-/**
+struct json_stream;
- * Placeholder file, we aren't really exposing anything directly. RPC
+struct utxo;
- * handlers are registered using AUTODATA.
+
- */
+void json_add_utxos(struct json_stream *response,
+		    struct wallet *wallet,
+		    struct utxo **utxos);
 
 #endif /* LIGHTNING_WALLET_WALLETRPC_H */