API mods: SpendType, Descriptors, Funding return

ksedgwic · ksedgwic · commit d41d18abd456 · 2020-05-05T08:30:31.000-07:00
diff --git a/contrib/remote_hsmd/NOTES.md b/contrib/remote_hsmd/NOTES.md
@@ -74,7 +74,30 @@ Some popular tests:
     # sign-channel-announcement
     export THETEST=tests/test_closing.py::test_closing_different_fees
 
+    # P2SH_P2WPKH
+    export THETEST=tests/test_closing.py::test_onchain_first_commit 
+    export THETEST=tests/test_connection.py::test_disconnect_funder 
+    export THETEST=tests/test_connection.py::test_disconnect_fundee 
+    export THETEST=tests/test_connection.py::test_reconnect_signed 
+    export THETEST=tests/test_connection.py::test_reconnect_openingd 
+    export THETEST=tests/test_connection.py::test_shutdown_awaiting_lockin 
+
 rust-lightning-signer
 ----------------------------------------------------------------
 
     cargo run --bin server |& tee log3
+
+
+Signing Formats
+----------------------------------------------------------------
+
+```
+rust-lightning  c-lightning     rust-lightning-signer
+p2pkh                           P2PKH
+p2sh
+p2wpkh          p2wpkh          P2WPKH
+p2shwpkh        p2sh-p2wpkh     P2SH_P2WPKH
+p2wsh
+p2shwsh
+```
+
diff --git a/contrib/remote_hsmd/dump.cc b/contrib/remote_hsmd/dump.cc
@@ -75,23 +75,15 @@ string dump_pubkey(const struct pubkey *kp)
 	return dump_hex(kp->pubkey.data, sizeof(kp->pubkey.data));
 }
 
-string dump_witnesses(const u8 ***wp)
+string dump_signatures(const u8 **sp)
 {
 	ostringstream ostrm;
  	ostrm << "[";
-	for (size_t input_ndx = 0; input_ndx < tal_count(wp); ++input_ndx) {
+	for (size_t input_ndx = 0; input_ndx < tal_count(sp); ++input_ndx) {
 		if (input_ndx != 0)
 			ostrm << " ";
-		ostrm << "[";
-		u8 const **stack = wp[input_ndx];
-		for (size_t item_ndx = 0; item_ndx < tal_count(stack);
-		     ++item_ndx) {
-			if (item_ndx != 0)
-				ostrm << " ";
-			u8 const *item = stack[item_ndx];
-			ostrm << dump_hex(item, tal_count(item));
-		}
-		ostrm << "]";
+		u8 const *sig = sp[input_ndx];
+		ostrm << dump_hex(sig, tal_count(sig));
 	}
  	ostrm << "]";
 	return ostrm.str();
diff --git a/contrib/remote_hsmd/dump.h b/contrib/remote_hsmd/dump.h
@@ -9,7 +9,7 @@ std::string dump_secp256k1_ecdsa_recoverable_signature(const secp256k1_ecdsa_rec
 std::string dump_secret(const struct secret *sp);
 std::string dump_node_id(const struct node_id *pp);
 std::string dump_pubkey(const struct pubkey *kp);
-std::string dump_witnesses(const u8 ***wp);
+std::string dump_signatures(const u8 **sp);
 std::string dump_unilateral_close_info(const struct unilateral_close_info *ip);
 std::string dump_utxo(const struct utxo *in);
 std::string dump_utxos(const struct utxo **utxos);
diff --git a/contrib/remote_hsmd/hsmd.c b/contrib/remote_hsmd/hsmd.c
@@ -1638,6 +1638,7 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 	u32 change_keyindex;
 	struct utxo **utxos;
 	struct bitcoin_tx *tx;
+	struct bitcoin_tx *tx2;	// REF
 	struct pubkey changekey;
 	struct bitcoin_tx_output **outputs;
 	u32 nlocktime;
@@ -1655,20 +1656,6 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 			 cast_const2(const struct utxo **, utxos), outputs,
 			 &changekey, change_out, NULL, NULL, nlocktime);
 
-	u8 *** sigs;
-	proxy_stat rv = proxy_handle_sign_withdrawal_tx(
-		&c->id, c->dbid, &satoshi_out,
-		&change_out, change_keyindex,
-		outputs, utxos, tx, &sigs);
-	if (PROXY_PERMANENT(rv))
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-		              "proxy_%s failed: %s", __FUNCTION__,
-			      proxy_last_message());
-	else if (!PROXY_SUCCESS(rv))
-		return bad_req_fmt(conn, c, msg_in,
-				   "proxy_%s error: %s", __FUNCTION__,
-				   proxy_last_message());
-
 	/* FIXME - There are two things we can't do remotely yet:
 	 * 1. Handle P2SH inputs.
 	 * 2. Handle inputs w/ close_info.
@@ -1677,25 +1664,57 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 	for (size_t ii = 0; ii < tx->wtx->num_inputs; ii++)
 		if (utxos[ii]->is_p2sh || utxos[ii]->close_info)
 			demure = true;
+
 	if (!demure) {
+		u8 ** sigs;
+		proxy_stat rv = proxy_handle_sign_withdrawal_tx(
+			&c->id, c->dbid, &satoshi_out,
+			&change_out, change_keyindex,
+			outputs, utxos, tx, &sigs);
+		if (PROXY_PERMANENT(rv))
+			status_failed(STATUS_FAIL_INTERNAL_ERROR,
+				      "proxy_%s failed: %s", __FUNCTION__,
+				      proxy_last_message());
+		else if (!PROXY_SUCCESS(rv))
+			return bad_req_fmt(conn, c, msg_in,
+					   "proxy_%s error: %s", __FUNCTION__,
+					   proxy_last_message());
+
 		/* Sign w/ the remote lightning-signer. */
 		g_proxy_impl = PROXY_IMPL_COMPLETE;
 		assert(tal_count(sigs) == tal_count(utxos));
 		for (size_t ii = 0; ii < tal_count(sigs); ++ii) {
-			assert(tal_count(sigs[ii]) == 2);
-
+			/* Figure out keys to spend this. */
+			struct pubkey inkey;
+			u8 der_pubkey[PUBKEY_CMPR_LEN];
+			const struct utxo *in = utxos[ii];
+			hsm_key_for_utxo(NULL, &inkey, in);
+			pubkey_to_der(der_pubkey, &inkey);
 			u8 **witness = tal_arr(tx, u8 *, 2);
-			witness[0] = tal_dup_arr(witness, u8, sigs[ii][0],
-						 tal_count(sigs[ii][0]), 0);
-			witness[1] = tal_dup_arr(witness, u8, sigs[ii][1],
-						 tal_count(sigs[ii][1]), 0);
+			witness[0] = tal_dup_arr(witness, u8,
+						 sigs[ii],
+						 tal_count(sigs[ii]), 0);
+			witness[1] = tal_dup_arr(witness, u8,
+						 der_pubkey,
+						 sizeof(der_pubkey), 0);
 			bitcoin_tx_input_set_witness(tx, ii, take(witness));
 		}
+
+		print_tx("RLS", tx);
 	} else {
 		/* It's P2SH, need to sign here */
 		g_proxy_impl = PROXY_IMPL_MARSHALED;
 		sign_all_inputs(tx, utxos);
 	}
+
+	tx2 = withdraw_tx(tmpctx, c->chainparams,
+			  cast_const2(const struct utxo **, utxos), outputs,
+			  &changekey, change_out, NULL, NULL, nlocktime);
+
+	sign_all_inputs(tx2, utxos);
+
+	print_tx("REF", tx2);
+
 	return req_reply(conn, c,
 			 take(towire_hsm_sign_withdrawal_reply(NULL, tx)));
 }
diff --git a/contrib/remote_hsmd/proxy.cc b/contrib/remote_hsmd/proxy.cc
@@ -140,36 +140,38 @@ void marshal_single_input_tx(struct bitcoin_tx const *tx,
 	if (output_witscript) {
 		/* Called with a single witscript. */
 		assert(tx->wtx->num_outputs == 1);
-		o_tp->add_output_witscripts((const char *) output_witscript,
-					    tal_count(output_witscript));
 	} else if (output_witscripts) {
 		/* Called with an array of witscripts. */
-		size_t nwitscripts = tal_count(output_witscripts);
-		assert(nwitscripts == tx->wtx->num_outputs);
-		for (size_t ii = 0; ii < tx->wtx->num_outputs; ii++)
-			if (output_witscripts[ii])
-				o_tp->add_output_witscripts(
-					(const char *)
-					output_witscripts[ii]->ptr,
-					tal_count(output_witscripts[ii]->ptr));
-			else
-				o_tp->add_output_witscripts("");
-	} else {
-		/* Called with no witscrtipts. */
-		for (size_t ii = 0; ii < tx->wtx->num_outputs; ii++)
-			o_tp->add_output_witscripts("");
+		assert(tal_count(output_witscripts) == tx->wtx->num_outputs);
 	}
 
 	o_tp->set_raw_tx_bytes(serialized_tx(tx, true));
 
 	assert(tx->wtx->num_inputs == 1);
-	SignDescriptor *desc = o_tp->add_input_descs();
-	desc->mutable_output()->set_value(tx->input_amounts[0]->satoshis);
+	InputDescriptor *idesc = o_tp->add_input_descs();
+	idesc->mutable_prev_output()->set_value(tx->input_amounts[0]->satoshis);
 	/* FIXME - What else needs to be set? */
 
 	for (size_t ii = 0; ii < tx->wtx->num_outputs; ii++) {
-		SignDescriptor *desc = o_tp->add_output_descs();
-		/* FIXME - We don't need to set *anything* here? */
+		OutputDescriptor *odesc = o_tp->add_output_descs();
+		if (output_witscript) {
+			/* We have a single witscript. */
+			odesc->set_witscript((const char *) output_witscript,
+					     tal_count(output_witscript));
+		} else if (output_witscripts) {
+			/* We have an array of witscripts. */
+			if (output_witscripts[ii])
+				odesc->set_witscript(
+					(const char *)
+					output_witscripts[ii]->ptr,
+					tal_count(output_witscripts[ii]->ptr));
+			else
+				odesc->set_witscript("");
+		} else {
+			/* Called w/ no witscripts. */
+			odesc->set_witscript("");
+		}
+
 	}
 }
 
@@ -227,23 +229,17 @@ void unmarshal_ecdsa_recoverable_signature(ECDSARecoverableSignature const &es,
 	assert(ok);
 }
 
-void unmarshal_witnesses(RepeatedPtrField<WitnessStack> const &wits,
-		      u8 ****o_sigs)
+void unmarshal_signatures(RepeatedPtrField<BitcoinSignature> const &sigs,
+			  u8 ***o_sigs)
 {
-	u8 ***osigs = NULL;
-	int nsigs = wits.size();
+	u8 **osigs = NULL;
+	int nsigs = sigs.size();
 	if (nsigs > 0) {
-		osigs = tal_arrz(tmpctx, u8**, nsigs);
+		osigs = tal_arrz(tmpctx, u8*, nsigs);
 		for (size_t ii = 0; ii < nsigs; ++ii) {
-			WitnessStack const &sig = wits[ii];
-			int nelem = sig.item_size();
-			osigs[ii] = tal_arrz(osigs, u8*, nelem);
-			for (size_t jj = 0; jj < nelem; ++jj) {
-				string const &elem = sig.item(jj);
-				size_t elen = elem.size();
-				osigs[ii][jj] = tal_arr(osigs[ii], u8, elen);
-				memcpy(osigs[ii][jj], &elem[0], elen);
-			}
+			BitcoinSignature const &bs = sigs[ii];
+			osigs[ii] = tal_arr(osigs, u8, bs.data().size());
+			memcpy(osigs[ii], bs.data().data(), bs.data().size());
 		}
 	}
 	*o_sigs = osigs;
@@ -397,8 +393,25 @@ proxy_stat proxy_handle_sign_withdrawal_tx(
 	struct bitcoin_tx_output **outputs,
 	struct utxo **utxos,
 	struct bitcoin_tx *tx,
-	u8 ****o_sigs)
+	u8 ***o_sigs)
 {
+	fprintf(stderr,
+		"%s:%d %s self_id=%s peer_id=%s dbid=%" PRIu64 " "
+		"satoshi_out=%" PRIu64 " change_out=%" PRIu64 " "
+		"change_keyindex=%u utxos=%s outputs=%s tx=%s\n",
+		__FILE__, __LINE__, __FUNCTION__,
+		dump_node_id(&self_id).c_str(),
+		dump_node_id(peer_id).c_str(),
+		dbid,
+		satoshi_out->satoshis,
+		change_out->satoshis,
+		change_keyindex,
+		dump_utxos((const struct utxo **)utxos).c_str(),
+		dump_bitcoin_tx_outputs(
+			(const struct bitcoin_tx_output **)outputs).c_str(),
+		dump_tx(tx).c_str()
+		);
+
 	status_debug(
 		"%s:%d %s self_id=%s peer_id=%s dbid=%" PRIu64 " "
 		"satoshi_out=%" PRIu64 " change_out=%" PRIu64 " "
@@ -427,9 +440,12 @@ proxy_stat proxy_handle_sign_withdrawal_tx(
 	 	const struct utxo *in = utxos[ii];
 		/* Fails in tests/test_closing.py::test_onchain_first_commit */
 		/* assert(!in->is_p2sh); */
-		SignDescriptor *desc = req.mutable_tx()->add_input_descs();
-		desc->mutable_key_loc()->set_key_index(in->keyindex);
-		desc->mutable_output()->set_value(in->amount.satoshis);
+		InputDescriptor *idesc = req.mutable_tx()->add_input_descs();
+		idesc->mutable_key_loc()->set_key_index(in->keyindex);
+		idesc->mutable_prev_output()->set_value(in->amount.satoshis);
+		idesc->set_spend_type(in->is_p2sh
+				      ? SpendType::P2SH_P2WPKH
+				      : SpendType::P2WPKH);
 	}
 
 	/* We expect exactly two total ouputs, with one non-change. */
@@ -440,7 +456,7 @@ proxy_stat proxy_handle_sign_withdrawal_tx(
 	assert(tal_count(outputs) == 1);
 	for (size_t ii = 0; ii < tx->wtx->num_outputs; ii++) {
 	 	const struct wally_tx_output *out = &tx->wtx->outputs[ii];
-		SignDescriptor *desc = req.mutable_tx()->add_output_descs();
+		OutputDescriptor *odesc = req.mutable_tx()->add_output_descs();
 		/* Does this output match the funding output? */
 		if (memeq(out->script, out->script_len,
 			  outputs[0]->script, tal_count(outputs[0]->script))) {
@@ -449,22 +465,31 @@ proxy_stat proxy_handle_sign_withdrawal_tx(
 		} else {
 			/* Nope, this must be the change output. */
 			assert(out->satoshi == change_out->satoshis);
-			desc->mutable_key_loc()->set_key_index(change_keyindex);
+			odesc->mutable_key_loc()->
+				set_key_index(change_keyindex);
 		}
 	}
 
 	ClientContext context;
 	SignFundingTxReply rsp;
 	Status status = stub->SignFundingTx(&context, req, &rsp);
 	if (status.ok()) {
-		unmarshal_witnesses(rsp.witnesses(), o_sigs);
+		unmarshal_signatures(rsp.signatures(), o_sigs);
+		fprintf(stderr, "%s:%d %s self_id=%s witnesses=%s\n",
+			     __FILE__, __LINE__, __FUNCTION__,
+			     dump_node_id(&self_id).c_str(),
+			     dump_signatures((u8 const **) *o_sigs).c_str());
 		status_debug("%s:%d %s self_id=%s witnesses=%s",
 			     __FILE__, __LINE__, __FUNCTION__,
 			     dump_node_id(&self_id).c_str(),
-			     dump_witnesses((u8 const ***) *o_sigs).c_str());
+			     dump_signatures((u8 const **) *o_sigs).c_str());
 		last_message = "success";
 		return PROXY_OK;
 	} else {
+		fprintf(stderr, "%s:%d %s: self_id=%s %s\n",
+			       __FILE__, __LINE__, __FUNCTION__,
+			       dump_node_id(&self_id).c_str(),
+			       status.error_message().c_str());
 		status_unusual("%s:%d %s: self_id=%s %s",
 			       __FILE__, __LINE__, __FUNCTION__,
 			       dump_node_id(&self_id).c_str(),
@@ -1236,4 +1261,11 @@ proxy_stat proxy_handle_sign_node_announcement(
 	}
 }
 
+// FIXME - This routine allows us to pretty print the tx to stderr
+// from C code.  Probably should remove it in production ...
+void print_tx(char const *tag, struct bitcoin_tx const *tx)
+{
+	fprintf(stderr, "%s: tx=%s\n", tag, dump_tx(tx).c_str());
+}
+
 } /* extern "C" */
diff --git a/contrib/remote_hsmd/proxy.h b/contrib/remote_hsmd/proxy.h
@@ -55,7 +55,7 @@ proxy_stat proxy_handle_sign_withdrawal_tx(
 	struct bitcoin_tx_output **outputs,
 	struct utxo **utxos,
 	struct bitcoin_tx *tx,
-	u8 ****o_sigs);
+	u8 ***o_sigs);
 
 proxy_stat proxy_handle_sign_remote_commitment_tx(
 	struct bitcoin_tx *tx,
@@ -166,6 +166,9 @@ proxy_stat proxy_handle_sign_node_announcement(
 	u8 *node_announcement,
 	secp256k1_ecdsa_signature *o_sig);
 
+// FIXME - For debugging, remove for production.
+void print_tx(char const *tag, struct bitcoin_tx const *tx);
+
 #ifdef __cplusplus
 } /* extern C */
 #endif
