Draft: Wire protocol (DEP) #8
Conversation
|
We should also write "Security and Privacy Concerns" and "References" sections for this one. For references I could imagine:
For security/privacy:
|
proposals/0000-wire-protocol.md
Outdated
| discoveryKey, // out | ||
| 'HYPERCORE', // in (message) | ||
| publicKey // key | ||
| ) |
There was a problem hiding this comment.
(use Buffer.alloc) also this is basically an HMAC and we should note that
There was a problem hiding this comment.
I added Buffer.alloc().
Could you recommend language describing this as an HMAC?
There was a problem hiding this comment.
I would personally either keep this pseudocode or actual implementation. Pseudo code would be something like discoveryKey = blake2b(outputLength = 32, key = 'HYPERCORE', data = publicKey), outputLength being another important parameter (256 bit output)
There was a problem hiding this comment.
I moved to pseudo-code (which I also prefer to implementation or any specific language in the DEPs).
I think "keyed hash" clarifies things; maf if you want to include a note about HMAC I need something more specific.
|
Fixed, thanks
…On Thu, May 31, 2018 at 3:26 AM Andrew Osheroff ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In proposals/0000-wire-protocol.md
<#8 (comment)>:
> +message Have {
+ required uint64 start = 1;
+ optional uint64 length = 2 [default = 1]; // defaults to 1
+ optional bytes bitfield = 3;
+}
+```
+
+#### Unhave
+[msg-unhave]: #msg-unhave
+
+`type=4` Announces the loss of availability of Hypercore feed blocks for download. The `start` and `length` represent a continuous set of blocks.
+
+This message is sent in the following contexts:
+
+ - To inform the peer that data which was sent was rejected, for instance because it was not requested and pushes are not being allowed.
+ - To announce blocks have been from local storage within a remote-wanted range.
"have been from local storage" -> "have been removed from local storage"
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABNhUxNtWRR36JBgY7Zuvb0kR6LuVLiFks5t36k2gaJpZM4SbihG>
.
|
|
@pfrazee what's the status on this DEP? Are we blocked waiting on anything? If you move this somewhere I have git push privs I can try to push it through in the next couple weeks. |
|
@bnewbold I think it just needs to get written, I don't think there are any blockers in dev work. I'll give you push rights on my fork. |
|
Apologies for letting it stall! |
|
Made some more progress on this, resolving most of the TODOs and comments. Still need clarification/feedback on:
The security/privacy section in particular should get reviewed, and the whole thing probably needs a copy-edit and review for missing content (which I haven't done yet). jIt would also be nice to have more examples, but those can take a long time to write up, and at this point, with effort being put in elsewhere (like the protocol book), I don't think it's necessary here for Draft status. There are no "rationale" or "alternatives" sections; I think those make more sense for new proposals and less for these older/existing protocol bits, but i'm open to objections. Would be great to have this ready to vote, or at least review, by next WG meeting! |
|
Updates look good to me! |
|
I particularly liked the privacy/security discussion at the end. It was honest and covered all my "concerns". It's important to know that the public key is a very strong read capability, eg. that anyone with the public key can decrypt any traffic passively observed by any other peers communicating using that public key in their initial connection. So hypercores that are meant to be very sparsely replicated (eg. internet size ones) will leak a lot of metadata. |
proposals/0000-wire-protocol.md
Outdated
| 6 | ||
| ``` | ||
|
|
||
| If we want to fetch block 0, we need to communicate whether of not we already have the uncles (2, 5) and the parent (3). This information can be compressed into very small bit vector using the following scheme: |
There was a problem hiding this comment.
@vtduncan and I have been struggling to understand this section.
- Why is 1 not required? - After thinking about it, I was able to answer answered this with "1 is not required because we only need to validate the topmost-hash (3) but in order to calculate it we need 2 to calculate one and 1 + 5 to calculate 3". Is this correct? Can this be amended?
- How do we know that there are 7 entries? Do we need to figure out first that there are 7 entries before we can do the hash? What if the list grows in the meantime? My answer is: the digest's length specifies how big the merkle tree is we know: If we send 4bit, it means we know 3to7 entries, if we send 8bit, it means we know 8to63 entries?
- How would this look like with a less-symetric tree like 8 or 9? (that would make for a better example imo.)
- The namings "parent" and "uncle" may be common in merkle-tree nomenclature but it might help to explain the difference briefly.
There was a problem hiding this comment.
I’m trying using figures to understand how this works. I still don’t grasp it, but here is where I’m at trying to draw the example shown in this section:
There was a problem hiding this comment.
My question here would be: If you just want to verify chunk 0, why not send this instead?
Another question I had is: when you receive a signature, how do you know what hash that signature is for?
There was a problem hiding this comment.
For future reference, the code that implements this is in hypercore/tree-index.js and the flat functions within that come from flat-tree/index.js.
There was a problem hiding this comment.
I rewrote this section (and will push in a minute); please let me know if it clarified things. To answer a few questions directly:
The example of wanting to verify block 0 is bad; there's a direct signature of it. I believe that when you request a block, you always get the signature for the root hash as if that was the largest block index, and you're always trying to verify that signature, so I flipped the example to block 6.
This also answers the "how do you know how high the tree is" question.
You don't need hash 1 (in the old example) because you can calculate it from hash 0 and hash 2 (you calculate hash 2 directly from the data transmitted). I guess this is also why you don't need hash 3. Note that in other situations you would need other parent hashes.
I added a reference to the Hypercore DEP which is where the parent/uncle terminology should be defined (if it's unclear there we can take a patch to improve).
A more complex (non-symmetric) example would be nice, but also maybe more confusing here, where we're really just trying to explain the binary packing, not the whole Merkle hashing scheme?
Thanks @emilbayes. See also: holepunchto/hypercore#130
|
I think I've addressed everything above; we should give folks a few days to respond to things before a vote. The block digest section should get review from an actual implementer (I never implemented that bit; cc: @mafintosh @yoshuawuyts). Could probably also use another careful proof-read, but for Draft status I think it would be good to get this out and have eyes on it. |
|
To set the timer, I will say I am submitting this for review to be voted on at the next WG meeting, though if there are significant changes needed we can push that off. |
|
Thanks for review anybody! |
|
Was this Draft promoted to the Current (or however it is called) status? |
Rendered: Draft
Submitted for review. Working group will vote in next meeting (late February).