commit_tx: make interface side-agnostic.

It's currently written to produce "local" commit-txs, but of course we
need to produce remote ones too, for signing.

Thus instead of using "remote" and "local" we use "other" and "self",
and indicate with a single "side" flag which we're generating (because
that changes how HTLCs are interpreted).

This also adds to the tests: generate the remote view of the commit_tx
and make sure it matches!

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

Author: rustyrussell

lightningd/commit_tx.c

@@ -33,15 +33,15 @@ u64 commit_number_obscurer(const struct pubkey *opener_payment_basepoint,
 	return be64_to_cpu(obscurer);
 }
 
-static void subtract_fee(enum side funder,
-			 u64 base_fee_msat, u64 *local_msat, u64 *remote_msat)
+static void subtract_fee(enum side funder, enum side side,
+			 u64 base_fee_msat, u64 *self_msat, u64 *other_msat)
 {
 	u64 *funder_msat;
 
-	if (funder == LOCAL)
-		funder_msat = local_msat;
+	if (funder == side)
+		funder_msat = self_msat;
 	else
-		funder_msat = remote_msat;
+		funder_msat = other_msat;
 
 	if (*funder_msat >= base_fee_msat)
 		*funder_msat -= base_fee_msat;
@@ -97,7 +97,7 @@ static const struct htlc **untrimmed(const tal_t *ctx,
 	 */
 	arr = tal_arr(ctx, const struct htlc *, tal_count(htlcs));
 	for (i = n = 0; i < tal_count(htlcs); i++) {
-		if  (htlc_owner(htlcs[i]) != side)
+		if (htlc_owner(htlcs[i]) != side)
 			continue;
 		if (htlcs[i]->msatoshi / 1000 < dust_limit_satoshis + htlc_fee)
 			continue;
@@ -146,34 +146,35 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 			     enum side funder,
 			     u16 to_self_delay,
 			     const struct pubkey *revocation_pubkey,
-			     const struct pubkey *local_delayedkey,
-			     const struct pubkey *localkey,
-			     const struct pubkey *remotekey,
+			     const struct pubkey *self_delayedkey,
+			     const struct pubkey *selfkey,
+			     const struct pubkey *otherkey,
 			     u64 feerate_per_kw,
 			     u64 dust_limit_satoshis,
-			     u64 local_pay_msat,
-			     u64 remote_pay_msat,
+			     u64 self_pay_msat,
+			     u64 other_pay_msat,
 			     const struct htlc **htlcs,
 			     const struct htlc ***htlcmap,
-			     u64 obscured_commitment_number)
+			     u64 obscured_commitment_number,
+			     enum side side)
 {
 	const tal_t *tmpctx = tal_tmpctx(ctx);
 	const struct htlc **offered, **received;
 	u64 base_fee_msat;
 	struct bitcoin_tx *tx;
 	size_t i, n;
 
-	assert(local_pay_msat + remote_pay_msat <= funding_satoshis * 1000);
+	assert(self_pay_msat + other_pay_msat <= funding_satoshis * 1000);
 
 	/* BOLT #3:
 	 *
 	 * 1. Calculate which committed HTLCs need to be trimmed (see
 	 * [Trimmed Outputs](#trimmed-outputs)).
 	 */
-	offered = untrimmed(tmpctx, htlcs, LOCAL,
+	offered = untrimmed(tmpctx, htlcs, side,
 			    htlc_timeout_fee(feerate_per_kw),
 			    dust_limit_satoshis);
-	received = untrimmed(tmpctx, htlcs, REMOTE,
+	received = untrimmed(tmpctx, htlcs, !side,
 			     htlc_success_fee(feerate_per_kw),
 			     dust_limit_satoshis);
 
@@ -195,7 +196,8 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 	 * 3. Subtract this base fee from the funder (either `to-local` or
 	 * `to-remote`), with a floor of zero (see [Fee Payment](#fee-payment)).
 	 */
-	subtract_fee(funder, base_fee_msat, &local_pay_msat, &remote_pay_msat);
+	subtract_fee(funder, side, base_fee_msat,
+		     &self_pay_msat, &other_pay_msat);
 
 	/* Worst-case sizing: both to-local and to-remote outputs. */
 	tx = bitcoin_tx(ctx, 1, tal_count(offered) + tal_count(received) + 2);
@@ -211,7 +213,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 	n = 0;
 	for (i = 0; i < tal_count(offered); i++, n++) {
 		u8 *wscript = bitcoin_wscript_htlc_offer(tmpctx,
-							 localkey, remotekey,
+							 selfkey, otherkey,
 							 &offered[i]->rhash);
 		tx->output[n].amount = offered[i]->msatoshi / 1000;
 		tx->output[n].script = scriptpubkey_p2wsh(tx, wscript);
@@ -229,7 +231,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 	for (i = 0; i < tal_count(received); i++, n++) {
 		u8 *wscript = bitcoin_wscript_htlc_receive(tmpctx,
 							   &received[i]->expiry,
-							   localkey, remotekey,
+							   selfkey, otherkey,
 							   &received[i]->rhash);
 		tx->output[n].amount = received[i]->msatoshi / 1000;
 		tx->output[n].script = scriptpubkey_p2wsh(tx, wscript);
@@ -245,12 +247,12 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 	 *    `dust-limit-satoshis`, add a [To-Local
 	 *    Output](#to-local-output).
 	 */
-	if (local_pay_msat / 1000 >= dust_limit_satoshis) {
+	if (self_pay_msat / 1000 >= dust_limit_satoshis) {
 		u8 *wscript = bitcoin_wscript_to_local(tmpctx,
 						       to_self_delay,
 						       revocation_pubkey,
-						       local_delayedkey);
-		tx->output[n].amount = local_pay_msat / 1000;
+						       self_delayedkey);
+		tx->output[n].amount = self_pay_msat / 1000;
 		tx->output[n].script = scriptpubkey_p2wsh(tx, wscript);
 		(*htlcmap)[n] = NULL;
 		SUPERVERBOSE("# to-local amount %"PRIu64" wscript %s\n",
@@ -265,20 +267,20 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
 	 *    `dust-limit-satoshis`, add a [To-Remote
 	 *    Output](#to-remote-output).
 	 */
-	if (remote_pay_msat / 1000 >= dust_limit_satoshis) {
+	if (other_pay_msat / 1000 >= dust_limit_satoshis) {
 		/* BOLT #3:
 		 *
 		 * #### To-Remote Output
 		 *
 		 * This output sends funds to the other peer, thus is a simple
 		 * P2WPKH to `remotekey`.
 		 */
-		tx->output[n].amount = remote_pay_msat / 1000;
-		tx->output[n].script = scriptpubkey_p2wpkh(tx, remotekey);
+		tx->output[n].amount = other_pay_msat / 1000;
+		tx->output[n].script = scriptpubkey_p2wpkh(tx, otherkey);
 		(*htlcmap)[n] = NULL;
 		SUPERVERBOSE("# to-remote amount %"PRIu64" P2WPKH(%s)\n",
 			     tx->output[n].amount,
-			     type_to_string(tmpctx, struct pubkey, remotekey));
+			     type_to_string(tmpctx, struct pubkey, otherkey));
 		n++;
 	}
 

lightningd/commit_tx.h

@@ -20,23 +20,44 @@ u64 commit_number_obscurer(const struct pubkey *opener_payment_basepoint,
 u64 htlc_success_fee(u64 feerate_per_kw);
 u64 htlc_timeout_fee(u64 feerate_per_kw);
 
-/* Create commitment tx to spend the funding tx output; doesn't fill in
- * input scriptsig. */
+/**
+ * commit_tx: create (unsigned) commitment tx to spend the funding tx output
+ * @ctx: context to allocate transaction and @htlc_map from.
+ * @funding_txid, @funding_out, @funding_satoshis: funding outpoint.
+ * @funder: is the LOCAL or REMOTE paying the fee?
+ * @revocation_pubkey: revocation pubkey for this @side
+ * @self_delayedey: pubkey for delayed payments to this @side
+ * @selfkey: pubkey for HTLC payments to this @side
+ * @otherkey: pubkey for direct and HTLC payments to other side @side
+ * @feerate_per_kw: feerate to use
+ * @dust_limit_satoshis: dust limit below which to trim outputs.
+ * @self_pay_msat: amount to pay directly to self
+ * @other_pay_msat: amount to pay directly to the other side
+ * @htlcs: tal_arr of htlcs committed by transaction (some may be trimmed)
+ * @htlc_map: outputed map of outnum->HTLC (NULL for direct outputs).
+ * @obscured_commitment_number: number to encode in commitment transaction
+ * @side: side to generate commitment transaction for.
+ *
+ * We need to be able to generate the remote side's tx to create signatures,
+ * but the BOLT is expressed in terms of generating our local commitment
+ * transaction, so we carefully use the terms "self" and "other" here.
+ */
 struct bitcoin_tx *commit_tx(const tal_t *ctx,
 			     const struct sha256_double *funding_txid,
 			     unsigned int funding_txout,
 			     u64 funding_satoshis,
 			     enum side funder,
 			     u16 to_self_delay,
 			     const struct pubkey *revocation_pubkey,
-			     const struct pubkey *local_delayedkey,
-			     const struct pubkey *localkey,
-			     const struct pubkey *remotekey,
+			     const struct pubkey *self_delayedkey,
+			     const struct pubkey *selfkey,
+			     const struct pubkey *otherkey,
 			     u64 feerate_per_kw,
 			     u64 dust_limit_satoshis,
-			     u64 local_pay_msat,
-			     u64 remote_pay_msat,
+			     u64 self_pay_msat,
+			     u64 other_pay_msat,
 			     const struct htlc **htlcs,
 			     const struct htlc ***htlcmap,
-			     u64 commit_number_obscurer);
+			     u64 obscured_commitment_number,
+			     enum side side);
 #endif /* LIGHTNING_LIGHTNINGD_COMMIT_TX_H */

lightningd/test/run-commit_tx.c

@@ -11,6 +11,7 @@ static bool print_superverbose;
 #include <bitcoin/privkey.h>
 #include <bitcoin/pubkey.h>
 #include <ccan/array_size/array_size.h>
+#include <ccan/err/err.h>
 #include <ccan/str/hex/hex.h>
 #include <type_to_string.h>
 
@@ -56,6 +57,45 @@ static struct privkey privkey_from_hex(const char *hex)
 	return pk;
 }
 
+static void tx_must_be_eq(const struct bitcoin_tx *a,
+			  const struct bitcoin_tx *b)
+{
+	tal_t *tmpctx = tal_tmpctx(NULL);
+	u8 *lina, *linb;
+	size_t i, len;
+
+	lina = linearize_tx(tmpctx, a);
+	linb = linearize_tx(tmpctx, b);
+
+	len = tal_len(lina);
+	if (tal_len(linb) < len)
+		len = tal_len(linb);
+
+	for (i = 0; i < tal_len(lina); i++) {
+		if (i >= tal_len(linb))
+			errx(1, "Second tx is truncated:\n"
+			     "%s\n"
+			     "%s",
+			     tal_hex(tmpctx, lina),
+			     tal_hex(tmpctx, linb));
+		if (lina[i] != linb[i])
+			errx(1, "tx differ at offset %zu:\n"
+			     "%s\n"
+			     "%s",
+			     i,
+			     tal_hex(tmpctx, lina),
+			     tal_hex(tmpctx, linb));
+	}
+	if (i != tal_len(linb))
+		errx(1, "First tx is truncated:\n"
+		     "%s\n"
+		     "%s",
+		     tal_hex(tmpctx, lina),
+		     tal_hex(tmpctx, linb));
+
+	tal_free(tmpctx);
+}
+
 /* BOLT #3:
  *
  *    htlc 0 direction: remote->local
@@ -329,6 +369,25 @@ static u64 increase(u64 feerate_per_kw)
 }
 #endif
 
+/* HTLCs as seen from other side. */
+static const struct htlc **invert_htlcs(const struct htlc **htlcs)
+{
+	size_t i, n = tal_count(htlcs);
+	const struct htlc **inv = tal_arr(htlcs, const struct htlc *, n);
+
+	for (i = 0; i < n; i++) {
+		struct htlc *htlc;
+		inv[i] = htlc = tal_dup(inv, struct htlc, htlcs[i]);
+		if (inv[i]->state == RCVD_ADD_ACK_REVOCATION)
+			htlc->state = SENT_ADD_ACK_REVOCATION;
+		else {
+			assert(inv[i]->state == SENT_ADD_ACK_REVOCATION);
+			htlc->state = RCVD_ADD_ACK_REVOCATION;
+		}
+	}
+	return inv;
+}
+
 int main(void)
 {
 	tal_t *tmpctx = tal_tmpctx(NULL);
@@ -351,11 +410,12 @@ int main(void)
 	struct pubkey localkey, remotekey, tmpkey;
 	struct pubkey local_delayedkey;
 	struct pubkey local_revocation_key;
-	struct bitcoin_tx *tx;
+	struct bitcoin_tx *tx, *tx2;
 	u8 *wscript;
 	unsigned int funding_output_index;
 	u64 commitment_number, cn_obscurer, to_local_msat, to_remote_msat;
-	const struct htlc **htlcs = setup_htlcs(tmpctx), **htlc_map;
+	const struct htlc **htlcs = setup_htlcs(tmpctx), **htlc_map, **htlc_map2,
+		**inv_htlcs = invert_htlcs(htlcs);
 
 	secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
 						 | SECP256K1_CONTEXT_SIGN);
@@ -593,7 +653,23 @@ int main(void)
 		       dust_limit_satoshi,
 		       to_local_msat,
 		       to_remote_msat,
-		       NULL, &htlc_map, commitment_number ^ cn_obscurer);
+		       NULL, &htlc_map, commitment_number ^ cn_obscurer,
+		       LOCAL);
+	print_superverbose = false;
+	tx2 = commit_tx(tmpctx, &funding_txid, funding_output_index,
+			funding_amount_satoshi,
+			REMOTE, to_self_delay,
+			&local_revocation_key,
+			&local_delayedkey,
+			&localkey,
+			&remotekey,
+			feerate_per_kw,
+			dust_limit_satoshi,
+			to_local_msat,
+			to_remote_msat,
+			NULL, &htlc_map2, commitment_number ^ cn_obscurer,
+			REMOTE);
+	tx_must_be_eq(tx, tx2);
 	report(tx, wscript, &x_remote_funding_privkey, &remote_funding_pubkey,
 	       &local_funding_privkey, &local_funding_pubkey,
 	       to_self_delay,
@@ -635,7 +711,24 @@ int main(void)
 		       dust_limit_satoshi,
 		       to_local_msat,
 		       to_remote_msat,
-		       htlcs, &htlc_map, commitment_number ^ cn_obscurer);
+		       htlcs, &htlc_map, commitment_number ^ cn_obscurer,
+		       LOCAL);
+	print_superverbose = false;
+	tx2 = commit_tx(tmpctx, &funding_txid, funding_output_index,
+			funding_amount_satoshi,
+			REMOTE, to_self_delay,
+			&local_revocation_key,
+			&local_delayedkey,
+			&localkey,
+			&remotekey,
+			feerate_per_kw,
+			dust_limit_satoshi,
+			to_local_msat,
+			to_remote_msat,
+			inv_htlcs, &htlc_map2,
+			commitment_number ^ cn_obscurer,
+			REMOTE);
+	tx_must_be_eq(tx, tx2);
 	report(tx, wscript, &x_remote_funding_privkey, &remote_funding_pubkey,
 	       &local_funding_privkey, &local_funding_pubkey,
 	       to_self_delay,
@@ -665,7 +758,24 @@ int main(void)
 				  to_local_msat,
 				  to_remote_msat,
 				  htlcs, &htlc_map,
-				  commitment_number ^ cn_obscurer);
+				  commitment_number ^ cn_obscurer,
+				  LOCAL);
+		/* This is what it would look like for peer generating it! */
+		tx2 = commit_tx(tmpctx, &funding_txid, funding_output_index,
+				funding_amount_satoshi,
+				REMOTE, to_self_delay,
+				&local_revocation_key,
+				&local_delayedkey,
+				&localkey,
+				&remotekey,
+				feerate_per_kw,
+				dust_limit_satoshi,
+				to_local_msat,
+				to_remote_msat,
+				inv_htlcs, &htlc_map2,
+				commitment_number ^ cn_obscurer,
+				REMOTE);
+		tx_must_be_eq(newtx, tx2);
 #ifdef DEBUG
 		if (feerate_per_kw % 100000 == 0)
 			printf("feerate_per_kw = %"PRIu64", fees = %"PRIu64"\n",
@@ -697,7 +807,8 @@ int main(void)
 			       to_local_msat,
 			       to_remote_msat,
 			       htlcs, &htlc_map,
-			       commitment_number ^ cn_obscurer);
+			       commitment_number ^ cn_obscurer,
+			       LOCAL);
 		report(tx, wscript,
 		       &x_remote_funding_privkey, &remote_funding_pubkey,
 		       &local_funding_privkey, &local_funding_pubkey,
@@ -733,7 +844,8 @@ int main(void)
 				  to_local_msat,
 				  to_remote_msat,
 				  htlcs, &htlc_map,
-				  commitment_number ^ cn_obscurer);
+				  commitment_number ^ cn_obscurer,
+				  LOCAL);
 		report(newtx, wscript,
 		       &x_remote_funding_privkey, &remote_funding_pubkey,
 		       &local_funding_privkey, &local_funding_pubkey,
@@ -782,7 +894,8 @@ int main(void)
 			       to_local_msat,
 			       to_remote_msat,
 			       htlcs, &htlc_map,
-			       commitment_number ^ cn_obscurer);
+			       commitment_number ^ cn_obscurer,
+			       LOCAL);
 		report(tx, wscript,
 		       &x_remote_funding_privkey, &remote_funding_pubkey,
 		       &local_funding_privkey, &local_funding_pubkey,