Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Masternodes should be allowed to annonce ipv4 addresses only from now #1065

Merged
merged 2 commits into from
Oct 10, 2016
Merged

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Oct 9, 2016

No description provided.

@UdjinM6 UdjinM6 changed the title masternode should use ipv4 only from now Masternodes should be allowed to annonce ipv4 addresses only from now Oct 9, 2016
@schinzelh
Copy link

schinzelh commented Oct 9, 2016

Controversial, and my heart is bleeding. But IHMO a quick win for our "heterogenous network connectivity issue" until we have a better solution.

utACK

img_0227

@crowning-
Copy link

So about 140 Masternodes need to change providers?

Anyway...I guess this is in preparation to directly connect to nodes for IP verification?

If so, I'd rather have 2 individual Masternodes lists (IPv4 and IPv6. We ignore TOR for now because of lack of nodes), and IPv4 nodes verify IPv4 nodes and IPv6 nodes verify IPv6 nodes.
Each node knows in which network it runs, so if your implementation works for IPv4 it'll work for IPv6 as well.

There's no reason to exclude IPv6.

@thelazier
Copy link

The reason to exclude IPv6 as masternode is because IPv4 only nodes don't have access to these IPv6 masternodes so IPv4 should be mandatory requirement of masternode service, IMO.

@crowning-
Copy link

IPv4 only nodes don't have access to these IPv6 masternodes

I know, that's why I wrote

IPv4 nodes verify IPv4 nodes and IPv6 nodes verify IPv6 nodes

@bertlebbert
Copy link

bertlebbert commented Oct 9, 2016

Though both is nice, I would think IPv6 only should not be excluded... Ultimately IPv6 only will become the norm and the question will become, "Why even bother with IPv4?"

@UdjinM6
Copy link
Author

UdjinM6 commented Oct 9, 2016

There is no reason to have ipv6 just to have ipv6, there must be some usefulness of it. The reason to exclude all non-ipv4 is exactly because they do not provide service they are paid for - connectivity and mixing. It's still very rare this days for end users to have ipv6 connectivity, most of them still rely on ipv4 and that's why it's should be a requirement for masternodes, not an option. We can revert this once ipv6 became the norm but that's way too far from being the reality right now imo.

@crowning-
Copy link

I can accept this, but I do not agree.

It's the same chicken-egg argument ISPs tell you why they don't offer more IPv6 services, because there are not much IPv6 users who would be able to use it.
Removing IPv6 support would also remove the already existing IPv6 users. Welcome back in 2010.
And somewhere suddenly everyone cries because we're running out of IPv4 addresses.

Build the (Dash-) infrastructure and the users will follow.

@UdjinM6
Copy link
Author

UdjinM6 commented Oct 9, 2016

I'm not happy to implement this too but sadly that

Build the (Dash-) infrastructure and the users will follow.

is not the case. I have full ipv6 connectivity inside my local home network but my ISP doesn't provide it and I can't force them to do so. I bet most of users suffer from the same issue. I think ISPs will only upgrade when they themselves run out of IPs or they have no other choice but to upgrade all their hardware one day because the old one simply doesn't work anymore. I doubt they will invest in new hardware just because ipv6 is nice or because some user think it's nice - they already made huge capital investments building their current infrastructure so they are not going to throw that away that fast imo. We, on the other hand, need network connectivity and masternode layer consistency right now, we can't wait another 5-10 years for them to upgrade and just let our users/infrastructure suffer from this issue all these years.

Also keep in mind that this does NOT remove ipv6 support from node networking, (master)nodes still can advertise itself on network layer as ipv6 nodes (in addition to ipv4), same as before, it's only our custom masternode announce message that requires ipv4 now.

@crowning-
Copy link

We, on the other hand, need network connectivity and masternode layer consistency right now

I 100% agree.

But your argument against IPv6 was NOT consistency (as I wrote above, when your code works with IPv4 it'll also work with IPv6. You only have to split the check into a IPV4 check executed by IPv4 nodes and a IPV6 check executed by IPv6 nodes. The result is, independently of the IP network, broadcasted to the Dash network ).

Your argument above was that right now IPv6 nodes get paid even they don't provide any services.
And that's simply wrong. They provide exactly the same services as IPv4 nodes, only on a smaller scale (proportional to the percentage of IPv6 nodes/peers in our network).

I would like to know why you think your code for IPv4 won't work for IPv6.

@schinzelh
Copy link

schinzelh commented Oct 10, 2016

I would like to know why you think your code for IPv4 won't work for IPv6.

I did not see anyone arguing that the code is not working for IPv6 - it is working.

The problem is that we do not enforce dualstack, which leads to two distinct networks at the moment: IPv4 only nodes/wallets can't connect to IPv6 only masternodes and vice versa.

For me there are three solutions:

  1. enforce dualstack installations, so IF you have a IPv6 masternode you ALSO need a IPv4 entry (common in real life installations, you do not get a IPv6 connection without at least a 6to4 gateway at home)
  2. Prohibit IPv6 masternodes: This is the solution we are discussing here
  3. Select TWO masternodes in each round for mixing: One for IPv6, one for IPv4. Rewards are not affected.

From technical point of view i favor 1) or 3), while 2) is the quick win - still not happy about degrading to IPv4, it is so 2000s

@thelazier
Copy link

@schinzelh , I think current solution is not totally 2).
This solution just require IPv4 announcement , If you have a IPv6 masternode you need a IPv4 entry for announcement (as mentioned in your 1) ).

@schinzelh
Copy link

schinzelh commented Oct 10, 2016

@thelazier You are right, actually this PR is enforcing any masternode (IPv4 AND IPv6) to have a IPv4 entry in masternode list, so this is in fact option 1) here :)

So maybe we should name it "enforce dualstack IPv6 masternodes" :)

@crowning-
Copy link

I like 3) :-)

As for 1), when you're running 2 Masternodes with 2 IPv6 IPs on one host, do you mean one IPv4 address for all Masternodes (which I think would be difficult to implement), or one IPv4 address for each IPv6 address (which would make the dual-stack unnecessary because we would have a IPv4 IP per node anyway)?

@schinzelh
Copy link

@crowning- Obviously it must be one IPv4 for each IPv6, otherwise: What masternode will bind to the IPv4 address? Node1 or Node2? Given Node1: How do we reach Node2 via IPv4?

@schinzelh
Copy link

Maybe we should drop the port 9999 requirement, too?

@crowning-
Copy link

Maybe we should drop the port 9999 requirement, too?

This would make the most from the existing IPv4s.
Most services (e.g. HTTP) can be configured to use (almost) any available port, you only have to make sure that the clients KNOW (via the port in the URL) that they have to use a different port to connect. And the clients usually have all ports open for outgoing connections, only the Masternodes itself would have to open a different port for incoming connections.

The port is already part of the Masternode broadcast, so we would have covered this already.

@UdjinM6
Copy link
Author

UdjinM6 commented Oct 10, 2016

Ok, thanks everyone for participating 👍 (I see we drift to port discussion which is kind of offtopic here ;) )

As I said we can always revert this later if ipv6 become more popular/available option for the majority of end users or maybe we could implement smth more sophisticated but for now I don't really see another solution to quickly fix our infrastructure so I'm going to merge this. Let's see how this plays out on testnet.

@UdjinM6 UdjinM6 merged commit 321fd64 into dashpay:v0.12.1.x Oct 10, 2016
@crowning-
Copy link

crowning- commented Oct 10, 2016

quickly fix our infrastructure

That's actually my main concern. Our network's working fine with this for 2 years now, so it certainly would for another month or so.

I vote for fixing it right instead of quick, even when my vote is ignored.

@UdjinM6
Copy link
Author

UdjinM6 commented Oct 10, 2016

Our network's working fine with this for 2 years now

It has wrong incentives right now imo: ipv6 nodes do not provide service for majority of end users and yet they are paid just like ipv4. I see it more as a "fork" in the infrastructure rather than a valuable feature. ipv6 adoption is simply laughable unless you live in USA, Germany or Greece https://www.google.com/intl/en/ipv6/statistics.html#tab=per-country-ipv6-adoption&tab=per-country-ipv6-adoption

even when my vote is ignored

It's not. I hear you concerns but I see no other solutions so far. If you have one - you know how/where to submit your code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants