SpinResearch / merkle.rs

:christmas_tree: Merkle tree in Rust
BSD 3-Clause "New" or "Revised" License
129 stars 23 forks source link

Use `Vec` less, for improved efficiency. #18

Closed briansmith closed 6 years ago

romac commented 7 years ago

Awesome, I was actually just getting started on doing something similar, but this is much better. Thank you so much.

The build is currently failing because that changes breaks the Protocol Buffers serialization, but I can take care of it :)

romac commented 7 years ago

We should have described the build process (which involves passing --all-features to cargo build in order to enable the Protocol Buffer serialization code) somewhere in the README already. I'll make sure to fix that as well.

romac commented 7 years ago

@briansmith I will also address the TODO you left in HashUtils (if you haven't done so already). Thanks again for taking the time to review and improve the code, much appreciated!

briansmith commented 7 years ago

No trouble. It would be great if you could take over this PR. I just wanted to share it as a POC. Also I wanted to read your code to understand how ring can be improved. For example, we should consider adding Debug and Copy implementations to Digest.

If you don't mind, I'd like to suggest some more efficiency improvements, perhaps by filing some issues? But, I won't have time to write patches for them any time soon.

FredericJacobs commented 7 years ago

Thanks for the PoC. We'll take it from here.

If we stumble upon improvement opportunities for ring, we will make sure to let you know.

Please feel free to open issues with further efficiency improvements ideas. This implementation is quite young and there are many things we're still looking at doing in the future.