commit 4a1904c12665b98c356aba8e7b73a3f3fd508a5b Author: David Goulet dgoulet@torproject.org Date: Thu May 26 15:26:09 2016 -0400

prop250: Use the new dirvote_get_intermediate_param_value for AuthDirNumSRVAgreements

Signed-off-by: David Goulet dgoulet@torproject.org --- src/or/dirvote.c | 10 ++++++++- src/or/shared_random.c | 48 +++++++++++++++++++++++++++---------------- src/or/shared_random.h | 9 +++++++- src/test/test_shared_random.c | 34 +++--------------------------- 4 files changed, 50 insertions(+), 51 deletions(-)

diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 4ebea32..4fbd6b3 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1341,8 +1341,16 @@ networkstatus_compute_consensus(smartlist_t *votes, }

if (consensus_method >= MIN_METHOD_FOR_SHARED_RANDOM) { + int num_dirauth = get_n_authorities(V3_DIRINFO); + /* Default value of this is 2/3 of the total number of authorities. For + * instance, if we have 9 dirauth, the default value is 6. The following + * calculation will round it down. */ + int32_t num_srv_agreements = + dirvote_get_intermediate_param_value(param_list, + "AuthDirNumSRVAgreements", + (num_dirauth * 2) / 3); /* Add the shared random value. */ - char *srv_lines = sr_get_string_for_consensus(votes); + char *srv_lines = sr_get_string_for_consensus(votes, num_srv_agreements); if (srv_lines != NULL) { smartlist_add(chunks, srv_lines); } diff --git a/src/or/shared_random.c b/src/or/shared_random.c index 7915125..5cdf3d0 100644 --- a/src/or/shared_random.c +++ b/src/or/shared_random.c @@ -105,6 +105,12 @@ static const char current_srv_str[] = "shared-rand-current-value"; static const char commit_ns_str[] = "shared-rand-commit"; static const char sr_flag_ns_str[] = "shared-rand-participate";

+/* The value of the consensus param AuthDirNumSRVAgreements found in the + * vote. This is set once the consensus creation subsystem requests the + * SRV(s) that should be put in the consensus. We use this value to decide + * if we keep or not an SRV. */ +static int32_t num_srv_agreements_from_vote; + /* Return a heap allocated copy of the SRV <b>orig</b>. */ STATIC sr_srv_t * srv_dup(const sr_srv_t *orig) @@ -737,18 +743,6 @@ save_commit_to_state(sr_commit_t *commit) } }

-/* Return the number of required participants of the SR protocol. This is based - * on a consensus params. */ -static int -get_n_voters_for_srv_agreement(void) -{ - int num_dirauths = get_n_authorities(V3_DIRINFO); - /* If the params is not found, default value should always be the maximum - * number of trusted authorities. Let's not take any chances. */ - return networkstatus_get_param(NULL, "AuthDirNumSRVAgreements", - num_dirauths, 1, num_dirauths); -} - /* Return 1 if we should we keep an SRV voted by <b>n_agreements</b> auths. * Return 0 if we should ignore it. */ static int @@ -769,11 +763,9 @@ should_keep_srv(int n_agreements) * to keep it. */ if (sr_state_srv_is_fresh()) { /* Check if we have super majority for this new SRV value. */ - int num_required_agreements = get_n_voters_for_srv_agreement(); - - if (n_agreements < num_required_agreements) { + if (n_agreements < num_srv_agreements_from_vote) { log_notice(LD_DIR, "SR: New SRV didn't reach agreement [%d/%d]!", - n_agreements, num_required_agreements); + n_agreements, num_srv_agreements_from_vote); return 0; } } @@ -1253,9 +1245,14 @@ sr_get_string_for_vote(void) * * This is called when a consensus (any flavor) is bring created thus it * should NEVER change the state nor the state should be changed in between - * consensus creation. */ + * consensus creation. + * + * <b>num_srv_agreements</b> is taken from the votes thus the voted value + * that should be used. + * */ char * -sr_get_string_for_consensus(const smartlist_t *votes) +sr_get_string_for_consensus(const smartlist_t *votes, + int32_t num_srv_agreements) { char *srv_str; const or_options_t *options = get_options(); @@ -1269,6 +1266,9 @@ sr_get_string_for_consensus(const smartlist_t *votes) goto end; }

+ /* Set the global value of AuthDirNumSRVAgreements found in the votes. */ + num_srv_agreements_from_vote = num_srv_agreements; + /* Check the votes and figure out if SRVs should be included in the final * consensus. */ sr_srv_t *prev_srv = get_majority_srv_from_votes(votes, 0); @@ -1340,3 +1340,15 @@ sr_save_and_cleanup(void) sr_state_save(); sr_cleanup(); } + +#ifdef TOR_UNIT_TESTS + +/* Set the global value of number of SRV agreements so the test can play + * along by calling specific functions that don't parse the votes prior for + * the AuthDirNumSRVAgreements value. */ +void set_num_srv_agreements(int32_t value) +{ + num_srv_agreements_from_vote = value; +} + +#endif /* TOR_UNIT_TESTS */ diff --git a/src/or/shared_random.h b/src/or/shared_random.h index 6d68ad7..3922f33 100644 --- a/src/or/shared_random.h +++ b/src/or/shared_random.h @@ -112,7 +112,8 @@ void sr_handle_received_commits(smartlist_t *commits, sr_commit_t *sr_parse_commit(const smartlist_t *args); sr_srv_t *sr_parse_srv(const smartlist_t *args); char *sr_get_string_for_vote(void); -char *sr_get_string_for_consensus(const smartlist_t *votes); +char *sr_get_string_for_consensus(const smartlist_t *votes, + int32_t num_srv_agreements); void sr_commit_free(sr_commit_t *commit); void sr_srv_encode(char *dst, size_t dst_len, const sr_srv_t *srv);

@@ -156,4 +157,10 @@ STATIC void save_commit_during_reveal_phase(const sr_commit_t *commit);

#endif /* SHARED_RANDOM_PRIVATE */

+#ifdef TOR_UNIT_TESTS + +void set_num_srv_agreements(int32_t value); + +#endif /* TOR_UNIT_TESTS */ + #endif /* TOR_SHARED_RANDOM_H */ diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 8940045..b900350 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -830,14 +830,6 @@ get_test_vote_with_curr_srv(const char *srv) return vote; }

-/* Mock function to return the value located in the options instead of the - * consensus so we can modify it at will. */ -static networkstatus_t * -mock_networkstatus_get_latest_consensus(void) -{ - return mock_consensus; -} - /* Test the function that picks the right SRV given a bunch of votes. Make sure * that the function returns an SRV iff the majority/agreement requirements are * met. */ @@ -894,31 +886,13 @@ test_sr_get_majority_srv_from_votes(void *arg)

/* Now we achieve majority for SRV_1, but not the AuthDirNumSRVAgreements requirement. So still not picking an SRV. */ + set_num_srv_agreements(8); chosen_srv = get_majority_srv_from_votes(votes, 1); tt_assert(!chosen_srv);

/* We will now lower the AuthDirNumSRVAgreements requirement by tweaking the - * consensus parameter and we will try again. This time it should work. */ - { - char *my_net_params; - /* Set a dummy consensus parameter in every vote */ - SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, vote) { - vote->net_params = smartlist_new(); - smartlist_split_string(vote->net_params, - "AuthDirNumSRVAgreements=7", NULL, 0, 0); - } SMARTLIST_FOREACH_END(vote); - - /* Pretend you are making a consensus out of the votes */ - mock_consensus = tor_malloc_zero(sizeof(networkstatus_t)); - mock_consensus->net_params = smartlist_new(); - my_net_params = dirvote_compute_params(votes, 66, smartlist_len(votes)); - smartlist_split_string(mock_consensus->net_params, my_net_params, " ", - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); - } - - /* Use our fake consensus for finding consensus parameters. */ - MOCK(networkstatus_get_latest_consensus, - mock_networkstatus_get_latest_consensus); + * consensus parameter and we will try again. This time it should work. */ + set_num_srv_agreements(7); chosen_srv = get_majority_srv_from_votes(votes, 1); tt_assert(chosen_srv); tt_int_op(chosen_srv->num_reveals, ==, 42); @@ -928,8 +902,6 @@ test_sr_get_majority_srv_from_votes(void *arg) SMARTLIST_FOREACH(votes, networkstatus_t *, vote, networkstatus_vote_free(vote)); smartlist_free(votes); - networkstatus_vote_free(mock_consensus); - UNMOCK(networkstatus_get_latest_consensus); }

static void