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

_______________________________________________
Devl mailing list
Devl@freenetproject.org
http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Reply via email to