* Robert Hailey <robert at emu.freenetproject.org> [2008-01-02 18:23:28]:
>
> On Jan 2, 2008, at 3:44 PM, Matthew Toseland wrote:
>
> > On Wednesday 02 January 2008 17:22, Robert Hailey wrote:
> >>
> >> On Dec 29, 2007, at 5:39 AM, Florent Daigni?re wrote:
> >>
> >>>>> synchronized void updateShouldDisconnectNow() {
> >>>>> //FIXME: We should not update VERIFIED unless we
> >>>>> HANDSHAKE WITH
> >>>>> THE NODE
> >>>>> - if (isConnected()) {
> >>>>> + if (isConnected() || verifiedIncompatibleOlderVersion ||
> >>>>> verifiedIncompatibleNewerVersion) {
> >>>>> verifiedIncompatibleOlderVersion =
> >>>>> forwardInvalidVersion();
> >>>>> verifiedIncompatibleNewerVersion =
> >>>>> reverseInvalidVersion();
> >>>>> }
> >>>>
> >>>> I suggest you call isRoutable() instead
> >>>>
> >>>> NextGen$
> >>>
> >>> Hmm, on a latter thought, I don't understand why it helps... nor why
> >>> we
> >>> are calling it from PacketSender
> >>>
> >>> What about getting rid of both the "if" branch and the call in
> >>> PacketSender ?
> >>
> >> In PacketSender this function is called ~precisely~ at the version
> >> transition time so that all peers will be re-evaluated concerning
> >> compatible versioning; this seems like an odd place to stick this
> >> code, but also updateShouldDisconnectNow() does not explicitly
> >> disconnect the peer node, it only updates the 'incompatible' flags.
> >
> > We don't want to disconnect. We want to stay connected and to not
> > route
> > requests to the node or accept them from it. We will still accept
> > e.g. Update
> > Over Mandatory traffic.
>
> Fair enough, but this is only confused semantics. I said disconnect as
> it is in the function name, perhaps a better name would be
> updateVersionRoutablity().
>
> >> Concerning the if branch I added, the intended behaviour change is
> >> that this function should only (1) allow the disconnection of a
> >> connected peer by setting the incompatible flag,
> >
> > No, we don't want to disconnect. We just want to make it non-
> > routable, by
> > updating verified*.
>
> I used the wrong term. It will only update the verified* booleans (and
> thus make routable or not) if connected. But see below...
>
> >> Now, if the nodes can handshake, this code has probably has little
> >> effect, as it simply further restricts one of only two places that
> >> the
> >> incompatible flags are set (the other being on a handshake). What it
> >> will do is if the nodes can't talk to each other (like in my re-keyed
> >> node case), is not show 'TOO_OLD'/'TOO_NEW', but instead
> >> 'DISCONNECTED'.
> >
> > So you are setting verified* to false, because it's not verified?
>
> Not setting it to false, but... not allowing it to be set to true if
> that node cannot reach us (e.g. disconnected).
>
> >> And I suppose in the same error case... if the node is
> >> restarted, it would never be able to verify that the peer is too old/
> >> new and fetch the ark (keeping the version number up-to-date).
> >
> > Again, we should always fetch the ARK.
>
> The original implementation appears to use verifiedIncompatible* to
> optimize against a time-based trigger (Version-Incompatibility) from a
> bunch of ark requests being started.
>
> On Jan 2, 2008, at 12:30 PM, David Sowder wrote:
> > I'm not sure of the thinking of others regarding changes to the
> > verifiedIncompatible variables since, but my thinking in my initial
> > implementation was that they would only be set after a handshake, thus
> > we already had a connected to them and ARKs weren't needed. That
> > won't
> > change until the peer re-handshakes after restarting with a different
> > version of the node's software.
>
> As best I can see, the verified* booleans have changed meanings since
> update-over-mandatory. If it is still useful to know if the peer node
> is presently-verified-{older/newer}, then we should split the booleans
> (isOlder,isVerifiedOlder); otherwise just fetch an ark even if it is
> an older (as Matthew suggested). In either case the misleading
> comments and variable names should be removed and/or changed.
We want to fetch ARKs in any case imo.
NextGen$
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20080103/0a0bd17f/attachment.pgp>
