-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Add a NODE_GETUTXO service bit and document NODE_NETWORK. #5876
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
Conversation
|
Need update in formatServicesStr too: https://github.com/bitcoin/bitcoin/blob/master/src/qt/guiutil.cpp#L880 |
bd0b28d to
8b81d23
Compare
|
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. |
|
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. |
188f0a7 to
d4ab185
Compare
|
OK, I took out the tr() - how does it look now? |
|
I'm not really into this but wouldn't it be sufficient if just the "Bitcoin XT" nodes support the |
|
I guess it makes sense to mark some bits as "Being used by other software
on the network" in the core source code.
|
|
Okay. I see. |
|
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. |
|
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. |
|
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. |
|
Right. Sorry for unwrapping this. |
src/protocol.h
Outdated
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
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 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. |
|
@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. |
|
ACK |
|
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.
d4ab185 to
5983a4e
Compare
|
Mentioned BIP 64 and rebased. |
|
ACK. |
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)
Stop translating the NODE_* names as they are technical and cannot be translated. Github-Pull: bitcoin#5876 Rebased-From: 5983a4e
No description provided.