Skip to content

Conversation

@mikehearn
Copy link
Contributor

No description provided.

@laanwj laanwj added the P2P label Mar 11, 2015
@laanwj
Copy link
Member

laanwj commented Mar 11, 2015

@mikehearn mikehearn force-pushed the getutxo-service-flag branch 2 times, most recently from bd0b28d to 8b81d23 Compare March 12, 2015 01:33
@mikehearn
Copy link
Contributor Author

Done - but should those strings really be translated? I'm not sure how what a translator would do when faced with "GETUTXO" as it's a basically technical term, and "NETWORK" is basically also meaningless unless you know the protocol well. Perhaps they should be left English only.

@Diapolo
Copy link

Diapolo commented Mar 12, 2015

Seems I'm with @mikehearn here... let's remove translating these, as they are indeed technical terms and e.g. when searching the web a translations will likely be counter productive.

@mikehearn mikehearn force-pushed the getutxo-service-flag branch 2 times, most recently from 188f0a7 to d4ab185 Compare March 12, 2015 23:56
@mikehearn
Copy link
Contributor Author

OK, I took out the tr() - how does it look now?

@jonasschnelli
Copy link
Contributor

I'm not really into this but wouldn't it be sufficient if just the "Bitcoin XT" nodes support the NODE_GETUTXOS service flag (as they already do: https://github.com/bitcoinxt/bitcoinxt/blob/0.10rc3/src/protocol.h)? What is the benefit of adding this to bitcoin-core?

@sipa
Copy link
Member

sipa commented Mar 15, 2015 via email

@jonasschnelli
Copy link
Contributor

Okay. I see.
Just checked my dns-seed-crawler and detected 14 reliable Bitcoin XT nodes which is about ~0.21%.

:~/seed/bitcoin-seeder$ cat dnsseed.dump | grep 'Bitcoin XT'
54.94.178.0:8333                                    1   1426422998  100.00%  99.70%  85.63%  24.20%   6.26%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.102.244:8333                                 1   1426421035  100.00%  99.70%  85.60%  24.18%   6.25%  347713  00000003  70003 "/Bitcoin XT:0.10.0/"
54.69.31.201:8333                                   1   1426419635  100.00%  99.69%  85.38%  24.02%   6.21%  347709  00000003  70003 "/Bitcoin XT:0.10.0/"
104.236.90.214:8333                                 1   1426421986  100.00%  99.68%  85.21%  23.89%   6.17%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
83.150.2.99:8333                                    1   1426420111  100.00%  99.66%  84.99%  23.73%   6.13%  347710  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.103.14:8333                                  1   1426419355  100.00%  99.63%  84.49%  23.37%   6.02%  347708  00000003  70003 "/Bitcoin XT:0.10.0/"
46.229.238.187:8333                                 1   1426418904  100.00%  99.62%  84.46%  23.36%   6.02%  347708  00000003  70003 "/Bitcoin XT:0.10.0/"
54.76.31.80:8333                                    1   1426422103  100.00%  99.61%  84.20%  23.17%   5.96%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
80.218.159.87:8333                                  1   1426422845  100.00%  99.56%  83.59%  22.76%   5.85%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
87.230.87.140:8333                                  1   1426422808  100.00%  99.56%  83.59%  22.75%   5.85%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.103.4:8333                                   1   1426421948  100.00%  99.49%  82.78%  22.22%   5.70%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
82.134.66.146:8333                                  1   1426420221  100.00%  99.48%  82.66%  22.15%   5.67%  347710  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.103.18:8333                                  1   1426423626  100.00%  98.72%  76.58%  18.73%   4.72%  347717  00000003  70003 "/Bitcoin XT:0.10.0/"
212.60.121.11:8333                                  0   1426422254   64.30%  58.08%  46.80%  12.67%   3.26%  347714  00000003  70003 "/Bitcoin XT:0.10.0/"
31.15.198.29:8333                                   0   1426419776   86.57%  68.49%  46.43%  10.11%   2.50%  347710  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.102.193:8333                                 0   1426293783    0.00%   0.83%   8.07%   4.98%   1.40%  347515  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.119.134:8333                                 0   1426291835    0.00%   0.80%   7.42%   4.48%   1.26%  347512  00000003  70003 "/Bitcoin XT:0.10.0/"
83.212.103.5:8333                                   1   1426418499   82.63%  35.44%  13.57%   2.06%   0.49%  347706  00000003  70003 "/Bitcoin XT:0.10.0/"

@mikehearn
Copy link
Contributor Author

It's just to avoid service bit conflicts. The first release of XT didn't actually identify itself as such in the subver field, that was fixed in a respin, so there are a few more nodes that advertise this bit. But yeah it's not many. OTOH it's a very cheap request to serve, so there doesn't need to be many.

@jonasschnelli
Copy link
Contributor

Yes. This definitely make sense.

But for lighthouses UTXO queries, wouldn't it make more sense to try to get REST/getutxos in and support some full node users running the rest service through a hardened Apache (or other httpd) reverse proxy (sorry to hijack this issue)?

I just think a fork will usually burn useful resources.

@mikehearn
Copy link
Contributor Author

Let's not rehash all that here. I think the protocol design I picked is the right one and would have no interest in switching stuff around at this point. Besides, we may as well get in the habit of letting people reserve service bits for other stuff. It's just a bit field.

@jonasschnelli
Copy link
Contributor

Right. Sorry for unwrapping this.

src/protocol.h Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense to mention bip64 here in the comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the BIP should be mentioned here. That's more important than advertising what software implements it.

@theuni
Copy link
Member

theuni commented Mar 15, 2015

Thinking forward here a bit...

Would it make sense to instead add a NODE_EXTENSIONS bit, which indicates support a new "extensions" or "features" command? That command would either allow you to query a node like "extension getutxo", or just "extensions" to get a list.. I'm not sure which approach would make more sense.

Or alternatively, to reduce the effort needed, these supported extension strings could be appended to the version message in some well-defined format.

To me, there's a distinction between "basic services that this node offers" and "extra commands this node understands". Going forward, it seems likely that there will be more things like this that come up. Making experimental p2p commands discoverable would eliminate the need to maintain a list of things that core may never support.

@jgarzik
Copy link
Contributor

jgarzik commented Mar 15, 2015

@theuni That is precisely what NODE_EXT_SERVICES #4657 does... Only that bit is widely advertised, requiring a second disambiguation step.

@theuni
Copy link
Member

theuni commented Mar 15, 2015

@jgarzik It looks to me like that PR does pretty much the same thing the as getutxos though wrt the service bit, no? It sees a service bit and assumes that a new command is available as a result.

What I'm suggesting is a service bit for command reflection itself, call it NODE_EXT_COMMAND which definitively makes the "extension" (or some better name) command available.

For your case, i'd see that bit set in version, then do "extension getextsrv" and parse the result. I could do the same for "extension getutxos".

A service bit is not enough to know that a command is supported, because alternate implementations (or incomplete ones) can decide for themselves which bit to pick. That's very well illustrated by the fact that @jgarzik had to change bits after his original implementation due to a conflict.

@laanwj
Copy link
Member

laanwj commented Mar 16, 2015

@theuni @jgarzik There is lots of previous discussion about that on the mailing list. The problem is advertising/discovery. The current service bits are propagated with addr commands, but additional services provided by such an extension would not be. An extended advertising mechanism could be added, at the expense of more bandwidth usage, memory usage per known address.

I'm personally quite conservative with regard to this. I think it is harmful to have too many services piggyback on Bitcoin's P2P network. Keep it simple. The exception would be communication that is relevant for node-to-node coordination, for example to advertise block ranges in the presence of pruning and archive nodes.

Many of the proposed services however rely on a client/server view and that's not a good fit for a P2P network.

However if people claim do bits and use them on the network, I suppose it's slightly better to have the displayed information reflect this to avoid collisions... so hence ACK on this.

@gavinandresen
Copy link
Contributor

ACK

@jgarzik
Copy link
Contributor

jgarzik commented Mar 19, 2015

ACK

On Bitcoin-XT in general: I think competitive source code forks are healthy for the ecosystem.

(this is a general statement, not an endorsement for getutxos or any one specific change)

Stop translating the NODE_* names as they are technical and cannot be translated.
@mikehearn mikehearn force-pushed the getutxo-service-flag branch from d4ab185 to 5983a4e Compare March 21, 2015 18:35
@mikehearn
Copy link
Contributor Author

Mentioned BIP 64 and rebased.

@jonasschnelli
Copy link
Contributor

ACK.

@laanwj laanwj merged commit 5983a4e into bitcoin:master Mar 26, 2015
laanwj added a commit that referenced this pull request Mar 26, 2015
5983a4e Add a NODE_GETUTXO service bit and document NODE_NETWORK. Stop translating the NODE_* names as they are technical and cannot be translated. (Mike Hearn)
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 28, 2015
Stop translating the NODE_* names as they are technical and cannot be translated.

Github-Pull: bitcoin#5876
Rebased-From: 5983a4e
@rebroad rebroad mentioned this pull request Aug 25, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants