protocol: remove ack fields.

As per lightning-rfc commit 8ee09e749990a11fa53bea03d5961cfde4be4616, we remove the acks from the protocol now they're no longer needed (and all the infrastructure). We also place the commit number in the commit_info where it logically belongs, removing it from the peer struct. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2026-01-10 17:44:22 +01:00 · 2016-05-26 15:25:24 +09:30
parent 2bf43f1ebd
commit 45a6f81c3c
8 changed files with 53 additions and 148 deletions
--- a/daemon/cryptopkt.c
+++ b/daemon/cryptopkt.c
@@ -93,13 +93,6 @@ static void setup_crypto(struct dir_state *dir,
 	dir->cpkt = NULL;
 }

-struct ack {
-	struct list_node list;
-	u64 pktnum;
-	void (*ack_cb)(struct peer *peer, void *);
-	void *ack_arg;
-};
-
 struct io_data {
 	/* Stuff we need to keep around to talk to peer. */
 	struct dir_state in, out;
@@ -112,9 +105,6 @@ struct io_data {
 	
 	/* For negotiation phase. */
 	struct key_negotiate *neg;
-
-	/* Tracking what needs acks. */
-	struct list_head acks;
 };

 static void *proto_tal_alloc(void *allocator_data, size_t size)
@@ -236,25 +226,6 @@ static struct crypto_pkt *encrypt_pkt(struct peer *peer, const Pkt *pkt,
 	return cpkt;
 }

-void peer_process_acks(struct peer *peer, uint64_t acknum)
-{
-	struct io_data *iod = peer->io_data;
-	struct ack *ack;
-
-	while ((ack = list_top(&iod->acks, struct ack, list)) != NULL) {
-		if (acknum < ack->pktnum)
-			break;
-		ack->ack_cb(peer, ack->ack_arg);
-		list_del_from(&iod->acks, &ack->list);
-		tal_free(ack);
-	}
-}
-
-uint64_t peer_outgoing_ack(const struct peer *peer)
-{
-	return peer->io_data->in.count;
-}
-
 static struct io_plan *decrypt_body(struct io_conn *conn, struct peer *peer)
 {
 	struct io_data *iod = peer->io_data;
@@ -323,13 +294,11 @@ struct io_plan *peer_read_packet(struct io_conn *conn,
 }

 /* Caller must free data! */
-struct io_plan *peer_write_packet_(struct io_conn *conn,
-				   struct peer *peer,
-				   const Pkt *pkt,
-				   void (*ack_cb)(struct peer *peer, void *),
-				   void *ack_arg,
-				   struct io_plan *(*next)(struct io_conn *,
-							   struct peer *))
+struct io_plan *peer_write_packet(struct io_conn *conn,
+				  struct peer *peer,
+				  const Pkt *pkt,
+				  struct io_plan *(*next)(struct io_conn *,
+							  struct peer *))
 {
 	struct io_data *iod = peer->io_data;
 	size_t totlen;
@@ -340,15 +309,6 @@ struct io_plan *peer_write_packet_(struct io_conn *conn,

 	iod->out.cpkt = encrypt_pkt(peer, pkt, &totlen);

-	/* Set up ack callback if any. */
-	if (ack_cb) {
-		struct ack *ack = tal(peer, struct ack);
-		ack->pktnum = peer->io_data->out.count;
-		ack->ack_cb = ack_cb;
-		ack->ack_arg = ack_arg;
-		list_add_tail(&iod->acks, &ack->list);
-	}
-
 	/* We don't add to count for authenticate case. */
 	if (pkt->pkt_case != PKT__PKT_AUTH)
 		peer->io_data->out.count++;
@@ -507,7 +467,7 @@ static struct io_plan *keys_exchanged(struct io_conn *conn, struct peer *peer)

 	/* FIXME: Free auth afterwards. */
 	auth = authenticate_pkt(peer, &peer->dstate->id, &sig);
-	return peer_write_packet(conn, peer, auth, NULL, NULL, receive_proof);
+	return peer_write_packet(conn, peer, auth, receive_proof);
 }

 /* Read and ignore any extra bytes... */
@@ -602,7 +562,6 @@ struct io_plan *peer_crypto_setup(struct io_conn *conn, struct peer *peer,
 	BUILD_ASSERT(sizeof(struct crypto_pkt) == 20);

 	peer->io_data = tal(peer, struct io_data);
-	list_head_init(&peer->io_data->acks);

 	/* We store negotiation state here. */
 	neg = peer->io_data->neg = tal(peer->io_data, struct key_negotiate);
--- a/daemon/cryptopkt.h
+++ b/daemon/cryptopkt.h
@@ -18,25 +18,9 @@ struct io_plan *peer_read_packet(struct io_conn *conn,
 				 struct io_plan *(*cb)(struct io_conn *,
 						       struct peer *));

-struct io_plan *peer_write_packet_(struct io_conn *conn,
-				   struct peer *peer,
-				   const Pkt *pkt,
-				   void (*ack_cb)(struct peer *peer, void *),
-				   void *ack_arg,
-				   struct io_plan *(*next)(struct io_conn *,
-							   struct peer *));
-
-#define peer_write_packet(conn, peer, pkt, ack_cb, ack_arg, next) \
-	peer_write_packet_((conn), (peer), (pkt),			\
-			   typesafe_cb_preargs(void, void *,		\
-					       (ack_cb), (ack_arg),	\
-					       struct peer *),		\
-			   (ack_arg), (next))
-
-/* Acknowledgements are contained in some messages: caller must call this */
-void peer_process_acks(struct peer *peer, uint64_t ack);
-
-/* Ack counter for outgoing packets. */
-uint64_t peer_outgoing_ack(const struct peer *peer);
-
+struct io_plan *peer_write_packet(struct io_conn *conn,
+				  struct peer *peer,
+				  const Pkt *pkt,
+				  struct io_plan *(*next)(struct io_conn *,
+							  struct peer *));
 #endif /* LIGHTNING_DAEMON_CRYPTOPKT_H */
--- a/daemon/packets.c
+++ b/daemon/packets.c
@@ -64,15 +64,11 @@ static Pkt *make_pkt(const tal_t *ctx, Pkt__PktCase type, const void *msg)
 	return pkt;
 }

-static void queue_raw_pkt(struct peer *peer, Pkt *pkt,
-			  void (*ack_cb)(struct peer *peer, void *arg),
-			  void *ack_arg)
+static void queue_raw_pkt(struct peer *peer, Pkt *pkt)
 {
 	size_t n = tal_count(peer->outpkt);
 	tal_resize(&peer->outpkt, n+1);
-	peer->outpkt[n].pkt = pkt;
-	peer->outpkt[n].ack_cb = ack_cb;
-	peer->outpkt[n].ack_arg = ack_arg;
+	peer->outpkt[n] = pkt;

 	/* In case it was waiting for output. */
 	io_wake(peer);
@@ -80,7 +76,7 @@ static void queue_raw_pkt(struct peer *peer, Pkt *pkt,

 static void queue_pkt(struct peer *peer, Pkt__PktCase type, const void *msg)
 {
-	queue_raw_pkt(peer, make_pkt(peer, type, msg), NULL, NULL);
+	queue_raw_pkt(peer, make_pkt(peer, type, msg));
 }

 void queue_pkt_open(struct peer *peer, OpenChannel__AnchorOffer anchor)
@@ -269,6 +265,7 @@ void queue_pkt_commit(struct peer *peer)

 	/* Create new commit info for this commit tx. */
 	ci->prev = peer->remote.commit;
+	ci->commit_num = ci->prev->commit_num + 1;
 	ci->revocation_hash = peer->remote.next_revocation_hash;
 	/* BOLT #2:
 	 *
@@ -312,7 +309,6 @@ void queue_pkt_commit(struct peer *peer)
 	/* Now send message */
 	update_commit__init(u);
 	u->sig = signature_to_proto(u, &ci->sig->sig);
-	u->ack = peer_outgoing_ack(peer);

 	queue_pkt(peer, PKT__PKT_UPDATE_COMMIT, u);
 }
@@ -370,7 +366,6 @@ void queue_pkt_revocation(struct peer *peer)

 	update_revocation__init(u);

-	assert(peer->commit_tx_counter > 0);
 	assert(peer->local.commit);
 	assert(peer->local.commit->prev);
 	assert(!peer->local.commit->prev->revocation_preimage);
@@ -380,7 +375,7 @@ void queue_pkt_revocation(struct peer *peer)

 	peer->local.commit->prev->revocation_preimage
 		= tal(peer->local.commit->prev, struct sha256);
-	peer_get_revocation_preimage(peer, peer->commit_tx_counter-1,
+	peer_get_revocation_preimage(peer, peer->local.commit->prev->commit_num,
 				     peer->local.commit->prev->revocation_preimage);
 	peer_check_if_cleared(peer);
 	u->revocation_preimage
@@ -388,7 +383,6 @@ void queue_pkt_revocation(struct peer *peer)

 	u->next_revocation_hash = sha256_to_proto(u,
 						  &peer->local.next_revocation_hash);
-	u->ack = peer_outgoing_ack(peer);

 	queue_pkt(peer, PKT__PKT_UPDATE_REVOCATION, u);

@@ -424,7 +418,7 @@ Pkt *pkt_err(struct peer *peer, const char *msg, ...)

 void queue_pkt_err(struct peer *peer, Pkt *err)
 {
-	queue_raw_pkt(peer, err, NULL, NULL);
+	queue_raw_pkt(peer, err);
 }

 void queue_pkt_close_clearing(struct peer *peer)
@@ -745,6 +739,7 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt)

 	/* Create new commit info for this commit tx. */
 	ci->prev = peer->local.commit;
+	ci->commit_num = ci->prev->commit_num + 1;
 	ci->revocation_hash = peer->local.next_revocation_hash;

 	/* BOLT #2:
@@ -779,8 +774,7 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt)

 	/* Switch to the new commitment. */
 	peer->local.commit = ci;
-	peer->commit_tx_counter++;
-	peer_get_revocation_hash(peer, peer->commit_tx_counter + 1,
+	peer_get_revocation_hash(peer, ci->commit_num + 1,
 				 &peer->local.next_revocation_hash);

 	peer_check_if_cleared(peer);
--- a/daemon/peer.c
+++ b/daemon/peer.c
@@ -145,6 +145,19 @@ static void peer_breakdown(struct peer *peer)
 		log_info(peer->log, "Peer breakdown: nothing to do");
 }

+static bool peer_uncommitted_changes(const struct peer *peer)
+{
+	/* Not initialized yet? */
+	if (!peer->remote.staging_cstate
+	    || !peer->remote.commit
+	    || !peer->remote.commit->cstate)
+		return false;
+
+	/* We could have proposed changes to their commit */
+	return peer->remote.staging_cstate->changes
+		!= peer->remote.commit->cstate->changes;
+}
+
 static void state_single(struct peer *peer,
 			 const enum state_input input,
 			 const union input *idata)
@@ -168,8 +181,12 @@ static void state_single(struct peer *peer,
 		break;
 	}

+	/* If we added uncommitted changes, we should have set them to send. */
+	if (peer_uncommitted_changes(peer))
+		assert(peer->commit_timer);
+	
 	if (tal_count(peer->outpkt) > old_outpkts) {
-		Pkt *outpkt = peer->outpkt[old_outpkts].pkt;
+		Pkt *outpkt = peer->outpkt[old_outpkts];
 		log_add(peer->log, " (out %s)", pkt_name(outpkt->pkt_case));
 	}
 	if (broadcast)
@@ -313,7 +330,7 @@ void peer_check_if_cleared(struct peer *peer)

 static struct io_plan *pkt_out(struct io_conn *conn, struct peer *peer)
 {
-	struct out_pkt out;
+	Pkt *out;
 	size_t n = tal_count(peer->outpkt);

 	if (peer->fake_close)
@@ -329,8 +346,7 @@ static struct io_plan *pkt_out(struct io_conn *conn, struct peer *peer)
 	out = peer->outpkt[0];
 	memmove(peer->outpkt, peer->outpkt + 1, (sizeof(*peer->outpkt)*(n-1)));
 	tal_resize(&peer->outpkt, n-1);
-	return peer_write_packet(conn, peer, out.pkt, out.ack_cb, out.ack_arg,
-				 pkt_out);
+	return peer_write_packet(conn, peer, out, pkt_out);
 }

 static struct io_plan *pkt_in(struct io_conn *conn, struct peer *peer)
@@ -342,14 +358,6 @@ static struct io_plan *pkt_in(struct io_conn *conn, struct peer *peer)

 	/* We ignore packets if they tell us to. */
 	if (!peer->fake_close && peer->cond != PEER_CLOSED) {
-		/* These two packets contain acknowledgements. */
-		if (idata.pkt->pkt_case == PKT__PKT_UPDATE_COMMIT)
-			peer_process_acks(peer,
-					  idata.pkt->update_commit->ack);
-		else if (idata.pkt->pkt_case == PKT__PKT_UPDATE_REVOCATION)
-			peer_process_acks(peer,
-					  idata.pkt->update_revocation->ack);
-
 		state_event(peer, peer->inpkt->pkt_case, &idata);
 	}

@@ -479,12 +487,11 @@ static struct peer *new_peer(struct lightningd_state *dstate,
 	peer->io_data = NULL;
 	peer->secrets = NULL;
 	list_head_init(&peer->watches);
-	peer->outpkt = tal_arr(peer, struct out_pkt, 0);
+	peer->outpkt = tal_arr(peer, Pkt *, 0);
 	peer->curr_cmd.cmd = INPUT_NONE;
 	list_head_init(&peer->pending_cmd);
 	list_head_init(&peer->pending_input);
 	list_head_init(&peer->outgoing_txs);
-	peer->commit_tx_counter = 0;
 	peer->close_watch_timeout = NULL;
 	peer->anchor.watches = NULL;
 	peer->cur_commit.watch = NULL;
--- a/daemon/peer.h
+++ b/daemon/peer.h
@@ -55,6 +55,8 @@ struct anchor_input {
 struct commit_info {
 	/* Previous one, if any. */
 	struct commit_info *prev;
+	/* Commit number (0 == from open) */
+	u64 commit_num;
 	/* Revocation hash. */
 	struct sha256 revocation_hash;
 	/* Commit tx. */
@@ -148,7 +150,7 @@ struct peer {
 	Pkt *inpkt;

 	/* Queue of output packets. */
-	struct out_pkt *outpkt;
+	Pkt **outpkt;

 	/* Anchor tx output */
 	struct {
@@ -172,9 +174,6 @@ struct peer {
 		struct txwatch *watch;
 	} cur_commit;

-	/* Number of HTLC updates (== number of previous commit txs) */
-	u64 commit_tx_counter;
-
 	/* Counter to make unique HTLC ids. */
 	u64 htlc_id_counter;

--- a/lightning.pb-c.c
+++ b/lightning.pb-c.c
@@ -1902,7 +1902,7 @@ const ProtobufCMessageDescriptor update_fail_htlc__descriptor =
  (ProtobufCMessageInit) update_fail_htlc__init,
  NULL,NULL,NULL    /* reserved[123] */
 };
-static const ProtobufCFieldDescriptor update_commit__field_descriptors[2] =
+static const ProtobufCFieldDescriptor update_commit__field_descriptors[1] =
 {
  {
    "sig",
@@ -1916,27 +1916,14 @@ static const ProtobufCFieldDescriptor update_commit__field_descriptors[2] =
    0,             /* flags */
    0,NULL,NULL    /* reserved1,reserved2, etc */
  },
-  {
-    "ack",
-    2,
-    PROTOBUF_C_LABEL_REQUIRED,
-    PROTOBUF_C_TYPE_UINT64,
-    0,   /* quantifier_offset */
-    offsetof(UpdateCommit, ack),
-    NULL,
-    NULL,
-    0,             /* flags */
-    0,NULL,NULL    /* reserved1,reserved2, etc */
-  },
 };
 static const unsigned update_commit__field_indices_by_name[] = {
-  1,   /* field[1] = ack */
  0,   /* field[0] = sig */
 };
 static const ProtobufCIntRange update_commit__number_ranges[1 + 1] =
 {
  { 1, 0 },
-  { 0, 2 }
+  { 0, 1 }
 };
 const ProtobufCMessageDescriptor update_commit__descriptor =
 {
@@ -1946,14 +1933,14 @@ const ProtobufCMessageDescriptor update_commit__descriptor =
  "UpdateCommit",
  "",
  sizeof(UpdateCommit),
-  2,
+  1,
  update_commit__field_descriptors,
  update_commit__field_indices_by_name,
  1,  update_commit__number_ranges,
  (ProtobufCMessageInit) update_commit__init,
  NULL,NULL,NULL    /* reserved[123] */
 };
-static const ProtobufCFieldDescriptor update_revocation__field_descriptors[3] =
+static const ProtobufCFieldDescriptor update_revocation__field_descriptors[2] =
 {
  {
    "revocation_preimage",
@@ -1979,28 +1966,15 @@ static const ProtobufCFieldDescriptor update_revocation__field_descriptors[3] =
    0,             /* flags */
    0,NULL,NULL    /* reserved1,reserved2, etc */
  },
-  {
-    "ack",
-    3,
-    PROTOBUF_C_LABEL_REQUIRED,
-    PROTOBUF_C_TYPE_UINT64,
-    0,   /* quantifier_offset */
-    offsetof(UpdateRevocation, ack),
-    NULL,
-    NULL,
-    0,             /* flags */
-    0,NULL,NULL    /* reserved1,reserved2, etc */
-  },
 };
 static const unsigned update_revocation__field_indices_by_name[] = {
-  2,   /* field[2] = ack */
  1,   /* field[1] = next_revocation_hash */
  0,   /* field[0] = revocation_preimage */
 };
 static const ProtobufCIntRange update_revocation__number_ranges[1 + 1] =
 {
  { 1, 0 },
-  { 0, 3 }
+  { 0, 2 }
 };
 const ProtobufCMessageDescriptor update_revocation__descriptor =
 {
@@ -2010,7 +1984,7 @@ const ProtobufCMessageDescriptor update_revocation__descriptor =
  "UpdateRevocation",
  "",
  sizeof(UpdateRevocation),
-  3,
+  2,
  update_revocation__field_descriptors,
  update_revocation__field_indices_by_name,
  1,  update_revocation__number_ranges,
--- a/lightning.pb-c.h
+++ b/lightning.pb-c.h
@@ -368,7 +368,7 @@ struct  _UpdateFailHtlc


 /*
- * Commit all the staged HTLCs.
+ * Commit all the staged changes.
 */
 struct  _UpdateCommit
 {
@@ -377,14 +377,10 @@ struct  _UpdateCommit
   * Signature for your new commitment tx.
   */
  Signature *sig;
-  /*
-   * How many (non-authenticate) packets we've already received
-   */
-  uint64_t ack;
 };
 #define UPDATE_COMMIT__INIT \
 { PROTOBUF_C_MESSAGE_INIT (&update_commit__descriptor) \
-    , NULL, 0 }
+    , NULL }


 /*
@@ -401,14 +397,10 @@ struct  _UpdateRevocation
   * Revocation hash for my next commit transaction
   */
  Sha256Hash *next_revocation_hash;
-  /*
-   * How many (non-authenticate) packets we've already received
-   */
-  uint64_t ack;
 };
 #define UPDATE_REVOCATION__INIT \
 { PROTOBUF_C_MESSAGE_INIT (&update_revocation__descriptor) \
-    , NULL, NULL, 0 }
+    , NULL, NULL }


 /*
--- a/lightning.proto
+++ b/lightning.proto
@@ -153,12 +153,10 @@ message update_fail_htlc {
  required fail_reason reason = 2;
 }

-// Commit all the staged HTLCs.
+// Commit all the staged changes.
 message update_commit {
  // Signature for your new commitment tx.
  required signature sig = 1;
-  // How many (non-authenticate) packets we've already received
-  required uint64 ack = 2;
 }

 // Complete the update.
@@ -167,8 +165,6 @@ message update_revocation {
  required sha256_hash revocation_preimage = 1;
  // Revocation hash for my next commit transaction
  required sha256_hash next_revocation_hash = 2;
-  // How many (non-authenticate) packets we've already received
-  required uint64 ack = 3;
 }

 // Start clearing out the channel HTLCs so we can close it