Skip to content

Commit ac4ea91

Browse files
Fix testutils incompatibility with soroban-ledger-snapshot (#1476)
### 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: - stellar/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
1 parent 9653830 commit ac4ea91

File tree

3 files changed

+10
-6
lines changed

3 files changed

+10
-6
lines changed

soroban-ledger-snapshot/Cargo.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ edition = "2021"
1111
rust-version.workspace = true
1212

1313
[dependencies]
14-
soroban-env-host = { workspace = true, features = ["testutils"] }
14+
soroban-env-host = { workspace = true }
1515
soroban-env-common = {workspace = true, features = ["serde"]}
1616
serde = { version = "1.0.0", features = ["derive"] }
1717
serde_with = { version = "3.4.0", features = ["hex"] }
@@ -20,3 +20,6 @@ thiserror = "1.0"
2020

2121
[dev-dependencies]
2222
pretty_assertions = "1.2.1"
23+
24+
[features]
25+
testutils = ["soroban-env-host/testutils"]

soroban-ledger-snapshot/src/lib.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{
99
use soroban_env_host::{
1010
storage::SnapshotSource,
1111
xdr::{LedgerEntry, LedgerKey},
12-
Host, HostError, LedgerInfo,
12+
HostError, LedgerInfo,
1313
};
1414

1515
#[derive(thiserror::Error, Debug)]
@@ -50,13 +50,14 @@ impl LedgerSnapshot {
5050
s
5151
}
5252

53-
/// Update the snapshot with the state within the given [`Host`].
53+
/// Update the snapshot with the state within the given [`soroban_env_host::Host`].
5454
///
5555
/// The ledger info of the host will overwrite the ledger info in the
5656
/// snapshot. The entries in the host's storage will overwrite entries in
5757
/// the snapshot. Existing entries in the snapshot that are untouched by the
5858
/// host will remain.
59-
pub fn update(&mut self, host: &Host) {
59+
#[cfg(feature = "testutils")]
60+
pub fn update(&mut self, host: &soroban_env_host::Host) {
6061
let _result = host.with_ledger_info(|li| {
6162
self.set_ledger_info(li.clone());
6263
Ok(())

soroban-sdk/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ ctor = { version = "0.2.9", optional = true }
4242
[dev-dependencies]
4343
soroban-sdk-macros = { workspace = true, features = ["testutils"] }
4444
soroban-env-host = { workspace = true, features = ["testutils"] }
45+
soroban-ledger-snapshot = { workspace = true, features = ["testutils"] }
4546
stellar-xdr = { workspace = true, features = ["curr", "std"] }
4647
soroban-spec = { workspace = true }
47-
soroban-ledger-snapshot = { workspace = true }
4848
ed25519-dalek = "2.0.0"
4949
rand = "0.8.5"
5050
ctor = "0.2.9"
@@ -58,7 +58,7 @@ expect-test = "1.4.1"
5858

5959
[features]
6060
alloc = []
61-
testutils = ["soroban-sdk-macros/testutils", "soroban-env-host/testutils", "dep:ed25519-dalek", "dep:arbitrary", "dep:derive_arbitrary", "dep:ctor", "dep:soroban-ledger-snapshot"]
61+
testutils = ["soroban-sdk-macros/testutils", "soroban-env-host/testutils", "soroban-ledger-snapshot/testutils", "dep:ed25519-dalek", "dep:arbitrary", "dep:derive_arbitrary", "dep:ctor", "dep:soroban-ledger-snapshot"]
6262
hazmat = []
6363
docs = []
6464

0 commit comments

Comments
 (0)