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.
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, or (2) remove an
incompatible flag (e.g. on the fetch of an ark).
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'. 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).
Really this change just brings the functionality of the
incompatibility flags closer to the comments thereon: that they are
only set if we have verified incompatibility through a handshake. In
practice, it appears that they could still be set upon receiving a new
reference (ark?) as well.
--
Robert Hailey
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20080102/0836e813/attachment.html>
