Support code blobs compressed with zstd #8549
Conversation
rphmeier
added
A3-in_progress
rphmeier
added
B5-clientnoteworthy
and removed
B0-silent
github-actions
bot
added
A0-please_review
|
Why does it need a Polkadot PR? |
|
Tested upgrade to compressed wasm on a |
|
curious though, how much size reduction can we get with zstd compression on wasm blobs? |
| use std::io::Write; | ||
|
|
||
| if blob.len() > bomb_limit { | ||
| return None; |
There was a problem hiding this comment.
I think we should return an error here. Otherwise we could drop bomb_limit directly.
There was a problem hiding this comment.
Because the semantic of an error is different? The function is documented as returning None if the size exceeds the bomb limit.
| .expect("Failed to write WASM binary"); | ||
| } else { | ||
| println!( | ||
| "Writing uncompressed wasm. Exceeded maximum size {}", |
There was a problem hiding this comment.
Where do we write this?
If the compression fails, no file will exist, but you continue and assume the file exists.
There was a problem hiding this comment.
Where would be the best place to log an error?
client/executor/src/wasm_runtime.rs
Outdated
| const WASM_CODE_BOMB_LIMIT: usize = 50 * 1024 * 1024; | ||
|
|
||
|
|
There was a problem hiding this comment.
| const WASM_CODE_BOMB_LIMIT: usize = 50 * 1024 * 1024; |
Please move the constant out of this function and add some docs.
There was a problem hiding this comment.
And why do we allow 100MB in the wasm builder and only 50 mb here?
There was a problem hiding this comment.
Maybe we should put the constant into the sp_maybe_compressed crate to use every the same.
There was a problem hiding this comment.
I think it's better to have the constant closer to the usage. Initially I did have it in the crate, but then we started to talk about using the crate for PoVs as well. Those will want different max sizes.
There was a problem hiding this comment.
But we already got two constants with different values, in one pr.
There was a problem hiding this comment.
I'll add a CODE_SIZE_BOMB_LIMIT to the crate, but users will pass it through
It's about 3x for node-runtime. It goes from 2.2MB to 750KB |
Co-authored-by: Andronik Ordian <write@reusable.software>
|
bot merge |
|
Waiting for commit status. |
ghost
merged commit 
* begin maybe-compressed-blob * fix build * implement blob compression / decompression * add some tests * decode -> decompress * decompress code if compressed * make API of compresseed blob crate take limit as parameter * use new API in sc-executro * wasm-builder: compress wasm * fix typo * simplify * address review * fix wasm_project.rs * Update primitives/maybe-compressed-blob/Cargo.toml Co-authored-by: Andronik Ordian <write@reusable.software> Co-authored-by: Andronik Ordian <write@reusable.software>
cwerling
added
D1-audited 👍
* begin maybe-compressed-blob * fix build * implement blob compression / decompression * add some tests * decode -> decompress * decompress code if compressed * make API of compresseed blob crate take limit as parameter * use new API in sc-executro * wasm-builder: compress wasm * fix typo * simplify * address review * fix wasm_project.rs * Update primitives/maybe-compressed-blob/Cargo.toml Co-authored-by: Andronik Ordian <write@reusable.software> Co-authored-by: Andronik Ordian <write@reusable.software>
If the Wasm blob begins with a magic prefix, the code will be decompressed with zstd. This will be useful when sending Wasm blobs over the network or through XCM.
The use-case in particular is Statemint or other system parachains. The relay-chain needs to send a code blob over XCM which the Statemint parachain will then apply.