Gate Code for Testing Purposes Under testing Modules

[sai-deng]

This PR gates test-specific code under the testing modules.

Please use GitHub’s “Hide whitespace changes” feature for easier reviewing.

[0xaatif]

I'm in favour of the testing modules, and the code changes you've made, but I'd rather not see a test_utils feature.

• We don't really care about a narrow API - our only consumer is zero.
• We don't care about excluding symbols from production builds - dead code elimination is already done by the compiler.
• Development complexity is increased.
  • Now I've got to tell my editor to use --features test_utils
  • A more complex feature matrix, where we need to add e.g clippy coverage (which is easy to neglect c.f this PR ;) )

[sai-deng]

I'm in favour of the testing modules, and the code changes you've made, but I'd rather not see a test_utils feature.

• We don't really care about a narrow API - our only consumer is zero.

• We don't care about excluding symbols from production builds - dead code elimination is already done by the compiler.

• Development complexity is increased.

  • Now I've got to tell my editor to use --features test_utils
  • A more complex feature matrix, where we need to add e.g clippy coverage (which is easy to neglect c.f this PR ;) )

Thanks for reviewing! We don’t need to use --features test_utils, as it’s only used in evm_arithmetization/tests and hard coded in Cargo.toml. You’re right about the compiler optimization, so I’ve removed the feature.

[sai-deng]

I'm in favour of the testing modules, and the code changes you've made, but I'd rather not see a test_utils feature.

• We don't really care about a narrow API - our only consumer is zero.

• We don't care about excluding symbols from production builds - dead code elimination is already done by the compiler.

• Development complexity is increased.

  • Now I've got to tell my editor to use --features test_utils
  • A more complex feature matrix, where we need to add e.g clippy coverage (which is easy to neglect c.f this PR ;) )

Thanks for reviewing! We don’t need to use --features test_utils, as it’s only used in evm_arithmetization/tests and hard coded in Cargo.toml. You’re right about the compiler optimization, so I’ve removed the feature.

However, I would argue that without a feature gate or conditional compilation like #[cfg(test)] (which we cannot use, since the code is needed in evm_arithmetization/tests), we cannot guarantee that test-specific code won’t be accidentally used in production. While the Rust optimizer can remove unused code, having the test code always available increases the risk of it being inadvertently referenced or used in production.

[Nashtare]

Non-blocking comment, LGTM

[Nashtare]

In that case we could gate behind a testing module the intermediate verify_foo methods in fixed_recursive_verifier too (i.e. verify_root. / verify_segment_aggregation / verify_batch_aggregation), as well as prove_all_segments