search

antouhou / rs-merkle

The most advanced Merkle tree library for Rust
MIT License
168 stars 45 forks source link

fix: incorrect proof hash count causing panic #28

Closed witter-deland closed 1 year ago

witter-deland commented 1 year ago

fix #20

themighty1 commented 1 year ago

Hi, would it be possible to also include tests into this PR showcasing what was fixed?

witter-deland commented 1 year ago

The problem fixed is a bug, you can refer to #20, I also encountered a similar problem.

I don't know what kind of test is appropriate to build, it seems to just fix the original bug: use the old code, the merkle proof bug will appear, use the new code, the merkle proof will work normally

Hi, would it be possible to also include tests into this PR showcasing what was fixed?

antouhou commented 1 year ago

@witter-deland Thanks for the work! Yeah, please just include one simple test that passes an incorrect number of hashes, and check that this error is returned. Thank you very much!

witter-deland commented 1 year ago

It's done : )

@witter-deland Thanks for the work! Yeah, please just include one simple test that passes an incorrect number of hashes, and check that this error is returned. Thank you very much!

antouhou commented 1 year ago

@witter-deland Thank you! Merged. Going to publish a new patch version soon

antouhou commented 1 year ago

Version 1.4.1 published, thank you!

themighty1 commented 1 year ago

On second thought, maybe hold off fixing it @witter-deland for now. This PR doesn't seem to address the root cause of the panic. Im working on a PR which addresses it properly. It will subsume this fix.

themighty1 commented 1 year ago

nvm, i think this PR's fix covers all cases. thanks.