ExchangeUnion / xud

Exchange Union Daemon 🔁 ⚡️
https://exchangeunion.com
GNU Affero General Public License v3.0
115 stars 44 forks source link

Advertise version #154

Closed moshababo closed 5 years ago

moshababo commented 6 years ago

Advertise xud version on handshaking. When received, verify that it exceeds MIN_VERSION. If not, disconnect from peer.

sangaman commented 6 years ago

Advertising is handled by #243, but we still need to work out compatible versions.

kilrau commented 6 years ago

Nice - already almost done, isn't it just that with every release tag we should define a MIN_VERSION? It will be different every time. Sometimes we have breaking changes, sometimes we don't.

kilrau commented 6 years ago

yep, just noticed the version exchange on handshake:

2018-8-16 09:16:15 [P2P] debug: Received data (127.0.0.1:43716): {"header":{"id":"b2f606f0-a16f-11e8-8379-0d764156e61f","hash":"VOiI9r6Gxbut8T/AO4+jBg==","type":"HELLO"},"body":{"version":"1.0.0-prealpha.1","pairs":["BTC/LTC"],"nodePubKey":"03d2fc7499fa9ee64290acdd12cd653e2c63963d660642422903c3c66c5dc98931","addresses":[]}}
moshababo commented 6 years ago

Do we want this for alpha.1? I thought we said we want to wait with the MIN_VERSION compatibility checks.

kilrau commented 6 years ago

Ehm yes, if there's a good reason to wait, we'll wait @moshababo

sangaman commented 6 years ago

Personally I think it can wait. At least until we feel the p2p protocol is at least close to "complete", and then mark that point as the first minimum version we enforce.

kilrau commented 6 years ago

What do you think, should we do this now (after https://github.com/ExchangeUnion/xud/issues/152 & https://github.com/ExchangeUnion/xud/issues/616)? @moshababo @sangaman @offerm

offerm commented 6 years ago

AFAIK this is still not urgent.

In any case, I would advertise a P2P protocol level (number) and not xud version. Xud may change many times without a breaking change in the P2P protocol . I would consider a major version and a minor version. Major version must match. Minor version can be used to check if a specific feature is available by a peer.

This would make maintaining much easier.

my 2 cents

kilrau commented 6 years ago

There can be breaking changes in a lot of places. P2P protocol, swap protocol, order data formats etc.

Instead of thinking what could be breaking and define version numbers for all of them, I'd still go with the xud version and we just set it as not compatible if one of the above has breaking changes. Other best practices from your experience? @offerm @moshababo @sangaman

Since breaking changes in upcoming alpha.3 and probably also alpha.4 release, moved to alpha.5.

sangaman commented 6 years ago

I agree with Kilian, that we don't need a separate protocol version because we aren't looking for exact matches on version, just minimum requirements. So if version 3 is supported, we will accept a connection with version 4 given that version 4 might not have breaking p2p changes. However if a version does have breaking p2p changes, it should have a higher minimum version requirement for itself.

Where a protocol version might be useful is if there are ever alternative implementations.

I agree it's not urgent though - for now we always be trying to use at least the latest published version since things are changing rapidly.