merkle tree abstraction #4626
merkle tree abstraction #4626
Conversation
arvidn
force-pushed
the
merkle-tree
branch
2 times, most recently
from
4c886d2
to
808b856
Compare
|
This seems like a reasonable approach to me. The one minor concern I have is that when the tree is sparse peers could craft a hash request which would require a large number of hash computations to generate proof hashes. E.g. request two leaf hashes and proofs all the way back to the file root. I'm not sure if this would be a problem in practice. I don't think it would be very effective as a DOS attack. It may still be a good idea to rate limit hash requests which require a lot of hash (re)computation. |
|
thanks for the review. Yeah, I should spend more time on improving the names and comments. I will also add unit tests for the Maybe a (future) approach to the DoS potential would be to keep score of how many hashes have been computed in response to a hash request (i.e. proofs higher up the tree have a higher cost) and rate limit based on that. |
|
btw. To further justify this whole project; the reason I consider the memory usage a big deal is because I failed to run my performance test that I run every now and then (once a year or so). Where I generate 100'000 torrents and load them up in client_test under a profiler. Running this experiment in master fails with out-of-memory, on my linux machine with 78.6 GiB of RAM. |
arvidn
force-pushed
the
merkle-tree
branch
6 times, most recently
from
ce17c80
to
a302906
Compare
| } | ||
|
|
||
| std::tuple<int, int, int> merkle_find_known_subtree(span<sha256_hash const> const tree | ||
| , int const block_index, int const num_valid_leafs) |
There was a problem hiding this comment.
@ssiloti I'm impressed you got all this right without extensive unit tests :)
arvidn
force-pushed
the
merkle-tree
branch
2 times, most recently
from
7c0cf33
to
20a6957
Compare
…into a member function of merkle_tree. This was the last operation that required to be able to mutate the tree, so mutating operator[] and fill and clear can now be made private
|
I think this patch is big enough to not build more onto. But the main missing part is still replacing (or at least replacing most use of) |
This is a proposed abstraction for the per-file merkle tree. Currently this tree is a
std::vector<sha256_hash>that's allocated to fit the full tree.The goal of this abstraction is to enable a sparse representation of this tree, but this is not implemented yet. The
aux::merkle_treeclass does not allow direct write access to the tree, but the tree can only be mutated via a few high level operations. I believe these operations are at somewhat reasonable levels.I've divided up the patch in bite-sized commits that can be reviewed individually.
@ssiloti I'm interested in whether you think this is a reasonable abstraction, or if you anticipate problems I may run into going further in this direction.