* David Sowder <[EMAIL PROTECTED]> [2008-01-03 07:36:39]:
> Florent Daignière wrote:
>> * Robert Hailey <[EMAIL PROTECTED]> [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.
>>
> And in my opinion, fetching an ARK for a connected peer is pointless, but
> perhaps we agree in our assessments and I misunderstand by thinking you
> mean to _always_ fetch ARKs regardless of current peer status.It's not pointless. We want to keep an up to date "edition" for the ark so that when we will need it we will be able to reach it... Of course we could exchange and update the edition number on handshake... but atm we don't. NextGen$
signature.asc
Description: Digital signature
_______________________________________________ Devl mailing list Devl@freenetproject.org http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
