prop224: Refactor descriptor reupload logic.

We refactor the descriptor reupload logic to be similar to the v2 logic
where we update a global 'consider_republishing_rend_descriptors' flag
and then we use that to check for hash ring changes during the global
hidden service callbacks.

This fixes bugs where we would inspect the hash ring immediately as we
receive new dirinfo (e.g. consensus) but before running the hidden
service housekeeping events. That was leaving us in an inconsistent
state wrt hsdir indices and causing bugs all around.

2 files changed, 16 insertions, 4 deletions

diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c
index ec938ff5d1..b1e47adf24 100644
--- a/src/test/test_hs_common.c
+++ b/src/test/test_hs_common.c
@@ -626,12 +626,11 @@ test_desc_reupload_logic(void *arg)
                                       curr_hsdir_index, nickname, 1);
   }
 
-  /* Now call router_dir_info_changed() again and see that it detected the hash
-     ring change and updated the upload time */
+  /* Now call service_desc_hsdirs_changed() and see that it detected the hash
+     ring change */
   time_t now = approx_time();
   tt_assert(now);
-  router_dir_info_changed();
-  tt_int_op(desc->next_upload_time, OP_EQ, now);
+  tt_int_op(service_desc_hsdirs_changed(service, desc), OP_EQ, 1);
 
   /* Now pretend that the descriptor changed, and order a reupload to all
      HSDirs. Make sure that the set of previous HSDirs was cleared. */
diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c
index cf1c36a528..4c7880c0ee 100644
--- a/src/test/test_hs_service.c
+++ b/src/test/test_hs_service.c
@@ -1005,6 +1005,13 @@ test_rotate_descriptors(void *arg)
             OP_EQ, 0);
   tt_assert(service->desc_next == NULL);
 
+  /* Now let's re-create desc_next and get out of overlap period. We should
+     test that desc_current gets replaced by desc_next, and desc_next becomes
+     NULL. */
+  desc_next = service_descriptor_new();
+  desc_next->next_upload_time = 240; /* Our marker to recognize it. */
+  service->desc_next = desc_next;
+
   /* Going out of the overlap period. */
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 12:00:00 UTC",
                            &mock_ns.valid_after);
@@ -1017,6 +1024,12 @@ test_rotate_descriptors(void *arg)
   tt_mem_op(service->desc_current, OP_EQ, desc_next, sizeof(*desc_next));
   tt_assert(service->desc_next == NULL);
 
+  /* Calling rotate_all_descriptors() another time should do nothing */
+  rotate_all_descriptors(now);
+  tt_int_op(service->state.in_overlap_period, OP_EQ, 0);
+  tt_mem_op(service->desc_current, OP_EQ, desc_next, sizeof(*desc_next));
+  tt_assert(service->desc_next == NULL);
+
  done:
   hs_free_all();
   UNMOCK(circuit_mark_for_close_);