Fix testutils incompatibility with soroban-ledger-snapshot

[leighmcculloch]

What

Add a testutils feature to soroban-ledger-snapshot, and use it to gate the functionality in ledger snapshots that uses the soroban-env-host's testutils features.

Why

In #1467 the soroban-ledger-snapshot crate dependency on soroban-env-host was changed to include an always-enabled dependency on the testutils feature. This appears to have been done because the function get_stored_entries of the Host was changed from being always available, to only available in testutils mode. The soroban-ledger-snapshot crate uses the get_stored_entries function.

A side-effect of of always pulling in the soroban-env-host is that the soroban-ledger-snapshot crate became incompatible with any project that also depends on the soroban-sdk without its testutil feature enabled. This is because the soroban_env_host::EnvBase trait type, which is implemented by the soroban-sdk crate, has functions that are required when the testutil feature is enabled.

To resolve the above challenges with only changes to this repository the most straight-forward thing to do is to move the functionality in soroban-ledger-snapshot that now needs testutils in soroban-env-host behind a testutils feature as well, which soroban-sdk enables when it uses soroban-ledger-snapshot.

Known Limitations

Features on traits, like EnvBase, inherently create situations where dependencies can get stuck in situations where they are incompatible because another dep might be dependent on the trait in the opposite state. We should probably revisit EnvBase's liberal use of features, breaking it up into separate traits. Rust features should always be, where possible, additively compatible. I've opened the following issue for us to explore that sometime:

• Separate EnvBase testutils functions into a separate trait rs-soroban-env#1570

We may also want to revisit whether some simple getters like get_stored_entries truly need to be gated behind a testutils flag. Features in general come at a cost in maintenance and rigidity. Typically features in Rust projects are used to reduce dependencies, but in soroban-env-host we are using it to limit features in different use cases.

After #1467 and the change in this PR, some of the functionality in soroban-ledger-snapshot crate is unusable with the Host, unless the Host is being used in testutils mode. Today the only uses of the library that use that functionality is when the soroban-sdk is also being used with testutils enabled, but it is feasible that we may want to use the functionality in situations where it doesn't make sense to run the Host in testutils mode. This would probably mean changing the function in soroban-env-host that became testutils only, to be available without testutils again. We don't need it today, so I'm not proposing it today.

cc @stellar/devx @stellar/contract-committers @fnando

[dmkozh]

We may also want to revisit whether some simple getters like get_stored_entries truly need to be gated behind a testutils flag. Features in general come at a cost in maintenance and rigidity. Typically features in Rust projects are used to reduce dependencies, but in soroban-env-host we are using it to limit features in different use cases.

I would argue that dependency reduction is a side effect, and the primary point of 'features' is to conditionally compile, well, features, as the name suggests. And in case of testutils we have a pretty clear cut for what is or is not a test utility. Given that host implements the protocol, I would really hesitate about unconditionally adding any functionality that doesn't contribute into the protocol implementation. Besides making the protocol safer, I think this feature gate is really helpful for being able to ship the SDK features and test utilities without worrying about the protocol behavior changes.

While I don't mind this PR, I don't quite understand the motivation behind not enabling testutils in snapshot, specifically this part:

today the only uses of the library that use that functionality is when the soroban-sdk is also being used with testutils enabled, but it is feasible that we may want to use the functionality in situations where it doesn't make sense to run the Host in testutils mode.

Isn't the snapshot a test utility as a whole? Also, I can't imagine a scenario where host doesn't run in testutils mode and the snapshot is used. Getting the intermediate stored entries is not something that Host is meant to do in the normal operation mode (which is why I've moved the function into test utils in the first place).

[leighmcculloch]

Rust features are ideally additive, they're really designed around being additive. The compiler can enable them even for crates that don't request them, so the assumption in the tooling is that they're additive. That becomes a problem when the testutils feature permeating across so many crates, and traits also change depending on the feature.

The stellar-cli workspace includes the soroban-sdk without testutils, and the soroban-ledger-snapshot without needing the Host functionality. That's why I've removed the unconditional inclusion of testutils on soroban-legder-snapshot, because we can't enable the soroban-sdk with testutils in its use cases in the stellar-cli workspace.

[dmkozh]

Rust features are ideally additive, they're really designed around being additive. The compiler can enable them even for crates that don't request them, so the assumption in the tooling is that they're additive. That becomes a problem when the testutils feature permeating across so many crates, and traits also change depending on the feature.

Sorry, I'm not sure I understand the sentiment here. I agree that changing the traits is not ideal and we need to break them down instead. But otherwise, I don't see what's wrong with passing the feature around for crates that actually need it. If that's about the name, then we could name it 'dev_mode' or something like that in order to not imply just the testing use cases.

The stellar-cli workspace includes the soroban-sdk without testutils, and the soroban-ledger-snapshot without needing the Host functionality.

The CLI->SDK dependency doesn't seem necessary at all, I can only see the SDK crate itself being used for the XDR dependencies, which is actually wrong. As for the snapshot dependency, doesn't that mean that snapshot should have a feature for the host functionality? (in case if we care about reducing the amount of dependencies, that is).