-
Notifications
You must be signed in to change notification settings - Fork 3
protocol 1.6: minimal update to get broadcast_package out #6
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
Changes from 7 commits
8e26c35
b774837
0466646
7b84d2a
9657730
a4f158c
e54c0e4
b269a8f
919df4f
c617d4d
394a8fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| Protocol Changes | ||
| ================ | ||
|
|
||
| This documents lists changes made by protocol version. | ||
| This document lists changes made by protocol version. | ||
|
|
||
| Version 1.0 | ||
| =========== | ||
|
|
@@ -175,3 +175,44 @@ New methods | |
| ----------- | ||
|
|
||
| * :func:`blockchain.name.get_value_proof` to resolve a name (with proof). Name index coins (e.g. Namecoin) only. | ||
|
|
||
|
|
||
| Version 1.5 | ||
| =========== | ||
|
|
||
| (this version number was skipped, no corresponding protocol is defined) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that I skipped 1.5.x as Fulcrum and https://github.com/cculianu/electrum-cash-protocol uses these version numbers. By design of From a quick read of "electrum-cash-protocol", I think all the changes in the interim versions are backwards compatible. So maybe this is all ~fine... Nevertheless, the situation is unfortunate. |
||
|
|
||
|
|
||
| .. _version 1.6: | ||
|
|
||
| Version 1.6 | ||
| =========== | ||
|
|
||
| Changes | ||
| ------- | ||
|
|
||
| * Breaking change for the version negotiation: we now mandate that | ||
| the :func:`server.version` message must be the first message sent. | ||
| That is, version negotiation must happen before any other messages. | ||
| * Also for :func:`server.version`, the server must tolerate and ignore | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is stupid. Again, this is what the features map is for. But whatever. |
||
| extraneous arguments, to allow for extensions in future protocol versions. | ||
| * The status of a scripthash has its definition tightened in a | ||
| backwards-compatible way: mempool txs now have a canonical ordering | ||
| defined for the calculation (previously their order was undefined). | ||
| * :func:`blockchain.scripthash.get_mempool` previously did not define | ||
| an order for mempool transactions. We now mandate a specific ordering. | ||
| * Optional *mode* argument added to :func:`blockchain.estimatefee`. | ||
| * :func:`blockchain.block.headers` now returns headers as a list, | ||
| instead of a single concatenated hex string. | ||
|
|
||
| New methods | ||
| ----------- | ||
|
|
||
| * :func:`blockchain.transaction.broadcast_package` to broadcast a package of transactions (submitpackage). | ||
| * :func:`mempool.get_info` to get more detailed and general relayfee info. | ||
|
|
||
| Removed methods | ||
| --------------- | ||
|
|
||
| * :func:`blockchain.relayfee` is removed. The `minrelaytxfee` field | ||
| of the new :func:`mempool.get_info` RPC is a direct replacement. | ||
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.
Note that the byteorder used for the sorting is not the human-readable txid byteorder but the network one.
I chose this because it fits the existing internals of electrumx much nicer.
However in cculianu/electrum-cash-protocol#5 (comment) @cculianu expressed frustration about this choice.
I guess I don't mind either way though.
Any strong opinions?
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.
I appreciate your asking others for feedback on this.
Just to be annoying -- I'll smartly say that it can be argued the "network" order is only useful if you speak the p2p protocol. Otherwise it can be argued the true network order is the RPC order. :P
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.
Network byteorder makes sense to me for such an internal behavior, IMO. But, the most important thing is that it's well defined.
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.
I added a detailed example in 919df4f, to try to avoid any misunderstanding.
Uh oh!
There was an error while loading. Please reload this page.
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.
You guys are seriously muddled in your thinking:
This latter thing you are calling "network" order. However the term is misleading because for our network, the network order is actually the other order! The big endian order!
Are we clear on that?
So what you are proposing is that this will be the only place anywhere within this protocol where we are to look at txn hashes as being in the opposite order from every other place.
Are we clear on what is happening here?
I strongly disagree with this approach since:
Please change this to what Fulcrum was already doing since 2020.
I won't comply with this in Fulcrum because it's dumb. Sorry. I already solved this problem in 2020 and it's doing the "natural" thing -- using the hash order for the hashes as they appear everywhere else in the protocol. That's what most people expect.
What you want to do here is just unexpected and creates more work for me. And all for no good reason. No can do. Sorry.
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.
I don't have a strong opinion, but Cormorant (Sparrow's EPS clone) is using big endian order as well. And I agree stating 'big endian' or 'little endian' is clearer.
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.
Fair enough, I meant the bitcoin p2p network.
Is that really more precise? Unlike what you say, the output of sha256 is big-endian. :D
see page 12 (of 36) of https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf :
See also, https://en.wikipedia.org/wiki/SHA-256, where at least for the pseudo-code it states:
Or see e.g. https://learnmeabitcoin.com/technical/general/little-endian/ , which says little-endian is used for integers but big-endian is used for hashes in the bitcoin p2p protocol.
Even if you still disagree, at least please accept that it is not intuitive or clear what big-endian or little-endian is when thinking of the byte-vector output of a hash function. The output is not an integer.
Clearly it is not intuitive if what you thought is actually the opposite of what the standard says.
Ok, I changed the order to what you suggested.
I wish you had been clearer from the start that this is a complete blocker for you.
See 394a8fd
I disagree. I hope I illustrated how that would not be clear at all. :P
The updated text now says:
I think that should be clear. And in any case, there is an example.
I just spent an hour updating the server code, the client code, and writing a new example for the spec.
It was writing the new example that took most of that time.
Is it really that much work to add a reverse iterator or a minus sign to your comparator?
The diff was 6 characters in e-x:
spesmilo/electrumx@0f3e261