plonky2
plonky2 copied to clipboard
improve portability
There are a lot of dependencies that are either 1) only used in tests/examples (e.g. rand), or 2) do not compile in rust-based smart contract runtimes (e.g. env_logger, log, std::time, rayon). If we intend this library to be used outside a standard environment, we need to be able to build the library without these dependencies.
To address this, I suggest the following:
- feature-gate
log/TimingTreebehind a feature that's enabled by default, and use conditional compilation to removetimed!,with_context!,TimingTree, andContextTreeif disabled - Wrap
rayon'spar_iterwith a macrocfg_iterthat conditionally switches betweeniter()andpar_iterlike arkworks does here - add a
test_utilsfeature to deal with stuff like thegate_testing.rsin the interrim until cargo supports this functionality
Happy to open a PR for this (I'm already working on it because I need it)
I've got a branch I'm using for this at https://github.com/proxima-one/plonky2/tree/feature-gate-stuff
For logging, shouldn't log itself be portable since it no-ops by default? I see why env_logger could be an issue though; would moving it to dev-dependencies address the concern?
For rand, I think a feature gate sounds good, like num has.
For Rayon, I think the feature would ideally be in Rayon itself, see rayon-rs/rayon#861. If we don't want to wait for that we can add a shim temporarily, but I don't really like the arkworks approach since it changes all the call sites to use macros. I'd prefer either
- conditionally importing a
rayonmodule with shims forParallelIteratoretc, or - having a
maybe_rayonmodule, containing aMaybeParallelIteratorwith amaybe_par_iter(), etc
A feature gate for timed! and with_context! sounds fine, as long as it doesn't change the API; seems like the changes can be internal to those macros.
I personally need a shim for rayon now, but I can keep that part in our branch for now if others don't need it too urgently.
As for timed! and with_context!, the main thing is that all of the callsites change because the timing parameter taken by many functions won't exist if there's no TimingTree (no TimingTree because no std::time). So you still end up having conditional compilation to change the invocation each time timed! or with_context! is called, along with any time any function that takes a timing parameter is called even if you're making the change within the macro. I'm not sure if there's a cleaner way to do it.
Log might actually be fine after looking at the source - I was running into some compilation issues with it but it might have actually been something else.
Do you have an idea for how we could handle ZK configurations if rand is disabled? It looks like there's still a rand_vec call in your branch (see oracle.rs). Since ZK is a runtime setting while rand would be a compile time setting, I guess we would need to just add an assert!(!zero_knowledge) and panic at runtime.
Oh, that's a good point. I guess you'd need to have that assertion. Then it can be a separate feature that some runtimes can enable. For instance in a browser, where there's no rayon but rand might be available.
For Rayon, I think the feature would ideally be in Rayon itself, see rayon-rs/rayon#861. If we don't want to wait for that we can add a shim temporarily, but I don't really like the arkworks approach since it changes all the call sites to use macros. I'd prefer either
- conditionally importing a
rayonmodule with shims forParallelIteratoretc, or- having a
maybe_rayonmodule, containing aMaybeParallelIteratorwith amaybe_par_iter(), etc
Of these, I think I prefer the second option, as it's more explicit that's what's happening and it's less code to write. In either case, the iter() method of stuff like strings, slices, and vecs aren't part of an Iter trait, so we'll have to implement the trait for each type we want to usemaybe_par_iter() on. We'll also need to repeat the process for maybe_into_par_iter(), maybe_par_iter_mut(), maybe_par_chunks(), and maybe_par_chunks_mut().
It ends up being a lot more code than the macro approach, which can be replaced later relatively easily once https://github.com/rayon-rs/rayon/issues/861 happens. You just have to go through and replace all the call sites, which doesn't take that long.
We can also just yoink similar instances of it like this: https://github.com/shssoichiro/oxipng/blob/master/src/rayon.rs. I'll look over the options and open a PR for whatever seems cleaner.
@dlubarov @Sladuca -- Can we close this?
I think the original inspiration for this (Plonky2 verification on Solana) went away, so it's at least not a priority; I think we can close for now and maybe reopen if the need arises