permute: use BIP69 order.

It's a canonical ordering, rather than a random shuffle.  Far simpler.

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

anchor.c

@@ -127,10 +127,8 @@ struct bitcoin_tx *anchor_tx_create(const tal_t *ctx,
 	else
 		outmap = NULL;
 		
-	permute_inputs(o1->seed, o2->seed, 0, tx->input, tx->input_count,
+	permute_inputs(tx->input, tx->input_count, inmap);
-		       inmap);
+	permute_outputs(tx->output, tx->output_count, outmap);
-	permute_outputs(o1->seed, o2->seed, 0, tx->output, tx->output_count,
-			outmap);
 	return tx;
 }
 

close_tx.c

@@ -55,6 +55,6 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx,
 	tx->output[1].script_length = tal_count(tx->output[1].script);
 
 	tx->fee = ours->commitment_fee + theirs->commitment_fee;
-	permute_outputs(ours->seed, theirs->seed, 1, tx->output, 2, NULL);
+	permute_outputs(tx->output, 2, NULL);
 	return tx;
 }

commit_tx.c

@@ -76,6 +76,6 @@ struct bitcoin_tx *create_commit_tx(const tal_t *ctx,
 	tx->fee = tx->input[0].input_amount
 		- (tx->output[0].amount + tx->output[1].amount);
 	
-	permute_outputs(ours->seed, theirs->seed, 1, tx->output, 2, NULL);
+	permute_outputs(tx->output, 2, NULL);
 	return tx;
 }

lightning.pb-c.c

@@ -1474,20 +1474,8 @@ const ProtobufCMessageDescriptor anchor__descriptor =
   (ProtobufCMessageInit) anchor__init,
   NULL,NULL,NULL    /* reserved[123] */
 };
-static const ProtobufCFieldDescriptor open_channel__field_descriptors[8] =
+static const ProtobufCFieldDescriptor open_channel__field_descriptors[7] =
 {
-  {
-    "seed",
-    1,
-    PROTOBUF_C_LABEL_REQUIRED,
-    PROTOBUF_C_TYPE_UINT64,
-    0,   /* quantifier_offset */
-    offsetof(OpenChannel, seed),
-    NULL,
-    NULL,
-    0,             /* flags */
-    0,NULL,NULL    /* reserved1,reserved2, etc */
-  },
   {
     "locktime_seconds",
     2,
@@ -1574,19 +1562,18 @@ static const ProtobufCFieldDescriptor open_channel__field_descriptors[8] =
   },
 };
 static const unsigned open_channel__field_indices_by_name[] = {
-  6,   /* field[6] = anchor */
+  5,   /* field[5] = anchor */
-  5,   /* field[5] = commitment_fee */
+  4,   /* field[4] = commitment_fee */
-  4,   /* field[4] = final */
+  3,   /* field[3] = final */
-  2,   /* field[2] = locktime_blocks */
+  1,   /* field[1] = locktime_blocks */
-  1,   /* field[1] = locktime_seconds */
+  0,   /* field[0] = locktime_seconds */
-  3,   /* field[3] = revocation_hash */
+  2,   /* field[2] = revocation_hash */
-  0,   /* field[0] = seed */
+  6,   /* field[6] = tx_version */
-  7,   /* field[7] = tx_version */
 };
 static const ProtobufCIntRange open_channel__number_ranges[1 + 1] =
 {
-  { 1, 0 },
+  { 2, 0 },
-  { 0, 8 }
+  { 0, 7 }
 };
 const ProtobufCMessageDescriptor open_channel__descriptor =
 {
@@ -1596,7 +1583,7 @@ const ProtobufCMessageDescriptor open_channel__descriptor =
   "OpenChannel",
   "",
   sizeof(OpenChannel),
-  8,
+  7,
   open_channel__field_descriptors,
   open_channel__field_indices_by_name,
   1,  open_channel__number_ranges,

lightning.pb-c.h

@@ -185,10 +185,6 @@ typedef enum {
 struct  _OpenChannel
 {
   ProtobufCMessage base;
-  /*
-   * Seed which sets order we create outputs for all transactions.
-   */
-  uint64_t seed;
   /*
    * Hash seed for revoking commitment transactions.
    */
@@ -217,7 +213,7 @@ struct  _OpenChannel
 };
 #define OPEN_CHANNEL__INIT \
  { PROTOBUF_C_MESSAGE_INIT (&open_channel__descriptor) \
-    , 0, NULL, NULL, 0, NULL, 0, OPEN_CHANNEL__LOCKTIME__NOT_SET, {} }
+    , NULL, NULL, 0, NULL, 0, OPEN_CHANNEL__LOCKTIME__NOT_SET, {} }
 
 
 /*

lightning.proto

@@ -70,8 +70,6 @@ message anchor {
 
 // Set channel params.
 message open_channel {
-  // Seed which sets order we create outputs for all transactions.
-  required uint64 seed = 1;
   // Relative locktime for outputs going to us.
   oneof locktime {
     uint32 locktime_seconds = 2;

permute_tx.c

@@ -1,39 +1,7 @@
-#include <ccan/crypto/sha256/sha256.h>
 #include <stdbool.h>
 #include <string.h>
 #include "permute_tx.h"
 
-static u32 get_next_rand(struct sha256 *h, size_t *randidx)
-{
-	u32 ret = h->u.u32[(*randidx)++];
-	if (*randidx == 8) {
-		*randidx = 0;
-		sha256(h, h, sizeof(*h));
-	}
-	return ret;
-}
-
-static void init_rand(struct sha256 *h, size_t *randidx,
-		      uint64_t seed1, uint64_t seed2,
-		      uint64_t transaction_num,
-		      enum permute_style style)
-{
-	struct sha256_ctx shactx;
-
-	sha256_init(&shactx);
-	if (seed1 < seed2) {
-		sha256_le64(&shactx, seed1);
-		sha256_le64(&shactx, seed2);
-	} else {
-		sha256_le64(&shactx, seed2);
-		sha256_le64(&shactx, seed1);
-	}
-	sha256_le64(&shactx, transaction_num);
-	sha256_u8(&shactx, style);
-	sha256_done(&shactx, h);
-	*randidx = 0;
-}
-
 static void init_map(size_t *map, size_t len)
 {
 	size_t i;
@@ -45,21 +13,6 @@ static void init_map(size_t *map, size_t len)
 		map[i] = i;
 }
 
-/* This map says where things ended up, eg. 0 might be in slot 3.  we
- * want to change it so map[0] = 3. */
-static void invert_map(size_t *map, size_t len)
-{
-	if (map) {
-		size_t i, newmap[len];
-
-		memset(newmap, 0, sizeof(newmap));
-		for (i = 0; i < len; i++) {
-			newmap[map[i]] = i;
-		}
-		memcpy(map, newmap, sizeof(newmap));
-	}
-}
-
 static bool input_better(const struct bitcoin_tx_input *a,
 			 const struct bitcoin_tx_input *b)
 {
@@ -108,13 +61,11 @@ static void swap_inputs(struct bitcoin_tx_input *inputs, size_t *map,
 	}
 }
 
-void permute_inputs(uint64_t seed1, uint64_t seed2, uint64_t tx_num,
+void permute_inputs(struct bitcoin_tx_input *inputs,
-		    struct bitcoin_tx_input *inputs,
 		    size_t num_inputs,
 		    size_t *map)
 {
-	struct sha256 h;
+	size_t i;
-	size_t i, randidx;
 
 	init_map(map, num_inputs);
 
@@ -124,16 +75,6 @@ void permute_inputs(uint64_t seed1, uint64_t seed2, uint64_t tx_num,
 		swap_inputs(inputs, map,
 			    i, i + find_best_in(inputs + i, num_inputs - i));
 	}
-
-	init_rand(&h, &randidx, seed1, seed2, tx_num, PERMUTE_INPUT_STYLE);
-		  
-	/* Now, Fisher-Yates shuffle, but using SHA256 as "random" source. */
-	for (i = 0; i + 1 < num_inputs; i++) {
-		size_t r = get_next_rand(&h, &randidx) % (num_inputs - i - 1);
-		swap_inputs(inputs, map, i, i + 1 + r);
-	}
-
-	invert_map(map, num_inputs);
 }
 
 static void swap_outputs(struct bitcoin_tx_output *outputs, size_t *map,
@@ -156,13 +97,23 @@ static void swap_outputs(struct bitcoin_tx_output *outputs, size_t *map,
 static bool output_better(const struct bitcoin_tx_output *a,
 			  const struct bitcoin_tx_output *b)
 {
+	size_t len;
+	int ret;
+
 	if (a->amount != b->amount)
 		return a->amount < b->amount;
 
-	if (a->script_length != b->script_length)
+	/* Lexographic sort. */
-		return a->script_length < b->script_length;
+	if (a->script_length < b->script_length)
+		len = a->script_length;
+	else
+		len = b->script_length;
 
-	return memcmp(a->script, b->script, a->script_length) < 0;
+	ret = memcmp(a->script, b->script, len);
+	if (ret != 0)
+		return ret < 0;
+
+	return a->script_length < b->script_length;
 }
 
 static size_t find_best_out(struct bitcoin_tx_output *outputs, size_t num)
@@ -176,13 +127,11 @@ static size_t find_best_out(struct bitcoin_tx_output *outputs, size_t num)
 	return best;
 }
 
-void permute_outputs(uint64_t seed1, uint64_t seed2, size_t tx_num,
+void permute_outputs(struct bitcoin_tx_output *outputs,
-		     struct bitcoin_tx_output *outputs,
 		     size_t num_outputs,
 		     size_t *map)
 {
-	struct sha256 h;
+	size_t i;
-	size_t i, randidx;
 
 	init_map(map, num_outputs);
 
@@ -192,14 +141,4 @@ void permute_outputs(uint64_t seed1, uint64_t seed2, size_t tx_num,
 		swap_outputs(outputs, map,
 			     i, i + find_best_out(outputs + i, num_outputs - i));
 	}
-
-	init_rand(&h, &randidx, seed1, seed2, tx_num, PERMUTE_OUTPUT_STYLE);
-		  
-	/* Now, Fisher-Yates shuffle, but using SHA256 as "random" source. */
-	for (i = 0; i + 1 < num_outputs; i++) {
-		size_t r = get_next_rand(&h, &randidx) % (num_outputs - i - 1);
-		swap_outputs(outputs, map, i, i + 1 + r);
-	}
-
-	invert_map(map, num_outputs);
 }

permute_tx.h

@@ -2,24 +2,14 @@
 #define LIGHTNING_PERMUTE_TX_H
 #include "bitcoin/tx.h"
 
-/* Given the two seeds, permute the transaction inputs.
+/* Permute the transaction into BIP69 order.
  * map[0] is set to the new index of input 0, etc.
  */
-void permute_inputs(uint64_t seed1, uint64_t seed2,
+void permute_inputs(struct bitcoin_tx_input *inputs,
-		    size_t transaction_num,
-		    struct bitcoin_tx_input *inputs,
 		    size_t num_inputs,
 		    size_t *map);
 
-void permute_outputs(uint64_t seed1, uint64_t seed2,
+void permute_outputs(struct bitcoin_tx_output *outputs,
-		     size_t transaction_num,
-		     struct bitcoin_tx_output *outputs,
 		     size_t num_outputs,
 		     size_t *map);
-
-enum permute_style {
-	PERMUTE_INPUT_STYLE = 0,
-	PERMUTE_OUTPUT_STYLE = 1
-};
-
 #endif /* LIGHTNING_PERMUTE_TX_H */

pkt.c

@@ -32,7 +32,6 @@ static struct pkt *to_pkt(const tal_t *ctx, Pkt__PktCase type, void *msg)
 }
 
 struct pkt *openchannel_pkt(const tal_t *ctx,
-			    u64 seed,
 			    const struct sha256 *revocation_hash,
 			    const struct pubkey *to_me,
 			    u64 commitment_fee,
@@ -45,7 +44,6 @@ struct pkt *openchannel_pkt(const tal_t *ctx,
 	assert(anchor->inputs);
 	assert(anchor->pubkey);
 
-	o.seed = seed;
 	o.revocation_hash = sha256_to_proto(ctx, revocation_hash);
 	o.final = pubkey_to_proto(ctx, to_me);
 	o.commitment_fee = commitment_fee;

pkt.h

@@ -31,7 +31,6 @@ struct pubkey;
 /**
  * openchannel_pkt - create an openchannel message
  * @ctx: tal context to allocate off.
- * @seed: psuedo-random seed to shuffle inputs.
  * @revocation_hash: first hash value generated from seed.
  * @to_me: the pubkey for the commit transactions' P2SH output.
  * @commitment_fee: the fee to use for commitment tx.
@@ -39,7 +38,6 @@ struct pubkey;
  * @anchor: the anchor transaction details.
  */
 struct pkt *openchannel_pkt(const tal_t *ctx,
-			    u64 seed,
 			    const struct sha256 *revocation_hash,
 			    const struct pubkey *to_me,
 			    u64 commitment_fee,

test-cli/open-channel.c

@@ -59,12 +59,6 @@ static BitcoinInput *parse_anchor_input(const tal_t *ctx, const char *spec)
 	return in;
 }
 
-/* FIXME: This is too weak, even for us! */
-static u64 weak_random64(void)
-{
-	return time(NULL);
-}
-
 /* Simple helper to open a channel. */
 int main(int argc, char *argv[])
 {
@@ -166,7 +160,7 @@ int main(int argc, char *argv[])
 	sha256(&revocation_hash,
 	       revocation_hash.u.u8, sizeof(revocation_hash.u.u8));
 
-	pkt = openchannel_pkt(ctx, weak_random64(), &revocation_hash, &outkey,
+	pkt = openchannel_pkt(ctx, &revocation_hash, &outkey,
 			      commit_tx_fee, locktime_seconds, &anchor);
 
 	if (!write_all(STDOUT_FILENO, pkt, pkt_totlen(pkt)))