antouhou / rs-merkle

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

Remove dependency on micromath, compute height with integers only #18

Closed matthiasgoergens closed 1 year ago

matthiasgoergens commented 1 year ago

This reduces the overall dependency tree, which was only being used in computing the tree height. It may also be benefitial in some environments (e.g.: zk) to be able to use integer math only.

The snippet leaves_count as f32 caused some trouble. Let's remove it.

(Description stolen from https://github.com/antouhou/rs-merkle/pull/16 because it's honestly much better than what I had originally typed up.)

sashaduke commented 1 year ago

To expand on this - the use of floats is not supported on CosmWasm's WASM VM, so removing this means the crate will now be able to used in smart contracts deployed on CosmWasm without any issues.

matthiasgoergens commented 1 year ago

To expand on this - the use of floats is not supported in WebAssembly, so removing this means the crate will now be able to used in programs which are compiled to WASM without any issues.

To be more precise WebAssembly supports floats just fine by itself, but Sasha has an application that only supports a subset of WASM that doesn't support floats.

antouhou commented 1 year ago

@matthiasgoergens @sashaduke This uses std though, which is not compatible with the no-std feature. I'll try to think how to do that without using an f32 and std, thank you for reporting this!

matthiasgoergens commented 1 year ago

Thanks for checking!

I wonder if we can ask the compiler or so to enforce the no-std rule?

sashaduke commented 1 year ago

no_std compatibility can be maintained by using core::mem::sizeof instead of std::mem::sizeof

matthiasgoergens commented 1 year ago

@sashaduke Thanks, I implemented your suggestion!

@antouhou I can build the fixed version with cargo build --no-default-features (and just to confirm, this check fails when trying on my original version).

I think about how to automate this check.