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

Already on GitHub? Sign in to your account

[WIP] Add compact blocks #1386

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

jnewbery commented Oct 6, 2016

This PR adds documentation for compact blocks.

I've added sequence diagrams for compact block message exchange using .msc markup, and .svc/.png files generated at http://mscgen.js.org/. I've marked this as WIP since I haven't regenerated the page to check that the diagrams look good.

@TheBlueMatt - can you take a look at the changes and confirm that you're happy with the documentation?

harding suggested changes Oct 18, 2016 edited

A few comments below, but this otherwise LGTM from a documentation perspective. (I'm not familiar enough with compact blocks to comment on technical accuracy.)

When the changes suggested have been made (or it's decided not to make them), I can generate a preview of the rendered page for others to review.

Thanks!

_includes/devdoc/ref_p2p_networking.md
+*Added in protocol version 70014 as described by BIP152.*
+
+A `cmpctblock` message is a compact enocoding of a new block, used to relay
+blocks with faster propogation and lower bandwidth than relaying the full
@harding

harding Oct 18, 2016

Contributor

s/propogation/propagation/

@jnewbery

jnewbery Oct 18, 2016

Contributor

fixed

_includes/devdoc/ref_p2p_networking.md
+
+| Bytes | Name | Data Type | Description
+|-------------------|-------------|------------------|----------------
+| *Varies* (1 or 3) | index | compactSize uint | The index of the transaction in the block, differentially encoded.
@harding

harding Oct 18, 2016

Contributor

Could you add "(see below)" after "differentially encoded", as I was confused by this and didn't realize that it was described in more detail in the following paragraph.

@jnewbery

jnewbery Oct 18, 2016

Contributor

agree - done

_includes/devdoc/ref_p2p_networking.md
+The indexes are *differentially encoded*. Instead of using raw indexes, the
+number encoded is the difference between the current index and the previous
+index, minus one. For example, a first index of 0 implies a real index of 0,
+a second index of 0 thereafter refers to a real index of 1, etc.
@harding

harding Oct 18, 2016

Contributor

I'd assume that means that the first prefilled txn is offset from the coinbase transaction. If that's the case, maybe we can say that explicitly. If that's not the case, then clarification could be useful.

@jnewbery

jnewbery Oct 18, 2016

Contributor

index 0 is the coinbase transaction (which will always be included in a cmpct block message, since the peer would never have the coinbase transaction in their mempool). I've added a note to make that clear.

_includes/devdoc/ref_p2p_networking.md
+Short transaction IDs are used as a compact representation of transaction to
+relay without sending a full 256-bit hash. They are calculated by:
+
+1. Hashing `block header||nonce` (in little-endian) with a single-SHA256 hash
@harding

harding Oct 18, 2016

Contributor

The Travis CI build error is because our auto-crossref tool is trying to link block header to our glossary entry for block header. You can resolve this error by changing the text above to:

`block<!--noref--> header<!--noref-->||nonce`

(Ugly, I know, but it works.)

@harding

harding Oct 18, 2016

Contributor

To be clear, the <!--noref--> will be automatically removed so that it renders correctly:

2016-10-18-15_35_59_164696368

_includes/devdoc/ref_p2p_networking.md
+relay without sending a full 256-bit hash. They are calculated by:
+
+1. Hashing `block header||nonce` (in little-endian) with a single-SHA256 hash
+2. Hashing the transaction ID with SipHash-2-4. The keys (k0/k1) are set to the first two little-endian 64-bit integers from the above hash, respectively.
@harding

harding Oct 18, 2016

Contributor

Can we add a link to a description of "siphash-2-4" here? An overview is fine; it doesn't have to be a spec.

@jnewbery

jnewbery Oct 18, 2016

Contributor

I've added a link to the wikipedia page explaining SipHash.

Contributor

jnewbery commented Oct 18, 2016

Thanks for the comments @harding! I've made the suggested markups.

I'm leaving this PR as WIP for now. Now that V0.13.1 is out, I need to update this to include cmpctblocks v2 (ie cmpct blocks with segwit).

@wbnns wbnns self-assigned this Dec 9, 2016

@wbnns wbnns added the On Hold label Dec 17, 2016

@wbnns wbnns added the Dev Docs label Mar 1, 2017

Contributor

wbnns commented Mar 29, 2017

@jnewbery Hey, since this is WIP and it has been a while, I'm closing this for now to keep the current PRs up to date and organized. Feel free to just open a new PR when you're ready.

Thanks for your help with this and I hope all is well!

@wbnns wbnns closed this Mar 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment