prop224: Refactor pick_hsdir() to be used by both v2 and v3.

Also refactor rendclient.c to use the new hs_pick_hdsir() func.

Signed-off-by: David Goulet <dgoulet@torproject.org>

3 files changed, 22 insertions, 16 deletions

diff --git a/src/or/hs_common.c b/src/or/hs_common.c
index dbd3848332..bc44265d53 100644
--- a/src/or/hs_common.c
+++ b/src/or/hs_common.c
@@ -21,6 +21,7 @@
 #include "hs_service.h"
 #include "rendcommon.h"
 #include "rendservice.h"
+#include "routerset.h"
 #include "router.h"
 #include "routerset.h"
 #include "shared_random.h"
@@ -1491,35 +1492,33 @@ hs_purge_last_hid_serv_requests(void)
 
 /***********************************************************************/
 
-/** This returns a good valid hs dir that should be used for the given
- * descriptor id.
+/** Given the list of responsible HSDirs in <b>responsible_dirs</b>, pick the
+ *  one that we should use to fetch a descriptor right now. Take into account
+ *  previous failed attempts at fetching this descriptor from HSDirs using the
+ *  string identifier <b>req_key_str</b>.
+ *
+ *  Steals ownership of <b>responsible_dirs</b>.
  *
- * Return NULL on error else the hsdir node pointer. */
+ *  Return the routerstatus of the chosen HSDir if successful, otherwise return
+ *  NULL if no HSDirs are worth trying right now. */
 routerstatus_t *
-pick_hsdir(const char *desc_id, const char *desc_id_base32)
+hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str)
 {
-  smartlist_t *responsible_dirs = smartlist_new();
   smartlist_t *usable_responsible_dirs = smartlist_new();
   const or_options_t *options = get_options();
   routerstatus_t *hs_dir;
   time_t now = time(NULL);
   int excluded_some;
 
-  tor_assert(desc_id);
-  tor_assert(desc_id_base32);
-
-  /* Determine responsible dirs. Even if we can't get all we want, work with
-   * the ones we have. If it's empty, we'll notice below. */
-  hid_serv_get_responsible_directories(responsible_dirs, desc_id);
+  tor_assert(req_key_str);
 
-  /* Clean request history first. */
+  /* Clean outdated request history first. */
   hs_clean_last_hid_serv_requests(now);
 
   /* Only select those hidden service directories to which we did not send a
    * request recently and for which we have a router descriptor here. */
   SMARTLIST_FOREACH_BEGIN(responsible_dirs, routerstatus_t *, dir) {
-    time_t last = hs_lookup_last_hid_serv_request(dir, desc_id_base32,
-                                                  0, 0);
+    time_t last = hs_lookup_last_hid_serv_request(dir, req_key_str, 0, 0);
     const node_t *node = node_get_by_id(dir->identity_digest);
     if (last + hs_hsdir_requery_period(options) >= now ||
         !node || !node_has_descriptor(node)) {
@@ -1553,7 +1552,7 @@ pick_hsdir(const char *desc_id, const char *desc_id_base32)
   } else {
     /* Remember that we are requesting a descriptor from this hidden service
      * directory now. */
-    hs_lookup_last_hid_serv_request(hs_dir, desc_id_base32, now, 1);
+    hs_lookup_last_hid_serv_request(hs_dir, req_key_str, now, 1);
   }
 
   return hs_dir;
diff --git a/src/or/hs_common.h b/src/or/hs_common.h
index 4692a3a019..6ad68d0a8a 100644
--- a/src/or/hs_common.h
+++ b/src/or/hs_common.h
@@ -221,6 +221,8 @@ int32_t hs_get_hsdir_spread_store(void);
 void hs_get_responsible_hsdirs(const ed25519_public_key_t *blinded_pk,
                                uint64_t time_period_num, int is_next_period,
                                int is_client, smartlist_t *responsible_dirs);
+routerstatus_t *hs_pick_hsdir(smartlist_t *responsible_dirs,
+                              const char *req_key_str);
 
 time_t hs_hsdir_requery_period(const or_options_t *options);
 time_t hs_lookup_last_hid_serv_request(routerstatus_t *hs_dir,
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 9cf4059469..a6fb88624d 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -497,7 +497,12 @@ directory_get_from_hs_dir(const char *desc_id,
 
   /* Automatically pick an hs dir if none given. */
   if (!rs_hsdir) {
-    hs_dir = pick_hsdir(desc_id, desc_id_base32);
+    /* Determine responsible dirs. Even if we can't get all we want, work with
+     * the ones we have. If it's empty, we'll notice in hs_pick_hsdir(). */
+    smartlist_t *responsible_dirs = smartlist_new();
+    hid_serv_get_responsible_directories(responsible_dirs, desc_id);
+
+    hs_dir = hs_pick_hsdir(responsible_dirs, desc_id_base32);
     if (!hs_dir) {
       /* No suitable hs dir can be found, stop right now. */
       control_event_hs_descriptor_failed(rend_query, NULL, "QUERY_NO_HSDIR");