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.
>
> 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*.
> 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'.
So you are setting verified* to false, because it's not verified?
> 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.
>
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20080102/e532040c/attachment.pgp>