Stream Merkle root computation when downloading #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These enable streaming sector downloads. The Go 1.13 bump is due to a single occurrence of 1 << int. Not really worth it, but Go 1.14 is due to be released soon, and I like to stay within at least one release of master.
This is some gnarly code, and it took quite some time to get it right, but ultimately I'm quite happy with it. Chaining all those Readers and Writers together is really satisfying. Anyway, this lets callers stream RPC messages without authenticating them first. Which is generally a bad idea -- there's a reason AEAD is a thing! But for our purposes, this allows us to start hashing sector data immediately, which is a Big Deal for performance (since BLAKE2b is currently our primary bottleneck). You just need to be careful to not do anything reckless with the data before verifying it, like writing it directly to disk or streaming it to a client over HTTP. Fortunately, Sia's design makes this difficult to do anyway; we still wait for the full message to arrive before proceeding to the erasure decoding step, so as long as verify the poly1305 tag before decoding, there's no loss of security. I do still feel a bit guilty about introducing API surface that, if misused, could constitute an attack vector. If our hashing was on par with our encryption and erasure coding (that is, >10 Gbps), I would happily skip this optimization. Alas.
This significantly increases download throughput when network bandwidth is between 100-2000 Mbps -- about 1.5x faster than non-streaming. Unfortunately I ran into bug after bug when implementing this, which gives me reduced confidence in its correctness. Maybe some fuzzing is warranted. Also, as mentioned in the docstring, callers now need to be a bit more careful with the data written to w.
|
Goober, please test and merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially implements #29. See that issue for benchmarks.