[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #16943 [Tor]: Implement prop250 (Random Number Generation During Tor Voting)
#16943: Implement prop250 (Random Number Generation During Tor Voting)
-------------------------+------------------------------------
Reporter: asn | Owner: dgoulet
Type: enhancement | Status: needs_review
Priority: High | Milestone: Tor: 0.2.8.x-final
Component: Tor | Version:
Severity: Normal | Resolution:
Keywords: tor-hs | Actual Points:
Parent ID: #8244 | Points: large
Sponsor: SponsorR |
-------------------------+------------------------------------
Changes (by dgoulet):
* status: needs_revision => needs_review
Comment:
Replying to [comment:34 teor]:
> > > There are missed opportunities for const pointers in this commit.
> >
> > All fixed (hopefully).
>
> Except for the new smartlist_get_most_frequent_srv, which can take
`const smartlist_t *sl`.
> (It was added in 3b04ebdb94e29c0b2569ff8c64256f37f1389180.)
Fixed 136da34.
>
> > > Nitpick: get_majority_srv_from_votes() uses `unsigned int current`
for a boolean value. Tor typically uses int for boolean values.
> >
> > Should we really continue that "tradition"? :)
>
> I'm in favour of consistently using int, but it doesn't matter that
much.
> (We could decide to move to `stdbool.h`, and assert that our booleans
are actually 0 or 1, but I think that's a topic for another ticket.)
Fair enough. Fixed 279764a
>
> > > Why pick the microdesc consensus to update the state? Any particular
reason?
> > > Should we fall back to the NS consensus if there's no microdesc
consensus or SR values?
> > > {{{
> > > /* A new consensus has been created: pass it to the shared random
subsystem
> > > to update the SR state. */
> > > sr_act_post_consensus(pending[FLAV_MICRODESC].consensus);
> > > }}}
> >
> > It's the consensus that client uses so we use that consensus to set
our SRV
> > value post consensus thus having the same view as client. Does that
make sense?
> > You think we should use `FLAV_NS`?
>
> Most clients use the microdesc consensus, some still use NS.
>
> I don't mind which consensus we use, but:
> * I think we should have a comment explaining the arbitrary choice, and
I actually changed it to use the FLAV_NS which is the main one that if we
can't
create, problem! I also checked before using the consensus pointer that it
does
exists.
See 63abee2
> * I wonder what happens if one consensus fails, but the other doesn't
(can this even happen?).
Yeah, quite unsure but plausible from what I can see.
[snip]
> Now I see `7/724 TESTS FAILED. (1 skipped)`.
>
> 6 of these are the crashes above, the final one is:
>
Thanks to asn, we found out why the failures! See 30edde4.
See branch: `prop250_final_v5` with the fixups.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16943#comment:35>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs