search

sigp / lighthouse

Ethereum consensus client in Rust
https://lighthouse.sigmaprime.io/
Apache License 2.0
2.94k stars 745 forks source link

Non-release tests using KZG verification stack overflow on macOS M1 #4987

Open realbigsean opened 11 months ago

realbigsean commented 11 months ago

Steps to reproduce

cd beacon_node/beacon_chain/src/data_availability_checker
cargo test -- overflow_cache

Info

I haven't investigated whether the stacks usage is high in this particular test prior to this call or if there's a different issue:

https://github.com/ethereum/c-kzg-4844/blob/main/bindings/rust/src/bindings/mod.rs#L234

I think we should explore updating c-kzg bindings to accept slices so we don't have to do this transition from lighthouse types to c-kzg types (which is forcing this stack allocation, and also forces us to copy/clone blobs during verification)

mratsim commented 11 months ago

A Blob is 131kB, MacOS default stack size is 512 kB, so a single Blob does use more than 25% of what's available.

Miscellaneous copies would lead to an overflow.

Now in general Blob should always be on the heap due to their size.

realbigsean commented 11 months ago

Thanks @mratsim

Now in general Blob should always be on the heap due to their size.

I agree. We don't store them internally on the stack, but when we pass them to c-kzg, we're forced to initialize a blob-sized array (at the link in the PR description). I'm interested in trying out constantine, either directly or via rust-kzg.